[PATCH] bpftool: add support of pin prog by name

From: Menglong Dong <redacted>

For now, the command 'bpftool prog loadall' use section name as the
name of the pin file. However, once there are prog with the same
section name in ELF file, this command will failed with the error
'File Exist'.

So, add the support of pin prog by function name with the 'pinbyname'
argument.

Signed-off-by: Menglong Dong <redacted>
---
 tools/bpf/bpftool/prog.c | 7 +++++++
 tools/lib/bpf/libbpf.c   | 5 +++++
 tools/lib/bpf/libbpf.h   | 2 ++
 3 files changed, 14 insertions(+)

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index e47e8b06cc3d..74e0aaebfefc 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c

@@ -1471,6 +1471,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
 	unsigned int old_map_fds = 0;
 	const char *pinmaps = NULL;
 	struct bpf_object *obj;
+	bool pinbyname = false;
 	struct bpf_map *map;
 	const char *pinfile;
 	unsigned int i, j;

@@ -1589,6 +1590,9 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
 				goto err_free_reuse_maps;
 
 			pinmaps = GET_ARG();
+		} else if (is_prefix(*argv, "pinbyname")) {
+			pinbyname = true;
+			NEXT_ARG();
 		} else {
 			p_err("expected no more arguments, 'type', 'map' or 'dev', got: '%s'?",
 			      *argv);

@@ -1616,6 +1620,9 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
 				goto err_close_obj;
 		}
 
+		if (pinbyname)
+			bpf_program__set_pinname(pos,
+						 (char *)bpf_program__name(pos));
 		bpf_program__set_ifindex(pos, ifindex);
 		bpf_program__set_type(pos, prog_type);
 		bpf_program__set_expected_attach_type(pos, expected_attach_type);

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index f6faa33c80fa..e8fc1d0fe16e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c

@@ -8119,6 +8119,11 @@ void bpf_program__set_ifindex(struct bpf_program *prog, __u32 ifindex)
 	prog->prog_ifindex = ifindex;
 }
 
+void bpf_program__set_pinname(struct bpf_program *prog, char *name)
+{
+	prog->pin_name = name;
+}
+
 const char *bpf_program__name(const struct bpf_program *prog)
 {
 	return prog->name;

diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 4ec69f224342..107cf736c2bb 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h

@@ -216,6 +216,8 @@ LIBBPF_API int bpf_program__set_priv(struct bpf_program *prog, void *priv,
 LIBBPF_API void *bpf_program__priv(const struct bpf_program *prog);
 LIBBPF_API void bpf_program__set_ifindex(struct bpf_program *prog,
 					 __u32 ifindex);
+LIBBPF_API void bpf_program__set_pinname(struct bpf_program *prog,
+					 char *name);
 
 LIBBPF_API const char *bpf_program__name(const struct bpf_program *prog);
 LIBBPF_API const char *bpf_program__section_name(const struct bpf_program *prog);

-- 
2.30.2

On Sat, Dec 4, 2021 at 8:51 PM [off-list ref] wrote:

From: Menglong Dong <redacted>

For now, the command 'bpftool prog loadall' use section name as the
name of the pin file. However, once there are prog with the same
section name in ELF file, this command will failed with the error
'File Exist'.

So, add the support of pin prog by function name with the 'pinbyname'
argument.

Signed-off-by: Menglong Dong <redacted>
---

Doesn't [0] do that already?

  [0] https://patchwork.kernel.org/project/netdevbpf/patch/20211021214814.1236114-2-sdf@google.com/

BPF maps have bpf_map__set_pin_path(), setting a full path is more
flexible approach, I think, so if we had to do something here, it's
better to add bpf_program__set_ping_path().

On Tue, Dec 7, 2021 at 5:22 AM Andrii Nakryiko
[off-list ref] wrote:

On Sat, Dec 4, 2021 at 8:51 PM [off-list ref] wrote:

quoted

From: Menglong Dong <redacted>

For now, the command 'bpftool prog loadall' use section name as the
name of the pin file. However, once there are prog with the same
section name in ELF file, this command will failed with the error
'File Exist'.

So, add the support of pin prog by function name with the 'pinbyname'
argument.

Signed-off-by: Menglong Dong <redacted>
---

Doesn't [0] do that already?

  [0] https://patchwork.kernel.org/project/netdevbpf/patch/20211021214814.1236114-2-sdf@google.com/

Ops....Sorry, I didn't notice that patch :/

quoted

 tools/bpf/bpftool/prog.c | 7 +++++++
 tools/lib/bpf/libbpf.c   | 5 +++++
 tools/lib/bpf/libbpf.h   | 2 ++
 3 files changed, 14 insertions(+)

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index e47e8b06cc3d..74e0aaebfefc 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c

@@ -1471,6 +1471,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
        unsigned int old_map_fds = 0;
        const char *pinmaps = NULL;
        struct bpf_object *obj;
+       bool pinbyname = false;
        struct bpf_map *map;
        const char *pinfile;
        unsigned int i, j;

@@ -1589,6 +1590,9 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
                                goto err_free_reuse_maps;

                        pinmaps = GET_ARG();
+               } else if (is_prefix(*argv, "pinbyname")) {
+                       pinbyname = true;
+                       NEXT_ARG();
                } else {
                        p_err("expected no more arguments, 'type', 'map' or 'dev', got: '%s'?",
                              *argv);

@@ -1616,6 +1620,9 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
                                goto err_close_obj;
                }

+               if (pinbyname)
+                       bpf_program__set_pinname(pos,
+                                                (char *)bpf_program__name(pos));
                bpf_program__set_ifindex(pos, ifindex);
                bpf_program__set_type(pos, prog_type);
                bpf_program__set_expected_attach_type(pos, expected_attach_type);

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index f6faa33c80fa..e8fc1d0fe16e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c

@@ -8119,6 +8119,11 @@ void bpf_program__set_ifindex(struct bpf_program *prog, __u32 ifindex)
        prog->prog_ifindex = ifindex;
 }

+void bpf_program__set_pinname(struct bpf_program *prog, char *name)
+{
+       prog->pin_name = name;

BPF maps have bpf_map__set_pin_path(), setting a full path is more
flexible approach, I think, so if we had to do something here, it's
better to add bpf_program__set_ping_path().

Yeah, I think it's a good idea. I'll do something about it.

Thanks!
Menglong Dong

quoted

+}
+
 const char *bpf_program__name(const struct bpf_program *prog)
 {
        return prog->name;

diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 4ec69f224342..107cf736c2bb 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h

@@ -216,6 +216,8 @@ LIBBPF_API int bpf_program__set_priv(struct bpf_program *prog, void *priv,
 LIBBPF_API void *bpf_program__priv(const struct bpf_program *prog);
 LIBBPF_API void bpf_program__set_ifindex(struct bpf_program *prog,
                                         __u32 ifindex);
+LIBBPF_API void bpf_program__set_pinname(struct bpf_program *prog,
+                                        char *name);

 LIBBPF_API const char *bpf_program__name(const struct bpf_program *prog);
 LIBBPF_API const char *bpf_program__section_name(const struct bpf_program *prog);
--

2.30.2