netdev - Re: [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow dissector support with vnet hdr

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20210616001251.atqtz54qin4z2pg7@kafai-mbp>
Date:   Tue, 15 Jun 2021 17:12:51 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     Tanner Love <tannerlove.kernel@...il.com>
CC:     Network Development <netdev@...r.kernel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Willem de Bruijn <willemb@...gle.com>,
        Petar Penkov <ppenkov@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        "Michael S . Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Tanner Love <tannerlove@...gle.com>,
        Stanislav Fomichev <sdf@...gle.com>
Subject: Re: [PATCH net-next v6 1/3] net: flow_dissector: extend bpf flow
 dissector support with vnet hdr

On Tue, Jun 15, 2021 at 04:50:53PM -0700, Tanner Love wrote:
[ ... ]

> > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > index 418b9b813d65..e1ac34548f9a 100644
> > > --- a/include/uapi/linux/bpf.h
> > > +++ b/include/uapi/linux/bpf.h
> > > @@ -6017,6 +6017,8 @@ struct bpf_flow_keys {
> > >       };
> > >       __u32   flags;
> > >       __be32  flow_label;
> > > +     __bpf_md_ptr(const struct virtio_net_hdr *, vhdr);
> > > +     __u8    vhdr_is_little_endian;
> > I am not familiar with virtio.  A question on the "vhdr_is_little_endian" field.
> > The commit message said
> > "to handle the case of a version 1+ header on a big endian machine".
> > iiuc, version 1+ is always in little endian?
> 
> That's right.
> 
> > Does it mean most cases are in little endian?
> > and at least will eventually be moved to version 1+?
hmm..... so the common cases are little endian or
at least the remaining cases will eventually be moved to
version 1+ which is little endian?

> >
> > I wonder if this field will eventually be useless (because of always
> > true) and can it be avoided from the uapi now.  The current uapi
> > fields (e.g. in bpf_sock) are always in one particular order.
> >
> > If it is in big endian, can it be changed to little endian first
> > before calling the bpf prog?
> 
> In fact, v1 of this patch set did the conversion prior to passing
> the fields to the bpf prog, which meant that the bpf prog did not
> have to do anything about endianness. I changed that, though,
> at the suggestion of Alexei; Alexei suggested that we pass a
> pointer to struct virtio_net_hdr, rather than copying the individual
> virtio_net_hdr fields. V1 did the endianness conversion as part
> of that copying process. If we go back to doing it like that, then
> we lose the advantage that Alexei's suggestion aimed to achieve
> (i.e. avoiding the cost of copying the fields).
If the common case is little endian, then the vhdr pointer can still
be passed to bpf prog as-is directly for speed.  A deep copy and
conversion are only needed for the less common big endian case which
eventually will be moved to version 1+?