From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B1384C61DA4 for ; Thu, 23 Feb 2023 16:43:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233513AbjBWQm7 (ORCPT ); Thu, 23 Feb 2023 11:42:59 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43880 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232923AbjBWQm6 (ORCPT ); Thu, 23 Feb 2023 11:42:58 -0500 Received: from eggs.gnu.org (eggs.gnu.org [IPv6:2001:470:142:3::10]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3B23814EAB for ; Thu, 23 Feb 2023 08:42:55 -0800 (PST) Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pVEgG-0002jo-B8; Thu, 23 Feb 2023 11:42:52 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-Version:Date:References:In-Reply-To:Subject:To: From; bh=8G1yHTRxIga12nccRnHNggy5P3bOzP2h9IuvovD1O6k=; b=HmLBcGS0yfTdNCibYEAn +SRH9stIKosKZ0Fc3EGhCGJYruy+6TRyt6ota58RB9MLnwWRzB7cVaREUC+y82hnIuCDst+ehYDnt zsagYCzve4KqaLqt7ESC5HDoSGaMuBNaw95cMv3142hJD8EtFewEmdsvwehepAtgsXsYkN3tLwF4K b9Tu4fVG5CZ70ri+v1+PkVVlc5z6VljnDAEkzrnTfjYKHg2xHegZT5Gdy32QC+hHG2PLGNdqPcjIp ToA4X01BkNG/rW0paVH8PkwRFurRMMDdwD7cDROQZDcls/LyKp6Hxn5zP6PW0IC4RKB3HWOj6edln A9AmCcrZ56HB8A==; Received: from [141.143.193.79] (helo=termi) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pVEgF-0005ub-Qy; Thu, 23 Feb 2023 11:42:52 -0500 From: "Jose E. Marchesi" To: Alexei Starovoitov Cc: "Jose E. Marchesi" , Dave Thaler , bpf , bpf@ietf.org, Dave Thaler , David Vernet Subject: Re: [Bpf] [PATCH bpf-next v2] bpf, docs: Add explanation of endianness In-Reply-To: (Alexei Starovoitov's message of "Thu, 23 Feb 2023 08:40:44 -0800") References: <20230220223742.1347-1-dthaler1968@googlemail.com> <87ttzdwagy.fsf@oracle.com> <87bklkseo4.fsf@gnu.org> Date: Thu, 23 Feb 2023 17:42:46 +0100 Message-ID: <87mt549vuh.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org > On Thu, Feb 23, 2023 at 5:19 AM Jose E. Marchesi wrote: >> >> >> > On Wed, Feb 22, 2023 at 3:23 PM Jose E. Marchesi >> > wrote: >> >> >> >> >> >> > On Mon, Feb 20, 2023 at 2:37 PM Dave Thaler >> >> > wrote: >> >> >> >> >> >> From: Dave Thaler >> >> >> >> >> >> Document the discussion from the email thread on the IETF bpf list, >> >> >> where it was explained that the raw format varies by endianness >> >> >> of the processor. >> >> >> >> >> >> Signed-off-by: Dave Thaler >> >> >> >> >> >> Acked-by: David Vernet >> >> >> --- >> >> >> >> >> >> V1 -> V2: rebased on top of latest master >> >> >> --- >> >> >> Documentation/bpf/instruction-set.rst | 16 ++++++++++++++-- >> >> >> 1 file changed, 14 insertions(+), 2 deletions(-) >> >> >> >> >> >> diff --git a/Documentation/bpf/instruction-set.rst b/Documentation/bpf/instruction-set.rst >> >> >> index af515de5fc3..1d473f060fa 100644 >> >> >> --- a/Documentation/bpf/instruction-set.rst >> >> >> +++ b/Documentation/bpf/instruction-set.rst >> >> >> @@ -38,8 +38,9 @@ eBPF has two instruction encodings: >> >> >> * the wide instruction encoding, which appends a second 64-bit immediate (i.e., >> >> >> constant) value after the basic instruction for a total of 128 bits. >> >> >> >> >> >> -The basic instruction encoding is as follows, where MSB and LSB mean the most significant >> >> >> -bits and least significant bits, respectively: >> >> >> +The basic instruction encoding looks as follows for a little-endian processor, >> >> >> +where MSB and LSB mean the most significant bits and least significant bits, >> >> >> +respectively: >> >> >> >> >> >> ============= ======= ======= ======= ============ >> >> >> 32 bits (MSB) 16 bits 4 bits 4 bits 8 bits (LSB) >> >> >> @@ -63,6 +64,17 @@ imm offset src_reg dst_reg opcode >> >> >> **opcode** >> >> >> operation to perform >> >> >> >> >> >> +and as follows for a big-endian processor: >> >> >> + >> >> >> +============= ======= ==================== =============== ============ >> >> >> +32 bits (MSB) 16 bits 4 bits 4 bits 8 bits (LSB) >> >> >> +============= ======= ==================== =============== ============ >> >> >> +immediate offset destination register source register opcode >> >> >> +============= ======= ==================== =============== ============ >> >> > >> >> > I've changed it to: >> >> > imm offset dst_reg src_reg opcode >> >> > >> >> > to match the little endian table, >> >> > but now one of the tables feels wrong. >> >> > The encoding is always done by applying C standard to the struct: >> >> > struct bpf_insn { >> >> > __u8 code; /* opcode */ >> >> > __u8 dst_reg:4; /* dest register */ >> >> > __u8 src_reg:4; /* source register */ >> >> > __s16 off; /* signed offset */ >> >> > __s32 imm; /* signed immediate constant */ >> >> > }; >> >> > I'm not sure how to express this clearly in the table. >> >> >> >> Perhaps it would be simpler to document how the instruction bytes are >> >> stored (be it in an ELF file or as bytes in a memory buffer to be loaded >> >> into the kernel or some other BPF consumer) as opposed to how the >> >> instructions look like once loaded (as a 64-bit word) by a little-endian >> >> or big-endian kernel? >> >> >> >> Stored little-endian BPF instructions: >> >> >> >> code src_reg dst_reg off imm >> >> >> >> foo-le.o: file format elf64-bpfle >> >> >> >> 0000000000000000 <.text>: >> >> 0: 07 01 00 00 ef be ad de r1 += 0xdeadbeef >> >> >> >> Stored big-endian BPF instructions: >> >> >> >> code dst_reg src_reg off imm >> >> >> >> foo-be.o: file format elf64-bpfbe >> >> >> >> 0000000000000000 <.text>: >> >> 0: 07 10 00 00 de ad be ef r1 += 0xdeadbeef >> >> >> >> i.e. in the stored bytes the code always comes first, then the >> >> registers, then the offset, then the immediate, regardless of >> >> endianness. >> >> >> >> This may be easier to understand by implementors looking to generate >> >> and/or consume bytes conforming BPF instructions. >> > >> > +1 >> > I like this format more as well. >> > Maybe we can drop the table and use a diagram of a kind ? >> > >> > opcode src dst offset imm assembly >> > 07 0 1 00 00 ef be ad de r1 += 0xdeadbeef // little >> > 07 1 0 00 00 de ad be ef r1 += 0xdeadbeef // big >> >> Good idea. What about something like this: >> >> opcode offset imm assembly >> src dst >> 07 0 1 00 00 44 33 22 11 r1 += 0x11223344 // little >> dst src >> 07 1 0 00 00 11 22 33 44 r1 += 0x11223344 // big >> >> I changed the immediate because 0xdeadbeef is negative and it may be >> confusing in the assembly part: strictly it would be r1 += -559038737. > > Looks great to me. Do you want to send your first kernel patch? :) Sure, will prepare it :)