netdev - Re: [PATCH bpf-next] net: xdp: introduce xdp_init

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]

Date:   Thu, 10 Dec 2020 10:10:58 -0800
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc:     Lorenzo Bianconi <lorenzo@...nel.org>, bpf <bpf@...r.kernel.org>,
        Netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        lorenzo.bianconi@...hat.com, Saeed Mahameed <saeed@...nel.org>
Subject: Re: [PATCH bpf-next] net: xdp: introduce xdp_init_buff utility routine

On Thu, Dec 10, 2020 at 9:05 AM Maciej Fijalkowski
<maciej.fijalkowski@...el.com> wrote:
>
> On Thu, Dec 10, 2020 at 05:32:41PM +0100, Lorenzo Bianconi wrote:
> > > On Thu, Dec 10, 2020 at 04:50:42PM +0100, Lorenzo Bianconi wrote:
> > > > Introduce xdp_init_buff utility routine to initialize xdp_buff data
> > > > structure. Rely on xdp_init_buff in all XDP capable drivers.
> > >
> > > Hm, Jesper was suggesting two helpers, one that you implemented for things
> > > that are set once per NAPI and the other that is set per each buffer.
> > >
> > > Not sure about the naming for a second one - xdp_prepare_buff ?
> > > xdp_init_buff that you have feels ok.
> >
> > ack, so we can have xdp_init_buff() for initialization done once per NAPI run and
> > xdp_prepare_buff() for per-NAPI iteration initialization, e.g.
> >
> > static inline void
> > xdp_prepare_buff(struct xdp_buff *xdp, unsigned char *hard_start,
> >                int headroom, int data_len)
> > {
> >       xdp->data_hard_start = hard_start;
> >       xdp->data = hard_start + headroom;
> >       xdp->data_end = xdp->data + data_len;
> >       xdp_set_data_meta_invalid(xdp);
> > }
>
> I think we should allow for setting the data_meta as well.
> x64 calling convention states that first four args are placed onto
> registers, so to keep it fast maybe have a third helper:
>
> static inline void
> xdp_prepare_buff_meta(struct xdp_buff *xdp, unsigned char *hard_start,
>                       int headroom, int data_len)
> {
>         xdp->data_hard_start = hard_start;
>         xdp->data = hard_start + headroom;
>         xdp->data_end = xdp->data + data_len;
>         xdp->data_meta = xdp->data;
> }
>
> Thoughts?

We only really need the 2, init and prepare. Also this is inline so I
don't think the arguments passed really play much of a role since
values like the headroom are likely to be constants and treated as
such.

- Alex