Re: [Bpf] ISA RFC compliance question

From: David Vernet <void@manifault.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Dave Thaler <dthaler@microsoft.com>,
	"bpf@ietf.org" <bpf@ietf.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [Bpf] ISA RFC compliance question
Date: Mon, 23 Oct 2023 17:15:40 -0500	[thread overview]
Message-ID: <20231023221540.GE32029@maniforge> (raw)
In-Reply-To: <CAADnVQ+BOdrU4x3qKHJVbpZCJwTWe6HXWhuMqOk-x5UK22yPDQ@mail.gmail.com>

On Fri, Oct 06, 2023 at 04:06:53PM -0700, Alexei Starovoitov wrote:
> On Thu, Oct 5, 2023 at 1:14 PM Dave Thaler <dthaler@microsoft.com> wrote:
> >
> > > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> > > On Fri, Sep 29, 2023 at 1:17 PM Dave Thaler
> > > <dthaler=40microsoft.com@dmarc.ietf.org> wrote:
> > > > Now that we have some new "v4" instructions, it seems a good time to
> > > > ask about what it means to support (or comply with) the ISA RFC once
> > > > published.  Does it mean that a verifier/disassembler/JIT compiler/etc. MUST
> > > support *all* the
> > > > non-deprecated instructions in the document?   That is any runtime or tool that
> > > > doesn't support the new instructions is considered non-compliant with the BPF
> > > ISA?
> > [...]
> > > > Or should we create some things that are SHOULDs, or finer grained
> > > > units of compliance so as to not declare existing deployments non-compliant?
> > >
> > > I suspect 'non-compliance' label will cause an unnecessary backlash, so I would
> > > go with SHOULD wording.
> >
> > Yeah, but if each instruction is a separate SHOULD, then a runtime could (say)
> > support one atomic instruction and not others.  Having that level of granularity
> > would really complicate interoperability and cross-platform tooling in my opinion.
> > So it might be better to list groups of instructions and have the SHOULD be at the
> > granularity of a group?
> 
> I guess we can group them based on LLVM evolution of the instruction set:
> -mcpu=v1,v2,v3,v4
> but it would have mainly historical benefits and not practical.

We will discuss more at IETF 118, but I agree that grouping based on
LLVM instruction set releases is not a good idea. It's the same
sentiment for why we don't want to standardize the .maps ELF section
just because it's what libbpf expects.

> Grouping atomic vs not is not realistic either, since atomic_xadd
> was there since the very beginning.

This might be a dumb question, but I'm not sure I'm following why it
being introduced since the beginning would preclude it from being
SHOULD?

> I suspect any kind of grouping scheme will end up in bike shedding.
> My preference would be to agree on either SHOULD or MUST for
> all insns currently described in ISA doc.

SHOULD for all instructions seems very risky for compliance. Wouldn't
that functionally make the standard entirely optional?

> We can go with MUST to force compliance.
> Some archs, OSes, JITs won't be compliant in the short term,
> but MUST wording will be a good motivation to do the work now instead of later.

This seems like the overall lowest risk to me, though there are some
nuances we'll have to consider. For example, it would require that all
platforms support BTF in order to be compliant with BPF_CALL by BTF ID.
Realistically that seems desirable. Unless there are groups of
instructions that could be submitted logically as their own separate
extensions, being consistent with MUST seems like the least error prone
approach.

Thanks,
David