* [RFC bpf-next PATCH] bpf: add comments to BPF ld/ldx sizes
@ 2018-01-16 11:31 Jesper Dangaard Brouer
2018-01-16 23:21 ` Daniel Borkmann
0 siblings, 1 reply; 3+ messages in thread
From: Jesper Dangaard Brouer @ 2018-01-16 11:31 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov; +Cc: netdev, Jesper Dangaard Brouer
Doc BPF ld/ldx size defines, as it help me understand the code in filter.c.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
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 */
#define BPF_MODE(code) ((code) & 0xe0)
#define BPF_IMM 0x00
#define BPF_ABS 0x20
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC bpf-next PATCH] bpf: add comments to BPF ld/ldx sizes
2018-01-16 11:31 [RFC bpf-next PATCH] bpf: add comments to BPF ld/ldx sizes Jesper Dangaard Brouer
@ 2018-01-16 23:21 ` Daniel Borkmann
2018-01-17 10:41 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Borkmann @ 2018-01-16 23:21 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Daniel Borkmann, Alexei Starovoitov; +Cc: netdev
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 <brouer@redhat.com>
> ---
> 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.
> #define BPF_MODE(code) ((code) & 0xe0)
> #define BPF_IMM 0x00
> #define BPF_ABS 0x20
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC bpf-next PATCH] bpf: add comments to BPF ld/ldx sizes
2018-01-16 23:21 ` Daniel Borkmann
@ 2018-01-17 10:41 ` Jesper Dangaard Brouer
0 siblings, 0 replies; 3+ messages in thread
From: Jesper Dangaard Brouer @ 2018-01-17 10:41 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Daniel Borkmann, Alexei Starovoitov, netdev, brouer
On Wed, 17 Jan 2018 00:21:27 +0100
Daniel Borkmann <daniel@iogearbox.net> 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 <brouer@redhat.com>
> > ---
> > 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-01-17 10:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-16 11:31 [RFC bpf-next PATCH] bpf: add comments to BPF ld/ldx sizes Jesper Dangaard Brouer
2018-01-16 23:21 ` Daniel Borkmann
2018-01-17 10:41 ` Jesper Dangaard Brouer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).