From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesper Dangaard Brouer Subject: Re: [RFC bpf-next PATCH] bpf: add comments to BPF ld/ldx sizes Date: Wed, 17 Jan 2018 11:41:02 +0100 Message-ID: <20180117114102.5e4d142a@redhat.com> References: <151610226596.29962.4661213256715212454.stgit@firesoul> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Daniel Borkmann , Alexei Starovoitov , netdev@vger.kernel.org, brouer@redhat.com To: Daniel Borkmann Return-path: Received: from mx1.redhat.com ([209.132.183.28]:47962 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752814AbeAQKlK (ORCPT ); Wed, 17 Jan 2018 05:41:10 -0500 In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Wed, 17 Jan 2018 00:21:27 +0100 Daniel Borkmann wrote: > On 01/16/2018 12:31 PM, Jesper Dangaard Brouer wrote: > > Doc BPF ld/ldx size defines, as it help me understand the code in filter.c. > > > > Signed-off-by: Jesper Dangaard Brouer > > --- > > 0 files changed > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > index 395d261948de..4729d9a002d4 100644 > > --- a/include/uapi/linux/bpf.h > > +++ b/include/uapi/linux/bpf.h > > @@ -17,7 +17,7 @@ > > #define BPF_ALU64 0x07 /* alu mode in double word width */ > > > > /* ld/ldx fields */ > > -#define BPF_DW 0x18 /* double word */ > > +#define BPF_DW 0x18 /* double word (64-bit) */ > > #define BPF_XADD 0xc0 /* exclusive add */ > > > > /* alu/jmp fields */ > > diff --git a/include/uapi/linux/bpf_common.h b/include/uapi/linux/bpf_common.h > > index 18be90725ab0..ee97668bdadb 100644 > > --- a/include/uapi/linux/bpf_common.h > > +++ b/include/uapi/linux/bpf_common.h > > @@ -15,9 +15,10 @@ > > > > /* ld/ldx fields */ > > #define BPF_SIZE(code) ((code) & 0x18) > > -#define BPF_W 0x00 > > -#define BPF_H 0x08 > > -#define BPF_B 0x10 > > +#define BPF_W 0x00 /* 32-bit */ > > +#define BPF_H 0x08 /* 16-bit */ > > +#define BPF_B 0x10 /* 8-bit */ > > +/* eBPF BPF_DW 0x18 64-bit */ > > Hmm, I don't really mind, but we do have it documented in: > > Documentation/networking/filter.txt +942 > > Feels like if we put a comment only on BPF_{B,H,W}, then we > might also want to document all the others such as ALU ops, > etc. We can start out small. I made an actual mistake by misunderstanding these sizes (this was also because BPF_DW is located in another file, so I didn't deduce BPF_W was 32-bit not 64-bit). I missed the documentation you reference. Documentation is good, but I practice placing the documentation as close as possible to where you need it. In a programming setting, I looked up the define BPF_W (with cscope) in a second, while it will take minutes finding the right documentation. -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer