On Tue, Dec 12, 2023 at 2:01 PM <dthaler1968@xxxxxxxxxxxxxx> wrote:
>
> > > For example, let's take a look at #2 atomic...
> > > Should it include or exclude atomic_add insn ? It was added at the
> > > very beginning of BPF ISA and was used from day one.
> > > Without it it's impossible to count stats. The typical network or
> > > tracing use case needs to count events and one cannot do it without
> > > atomic increment. Eventually per-cpu maps were added as an alternative.
> > > I suspect any platform that supports #1 basic insn without atomic_add
> > > will not be practically useful.
> > > Should atomic_add be a part of "basic" then? But it's atomic.
> > > Then what about atomic_fetch_add insn? It's pretty close semantically.
> > > Part of atomic or part of basic?
> >
> > I think it's reasonable to expect that if you require an atomic add, that you
> > may also require the other atomic instructions as well and that it would be
> > logical to group them together, yes. I believe that Netronome supports all of
> > the atomic instructions, as one example. If you're providing a BPF runtime in
> > an environment where atomic adds are required, I think it stands to reason
> > that you should probably support the other atomics as well, no?
>
> I agree.
Your logical reasoning is indeed correct and
I agree with it,
but reality is different :)
drivers/net/ethernet/netronome/nfp/bpf/jit.c:
static int mem_atomic8(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
{
if (meta->insn.imm != BPF_ADD)
return -EOPNOTSUPP;
return mem_xadd(nfp_prog, meta, true);
}
It only supports atomic_add and no other atomics.
> > > Another example, #3 divide. bpf cpu=v1 ISA only has unsigned div/mod.
> > > Eventually we added a signed version. Integer division is one of the
> > > slowest operations in a HW. Different cpus have different flavors of
> > > them 64/32 64/64 32/32, etc. All with different quirks.
> > > cpu=v1 had modulo insn because in tracing one often needs to do it to
> > > select a slot in a table, but in networking there is rarely a need.
> > > So bpf offload into netronome HW doesn't support it (iirc).
> >
> > Correct, my understanding is that BPF offload in netronome supports neither
> > division nor modulo.
>
> In my opinion, this is a valid technical reason to put them into a separate
> conformance group, to allow hardware offload cards to support BPF without
> requiring division/modulo which they might not have space or other budget for.
Also logically correct and I agree with, but reality proves all of us wrong.
netronome doesn't support modulo,
but it supports integer division when the verifier can determine
property of the constant.
BPF_ALU64 | BPF_DIV | BPF_K works for positive imm32,
but BPF_X works when the verifier is smart with plenty of quirks
and subtle conditions.
It works with the help of cool math reciprocal_value_adv()
in include/linux/reciprocal_div.h
which converts div to shifts and muls.
So should div_K and div_X be in separate groups ?
Should mod_[K|X] be there as well or not?
To determine the grouping should we use logic or reality?
I'm arguing that whatever clean and logical grouping we can come up with
it won't stand a test of real use.
