linux-kernel - Re: [PATCH net-next V2 1/2] net/mlx5e: Support routed networks during IPsec MACs initialization

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] [day] [month] [year] [list]

Message-ID: <20250723071050.GL402218@unreal>
Date: Wed, 23 Jul 2025 10:10:50 +0300
From: Leon Romanovsky <leon@...nel.org>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
Cc: Tariq Toukan <tariqt@...dia.com>, Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Saeed Mahameed <saeed@...nel.org>, Gal Pressman <gal@...dia.com>,
	Saeed Mahameed <saeedm@...dia.com>, Mark Bloch <mbloch@...dia.com>,
	netdev@...r.kernel.org, linux-rdma@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Alexandre Cassen <acassen@...p.free.fr>
Subject: Re: [PATCH net-next V2 1/2] net/mlx5e: Support routed networks
 during IPsec MACs initialization

On Wed, Jul 23, 2025 at 08:52:09AM +0200, Michal Swiatkowski wrote:
> On Tue, Jul 22, 2025 at 05:23:47PM +0300, Tariq Toukan wrote:
> > From: Alexandre Cassen <acassen@...p.free.fr>
> > 
> > Remote IPsec tunnel endpoint may refer to a network segment that is
> > not directly connected to the host. In such a case, IPsec tunnel
> > endpoints are connected to a router and reachable via a routing path.
> > In IPsec packet offload mode, HW is initialized with the MAC address
> > of both IPsec tunnel endpoints.
> > 
> > Extend the current IPsec init MACs procedure to resolve nexthop for
> > routed networks. Direct neighbour lookup and probe is still used
> > for directly connected networks and as a fallback mechanism if fib
> > lookup fails.
> > 
> > Signed-off-by: Alexandre Cassen <acassen@...p.free.fr>
> > Signed-off-by: Leon Romanovsky <leonro@...dia.com>
> > Reviewed-by: Cosmin Ratiu <cratiu@...dia.com>
> > Signed-off-by: Tariq Toukan <tariqt@...dia.com>
> > ---
> >  .../mellanox/mlx5/core/en_accel/ipsec.c       | 82 ++++++++++++++++++-
> >  1 file changed, 80 insertions(+), 2 deletions(-)

<...>

> > @@ -274,18 +281,89 @@ static void mlx5e_ipsec_init_macs(struct mlx5e_ipsec_sa_entry *sa_entry,
> >  	case XFRM_DEV_OFFLOAD_IN:
> >  		src = attrs->dmac;
> >  		dst = attrs->smac;
> > -		pkey = &attrs->addrs.saddr.a4;
> > +
> > +		switch (addrs->family) {
> > +		case AF_INET:
> > +			fl4.flowi4_proto = x->sel.proto;
> > +			fl4.daddr = addrs->saddr.a4;
> > +			fl4.saddr = addrs->daddr.a4;
> > +			pkey = &addrs->saddr.a4;
> > +			break;
> > +		case AF_INET6:
> > +			fl6.flowi6_proto = x->sel.proto;
> > +			memcpy(fl6.daddr.s6_addr32, addrs->saddr.a6, 16);
> > +			memcpy(fl6.saddr.s6_addr32, addrs->daddr.a6, 16);
> > +			pkey = &addrs->saddr.a6;
> > +			break;
> > +		default:
> > +			return;
> > +		}
> >  		break;
> >  	case XFRM_DEV_OFFLOAD_OUT:
> >  		src = attrs->smac;
> >  		dst = attrs->dmac;
> > -		pkey = &attrs->addrs.daddr.a4;
> 
> Isn't it worth to move getting pkey to separate function? The switch is
> the same with OFFLOAD_IN and OFFLOAD_OUT.

The content of the switch is completely opposite, as an example for pkey:
In OFFLOAD_IN:
pkey = &addrs->...S...addr.a4;
pkey = &addrs->...S...addr.a6;

In OFFLOAD_OUT:
pkey = &addrs->...D...addr.a4;
pkey = &addrs->...D...addr.a6;

There is only one line which is common: "... = x->sel.proto;"

> 
> > +		switch (addrs->family) {
> > +		case AF_INET:
> > +			fl4.flowi4_proto = x->sel.proto;
> > +			fl4.daddr = addrs->daddr.a4;
> > +			fl4.saddr = addrs->saddr.a4;
> > +			pkey = &addrs->daddr.a4;
> > +			break;
> > +		case AF_INET6:
> > +			fl6.flowi6_proto = x->sel.proto;
> > +			memcpy(fl6.daddr.s6_addr32, addrs->daddr.a6, 16);
> > +			memcpy(fl6.saddr.s6_addr32, addrs->saddr.a6, 16);
> > +			pkey = &addrs->daddr.a6;
> > +			break;
> > +		default:
> > +			return;

<...>

> >  	n = neigh_lookup(&arp_tbl, pkey, netdev);
> >  	if (!n) {
> >  		n = neigh_create(&arp_tbl, pkey, netdev);
> 
> Code looks fine,
> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>

Thanks

> 
> > -- 
> > 2.31.1
>