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