linux-kernel - Re: [PATCH net-next 6/6] net/mlx5e: Properly match IPsec subnet addresses

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z8aw1gn5iFNiSxd3@mev-dev.igk.intel.com>
Date: Tue, 4 Mar 2025 08:50:46 +0100
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: Tariq Toukan <tariqt@...dia.com>
Cc: "David S. Miller" <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	Saeed Mahameed <saeedm@...dia.com>, Gal Pressman <gal@...dia.com>,
	Leon Romanovsky <leonro@...dia.com>,
	Leon Romanovsky <leon@...nel.org>, netdev@...r.kernel.org,
	linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 6/6] net/mlx5e: Properly match IPsec subnet
 addresses

On Wed, Feb 26, 2025 at 01:47:52PM +0200, Tariq Toukan wrote:
> From: Leon Romanovsky <leonro@...dia.com>
> 
> Existing match criteria didn't allow to match whole subnet and
> only by specific addresses only. This caused to tunnel mode do not
> forward such traffic through relevant SA.
> 
> In tunnel mode, policies look like this:
> src 192.169.0.0/16 dst 192.169.0.0/16
>         dir out priority 383615 ptype main
>         tmpl src 192.169.101.2 dst 192.169.101.1
>                 proto esp spi 0xc5141c18 reqid 1 mode tunnel
>         crypto offload parameters: dev eth2 mode packet
> 
> In this case, the XFRM core code handled all subnet calculations and
> forwarded network address to the drivers e.g. 192.169.0.0.
> 
> For mlx5 devices, there is a need to set relevant prefix e.g. 0xFFFF00
> to perform flow steering match operation.
> 
> Signed-off-by: Leon Romanovsky <leonro@...dia.com>
> Signed-off-by: Tariq Toukan <tariqt@...dia.com>
> ---
>  .../mellanox/mlx5/core/en_accel/ipsec.c       | 49 +++++++++++++++++++
>  .../mellanox/mlx5/core/en_accel/ipsec.h       |  9 +++-
>  .../mellanox/mlx5/core/en_accel/ipsec_fs.c    | 20 +++++---
>  3 files changed, 69 insertions(+), 9 deletions(-)
> 

[...]

>  
> +static __be32 word_to_mask(int prefix)
> +{
> +	if (prefix < 0)
> +		return 0;
> +
> +	if (!prefix || prefix > 31)
> +		return cpu_to_be32(0xFFFFFFFF);
> +
> +	return cpu_to_be32(((1U << prefix) - 1) << (32 - prefix));

Isn't it GENMASK(31, 32 - prefix)? I don't know if it is preferable to
use this macro in such place.

> +}
> +
> +static void mlx5e_ipsec_policy_mask(struct mlx5e_ipsec_addr *addrs,
> +				    struct xfrm_selector *sel)
> +{
> +	int i;
> +
> +	if (addrs->family == AF_INET) {
> +		addrs->smask.m4 = word_to_mask(sel->prefixlen_s);
> +		addrs->saddr.a4 &= addrs->smask.m4;
> +		addrs->dmask.m4 = word_to_mask(sel->prefixlen_d);
> +		addrs->daddr.a4 &= addrs->dmask.m4;
> +		return;
> +	}
> +
> +	for (i = 0; i < 4; i++) {
> +		if (sel->prefixlen_s != 32 * i)
> +			addrs->smask.m6[i] =
> +				word_to_mask(sel->prefixlen_s - 32 * i);
> +		addrs->saddr.a6[i] &= addrs->smask.m6[i];
> +
> +		if (sel->prefixlen_d != 32 * i)
> +			addrs->dmask.m6[i] =
> +				word_to_mask(sel->prefixlen_d - 32 * i);
> +		addrs->daddr.a6[i] &= addrs->dmask.m6[i];
> +	}
> +}
> +

[...]

Looks fine,
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>

Thanks