mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/1] bonding: crypto offload enabled, non-offload slave failover, rekey failed
@ 2026-09-03 22:01 David Dai
  2026-09-04  6:11 ` Hangbin Liu
  2026-09-10  1:02 ` netdev-bot+sashiko
  0 siblings, 2 replies; 6+ messages in thread
From: David Dai @ 2026-09-03 22:01 UTC (permalink / raw)
  To: jv, andrew+netdev, davem, edumazet, kuba, pabeni
  Cc: netdev, linux-kernel, pradeeps, zdai

Create a bonding device (i.e. bond0) in active-backup mode, 2 slaves.
Active slave: offload capable interface (i.e. eth1), primary interface.
Backup slave: non-offload capable interface(i.e. eth2).
Configure strongswan service swantl.conf child SA "hw_offload = crypto"
Start strongswan service
IPSec Crytpo Offload is enabled on top of bond0. i.e. 
ip xfrm state |grep offload
        crypto offload parameters: dev bond0 dir out mode crypto
	crypto offload parameters: dev bond0 dir in mode crypto

Active slave eth1 takes adavantage of IPSec Crypto Offload capability. 

If active slave eth1 is down for any reason (i.e. eth1 link down):
ip link set down dev eth1
non-offload capable interface eth2 failover to becomes active slave. 
The existing SAs can continue use software IPsec after failover.
Traffic still keeps going properly.

However if eth1 link had not recovered yet, strongswan service does
new child SA rekey, or uses swanctl command to do new child SA rekey, 
it will fail because active slave eth2 doesn't support crypto offload.
In bond_ipsec_add_sa routine, it returns -EINVAL now, which is 
treated as fatal error by xfrm_dev_state_add routine in kernel xfrm.

To make the non-offload active slave survive the child SA rekey, need 
to make bond_ipsec_add_sa routine returns -EOPNOTSUPP instead when 
active slave doesn't support IPsec Crypto offload, the xfrm will 
gracefully fallback to create new SA using Software IPsec. 
Network traffic can keep going.

After offload capable interface eth1 link is up, becomes active slave,
next time strongswan child SA rekey will create a new SA which enables
crypto offload again.

This way, network traffic is never interrupted, always keeps going.

Signed-off-by: David Dai <zdai@linux.ibm.com>
Tested-by: David Dai <zdai@linux.ibm.com>
---
 drivers/net/bonding/bond_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index ef9eb0c53c66..79fc892dab07 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -490,7 +490,7 @@ static int bond_ipsec_add_sa(struct net_device *bond_dev,
 	    !real_dev->xfrmdev_ops->xdo_dev_state_add ||
 	    netif_is_bond_master(real_dev)) {
 		NL_SET_ERR_MSG_MOD(extack, "Slave does not support ipsec offload");
-		err = -EINVAL;
+		err = -EOPNOTSUPP;
 		goto out;
 	}
 
-- 
2.55.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] bonding: crypto offload enabled, non-offload slave failover, rekey failed
  2026-09-03 22:01 [PATCH 1/1] bonding: crypto offload enabled, non-offload slave failover, rekey failed David Dai
@ 2026-09-04  6:11 ` Hangbin Liu
  2026-09-10  8:03   ` Hangbin Liu
  2026-09-10  1:02 ` netdev-bot+sashiko
  1 sibling, 1 reply; 6+ messages in thread
From: Hangbin Liu @ 2026-09-04  6:11 UTC (permalink / raw)
  To: David Dai
  Cc: jv, andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
	linux-kernel, pradeeps

On Thu, Sep 03, 2026 at 05:01:23PM -0500, David Dai wrote:
> Create a bonding device (i.e. bond0) in active-backup mode, 2 slaves.
> Active slave: offload capable interface (i.e. eth1), primary interface.
> Backup slave: non-offload capable interface(i.e. eth2).
> Configure strongswan service swantl.conf child SA "hw_offload = crypto"
> Start strongswan service
> IPSec Crytpo Offload is enabled on top of bond0. i.e. 
> ip xfrm state |grep offload
>         crypto offload parameters: dev bond0 dir out mode crypto
> 	crypto offload parameters: dev bond0 dir in mode crypto
> 
> Active slave eth1 takes adavantage of IPSec Crypto Offload capability. 
> 
> If active slave eth1 is down for any reason (i.e. eth1 link down):
> ip link set down dev eth1
> non-offload capable interface eth2 failover to becomes active slave. 
> The existing SAs can continue use software IPsec after failover.
> Traffic still keeps going properly.
> 
> However if eth1 link had not recovered yet, strongswan service does
> new child SA rekey, or uses swanctl command to do new child SA rekey, 
> it will fail because active slave eth2 doesn't support crypto offload.
> In bond_ipsec_add_sa routine, it returns -EINVAL now, which is 
> treated as fatal error by xfrm_dev_state_add routine in kernel xfrm.
> 
> To make the non-offload active slave survive the child SA rekey, need 
> to make bond_ipsec_add_sa routine returns -EOPNOTSUPP instead when 
> active slave doesn't support IPsec Crypto offload, the xfrm will 
> gracefully fallback to create new SA using Software IPsec. 
> Network traffic can keep going.
> 
> After offload capable interface eth1 link is up, becomes active slave,
> next time strongswan child SA rekey will create a new SA which enables
> crypto offload again.
> 
> This way, network traffic is never interrupted, always keeps going.
> 
> Signed-off-by: David Dai <zdai@linux.ibm.com>
> Tested-by: David Dai <zdai@linux.ibm.com>
> ---
>  drivers/net/bonding/bond_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index ef9eb0c53c66..79fc892dab07 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -490,7 +490,7 @@ static int bond_ipsec_add_sa(struct net_device *bond_dev,
>  	    !real_dev->xfrmdev_ops->xdo_dev_state_add ||
>  	    netif_is_bond_master(real_dev)) {
>  		NL_SET_ERR_MSG_MOD(extack, "Slave does not support ipsec offload");
> -		err = -EINVAL;
> +		err = -EOPNOTSUPP;
>  		goto out;
>  	}
>  
> -- 
> 2.55.0
> 

The patch looks good to me. I am just not sure whether we should shorten
the description. Let's wait for others' opinions.

Thanks
Hangbin

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] bonding: crypto offload enabled, non-offload slave failover, rekey failed
  2026-09-03 22:01 [PATCH 1/1] bonding: crypto offload enabled, non-offload slave failover, rekey failed David Dai
  2026-09-04  6:11 ` Hangbin Liu
@ 2026-09-10  1:02 ` netdev-bot+sashiko
  2026-09-10 22:30   ` David Dai
  1 sibling, 1 reply; 6+ messages in thread
From: netdev-bot+sashiko @ 2026-09-10  1:02 UTC (permalink / raw)
  To: zdai
  Cc: jv, andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
	linux-kernel, pradeeps

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 2

- [Medium] Incomplete error-code mapping in `bond_ipsec_add_sa()`: the
  sibling failure path that fires when the bond has no current active…
- [Low] Missing `Fixes:` tag / unclear target tree for a user-visible
  behaviour fix.
- [Low] No in-tree test covers the behaviour this patch introduces.

--

bonding: crypto offload enabled, non-offload slave failover, rekey failed

This changes bond_ipsec_add_sa() to return -EOPNOTSUPP instead of -EINVAL
when the current active slave has no xdo_dev_state_add, so that xfrm falls
back to a software SA instead of failing the SA add outright.  The reported
symptom is a child SA rekey failing after failover from an offload capable
slave to a non-offload capable one.

No Link: tags were present in the commit header.

> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index a9bff7663eeca..de2489c3d9bf2 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -490,7 +490,7 @@ static int bond_ipsec_add_sa(struct net_device *bond_dev,
>  	    !real_dev->xfrmdev_ops->xdo_dev_state_add ||
>  	    netif_is_bond_master(real_dev)) {
>  		NL_SET_ERR_MSG_MOD(extack, "Slave does not support ipsec offload");
> -		err = -EINVAL;
> +		err = -EOPNOTSUPP;
>  		goto out;
>  	}

[Severity: Low]
Should this carry a Fixes: tag, and which tree does it target?

The change fixes a user visible failure (child SA rekey is rejected while a
non-offload slave is active), so it reads like a net fix rather than a
net-next cleanup, but neither a Fixes: tag nor a [PATCH net] /
[PATCH net-next] marker is present.

git blame on the changed line at the baseline points at 2aeeef906d5a52
("bonding: change ipsec_lock from spin lock to mutex"), which only moved the
line.  The enclosing capability check goes back to 18cb261afd7bf5
("bonding: support hardware crypto offload") by way of 907ed83a7583e8.  Would
one of those be the right anchor for stable backports?

Also, the Tested-by: line names the same person as the Signed-off-by:.  Is
that intentional?

[Severity: Medium]
Should the sibling "no active slave" path in the same function be converted
too?  Just above this hunk, bond_ipsec_add_sa() still returns a fatal errno
when there is no current active slave:

drivers/net/bonding/bond_main.c:bond_ipsec_add_sa() {
	slave = rcu_dereference(bond->curr_active_slave);
	real_dev = slave ? slave->dev : NULL;
	netdev_hold(real_dev, &tracker, GFP_ATOMIC);
	rcu_read_unlock();
	if (!real_dev) {
		err = -ENODEV;
		goto out;
	}
	...
}

The consumer only whitelists -EOPNOTSUPP for the silent software fallback:

net/xfrm/xfrm_device.c:xfrm_dev_state_add() {
	...
	if ((err != -EOPNOTSUPP && !is_packet_offload) || is_packet_offload) {
		NL_SET_ERR_MSG_WEAK(extack, "Device failed to offload this state");
		return err;
	}
	...
}

and xfrm_state_construct() then kills the freshly allocated state:

net/xfrm/xfrm_user.c:xfrm_state_construct() {
	...
	err = xfrm_dev_state_add(net, x, nla_data(attrs[XFRMA_OFFLOAD_DEV]), extack);
	if (err)
		goto error;
	...
error:
	x->km.state = XFRM_STATE_DEAD;
	xfrm_state_put(x);
}

So with crypto offload requested and -ENODEV returned, the NEWSA still fails
hard and no software SA is created.  That is reachable when every slave is
down, in the window where bond_select_active_slave() has installed a NULL
curr_active_slave, or trivially with a bond that has no slaves yet:

	ip link add bond0 type bond
	ip xfrm state add ... offload dev bond0 dir out

Given the commit message says "network traffic is never interrupted, always
keeps going", does the -ENODEV case leave the same failure mode reachable in
a narrower window?

[Severity: Low]
Is there a plan to extend the existing selftest to cover the new fallback?

tools/testing/selftests/drivers/net/bonding/bond_ipsec_offload.sh enslaves
two netdevsim ports in setup_bond():

	echo "0 2" | ip netns exec "$ns" tee /sys/bus/netdevsim/new_device >/dev/null

Both provide xdo_dev_state_add, and the failover leg only moves the active
slave between those two before re-running test_offload().  No leg enslaves a
veth or dummy device, fails over to it, and then adds a new SA to check that
it is accepted with software fallback.

As it stands the script passes identically before and after this change, and
would keep passing if the fallback later regressed back to a hard error.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903220123.475685-1-zdai%40linux.ibm.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] bonding: crypto offload enabled, non-offload slave failover, rekey failed
  2026-09-04  6:11 ` Hangbin Liu
@ 2026-09-10  8:03   ` Hangbin Liu
  2026-09-10 14:23     ` David Dai
  0 siblings, 1 reply; 6+ messages in thread
From: Hangbin Liu @ 2026-09-10  8:03 UTC (permalink / raw)
  To: David Dai
  Cc: jv, andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
	linux-kernel, pradeeps

On Fri, Sep 04, 2026 at 02:11:57PM +0800, Hangbin Liu wrote:
> On Thu, Sep 03, 2026 at 05:01:23PM -0500, David Dai wrote:
> > Create a bonding device (i.e. bond0) in active-backup mode, 2 slaves.
> > Active slave: offload capable interface (i.e. eth1), primary interface.
> > Backup slave: non-offload capable interface(i.e. eth2).
> > Configure strongswan service swantl.conf child SA "hw_offload = crypto"
> > Start strongswan service
> > IPSec Crytpo Offload is enabled on top of bond0. i.e. 
> > ip xfrm state |grep offload
> >         crypto offload parameters: dev bond0 dir out mode crypto
> > 	crypto offload parameters: dev bond0 dir in mode crypto
> > 
> > Active slave eth1 takes adavantage of IPSec Crypto Offload capability. 
> > 
> > If active slave eth1 is down for any reason (i.e. eth1 link down):
> > ip link set down dev eth1
> > non-offload capable interface eth2 failover to becomes active slave. 
> > The existing SAs can continue use software IPsec after failover.
> > Traffic still keeps going properly.
> > 
> > However if eth1 link had not recovered yet, strongswan service does
> > new child SA rekey, or uses swanctl command to do new child SA rekey, 
> > it will fail because active slave eth2 doesn't support crypto offload.
> > In bond_ipsec_add_sa routine, it returns -EINVAL now, which is 
> > treated as fatal error by xfrm_dev_state_add routine in kernel xfrm.
> > 
> > To make the non-offload active slave survive the child SA rekey, need 
> > to make bond_ipsec_add_sa routine returns -EOPNOTSUPP instead when 
> > active slave doesn't support IPsec Crypto offload, the xfrm will 
> > gracefully fallback to create new SA using Software IPsec. 
> > Network traffic can keep going.
> > 
> > After offload capable interface eth1 link is up, becomes active slave,
> > next time strongswan child SA rekey will create a new SA which enables
> > crypto offload again.
> > 
> > This way, network traffic is never interrupted, always keeps going.
> > 
> > Signed-off-by: David Dai <zdai@linux.ibm.com>
> > Tested-by: David Dai <zdai@linux.ibm.com>
> > ---
> >  drivers/net/bonding/bond_main.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> > index ef9eb0c53c66..79fc892dab07 100644
> > --- a/drivers/net/bonding/bond_main.c
> > +++ b/drivers/net/bonding/bond_main.c
> > @@ -490,7 +490,7 @@ static int bond_ipsec_add_sa(struct net_device *bond_dev,
> >  	    !real_dev->xfrmdev_ops->xdo_dev_state_add ||
> >  	    netif_is_bond_master(real_dev)) {
> >  		NL_SET_ERR_MSG_MOD(extack, "Slave does not support ipsec offload");
> > -		err = -EINVAL;
> > +		err = -EOPNOTSUPP;
> >  		goto out;
> >  	}
> >  
> > -- 
> > 2.55.0
> > 
> 
> The patch looks good to me. I am just not sure whether we should shorten
> the description. Let's wait for others' opinions.
> 

BTW, please add

Fixes: 18cb261afd7b ("bonding: support hardware encryption offload to slaves")

if there is a next version.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] bonding: crypto offload enabled, non-offload slave failover, rekey failed
  2026-09-10  8:03   ` Hangbin Liu
@ 2026-09-10 14:23     ` David Dai
  0 siblings, 0 replies; 6+ messages in thread
From: David Dai @ 2026-09-10 14:23 UTC (permalink / raw)
  To: Hangbin Liu
  Cc: jv, andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
	linux-kernel, pradeeps

On Thu, 2026-09-10 at 16:03 +0800, Hangbin Liu wrote:
> On Fri, Sep 04, 2026 at 02:11:57PM +0800, Hangbin Liu wrote:
> > On Thu, Sep 03, 2026 at 05:01:23PM -0500, David Dai wrote:
> > > Create a bonding device (i.e. bond0) in active-backup mode, 2
> > > slaves.
> > > Active slave: offload capable interface (i.e. eth1), primary
> > > interface.
> > > Backup slave: non-offload capable interface(i.e. eth2).
> > > Configure strongswan service swantl.conf child SA "hw_offload =
> > > crypto"
> > > Start strongswan service
> > > IPSec Crytpo Offload is enabled on top of bond0. i.e. 
> > > ip xfrm state |grep offload
> > >         crypto offload parameters: dev bond0 dir out mode crypto
> > > 	crypto offload parameters: dev bond0 dir in mode crypto
> > > 
> > > Active slave eth1 takes adavantage of IPSec Crypto Offload
> > > capability. 
> > > 
> > > If active slave eth1 is down for any reason (i.e. eth1 link
> > > down):
> > > ip link set down dev eth1
> > > non-offload capable interface eth2 failover to becomes active
> > > slave. 
> > > The existing SAs can continue use software IPsec after failover.
> > > Traffic still keeps going properly.
> > > 
> > > However if eth1 link had not recovered yet, strongswan service
> > > does
> > > new child SA rekey, or uses swanctl command to do new child SA
> > > rekey, 
> > > it will fail because active slave eth2 doesn't support crypto
> > > offload.
> > > In bond_ipsec_add_sa routine, it returns -EINVAL now, which is 
> > > treated as fatal error by xfrm_dev_state_add routine in kernel
> > > xfrm.
> > > 
> > > To make the non-offload active slave survive the child SA rekey,
> > > need 
> > > to make bond_ipsec_add_sa routine returns -EOPNOTSUPP instead
> > > when 
> > > active slave doesn't support IPsec Crypto offload, the xfrm will 
> > > gracefully fallback to create new SA using Software IPsec. 
> > > Network traffic can keep going.
> > > 
> > > After offload capable interface eth1 link is up, becomes active
> > > slave,
> > > next time strongswan child SA rekey will create a new SA which
> > > enables
> > > crypto offload again.
> > > 
> > > This way, network traffic is never interrupted, always keeps
> > > going.
> > > 
> > > Signed-off-by: David Dai <zdai@linux.ibm.com>
> > > Tested-by: David Dai <zdai@linux.ibm.com>
> > > ---
> > >  drivers/net/bonding/bond_main.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/bonding/bond_main.c
> > > b/drivers/net/bonding/bond_main.c
> > > index ef9eb0c53c66..79fc892dab07 100644
> > > --- a/drivers/net/bonding/bond_main.c
> > > +++ b/drivers/net/bonding/bond_main.c
> > > @@ -490,7 +490,7 @@ static int bond_ipsec_add_sa(struct
> > > net_device *bond_dev,
> > >  	    !real_dev->xfrmdev_ops->xdo_dev_state_add ||
> > >  	    netif_is_bond_master(real_dev)) {
> > >  		NL_SET_ERR_MSG_MOD(extack, "Slave does not
> > > support ipsec offload");
> > > -		err = -EINVAL;
> > > +		err = -EOPNOTSUPP;
> > >  		goto out;
> > >  	}
> > >  
> > > -- 
> > > 2.55.0
> > > 
> > 
> > The patch looks good to me. I am just not sure whether we should
> > shorten
> > the description. Let's wait for others' opinions.
> > 
> 
> BTW, please add
> 
> Fixes: 18cb261afd7b ("bonding: support hardware encryption offload to
> slaves")
> 
> if there is a next version.
Thanks for your comment!
I'll add the Fixes line in my next v2 patch submission.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] bonding: crypto offload enabled, non-offload slave failover, rekey failed
  2026-09-10  1:02 ` netdev-bot+sashiko
@ 2026-09-10 22:30   ` David Dai
  0 siblings, 0 replies; 6+ messages in thread
From: David Dai @ 2026-09-10 22:30 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: jv, andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
	linux-kernel, pradeeps, hangbin.liu

On Thu, 2026-09-10 at 01:02 +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 2
> 
> - [Medium] Incomplete error-code mapping in `bond_ipsec_add_sa()`:
> the
>   sibling failure path that fires when the bond has no current
> active…
> - [Low] Missing `Fixes:` tag / unclear target tree for a user-visible
>   behaviour fix.
> - [Low] No in-tree test covers the behaviour this patch introduces.
> 
> --
> 
> bonding: crypto offload enabled, non-offload slave failover, rekey
> failed
> 
> This changes bond_ipsec_add_sa() to return -EOPNOTSUPP instead of -
> EINVAL
> when the current active slave has no xdo_dev_state_add, so that xfrm
> falls
> back to a software SA instead of failing the SA add outright.  The
> reported
> symptom is a child SA rekey failing after failover from an offload
> capable
> slave to a non-offload capable one.
> 
> No Link: tags were present in the commit header.
> 
> > diff --git a/drivers/net/bonding/bond_main.c
> > b/drivers/net/bonding/bond_main.c
> > index a9bff7663eeca..de2489c3d9bf2 100644
> > --- a/drivers/net/bonding/bond_main.c
> > +++ b/drivers/net/bonding/bond_main.c
> > @@ -490,7 +490,7 @@ static int bond_ipsec_add_sa(struct net_device
> > *bond_dev,
> >  	    !real_dev->xfrmdev_ops->xdo_dev_state_add ||
> >  	    netif_is_bond_master(real_dev)) {
> >  		NL_SET_ERR_MSG_MOD(extack, "Slave does not support
> > ipsec offload");
> > -		err = -EINVAL;
> > +		err = -EOPNOTSUPP;
> >  		goto out;
> >  	}
> 
> [Severity: Low]
> Should this carry a Fixes: tag, and which tree does it target?

Yes, I need to add this line when I resubmit it in v2 with:
Fixes: 18cb261afd7b ("bonding: support hardware encryption offload to
slaves")

> 
> The change fixes a user visible failure (child SA rekey is rejected
> while a
> non-offload slave is active), so it reads like a net fix rather than
> a
> net-next cleanup, but neither a Fixes: tag nor a [PATCH net] /
> [PATCH net-next] marker is present.
> 
Yes, it's a net fix. I need to use this in the next submit:
[PATCH net v2 1/1] bonding: crypto offload enabled, non-offload slave
failover, rekey failed

> git blame on the changed line at the baseline points at
> 2aeeef906d5a52
> ("bonding: change ipsec_lock from spin lock to mutex"), which only
> moved the
> line.  The enclosing capability check goes back to 18cb261afd7bf5
> ("bonding: support hardware crypto offload") by way of
> 907ed83a7583e8.  Would
> one of those be the right anchor for stable backports?
> 
> Also, the Tested-by: line names the same person as the Signed-off-
> by:.  Is
> that intentional?
> 
My original intention is to imply this patch has been tested before
submission. So it was intentional. If it's redundant, I can remove it.

> [Severity: Medium]
> Should the sibling "no active slave" path in the same function be
> converted
> too?  Just above this hunk, bond_ipsec_add_sa() still returns a fatal
> errno
> when there is no current active slave:
> 
> drivers/net/bonding/bond_main.c:bond_ipsec_add_sa() {
> 	slave = rcu_dereference(bond->curr_active_slave);
> 	real_dev = slave ? slave->dev : NULL;
> 	netdev_hold(real_dev, &tracker, GFP_ATOMIC);
> 	rcu_read_unlock();
> 	if (!real_dev) {
> 		err = -ENODEV;
> 		goto out;
> 	}
> 	...
> }
> 
> The consumer only whitelists -EOPNOTSUPP for the silent software
> fallback:
> 
> net/xfrm/xfrm_device.c:xfrm_dev_state_add() {
> 	...
> 	if ((err != -EOPNOTSUPP && !is_packet_offload) ||
> is_packet_offload) {
> 		NL_SET_ERR_MSG_WEAK(extack, "Device failed to
> offload this state");
> 		return err;
> 	}
> 	...
> }
> 
> and xfrm_state_construct() then kills the freshly allocated state:
> 
> net/xfrm/xfrm_user.c:xfrm_state_construct() {
> 	...
> 	err = xfrm_dev_state_add(net, x,
> nla_data(attrs[XFRMA_OFFLOAD_DEV]), extack);
> 	if (err)
> 		goto error;
> 	...
> error:
> 	x->km.state = XFRM_STATE_DEAD;
> 	xfrm_state_put(x);
> }
> 
> So with crypto offload requested and -ENODEV returned, the NEWSA
> still fails
> hard and no software SA is created.  That is reachable when every
> slave is
> down, in the window where bond_select_active_slave() has installed a
> NULL
> curr_active_slave, or trivially with a bond that has no slaves yet:
> 
> 	ip link add bond0 type bond
> 	ip xfrm state add ... offload dev bond0 dir out
> 
> Given the commit message says "network traffic is never interrupted,
> always
> keeps going", does the -ENODEV case leave the same failure mode
> reachable in
> a narrower window?
> 
Before my patch, bond0 had ipsec crypto offload enabled,
non-offload capable eth2 failover to be active slave, the ipsec traffic
will use SW path, no interruption, no problem.
However when strongswan service, or swanctl tool does child SA rekey,
it failed.

My patch deals with the child SA rekey failure issue when non-offload
capable eth2 is active slave.
By changing return code from -EINVAL to -EOPNOTSUPP, xfrm will then
create a SA without crypto offload when non-offload capable eth2 is the
active slave.
To keep the traffic going, at any time, either one of the slave
(offload capable interface eth1 or non-offload capable interface eth2)
is up at any give time.
So real_dev is a valid pointer. Traffic is not interrupted.

I need to delete the last line in my v1 patch comment to avoid
confusion:
"This way, network traffic is never interrupted, always keeps going.".

W.r.t. "ip xfrm state add ... offload dev bond0 dir out",
my understanding is child SA created by ip xfrm command will never
expire. There is no need/way to do child SA rekey either.

W.r.t. the sibling check that real_slave is NULL condition,
If real_dev is NULL, no slave is available to handle traffic.
In my opinion, it's a fatal error. Setting err to -ENODEV is correct.
If it happens, child SA rekey should fail hard.

> [Severity: Low]
> Is there a plan to extend the existing selftest to cover the new
> fallback?
> 
No, not in this patch submission.
The current bond_ipsec_offload.sh uses "ip xfrm state add ..." to set
child SA keys, which will never expire. ip command can't do child SA
reky.

This patch deals with child SA rekey when non-offload capable interface
eth2 is the active slave, which requires ipsec applications like
libreswan, strongswan, etc...
Different linux distros have different ipsec applications.
If I can find a universal way to be tested in the selftest later,
will address it in a separate patch.

> tools/testing/selftests/drivers/net/bonding/bond_ipsec_offload.sh
> enslaves
> two netdevsim ports in setup_bond():
> 
> 	echo "0 2" | ip netns exec "$ns" tee
> /sys/bus/netdevsim/new_device >/dev/null
> 
> Both provide xdo_dev_state_add, and the failover leg only moves the
> active
> slave between those two before re-running test_offload().  No leg
> enslaves a
> veth or dummy device, fails over to it, and then adds a new SA to
> check that
> it is accepted with software fallback.
> 
> As it stands the script passes identically before and after this
> change, and
> would keep passing if the fallback later regressed back to a hard
> error.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-10 22:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-03 22:01 [PATCH 1/1] bonding: crypto offload enabled, non-offload slave failover, rekey failed David Dai
2026-09-04  6:11 ` Hangbin Liu
2026-09-10  8:03   ` Hangbin Liu
2026-09-10 14:23     ` David Dai
2026-09-10  1:02 ` netdev-bot+sashiko
2026-09-10 22:30   ` David Dai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®