[RFC PATCH iproute2] Add ability to specify eBPF pin path

[RFC PATCH iproute2] Add ability to specify eBPF pin path

[Max Tottenham]

We have a use case where we have several different applications composed of
sets of eBPF programs (programs that may be attached at the TC/XDP layers),
that need to share maps and not conflict with each other.

For XDP based programs, we are using the xdp-loader from the xdp-tools
project[1], it exposes an option to set the 'pin-path' for a given program.
However, programs loaded via tc don't appear to have that ability, all I have
found is the use of LIBBPF_PIN_BY_NAME or the older
PIN_OBJECT_NS/PIN_GLOBAL_NS, but those don't let the user specify the path.

I've whipped up a quick patch to be able to pass along a 'pin_path'  similar to
the xdp-loader. I don't know if this is the *right* approach so I'm more than
happy to be pointed in the right direction.


Thanks

Max

[1] https://github.com/xdp-project/xdp-tools/tree/master/xdp-loader
 

[RFC PATCH iproute2] Add ability to specify eBPF pin path

[Max Tottenham]

---
 include/bpf_util.h |  1 +
 lib/bpf_legacy.c   | 11 +++++++++--
 lib/bpf_libbpf.c   | 16 +++++++++-------
 tc/f_bpf.c         |  4 +++-
 4 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/include/bpf_util.h b/include/bpf_util.h
index 6a5f8ec6..e7ad643b 100644
--- a/include/bpf_util.h
+++ b/include/bpf_util.h
@@ -70,6 +70,7 @@ struct bpf_cfg_in {
 	const char *section;
 	const char *prog_name;
 	const char *uds;
+	const char *pin_path;
 	enum bpf_prog_type type;
 	enum bpf_mode mode;
 	__u32 ifindex;
diff --git a/lib/bpf_legacy.c b/lib/bpf_legacy.c
index 8ac64235..18b2760c 100644
--- a/lib/bpf_legacy.c
+++ b/lib/bpf_legacy.c
@@ -827,7 +827,7 @@ static int bpf_obj_pinned(const char *pathname, enum bpf_prog_type type)
 
 static int bpf_do_parse(struct bpf_cfg_in *cfg, const bool *opt_tbl)
 {
-	const char *file, *section, *uds_name, *prog_name;
+	const char *file, *section, *uds_name, *prog_name, *pin_path;
 	bool verbose = false;
 	int i, ret, argc;
 	char **argv;
@@ -858,7 +858,7 @@ static int bpf_do_parse(struct bpf_cfg_in *cfg, const bool *opt_tbl)
 	}
 
 	NEXT_ARG();
-	file = section = uds_name = prog_name = NULL;
+	file = section = uds_name = prog_name = pin_path = NULL;
 	if (cfg->mode == EBPF_OBJECT || cfg->mode == EBPF_PINNED) {
 		file = *argv;
 		NEXT_ARG_FWD();
@@ -901,6 +901,12 @@ static int bpf_do_parse(struct bpf_cfg_in *cfg, const bool *opt_tbl)
 			NEXT_ARG_FWD();
 		}
 
+		if (argc > 0 && strcmp(*argv, "pin_path") == 0) {
+			NEXT_ARG();
+			pin_path = *argv;
+			NEXT_ARG_FWD();
+		}
+
 		if (__bpf_prog_meta[cfg->type].may_uds_export) {
 			uds_name = getenv(BPF_ENV_UDS);
 			if (argc > 0 && !uds_name &&
@@ -939,6 +945,7 @@ static int bpf_do_parse(struct bpf_cfg_in *cfg, const bool *opt_tbl)
 	cfg->argv    = argv;
 	cfg->verbose = verbose;
 	cfg->prog_name = prog_name;
+	cfg->pin_path = pin_path;
 
 	return ret;
 }
diff --git a/lib/bpf_libbpf.c b/lib/bpf_libbpf.c
index e1c211a1..6c4e18f7 100644
--- a/lib/bpf_libbpf.c
+++ b/lib/bpf_libbpf.c
@@ -226,7 +226,7 @@ static int handle_legacy_maps(struct bpf_object *obj)
 
 	bpf_object__for_each_map(map, obj) {
 		map_name = bpf_map__name(map);
-
+		fprintf(stderr, "Processing map: %s\n", map_name);
 		ret = handle_legacy_map_in_map(obj, map, map_name);
 		if (ret)
 			return ret;
@@ -277,16 +277,18 @@ static int load_bpf_object(struct bpf_cfg_in *cfg)
 	char root_path[PATH_MAX];
 	struct bpf_map *map;
 	int prog_fd, ret = 0;
-
-	ret = iproute2_get_root_path(root_path, PATH_MAX);
-	if (ret)
-		return ret;
-
+	if (cfg->pin_path) {
+		strncpy(root_path , cfg->pin_path, PATH_MAX);
+	} else {
+		ret = iproute2_get_root_path(root_path, PATH_MAX);
+		if (ret)
+			return ret;
+	}
 	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, open_opts,
 			.relaxed_maps = true,
 			.pin_root_path = root_path,
 	);
-
+	fprintf(stderr, "About to bpf_object__open_file()\n");
 	obj = bpf_object__open_file(cfg->object, &open_opts);
 	if (libbpf_get_error(obj)) {
 		fprintf(stderr, "ERROR: opening BPF object file failed\n");
diff --git a/tc/f_bpf.c b/tc/f_bpf.c
index a6d4875f..4eb3e817 100644
--- a/tc/f_bpf.c
+++ b/tc/f_bpf.c
@@ -28,7 +28,7 @@ static void explain(void)
 		"\n"
 		"eBPF use case:\n"
 		" object-file FILE [ section CLS_NAME ] [ export UDS_FILE ]"
-		" [ verbose ] [ direct-action ] [ skip_hw | skip_sw ]\n"
+		" [ verbose ] [ direct-action ] [ skip_hw | skip_sw ] [pin_path PATH]\n"
 		" object-pinned FILE [ direct-action ] [ skip_hw | skip_sw ]\n"
 		"\n"
 		"Common remaining options:\n"
@@ -48,6 +48,8 @@ static void explain(void)
 		"Where UDS_FILE points to a unix domain socket file in order\n"
 		"to hand off control of all created eBPF maps to an agent.\n"
 		"\n"
+		"Where PATH points to a path under bpffs where all eBPF maps will be pinned.\n"
+		"\n"
 		"ACTION_SPEC := ... look at individual actions\n"
 		"NOTE: CLASSID is parsed as hexadecimal input.\n",
 		bpf_prog_to_default_section(bpf_type));
-- 
2.25.1

Re: [RFC PATCH iproute2] Add ability to specify eBPF pin path

[Stephen Hemminger]

On Wed, 3 May 2023 13:33:49 -0400
Max Tottenham <mtottenh@akamai.com> wrote:

> +	fprintf(stderr, "About to bpf_object__open_file()\n");

Please do not add your debug code

Re: [RFC PATCH iproute2] Add ability to specify eBPF pin path

[Stephen Hemminger]

On Wed, 3 May 2023 13:33:49 -0400
Max Tottenham <mtottenh@akamai.com> wrote:

> diff --git a/tc/f_bpf.c b/tc/f_bpf.c
> index a6d4875f..4eb3e817 100644
> --- a/tc/f_bpf.c
> +++ b/tc/f_bpf.c
> @@ -28,7 +28,7 @@ static void explain(void)
>  		"\n"
>  		"eBPF use case:\n"
>  		" object-file FILE [ section CLS_NAME ] [ export UDS_FILE ]"
> -		" [ verbose ] [ direct-action ] [ skip_hw | skip_sw ]\n"
> +		" [ verbose ] [ direct-action ] [ skip_hw | skip_sw ] [pin_path PATH]\n"

Help text should match other options.

You need to update man page man/man8/tc-bpf.8 as well.

[RFC PATCH v2 iproute2 0/1] Add ability to specify eBPF map pin path

[Max Tottenham]

We have a use case where we have several different applications composed of
sets of eBPF programs (programs that may be attached at the TC/XDP layers),
that need to share maps and not conflict with each other.

For XDP based programs, we are using the xdp-loader from the xdp-tools
project[1], it exposes an option to set the 'pin-path' for a given program.
However, programs loaded via tc don't appear to have that ability, all I have
found is the use of LIBBPF_PIN_BY_NAME or the older
PIN_OBJECT_NS/PIN_GLOBAL_NS, but those don't let the user specify the path.

I've whipped up a quick patch to be able to pass along a 'pin_path'  similar to
the xdp-loader. I don't know if this is the *right* approach so I'm more than
happy to be pointed in the right direction.


Thanks

Max

[1] https://github.com/xdp-project/xdp-tools/tree/master/xdp-loader


Changes since V1:

 * Remove debug code.
 * Update man page.

Max Tottenham (1):
  tc/f_bpf.c: Add ability to specify eBPF map pin path

 include/bpf_util.h |  1 +
 lib/bpf_legacy.c   | 11 +++++++++--
 lib/bpf_libbpf.c   | 14 +++++++-------
 man/man8/tc-bpf.8  |  8 ++++++++
 tc/f_bpf.c         |  4 +++-
 5 files changed, 28 insertions(+), 10 deletions(-)

-- 
2.25.1

[RFC PATCH v2 iproute2 1/1] tc/f_bpf.c: Add ability to specify eBPF map pin path

[Max Tottenham]

Allow users of TCs eBPF classifier to specify a path for pinning eBPF
maps via the use of the 'pin_path' argument. For example:

tc -filter add dev ens3 ingress bpf object-file ./bpf_shared.o section \
  ingress pin_path /sys/fs/bpf/my_prog verbose da

Will create/pin any maps under /sys/fs/bpf/my_prog/..., or reuse
existing maps there if present.

Signed-off-by: Max Tottenham <mtottenh@akamai.com>
---
 include/bpf_util.h |  1 +
 lib/bpf_legacy.c   | 11 +++++++++--
 lib/bpf_libbpf.c   | 14 +++++++-------
 man/man8/tc-bpf.8  |  8 ++++++++
 tc/f_bpf.c         |  4 +++-
 5 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/include/bpf_util.h b/include/bpf_util.h
index 6a5f8ec6..e7ad643b 100644
--- a/include/bpf_util.h
+++ b/include/bpf_util.h
@@ -70,6 +70,7 @@ struct bpf_cfg_in {
 	const char *section;
 	const char *prog_name;
 	const char *uds;
+	const char *pin_path;
 	enum bpf_prog_type type;
 	enum bpf_mode mode;
 	__u32 ifindex;
diff --git a/lib/bpf_legacy.c b/lib/bpf_legacy.c
index 8ac64235..18b2760c 100644
--- a/lib/bpf_legacy.c
+++ b/lib/bpf_legacy.c
@@ -827,7 +827,7 @@ static int bpf_obj_pinned(const char *pathname, enum bpf_prog_type type)
 
 static int bpf_do_parse(struct bpf_cfg_in *cfg, const bool *opt_tbl)
 {
-	const char *file, *section, *uds_name, *prog_name;
+	const char *file, *section, *uds_name, *prog_name, *pin_path;
 	bool verbose = false;
 	int i, ret, argc;
 	char **argv;
@@ -858,7 +858,7 @@ static int bpf_do_parse(struct bpf_cfg_in *cfg, const bool *opt_tbl)
 	}
 
 	NEXT_ARG();
-	file = section = uds_name = prog_name = NULL;
+	file = section = uds_name = prog_name = pin_path = NULL;
 	if (cfg->mode == EBPF_OBJECT || cfg->mode == EBPF_PINNED) {
 		file = *argv;
 		NEXT_ARG_FWD();
@@ -901,6 +901,12 @@ static int bpf_do_parse(struct bpf_cfg_in *cfg, const bool *opt_tbl)
 			NEXT_ARG_FWD();
 		}
 
+		if (argc > 0 && strcmp(*argv, "pin_path") == 0) {
+			NEXT_ARG();
+			pin_path = *argv;
+			NEXT_ARG_FWD();
+		}
+
 		if (__bpf_prog_meta[cfg->type].may_uds_export) {
 			uds_name = getenv(BPF_ENV_UDS);
 			if (argc > 0 && !uds_name &&
@@ -939,6 +945,7 @@ static int bpf_do_parse(struct bpf_cfg_in *cfg, const bool *opt_tbl)
 	cfg->argv    = argv;
 	cfg->verbose = verbose;
 	cfg->prog_name = prog_name;
+	cfg->pin_path = pin_path;
 
 	return ret;
 }
diff --git a/lib/bpf_libbpf.c b/lib/bpf_libbpf.c
index e1c211a1..9cfd4561 100644
--- a/lib/bpf_libbpf.c
+++ b/lib/bpf_libbpf.c
@@ -226,7 +226,6 @@ static int handle_legacy_maps(struct bpf_object *obj)
 
 	bpf_object__for_each_map(map, obj) {
 		map_name = bpf_map__name(map);
-
 		ret = handle_legacy_map_in_map(obj, map, map_name);
 		if (ret)
 			return ret;
@@ -277,16 +276,17 @@ static int load_bpf_object(struct bpf_cfg_in *cfg)
 	char root_path[PATH_MAX];
 	struct bpf_map *map;
 	int prog_fd, ret = 0;
-
-	ret = iproute2_get_root_path(root_path, PATH_MAX);
-	if (ret)
-		return ret;
-
+	if (cfg->pin_path) {
+		strncpy(root_path , cfg->pin_path, PATH_MAX-1);
+	} else {
+		ret = iproute2_get_root_path(root_path, PATH_MAX);
+		if (ret)
+			return ret;
+	}
 	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, open_opts,
 			.relaxed_maps = true,
 			.pin_root_path = root_path,
 	);
-
 	obj = bpf_object__open_file(cfg->object, &open_opts);
 	if (libbpf_get_error(obj)) {
 		fprintf(stderr, "ERROR: opening BPF object file failed\n");
diff --git a/man/man8/tc-bpf.8 b/man/man8/tc-bpf.8
index 01230ce6..df2e87e6 100644
--- a/man/man8/tc-bpf.8
+++ b/man/man8/tc-bpf.8
@@ -22,6 +22,8 @@ UDS_FILE ] [
 |
 .B skip_sw
 ] [
+.B pin_path
+PATH ] [
 .B police
 POLICE_SPEC ] [
 .B action
@@ -189,6 +191,12 @@ forces the offload and disables running the eBPF program in the kernel.
 If hardware offload is not possible and this flag was set kernel will
 report an error and filter will not be installed at all.
 
+.SS pin_path
+points to a path under bpffs. On loading an eBPF object file, if the file
+contains a section named "maps" with eBPF map specifications, this path will be
+checked for existing pinned maps with matching names. If existing maps are
+found, they will be reused.
+
 .SS police
 is an optional parameter for an eBPF/cBPF classifier that specifies a
 police in
diff --git a/tc/f_bpf.c b/tc/f_bpf.c
index a6d4875f..9f121b00 100644
--- a/tc/f_bpf.c
+++ b/tc/f_bpf.c
@@ -28,7 +28,7 @@ static void explain(void)
 		"\n"
 		"eBPF use case:\n"
 		" object-file FILE [ section CLS_NAME ] [ export UDS_FILE ]"
-		" [ verbose ] [ direct-action ] [ skip_hw | skip_sw ]\n"
+		" [ verbose ] [ direct-action ] [ skip_hw | skip_sw ] [ pin_path PATH ]\n"
 		" object-pinned FILE [ direct-action ] [ skip_hw | skip_sw ]\n"
 		"\n"
 		"Common remaining options:\n"
@@ -48,6 +48,8 @@ static void explain(void)
 		"Where UDS_FILE points to a unix domain socket file in order\n"
 		"to hand off control of all created eBPF maps to an agent.\n"
 		"\n"
+		"Where PATH points to a path under bpffs where all eBPF maps will be pinned.\n"
+		"\n"
 		"ACTION_SPEC := ... look at individual actions\n"
 		"NOTE: CLASSID is parsed as hexadecimal input.\n",
 		bpf_prog_to_default_section(bpf_type));
-- 
2.25.1