On Fri, Jan 14, 2022 at 8:35 AM Lorenzo Bianconi [off-list ref] wrote:

quoted

On Thu, Jan 13, 2022 at 2:22 AM Lorenzo Bianconi [off-list ref] wrote:

quoted

quoted

quoted

quoted

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]" ?

If we change xdp_devmap/ in xdp/devmap (and xdp_cpumap/ in xdp/cpumap),
are we going to break backward compatibility?
Maybe there are programs already deployed using it.
This is not a xdp multi-buff problem since we are not breaking backward
compatibility there, we can just use:

xdp.frags/devmap
xdp.frags/cpumap

Moreover in samples/bpf we have something like:

SEC("xdp_devmap/egress")

It seems to me the egress postfix is not really used, right? Can we just drop
it?

Yeah, by current rules it should be just SEC("xdp_devmap"). This will
break in libbpf 1.0 mode. For anyone who knows how to actually test
BPF samples, it would be great to add
libbpf_set_strict_mode(LIBBPF_STRICT_ALL); in every sample and make
sure everything is still working. We've cleaned up selftests and all
other places I knew about, but missed samples (and I can't test them
properly).

Regards,
Lorenzo

quoted

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.