[PATCHv2 perf/core 0/2] libbpf: Synchronize implementations

[PATCHv2 perf/core 0/2] libbpf: Synchronize implementations

[Joe Stringer]

Update tools/lib/bpf to provide more functionality and improve interoperation
with other tools that generate and use eBPF code:
* The kernel uapi headers are a bit newer than the version in the tools/
  directory; synchronize those.
* samples/bpf/libbpf* has a bit more functionality than tools/lib/bpf, so
  extend tools/lib/bpf/bpf* with these functions to bring them into parity.

I've got a separate series to update samples/bpf/* to rely on these libraries,
but there's a conflict with davem's tree at the moment so I suppose that the
way forward is to get these patches through first, then take the samples
through net-next at a later time.

---
v2: Don't shift non-bpf code into libbpf.
    Drop the patch to synchronize ELF definitions with tc.
v1: https://www.mail-archive.com/netdev@vger.kernel.org/msg135088.html
    First post.

Joe Stringer (2):
  tools lib bpf: Sync {tools,}/include/uapi/linux/bpf.h
  tools lib bpf: Sync with samples/bpf/libbpf

 tools/include/uapi/linux/bpf.h |  51 +++++++++++
 tools/lib/bpf/bpf.c            | 107 +++++++++++++++++-----
 tools/lib/bpf/bpf.h            | 202 +++++++++++++++++++++++++++++++++++++++--
 tools/lib/bpf/libbpf.c         |   3 +-
 4 files changed, 330 insertions(+), 33 deletions(-)

-- 
2.9.3

[PATCHv2 perf/core 1/2] tools lib bpf: Sync {tools,}/include/uapi/linux/bpf.h

[Joe Stringer]

The tools version of this header is out of date; update it to the latest
version from the kernel headers.

Signed-off-by: Joe Stringer <joe@ovn.org>
---
v2: No change.
---
 tools/include/uapi/linux/bpf.h | 51 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 9e5fc168c8a3..f09c70b97eca 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -95,6 +95,7 @@ enum bpf_prog_type {
 	BPF_PROG_TYPE_SCHED_ACT,
 	BPF_PROG_TYPE_TRACEPOINT,
 	BPF_PROG_TYPE_XDP,
+	BPF_PROG_TYPE_PERF_EVENT,
 };
 
 #define BPF_PSEUDO_MAP_FD	1
@@ -375,6 +376,56 @@ enum bpf_func_id {
 	 */
 	BPF_FUNC_probe_write_user,
 
+	/**
+	 * bpf_current_task_under_cgroup(map, index) - Check cgroup2 membership of current task
+	 * @map: pointer to bpf_map in BPF_MAP_TYPE_CGROUP_ARRAY type
+	 * @index: index of the cgroup in the bpf_map
+	 * Return:
+	 *   == 0 current failed the cgroup2 descendant test
+	 *   == 1 current succeeded the cgroup2 descendant test
+	 *    < 0 error
+	 */
+	BPF_FUNC_current_task_under_cgroup,
+
+	/**
+	 * bpf_skb_change_tail(skb, len, flags)
+	 * The helper will resize the skb to the given new size,
+	 * to be used f.e. with control messages.
+	 * @skb: pointer to skb
+	 * @len: new skb length
+	 * @flags: reserved
+	 * Return: 0 on success or negative error
+	 */
+	BPF_FUNC_skb_change_tail,
+
+	/**
+	 * bpf_skb_pull_data(skb, len)
+	 * The helper will pull in non-linear data in case the
+	 * skb is non-linear and not all of len are part of the
+	 * linear section. Only needed for read/write with direct
+	 * packet access.
+	 * @skb: pointer to skb
+	 * @len: len to make read/writeable
+	 * Return: 0 on success or negative error
+	 */
+	BPF_FUNC_skb_pull_data,
+
+	/**
+	 * bpf_csum_update(skb, csum)
+	 * Adds csum into skb->csum in case of CHECKSUM_COMPLETE.
+	 * @skb: pointer to skb
+	 * @csum: csum to add
+	 * Return: csum on success or negative error
+	 */
+	BPF_FUNC_csum_update,
+
+	/**
+	 * bpf_set_hash_invalid(skb)
+	 * Invalidate current skb>hash.
+	 * @skb: pointer to skb
+	 */
+	BPF_FUNC_set_hash_invalid,
+
 	__BPF_FUNC_MAX_ID,
 };
 
-- 
2.9.3

[PATCHv2 perf/core 2/2] tools lib bpf: Sync with samples/bpf/libbpf

[Joe Stringer]

Extend the tools/ version of libbpf to include all of the functionality
provided in the samples/bpf version.

Signed-off-by: Joe Stringer <joe@ovn.org>
---
v2: Don't shift non-bpf changes across.
    Various type cleanups, removal of extraneous declarations
---
 tools/lib/bpf/bpf.c    | 107 ++++++++++++++++++++------
 tools/lib/bpf/bpf.h    | 202 +++++++++++++++++++++++++++++++++++++++++++++++--
 tools/lib/bpf/libbpf.c |   3 +-
 3 files changed, 279 insertions(+), 33 deletions(-)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 4212ed62235b..5e061851ac00 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -20,10 +20,17 @@
  */
 
 #include <stdlib.h>
-#include <memory.h>
+#include <stdio.h>
 #include <unistd.h>
 #include <asm/unistd.h>
+#include <string.h>
+#include <linux/netlink.h>
 #include <linux/bpf.h>
+#include <errno.h>
+#include <net/ethernet.h>
+#include <net/if.h>
+#include <linux/if_packet.h>
+#include <arpa/inet.h>
 #include "bpf.h"
 
 /*
@@ -53,24 +60,71 @@ static int sys_bpf(enum bpf_cmd cmd, union bpf_attr *attr,
 	return syscall(__NR_bpf, cmd, attr, size);
 }
 
-int bpf_create_map(enum bpf_map_type map_type, int key_size,
-		   int value_size, int max_entries)
+int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size,
+		   int max_entries, int map_flags)
 {
-	union bpf_attr attr;
+	union bpf_attr attr = {
+		.map_type = map_type,
+		.key_size = key_size,
+		.value_size = value_size,
+		.max_entries = max_entries,
+		.map_flags = map_flags,
+	};
 
-	memset(&attr, '\0', sizeof(attr));
+	return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
+}
 
-	attr.map_type = map_type;
-	attr.key_size = key_size;
-	attr.value_size = value_size;
-	attr.max_entries = max_entries;
+int bpf_update_elem(int fd, void *key, void *value, unsigned long long flags)
+{
+	union bpf_attr attr = {
+		.map_fd = fd,
+		.key = ptr_to_u64(key),
+		.value = ptr_to_u64(value),
+		.flags = flags,
+	};
 
-	return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
+	return sys_bpf(BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr));
+}
+
+int bpf_lookup_elem(int fd, void *key, void *value)
+{
+	union bpf_attr attr = {
+		.map_fd = fd,
+		.key = ptr_to_u64(key),
+		.value = ptr_to_u64(value),
+	};
+
+	return sys_bpf(BPF_MAP_LOOKUP_ELEM, &attr, sizeof(attr));
+}
+
+int bpf_delete_elem(int fd, void *key)
+{
+	union bpf_attr attr = {
+		.map_fd = fd,
+		.key = ptr_to_u64(key),
+	};
+
+	return sys_bpf(BPF_MAP_DELETE_ELEM, &attr, sizeof(attr));
 }
 
-int bpf_load_program(enum bpf_prog_type type, struct bpf_insn *insns,
-		     size_t insns_cnt, char *license,
-		     u32 kern_version, char *log_buf, size_t log_buf_sz)
+int bpf_get_next_key(int fd, void *key, void *next_key)
+{
+	union bpf_attr attr = {
+		.map_fd = fd,
+		.key = ptr_to_u64(key),
+		.next_key = ptr_to_u64(next_key),
+	};
+
+	return sys_bpf(BPF_MAP_GET_NEXT_KEY, &attr, sizeof(attr));
+}
+
+#define ROUND_UP(x, n) (((x) + (n) - 1u) & ~((n) - 1u))
+
+
+int bpf_load_program(enum bpf_prog_type type,
+		     const struct bpf_insn *insns, int insns_cnt,
+		     const char *license, int kern_version,
+		     char *log_buf, size_t log_buf_sz)
 {
 	int fd;
 	union bpf_attr attr;
@@ -78,8 +132,8 @@ int bpf_load_program(enum bpf_prog_type type, struct bpf_insn *insns,
 	bzero(&attr, sizeof(attr));
 	attr.prog_type = type;
 	attr.insn_cnt = (__u32)insns_cnt;
-	attr.insns = ptr_to_u64(insns);
-	attr.license = ptr_to_u64(license);
+	attr.insns = ptr_to_u64((void *)insns);
+	attr.license = ptr_to_u64((void *)license);
 	attr.log_buf = ptr_to_u64(NULL);
 	attr.log_size = 0;
 	attr.log_level = 0;
@@ -97,16 +151,21 @@ int bpf_load_program(enum bpf_prog_type type, struct bpf_insn *insns,
 	return sys_bpf(BPF_PROG_LOAD, &attr, sizeof(attr));
 }
 
-int bpf_map_update_elem(int fd, void *key, void *value,
-			u64 flags)
+int bpf_obj_pin(int fd, const char *pathname)
 {
-	union bpf_attr attr;
+	union bpf_attr attr = {
+		.pathname	= ptr_to_u64((void *)pathname),
+		.bpf_fd		= fd,
+	};
 
-	bzero(&attr, sizeof(attr));
-	attr.map_fd = fd;
-	attr.key = ptr_to_u64(key);
-	attr.value = ptr_to_u64(value);
-	attr.flags = flags;
+	return sys_bpf(BPF_OBJ_PIN, &attr, sizeof(attr));
+}
 
-	return sys_bpf(BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr));
+int bpf_obj_get(const char *pathname)
+{
+	union bpf_attr attr = {
+		.pathname	= ptr_to_u64((void *)pathname),
+	};
+
+	return sys_bpf(BPF_OBJ_GET, &attr, sizeof(attr));
 }
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index e8ba54087497..4dba36995771 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -23,16 +23,202 @@
 
 #include <linux/bpf.h>
 
+struct bpf_insn;
+
 int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size,
-		   int max_entries);
+		   int max_entries, int map_flags);
+int bpf_update_elem(int fd, void *key, void *value, unsigned long long flags);
+int bpf_lookup_elem(int fd, void *key, void *value);
+int bpf_delete_elem(int fd, void *key);
+int bpf_get_next_key(int fd, void *key, void *next_key);
+
+int bpf_load_program(enum bpf_prog_type prog_type,
+		     const struct bpf_insn *insns, int insn_len,
+		     const char *license, int kern_version,
+		     char *log_buf, size_t log_buf_sz);
+
+int bpf_obj_pin(int fd, const char *pathname);
+int bpf_obj_get(const char *pathname);
 
-/* Recommend log buffer size */
 #define BPF_LOG_BUF_SIZE 65536
-int bpf_load_program(enum bpf_prog_type type, struct bpf_insn *insns,
-		     size_t insns_cnt, char *license,
-		     u32 kern_version, char *log_buf,
-		     size_t log_buf_sz);
 
-int bpf_map_update_elem(int fd, void *key, void *value,
-			u64 flags);
+/* ALU ops on registers, bpf_add|sub|...: dst_reg += src_reg */
+
+#define BPF_ALU64_REG(OP, DST, SRC)				\
+	((struct bpf_insn) {					\
+		.code  = BPF_ALU64 | BPF_OP(OP) | BPF_X,	\
+		.dst_reg = DST,					\
+		.src_reg = SRC,					\
+		.off   = 0,					\
+		.imm   = 0 })
+
+#define BPF_ALU32_REG(OP, DST, SRC)				\
+	((struct bpf_insn) {					\
+		.code  = BPF_ALU | BPF_OP(OP) | BPF_X,		\
+		.dst_reg = DST,					\
+		.src_reg = SRC,					\
+		.off   = 0,					\
+		.imm   = 0 })
+
+/* ALU ops on immediates, bpf_add|sub|...: dst_reg += imm32 */
+
+#define BPF_ALU64_IMM(OP, DST, IMM)				\
+	((struct bpf_insn) {					\
+		.code  = BPF_ALU64 | BPF_OP(OP) | BPF_K,	\
+		.dst_reg = DST,					\
+		.src_reg = 0,					\
+		.off   = 0,					\
+		.imm   = IMM })
+
+#define BPF_ALU32_IMM(OP, DST, IMM)				\
+	((struct bpf_insn) {					\
+		.code  = BPF_ALU | BPF_OP(OP) | BPF_K,		\
+		.dst_reg = DST,					\
+		.src_reg = 0,					\
+		.off   = 0,					\
+		.imm   = IMM })
+
+/* Short form of mov, dst_reg = src_reg */
+
+#define BPF_MOV64_REG(DST, SRC)					\
+	((struct bpf_insn) {					\
+		.code  = BPF_ALU64 | BPF_MOV | BPF_X,		\
+		.dst_reg = DST,					\
+		.src_reg = SRC,					\
+		.off   = 0,					\
+		.imm   = 0 })
+
+#define BPF_MOV32_REG(DST, SRC)					\
+	((struct bpf_insn) {					\
+		.code  = BPF_ALU | BPF_MOV | BPF_X,		\
+		.dst_reg = DST,					\
+		.src_reg = SRC,					\
+		.off   = 0,					\
+		.imm   = 0 })
+
+/* Short form of mov, dst_reg = imm32 */
+
+#define BPF_MOV64_IMM(DST, IMM)					\
+	((struct bpf_insn) {					\
+		.code  = BPF_ALU64 | BPF_MOV | BPF_K,		\
+		.dst_reg = DST,					\
+		.src_reg = 0,					\
+		.off   = 0,					\
+		.imm   = IMM })
+
+#define BPF_MOV32_IMM(DST, IMM)					\
+	((struct bpf_insn) {					\
+		.code  = BPF_ALU | BPF_MOV | BPF_K,		\
+		.dst_reg = DST,					\
+		.src_reg = 0,					\
+		.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 })
+
+#ifndef BPF_PSEUDO_MAP_FD
+# define BPF_PSEUDO_MAP_FD	1
+#endif
+
+/* pseudo BPF_LD_IMM64 insn used to refer to process-local map_fd */
+#define BPF_LD_MAP_FD(DST, MAP_FD)				\
+	BPF_LD_IMM64_RAW(DST, BPF_PSEUDO_MAP_FD, MAP_FD)
+
+
+/* Direct packet access, R0 = *(uint *) (skb->data + imm32) */
+
+#define BPF_LD_ABS(SIZE, IMM)					\
+	((struct bpf_insn) {					\
+		.code  = BPF_LD | BPF_SIZE(SIZE) | BPF_ABS,	\
+		.dst_reg = 0,					\
+		.src_reg = 0,					\
+		.off   = 0,					\
+		.imm   = IMM })
+
+/* Memory load, dst_reg = *(uint *) (src_reg + off16) */
+
+#define BPF_LDX_MEM(SIZE, DST, SRC, OFF)			\
+	((struct bpf_insn) {					\
+		.code  = BPF_LDX | BPF_SIZE(SIZE) | BPF_MEM,	\
+		.dst_reg = DST,					\
+		.src_reg = SRC,					\
+		.off   = OFF,					\
+		.imm   = 0 })
+
+/* Memory store, *(uint *) (dst_reg + off16) = src_reg */
+
+#define BPF_STX_MEM(SIZE, DST, SRC, OFF)			\
+	((struct bpf_insn) {					\
+		.code  = BPF_STX | BPF_SIZE(SIZE) | BPF_MEM,	\
+		.dst_reg = DST,					\
+		.src_reg = SRC,					\
+		.off   = OFF,					\
+		.imm   = 0 })
+
+/* Memory store, *(uint *) (dst_reg + off16) = imm32 */
+
+#define BPF_ST_MEM(SIZE, DST, OFF, IMM)				\
+	((struct bpf_insn) {					\
+		.code  = BPF_ST | BPF_SIZE(SIZE) | BPF_MEM,	\
+		.dst_reg = DST,					\
+		.src_reg = 0,					\
+		.off   = OFF,					\
+		.imm   = IMM })
+
+/* Conditional jumps against registers, if (dst_reg 'op' src_reg) goto pc + off16 */
+
+#define BPF_JMP_REG(OP, DST, SRC, OFF)				\
+	((struct bpf_insn) {					\
+		.code  = BPF_JMP | BPF_OP(OP) | BPF_X,		\
+		.dst_reg = DST,					\
+		.src_reg = SRC,					\
+		.off   = OFF,					\
+		.imm   = 0 })
+
+/* Conditional jumps against immediates, if (dst_reg 'op' imm32) goto pc + off16 */
+
+#define BPF_JMP_IMM(OP, DST, IMM, OFF)				\
+	((struct bpf_insn) {					\
+		.code  = BPF_JMP | BPF_OP(OP) | BPF_K,		\
+		.dst_reg = DST,					\
+		.src_reg = 0,					\
+		.off   = OFF,					\
+		.imm   = IMM })
+
+/* Raw code statement block */
+
+#define BPF_RAW_INSN(CODE, DST, SRC, OFF, IMM)			\
+	((struct bpf_insn) {					\
+		.code  = CODE,					\
+		.dst_reg = DST,					\
+		.src_reg = SRC,					\
+		.off   = OFF,					\
+		.imm   = IMM })
+
+/* Program exit */
+
+#define BPF_EXIT_INSN()						\
+	((struct bpf_insn) {					\
+		.code  = BPF_JMP | BPF_EXIT,			\
+		.dst_reg = 0,					\
+		.src_reg = 0,					\
+		.off   = 0,					\
+		.imm   = 0 })
+
 #endif
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b699aea9a025..951e11023554 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -796,7 +796,8 @@ bpf_object__create_maps(struct bpf_object *obj)
 		*pfd = bpf_create_map(def->type,
 				      def->key_size,
 				      def->value_size,
-				      def->max_entries);
+				      def->max_entries,
+				      0);
 		if (*pfd < 0) {
 			size_t j;
 			int err = *pfd;
-- 
2.9.3

Re: [PATCHv2 perf/core 1/2] tools lib bpf: Sync {tools,}/include/uapi/linux/bpf.h

[Wangnan (F)]

On 2016/11/17 1:43, Joe Stringer wrote:
> The tools version of this header is out of date; update it to the latest
> version from the kernel headers.
>
> Signed-off-by: Joe Stringer <joe@ovn.org>
> ---
> v2: No change.
> ---
>   tools/include/uapi/linux/bpf.h | 51 ++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 51 insertions(+)

Acked-by: Wang Nan <wangnan0@huawei.com>

> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 9e5fc168c8a3..f09c70b97eca 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -95,6 +95,7 @@ enum bpf_prog_type {
>   	BPF_PROG_TYPE_SCHED_ACT,
>   	BPF_PROG_TYPE_TRACEPOINT,
>   	BPF_PROG_TYPE_XDP,
> +	BPF_PROG_TYPE_PERF_EVENT,
>   };
>   
>   #define BPF_PSEUDO_MAP_FD	1
> @@ -375,6 +376,56 @@ enum bpf_func_id {
>   	 */
>   	BPF_FUNC_probe_write_user,
>   
> +	/**
> +	 * bpf_current_task_under_cgroup(map, index) - Check cgroup2 membership of current task
> +	 * @map: pointer to bpf_map in BPF_MAP_TYPE_CGROUP_ARRAY type
> +	 * @index: index of the cgroup in the bpf_map
> +	 * Return:
> +	 *   == 0 current failed the cgroup2 descendant test
> +	 *   == 1 current succeeded the cgroup2 descendant test
> +	 *    < 0 error
> +	 */
> +	BPF_FUNC_current_task_under_cgroup,
> +
> +	/**
> +	 * bpf_skb_change_tail(skb, len, flags)
> +	 * The helper will resize the skb to the given new size,
> +	 * to be used f.e. with control messages.
> +	 * @skb: pointer to skb
> +	 * @len: new skb length
> +	 * @flags: reserved
> +	 * Return: 0 on success or negative error
> +	 */
> +	BPF_FUNC_skb_change_tail,
> +
> +	/**
> +	 * bpf_skb_pull_data(skb, len)
> +	 * The helper will pull in non-linear data in case the
> +	 * skb is non-linear and not all of len are part of the
> +	 * linear section. Only needed for read/write with direct
> +	 * packet access.
> +	 * @skb: pointer to skb
> +	 * @len: len to make read/writeable
> +	 * Return: 0 on success or negative error
> +	 */
> +	BPF_FUNC_skb_pull_data,
> +
> +	/**
> +	 * bpf_csum_update(skb, csum)
> +	 * Adds csum into skb->csum in case of CHECKSUM_COMPLETE.
> +	 * @skb: pointer to skb
> +	 * @csum: csum to add
> +	 * Return: csum on success or negative error
> +	 */
> +	BPF_FUNC_csum_update,
> +
> +	/**
> +	 * bpf_set_hash_invalid(skb)
> +	 * Invalidate current skb>hash.
> +	 * @skb: pointer to skb
> +	 */
> +	BPF_FUNC_set_hash_invalid,
> +
>   	__BPF_FUNC_MAX_ID,
>   };
>   

Re: [PATCHv2 perf/core 2/2] tools lib bpf: Sync with samples/bpf/libbpf

[Wangnan (F)]

I'm also working on improving bpf.c. Please have a look at:

https://lkml.org/lkml/2016/11/14/1078

Since bpf.c is simple, I think we can add more functions and fixes
gradually, instead of a full copy.

See my inline comment below.

On 2016/11/17 1:43, Joe Stringer wrote:
> Extend the tools/ version of libbpf to include all of the functionality
> provided in the samples/bpf version.
>
> Signed-off-by: Joe Stringer <joe@ovn.org>
> ---
> v2: Don't shift non-bpf changes across.
>      Various type cleanups, removal of extraneous declarations
> ---
>   tools/lib/bpf/bpf.c    | 107 ++++++++++++++++++++------
>   tools/lib/bpf/bpf.h    | 202 +++++++++++++++++++++++++++++++++++++++++++++++--
>   tools/lib/bpf/libbpf.c |   3 +-
>   3 files changed, 279 insertions(+), 33 deletions(-)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 4212ed62235b..5e061851ac00 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -20,10 +20,17 @@
>    */
>   
>   #include <stdlib.h>
> -#include <memory.h>
> +#include <stdio.h>
>   #include <unistd.h>
>   #include <asm/unistd.h>
> +#include <string.h>
> +#include <linux/netlink.h>
>   #include <linux/bpf.h>
> +#include <errno.h>
> +#include <net/ethernet.h>
> +#include <net/if.h>
> +#include <linux/if_packet.h>
> +#include <arpa/inet.h>
>   #include "bpf.h"
>   

Why we need these network related headers?

>   /*
> @@ -53,24 +60,71 @@ static int sys_bpf(enum bpf_cmd cmd, union bpf_attr *attr,
>   	return syscall(__NR_bpf, cmd, attr, size);
>   }
>   
> -int bpf_create_map(enum bpf_map_type map_type, int key_size,
> -		   int value_size, int max_entries)
> +int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size,
> +		   int max_entries, int map_flags)
>   {
> -	union bpf_attr attr;
> +	union bpf_attr attr = {
> +		.map_type = map_type,
> +		.key_size = key_size,
> +		.value_size = value_size,
> +		.max_entries = max_entries,
> +		.map_flags = map_flags,
> +	};
>   
> -	memset(&attr, '\0', sizeof(attr));
> +	return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
> +}
>   

I lost map_flags in original bpf.c. Thanks to your patch. map_flags is 
useful
when creating BPF_MAP_TYPE_PERCPU_HASH: BPF_F_NO_PREALLOC is meanful in this
case.

Although it is okay in samples, I still prefer a explicit bzero() or 
memset(),
because kernel checks if unused field in this union is zero. However 
I'll check
c standard to see how unused field would be initialized.


<SNIP>

> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index e8ba54087497..4dba36995771 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -23,16 +23,202 @@
>   
>   #include <linux/bpf.h>
>   
> +struct bpf_insn;
> +
>   int bpf_create_map(enum bpf_map_type map_type, int key_size, int value_size,
> -		   int max_entries);
> +		   int max_entries, int map_flags);
> +int bpf_update_elem(int fd, void *key, void *value, unsigned long long flags);
> +int bpf_lookup_elem(int fd, void *key, void *value);
> +int bpf_delete_elem(int fd, void *key);
> +int bpf_get_next_key(int fd, void *key, void *next_key);
> +
> +int bpf_load_program(enum bpf_prog_type prog_type,
> +		     const struct bpf_insn *insns, int insn_len,
> +		     const char *license, int kern_version,
> +		     char *log_buf, size_t log_buf_sz);
> +
> +int bpf_obj_pin(int fd, const char *pathname);
> +int bpf_obj_get(const char *pathname);
>   
> -/* Recommend log buffer size */
>   #define BPF_LOG_BUF_SIZE 65536
> -int bpf_load_program(enum bpf_prog_type type, struct bpf_insn *insns,
> -		     size_t insns_cnt, char *license,
> -		     u32 kern_version, char *log_buf,
> -		     size_t log_buf_sz);
>   
> -int bpf_map_update_elem(int fd, void *key, void *value,
> -			u64 flags);
> +/* ALU ops on registers, bpf_add|sub|...: dst_reg += src_reg */
> +
> +#define BPF_ALU64_REG(OP, DST, SRC)				\
> +	((struct bpf_insn) {					\
> +		.code  = BPF_ALU64 | BPF_OP(OP) | BPF_X,	\
> +		.dst_reg = DST,					\
> +		.src_reg = SRC,					\
> +		.off   = 0,					\
> +		.imm   = 0 })
> +

Should we define these macros here? They are in include/linux/filter.h
and duplicated in tools/include/linux/filter.h. Redefining them here
would cause conflict.

Thank you.

Re: [PATCHv2 perf/core 2/2] tools lib bpf: Sync with samples/bpf/libbpf

[Joe Stringer]

On 16 November 2016 at 18:10, Wangnan (F) <wangnan0@huawei.com> wrote:
> I'm also working on improving bpf.c. Please have a look at:
>
> https://lkml.org/lkml/2016/11/14/1078
>
> Since bpf.c is simple, I think we can add more functions and fixes
> gradually, instead of a full copy.
>
> See my inline comment below.

Ah, I missed this, my apologies. It looks like it will provide much of
what I need, I can reassess this patch with your series in mind.

One comment though for your patch (I don't have the original thread to
respond to unfortunately): The map_pin and map_get functions in your
patch series can be used to pin progs too, so maybe there is a better
name? You'll see that this patch uses bpf_obj_{pin,get}() - although I
wouldn't want those to be confused with the libbpf.c objects so maybe
there's a clearer name that could be used.

I also have some patches to rework the samples/bpf/* code to use
libbpf instead of the sample code that is there, is it worth me
submitting that? It will need to wait for your patch to go in, plus a
merge with davem's tree.

>
> On 2016/11/17 1:43, Joe Stringer wrote:
>>
>> Extend the tools/ version of libbpf to include all of the functionality
>> provided in the samples/bpf version.
>>
>> Signed-off-by: Joe Stringer <joe@ovn.org>
>> ---
>> v2: Don't shift non-bpf changes across.
>>      Various type cleanups, removal of extraneous declarations
>> ---
>>   tools/lib/bpf/bpf.c    | 107 ++++++++++++++++++++------
>>   tools/lib/bpf/bpf.h    | 202
>> +++++++++++++++++++++++++++++++++++++++++++++++--
>>   tools/lib/bpf/libbpf.c |   3 +-
>>   3 files changed, 279 insertions(+), 33 deletions(-)
>>
>> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
>> index 4212ed62235b..5e061851ac00 100644
>> --- a/tools/lib/bpf/bpf.c
>> +++ b/tools/lib/bpf/bpf.c
>> @@ -20,10 +20,17 @@
>>    */
>>     #include <stdlib.h>
>> -#include <memory.h>
>> +#include <stdio.h>
>>   #include <unistd.h>
>>   #include <asm/unistd.h>
>> +#include <string.h>
>> +#include <linux/netlink.h>
>>   #include <linux/bpf.h>
>> +#include <errno.h>
>> +#include <net/ethernet.h>
>> +#include <net/if.h>
>> +#include <linux/if_packet.h>
>> +#include <arpa/inet.h>
>>   #include "bpf.h"
>>
>
>
> Why we need these network related headers?

I started with a copy/paste, assuming that the headers were all in use
but I guess that assumption was wrong.

>>   /*
>> @@ -53,24 +60,71 @@ static int sys_bpf(enum bpf_cmd cmd, union bpf_attr
>> *attr,
>>         return syscall(__NR_bpf, cmd, attr, size);
>>   }
>>   -int bpf_create_map(enum bpf_map_type map_type, int key_size,
>> -                  int value_size, int max_entries)
>> +int bpf_create_map(enum bpf_map_type map_type, int key_size, int
>> value_size,
>> +                  int max_entries, int map_flags)
>>   {
>> -       union bpf_attr attr;
>> +       union bpf_attr attr = {
>> +               .map_type = map_type,
>> +               .key_size = key_size,
>> +               .value_size = value_size,
>> +               .max_entries = max_entries,
>> +               .map_flags = map_flags,
>> +       };
>>   -     memset(&attr, '\0', sizeof(attr));
>> +       return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
>> +}
>>
>
>
> I lost map_flags in original bpf.c. Thanks to your patch. map_flags is
> useful
> when creating BPF_MAP_TYPE_PERCPU_HASH: BPF_F_NO_PREALLOC is meanful in this
> case.

Do you want me to resubmit this piece as a separate patch or will you
address this?

> Although it is okay in samples, I still prefer a explicit bzero() or
> memset(),
> because kernel checks if unused field in this union is zero. However I'll
> check
> c standard to see how unused field would be initialized.

OK.

>> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
>> index e8ba54087497..4dba36995771 100644
>> --- a/tools/lib/bpf/bpf.h
>> +++ b/tools/lib/bpf/bpf.h
>> @@ -23,16 +23,202 @@
>>     #include <linux/bpf.h>
>>   +struct bpf_insn;
>> +
>>   int bpf_create_map(enum bpf_map_type map_type, int key_size, int
>> value_size,
>> -                  int max_entries);
>> +                  int max_entries, int map_flags);
>> +int bpf_update_elem(int fd, void *key, void *value, unsigned long long
>> flags);
>> +int bpf_lookup_elem(int fd, void *key, void *value);
>> +int bpf_delete_elem(int fd, void *key);
>> +int bpf_get_next_key(int fd, void *key, void *next_key);
>> +
>> +int bpf_load_program(enum bpf_prog_type prog_type,
>> +                    const struct bpf_insn *insns, int insn_len,
>> +                    const char *license, int kern_version,
>> +                    char *log_buf, size_t log_buf_sz);
>> +
>> +int bpf_obj_pin(int fd, const char *pathname);
>> +int bpf_obj_get(const char *pathname);
>>   -/* Recommend log buffer size */
>>   #define BPF_LOG_BUF_SIZE 65536
>> -int bpf_load_program(enum bpf_prog_type type, struct bpf_insn *insns,
>> -                    size_t insns_cnt, char *license,
>> -                    u32 kern_version, char *log_buf,
>> -                    size_t log_buf_sz);
>>   -int bpf_map_update_elem(int fd, void *key, void *value,
>> -                       u64 flags);
>> +/* ALU ops on registers, bpf_add|sub|...: dst_reg += src_reg */
>> +
>> +#define BPF_ALU64_REG(OP, DST, SRC)                            \
>> +       ((struct bpf_insn) {                                    \
>> +               .code  = BPF_ALU64 | BPF_OP(OP) | BPF_X,        \
>> +               .dst_reg = DST,                                 \
>> +               .src_reg = SRC,                                 \
>> +               .off   = 0,                                     \
>> +               .imm   = 0 })
>> +
>
>
> Should we define these macros here? They are in include/linux/filter.h
> and duplicated in tools/include/linux/filter.h. Redefining them here
> would cause conflict.

Probably not; including the correct header file would be sufficient.

Re: [PATCHv2 perf/core 2/2] tools lib bpf: Sync with samples/bpf/libbpf

[Wangnan (F)]

On 2016/11/17 10:46, Joe Stringer wrote:
> On 16 November 2016 at 18:10, Wangnan (F) <wangnan0@huawei.com> wrote:
>> I'm also working on improving bpf.c. Please have a look at:
>>
>> https://lkml.org/lkml/2016/11/14/1078
>>
>> Since bpf.c is simple, I think we can add more functions and fixes
>> gradually, instead of a full copy.
>>
>> See my inline comment below.
> Ah, I missed this, my apologies. It looks like it will provide much of
> what I need, I can reassess this patch with your series in mind.
>
> One comment though for your patch (I don't have the original thread to
> respond to unfortunately): The map_pin and map_get functions in your
> patch series can be used to pin progs too, so maybe there is a better
> name? You'll see that this patch uses bpf_obj_{pin,get}() - although I
> wouldn't want those to be confused with the libbpf.c objects so maybe
> there's a clearer name that could be used.

The full thread can be found:

https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1272045.html

(lkml.kernel.org is not working for me, sorry)

In that patch set, bpf_map_pin/get is linked into perf hooks, so BPF script
can pin a map to sysfs. I think this feature would be useful, but I don't
have an example to show how to use it. I didn't provide program pinning/get
interface because in perf hook they are not useful. After rethinking your
suggestion now I think it is okay to provide bpf_obj_{pin,get} in bpf.c
and export bpf_map_pin to perf hook. I'll adjust my own patch.

> I also have some patches to rework the samples/bpf/* code to use
> libbpf instead of the sample code that is there, is it worth me
> submitting that? It will need to wait for your patch to go in, plus a
> merge with davem's tree.
>
>> On 2016/11/17 1:43, Joe Stringer wrote:

[SNIP]

>>>    /*
>>> @@ -53,24 +60,71 @@ static int sys_bpf(enum bpf_cmd cmd, union bpf_attr
>>> *attr,
>>>          return syscall(__NR_bpf, cmd, attr, size);
>>>    }
>>>    -int bpf_create_map(enum bpf_map_type map_type, int key_size,
>>> -                  int value_size, int max_entries)
>>> +int bpf_create_map(enum bpf_map_type map_type, int key_size, int
>>> value_size,
>>> +                  int max_entries, int map_flags)
>>>    {
>>> -       union bpf_attr attr;
>>> +       union bpf_attr attr = {
>>> +               .map_type = map_type,
>>> +               .key_size = key_size,
>>> +               .value_size = value_size,
>>> +               .max_entries = max_entries,
>>> +               .map_flags = map_flags,
>>> +       };
>>>    -     memset(&attr, '\0', sizeof(attr));
>>> +       return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
>>> +}
>>>
>>
>> I lost map_flags in original bpf.c. Thanks to your patch. map_flags is
>> useful
>> when creating BPF_MAP_TYPE_PERCPU_HASH: BPF_F_NO_PREALLOC is meanful in this
>> case.
> Do you want me to resubmit this piece as a separate patch or will you
> address this?

Please send it.

Thank you.

Re: [PATCHv2 perf/core 2/2] tools lib bpf: Sync with samples/bpf/libbpf

[Wangnan (F)]

On 2016/11/17 10:46, Joe Stringer wrote:
> On 16 November 2016 at 18:10, Wangnan (F) <wangnan0@huawei.com> wrote:
>> I'm also working on improving bpf.c. Please have a look at:
>>
>> https://lkml.org/lkml/2016/11/14/1078
>>
>> Since bpf.c is simple, I think we can add more functions and fixes
>> gradually, instead of a full copy.
>>
>> See my inline comment below.
> Ah, I missed this, my apologies. It looks like it will provide much of
> what I need, I can reassess this patch with your series in mind.
>
> One comment though for your patch (I don't have the original thread to
> respond to unfortunately): The map_pin and map_get functions in your
> patch series can be used to pin progs too, so maybe there is a better
> name? You'll see that this patch uses bpf_obj_{pin,get}() - although I
> wouldn't want those to be confused with the libbpf.c objects so maybe
> there's a clearer name that could be used.
>
> I also have some patches to rework the samples/bpf/* code to use
> libbpf instead of the sample code that is there, is it worth me
> submitting that? It will need to wait for your patch to go in, plus a
> merge with davem's tree.
>
I'm happy to see you are trying to replace samples/bpf 's own
libbpf with tools/lib/bpf. I think you can submit your work
on libbpf and patches on samples together if they are ready.
Arnaldo can pick up patches good for him, and you can improve
other patches based on his newest branch.

Thank you.

Re: [PATCHv2 perf/core 2/2] tools lib bpf: Sync with samples/bpf/libbpf

[Joe Stringer]

On 16 November 2016 at 19:21, Wangnan (F) <wangnan0@huawei.com> wrote:
>
>
> On 2016/11/17 10:46, Joe Stringer wrote:
>>
>> On 16 November 2016 at 18:10, Wangnan (F) <wangnan0@huawei.com> wrote:
>>>
>>> I'm also working on improving bpf.c. Please have a look at:
>>>
>>> https://lkml.org/lkml/2016/11/14/1078
>>>
>>> Since bpf.c is simple, I think we can add more functions and fixes
>>> gradually, instead of a full copy.
>>>
>>> See my inline comment below.
>>
>> Ah, I missed this, my apologies. It looks like it will provide much of
>> what I need, I can reassess this patch with your series in mind.
>>
>> One comment though for your patch (I don't have the original thread to
>> respond to unfortunately): The map_pin and map_get functions in your
>> patch series can be used to pin progs too, so maybe there is a better
>> name? You'll see that this patch uses bpf_obj_{pin,get}() - although I
>> wouldn't want those to be confused with the libbpf.c objects so maybe
>> there's a clearer name that could be used.
>
>
> The full thread can be found:
>
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1272045.html
>
> (lkml.kernel.org is not working for me, sorry)
>
> In that patch set, bpf_map_pin/get is linked into perf hooks, so BPF script
> can pin a map to sysfs. I think this feature would be useful, but I don't
> have an example to show how to use it. I didn't provide program pinning/get
> interface because in perf hook they are not useful. After rethinking your
> suggestion now I think it is okay to provide bpf_obj_{pin,get} in bpf.c
> and export bpf_map_pin to perf hook. I'll adjust my own patch.

OK. For what it's worth, I also have a slightly higher level user of
this interface with a patch series for libbpf.[ch] that allows to you
pin a bpf_object * to the filesystem; The idea is that you pass it a
subpath, then it will make sure the BPF FS is mounted, then put your
programs and maps under, for example,
/sys/fs/bpf/$subpath/{maps,progs}/foo (where 'foo' is a symbol name
from your eBPF ELF). I also plan to post this soon, but it depends on
bpf_obj_pin() so will need your patch to go in first. I will submit it
when I can.

>>>>    /*
>>>> @@ -53,24 +60,71 @@ static int sys_bpf(enum bpf_cmd cmd, union bpf_attr
>>>> *attr,
>>>>          return syscall(__NR_bpf, cmd, attr, size);
>>>>    }
>>>>    -int bpf_create_map(enum bpf_map_type map_type, int key_size,
>>>> -                  int value_size, int max_entries)
>>>> +int bpf_create_map(enum bpf_map_type map_type, int key_size, int
>>>> value_size,
>>>> +                  int max_entries, int map_flags)
>>>>    {
>>>> -       union bpf_attr attr;
>>>> +       union bpf_attr attr = {
>>>> +               .map_type = map_type,
>>>> +               .key_size = key_size,
>>>> +               .value_size = value_size,
>>>> +               .max_entries = max_entries,
>>>> +               .map_flags = map_flags,
>>>> +       };
>>>>    -     memset(&attr, '\0', sizeof(attr));
>>>> +       return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
>>>> +}
>>>>
>>>
>>> I lost map_flags in original bpf.c. Thanks to your patch. map_flags is
>>> useful
>>> when creating BPF_MAP_TYPE_PERCPU_HASH: BPF_F_NO_PREALLOC is meanful in
>>> this
>>> case.
>>
>> Do you want me to resubmit this piece as a separate patch or will you
>> address this?
>
>
> Please send it.

OK.

Re: [PATCHv2 perf/core 2/2] tools lib bpf: Sync with samples/bpf/libbpf

[Joe Stringer]

On 16 November 2016 at 19:39, Wangnan (F) <wangnan0@huawei.com> wrote:
>
>
> On 2016/11/17 10:46, Joe Stringer wrote:
>>
>> On 16 November 2016 at 18:10, Wangnan (F) <wangnan0@huawei.com> wrote:
>>>
>>> I'm also working on improving bpf.c. Please have a look at:
>>>
>>> https://lkml.org/lkml/2016/11/14/1078
>>>
>>> Since bpf.c is simple, I think we can add more functions and fixes
>>> gradually, instead of a full copy.
>>>
>>> See my inline comment below.
>>
>> Ah, I missed this, my apologies. It looks like it will provide much of
>> what I need, I can reassess this patch with your series in mind.
>>
>> One comment though for your patch (I don't have the original thread to
>> respond to unfortunately): The map_pin and map_get functions in your
>> patch series can be used to pin progs too, so maybe there is a better
>> name? You'll see that this patch uses bpf_obj_{pin,get}() - although I
>> wouldn't want those to be confused with the libbpf.c objects so maybe
>> there's a clearer name that could be used.
>>
>> I also have some patches to rework the samples/bpf/* code to use
>> libbpf instead of the sample code that is there, is it worth me
>> submitting that? It will need to wait for your patch to go in, plus a
>> merge with davem's tree.
>>
> I'm happy to see you are trying to replace samples/bpf 's own
> libbpf with tools/lib/bpf. I think you can submit your work
> on libbpf and patches on samples together if they are ready.
> Arnaldo can pick up patches good for him, and you can improve
> other patches based on his newest branch.

I'll look into it.