netdev - Re: [PATCH] [PATH xfrm offload] xfrm: bonding: Add xfrm packet offload for active-backup mode

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

Message-ID: <aGTlcAOa6_ItYemu@fedora>
Date: Wed, 2 Jul 2025 07:53:20 +0000
From: Hangbin Liu <liuhangbin@...il.com>
To: Erwan Dufour <erwan.dufour@...hings.com>
Cc: Erwan Dufour <mrarmonius@...il.com>, netdev@...r.kernel.org,
	steffen.klassert@...unet.com, herbert@...dor.apana.org.au,
	davem@...emloft.net, jv@...sburgh.net, saeedm@...dia.com,
	tariqt@...dia.com, Cosmin Ratiu <cratiu@...dia.com>
Subject: Re: [PATCH] [PATH xfrm offload] xfrm: bonding: Add xfrm packet
 offload for active-backup mode

Hi Erwan,

On Tue, Jul 01, 2025 at 07:29:48PM +0200, Erwan Dufour wrote:
> Hi Liu,
> Thank you for the link.
> The new patch with the good tab size can be found at the end of this email.
> 
> Hmm, I'm not very familiar with IPsec. I thought we can config xfrm state
> > and
> > policy on the interface at same time. Need others review this part.
> 
> The ip XFRM state and ip XFRM policy that you see when you use iproute with
> 'ip xfrm state' or 'ip xfrm policy' command in cli may be on the same
> interface.
> But in the code here, the struct bond_ipsec is just an element of a list
> used to store all the SAs and SPs linked to this device bond.
> So this structure allows us to remove the SA and SP offloads from the old
> primary slave and add them to the new one during primary current slave
> exchanges. (function bond_change_active_slave() )
> The bond_ipsec struct is an element of a list which stores all the SAs and
> SPs of the device bond.
> So every time we add an SA or SP to our bond, we create a new bond_ipsec
> object and add it to our list. This is why, in our structure, we cannot
> have an SA and an SP in the same bond_ipsec object.

Thanks for your explanation. Unfortunately，the alignment still not works.

e.g.

> 
> -		if (real_dev->xfrmdev_ops->xdo_dev_state_add(real_dev,
> -							     ipsec->xs, NULL)) {

Here the ipsec->xs is aligned with real_dev.

> -			slave_warn(bond_dev, real_dev, "%s: failed to add SA\n", __func__);
> -			continue;
> -		}
> +			if (real_dev->xfrmdev_ops->xdo_dev_state_add(real_dev,
> +									ipsec->xs, NULL)) {

But here, ipsec->xs is not aligned with real_dev.
If the code cannot be aligned properly using tabs alone, you can use spaces
to complete the alignment.

Thanks
Hangbin