netdev - Re: [RFC,PATCH] loopback: calls netif_receive_skb() instead of netif

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 22 Feb 2008 00:19:19 +0100
From:	Eric Dumazet <dada1@...mosbay.com>
To:	Daniel Lezcano <dlezcano@...ibm.com>
CC:	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [RFC,PATCH] loopback: calls netif_receive_skb() instead of netif_rx()

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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html