[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230420115243.GC4423@unreal>
Date: Thu, 20 Apr 2023 14:52:43 +0300
From: Leon Romanovsky <leon@...nel.org>
To: Simon Horman <simon.horman@...igine.com>
Cc: Jakub Kicinski <kuba@...nel.org>, Emeel Hakim <ehakim@...dia.com>,
Eric Dumazet <edumazet@...gle.com>, netdev@...r.kernel.org,
Paolo Abeni <pabeni@...hat.com>, Raed Salem <raeds@...dia.com>,
Saeed Mahameed <saeedm@...dia.com>,
Steffen Klassert <steffen.klassert@...unet.com>
Subject: Re: [PATCH net-next 3/5] net/mlx5e: Compare all fields in IPv6
address
On Thu, Apr 20, 2023 at 01:09:23PM +0200, Simon Horman wrote:
> On Thu, Apr 20, 2023 at 11:02:49AM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@...dia.com>
> >
> > Fix size argument in memcmp to compare whole IPv6 address.
> >
> > Fixes: b3beba1fb404 ("net/mlx5e: Allow policies with reqid 0, to support IKE policy holes")
> > Reviewed-by: Raed Salem <raeds@...dia.com>
> > Reviewed-by: Emeel Hakim <ehakim@...dia.com>
> > Signed-off-by: Leon Romanovsky <leonro@...dia.com>
> > ---
> > drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
> > index f7f7c09d2b32..4e9887171508 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
> > @@ -287,7 +287,7 @@ static inline bool addr6_all_zero(__be32 *addr6)
> > {
> > static const __be32 zaddr6[4] = {};
> >
> > - return !memcmp(addr6, zaddr6, sizeof(*zaddr6));
> > + return !memcmp(addr6, zaddr6, sizeof(zaddr6));
>
> 1. Perhaps array_size() is appropriate here?
It is overkill here, sizeof(zaddr6) is constant and can't overflow.
238 /**
239 * array_size() - Calculate size of 2-dimensional array.
240 * @a: dimension one
241 * @b: dimension two
242 *
243 * Calculates size of 2-dimensional array: @a * @b.
244 *
245 * Returns: number of bytes needed to represent the array or SIZE_MAX on
246 * overflow.
247 */
248 #define array_size(a, b) size_mul(a, b)
> 2. It's a shame that ipv6_addr_any() or some other common helper
> can't be used.
I didn't use ipv6_addr_any() as it required from me to cast "__be32 *addr6"
to be "struct in6_addr *" just to replace one line memcmp to another one
line function.
Do you want me to post this code instead?
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
index 55b38544422f..a7c8e38658a0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
@@ -945,7 +945,8 @@ static int mlx5e_xfrm_validate_policy(struct mlx5_core_dev *mdev,
}
if (!x->xfrm_vec[0].reqid && sel->proto == IPPROTO_IP &&
- addr6_all_zero(sel->saddr.a6) && addr6_all_zero(sel->daddr.a6)) {
+ ipv6_addr_any((struct in6_addr *)sel->saddr.a6) &&
+ ipv6_addr_any((struct in6_addr *)sel->daddr.a6)) {
NL_SET_ERR_MSG_MOD(extack, "Unsupported policy with reqid 0 without at least one of upper protocol or ip addr(s) different than 0");
return -EINVAL;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
index 4e9887171508..097001ce5dc1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
@@ -283,12 +283,6 @@ mlx5e_ipsec_pol2dev(struct mlx5e_ipsec_pol_entry *pol_entry)
return pol_entry->ipsec->mdev;
}
-static inline bool addr6_all_zero(__be32 *addr6)
-{
- static const __be32 zaddr6[4] = {};
-
- return !memcmp(addr6, zaddr6, sizeof(zaddr6));
-}
#else
static inline void mlx5e_ipsec_init(struct mlx5e_priv *priv)
{
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
index dbe87bf89c0d..e48113923c12 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_fs.c
@@ -721,7 +721,8 @@ static void setup_fte_addr4(struct mlx5_flow_spec *spec, __be32 *saddr,
static void setup_fte_addr6(struct mlx5_flow_spec *spec, __be32 *saddr,
__be32 *daddr)
{
- if (addr6_all_zero(saddr) && addr6_all_zero(daddr))
+ if (ipv6_addr_any((struct in6_addr *)saddr) &&
+ ipv6_addr_any((struct in6_addr *)daddr))
return;
spec->match_criteria_enable |= MLX5_MATCH_OUTER_HEADERS;
@@ -729,14 +730,14 @@ static void setup_fte_addr6(struct mlx5_flow_spec *spec, __be32 *saddr,
MLX5_SET_TO_ONES(fte_match_param, spec->match_criteria, outer_headers.ip_version);
MLX5_SET(fte_match_param, spec->match_value, outer_headers.ip_version, 6);
- if (!addr6_all_zero(saddr)) {
+ if (!ipv6_addr_any((struct in6_addr *)saddr)) {
memcpy(MLX5_ADDR_OF(fte_match_param, spec->match_value,
outer_headers.src_ipv4_src_ipv6.ipv6_layout.ipv6), saddr, 16);
memset(MLX5_ADDR_OF(fte_match_param, spec->match_criteria,
outer_headers.src_ipv4_src_ipv6.ipv6_layout.ipv6), 0xff, 16);
}
- if (!addr6_all_zero(daddr)) {
+ if (!ipv6_addr_any((struct in6_addr *)daddr)) {
memcpy(MLX5_ADDR_OF(fte_match_param, spec->match_value,
outer_headers.dst_ipv4_dst_ipv6.ipv6_layout.ipv6), daddr, 16);
memset(MLX5_ADDR_OF(fte_match_param, spec->match_criteria,
Thanks
>
> > }
> > #else
> > static inline void mlx5e_ipsec_init(struct mlx5e_priv *priv)
> > --
> > 2.40.0
> >
Powered by blists - more mailing lists