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

[Qi Tang <tpluszz77@gmail.com>]

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?