* [PATCHv2 bpf-next] libbpf: making bpf_prog_load() ignore name if kernel doesn't support
@ 2022-08-13 0:09 Hangbin Liu
2022-08-15 15:36 ` Quentin Monnet
0 siblings, 1 reply; 4+ messages in thread
From: Hangbin Liu @ 2022-08-13 0:09 UTC (permalink / raw)
To: netdev
Cc: Quentin Monnet, Andrii Nakryiko, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
bpf, Hangbin Liu
Similar with commit 10b62d6a38f7 ("libbpf: Add names for auxiliary maps"),
let's make bpf_prog_load() also ignore name if kernel doesn't support
program name.
To achieve this, we need to call sys_bpf_prog_load() directly in
probe_kern_prog_name() to avoid circular dependency. sys_bpf_prog_load()
also need to be exported in the libbpf_internal.h file.
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
v2: move sys_bpf_prog_load definition to libbpf_internal.h. memset attr
to 0 specifically to aviod padding.
---
tools/lib/bpf/bpf.c | 6 ++----
tools/lib/bpf/libbpf.c | 12 ++++++++++--
tools/lib/bpf/libbpf_internal.h | 3 +++
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 6a96e665dc5d..575867d69496 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -84,9 +84,7 @@ static inline int sys_bpf_fd(enum bpf_cmd cmd, union bpf_attr *attr,
return ensure_good_fd(fd);
}
-#define PROG_LOAD_ATTEMPTS 5
-
-static inline int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts)
+int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts)
{
int fd;
@@ -263,7 +261,7 @@ int bpf_prog_load(enum bpf_prog_type prog_type,
attr.prog_ifindex = OPTS_GET(opts, prog_ifindex, 0);
attr.kern_version = OPTS_GET(opts, kern_version, 0);
- if (prog_name)
+ if (prog_name && kernel_supports(NULL, FEAT_PROG_NAME))
libbpf_strlcpy(attr.prog_name, prog_name, sizeof(attr.prog_name));
attr.license = ptr_to_u64(license);
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 3f01f5cd8a4c..4a351897bdcc 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4419,10 +4419,18 @@ static int probe_kern_prog_name(void)
BPF_MOV64_IMM(BPF_REG_0, 0),
BPF_EXIT_INSN(),
};
- int ret, insn_cnt = ARRAY_SIZE(insns);
+ union bpf_attr attr;
+ int ret;
+
+ memset(&attr, 0, sizeof(attr));
+ attr.prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
+ attr.license = ptr_to_u64("GPL");
+ attr.insns = ptr_to_u64(insns);
+ attr.insn_cnt = (__u32)ARRAY_SIZE(insns);
+ libbpf_strlcpy(attr.prog_name, "test", sizeof(attr.prog_name));
/* make sure loading with name works */
- ret = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, "test", "GPL", insns, insn_cnt, NULL);
+ ret = sys_bpf_prog_load(&attr, sizeof(attr), PROG_LOAD_ATTEMPTS);
return probe_fd(ret);
}
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 4135ae0a2bc3..377642ff51fc 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -573,4 +573,7 @@ static inline bool is_pow_of_2(size_t x)
return x && (x & (x - 1)) == 0;
}
+#define PROG_LOAD_ATTEMPTS 5
+int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts);
+
#endif /* __LIBBPF_LIBBPF_INTERNAL_H */
--
2.31.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCHv2 bpf-next] libbpf: making bpf_prog_load() ignore name if kernel doesn't support
2022-08-13 0:09 [PATCHv2 bpf-next] libbpf: making bpf_prog_load() ignore name if kernel doesn't support Hangbin Liu
@ 2022-08-15 15:36 ` Quentin Monnet
2022-08-15 21:59 ` Andrii Nakryiko
0 siblings, 1 reply; 4+ messages in thread
From: Quentin Monnet @ 2022-08-15 15:36 UTC (permalink / raw)
To: Hangbin Liu, netdev
Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, bpf
On 13/08/2022 01:09, Hangbin Liu wrote:
> Similar with commit 10b62d6a38f7 ("libbpf: Add names for auxiliary maps"),
> let's make bpf_prog_load() also ignore name if kernel doesn't support
> program name.
>
> To achieve this, we need to call sys_bpf_prog_load() directly in
> probe_kern_prog_name() to avoid circular dependency. sys_bpf_prog_load()
> also need to be exported in the libbpf_internal.h file.
>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> v2: move sys_bpf_prog_load definition to libbpf_internal.h. memset attr
> to 0 specifically to aviod padding.
> ---
> tools/lib/bpf/bpf.c | 6 ++----
> tools/lib/bpf/libbpf.c | 12 ++++++++++--
> tools/lib/bpf/libbpf_internal.h | 3 +++
> 3 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 6a96e665dc5d..575867d69496 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -84,9 +84,7 @@ static inline int sys_bpf_fd(enum bpf_cmd cmd, union bpf_attr *attr,
> return ensure_good_fd(fd);
> }
>
> -#define PROG_LOAD_ATTEMPTS 5
> -
> -static inline int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts)
> +int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts)
> {
> int fd;
>
> @@ -263,7 +261,7 @@ int bpf_prog_load(enum bpf_prog_type prog_type,
> attr.prog_ifindex = OPTS_GET(opts, prog_ifindex, 0);
> attr.kern_version = OPTS_GET(opts, kern_version, 0);
>
> - if (prog_name)
> + if (prog_name && kernel_supports(NULL, FEAT_PROG_NAME))
> libbpf_strlcpy(attr.prog_name, prog_name, sizeof(attr.prog_name));
> attr.license = ptr_to_u64(license);
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 3f01f5cd8a4c..4a351897bdcc 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4419,10 +4419,18 @@ static int probe_kern_prog_name(void)
> BPF_MOV64_IMM(BPF_REG_0, 0),
> BPF_EXIT_INSN(),
> };
> - int ret, insn_cnt = ARRAY_SIZE(insns);
> + union bpf_attr attr;
> + int ret;
> +
> + memset(&attr, 0, sizeof(attr));
> + attr.prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
> + attr.license = ptr_to_u64("GPL");
> + attr.insns = ptr_to_u64(insns);
> + attr.insn_cnt = (__u32)ARRAY_SIZE(insns);
> + libbpf_strlcpy(attr.prog_name, "test", sizeof(attr.prog_name));
>
> /* make sure loading with name works */
> - ret = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, "test", "GPL", insns, insn_cnt, NULL);
> + ret = sys_bpf_prog_load(&attr, sizeof(attr), PROG_LOAD_ATTEMPTS);
> return probe_fd(ret);
> }
>
> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index 4135ae0a2bc3..377642ff51fc 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -573,4 +573,7 @@ static inline bool is_pow_of_2(size_t x)
> return x && (x & (x - 1)) == 0;
> }
>
> +#define PROG_LOAD_ATTEMPTS 5
> +int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts);
> +
> #endif /* __LIBBPF_LIBBPF_INTERNAL_H */
Looks good to me, thanks!
Acked-by: Quentin Monnet <quentin@isovalent.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCHv2 bpf-next] libbpf: making bpf_prog_load() ignore name if kernel doesn't support
2022-08-15 15:36 ` Quentin Monnet
@ 2022-08-15 21:59 ` Andrii Nakryiko
2022-08-16 3:29 ` Hangbin Liu
0 siblings, 1 reply; 4+ messages in thread
From: Andrii Nakryiko @ 2022-08-15 21:59 UTC (permalink / raw)
To: Quentin Monnet
Cc: Hangbin Liu, netdev, Andrii Nakryiko, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
bpf
On Mon, Aug 15, 2022 at 8:36 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> On 13/08/2022 01:09, Hangbin Liu wrote:
> > Similar with commit 10b62d6a38f7 ("libbpf: Add names for auxiliary maps"),
> > let's make bpf_prog_load() also ignore name if kernel doesn't support
> > program name.
> >
> > To achieve this, we need to call sys_bpf_prog_load() directly in
> > probe_kern_prog_name() to avoid circular dependency. sys_bpf_prog_load()
> > also need to be exported in the libbpf_internal.h file.
> >
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > ---
> > v2: move sys_bpf_prog_load definition to libbpf_internal.h. memset attr
> > to 0 specifically to aviod padding.
> > ---
> > tools/lib/bpf/bpf.c | 6 ++----
> > tools/lib/bpf/libbpf.c | 12 ++++++++++--
> > tools/lib/bpf/libbpf_internal.h | 3 +++
> > 3 files changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> > index 6a96e665dc5d..575867d69496 100644
> > --- a/tools/lib/bpf/bpf.c
> > +++ b/tools/lib/bpf/bpf.c
> > @@ -84,9 +84,7 @@ static inline int sys_bpf_fd(enum bpf_cmd cmd, union bpf_attr *attr,
> > return ensure_good_fd(fd);
> > }
> >
> > -#define PROG_LOAD_ATTEMPTS 5
> > -
> > -static inline int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts)
> > +int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts)
> > {
> > int fd;
> >
> > @@ -263,7 +261,7 @@ int bpf_prog_load(enum bpf_prog_type prog_type,
> > attr.prog_ifindex = OPTS_GET(opts, prog_ifindex, 0);
> > attr.kern_version = OPTS_GET(opts, kern_version, 0);
> >
> > - if (prog_name)
> > + if (prog_name && kernel_supports(NULL, FEAT_PROG_NAME))
> > libbpf_strlcpy(attr.prog_name, prog_name, sizeof(attr.prog_name));
> > attr.license = ptr_to_u64(license);
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 3f01f5cd8a4c..4a351897bdcc 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -4419,10 +4419,18 @@ static int probe_kern_prog_name(void)
> > BPF_MOV64_IMM(BPF_REG_0, 0),
> > BPF_EXIT_INSN(),
> > };
> > - int ret, insn_cnt = ARRAY_SIZE(insns);
> > + union bpf_attr attr;
> > + int ret;
> > +
> > + memset(&attr, 0, sizeof(attr));
> > + attr.prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
> > + attr.license = ptr_to_u64("GPL");
> > + attr.insns = ptr_to_u64(insns);
> > + attr.insn_cnt = (__u32)ARRAY_SIZE(insns);
> > + libbpf_strlcpy(attr.prog_name, "test", sizeof(attr.prog_name));
> >
> > /* make sure loading with name works */
> > - ret = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, "test", "GPL", insns, insn_cnt, NULL);
> > + ret = sys_bpf_prog_load(&attr, sizeof(attr), PROG_LOAD_ATTEMPTS);
> > return probe_fd(ret);
> > }
> >
> > diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> > index 4135ae0a2bc3..377642ff51fc 100644
> > --- a/tools/lib/bpf/libbpf_internal.h
> > +++ b/tools/lib/bpf/libbpf_internal.h
> > @@ -573,4 +573,7 @@ static inline bool is_pow_of_2(size_t x)
> > return x && (x & (x - 1)) == 0;
> > }
> >
> > +#define PROG_LOAD_ATTEMPTS 5
> > +int sys_bpf_prog_load(union bpf_attr *attr, unsigned int size, int attempts);
> > +
> > #endif /* __LIBBPF_LIBBPF_INTERNAL_H */
>
> Looks good to me, thanks!
>
> Acked-by: Quentin Monnet <quentin@isovalent.com>
I did a small adjustment to not fill out entire big bpf_attr union
completely (and added a bit more meaningful "libbpf_nametest" prog
name):
$ git diff --staged
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 4a351897bdcc..f05dd61a8a7f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4415,6 +4415,7 @@ static int probe_fd(int fd)
static int probe_kern_prog_name(void)
{
+ const size_t attr_sz = offsetofend(union bpf_attr, prog_name);
struct bpf_insn insns[] = {
BPF_MOV64_IMM(BPF_REG_0, 0),
BPF_EXIT_INSN(),
@@ -4422,12 +4423,12 @@ static int probe_kern_prog_name(void)
union bpf_attr attr;
int ret;
- memset(&attr, 0, sizeof(attr));
+ memset(&attr, 0, attr_sz);
attr.prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
attr.license = ptr_to_u64("GPL");
attr.insns = ptr_to_u64(insns);
attr.insn_cnt = (__u32)ARRAY_SIZE(insns);
- libbpf_strlcpy(attr.prog_name, "test", sizeof(attr.prog_name));
+ libbpf_strlcpy(attr.prog_name, "libbpf_nametest",
sizeof(attr.prog_name));
Pushed to bpf-next, thanks!
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCHv2 bpf-next] libbpf: making bpf_prog_load() ignore name if kernel doesn't support
2022-08-15 21:59 ` Andrii Nakryiko
@ 2022-08-16 3:29 ` Hangbin Liu
0 siblings, 0 replies; 4+ messages in thread
From: Hangbin Liu @ 2022-08-16 3:29 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: Quentin Monnet, netdev, bpf
On Mon, Aug 15, 2022 at 02:59:50PM -0700, Andrii Nakryiko wrote:
> I did a small adjustment to not fill out entire big bpf_attr union
> completely (and added a bit more meaningful "libbpf_nametest" prog
> name):
>
Thanks for the adjustment.
Hangbin
> $ git diff --staged
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 4a351897bdcc..f05dd61a8a7f 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4415,6 +4415,7 @@ static int probe_fd(int fd)
>
> static int probe_kern_prog_name(void)
> {
> + const size_t attr_sz = offsetofend(union bpf_attr, prog_name);
> struct bpf_insn insns[] = {
> BPF_MOV64_IMM(BPF_REG_0, 0),
> BPF_EXIT_INSN(),
> @@ -4422,12 +4423,12 @@ static int probe_kern_prog_name(void)
> union bpf_attr attr;
> int ret;
>
> - memset(&attr, 0, sizeof(attr));
> + memset(&attr, 0, attr_sz);
> attr.prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
> attr.license = ptr_to_u64("GPL");
> attr.insns = ptr_to_u64(insns);
> attr.insn_cnt = (__u32)ARRAY_SIZE(insns);
> - libbpf_strlcpy(attr.prog_name, "test", sizeof(attr.prog_name));
> + libbpf_strlcpy(attr.prog_name, "libbpf_nametest",
> sizeof(attr.prog_name));
>
> Pushed to bpf-next, thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-16 7:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-13 0:09 [PATCHv2 bpf-next] libbpf: making bpf_prog_load() ignore name if kernel doesn't support Hangbin Liu
2022-08-15 15:36 ` Quentin Monnet
2022-08-15 21:59 ` Andrii Nakryiko
2022-08-16 3:29 ` Hangbin Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.