| Message ID | 20211205045041.129716-1-imagedong@tencent.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | bpftool: add support of pin prog by name | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | fail | PR summary |
| bpf/vmtest-bpf-next | fail | VM_Test |
| netdev/tree_selection | success | Not a local patch |
On Sat, Dec 4, 2021 at 8:51 PM <menglong8.dong@gmail.com> wrote: > > From: Menglong Dong <imagedong@tencent.com> > > 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 <imagedong@tencent.com> > --- Doesn't [0] do that already? [0] https://patchwork.kernel.org/project/netdevbpf/patch/20211021214814.1236114-2-sdf@google.com/ > 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(). > +} > + > 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 Tue, Dec 7, 2021 at 5:22 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Sat, Dec 4, 2021 at 8:51 PM <menglong8.dong@gmail.com> wrote: > > > > From: Menglong Dong <imagedong@tencent.com> > > > > 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 <imagedong@tencent.com> > > --- > > 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 :/ > > 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 > > > > +} > > + > > 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 > >
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);