netdev - Re: [PATCH v21 bpf-next 18/23] libbpf: Add SEC name for xdp

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, 13 Jan 2022 12:19:40 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Lorenzo Bianconi <lorenzo@...nel.org>
Cc:     Jesper Dangaard Brouer <jbrouer@...hat.com>,
        Toke Høiland-Jørgensen <toke@...hat.com>,
        Lorenzo Bianconi <lorenzo.bianconi@...hat.com>,
        Zvi Effron <zeffron@...tgames.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Andrii Nakryiko <andrii.nakryiko@...il.com>,
        bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Shay Agroskin <shayagr@...zon.com>,
        john fastabend <john.fastabend@...il.com>,
        David Ahern <dsahern@...nel.org>,
        Eelco Chaudron <echaudro@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Alexander Duyck <alexander.duyck@...il.com>,
        Saeed Mahameed <saeed@...nel.org>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        tirthendu.sarkar@...el.com
Subject: Re: [PATCH v21 bpf-next 18/23] libbpf: Add SEC name for xdp_mb programs

On Thu, Jan 13, 2022 at 2:22 AM Lorenzo Bianconi <lorenzo@...nel.org> wrote:
> > > >
> > > > I would prefer to keep the "_mb" postfix, but naming is hard and I am
> > > > polarized :)
> > >
> > > I would lean towards keeping _mb as well, but if it does have to be
> > > changed why not _mbuf? At least that's not quite as verbose :)
> >
> > I dislike the "mb" abbreviation as I forget it stands for multi-buffer.
> > I like the "mbuf" suggestion, even-though it conflicts with (Free)BSD mbufs
> > (which is their SKB).
>
> If we all agree, I can go over the series and substitute mb postfix with mbuf.
> Any objections?

mbuf has too much bsd taste.

How about ".frags" instead?
Then xdp_buff_is_mb() will be xdp_buff_has_frags().

I agree that it's not obvious what "mb" suffix stands for,
but I don't buy at all that it can be confused with "megabyte".
It's the context that matters.
In "100mb" it's obvious that "mb" is likely "megabyte",
but in "xdp.mb" it's certainly not "xdp megabyte".
Such a sentence has no meaning.
Imagine we used that suffix for "tc"...
it would be "tc.mb"... "Traffic Control Megabyte" ??

Anyway "xdp.frags" ?

Btw "xdp_cpumap" should be cleaned up.
xdp_cpumap is an attach type. It's not prog type.
Probably it should be "xdp/cpumap" to align with "cgroup/bind[46]" ?

In patch 22 there is a comment:
/* try to attach BPF_XDP_DEVMAP multi-buff program"

It creates further confusion. There is no XDP_DEVMAP program type.
It should probably read
"Attach BPF_XDP program with frags to devmap"

Patch 21 still has "CHECK". Pls replace it with ASSERT.