Re: [Bpf] ISA: BPF_MSH and deprecated packet access instructions

From: "Jose E. Marchesi" <jose.marchesi@oracle.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Dave Thaler <dthaler1968@googlemail.com>,
	Yonghong Song <yonghong.song@linux.dev>,
	bpf@ietf.org, bpf <bpf@vger.kernel.org>
Subject: Re: [Bpf] ISA: BPF_MSH and deprecated packet access instructions
Date: Tue, 30 Jan 2024 13:13:15 +0100	[thread overview]
Message-ID: <87a5onkois.fsf@oracle.com> (raw)
In-Reply-To: <87plxkqr56.fsf@oracle.com> (Jose E. Marchesi's message of "Mon, 29 Jan 2024 13:07:49 +0100")

>> On Sat, Jan 27, 2024 at 10:59 PM <dthaler1968@googlemail.com> wrote:
>>>
>>> I asked:
>>> > >> What about DW and LDX variants of BPF_IND and BPF_ABS?
>>>
>>> Jose E. Marchesi <jose.marchesi@oracle.com> wrote:
>>> > These we support:
>>> >
>>> >   /* Absolute load instructions, designed to be used in socket filters.
>>> */
>>> >   {BPF_INSN_LDABSB, "ldabsb%W%i32", "r0 = * ( u8 * ) skb [ %i32 ]",
>>> >    BPF_V1, BPF_CODE, BPF_CLASS_LD|BPF_SIZE_B|BPF_MODE_ABS},
>>> >   {BPF_INSN_LDABSH, "ldabsh%W%i32", "r0 = * ( u16 * ) skb [ %i32 ]",
>>> >    BPF_V1, BPF_CODE, BPF_CLASS_LD|BPF_SIZE_H|BPF_MODE_ABS},
>>> >   {BPF_INSN_LDABSW, "ldabsw%W%i32", "r0 = * ( u32 * ) skb [ %i32 ]",
>>> >    BPF_V1, BPF_CODE, BPF_CLASS_LD|BPF_SIZE_W|BPF_MODE_ABS},
>>> >   {BPF_INSN_LDABSDW, "ldabsdw%W%i32", "r0 = * ( u64 * ) skb [ %i32 ]",
>>> >    BPF_V1, BPF_CODE, BPF_CLASS_LD|BPF_SIZE_DW|BPF_MODE_ABS},
>>> >
>>> >   /* Generic load instructions (to register.)  */
>>> >   {BPF_INSN_LDXB, "ldxb%W%dr , [ %sr %o16 ]", "%dr = * ( u8 * ) ( %sr %o16
>>> )",
>>> >    BPF_V1, BPF_CODE, BPF_CLASS_LDX|BPF_SIZE_B|BPF_MODE_MEM},
>>> >   {BPF_INSN_LDXH, "ldxh%W%dr , [ %sr %o16 ]", "%dr = * ( u16 * ) ( %sr
>>> %o16
>>> > )",
>>> >    BPF_V1, BPF_CODE, BPF_CLASS_LDX|BPF_SIZE_H|BPF_MODE_MEM},
>>> >   {BPF_INSN_LDXW, "ldxw%W%dr , [ %sr %o16 ]", "%dr = * ( u32 * ) ( %sr
>>> %o16
>>> > )",
>>> >    BPF_V1, BPF_CODE, BPF_CLASS_LDX|BPF_SIZE_W|BPF_MODE_MEM},
>>> >   {BPF_INSN_LDXDW, "ldxdw%W%dr , [ %sr %o16 ]","%dr = * ( u64 * ) ( %sr
>>> > %o16 )",
>>> >    BPF_V1, BPF_CODE, BPF_CLASS_LDX|BPF_SIZE_DW|BPF_MODE_MEM},
>>>
>>> Yonghong Song <yonghong.song@linux.dev> wrote:
>>> > I don't know how to do proper wording in the standard. But DW and LDX
>>> > variants of BPF_IND/BPF_ABS are not supported by verifier for now and they
>>> > are considered illegal insns.
>>>
>>> Although the Linux verifier doesn't support them, the fact that gcc does
>>> support
>>> them tells me that it's probably safest to list the DW and LDX variants as
>>> deprecated as well, which is what the draft already did in the appendix so
>>> that's good (nothing to change there, I think).
>>
>> DW never existed in classic bpf, so abs/ind never had DW flavor.
>> If some assembler/compiler decided to "support" them it's on them.
>> The standard must not list such things as deprecated. They never
>> existed. So nothing is deprecated.
>
> We have no reason to support these instructions in the assembler, and
> GCC certainly never generates these.  So I will remove the ABS/IND DW
> instructions from the assembler today.

This has been done now in upstream binutils.

>> Same with MSH. BPF_LDX | BPF_MSH | BPF_B is the only insn ever existed.
>> It's a legacy insn. Just like abs/ind.