On 2022-02-02 09:43:14 [-0800], Eric Dumazet wrote:
> Maybe worth mentioning this commit will show a negative impact, for
> network traffic
> over loopback interface.
>
> My measure of the cost of local_bh_disable()/local_bh_enable() is ~6
> nsec on one of my lab x86 hosts.
So you are worried that
dev_loopback_xmit() -> netif_rx_ni()
becomes
dev_loopback_xmit() -> netif_rx()
and by that 6nsec slower because of that bh off/on? Can these 6nsec get
a little lower if we substract the overhead of preempt-off/on?
But maybe I picked the wrong loopback here.
> Perhaps we could have a generic netif_rx(), and a __netif_rx() for the
> virtual drivers (lo and maybe tunnels).
>
> void __netif_rx(struct sk_buff *skb);
>
> static inline int netif_rx(struct sk_buff *skb)
> {
> int res;
> local_bh_disable();
> res = __netif_rx(skb);
> local_bh_enable();
> return res;
> }
But what is __netif_rx() doing? netif_rx_ni() has this part:
| preempt_disable();
| err = netif_rx_internal(skb);
| if (local_softirq_pending())
| do_softirq();
| preempt_enable();
to ensure that smp_processor_id() and friends are quiet plus any raised
softirqs are processed. With the current netif_rx() we end up with:
| local_bh_disable();
| ret = netif_rx_internal(skb);
| local_bh_enable();
which provides the same. Assuming __netif_rx() as:
| int __netif_rx(skb)
| {
| trace_netif_rx_entry(skb);
|
| ret = netif_rx_internal(skb);
| trace_netif_rx_exit(ret);
|
| return ret;
| }
and the loopback interface is not invoking this in_interrupt() context.
Sebastian
