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

Hi David,

I've been thinking on the minimum first patch set.
Came up with the following two:

1st patch adds 'load 64-bit immediate' instruction which by itself
is harmless and used to load constants only. In the future we may
add pseudo variants of this insn, so user space can request
internal kernel pointer. More detailed explanation in the commit log.

2nd patch exposed eBPF ISA to user space. It moves 55 lines from
filter.h into uapi/linux/bpf.h
Though there is no way currently to load eBPF programs from user
space, this patch shows the intent that eventually it will be possible.
The main goal here is to unblock LLVM upstreaming process.
Once these two are in, I can start posting LLVM RFCs to llvmdev list
and getting compiler bits in, so by the time bpf syscall and verifier
are in, we may have LLVM backend upstreamed as well.
LLVM wouldn't care what eBPF is used for, whether syscall is used
or some other mechanism. It just compiles C into eBPF ISA.
So these two patches are sufficient to start LLVM upstreaming.

All,
why do we need all of these?
Same reason why we're still using classic BPF and keep trying to extend it.
There are places in kernel where safe dynamic programs are mandatory.
network traffic capture needs in-kernel filtering,
seccomp needs safe mini programs to sandbox applications,
tracing needs them to filter events and so on.

Few LWN articles that explain things way better than my commit logs:
http://lwn.net/Articles/599755/
http://lwn.net/Articles/603983/
http://lwn.net/Articles/606089/
http://lwn.net/Articles/575531/

The first target for eBPF is to have dtrace equivalent that can be
used in _production_. Safety of programs is paramount.
Just like performance. eBPF programs in tracing should not affect 
performance of production severs, so huge effort on optimizing last bit.

ebpf+tracing, ebpf+seccomp, ebpf+sockets are the most obvious use cases.
ebpf+ovs is the one we use in large kvm hypervisors.
40Gbps of traffic are going through these programs, so performance
and safety are vital. Performance implications ruling out run-time checks
in critical path, so verifier is large mainly because it needs to do
all the checks during static analysis.

I think next patch set will include syscall shell with minimal functionality,
syscall doc and simple test.

Alexei Starovoitov (2):
  net: filter: add "load 64-bit immediate" eBPF instruction
  net: filter: split filter.h and expose eBPF to user space

 Documentation/networking/filter.txt |    8 +++-
 arch/x86/net/bpf_jit_comp.c         |   17 ++++++++
 include/linux/filter.h              |   74 +++++++++--------------------------
 include/uapi/linux/Kbuild           |    1 +
 include/uapi/linux/bpf.h            |   65 ++++++++++++++++++++++++++++++
 kernel/bpf/core.c                   |    5 +++
 lib/test_bpf.c                      |   21 ++++++++++
 7 files changed, 135 insertions(+), 56 deletions(-)
 create mode 100644 include/uapi/linux/bpf.h

-- 
1.7.9.5

add BPF_LD_IMM64 instruction to load 64-bit immediate value into a register.
All previous instructions were 8-byte. This is first 16-byte instruction.
Two consecutive 'struct bpf_insn' blocks are interpreted as single instruction:
insn[0].code = BPF_LD | BPF_DW | BPF_IMM
insn[0].dst_reg = destination register
insn[0].imm = lower 32-bit
insn[1].code = 0
insn[1].imm = upper 32-bit
All unused fields must be zero.

Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM
which loads 32-bit immediate value into a register.

x64 JITs it as single 'movabsq %rax, imm64'
arm64 may JIT as sequence of four 'movk x0, #imm16, lsl #shift' insn

Note that old eBPF programs are binary compatible with new interpreter.

It helps eBPF programs load 64-bit constant into a register with one
instruction instead of using two registers and 4 instructions:
BPF_MOV32_IMM(R1, imm32)
BPF_ALU64_IMM(BPF_LSH, R1, 32)
BPF_MOV32_IMM(R2, imm32)
BPF_ALU64_REG(BPF_OR, R1, R2)

User space generated programs will use this instruction to load constants only.

To tell kernel that user space needs a pointer the _pseudo_ variant of
this instruction may be added later, which will use extra bits of encoding
to indicate what type of pointer user space is asking kernel to provide.
For example 'off' or 'src_reg' fields can be used for such purpose.
src_reg = 1 could mean that user space is asking kernel to validate and
load in-kernel map pointer.
src_reg = 2 could mean that user space needs readonly data section pointer
src_reg = 3 could mean that user space needs a pointer to per-cpu local data
All such future pseudo instructions will not be carrying the actual pointer
as part of the instruction, but rather will be treated as a request to kernel
to provide one. The kernel will verify the request_for_a_pointer, then
will drop pseudo marking and will store actual internal pointer inside
the instruction, so the end result is the interpreter and JITs never
see pseudo BPF_LD_IMM64 insns and only operate on single generic BPF_LD_IMM64.
User space never operates on direct pointers and verifier can easily
recognize request_for_pointer_pseudo_insn vs other instructions.

Signed-off-by: Alexei Starovoitov <redacted>
---
 Documentation/networking/filter.txt |    8 +++++++-
 arch/x86/net/bpf_jit_comp.c         |   17 +++++++++++++++++
 include/linux/filter.h              |   18 ++++++++++++++++++
 kernel/bpf/core.c                   |    5 +++++
 lib/test_bpf.c                      |   21 +++++++++++++++++++++
 5 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/filter.txt b/Documentation/networking/filter.txt
index c48a9704bda8..81916ab5d96f 100644
--- a/Documentation/networking/filter.txt
+++ b/Documentation/networking/filter.txt

@@ -951,7 +951,7 @@ Size modifier is one of ...
 
 Mode modifier is one of:
 
-  BPF_IMM  0x00  /* classic BPF only, reserved in eBPF */
+  BPF_IMM  0x00  /* used for 32-bit mov in classic BPF and 64-bit in eBPF */
   BPF_ABS  0x20
   BPF_IND  0x40
   BPF_MEM  0x60

@@ -995,6 +995,12 @@ BPF_XADD | BPF_DW | BPF_STX: lock xadd *(u64 *)(dst_reg + off16) += src_reg
 Where size is one of: BPF_B or BPF_H or BPF_W or BPF_DW. Note that 1 and
 2 byte atomic increments are not supported.
 
+eBPF has one 16-byte instruction: BPF_LD | BPF_DW | BPF_IMM which consists
+of two consecutive 'struct bpf_insn' 8-byte blocks and interpreted as single
+instruction that loads 64-bit immediate value into a dst_reg.
+Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM which loads
+32-bit immediate value into a register.
+
 Testing
 -------
 

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index b08a98c59530..98837147ee57 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c

@@ -393,6 +393,23 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
 			EMIT1_off32(add_1reg(0xB8, dst_reg), imm32);
 			break;
 
+		case BPF_LD | BPF_IMM | BPF_DW:
+			if (insn[1].code != 0 || insn[1].src_reg != 0 ||
+			    insn[1].dst_reg != 0 || insn[1].off != 0) {
+				/* verifier must catch invalid insns */
+				pr_err("invalid BPF_LD_IMM64 insn\n");
+				return -EINVAL;
+			}
+
+			/* movabsq %rax, imm64 */
+			EMIT2(add_1mod(0x48, dst_reg), add_1reg(0xB8, dst_reg));
+			EMIT(insn[0].imm, 4);
+			EMIT(insn[1].imm, 4);
+
+			insn++;
+			i++;
+			break;
+
 			/* dst %= src, dst /= src, dst %= imm32, dst /= imm32 */
 		case BPF_ALU | BPF_MOD | BPF_X:
 		case BPF_ALU | BPF_DIV | BPF_X:

diff --git a/include/linux/filter.h b/include/linux/filter.h
index a5227ab8ccb1..f3262b598262 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h

@@ -161,6 +161,24 @@ enum {
 		.off   = 0,					\
 		.imm   = IMM })
 
+/* BPF_LD_IMM64 macro encodes single 'load 64-bit immediate' insn */
+#define BPF_LD_IMM64(DST, IMM)					\
+	BPF_LD_IMM64_RAW(DST, 0, IMM)
+
+#define BPF_LD_IMM64_RAW(DST, SRC, IMM)				\
+	((struct bpf_insn) {					\
+		.code  = BPF_LD | BPF_DW | BPF_IMM,		\
+		.dst_reg = DST,					\
+		.src_reg = SRC,					\
+		.off   = 0,					\
+		.imm   = (__u32) (IMM) }),			\
+	((struct bpf_insn) {					\
+		.code  = 0, /* zero is reserved opcode */	\
+		.dst_reg = 0,					\
+		.src_reg = 0,					\
+		.off   = 0,					\
+		.imm   = ((__u64) (IMM)) >> 32 })
+
 /* Short form of mov based on type, BPF_X: dst_reg = src_reg, BPF_K: dst_reg = imm32 */
 
 #define BPF_MOV64_RAW(TYPE, DST, SRC, IMM)			\

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 7f0dbcbb34af..0434c2170f2b 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c

@@ -180,6 +180,7 @@ static unsigned int __bpf_prog_run(void *ctx, const struct bpf_insn *insn)
 		[BPF_LD | BPF_IND | BPF_W] = &&LD_IND_W,
 		[BPF_LD | BPF_IND | BPF_H] = &&LD_IND_H,
 		[BPF_LD | BPF_IND | BPF_B] = &&LD_IND_B,
+		[BPF_LD | BPF_IMM | BPF_DW] = &&LD_IMM_DW,
 	};
 	void *ptr;
 	int off;

@@ -239,6 +240,10 @@ select_insn:
 	ALU64_MOV_K:
 		DST = IMM;
 		CONT;
+	LD_IMM_DW:
+		DST = (u64) (u32) insn[0].imm | ((u64) (u32) insn[1].imm) << 32;
+		insn++;
+		CONT;
 	ALU64_ARSH_X:
 		(*(s64 *) &DST) >>= SRC;
 		CONT;

diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index 8c66c6aace04..46ab1a7ef135 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c

@@ -1735,6 +1735,27 @@ static struct bpf_test tests[] = {
 		{ },
 		{ { 1, 0 } },
 	},
+	{
+		"load 64-bit immediate",
+		.u.insns_int = {
+			BPF_LD_IMM64(R1, 0x567800001234L),
+			BPF_MOV64_REG(R2, R1),
+			BPF_MOV64_REG(R3, R2),
+			BPF_ALU64_IMM(BPF_RSH, R2, 32),
+			BPF_ALU64_IMM(BPF_LSH, R3, 32),
+			BPF_ALU64_IMM(BPF_RSH, R3, 32),
+			BPF_ALU64_IMM(BPF_MOV, R0, 0),
+			BPF_JMP_IMM(BPF_JEQ, R2, 0x5678, 1),
+			BPF_EXIT_INSN(),
+			BPF_JMP_IMM(BPF_JEQ, R3, 0x1234, 1),
+			BPF_EXIT_INSN(),
+			BPF_ALU64_IMM(BPF_MOV, R0, 1),
+			BPF_EXIT_INSN(),
+		},
+		INTERNAL,
+		{ },
+		{ { 0, 1 } }
+	},
 };
 
 static struct net_device dev;

-- 
1.7.9.5

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>
---
 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 f3262b598262..3150666cd4b9 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h

@@ -9,53 +9,7 @@
 #include <linux/skbuff.h>
 #include <linux/workqueue.h>
 #include <uapi/linux/filter.h>
-
-/* 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
+#include <uapi/linux/bpf.h>
 
 /* ArgX, context and stack frame pointer register positions. Note,
  * Arg1, Arg2, Arg3, etc are used as argument mappings of function

@@ -317,14 +271,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

On 08/27/2014 10:37 PM, Alexei Starovoitov wrote:

allow user space to generate eBPF programs

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

linux/filter.h: the rest

Very sorry for being late, but just a thought since we're touching user
space headers anyway ...

Wouldn't it be more consistent to have it organized as follows ...

  - uapi/linux/bpf.h    : classic BPF instruction set parts only
  - uapi/linux/ebpf.h   : eBPF instruction set definition (which also
                          includes uapi/linux/bpf.h though)
... and have ...

  - uapi/linux/filter.h : just include uapi/linux/bpf.h but rest is empty

That way, it would be more consistent ...

Old legacy application can stay with linux/filter.h; new applications
based on their needs can choose between linux/{e,}bpf.h and in the kernel,
we can just include linux/ebpf.h.

Right now, it seems, an eBPF user space program would need to include
2 header files in user space (linux/filter.h, linux/bpf.h) which I find
a bit confusing.

If you want, I could also take care of that later, but just thinking out
loudly ...

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 <redacted>

On Fri, Aug 29, 2014 at 10:39 AM, Daniel Borkmann [off-list ref] wrote:

On 08/27/2014 10:37 PM, Alexei Starovoitov wrote:

quoted

allow user space to generate eBPF programs

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

linux/filter.h: the rest

Very sorry for being late, but just a thought since we're touching user
space headers anyway ...

Wouldn't it be more consistent to have it organized as follows ...

 - uapi/linux/bpf.h    : classic BPF instruction set parts only
 - uapi/linux/ebpf.h   : eBPF instruction set definition (which also
                         includes uapi/linux/bpf.h though)
... and have ...

 - uapi/linux/filter.h : just include uapi/linux/bpf.h but rest is empty

That way, it would be more consistent ...

Old legacy application can stay with linux/filter.h; new applications
based on their needs can choose between linux/{e,}bpf.h and in the kernel,
we can just include linux/ebpf.h.

Right now, it seems, an eBPF user space program would need to include
2 header files in user space (linux/filter.h, linux/bpf.h) which I find
a bit confusing.

It's been bugging me as well, but I suspect having it the way you
described won't work. Mainly because we cannot do include <uapi/..>
inside uapi/*.h, so we would need to do include <linux/bpf.h>
inside uapi/linux/filter.h, but that will cause serious include path
confusion. That was the reason I didn't simply do include <linux/filter.h>
inside uapi/linux/bpf.h

Also I really dislike 'ebpf' name in all lower case. If we make such header
file name, we would need to rename all macros and function names
to EBPF_... which I find very ugly looking. I think all good abbreviations are
three letters :)
So I very much prefer bpf.h as a main file name.
Later we can move some of old classic BPF defines into
uapi/linux/bpf_common.h and then include it in both uapi/linux/bpf.h
and in uapi/linux/filter.h, then the nuisance of two include files for
user space will go away. Classic users will keep using linux/filter.h
and new apps will include linux/bpf.h only.
I think we should probably do such header optimization later and very carefully.
I'm a bit afraid to touch uapi/linux/filter.h since it's used in so
many user apps.

On 08/29/2014 08:02 PM, Alexei Starovoitov wrote:

On Fri, Aug 29, 2014 at 10:39 AM, Daniel Borkmann [off-list ref] wrote:

quoted

On 08/27/2014 10:37 PM, Alexei Starovoitov wrote:

quoted

allow user space to generate eBPF programs

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

linux/filter.h: the rest

Very sorry for being late, but just a thought since we're touching user
space headers anyway ...

Wouldn't it be more consistent to have it organized as follows ...

  - uapi/linux/bpf.h    : classic BPF instruction set parts only
  - uapi/linux/ebpf.h   : eBPF instruction set definition (which also
                          includes uapi/linux/bpf.h though)
... and have ...

  - uapi/linux/filter.h : just include uapi/linux/bpf.h but rest is empty

That way, it would be more consistent ...

Old legacy application can stay with linux/filter.h; new applications
based on their needs can choose between linux/{e,}bpf.h and in the kernel,
we can just include linux/ebpf.h.

Right now, it seems, an eBPF user space program would need to include
2 header files in user space (linux/filter.h, linux/bpf.h) which I find
a bit confusing.

It's been bugging me as well, but I suspect having it the way you
described won't work. Mainly because we cannot do include <uapi/..>
inside uapi/*.h, so we would need to do include <linux/bpf.h>
inside uapi/linux/filter.h, but that will cause serious include path
confusion. That was the reason I didn't simply do include <linux/filter.h>
inside uapi/linux/bpf.h

Also I really dislike 'ebpf' name in all lower case. If we make such header
file name, we would need to rename all macros and function names
to EBPF_... which I find very ugly looking. I think all good abbreviations are
three letters :)

I don't think we would have to name defines that way, really, that would be
terrible. We can keep them simply *as is*. Not sure though why bpf.h + ebpf.h
would be that bad. ;) I haven't tried it out yet, but if we would indeed run
into a name collision, above proposal would resolve that.

On Fri, Aug 29, 2014 at 3:24 PM, Daniel Borkmann [off-list ref] wrote:

quoted

Also I really dislike 'ebpf' name in all lower case. If we make such
header
file name, we would need to rename all macros and function names
to EBPF_... which I find very ugly looking. I think all good abbreviations
are
three letters :)

I don't think we would have to name defines that way, really, that would be
terrible. We can keep them simply *as is*. Not sure though why bpf.h +
ebpf.h
would be that bad. ;) I haven't tried it out yet, but if we would indeed run
into a name collision, above proposal would resolve that.

imo it's a consistency issue. If main uapi header is ebpf.h then
corresponding kernel internal header should be ebpf.h as well
and kernel/ebpf/ directory and so on.
That's why I insist on uapi/linux/bpf.h and no other name.
Note I didn't move any of the BPF_ALU64_REG, BPF_ALU32_IMM
macros from linux/filter.h. Without them my verifier testsuite
won't compile, so more lines would be added to bpf.h in the future.
At that time we can take 45 lines out of uapi/linux/filter.h and move
them into bpf_common.h. My request is let's not fight about it
right now. We didn't even cross the bridge yet and arguing
about beauty of user apps that come in 30 patches from now...

These two patches are about _intent_ of making eBPF usable
from userspace, so I can move along with llvm.
Also worth noting that llmv will not be including this uapi/linux/bpf.h
It has its own infra to generate instructions. Look at:
tools/bpf/llvm/lib/Target/BPF/BPFInstrInfo.td
it's a special 'table definition' language for describing bits and fields
of instructions.
So these two patches are mainly establishing _intent_ and bpf.h file
name. That's why I'm so paranoid about naming.

btw, I've spent last two days writing syscall manpage :(
What is the best way to present it for review?
If I just attach it raw, it's unreadable... I can include a link
to html page, but man2html produces ugly pages comparing
to what 'man' command shows. Any nice man converters
that generate stuff seen on man7.org ?

On 08/30/2014 01:01 AM, Alexei Starovoitov wrote:
...

btw, I've spent last two days writing syscall manpage :(
What is the best way to present it for review?
If I just attach it raw, it's unreadable... I can include a link
to html page, but man2html produces ugly pages comparing
to what 'man' command shows. Any nice man converters
that generate stuff seen on man7.org ?

What about :

   man foo > bar

And then copy that into your mail client?

On 08/30/2014 01:01 AM, Alexei Starovoitov wrote:
...

imo it's a consistency issue. If main uapi header is ebpf.h then
corresponding kernel internal header should be ebpf.h as well
and kernel/ebpf/ directory and so on.

I don't think that has to be enforced, but fair enough, if you
feel that way.

That's why I insist on uapi/linux/bpf.h and no other name.

...

them into bpf_common.h. My request is let's not fight about it
right now. We didn't even cross the bridge yet and arguing
about beauty of user apps that come in 30 patches from now...

...

So these two patches are mainly establishing _intent_ and bpf.h file
name. That's why I'm so paranoid about naming.

I understand, and that's why I said it could also be resolved later
in my previous email (at latest before it gets shipped though), but
just to give this some thought ...

I have attached one example, it doesn't have to be that way, but it's
one possibility if you want to stay with linux/bpf.h only.