Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space

Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space

[Alexei Starovoitov]

On Wed, Sep 3, 2014 at 8:46 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
> On 09/03/2014 05:17 AM, Alexei Starovoitov wrote:
>>
>> allow user space to generate eBPF programs
>>
>> uapi/linux/bpf.h: eBPF instruction set definition
>>
>> linux/filter.h: the rest
>>
>> This patch only moves macro definitions, but practically it freezes
>> existing
>> eBPF instruction set, though new instructions can still be added in the
>> future.
>>
>> These eBPF definitions cannot go into uapi/linux/filter.h, since the names
>> may conflict with existing applications.
>>
>> Full eBPF ISA description is in Documentation/networking/filter.txt
>>
>> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
>
>
> Ok, given you post the remaining two RFCs later on this window as
> you indicate, I have no objections:
>
> Acked-by: Daniel Borkmann <dborkman@redhat.com>

David,

I see that all 4 patches are marked as rfc, whereas I tagged
only 3 and 4 as 'rfc' and mentioned in cover letter that they
will be coming for real in stage III. Here they support patch 2
and show the future changes to bpf.h file.
Patches 1 and 2 are good to go.
Note that they're on top of Hannes's patch:
http://patchwork.ozlabs.org/patch/385266/
Do you want me to resubmit just first two?

Thanks

Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space

[David Miller]

From: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Date: Wed, 3 Sep 2014 22:41:48 -0700

> Do you want me to resubmit just first two?

Yes, you can't submit hodge-podge path series, either it's all to
be applied or it's all RFC material.

[PATCH v9 net-next 0/4] load imm64 insn and uapi/linux/bpf.h

[Alexei Starovoitov]

Hi,

V8->V9
- rebase on top of Hannes's patch in the same area [1]
- no changes to patches 1 and 2
- added patches 3 and 4 as RFC to address Daniel's concern.
  patch 3 moves eBPF instruction macros and
  patch 4 split macros which are shared between classic and eBPF
  into bpf_common.h
  3 and 4 will be used by verifier testsuite and will be reposted later
  during stage III

V8 thread with 'why' reasoning and end goal:
https://lkml.org/lkml/2014/8/27/628

Original set [2] of ~28 patches I'm planning to present in 4 stages:

  I. this 2 patches to fork off llvm upstreaming
 II. bpf syscall with manpage and map implementation
III. bpf program load/unload with verifier testsuite (1st user of
     instruction macros from bpf.h and 1st user of load imm64 insn)
 IV. tracing, etc

[1] http://patchwork.ozlabs.org/patch/385266/
[2] https://lkml.org/lkml/2014/8/26/859

[PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space

[Alexei Starovoitov]

allow user space to generate eBPF programs

uapi/linux/bpf.h: eBPF instruction set definition

linux/filter.h: the rest

This patch only moves macro definitions, but practically it freezes existing
eBPF instruction set, though new instructions can still be added in the future.

These eBPF definitions cannot go into uapi/linux/filter.h, since the names
may conflict with existing applications.

Full eBPF ISA description is in Documentation/networking/filter.txt

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
 include/linux/filter.h    |   56 +-------------------------------------
 include/uapi/linux/Kbuild |    1 +
 include/uapi/linux/bpf.h  |   65 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 55 deletions(-)
 create mode 100644 include/uapi/linux/bpf.h

diff --git a/include/linux/filter.h b/include/linux/filter.h
index bf323da77950..8f82ef3f1cdd 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -10,58 +10,12 @@
 #include <linux/workqueue.h>
 #include <uapi/linux/filter.h>
 #include <asm/cacheflush.h>
+#include <uapi/linux/bpf.h>
 
 struct sk_buff;
 struct sock;
 struct seccomp_data;
 
-/* Internally used and optimized filter representation with extended
- * instruction set based on top of classic BPF.
- */
-
-/* instruction classes */
-#define BPF_ALU64	0x07	/* alu mode in double word width */
-
-/* ld/ldx fields */
-#define BPF_DW		0x18	/* double word */
-#define BPF_XADD	0xc0	/* exclusive add */
-
-/* alu/jmp fields */
-#define BPF_MOV		0xb0	/* mov reg to reg */
-#define BPF_ARSH	0xc0	/* sign extending arithmetic shift right */
-
-/* change endianness of a register */
-#define BPF_END		0xd0	/* flags for endianness conversion: */
-#define BPF_TO_LE	0x00	/* convert to little-endian */
-#define BPF_TO_BE	0x08	/* convert to big-endian */
-#define BPF_FROM_LE	BPF_TO_LE
-#define BPF_FROM_BE	BPF_TO_BE
-
-#define BPF_JNE		0x50	/* jump != */
-#define BPF_JSGT	0x60	/* SGT is signed '>', GT in x86 */
-#define BPF_JSGE	0x70	/* SGE is signed '>=', GE in x86 */
-#define BPF_CALL	0x80	/* function call */
-#define BPF_EXIT	0x90	/* function return */
-
-/* Register numbers */
-enum {
-	BPF_REG_0 = 0,
-	BPF_REG_1,
-	BPF_REG_2,
-	BPF_REG_3,
-	BPF_REG_4,
-	BPF_REG_5,
-	BPF_REG_6,
-	BPF_REG_7,
-	BPF_REG_8,
-	BPF_REG_9,
-	BPF_REG_10,
-	__MAX_BPF_REG,
-};
-
-/* BPF has 10 general purpose 64-bit registers and stack frame. */
-#define MAX_BPF_REG	__MAX_BPF_REG
-
 /* ArgX, context and stack frame pointer register positions. Note,
  * Arg1, Arg2, Arg3, etc are used as argument mappings of function
  * calls in BPF_CALL instruction.
@@ -322,14 +276,6 @@ enum {
 #define SK_RUN_FILTER(filter, ctx) \
 	(*filter->prog->bpf_func)(ctx, filter->prog->insnsi)
 
-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 */
-};
-
 #ifdef CONFIG_COMPAT
 /* A struct sock_filter is architecture independent. */
 struct compat_sock_fprog {
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 24e9033f8b3f..fb3f7b675229 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -67,6 +67,7 @@ header-y += bfs_fs.h
 header-y += binfmts.h
 header-y += blkpg.h
 header-y += blktrace_api.h
+header-y += bpf.h
 header-y += bpqether.h
 header-y += bsg.h
 header-y += btrfs.h
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
new file mode 100644
index 000000000000..479ed0b6be16
--- /dev/null
+++ b/include/uapi/linux/bpf.h
@@ -0,0 +1,65 @@
+/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+#ifndef _UAPI__LINUX_BPF_H__
+#define _UAPI__LINUX_BPF_H__
+
+#include <linux/types.h>
+
+/* Extended instruction set based on top of classic BPF */
+
+/* instruction classes */
+#define BPF_ALU64	0x07	/* alu mode in double word width */
+
+/* ld/ldx fields */
+#define BPF_DW		0x18	/* double word */
+#define BPF_XADD	0xc0	/* exclusive add */
+
+/* alu/jmp fields */
+#define BPF_MOV		0xb0	/* mov reg to reg */
+#define BPF_ARSH	0xc0	/* sign extending arithmetic shift right */
+
+/* change endianness of a register */
+#define BPF_END		0xd0	/* flags for endianness conversion: */
+#define BPF_TO_LE	0x00	/* convert to little-endian */
+#define BPF_TO_BE	0x08	/* convert to big-endian */
+#define BPF_FROM_LE	BPF_TO_LE
+#define BPF_FROM_BE	BPF_TO_BE
+
+#define BPF_JNE		0x50	/* jump != */
+#define BPF_JSGT	0x60	/* SGT is signed '>', GT in x86 */
+#define BPF_JSGE	0x70	/* SGE is signed '>=', GE in x86 */
+#define BPF_CALL	0x80	/* function call */
+#define BPF_EXIT	0x90	/* function return */
+
+/* Register numbers */
+enum {
+	BPF_REG_0 = 0,
+	BPF_REG_1,
+	BPF_REG_2,
+	BPF_REG_3,
+	BPF_REG_4,
+	BPF_REG_5,
+	BPF_REG_6,
+	BPF_REG_7,
+	BPF_REG_8,
+	BPF_REG_9,
+	BPF_REG_10,
+	__MAX_BPF_REG,
+};
+
+/* BPF has 10 general purpose 64-bit registers and stack frame. */
+#define MAX_BPF_REG	__MAX_BPF_REG
+
+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 */
+};
+
+#endif /* _UAPI__LINUX_BPF_H__ */
-- 
1.7.9.5

Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space

[Daniel Borkmann]

On 09/03/2014 05:17 AM, Alexei Starovoitov wrote:
> allow user space to generate eBPF programs
>
> uapi/linux/bpf.h: eBPF instruction set definition
>
> linux/filter.h: the rest
>
> This patch only moves macro definitions, but practically it freezes existing
> eBPF instruction set, though new instructions can still be added in the future.
>
> These eBPF definitions cannot go into uapi/linux/filter.h, since the names
> may conflict with existing applications.
>
> Full eBPF ISA description is in Documentation/networking/filter.txt
>
> Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>

Ok, given you post the remaining two RFCs later on this window as
you indicate, I have no objections:

Acked-by: Daniel Borkmann <dborkman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space

[Daniel Borkmann]

On 09/03/2014 05:46 PM, Daniel Borkmann wrote:
...
> Ok, given you post the remaining two RFCs later on this window as
> you indicate, I have no objections:
>
> Acked-by: Daniel Borkmann <dborkman@redhat.com>

Ping, Alexei, are you still sending the patch for bpf_common.h or
do you want me to take care of this?

Cheers,
Daniel

Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space

[Alexei Starovoitov]

On Mon, Oct 13, 2014 at 10:21 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
> On 09/03/2014 05:46 PM, Daniel Borkmann wrote:
> ...
>>
>> Ok, given you post the remaining two RFCs later on this window as
>> you indicate, I have no objections:
>>
>> Acked-by: Daniel Borkmann <dborkman@redhat.com>
>
>
> Ping, Alexei, are you still sending the patch for bpf_common.h or
> do you want me to take care of this?

It's not forgotten.
I'm not sending it only because net-next is closed
and it seems to be -next material.

Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space

[Daniel Borkmann]

On 10/13/2014 11:49 PM, Alexei Starovoitov wrote:
> On Mon, Oct 13, 2014 at 10:21 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
>> On 09/03/2014 05:46 PM, Daniel Borkmann wrote:
>> ...
>>>
>>> Ok, given you post the remaining two RFCs later on this window as
>>> you indicate, I have no objections:
>>>
>>> Acked-by: Daniel Borkmann <dborkman@redhat.com>
>>
>> Ping, Alexei, are you still sending the patch for bpf_common.h or
>> do you want me to take care of this?
>
> It's not forgotten.
> I'm not sending it only because net-next is closed
> and it seems to be -next material.

Well, the point was since it's UAPI you're modifying, that it needs
to be shipped before it first gets exposed to user land ...

I think that should be reason enough ... there's no point in doing
this at a later point in time.

Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space

[Alexei Starovoitov]

On Tue, Oct 14, 2014 at 12:34 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
> On 10/13/2014 11:49 PM, Alexei Starovoitov wrote:
>>
>> On Mon, Oct 13, 2014 at 10:21 AM, Daniel Borkmann <dborkman@redhat.com>
>> wrote:
>>>
>>> On 09/03/2014 05:46 PM, Daniel Borkmann wrote:
>>> ...
>>>>
>>>>
>>>> Ok, given you post the remaining two RFCs later on this window as
>>>> you indicate, I have no objections:
>>>>
>>>> Acked-by: Daniel Borkmann <dborkman@redhat.com>
>>>
>>>
>>> Ping, Alexei, are you still sending the patch for bpf_common.h or
>>> do you want me to take care of this?
>>
>>
>> It's not forgotten.
>> I'm not sending it only because net-next is closed
>> and it seems to be -next material.
>
>
> Well, the point was since it's UAPI you're modifying, that it needs
> to be shipped before it first gets exposed to user land ...
>
> I think that should be reason enough ... there's no point in doing
> this at a later point in time.

Moving common #defines from filter.h into bpf_common.h can
be done at any point in time. For the sake of argument if
there is an app that includes both filter.h and bpf.h, it will
continue to work just fine.
Anyway, since you insist, I will send it now, though it definitely
can wait.

Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space

[Daniel Borkmann]

On 10/14/2014 10:43 AM, Alexei Starovoitov wrote:
> On Tue, Oct 14, 2014 at 12:34 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
>> On 10/13/2014 11:49 PM, Alexei Starovoitov wrote:
>>>
>>> On Mon, Oct 13, 2014 at 10:21 AM, Daniel Borkmann <dborkman@redhat.com>
>>> wrote:
>>>>
>>>> On 09/03/2014 05:46 PM, Daniel Borkmann wrote:
>>>> ...
>>>>>
>>>>> Ok, given you post the remaining two RFCs later on this window as
>>>>> you indicate, I have no objections:
>>>>>
>>>>> Acked-by: Daniel Borkmann <dborkman@redhat.com>
>>>>
>>>> Ping, Alexei, are you still sending the patch for bpf_common.h or
>>>> do you want me to take care of this?
>>>
>>> It's not forgotten.
>>> I'm not sending it only because net-next is closed
>>> and it seems to be -next material.
>>
>> Well, the point was since it's UAPI you're modifying, that it needs
>> to be shipped before it first gets exposed to user land ...
>>
>> I think that should be reason enough ... there's no point in doing
>> this at a later point in time.
>
> Moving common #defines from filter.h into bpf_common.h can
> be done at any point in time. For the sake of argument if
> there is an app that includes both filter.h and bpf.h, it will
> continue to work just fine.

Correct, but the argument was that we can _avoid_ this from the
very beginning. Thus, user space applications making use of eBPF
only need to include <linux/bpf.h>, nothing more.

Doing this at any later point in time will just lead to the need
to include both headers.