On Wed, Mar 05, 2025 at 04:12:18PM +0000, Cosmin Ratiu wrote:
+++ b/drivers/net/bonding/bond_main.c @@ -613,8 +613,11 @@ static void bond_ipsec_del_sa_all(struct bonding *bond) mutex_lock(&bond->ipsec_lock); list_for_each_entry(ipsec, &bond->ipsec_list, list) {
if (!ipsec->xs->xso.real_dev)
spin_lock(&ipsec->x->lock);
if (!ipsec->xs->xso.real_dev) {
spin_unlock(&ipsec->x->lock); continue;
}
if (!real_dev->xfrmdev_ops || !real_dev->xfrmdev_ops->xdo_dev_state_delete || @@ -622,12 +625,16 @@ static void bond_ipsec_del_sa_all(struct bonding *bond) slave_warn(bond_dev, real_dev, "%s: no slave xdo_dev_state_delete\n", __func__);
} else {
real_dev->xfrmdev_ops-
xdo_dev_state_delete(real_dev, ipsec->xs);
if (real_dev->xfrmdev_ops->xdo_dev_state_free)
real_dev->xfrmdev_ops-
xdo_dev_state_free(ipsec->xs);
ipsec->xs->xso.real_dev = NULL;
spin_unlock(&ipsec->x->lock);
continue; }
real_dev->xfrmdev_ops->xdo_dev_state_delete(real_dev,
ipsec->xs);
ipsec->xs->xso.real_dev = NULL;
/* Unlock before freeing device state, it could sleep.
*/
spin_unlock(&ipsec->x->lock);
if (real_dev->xfrmdev_ops->xdo_dev_state_free)
real_dev->xfrmdev_ops-
xdo_dev_state_free(ipsec->xs);
BTW, with setting real_dev = NULL here, I think
To fix that, these entries should be freed here and the WARN_ON in bond_ipsec_free_sa() should be converted to an if...goto out, so that bond_ipsec_free_sa() calls would hit one of these conditions:
- "if (!slave)", when no active device exists.
- "if (!xs->xso.real_dev)", when xdo_dev_state_add() failed.
- "if (xs->xso.real_dev != real_dev)", when a DEAD xs was already
freed by bond_ipsec_del_sa_all() migration to a new device. In all 3 cases, xdo_dev_state_free() shouldn't be called, only xs removed from the bond->ipsec list.
The if (xs->xso.real_dev != real_dev) should never happen again. As the real_dev = NULL, it will trigger 2 "if (!xs->xso.real_dev)" directly.
And in bond_ipsec_add_sa_all(), it will set ipsec->xs->xso.real_dev = real_dev, which the active slave already finished changing.
Thanks Hangbin