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

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]

Message-ID: <CAF=yD-KSJRO9SFE+bNNAWXihTi6VvD3pvUC36OJzSv9vNXS6vg@mail.gmail.com>
Date:   Wed, 27 Jun 2018 12:00:07 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     ecree@...arflare.com
Cc:     linux-net-drivers@...arflare.com,
        Network Development <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>
Subject: Re: [RFC PATCH v2 net-next 06/12] net: core: propagate SKB lists
 through packet_type lookup

On Wed, Jun 27, 2018 at 10:49 AM Edward Cree <ecree@...arflare.com> wrote:
>
> On 27/06/18 15:36, Willem de Bruijn wrote:
> > On Tue, Jun 26, 2018 at 8:19 PM Edward Cree <ecree@...arflare.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@...arflare.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.