On Wed, Feb 26, 2025 at 01:47:52PM +0200, Tariq Toukan wrote:
> From: Leon Romanovsky <leonro@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Signed-off-by: Tariq Toukan <tariqt@xxxxxxxxxx>
> ---
> .../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@xxxxxxxxxxxxxxx>
Thanks
