Re: [RFC,PATCH] loopback: calls netif_receive_skb() instead of netif_rx()

[Daniel Lezcano <dlezcano@fr.ibm.com>]

Eric Dumazet wrote:
> Daniel Lezcano a écrit :
>> Eric Dumazet wrote:
>>> Hi David
>>>
>>> This is an RFC, based on net-2.6 for convenience only.
>>>
>>> Thank you
>>>
>>> [RFC,PATCH] loopback: calls netif_receive_skb() instead of netif_rx()
>>>
>>> Loopback transmit function loopback_xmit() actually calls netif_rx() 
>>> to queue
>>> a skb to the softnet queue, and arms a softirq so that this skb can 
>>> be handled later.
>>>
>>> This has a cost on SMP, because we need to hold a reference on the 
>>> device, and free this
>>> reference when softirq dequeues packet.
>>>
>>> Following patch directly calls netif_receive_skb() and avoids lot of 
>>> atomic operations.
>>> (atomic_inc(&dev->refcnt), set_and_set_bit(NAPI_STATE_SCHED, 
>>> &n->state), ...
>>>  atomic_dec(&dev->refcnt)...), cache line ping-pongs on device 
>>> refcnt, but also softirq overhead.
>>>
>>> This gives a nice boost on tbench for example (5 % on my machine)
>>
>> I understand this is interesting for the loopback when there is no 
>> multiple instances of it and it can't be unregistered. But now with 
>> the network namespaces, we can have multiple instances of the loopback 
>> and it can to be unregistered. Shouldn't we still use netif_rx ?
>> Perhaps we can do something like:
>>
>>     if (dev->nd_net == &init_net)
>>         netif_receive_skb(skb);
>>     else
>>         netif_rx(skb);
> 
> or
> 
> #ifdef CONFIG_NET_NS
>     if (dev->nd_net != &init_net)
>         netif_rx(skb);
>     else
> #endif
>         netif_receive_skb(skb);
> 
>>
>> Or we create:
>>     init_loopback_xmit() calling netif_receive_skb(skb);
>>     and setup this function when creating the loopback for init_net,
>>     otherwise we setup the usual loopback_xmit.
>>
>> We are still safe for multiple network namespaces and we have the 
>> improvement for init_net loopback.
>>
> 
> I dont understand how my patch could degrade loopbackdev unregister 
> logic. It should only help it, by avoiding a queue of 'pending packets' 
> per cpu.
> 
> When we want to unregister a network device, stack makes sure that no 
> more calls to dev->hard_start_xmit() can occur.
> 
> If no more loopback_xmit() calls are done on this device, it doesnt 
> matter if it internally uses netif_rx() or netif_receive_skb(skb)
> 
> loopback device has no queue, its really unfortunate to use the 
> 'softirq' internal queue.

Fair enough :)