linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] bpftool: add support of pin prog by name
@ 2021-12-05  4:50 menglong8.dong
  2021-12-06 21:21 ` Andrii Nakryiko
  0 siblings, 1 reply; 3+ messages in thread
From: menglong8.dong @ 2021-12-05  4:50 UTC (permalink / raw)
  To: ast
  Cc: daniel, andrii, kafai, songliubraving, yhs, john.fastabend,
	kpsingh, quentin, cong.wang, liujian56, davemarchevsky, sdf,
	netdev, bpf, linux-kernel, Menglong Dong

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>
---
 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] bpftool: add support of pin prog by name
  2021-12-05  4:50 [PATCH] bpftool: add support of pin prog by name menglong8.dong
@ 2021-12-06 21:21 ` Andrii Nakryiko
  2021-12-07  1:55   ` Menglong Dong
  0 siblings, 1 reply; 3+ messages in thread
From: Andrii Nakryiko @ 2021-12-06 21:21 UTC (permalink / raw)
  To: menglong8.dong, Stanislav Fomichev
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Martin Lau,
	Song Liu, Yonghong Song, john fastabend, KP Singh,
	Quentin Monnet, Cong Wang, liujian56, Dave Marchevsky,
	Networking, bpf, open list, Menglong Dong

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
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] bpftool: add support of pin prog by name
  2021-12-06 21:21 ` Andrii Nakryiko
@ 2021-12-07  1:55   ` Menglong Dong
  0 siblings, 0 replies; 3+ messages in thread
From: Menglong Dong @ 2021-12-07  1:55 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Stanislav Fomichev, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin Lau, Song Liu, Yonghong Song,
	john fastabend, KP Singh, Quentin Monnet, Cong Wang, liujian56,
	Dave Marchevsky, Networking, bpf, open list, Menglong Dong

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
> >

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-12-07  1:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-05  4:50 [PATCH] bpftool: add support of pin prog by name menglong8.dong
2021-12-06 21:21 ` Andrii Nakryiko
2021-12-07  1:55   ` Menglong Dong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).