sk_buff and reference counting netdev pointers

sk_buff and reference counting netdev pointers

[Joel Cunningham]

Hi,

I’m working on a research effort to understand the synchronization mechanisms for accessing and modifying a struct net_device object.  One area that isn’t clear is the net device pointer (dev) stored in a struct sk_buff.  From my investigation, the pointer appears to be assigned without increasing the struct net_device’s reference count (example __netdev_alloc_skb doesn’t call dev_hold) and also when the sk_buff is freed (kfree_skb) no call to dev_put() is made.  This seems to leave a possibility of an skb referencing a stale net device unless something is cleaning up all the skbs during unregister_netdevice() (which waits for all outstanding references to be released).  Any insight in understanding how this is working would be appreciated!

Joel

Re: sk_buff and reference counting netdev pointers

[Cong Wang]

On Mon, Jan 23, 2017 at 2:37 PM, Joel Cunningham <joel.cunningham@me.com> wrote:
> Hi,
>
> I’m working on a research effort to understand the synchronization mechanisms for accessing and modifying a struct net_device object.  One area that isn’t clear is the net device pointer (dev) stored in a struct sk_buff.  From my investigation, the pointer appears to be assigned without increasing the struct net_device’s reference count (example __netdev_alloc_skb doesn’t call dev_hold) and also when the sk_buff is freed (kfree_skb) no call to dev_put() is made.  This seems to leave a possibility of an skb referencing a stale net device unless something is cleaning up all the skbs during unregister_netdevice() (which waits for all outstanding references to be released).  Any insight in understanding how this is working would be appreciated!
>

This is a very common question.

synchronize_net() is supposed to wait for on-flying packets, since
both for TX and RX paths we acquire RCU read lock.

Re: sk_buff and reference counting netdev pointers

[Joel Cunningham]

> On Jan 23, 2017, at 5:45 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> 
> On Mon, Jan 23, 2017 at 2:37 PM, Joel Cunningham <joel.cunningham@me.com> wrote:
>> Hi,
>> 
>> I’m working on a research effort to understand the synchronization mechanisms for accessing and modifying a struct net_device object.  One area that isn’t clear is the net device pointer (dev) stored in a struct sk_buff.  From my investigation, the pointer appears to be assigned without increasing the struct net_device’s reference count (example __netdev_alloc_skb doesn’t call dev_hold) and also when the sk_buff is freed (kfree_skb) no call to dev_put() is made.  This seems to leave a possibility of an skb referencing a stale net device unless something is cleaning up all the skbs during unregister_netdevice() (which waits for all outstanding references to be released).  Any insight in understanding how this is working would be appreciated!
>> 
> 
> This is a very common question.
> 
> synchronize_net() is supposed to wait for on-flying packets, since
> both for TX and RX paths we acquire RCU read lock.

Thanks for the insight on in-flight packets!  I continued my research effort on how queued/outstanding sk_buffs with dev pointer set are cleaned up during an unregister.  Do my findings sound correct?

1. RX packets queued in the link-layer backlog are freed via flush_backlog (called from netdev_run_todo)
2. RX packets that have been processed and queued in an upper layer buffer have already had their sk_buff->dev pointer NULLed out (I saw this in tcp_v4_rcv() and sock_queue_rcv_skb() for UDP pathway)
3. TX packets waiting in a queue discipline are freed via device_deactive_many()/dev_deactivate_queue()
4. TX packets that have been accepted by the driver in ndo_start_xmit() but are queued for asynchronous transmit will be freed during ndo_stop()

Thanks,

Joel