Re: [PATCH net 0/2] bond: fix xfrm offload feature during init

Jianbo Liu
2025-01-09T12:22:22Z

On 1/9/2025 6:17 PM, Hangbin Liu wrote:
> On Thu, Jan 09, 2025 at 05:51:07PM +0800, Jianbo Liu wrote:
>>>> No, we don't need. But I am trying to understand what you said in your last
>>>> email about adding a new lock, or unlocking spin lock in
>>>
>>> I *thought* we need the spin lock in xfrm_state_delete(). So to protect xfrm_state,
>>
>> But not need in bond_ipsec_del_sa() because the state still hold by
>> xfrm_state_hold(), right?
>
> Hmm, I'm not sure. If xfrm_state_hold() is safe. Why not just remove the spin
> lock in xfrm_state_delete(). This is more straightforward. e.g.
>

We can't remove the spin lock in xfrm_state_delete(), but I think we can
access the state while holding it, for example, checking (ipsec->xs ==
xs) as you mentioned before, because memory is not freed yet.

> diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
> index 67ca7ac955a3..150562abf513 100644
> --- a/net/xfrm/xfrm_state.c
> +++ b/net/xfrm/xfrm_state.c
> @@ -784,9 +784,7 @@ int xfrm_state_delete(struct xfrm_state *x)
> {
> int err;
>
> - spin_lock_bh(&x->lock);
> err = __xfrm_state_delete(x);
> - spin_unlock_bh(&x->lock);
>
> return err;
> }
>
> We can even rename xfrm_state_delete() to xfrm_state_delete() directly.
>
> Thanks
> Hangbin