Re: [RFC PATCH v2 net-next 06/12] net: core: propagate SKB lists through packet_type lookup

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

On Wed, Jun 27, 2018 at 10:49 AM Edward Cree <ecree@solarflare.com> wrote:
>
> On 27/06/18 15:36, Willem de Bruijn wrote:
> > On Tue, Jun 26, 2018 at 8:19 PM Edward Cree <ecree@solarflare.com> wrote:
> >> __netif_receive_skb_taps() does a depressingly large amount of per-packet
> >>  work that can't easily be listified, because the another_round looping
> >>  makes it nontrivial to slice up into smaller functions.
> >> Fortunately, most of that work disappears in the fast path:
> >>  * Hardware devices generally don't have an rx_handler
> >>  * Unless you're tcpdumping or something, there is usually only one ptype
> >>  * VLAN processing comes before the protocol ptype lookup, so doesn't force
> >>    a pt_prev deliver
> >>  so normally, __netif_receive_skb_taps() will run straight through and return
> >>  the one ptype found in ptype_base[hash of skb->protocol].
> >>
> >> Signed-off-by: Edward Cree <ecree@solarflare.com>
> >> ---
> >> -static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
> >> +static int __netif_receive_skb_taps(struct sk_buff *skb, bool pfmemalloc,
> >> +                                   struct packet_type **pt_prev)
> > A lot of code churn can be avoided by keeping local variable pt_prev and
> > calling this ppt_prev or so, then assigning just before returning on success.
> Good idea, I'll try that.
>
> > Also, this function does more than just process network taps.
> This is true, but naming things is hard, and I couldn't think of either a
>  better new name for this function or a name that could fit in between
>  __netif_receive_skb() and __netif_receive_skb_core() for the new function
>  in my patch named __netif_receive_skb_core().  Any suggestions?

____netif_receive_skb_core? Not that four underscores is particularly
readable. Perhaps __netif_receive_skb_core_inner. It's indeed tricky (and
not the most important, I didn't mean to bikeshed).

Come to think of it, from your fast path assumptions, we could perhaps wrap
ptype_all and rx_handler logic in a static_branch similar to tc and netfilter
(and sk_memalloc_socks). Remaining branches like skip_classify, pfmemalloc
and deliver_exact can also not be reached if all these are off, so this entire
section can be skipped. Then it could become __netif_receive_skb_slow,
taken only on the static branch or for vlan packets.  I do not suggest it as
part of this patchset. it would be a pretty complex change on its own.