On Wed, 2025-02-26 at 12:07 +0000, Hangbin Liu wrote:
During bonding testing, we also found a case that would trigger the WARN_ON(xs->xso.real_dev != real_dev).
If we create active-backup mode bonding and create ipsec tunnel over bonding device, then remove bonding device. There is a possibility that the bond call bond_ipsec_del_sa_all() to delete the ipsec state first, then change active slave to another interface.
At the same time, ipsec gc was called and then bond_ipsec_free_sa(). This will cause the xs->xso.real_dev != active_slave as the failover triggered. The call traces looks like: [..]
This seems like another situation that could not simply fit 3. "if (xs->xso.real_dev != real_dev), goto out. I'm not sure what's the xs->km.state should be during xfrm_state_gc_task(). Is it also set to XFRM_STATE_DEAD, because I didn't see it.
XFRM_STATE_DEAD is set in __xfrm_state_delete() (and other places for what seems like error conditions), plus there's a WARN_ON(x->km.state != XFRM_STATE_DEAD) in __xfrm_state_destroy(). This last function is the main way xfrm states are destroyed, besides xfrm_dev_state_flush and xfrm_state_find (where xfrm_state_delete + xfrm_dev_state_free are used directly). So I am pretty sure that when bond .xdo_dev_state_free() is called via either one of the above three mechanisms, the state should be XFRM_STATE_DEAD. But maybe I'm missing something.
Especially if the bond change active slave and xfrm_state_gc_task() run in parallel, like
bond_ipsec_del_sa_all() xfrm_state_gc_task() xfrm_dev_state_free() bond_ipsec_free_sa() bond_ipsec_add_sa_all()
If the xs->km.state is not XFRM_STATE_DEAD. How to avoid the WARN_ON(xs->xso.real_dev != real_dev) in bond_ipsec_free_sa() and how to make bond_ipsec_add_sa_all() not added the entry again.
I am proposing you change this WARN_ON to an if, avoid calling xdo_dev_state_free on real_dev in that case and just remove the entry from bond->ipsec.
Cosmin.