[PATCH iproute2/master 00/11] bpf: pass ifindex for bpf offload

[PATCH iproute2/master 00/11] bpf: pass ifindex for bpf offload

[Jakub Kicinski]

Hi!

This series allows us to pass ifindex automatically when we
set up TC cls_bpf or XDP offload.  There is a fair bit of
refactoring to separate the parse and load stages of lib/bpf.c.
In case of TC the skip_sw flag may come after the program
arguments (e.g. "bpf obj prog.o da skip_sw"), so we can't
just load the program as we parse the arguments.  Note that
this impacts only loading of the program, all other supported
methods of finding a program (pinned, bytecode, bytefile-file)
are handled as previously, the load call will do nothing for
them.

To simplify the implementation f_bpf and m_bpf will no longer
allow specifying programs multiple times.  Device ifindex is 
also resolved before running filter-specific code.


Jakub Kicinski (11):
  bpf: pass program type in struct bpf_cfg_in
  bpf: keep parsed program mode in struct bpf_cfg_in
  bpf: allocate opcode table in struct bpf_cfg_in
  bpf: split parse from program loading
  bpf: rename bpf_parse_common() to bpf_parse_and_load_common()
  bpf: expose bpf_parse_common() and bpf_load_common()
  bpf: allow loading programs for a specific ifindex
  {f,m}_bpf: don't allow specifying multiple bpf programs
  tc_filter: resolve device name before parsing filter
  f_bpf: communicate ifindex for eBPF offload
  iplink: communicate ifindex for xdp offload

 include/bpf_util.h    |  25 +++++++-
 ip/iplink.c           |   4 +-
 ip/iplink_xdp.c       |  13 ++++-
 ip/iproute_lwtunnel.c |   3 +-
 ip/xdp.h              |   4 +-
 lib/bpf.c             | 155 ++++++++++++++++++++++++++++++--------------------
 tc/f_bpf.c            |  18 +++++-
 tc/m_bpf.c            |   6 +-
 tc/tc_filter.c        |  50 ++++++++--------
 9 files changed, 178 insertions(+), 100 deletions(-)

-- 
2.14.1

[PATCH iproute2/master 01/11] bpf: pass program type in struct bpf_cfg_in

[Jakub Kicinski]

Program type is needed both for parsing and loading of
the program.  Parsing may also induce the type based on
signatures from __bpf_prog_meta.  Instead of passing
the type around keep it in struct bpf_cfg_in.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 include/bpf_util.h    |  5 +++--
 ip/iplink_xdp.c       |  3 ++-
 ip/iproute_lwtunnel.c |  3 ++-
 lib/bpf.c             | 38 +++++++++++++++++++-------------------
 tc/f_bpf.c            |  3 ++-
 tc/m_bpf.c            |  3 ++-
 6 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/include/bpf_util.h b/include/bpf_util.h
index e818221d70d9..0da4b85c979a 100644
--- a/include/bpf_util.h
+++ b/include/bpf_util.h
@@ -60,6 +60,7 @@ struct bpf_cfg_in {
 	const char *object;
 	const char *section;
 	const char *uds;
+	enum bpf_prog_type type;
 	int argc;
 	char **argv;
 	struct sock_filter *ops;
@@ -244,8 +245,8 @@ struct bpf_cfg_in {
 		.off   = 0,					\
 		.imm   = 0 })
 
-int bpf_parse_common(enum bpf_prog_type type, struct bpf_cfg_in *cfg,
-		     const struct bpf_cfg_ops *ops, void *nl);
+int bpf_parse_common(struct bpf_cfg_in *cfg, const struct bpf_cfg_ops *ops,
+		     void *nl);
 
 const char *bpf_prog_to_default_section(enum bpf_prog_type type);
 
diff --git a/ip/iplink_xdp.c b/ip/iplink_xdp.c
index 2d2953aa47a8..993e44d7878a 100644
--- a/ip/iplink_xdp.c
+++ b/ip/iplink_xdp.c
@@ -52,6 +52,7 @@ int xdp_parse(int *argc, char ***argv, struct iplink_req *req, bool generic,
 	      bool drv, bool offload)
 {
 	struct bpf_cfg_in cfg = {
+		.type = BPF_PROG_TYPE_XDP,
 		.argc = *argc,
 		.argv = *argv,
 	};
@@ -74,7 +75,7 @@ int xdp_parse(int *argc, char ***argv, struct iplink_req *req, bool generic,
 			return xdp_delete(&xdp);
 	}
 
-	if (bpf_parse_common(BPF_PROG_TYPE_XDP, &cfg, &bpf_cb_ops, &xdp))
+	if (bpf_parse_common(&cfg, &bpf_cb_ops, &xdp))
 		return -1;
 
 	*argc = cfg.argc;
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 1c8adbe78ed2..9f0f647e261f 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -872,6 +872,7 @@ static int lwt_parse_bpf(struct rtattr *rta, size_t len,
 			 int attr, const enum bpf_prog_type bpf_type)
 {
 	struct bpf_cfg_in cfg = {
+		.type = bpf_type,
 		.argc = *argcp,
 		.argv = *argvp,
 	};
@@ -883,7 +884,7 @@ static int lwt_parse_bpf(struct rtattr *rta, size_t len,
 	int err;
 
 	nest = rta_nest(rta, len, attr);
-	err = bpf_parse_common(bpf_type, &cfg, &bpf_cb_ops, &x);
+	err = bpf_parse_common(&cfg, &bpf_cb_ops, &x);
 	if (err < 0) {
 		fprintf(stderr, "Failed to parse eBPF program: %s\n",
 			strerror(-err));
diff --git a/lib/bpf.c b/lib/bpf.c
index fdc28772fb71..5e65682b5ea4 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -813,8 +813,8 @@ enum bpf_mode {
 	BPF_MODE_MAX,
 };
 
-static int bpf_parse(enum bpf_prog_type *type, enum bpf_mode *mode,
-		     struct bpf_cfg_in *cfg, const bool *opt_tbl)
+static int bpf_parse(enum bpf_mode *mode, struct bpf_cfg_in *cfg,
+		     const bool *opt_tbl)
 {
 	const char *file, *section, *uds_name;
 	bool verbose = false;
@@ -852,7 +852,7 @@ static int bpf_parse(enum bpf_prog_type *type, enum bpf_mode *mode,
 		file = *argv;
 		NEXT_ARG_FWD();
 
-		if (*type == BPF_PROG_TYPE_UNSPEC) {
+		if (cfg->type == BPF_PROG_TYPE_UNSPEC) {
 			if (argc > 0 && matches(*argv, "type") == 0) {
 				NEXT_ARG();
 				for (i = 0; i < ARRAY_SIZE(__bpf_prog_meta);
@@ -861,30 +861,30 @@ static int bpf_parse(enum bpf_prog_type *type, enum bpf_mode *mode,
 						continue;
 					if (!matches(*argv,
 						     __bpf_prog_meta[i].type)) {
-						*type = i;
+						cfg->type = i;
 						break;
 					}
 				}
 
-				if (*type == BPF_PROG_TYPE_UNSPEC) {
+				if (cfg->type == BPF_PROG_TYPE_UNSPEC) {
 					fprintf(stderr, "What type is \"%s\"?\n",
 						*argv);
 					return -1;
 				}
 				NEXT_ARG_FWD();
 			} else {
-				*type = BPF_PROG_TYPE_SCHED_CLS;
+				cfg->type = BPF_PROG_TYPE_SCHED_CLS;
 			}
 		}
 
-		section = bpf_prog_to_default_section(*type);
+		section = bpf_prog_to_default_section(cfg->type);
 		if (argc > 0 && matches(*argv, "section") == 0) {
 			NEXT_ARG();
 			section = *argv;
 			NEXT_ARG_FWD();
 		}
 
-		if (__bpf_prog_meta[*type].may_uds_export) {
+		if (__bpf_prog_meta[cfg->type].may_uds_export) {
 			uds_name = getenv(BPF_ENV_UDS);
 			if (argc > 0 && !uds_name &&
 			    matches(*argv, "export") == 0) {
@@ -905,9 +905,9 @@ static int bpf_parse(enum bpf_prog_type *type, enum bpf_mode *mode,
 	if (*mode == CBPF_BYTECODE || *mode == CBPF_FILE)
 		ret = bpf_ops_parse(argc, argv, cfg->ops, *mode == CBPF_FILE);
 	else if (*mode == EBPF_OBJECT)
-		ret = bpf_obj_open(file, *type, section, verbose);
+		ret = bpf_obj_open(file, cfg->type, section, verbose);
 	else if (*mode == EBPF_PINNED)
-		ret = bpf_obj_pinned(file, *type);
+		ret = bpf_obj_pinned(file, cfg->type);
 	else
 		return -1;
 
@@ -920,7 +920,7 @@ static int bpf_parse(enum bpf_prog_type *type, enum bpf_mode *mode,
 	return ret;
 }
 
-static int bpf_parse_opt_tbl(enum bpf_prog_type type, struct bpf_cfg_in *cfg,
+static int bpf_parse_opt_tbl(struct bpf_cfg_in *cfg,
 			     const struct bpf_cfg_ops *ops, void *nl,
 			     const bool *opt_tbl)
 {
@@ -930,7 +930,7 @@ static int bpf_parse_opt_tbl(enum bpf_prog_type type, struct bpf_cfg_in *cfg,
 	int ret;
 
 	cfg->ops = opcodes;
-	ret = bpf_parse(&type, &mode, cfg, opt_tbl);
+	ret = bpf_parse(&mode, cfg, opt_tbl);
 	cfg->ops = NULL;
 	if (ret < 0)
 		return ret;
@@ -947,8 +947,8 @@ static int bpf_parse_opt_tbl(enum bpf_prog_type type, struct bpf_cfg_in *cfg,
 	return 0;
 }
 
-int bpf_parse_common(enum bpf_prog_type type, struct bpf_cfg_in *cfg,
-		     const struct bpf_cfg_ops *ops, void *nl)
+int bpf_parse_common(struct bpf_cfg_in *cfg, const struct bpf_cfg_ops *ops,
+		     void *nl)
 {
 	bool opt_tbl[BPF_MODE_MAX] = {};
 
@@ -962,12 +962,11 @@ int bpf_parse_common(enum bpf_prog_type type, struct bpf_cfg_in *cfg,
 		opt_tbl[EBPF_PINNED]   = true;
 	}
 
-	return bpf_parse_opt_tbl(type, cfg, ops, nl, opt_tbl);
+	return bpf_parse_opt_tbl(cfg, ops, nl, opt_tbl);
 }
 
 int bpf_graft_map(const char *map_path, uint32_t *key, int argc, char **argv)
 {
-	enum bpf_prog_type type = BPF_PROG_TYPE_UNSPEC;
 	const bool opt_tbl[BPF_MODE_MAX] = {
 		[EBPF_OBJECT]	= true,
 		[EBPF_PINNED]	= true,
@@ -978,6 +977,7 @@ int bpf_graft_map(const char *map_path, uint32_t *key, int argc, char **argv)
 		.size_value	= sizeof(int),
 	};
 	struct bpf_cfg_in cfg = {
+		.type		= BPF_PROG_TYPE_UNSPEC,
 		.argc		= argc,
 		.argv		= argv,
 	};
@@ -986,7 +986,7 @@ int bpf_graft_map(const char *map_path, uint32_t *key, int argc, char **argv)
 	enum bpf_mode mode;
 	uint32_t map_key;
 
-	prog_fd = bpf_parse(&type, &mode, &cfg, opt_tbl);
+	prog_fd = bpf_parse(&mode, &cfg, opt_tbl);
 	if (prog_fd < 0)
 		return prog_fd;
 	if (key) {
@@ -1000,7 +1000,7 @@ int bpf_graft_map(const char *map_path, uint32_t *key, int argc, char **argv)
 		}
 	}
 
-	map_fd = bpf_obj_get(map_path, type);
+	map_fd = bpf_obj_get(map_path, cfg.type);
 	if (map_fd < 0) {
 		fprintf(stderr, "Couldn\'t retrieve pinned map \'%s\': %s\n",
 			map_path, strerror(errno));
@@ -1010,7 +1010,7 @@ int bpf_graft_map(const char *map_path, uint32_t *key, int argc, char **argv)
 
 	ret = bpf_map_selfcheck_pinned(map_fd, &test, &ext,
 				       offsetof(struct bpf_elf_map, max_elem),
-				       type);
+				       cfg.type);
 	if (ret < 0) {
 		fprintf(stderr, "Map \'%s\' self-check failed!\n", map_path);
 		goto out_map;
diff --git a/tc/f_bpf.c b/tc/f_bpf.c
index 3f619d0dc738..a38ec2ab7786 100644
--- a/tc/f_bpf.c
+++ b/tc/f_bpf.c
@@ -103,10 +103,11 @@ static int bpf_parse_opt(struct filter_util *qu, char *handle,
 			NEXT_ARG();
 opt_bpf:
 			seen_run = true;
+			cfg.type = bpf_type;
 			cfg.argc = argc;
 			cfg.argv = argv;
 
-			if (bpf_parse_common(bpf_type, &cfg, &bpf_cb_ops, n))
+			if (bpf_parse_common(&cfg, &bpf_cb_ops, n))
 				return -1;
 
 			argc = cfg.argc;
diff --git a/tc/m_bpf.c b/tc/m_bpf.c
index e3d0a2b11838..f2ce3892e4ed 100644
--- a/tc/m_bpf.c
+++ b/tc/m_bpf.c
@@ -98,10 +98,11 @@ static int bpf_parse_opt(struct action_util *a, int *ptr_argc, char ***ptr_argv,
 			NEXT_ARG();
 opt_bpf:
 			seen_run = true;
+			cfg.type = bpf_type;
 			cfg.argc = argc;
 			cfg.argv = argv;
 
-			if (bpf_parse_common(bpf_type, &cfg, &bpf_cb_ops, n))
+			if (bpf_parse_common(&cfg, &bpf_cb_ops, n))
 				return -1;
 
 			argc = cfg.argc;
-- 
2.14.1

[PATCH iproute2/master 02/11] bpf: keep parsed program mode in struct bpf_cfg_in

[Jakub Kicinski]

bpf_parse() will parse command line arguments to find out the
program mode.  This mode will later be needed at loading time.
Instead of keeping it locally add it to struct bpf_cfg_in,
this will allow splitting parsing and loading stages.

enum bpf_mode has to be moved to the header file, because C
doesn't allow forward declaration of enums.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 include/bpf_util.h |  9 +++++++++
 lib/bpf.c          | 42 ++++++++++++++++--------------------------
 2 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/include/bpf_util.h b/include/bpf_util.h
index 0da4b85c979a..a6f4eeb5fe01 100644
--- a/include/bpf_util.h
+++ b/include/bpf_util.h
@@ -56,11 +56,20 @@ struct bpf_cfg_ops {
 	void (*ebpf_cb)(void *nl, int fd, const char *annotation);
 };
 
+enum bpf_mode {
+	CBPF_BYTECODE,
+	CBPF_FILE,
+	EBPF_OBJECT,
+	EBPF_PINNED,
+	BPF_MODE_MAX,
+};
+
 struct bpf_cfg_in {
 	const char *object;
 	const char *section;
 	const char *uds;
 	enum bpf_prog_type type;
+	enum bpf_mode mode;
 	int argc;
 	char **argv;
 	struct sock_filter *ops;
diff --git a/lib/bpf.c b/lib/bpf.c
index 5e65682b5ea4..33c92d6c8a69 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -805,16 +805,7 @@ static int bpf_obj_pinned(const char *pathname, enum bpf_prog_type type)
 	return prog_fd;
 }
 
-enum bpf_mode {
-	CBPF_BYTECODE,
-	CBPF_FILE,
-	EBPF_OBJECT,
-	EBPF_PINNED,
-	BPF_MODE_MAX,
-};
-
-static int bpf_parse(enum bpf_mode *mode, struct bpf_cfg_in *cfg,
-		     const bool *opt_tbl)
+static int bpf_parse(struct bpf_cfg_in *cfg, const bool *opt_tbl)
 {
 	const char *file, *section, *uds_name;
 	bool verbose = false;
@@ -827,20 +818,20 @@ static int bpf_parse(enum bpf_mode *mode, struct bpf_cfg_in *cfg,
 	if (opt_tbl[CBPF_BYTECODE] &&
 	    (matches(*argv, "bytecode") == 0 ||
 	     strcmp(*argv, "bc") == 0)) {
-		*mode = CBPF_BYTECODE;
+		cfg->mode = CBPF_BYTECODE;
 	} else if (opt_tbl[CBPF_FILE] &&
 		   (matches(*argv, "bytecode-file") == 0 ||
 		    strcmp(*argv, "bcf") == 0)) {
-		*mode = CBPF_FILE;
+		cfg->mode = CBPF_FILE;
 	} else if (opt_tbl[EBPF_OBJECT] &&
 		   (matches(*argv, "object-file") == 0 ||
 		    strcmp(*argv, "obj") == 0)) {
-		*mode = EBPF_OBJECT;
+		cfg->mode = EBPF_OBJECT;
 	} else if (opt_tbl[EBPF_PINNED] &&
 		   (matches(*argv, "object-pinned") == 0 ||
 		    matches(*argv, "pinned") == 0 ||
 		    matches(*argv, "fd") == 0)) {
-		*mode = EBPF_PINNED;
+		cfg->mode = EBPF_PINNED;
 	} else {
 		fprintf(stderr, "What mode is \"%s\"?\n", *argv);
 		return -1;
@@ -848,7 +839,7 @@ static int bpf_parse(enum bpf_mode *mode, struct bpf_cfg_in *cfg,
 
 	NEXT_ARG();
 	file = section = uds_name = NULL;
-	if (*mode == EBPF_OBJECT || *mode == EBPF_PINNED) {
+	if (cfg->mode == EBPF_OBJECT || cfg->mode == EBPF_PINNED) {
 		file = *argv;
 		NEXT_ARG_FWD();
 
@@ -902,11 +893,12 @@ static int bpf_parse(enum bpf_mode *mode, struct bpf_cfg_in *cfg,
 		PREV_ARG();
 	}
 
-	if (*mode == CBPF_BYTECODE || *mode == CBPF_FILE)
-		ret = bpf_ops_parse(argc, argv, cfg->ops, *mode == CBPF_FILE);
-	else if (*mode == EBPF_OBJECT)
+	if (cfg->mode == CBPF_BYTECODE || cfg->mode == CBPF_FILE)
+		ret = bpf_ops_parse(argc, argv, cfg->ops,
+				    cfg->mode == CBPF_FILE);
+	else if (cfg->mode == EBPF_OBJECT)
 		ret = bpf_obj_open(file, cfg->type, section, verbose);
-	else if (*mode == EBPF_PINNED)
+	else if (cfg->mode == EBPF_PINNED)
 		ret = bpf_obj_pinned(file, cfg->type);
 	else
 		return -1;
@@ -926,20 +918,19 @@ static int bpf_parse_opt_tbl(struct bpf_cfg_in *cfg,
 {
 	struct sock_filter opcodes[BPF_MAXINSNS];
 	char annotation[256];
-	enum bpf_mode mode;
 	int ret;
 
 	cfg->ops = opcodes;
-	ret = bpf_parse(&mode, cfg, opt_tbl);
+	ret = bpf_parse(cfg, opt_tbl);
 	cfg->ops = NULL;
 	if (ret < 0)
 		return ret;
 
-	if (mode == CBPF_BYTECODE || mode == CBPF_FILE)
+	if (cfg->mode == CBPF_BYTECODE || cfg->mode == CBPF_FILE)
 		ops->cbpf_cb(nl, opcodes, ret);
-	if (mode == EBPF_OBJECT || mode == EBPF_PINNED) {
+	if (cfg->mode == EBPF_OBJECT || cfg->mode == EBPF_PINNED) {
 		snprintf(annotation, sizeof(annotation), "%s:[%s]",
-			 basename(cfg->object), mode == EBPF_PINNED ?
+			 basename(cfg->object), cfg->mode == EBPF_PINNED ?
 			 "*fsobj" : cfg->section);
 		ops->ebpf_cb(nl, ret, annotation);
 	}
@@ -983,10 +974,9 @@ int bpf_graft_map(const char *map_path, uint32_t *key, int argc, char **argv)
 	};
 	struct bpf_map_ext ext = {};
 	int ret, prog_fd, map_fd;
-	enum bpf_mode mode;
 	uint32_t map_key;
 
-	prog_fd = bpf_parse(&mode, &cfg, opt_tbl);
+	prog_fd = bpf_parse(&cfg, opt_tbl);
 	if (prog_fd < 0)
 		return prog_fd;
 	if (key) {
-- 
2.14.1

[PATCH iproute2/master 03/11] bpf: allocate opcode table in struct bpf_cfg_in

[Jakub Kicinski]

struct bpf_cfg_in already carries a pointer to sock_filter ops.
It's currently set to a local variable in bpf_parse_opt_tbl(),
shared between parsing and loading stages.  Move the array
entirely to struct bpf_cfg_in, this will allow us to split
parsing and loading.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 include/bpf_util.h | 2 +-
 lib/bpf.c          | 7 ++-----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/include/bpf_util.h b/include/bpf_util.h
index a6f4eeb5fe01..638721f6315a 100644
--- a/include/bpf_util.h
+++ b/include/bpf_util.h
@@ -72,7 +72,7 @@ struct bpf_cfg_in {
 	enum bpf_mode mode;
 	int argc;
 	char **argv;
-	struct sock_filter *ops;
+	struct sock_filter opcodes[BPF_MAXINSNS];
 };
 
 /* ALU ops on registers, bpf_add|sub|...: dst_reg += src_reg */
diff --git a/lib/bpf.c b/lib/bpf.c
index 33c92d6c8a69..7493595ab1d6 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -894,7 +894,7 @@ static int bpf_parse(struct bpf_cfg_in *cfg, const bool *opt_tbl)
 	}
 
 	if (cfg->mode == CBPF_BYTECODE || cfg->mode == CBPF_FILE)
-		ret = bpf_ops_parse(argc, argv, cfg->ops,
+		ret = bpf_ops_parse(argc, argv, cfg->opcodes,
 				    cfg->mode == CBPF_FILE);
 	else if (cfg->mode == EBPF_OBJECT)
 		ret = bpf_obj_open(file, cfg->type, section, verbose);
@@ -916,18 +916,15 @@ static int bpf_parse_opt_tbl(struct bpf_cfg_in *cfg,
 			     const struct bpf_cfg_ops *ops, void *nl,
 			     const bool *opt_tbl)
 {
-	struct sock_filter opcodes[BPF_MAXINSNS];
 	char annotation[256];
 	int ret;
 
-	cfg->ops = opcodes;
 	ret = bpf_parse(cfg, opt_tbl);
-	cfg->ops = NULL;
 	if (ret < 0)
 		return ret;
 
 	if (cfg->mode == CBPF_BYTECODE || cfg->mode == CBPF_FILE)
-		ops->cbpf_cb(nl, opcodes, ret);
+		ops->cbpf_cb(nl, cfg->opcodes, ret);
 	if (cfg->mode == EBPF_OBJECT || cfg->mode == EBPF_PINNED) {
 		snprintf(annotation, sizeof(annotation), "%s:[%s]",
 			 basename(cfg->object), cfg->mode == EBPF_PINNED ?
-- 
2.14.1

[PATCH iproute2/master 04/11] bpf: split parse from program loading

[Jakub Kicinski]

Parsing command line is currently done together with potentially
loading a new eBPF program.  This makes it more difficult to
provide additional parameters for loading (which may come after
the eBPF program info on the command line).

Split the two (only internally for now).  Verbose parameter
has to be saved in struct bpf_cfg_in to be carried between
the stages.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 include/bpf_util.h |  5 +++++
 lib/bpf.c          | 49 +++++++++++++++++++++++++++++++++++++------------
 2 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/include/bpf_util.h b/include/bpf_util.h
index 638721f6315a..8e39a2d489db 100644
--- a/include/bpf_util.h
+++ b/include/bpf_util.h
@@ -70,9 +70,14 @@ struct bpf_cfg_in {
 	const char *uds;
 	enum bpf_prog_type type;
 	enum bpf_mode mode;
+	bool verbose;
 	int argc;
 	char **argv;
 	struct sock_filter opcodes[BPF_MAXINSNS];
+	union {
+		int n_opcodes;
+		int prog_fd;
+	};
 };
 
 /* ALU ops on registers, bpf_add|sub|...: dst_reg += src_reg */
diff --git a/lib/bpf.c b/lib/bpf.c
index 7493595ab1d6..52f7c790065f 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -805,7 +805,7 @@ static int bpf_obj_pinned(const char *pathname, enum bpf_prog_type type)
 	return prog_fd;
 }
 
-static int bpf_parse(struct bpf_cfg_in *cfg, const bool *opt_tbl)
+static int bpf_do_parse(struct bpf_cfg_in *cfg, const bool *opt_tbl)
 {
 	const char *file, *section, *uds_name;
 	bool verbose = false;
@@ -893,25 +893,39 @@ static int bpf_parse(struct bpf_cfg_in *cfg, const bool *opt_tbl)
 		PREV_ARG();
 	}
 
-	if (cfg->mode == CBPF_BYTECODE || cfg->mode == CBPF_FILE)
+	if (cfg->mode == CBPF_BYTECODE || cfg->mode == CBPF_FILE) {
 		ret = bpf_ops_parse(argc, argv, cfg->opcodes,
 				    cfg->mode == CBPF_FILE);
-	else if (cfg->mode == EBPF_OBJECT)
-		ret = bpf_obj_open(file, cfg->type, section, verbose);
-	else if (cfg->mode == EBPF_PINNED)
+		cfg->n_opcodes = ret;
+	} else if (cfg->mode == EBPF_OBJECT) {
+		ret = 0; /* program will be loaded by load stage */
+	} else if (cfg->mode == EBPF_PINNED) {
 		ret = bpf_obj_pinned(file, cfg->type);
-	else
+		cfg->prog_fd = ret;
+	} else {
 		return -1;
+	}
 
 	cfg->object  = file;
 	cfg->section = section;
 	cfg->uds     = uds_name;
 	cfg->argc    = argc;
 	cfg->argv    = argv;
+	cfg->verbose = verbose;
 
 	return ret;
 }
 
+static int bpf_do_load(struct bpf_cfg_in *cfg)
+{
+	if (cfg->mode == EBPF_OBJECT) {
+		cfg->prog_fd = bpf_obj_open(cfg->object, cfg->type,
+					    cfg->section, cfg->verbose);
+		return cfg->prog_fd;
+	}
+	return 0;
+}
+
 static int bpf_parse_opt_tbl(struct bpf_cfg_in *cfg,
 			     const struct bpf_cfg_ops *ops, void *nl,
 			     const bool *opt_tbl)
@@ -919,17 +933,21 @@ static int bpf_parse_opt_tbl(struct bpf_cfg_in *cfg,
 	char annotation[256];
 	int ret;
 
-	ret = bpf_parse(cfg, opt_tbl);
+	ret = bpf_do_parse(cfg, opt_tbl);
+	if (ret < 0)
+		return ret;
+
+	ret = bpf_do_load(cfg);
 	if (ret < 0)
 		return ret;
 
 	if (cfg->mode == CBPF_BYTECODE || cfg->mode == CBPF_FILE)
-		ops->cbpf_cb(nl, cfg->opcodes, ret);
+		ops->cbpf_cb(nl, cfg->opcodes, cfg->n_opcodes);
 	if (cfg->mode == EBPF_OBJECT || cfg->mode == EBPF_PINNED) {
 		snprintf(annotation, sizeof(annotation), "%s:[%s]",
 			 basename(cfg->object), cfg->mode == EBPF_PINNED ?
 			 "*fsobj" : cfg->section);
-		ops->ebpf_cb(nl, ret, annotation);
+		ops->ebpf_cb(nl, cfg->prog_fd, annotation);
 	}
 
 	return 0;
@@ -973,9 +991,16 @@ int bpf_graft_map(const char *map_path, uint32_t *key, int argc, char **argv)
 	int ret, prog_fd, map_fd;
 	uint32_t map_key;
 
-	prog_fd = bpf_parse(&cfg, opt_tbl);
-	if (prog_fd < 0)
-		return prog_fd;
+	ret = bpf_do_parse(&cfg, opt_tbl);
+	if (ret < 0)
+		return ret;
+
+	ret = bpf_do_load(&cfg);
+	if (ret < 0)
+		return ret;
+
+	prog_fd = cfg.prog_fd;
+
 	if (key) {
 		map_key = *key;
 	} else {
-- 
2.14.1

[PATCH iproute2/master 05/11] bpf: rename bpf_parse_common() to bpf_parse_and_load_common()

[Jakub Kicinski]

bpf_parse_common() parses and loads the program.  Rename it
accordingly.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 include/bpf_util.h    | 4 ++--
 ip/iplink_xdp.c       | 2 +-
 ip/iproute_lwtunnel.c | 2 +-
 lib/bpf.c             | 4 ++--
 tc/f_bpf.c            | 2 +-
 tc/m_bpf.c            | 2 +-
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/bpf_util.h b/include/bpf_util.h
index 8e39a2d489db..da2dee8bb3aa 100644
--- a/include/bpf_util.h
+++ b/include/bpf_util.h
@@ -259,8 +259,8 @@ struct bpf_cfg_in {
 		.off   = 0,					\
 		.imm   = 0 })
 
-int bpf_parse_common(struct bpf_cfg_in *cfg, const struct bpf_cfg_ops *ops,
-		     void *nl);
+int bpf_parse_and_load_common(struct bpf_cfg_in *cfg,
+			      const struct bpf_cfg_ops *ops, void *nl);
 
 const char *bpf_prog_to_default_section(enum bpf_prog_type type);
 
diff --git a/ip/iplink_xdp.c b/ip/iplink_xdp.c
index 993e44d7878a..edaec2a250e7 100644
--- a/ip/iplink_xdp.c
+++ b/ip/iplink_xdp.c
@@ -75,7 +75,7 @@ int xdp_parse(int *argc, char ***argv, struct iplink_req *req, bool generic,
 			return xdp_delete(&xdp);
 	}
 
-	if (bpf_parse_common(&cfg, &bpf_cb_ops, &xdp))
+	if (bpf_parse_and_load_common(&cfg, &bpf_cb_ops, &xdp))
 		return -1;
 
 	*argc = cfg.argc;
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 9f0f647e261f..2280c7a7b192 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -884,7 +884,7 @@ static int lwt_parse_bpf(struct rtattr *rta, size_t len,
 	int err;
 
 	nest = rta_nest(rta, len, attr);
-	err = bpf_parse_common(&cfg, &bpf_cb_ops, &x);
+	err = bpf_parse_and_load_common(&cfg, &bpf_cb_ops, &x);
 	if (err < 0) {
 		fprintf(stderr, "Failed to parse eBPF program: %s\n",
 			strerror(-err));
diff --git a/lib/bpf.c b/lib/bpf.c
index 52f7c790065f..9a0867124fc4 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -953,8 +953,8 @@ static int bpf_parse_opt_tbl(struct bpf_cfg_in *cfg,
 	return 0;
 }
 
-int bpf_parse_common(struct bpf_cfg_in *cfg, const struct bpf_cfg_ops *ops,
-		     void *nl)
+int bpf_parse_and_load_common(struct bpf_cfg_in *cfg,
+			      const struct bpf_cfg_ops *ops, void *nl)
 {
 	bool opt_tbl[BPF_MODE_MAX] = {};
 
diff --git a/tc/f_bpf.c b/tc/f_bpf.c
index a38ec2ab7786..21ba759c4b01 100644
--- a/tc/f_bpf.c
+++ b/tc/f_bpf.c
@@ -107,7 +107,7 @@ static int bpf_parse_opt(struct filter_util *qu, char *handle,
 			cfg.argc = argc;
 			cfg.argv = argv;
 
-			if (bpf_parse_common(&cfg, &bpf_cb_ops, n))
+			if (bpf_parse_and_load_common(&cfg, &bpf_cb_ops, n))
 				return -1;
 
 			argc = cfg.argc;
diff --git a/tc/m_bpf.c b/tc/m_bpf.c
index f2ce3892e4ed..e275afd01fb3 100644
--- a/tc/m_bpf.c
+++ b/tc/m_bpf.c
@@ -102,7 +102,7 @@ static int bpf_parse_opt(struct action_util *a, int *ptr_argc, char ***ptr_argv,
 			cfg.argc = argc;
 			cfg.argv = argv;
 
-			if (bpf_parse_common(&cfg, &bpf_cb_ops, n))
+			if (bpf_parse_and_load_common(&cfg, &bpf_cb_ops, n))
 				return -1;
 
 			argc = cfg.argc;
-- 
2.14.1

[PATCH iproute2/master 06/11] bpf: expose bpf_parse_common() and bpf_load_common()

[Jakub Kicinski]

Expose bpf_parse_common() and bpf_load_common() functions
for those users who may want to modify the parameters to
load after parsing is done.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 include/bpf_util.h |  3 +++
 lib/bpf.c          | 26 ++++++++++++++++----------
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/include/bpf_util.h b/include/bpf_util.h
index da2dee8bb3aa..f6371994252e 100644
--- a/include/bpf_util.h
+++ b/include/bpf_util.h
@@ -259,6 +259,9 @@ struct bpf_cfg_in {
 		.off   = 0,					\
 		.imm   = 0 })
 
+int bpf_parse_common(struct bpf_cfg_in *cfg, const struct bpf_cfg_ops *ops);
+int bpf_load_common(struct bpf_cfg_in *cfg, const struct bpf_cfg_ops *ops,
+		    void *nl);
 int bpf_parse_and_load_common(struct bpf_cfg_in *cfg,
 			      const struct bpf_cfg_ops *ops, void *nl);
 
diff --git a/lib/bpf.c b/lib/bpf.c
index 9a0867124fc4..f25f7016d10f 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -926,17 +926,12 @@ static int bpf_do_load(struct bpf_cfg_in *cfg)
 	return 0;
 }
 
-static int bpf_parse_opt_tbl(struct bpf_cfg_in *cfg,
-			     const struct bpf_cfg_ops *ops, void *nl,
-			     const bool *opt_tbl)
+int bpf_load_common(struct bpf_cfg_in *cfg, const struct bpf_cfg_ops *ops,
+		    void *nl)
 {
 	char annotation[256];
 	int ret;
 
-	ret = bpf_do_parse(cfg, opt_tbl);
-	if (ret < 0)
-		return ret;
-
 	ret = bpf_do_load(cfg);
 	if (ret < 0)
 		return ret;
@@ -953,8 +948,7 @@ static int bpf_parse_opt_tbl(struct bpf_cfg_in *cfg,
 	return 0;
 }
 
-int bpf_parse_and_load_common(struct bpf_cfg_in *cfg,
-			      const struct bpf_cfg_ops *ops, void *nl)
+int bpf_parse_common(struct bpf_cfg_in *cfg, const struct bpf_cfg_ops *ops)
 {
 	bool opt_tbl[BPF_MODE_MAX] = {};
 
@@ -968,7 +962,19 @@ int bpf_parse_and_load_common(struct bpf_cfg_in *cfg,
 		opt_tbl[EBPF_PINNED]   = true;
 	}
 
-	return bpf_parse_opt_tbl(cfg, ops, nl, opt_tbl);
+	return bpf_do_parse(cfg, opt_tbl);
+}
+
+int bpf_parse_and_load_common(struct bpf_cfg_in *cfg,
+			      const struct bpf_cfg_ops *ops, void *nl)
+{
+	int ret;
+
+	ret = bpf_parse_common(cfg, ops);
+	if (ret < 0)
+		return ret;
+
+	return bpf_load_common(cfg, ops, nl);
 }
 
 int bpf_graft_map(const char *map_path, uint32_t *key, int argc, char **argv)
-- 
2.14.1

[PATCH iproute2/master 07/11] bpf: allow loading programs for a specific ifindex

[Jakub Kicinski]

For BPF offload we need to specify the ifindex when program is
loaded now.  Extend the bpf common code to accommodate that.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 include/bpf_util.h |  1 +
 lib/bpf.c          | 37 ++++++++++++++++++++++++++-----------
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/include/bpf_util.h b/include/bpf_util.h
index f6371994252e..219beb40cd25 100644
--- a/include/bpf_util.h
+++ b/include/bpf_util.h
@@ -70,6 +70,7 @@ struct bpf_cfg_in {
 	const char *uds;
 	enum bpf_prog_type type;
 	enum bpf_mode mode;
+	__u32 ifindex;
 	bool verbose;
 	int argc;
 	char **argv;
diff --git a/lib/bpf.c b/lib/bpf.c
index f25f7016d10f..d32f1b808180 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -113,10 +113,10 @@ const char *bpf_prog_to_default_section(enum bpf_prog_type type)
 
 #ifdef HAVE_ELF
 static int bpf_obj_open(const char *path, enum bpf_prog_type type,
-			const char *sec, bool verbose);
+			const char *sec, __u32 ifindex, bool verbose);
 #else
 static int bpf_obj_open(const char *path, enum bpf_prog_type type,
-			const char *sec, bool verbose)
+			const char *sec, __u32 ifindex, bool verbose)
 {
 	fprintf(stderr, "No ELF library support compiled in.\n");
 	errno = ENOSYS;
@@ -920,7 +920,8 @@ static int bpf_do_load(struct bpf_cfg_in *cfg)
 {
 	if (cfg->mode == EBPF_OBJECT) {
 		cfg->prog_fd = bpf_obj_open(cfg->object, cfg->type,
-					    cfg->section, cfg->verbose);
+					    cfg->section, cfg->ifindex,
+					    cfg->verbose);
 		return cfg->prog_fd;
 	}
 	return 0;
@@ -1065,9 +1066,10 @@ int bpf_prog_detach_fd(int target_fd, enum bpf_attach_type type)
 	return bpf(BPF_PROG_DETACH, &attr, sizeof(attr));
 }
 
-int bpf_prog_load(enum bpf_prog_type type, const struct bpf_insn *insns,
-		  size_t size_insns, const char *license, char *log,
-		  size_t size_log)
+static int bpf_prog_load_dev(enum bpf_prog_type type,
+			     const struct bpf_insn *insns, size_t size_insns,
+			     const char *license, __u32 ifindex,
+			     char *log, size_t size_log)
 {
 	union bpf_attr attr = {};
 
@@ -1075,6 +1077,7 @@ int bpf_prog_load(enum bpf_prog_type type, const struct bpf_insn *insns,
 	attr.insns = bpf_ptr_to_u64(insns);
 	attr.insn_cnt = size_insns / sizeof(struct bpf_insn);
 	attr.license = bpf_ptr_to_u64(license);
+	attr.prog_ifindex = ifindex;
 
 	if (size_log > 0) {
 		attr.log_buf = bpf_ptr_to_u64(log);
@@ -1085,6 +1088,14 @@ int bpf_prog_load(enum bpf_prog_type type, const struct bpf_insn *insns,
 	return bpf(BPF_PROG_LOAD, &attr, sizeof(attr));
 }
 
+int bpf_prog_load(enum bpf_prog_type type, const struct bpf_insn *insns,
+		  size_t size_insns, const char *license, char *log,
+		  size_t size_log)
+{
+	return bpf_prog_load_dev(type, insns, size_insns, license, 0,
+				 log, size_log);
+}
+
 #ifdef HAVE_ELF
 struct bpf_elf_prog {
 	enum bpf_prog_type	type;
@@ -1120,6 +1131,7 @@ struct bpf_elf_ctx {
 	int			sec_maps;
 	char			license[ELF_MAX_LICENSE_LEN];
 	enum bpf_prog_type	type;
+	__u32			ifindex;
 	bool			verbose;
 	struct bpf_elf_st	stat;
 	struct bpf_hash_entry	*ht[256];
@@ -1492,8 +1504,9 @@ static int bpf_prog_attach(const char *section,
 	int tries = 0, fd;
 retry:
 	errno = 0;
-	fd = bpf_prog_load(prog->type, prog->insns, prog->size,
-			   prog->license, ctx->log, ctx->log_size);
+	fd = bpf_prog_load_dev(prog->type, prog->insns, prog->size,
+			       prog->license, ctx->ifindex,
+			       ctx->log, ctx->log_size);
 	if (fd < 0 || ctx->verbose) {
 		/* The verifier log is pretty chatty, sometimes so chatty
 		 * on larger programs, that we could fail to dump everything
@@ -2421,7 +2434,8 @@ static void bpf_get_cfg(struct bpf_elf_ctx *ctx)
 }
 
 static int bpf_elf_ctx_init(struct bpf_elf_ctx *ctx, const char *pathname,
-			    enum bpf_prog_type type, bool verbose)
+			    enum bpf_prog_type type, __u32 ifindex,
+			    bool verbose)
 {
 	int ret = -EINVAL;
 
@@ -2433,6 +2447,7 @@ static int bpf_elf_ctx_init(struct bpf_elf_ctx *ctx, const char *pathname,
 	bpf_get_cfg(ctx);
 	ctx->verbose = verbose;
 	ctx->type    = type;
+	ctx->ifindex = ifindex;
 
 	ctx->obj_fd = open(pathname, O_RDONLY);
 	if (ctx->obj_fd < 0)
@@ -2524,12 +2539,12 @@ static void bpf_elf_ctx_destroy(struct bpf_elf_ctx *ctx, bool failure)
 static struct bpf_elf_ctx __ctx;
 
 static int bpf_obj_open(const char *pathname, enum bpf_prog_type type,
-			const char *section, bool verbose)
+			const char *section, __u32 ifindex, bool verbose)
 {
 	struct bpf_elf_ctx *ctx = &__ctx;
 	int fd = 0, ret;
 
-	ret = bpf_elf_ctx_init(ctx, pathname, type, verbose);
+	ret = bpf_elf_ctx_init(ctx, pathname, type, ifindex, verbose);
 	if (ret < 0) {
 		fprintf(stderr, "Cannot initialize ELF context!\n");
 		return ret;
-- 
2.14.1

[PATCH iproute2/master 08/11] {f,m}_bpf: don't allow specifying multiple bpf programs

[Jakub Kicinski]

Both BPF filter and action will allow users to specify run
multiple times, and only the last one will be considered by
the kernel.  Explicitly refuse such command lines.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 tc/f_bpf.c | 3 +++
 tc/m_bpf.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/tc/f_bpf.c b/tc/f_bpf.c
index 21ba759c4b01..f598784e8b08 100644
--- a/tc/f_bpf.c
+++ b/tc/f_bpf.c
@@ -101,6 +101,9 @@ static int bpf_parse_opt(struct filter_util *qu, char *handle,
 	while (argc > 0) {
 		if (matches(*argv, "run") == 0) {
 			NEXT_ARG();
+
+			if (seen_run)
+				duparg("run", *argv);
 opt_bpf:
 			seen_run = true;
 			cfg.type = bpf_type;
diff --git a/tc/m_bpf.c b/tc/m_bpf.c
index e275afd01fb3..1c1f71cdb83f 100644
--- a/tc/m_bpf.c
+++ b/tc/m_bpf.c
@@ -96,6 +96,9 @@ static int bpf_parse_opt(struct action_util *a, int *ptr_argc, char ***ptr_argv,
 	while (argc > 0) {
 		if (matches(*argv, "run") == 0) {
 			NEXT_ARG();
+
+			if (seen_run)
+				duparg("run", *argv);
 opt_bpf:
 			seen_run = true;
 			cfg.type = bpf_type;
-- 
2.14.1

[PATCH iproute2/master 09/11] tc_filter: resolve device name before parsing filter

[Jakub Kicinski]

Move resolving device name into an ifindex before calling filter
specific callbacks.  This way if filters need the ifindex, they
can read it from the request.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 tc/tc_filter.c | 50 ++++++++++++++++++++++++--------------------------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/tc/tc_filter.c b/tc/tc_filter.c
index d0c967a9633a..0a3f7e696af8 100644
--- a/tc/tc_filter.c
+++ b/tc/tc_filter.c
@@ -160,6 +160,16 @@ static int tc_filter_modify(int cmd, unsigned int flags, int argc, char **argv)
 	if (k[0])
 		addattr_l(&req.n, sizeof(req), TCA_KIND, k, strlen(k)+1);
 
+	if (d[0])  {
+		ll_init_map(&rth);
+
+		req.t.tcm_ifindex = ll_name_to_index(d);
+		if (req.t.tcm_ifindex == 0) {
+			fprintf(stderr, "Cannot find device \"%s\"\n", d);
+			return 1;
+		}
+	}
+
 	if (q) {
 		if (q->parse_fopt(q, fhandle, argc, argv, &req.n))
 			return 1;
@@ -182,17 +192,6 @@ static int tc_filter_modify(int cmd, unsigned int flags, int argc, char **argv)
 	if (est.ewma_log)
 		addattr_l(&req.n, sizeof(req), TCA_RATE, &est, sizeof(est));
 
-
-	if (d[0])  {
-		ll_init_map(&rth);
-
-		req.t.tcm_ifindex = ll_name_to_index(d);
-		if (req.t.tcm_ifindex == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", d);
-			return 1;
-		}
-	}
-
 	if (rtnl_talk(&rth, &req.n, NULL) < 0) {
 		fprintf(stderr, "We have an error talking to the kernel\n");
 		return 2;
@@ -452,10 +451,23 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
 		return -1;
 	}
 
+	if (d[0])  {
+		ll_init_map(&rth);
+
+		req.t.tcm_ifindex = ll_name_to_index(d);
+		if (req.t.tcm_ifindex  == 0) {
+			fprintf(stderr, "Cannot find device \"%s\"\n", d);
+			return 1;
+		}
+		filter_ifindex = req.t.tcm_ifindex;
+	} else {
+		fprintf(stderr, "Must specify netdevice \"dev\"\n");
+		return -1;
+	}
+
 	if (q->parse_fopt(q, fhandle, argc, argv, &req.n))
 		return 1;
 
-
 	if (!fhandle) {
 		fprintf(stderr, "Must specify filter \"handle\"\n");
 		return -1;
@@ -470,20 +482,6 @@ static int tc_filter_get(int cmd, unsigned int flags, int argc, char **argv)
 		return -1;
 	}
 
-	if (d[0])  {
-		ll_init_map(&rth);
-
-		req.t.tcm_ifindex = ll_name_to_index(d);
-		if (req.t.tcm_ifindex  == 0) {
-			fprintf(stderr, "Cannot find device \"%s\"\n", d);
-			return 1;
-		}
-		filter_ifindex = req.t.tcm_ifindex;
-	} else {
-		fprintf(stderr, "Must specify netdevice \"dev\"\n");
-		return -1;
-	}
-
 	if (rtnl_talk(&rth, &req.n, &answer) < 0) {
 		fprintf(stderr, "We have an error talking to the kernel\n");
 		return 2;
-- 
2.14.1

[PATCH iproute2/master 10/11] f_bpf: communicate ifindex for eBPF offload

[Jakub Kicinski]

Split parsing and loading of the eBPF program and if skip_sw is set
load the program for ifindex, to which the qdisc is attached.

Note that the ifindex will be ignored for programs which are already
loaded (e.g. when using pinned programs), but in that case we just
trust the user knows what he's doing.  Hopefully we will get extack
soon in the driver to help debugging this case.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 tc/f_bpf.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tc/f_bpf.c b/tc/f_bpf.c
index f598784e8b08..5906f8bb969d 100644
--- a/tc/f_bpf.c
+++ b/tc/f_bpf.c
@@ -82,6 +82,7 @@ static int bpf_parse_opt(struct filter_util *qu, char *handle,
 	unsigned int bpf_flags = 0;
 	struct bpf_cfg_in cfg = {};
 	bool seen_run = false;
+	bool skip_sw = false;
 	struct rtattr *tail;
 	int ret = 0;
 
@@ -110,8 +111,11 @@ static int bpf_parse_opt(struct filter_util *qu, char *handle,
 			cfg.argc = argc;
 			cfg.argv = argv;
 
-			if (bpf_parse_and_load_common(&cfg, &bpf_cb_ops, n))
+			if (bpf_parse_common(&cfg, &bpf_cb_ops) < 0) {
+				fprintf(stderr,
+					"Unable to parse bpf command line\n");
 				return -1;
+			}
 
 			argc = cfg.argc;
 			argv = cfg.argv;
@@ -135,6 +139,7 @@ static int bpf_parse_opt(struct filter_util *qu, char *handle,
 			bpf_gen_flags |= TCA_CLS_FLAGS_SKIP_HW;
 		} else if (matches(*argv, "skip_sw") == 0) {
 			bpf_gen_flags |= TCA_CLS_FLAGS_SKIP_SW;
+			skip_sw = true;
 		} else if (matches(*argv, "action") == 0) {
 			NEXT_ARG();
 			if (parse_action(&argc, &argv, TCA_BPF_ACT, n)) {
@@ -164,6 +169,13 @@ static int bpf_parse_opt(struct filter_util *qu, char *handle,
 		NEXT_ARG_FWD();
 	}
 
+	if (skip_sw)
+		cfg.ifindex = t->tcm_ifindex;
+	if (bpf_load_common(&cfg, &bpf_cb_ops, n) < 0) {
+		fprintf(stderr, "Unable to load program\n");
+		return -1;
+	}
+
 	if (bpf_gen_flags)
 		addattr32(n, MAX_MSG, TCA_BPF_FLAGS_GEN, bpf_gen_flags);
 	if (bpf_flags)
-- 
2.14.1

[PATCH iproute2/master 11/11] iplink: communicate ifindex for xdp offload

[Jakub Kicinski]

When xdpoffload option is used, communicate the ifindex down
to the kernel to trigger device-specific load.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 ip/iplink.c     |  4 ++--
 ip/iplink_xdp.c | 10 ++++++++--
 ip/xdp.h        |  4 ++--
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/ip/iplink.c b/ip/iplink.c
index 0360a0753d4a..4928f1d9489e 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -631,8 +631,8 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req,
 			bool offload = strcmp(*argv, "xdpoffload") == 0;
 
 			NEXT_ARG();
-			if (xdp_parse(&argc, &argv, req, generic, drv,
-				      offload))
+			if (xdp_parse(&argc, &argv, req, dev_index,
+				      generic, drv, offload))
 				exit(-1);
 		} else if (strcmp(*argv, "netns") == 0) {
 			NEXT_ARG();
diff --git a/ip/iplink_xdp.c b/ip/iplink_xdp.c
index edaec2a250e7..6eeb820af0c3 100644
--- a/ip/iplink_xdp.c
+++ b/ip/iplink_xdp.c
@@ -48,8 +48,8 @@ static int xdp_delete(struct xdp_req *xdp)
 	return 0;
 }
 
-int xdp_parse(int *argc, char ***argv, struct iplink_req *req, bool generic,
-	      bool drv, bool offload)
+int xdp_parse(int *argc, char ***argv, struct iplink_req *req, __u32 ifindex,
+	      bool generic, bool drv, bool offload)
 {
 	struct bpf_cfg_in cfg = {
 		.type = BPF_PROG_TYPE_XDP,
@@ -60,6 +60,12 @@ int xdp_parse(int *argc, char ***argv, struct iplink_req *req, bool generic,
 		.req = req,
 	};
 
+	if (offload) {
+		if (!ifindex)
+			incomplete_command();
+		cfg.ifindex = ifindex;
+	}
+
 	if (!force)
 		xdp.flags |= XDP_FLAGS_UPDATE_IF_NOEXIST;
 	if (generic)
diff --git a/ip/xdp.h b/ip/xdp.h
index 1efd591b087c..7400792bbeb7 100644
--- a/ip/xdp.h
+++ b/ip/xdp.h
@@ -3,8 +3,8 @@
 
 #include "utils.h"
 
-int xdp_parse(int *argc, char ***argv, struct iplink_req *req, bool generic,
-	      bool drv, bool offload);
+int xdp_parse(int *argc, char ***argv, struct iplink_req *req, __u32 ifindex,
+	      bool generic, bool drv, bool offload);
 void xdp_dump(FILE *fp, struct rtattr *tb, bool link, bool details);
 
 #endif /* __XDP__ */
-- 
2.14.1

Re: [PATCH iproute2/master 00/11] bpf: pass ifindex for bpf offload

[Stephen Hemminger]

On Thu, 23 Nov 2017 18:11:57 -0800
Jakub Kicinski <jakub.kicinski@netronome.com> wrote:

> Hi!
> 
> This series allows us to pass ifindex automatically when we
> set up TC cls_bpf or XDP offload.  There is a fair bit of
> refactoring to separate the parse and load stages of lib/bpf.c.
> In case of TC the skip_sw flag may come after the program
> arguments (e.g. "bpf obj prog.o da skip_sw"), so we can't
> just load the program as we parse the arguments.  Note that
> this impacts only loading of the program, all other supported
> methods of finding a program (pinned, bytecode, bytefile-file)
> are handled as previously, the load call will do nothing for
> them.
> 
> To simplify the implementation f_bpf and m_bpf will no longer
> allow specifying programs multiple times.  Device ifindex is 
> also resolved before running filter-specific code.
> 
> 
> Jakub Kicinski (11):
>   bpf: pass program type in struct bpf_cfg_in
>   bpf: keep parsed program mode in struct bpf_cfg_in
>   bpf: allocate opcode table in struct bpf_cfg_in
>   bpf: split parse from program loading
>   bpf: rename bpf_parse_common() to bpf_parse_and_load_common()
>   bpf: expose bpf_parse_common() and bpf_load_common()
>   bpf: allow loading programs for a specific ifindex
>   {f,m}_bpf: don't allow specifying multiple bpf programs
>   tc_filter: resolve device name before parsing filter
>   f_bpf: communicate ifindex for eBPF offload
>   iplink: communicate ifindex for xdp offload
> 
>  include/bpf_util.h    |  25 +++++++-
>  ip/iplink.c           |   4 +-
>  ip/iplink_xdp.c       |  13 ++++-
>  ip/iproute_lwtunnel.c |   3 +-
>  ip/xdp.h              |   4 +-
>  lib/bpf.c             | 155 ++++++++++++++++++++++++++++++--------------------
>  tc/f_bpf.c            |  18 +++++-
>  tc/m_bpf.c            |   6 +-
>  tc/tc_filter.c        |  50 ++++++++--------
>  9 files changed, 178 insertions(+), 100 deletions(-)
> 


Looks good, applied. Thanks Jakub

[iproute2 -net-next] build broken ...

[Daniel Borkmann]

Hi Stephen,

after merge of master into net-next branch the build is now broken
for BPF loader as follows:

# make

lib
make[1]: Entering directory '/home/darkstar/iproute2/lib'
    CC       libgenl.o
    CC       ll_map.o
    CC       libnetlink.o
libnetlink.c:120:2: warning: #warning "libmnl required for error support" [-Wcpp]
 #warning "libmnl required for error support"
  ^~~~~~~
    AR       libnetlink.a
    CC       utils.o
    CC       rt_names.o
    CC       ll_types.o
    CC       ll_proto.o
    CC       ll_addr.o
    CC       inet_proto.o
    CC       namespace.o
    CC       json_writer.o
    CC       json_print.o
    CC       names.o
    CC       color.o
    CC       bpf.o
bpf.c: In function ‘bpf_prog_load_dev’:
bpf.c:1080:6: error: ‘union bpf_attr’ has no member named ‘prog_ifindex’; did you mean ‘prog_id’?
  attr.prog_ifindex = ifindex;
      ^
../config.mk:39: recipe for target 'bpf.o' failed
make[1]: *** [bpf.o] Error 1
make[1]: Leaving directory '/home/darkstar/iproute2/lib'
Makefile:59: recipe for target 'all' failed
make: *** [all] Error 2

Can you rebase kernel headers to fix it, in net-next branch they
are not up to date which is why the build error occurs in combination
with below merged series?

Thanks,
Daniel


On 11/24/2017 03:11 AM, Jakub Kicinski wrote:
> Hi!
> 
> This series allows us to pass ifindex automatically when we
> set up TC cls_bpf or XDP offload.  There is a fair bit of
> refactoring to separate the parse and load stages of lib/bpf.c.
> In case of TC the skip_sw flag may come after the program
> arguments (e.g. "bpf obj prog.o da skip_sw"), so we can't
> just load the program as we parse the arguments.  Note that
> this impacts only loading of the program, all other supported
> methods of finding a program (pinned, bytecode, bytefile-file)
> are handled as previously, the load call will do nothing for
> them.
> 
> To simplify the implementation f_bpf and m_bpf will no longer
> allow specifying programs multiple times.  Device ifindex is 
> also resolved before running filter-specific code.
> 
> 
> Jakub Kicinski (11):
>   bpf: pass program type in struct bpf_cfg_in
>   bpf: keep parsed program mode in struct bpf_cfg_in
>   bpf: allocate opcode table in struct bpf_cfg_in
>   bpf: split parse from program loading
>   bpf: rename bpf_parse_common() to bpf_parse_and_load_common()
>   bpf: expose bpf_parse_common() and bpf_load_common()
>   bpf: allow loading programs for a specific ifindex
>   {f,m}_bpf: don't allow specifying multiple bpf programs
>   tc_filter: resolve device name before parsing filter
>   f_bpf: communicate ifindex for eBPF offload
>   iplink: communicate ifindex for xdp offload
> 
>  include/bpf_util.h    |  25 +++++++-
>  ip/iplink.c           |   4 +-
>  ip/iplink_xdp.c       |  13 ++++-
>  ip/iproute_lwtunnel.c |   3 +-
>  ip/xdp.h              |   4 +-
>  lib/bpf.c             | 155 ++++++++++++++++++++++++++++++--------------------
>  tc/f_bpf.c            |  18 +++++-
>  tc/m_bpf.c            |   6 +-
>  tc/tc_filter.c        |  50 ++++++++--------
>  9 files changed, 178 insertions(+), 100 deletions(-)
> 

Re: [iproute2 -net-next] build broken ...

[Stephen Hemminger]

On Wed, 29 Nov 2017 02:05:09 +0100
Daniel Borkmann <daniel@iogearbox.net> wrote:

> Hi Stephen,
> 
> after merge of master into net-next branch the build is now broken
> for BPF loader as follows:
> 
> # make
> 
> lib
> make[1]: Entering directory '/home/darkstar/iproute2/lib'
>     CC       libgenl.o
>     CC       ll_map.o
>     CC       libnetlink.o
> libnetlink.c:120:2: warning: #warning "libmnl required for error support" [-Wcpp]
>  #warning "libmnl required for error support"
>   ^~~~~~~
>     AR       libnetlink.a
>     CC       utils.o
>     CC       rt_names.o
>     CC       ll_types.o
>     CC       ll_proto.o
>     CC       ll_addr.o
>     CC       inet_proto.o
>     CC       namespace.o
>     CC       json_writer.o
>     CC       json_print.o
>     CC       names.o
>     CC       color.o
>     CC       bpf.o
> bpf.c: In function ‘bpf_prog_load_dev’:
> bpf.c:1080:6: error: ‘union bpf_attr’ has no member named ‘prog_ifindex’; did you mean ‘prog_id’?
>   attr.prog_ifindex = ifindex;
>       ^
> ../config.mk:39: recipe for target 'bpf.o' failed
> make[1]: *** [bpf.o] Error 1
> make[1]: Leaving directory '/home/darkstar/iproute2/lib'
> Makefile:59: recipe for target 'all' failed
> make: *** [all] Error 2
> 
> Can you rebase kernel headers to fix it, in net-next branch they
> are not up to date which is why the build error occurs in combination
> with below merged series?
> 
> Thanks,
> Daniel
> 
> 
> On 11/24/2017 03:11 AM, Jakub Kicinski wrote:
> > Hi!
> > 
> > This series allows us to pass ifindex automatically when we
> > set up TC cls_bpf or XDP offload.  There is a fair bit of
> > refactoring to separate the parse and load stages of lib/bpf.c.
> > In case of TC the skip_sw flag may come after the program
> > arguments (e.g. "bpf obj prog.o da skip_sw"), so we can't
> > just load the program as we parse the arguments.  Note that
> > this impacts only loading of the program, all other supported
> > methods of finding a program (pinned, bytecode, bytefile-file)
> > are handled as previously, the load call will do nothing for
> > them.
> > 
> > To simplify the implementation f_bpf and m_bpf will no longer
> > allow specifying programs multiple times.  Device ifindex is 
> > also resolved before running filter-specific code.
> > 
> > 
> > Jakub Kicinski (11):
> >   bpf: pass program type in struct bpf_cfg_in
> >   bpf: keep parsed program mode in struct bpf_cfg_in
> >   bpf: allocate opcode table in struct bpf_cfg_in
> >   bpf: split parse from program loading
> >   bpf: rename bpf_parse_common() to bpf_parse_and_load_common()
> >   bpf: expose bpf_parse_common() and bpf_load_common()
> >   bpf: allow loading programs for a specific ifindex
> >   {f,m}_bpf: don't allow specifying multiple bpf programs
> >   tc_filter: resolve device name before parsing filter
> >   f_bpf: communicate ifindex for eBPF offload
> >   iplink: communicate ifindex for xdp offload
> > 
> >  include/bpf_util.h    |  25 +++++++-
> >  ip/iplink.c           |   4 +-
> >  ip/iplink_xdp.c       |  13 ++++-
> >  ip/iproute_lwtunnel.c |   3 +-
> >  ip/xdp.h              |   4 +-
> >  lib/bpf.c             | 155 ++++++++++++++++++++++++++++++--------------------
> >  tc/f_bpf.c            |  18 +++++-
> >  tc/m_bpf.c            |   6 +-
> >  tc/tc_filter.c        |  50 ++++++++--------
> >  9 files changed, 178 insertions(+), 100 deletions(-)
> >   
> 

Are you using your own uapi headers?
On current master branch...

$ git grep prog_ifindex
include/uapi/linux/bpf.h:               __u32           prog_ifindex;   /* ifindex of netdev to prep for */
lib/bpf.c:      attr.prog_ifindex = ifindex;

Re: [iproute2 -net-next] build broken ...

[Stephen Hemminger]

On Wed, 29 Nov 2017 02:05:09 +0100
Daniel Borkmann <daniel@iogearbox.net> wrote:

> Hi Stephen,
> 
> after merge of master into net-next branch the build is now broken
> for BPF loader as follows:
> 
> # make
> 
> lib
> make[1]: Entering directory '/home/darkstar/iproute2/lib'
>     CC       libgenl.o
>     CC       ll_map.o
>     CC       libnetlink.o
> libnetlink.c:120:2: warning: #warning "libmnl required for error support" [-Wcpp]
>  #warning "libmnl required for error support"
>   ^~~~~~~
>     AR       libnetlink.a
>     CC       utils.o
>     CC       rt_names.o
>     CC       ll_types.o
>     CC       ll_proto.o
>     CC       ll_addr.o
>     CC       inet_proto.o
>     CC       namespace.o
>     CC       json_writer.o
>     CC       json_print.o
>     CC       names.o
>     CC       color.o
>     CC       bpf.o
> bpf.c: In function ‘bpf_prog_load_dev’:
> bpf.c:1080:6: error: ‘union bpf_attr’ has no member named ‘prog_ifindex’; did you mean ‘prog_id’?
>   attr.prog_ifindex = ifindex;
>       ^
> ../config.mk:39: recipe for target 'bpf.o' failed
> make[1]: *** [bpf.o] Error 1
> make[1]: Leaving directory '/home/darkstar/iproute2/lib'
> Makefile:59: recipe for target 'all' failed
> make: *** [all] Error 2
> 
> Can you rebase kernel headers to fix it, in net-next branch they
> are not up to date which is why the build error occurs in combination
> with below merged series?
> 
> Thanks,
> Daniel
> 
> 
> On 11/24/2017 03:11 AM, Jakub Kicinski wrote:
> > Hi!
> > 
> > This series allows us to pass ifindex automatically when we
> > set up TC cls_bpf or XDP offload.  There is a fair bit of
> > refactoring to separate the parse and load stages of lib/bpf.c.
> > In case of TC the skip_sw flag may come after the program
> > arguments (e.g. "bpf obj prog.o da skip_sw"), so we can't
> > just load the program as we parse the arguments.  Note that
> > this impacts only loading of the program, all other supported
> > methods of finding a program (pinned, bytecode, bytefile-file)
> > are handled as previously, the load call will do nothing for
> > them.
> > 
> > To simplify the implementation f_bpf and m_bpf will no longer
> > allow specifying programs multiple times.  Device ifindex is 
> > also resolved before running filter-specific code.
> > 
> > 
> > Jakub Kicinski (11):
> >   bpf: pass program type in struct bpf_cfg_in
> >   bpf: keep parsed program mode in struct bpf_cfg_in
> >   bpf: allocate opcode table in struct bpf_cfg_in
> >   bpf: split parse from program loading
> >   bpf: rename bpf_parse_common() to bpf_parse_and_load_common()
> >   bpf: expose bpf_parse_common() and bpf_load_common()
> >   bpf: allow loading programs for a specific ifindex
> >   {f,m}_bpf: don't allow specifying multiple bpf programs
> >   tc_filter: resolve device name before parsing filter
> >   f_bpf: communicate ifindex for eBPF offload
> >   iplink: communicate ifindex for xdp offload
> > 
> >  include/bpf_util.h    |  25 +++++++-
> >  ip/iplink.c           |   4 +-
> >  ip/iplink_xdp.c       |  13 ++++-
> >  ip/iproute_lwtunnel.c |   3 +-
> >  ip/xdp.h              |   4 +-
> >  lib/bpf.c             | 155 ++++++++++++++++++++++++++++++--------------------
> >  tc/f_bpf.c            |  18 +++++-
> >  tc/m_bpf.c            |   6 +-
> >  tc/tc_filter.c        |  50 ++++++++--------
> >  9 files changed, 178 insertions(+), 100 deletions(-)
> >   
> 

Sorry, net-next branch had out of date headers. Now fixed.

Re: [iproute2 -net-next] build broken ...

[Daniel Borkmann]

On 11/29/2017 03:17 AM, Stephen Hemminger wrote:
[...]
> Sorry, net-next branch had out of date headers. Now fixed.

Yeah, master worked fine. Thanks for the fix!