On Tue, 2025-03-04 at 09:18 +0000, Hangbin Liu wrote:
Just to make sure I added the lock in correct place, would you please help confirm.
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index e85878b12376..c59ad3a5cf43 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -537,19 +537,25 @@ static void bond_ipsec_add_sa_all(struct bonding *bond) } list_for_each_entry(ipsec, &bond->ipsec_list, list) {
spin_lock_bh(&ipsec->xs->lock);/* Skip dead xfrm states, they'll be freed later. */
if (ipsec->xs->km.state == XFRM_STATE_DEAD)
if (ipsec->xs->km.state == XFRM_STATE_DEAD) {spin_unlock_bh(&ipsec->xs->lock);
Instead of unlocking on every branch, I recommend adding a "next:" tag before the unlock at the end of the loop and switching the "continue" statements with "goto next".
continue;
}/* If new state is added before ipsec_lock acquired */
if (ipsec->xs->xso.real_dev == real_dev)
if (ipsec->xs->xso.real_dev == real_dev) {spin_unlock_bh(&ipsec->xs->lock);continue;
}ipsec->xs->xso.real_dev = real_dev; if (real_dev->xfrmdev_ops->xdo_dev_state_add(ipsec-
xs, NULL)) {
slave_warn(bond_dev, real_dev, "%s: failed to add SA\n", __func__); ipsec->xs->xso.real_dev = NULL; }
Add the "next:" tag here.
spin_unlock_bh(&ipsec->xs->lock);} out: mutex_unlock(&bond->ipsec_lock); @@ -614,6 +620,7 @@ static void bond_ipsec_del_sa_all(struct bonding *bond) if (!ipsec->xs->xso.real_dev) continue;
The above if should be in the critical section as well.
spin_lock_bh(&ipsec->xs->lock);if (ipsec->xs->km.state == XFRM_STATE_DEAD) { /* already dead no need to delete again */ if (ipsec->xs->xso.real_dev == real_dev && @@ -621,6 +628,7 @@ static void bond_ipsec_del_sa_all(struct bonding *bond) real_dev->xfrmdev_ops-
xdo_dev_state_free(ipsec->xs);
list_del(&ipsec->list); kfree(ipsec);
spin_unlock_bh(&ipsec->xs->lock);
And I recommend the same thing with "goto next" here, jumping at the end of the loop, before the unlock.
continue; } @@ -635,6 +643,7 @@ static void bond_ipsec_del_sa_all(struct bonding *bond) if (real_dev->xfrmdev_ops-
xdo_dev_state_free)
real_dev->xfrmdev_ops-
xdo_dev_state_free(ipsec->xs);
}
spin_unlock_bh(&ipsec->xs->lock);} mutex_unlock(&bond->ipsec_lock); }
Thanks Hangbin