Re: [PATCH net] xfrm: hold skb->dev across async IPv6 transport reinject

[Steffen Klassert <steffen.klassert@secunet.com>]

On Thu, Mar 26, 2026 at 08:41:31PM +0800, Qi Tang wrote:
> I reworked the fix direction to avoid taking another netdev reference.
> 
> The current idea is to reuse the existing dev_hold() in xfrm_input(),
> carry that reference across the async transport reinject path, and drop
> it only once the skb is either consumed earlier or the async reinject
> callback has completed.
> 
> Very roughly, the change would look like this:
> 
>   - in xfrm_input(), do not drop the existing dev reference immediately
>     on async resume
>   - add a dedicated queueing path for skbs that already carry that
>     input-side reference
>   - in transport_finish(), switch from NF_HOOK() to nf_hook() so the
>     ret != 1 path can drop the carried reference when the skb is consumed
>     before okfn runs
>   - in xfrm_trans_reinject(), drop the transferred reference after the
>     callback completes
> 
> Something along these lines:
> 
>   diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
>   @@
>         if (encap_type == -1) {
>                 async = 1;
>   -             dev_put(skb->dev);
>                 seq = XFRM_SKB_CB(skb)->seq.input.low;
>                 ...
>         }
> 
>   @@
>   +int xfrm_trans_queue_in(struct net *net, struct sk_buff *skb, ...)
>   +{
>   +       ...
>   +       XFRM_TRANS_SKB_CB(skb)->dev_ref_held = true;
>   +       ...
>   +}
> 
>   diff --git a/net/ipv6/xfrm6_input.c b/net/ipv6/xfrm6_input.c
>   @@
>   -     NF_HOOK(..., xfrm6_transport_finish2);
>   +     ret = nf_hook(..., async ? xfrm6_transport_finish2_async :
>   +                            xfrm6_transport_finish2);
>   +     if (ret != 1) {
>   +             if (async)
>   +                     dev_put(dev);
>   +             return 0;
>   +     }
>   +     (async ? xfrm6_transport_finish2_async :
>   +              xfrm6_transport_finish2)(net, NULL, skb);
> 
>   diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
>   @@
>         while ((skb = __skb_dequeue(&queue))) {
>   +             bool dev_ref_held = cb->dev_ref_held;
>   +             struct net_device *dev = dev_ref_held ? skb->dev : NULL;
>                 cb->finish(cb->net, NULL, skb);
>   +             if (dev_ref_held)
>   +                     dev_put(dev);
>         }
> 
> Does this sound like a reasonable direction for a v2?

Please send the actual patch for review. It is hard to
guess from such snippets if that's the correct way to
go.

Thanks!