* [PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type
@ 2018-12-19 8:52 Taeung Song
2018-12-19 11:26 ` Quentin Monnet
2018-12-19 18:52 ` Jakub Kicinski
0 siblings, 2 replies; 6+ messages in thread
From: Taeung Song @ 2018-12-19 8:52 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov
Cc: netdev, Jakub Kicinski, Andrey Ignatov
We need to let users check their wrong section name
with proper section names when failed to get proper type from it.
Because there is no knowing what kind of section name can be used.
For example, when a 'cgroup' section name of a BPF program is used,
Before:
$ bpftool prog load bpf-prog.o /sys/fs/bpf/prog1
Error: failed to guess program type based on section name cgroup
After:
libbpf: failed to guess program type based on section name 'cgroup'
libbpf: possible section(type) names are: socket kprobe/ kretprobe/ classifier action tracepoint/ raw_tracepoint/ xdp perf_event lwt_in lwt_out lwt_xmit lwt_seg6local cgroup_skb/ingress cgroup_skb/egress cgroup/skb cgroup/sock cgroup/post_bind4 cgroup/post_bind6 cgroup/dev sockops sk_skb/stream_parser sk_skb/stream_verdict sk_skb sk_msg lirc_mode2 flow_dissector cgroup/bind4 cgroup/bind6 cgroup/connect4 cgroup/connect6 cgroup/sendmsg4 cgroup/sendmsg6
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Andrey Ignatov <rdna@fb.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
tools/bpf/bpftool/prog.c | 10 +++-------
tools/lib/bpf/libbpf.c | 18 ++++++++++++++++--
.../testing/selftests/bpf/test_socket_cookie.c | 4 +---
3 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 2d1bb7d6ff51..0640e9bc0ada 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -930,10 +930,9 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
err = libbpf_prog_type_by_name(type, &attr.prog_type,
&expected_attach_type);
free(type);
- if (err < 0) {
- p_err("unknown program type '%s'", *argv);
+ if (err < 0)
goto err_free_reuse_maps;
- }
+
NEXT_ARG();
} else if (is_prefix(*argv, "map")) {
void *new_map_replace;
@@ -1028,11 +1027,8 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
err = libbpf_prog_type_by_name(sec_name, &prog_type,
&expected_attach_type);
- if (err < 0) {
- p_err("failed to guess program type based on section name %s\n",
- sec_name);
+ if (err < 0)
goto err_close_obj;
- }
}
bpf_program__set_ifindex(pos, ifindex);
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 169e347c76f6..ad274ac8d630 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -78,6 +78,7 @@ do { \
#define pr_warning(fmt, ...) __pr(__pr_warning, fmt, ##__VA_ARGS__)
#define pr_info(fmt, ...) __pr(__pr_info, fmt, ##__VA_ARGS__)
#define pr_debug(fmt, ...) __pr(__pr_debug, fmt, ##__VA_ARGS__)
+#define pr_out(fmt, ...) __base_pr(fmt, ##__VA_ARGS__)
void libbpf_set_print(libbpf_print_fn_t warn,
libbpf_print_fn_t info,
@@ -2682,6 +2683,13 @@ int libbpf_prog_type_by_name(const char *name, enum bpf_prog_type *prog_type,
*expected_attach_type = section_names[i].expected_attach_type;
return 0;
}
+
+ pr_warning("failed to guess program type based on section name '%s'\n", name);
+ pr_info("possible section(type) names are:");
+ for (i = 0; i < ARRAY_SIZE(section_names); i++) {
+ pr_out(" %s", section_names[i].sec);
+ }
+ pr_out("\n");
return -EINVAL;
}
@@ -2701,6 +2709,14 @@ int libbpf_attach_type_by_name(const char *name,
*attach_type = section_names[i].attach_type;
return 0;
}
+
+ pr_warning("failed to guess attach type based on section name '%s'\n", name);
+ pr_info("attachable section(type) names are:");
+ for (i = 0; i < ARRAY_SIZE(section_names); i++) {
+ if (section_names[i].is_attachable)
+ pr_out(" %s", section_names[i].sec);
+ }
+ pr_out("\n");
return -EINVAL;
}
@@ -2907,8 +2923,6 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
err = bpf_program__identify_section(prog, &prog_type,
&expected_attach_type);
if (err < 0) {
- pr_warning("failed to guess program type based on section name %s\n",
- prog->section_name);
bpf_object__close(obj);
return -EINVAL;
}
diff --git a/tools/testing/selftests/bpf/test_socket_cookie.c b/tools/testing/selftests/bpf/test_socket_cookie.c
index b6c2c605d8c0..27c1576198d7 100644
--- a/tools/testing/selftests/bpf/test_socket_cookie.c
+++ b/tools/testing/selftests/bpf/test_socket_cookie.c
@@ -158,10 +158,8 @@ static int run_test(int cgfd)
bpf_object__for_each_program(prog, pobj) {
prog_name = bpf_program__title(prog, /*needs_copy*/ false);
- if (libbpf_attach_type_by_name(prog_name, &attach_type)) {
- log_err("Unexpected prog: %s", prog_name);
+ if (libbpf_attach_type_by_name(prog_name, &attach_type))
goto err;
- }
err = bpf_prog_attach(bpf_program__fd(prog), cgfd, attach_type,
BPF_F_ALLOW_OVERRIDE);
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type
2018-12-19 8:52 [PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type Taeung Song
@ 2018-12-19 11:26 ` Quentin Monnet
2018-12-20 0:17 ` Taeung Song
2018-12-19 18:52 ` Jakub Kicinski
1 sibling, 1 reply; 6+ messages in thread
From: Quentin Monnet @ 2018-12-19 11:26 UTC (permalink / raw)
To: Taeung Song, Daniel Borkmann, Alexei Starovoitov
Cc: netdev, Jakub Kicinski, Andrey Ignatov
2018-12-19 17:52 UTC+0900 ~ Taeung Song <treeze.taeung@gmail.com>
> We need to let users check their wrong section name
> with proper section names when failed to get proper type from it.
> Because there is no knowing what kind of section name can be used.
> For example, when a 'cgroup' section name of a BPF program is used,
>
> Before:
>
> $ bpftool prog load bpf-prog.o /sys/fs/bpf/prog1
> Error: failed to guess program type based on section name cgroup
>
> After:
>
> libbpf: failed to guess program type based on section name 'cgroup'
> libbpf: possible section(type) names are: socket kprobe/ kretprobe/ classifier action tracepoint/ raw_tracepoint/ xdp perf_event lwt_in lwt_out lwt_xmit lwt_seg6local cgroup_skb/ingress cgroup_skb/egress cgroup/skb cgroup/sock cgroup/post_bind4 cgroup/post_bind6 cgroup/dev sockops sk_skb/stream_parser sk_skb/stream_verdict sk_skb sk_msg lirc_mode2 flow_dissector cgroup/bind4 cgroup/bind6 cgroup/connect4 cgroup/connect6 cgroup/sendmsg4 cgroup/sendmsg6
>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: Andrey Ignatov <rdna@fb.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
> tools/bpf/bpftool/prog.c | 10 +++-------
> tools/lib/bpf/libbpf.c | 18 ++++++++++++++++--
> .../testing/selftests/bpf/test_socket_cookie.c | 4 +---
> 3 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 2d1bb7d6ff51..0640e9bc0ada 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -930,10 +930,9 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
> err = libbpf_prog_type_by_name(type, &attr.prog_type,
> &expected_attach_type);
> free(type);
> - if (err < 0) {
> - p_err("unknown program type '%s'", *argv);
> + if (err < 0)
> goto err_free_reuse_maps;
> - }
> +
> NEXT_ARG();
> } else if (is_prefix(*argv, "map")) {
> void *new_map_replace;
> @@ -1028,11 +1027,8 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
>
> err = libbpf_prog_type_by_name(sec_name, &prog_type,
> &expected_attach_type);
> - if (err < 0) {
> - p_err("failed to guess program type based on section name %s\n",
> - sec_name);
> + if (err < 0)
> goto err_close_obj;
> - }
> }
>
> bpf_program__set_ifindex(pos, ifindex);
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 169e347c76f6..ad274ac8d630 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -78,6 +78,7 @@ do { \
> #define pr_warning(fmt, ...) __pr(__pr_warning, fmt, ##__VA_ARGS__)
> #define pr_info(fmt, ...) __pr(__pr_info, fmt, ##__VA_ARGS__)
> #define pr_debug(fmt, ...) __pr(__pr_debug, fmt, ##__VA_ARGS__)
> +#define pr_out(fmt, ...) __base_pr(fmt, ##__VA_ARGS__)
Hi, thanks! Would be nice to get existing prog/attach types indeed.
Please can we avoid using a macro that unconditionally prints to stderr?
What will happen in your code below if any application uses
libbpf_set_print() to redefine pr_warning() and pr_info() is that only
the existing section names will be printed out, with no accompanying
message. Did you add pr_out() to avoid having the "libbpf: " prefix
printed in the middle of the list? If so, could we find something else
(e.g. alternative __pr() macro) and a way to replace that function by a
user-defined one (like libbpf_set_print() offers for the other ones)?
My main concern is to avoid breaking JSON output too much for bpftool.
Alternatively, would that make sense to have instead a function that
return a list of supported section names, and that could be called from
bpftool as a complement to the current error message? Just an idea.
>
> void libbpf_set_print(libbpf_print_fn_t warn,
> libbpf_print_fn_t info,
> @@ -2682,6 +2683,13 @@ int libbpf_prog_type_by_name(const char *name, enum bpf_prog_type *prog_type,
> *expected_attach_type = section_names[i].expected_attach_type;
> return 0;
> }
> +
> + pr_warning("failed to guess program type based on section name '%s'\n", name);
> + pr_info("possible section(type) names are:");
> + for (i = 0; i < ARRAY_SIZE(section_names); i++) {
> + pr_out(" %s", section_names[i].sec);
> + }
Nit: unnecessary brackets.
> + pr_out("\n");
> return -EINVAL;
> }
>
> @@ -2701,6 +2709,14 @@ int libbpf_attach_type_by_name(const char *name,
> *attach_type = section_names[i].attach_type;
> return 0;
> }
> +
> + pr_warning("failed to guess attach type based on section name '%s'\n", name);
> + pr_info("attachable section(type) names are:");
> + for (i = 0; i < ARRAY_SIZE(section_names); i++) {
> + if (section_names[i].is_attachable)
> + pr_out(" %s", section_names[i].sec);
> + }
Likewise.
> + pr_out("\n");
> return -EINVAL;
> }
>
> @@ -2907,8 +2923,6 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
> err = bpf_program__identify_section(prog, &prog_type,
> &expected_attach_type);
> if (err < 0) {
> - pr_warning("failed to guess program type based on section name %s\n",
> - prog->section_name);
> bpf_object__close(obj);
> return -EINVAL;
> }
> diff --git a/tools/testing/selftests/bpf/test_socket_cookie.c b/tools/testing/selftests/bpf/test_socket_cookie.c
> index b6c2c605d8c0..27c1576198d7 100644
> --- a/tools/testing/selftests/bpf/test_socket_cookie.c
> +++ b/tools/testing/selftests/bpf/test_socket_cookie.c
> @@ -158,10 +158,8 @@ static int run_test(int cgfd)
> bpf_object__for_each_program(prog, pobj) {
> prog_name = bpf_program__title(prog, /*needs_copy*/ false);
>
> - if (libbpf_attach_type_by_name(prog_name, &attach_type)) {
> - log_err("Unexpected prog: %s", prog_name);
> + if (libbpf_attach_type_by_name(prog_name, &attach_type))
> goto err;
> - }
>
> err = bpf_prog_attach(bpf_program__fd(prog), cgfd, attach_type,
> BPF_F_ALLOW_OVERRIDE);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type
2018-12-19 8:52 [PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type Taeung Song
2018-12-19 11:26 ` Quentin Monnet
@ 2018-12-19 18:52 ` Jakub Kicinski
2018-12-20 0:47 ` Taeung Song
1 sibling, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2018-12-19 18:52 UTC (permalink / raw)
To: Taeung Song; +Cc: Daniel Borkmann, Alexei Starovoitov, netdev, Andrey Ignatov
On Wed, 19 Dec 2018 17:52:40 +0900, Taeung Song wrote:
> We need to let users check their wrong section name
> with proper section names when failed to get proper type from it.
> Because there is no knowing what kind of section name can be used.
# bpftool prog help
Usage: bpftool prog { show | list } [PROG]
[...]
TYPE := { socket | kprobe | kretprobe | classifier | action |
tracepoint | raw_tracepoint | xdp | perf_event | cgroup/skb |
cgroup/sock | cgroup/dev | lwt_in | lwt_out | lwt_xmit |
lwt_seg6local | sockops | sk_skb | sk_msg | lirc_mode2 |
sk_reuseport | flow_dissector |
cgroup/bind4 | cgroup/bind6 | cgroup/post_bind4 |
cgroup/post_bind6 | cgroup/connect4 | cgroup/connect6 |
cgroup/sendmsg4 | cgroup/sendmsg6 }
ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse |
flow_dissector }
OPTIONS := { {-j|--json} [{-p|--pretty}] | {-f|--bpffs} |
{-m|--mapcompat} | {-n|--nomount} }
Also in man bpftool-prog.
Perhaps we could just point users to that info? IMHO having libraries
unconditionally print warnings is not great..
> For example, when a 'cgroup' section name of a BPF program is used,
>
> Before:
>
> $ bpftool prog load bpf-prog.o /sys/fs/bpf/prog1
> Error: failed to guess program type based on section name cgroup
>
> After:
>
> libbpf: failed to guess program type based on section name 'cgroup'
> libbpf: possible section(type) names are: socket kprobe/ kretprobe/ classifier action tracepoint/ raw_tracepoint/ xdp perf_event lwt_in lwt_out lwt_xmit lwt_seg6local cgroup_skb/ingress cgroup_skb/egress cgroup/skb cgroup/sock cgroup/post_bind4 cgroup/post_bind6 cgroup/dev sockops sk_skb/stream_parser sk_skb/stream_verdict sk_skb sk_msg lirc_mode2 flow_dissector cgroup/bind4 cgroup/bind6 cgroup/connect4 cgroup/connect6 cgroup/sendmsg4 cgroup/sendmsg6
>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: Andrey Ignatov <rdna@fb.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type
2018-12-19 11:26 ` Quentin Monnet
@ 2018-12-20 0:17 ` Taeung Song
0 siblings, 0 replies; 6+ messages in thread
From: Taeung Song @ 2018-12-20 0:17 UTC (permalink / raw)
To: Quentin Monnet
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, Jakub Kicinski,
Andrey Ignatov
On 12/19/18 8:26 PM, Quentin Monnet wrote:
> 2018-12-19 17:52 UTC+0900 ~ Taeung Song <treeze.taeung@gmail.com>
>> We need to let users check their wrong section name
>> with proper section names when failed to get proper type from it.
>> Because there is no knowing what kind of section name can be used.
>> For example, when a 'cgroup' section name of a BPF program is used,
>>
>> Before:
>>
>> $ bpftool prog load bpf-prog.o /sys/fs/bpf/prog1
>> Error: failed to guess program type based on section name cgroup
>>
>> After:
>>
>> libbpf: failed to guess program type based on section name 'cgroup'
>> libbpf: possible section(type) names are: socket kprobe/
>> kretprobe/ classifier action tracepoint/ raw_tracepoint/ xdp
>> perf_event lwt_in lwt_out lwt_xmit lwt_seg6local cgroup_skb/ingress
>> cgroup_skb/egress cgroup/skb cgroup/sock cgroup/post_bind4
>> cgroup/post_bind6 cgroup/dev sockops sk_skb/stream_parser
>> sk_skb/stream_verdict sk_skb sk_msg lirc_mode2 flow_dissector
>> cgroup/bind4 cgroup/bind6 cgroup/connect4 cgroup/connect6
>> cgroup/sendmsg4 cgroup/sendmsg6
>>
>> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
>> Cc: Andrey Ignatov <rdna@fb.com>
>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>> ---
>> tools/bpf/bpftool/prog.c | 10 +++-------
>> tools/lib/bpf/libbpf.c | 18 ++++++++++++++++--
>> .../testing/selftests/bpf/test_socket_cookie.c | 4 +---
>> 3 files changed, 20 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
>> index 2d1bb7d6ff51..0640e9bc0ada 100644
>> --- a/tools/bpf/bpftool/prog.c
>> +++ b/tools/bpf/bpftool/prog.c
>> @@ -930,10 +930,9 @@ static int load_with_options(int argc, char
>> **argv, bool first_prog_only)
>> err = libbpf_prog_type_by_name(type, &attr.prog_type,
>> &expected_attach_type);
>> free(type);
>> - if (err < 0) {
>> - p_err("unknown program type '%s'", *argv);
>> + if (err < 0)
>> goto err_free_reuse_maps;
>> - }
>> +
>> NEXT_ARG();
>> } else if (is_prefix(*argv, "map")) {
>> void *new_map_replace;
>> @@ -1028,11 +1027,8 @@ static int load_with_options(int argc, char
>> **argv, bool first_prog_only)
>> err = libbpf_prog_type_by_name(sec_name, &prog_type,
>> &expected_attach_type);
>> - if (err < 0) {
>> - p_err("failed to guess program type based on section
>> name %s\n",
>> - sec_name);
>> + if (err < 0)
>> goto err_close_obj;
>> - }
>> }
>> bpf_program__set_ifindex(pos, ifindex);
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 169e347c76f6..ad274ac8d630 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -78,6 +78,7 @@ do { \
>> #define pr_warning(fmt, ...) __pr(__pr_warning, fmt, ##__VA_ARGS__)
>> #define pr_info(fmt, ...) __pr(__pr_info, fmt, ##__VA_ARGS__)
>> #define pr_debug(fmt, ...) __pr(__pr_debug, fmt, ##__VA_ARGS__)
>> +#define pr_out(fmt, ...) __base_pr(fmt, ##__VA_ARGS__)
>
> Hi, thanks! Would be nice to get existing prog/attach types indeed.
>
> Please can we avoid using a macro that unconditionally prints to stderr?
> What will happen in your code below if any application uses
> libbpf_set_print() to redefine pr_warning() and pr_info() is that only
> the existing section names will be printed out, with no accompanying
> message. Did you add pr_out() to avoid having the "libbpf: " prefix
> printed in the middle of the list? If so, could we find something else
> (e.g. alternative __pr() macro) and a way to replace that function by a
> user-defined one (like libbpf_set_print() offers for the other ones)?
>
> My main concern is to avoid breaking JSON output too much for bpftool.
>
Hi, I got it. I also think it should be changed to unconditionally print
warnings.
> Alternatively, would that make sense to have instead a function that
> return a list of supported section names, and that could be called from
> bpftool as a complement to the current error message? Just an idea.
>
Yep, I also think the function for a list of supported section names
can be used for the 'TYPE' help message (bpftool-prog) Jakub said.
>> void libbpf_set_print(libbpf_print_fn_t warn,
>> libbpf_print_fn_t info,
>> @@ -2682,6 +2683,13 @@ int libbpf_prog_type_by_name(const char *name,
>> enum bpf_prog_type *prog_type,
>> *expected_attach_type = section_names[i].expected_attach_type;
>> return 0;
>> }
>> +
>> + pr_warning("failed to guess program type based on section name
>> '%s'\n", name);
>> + pr_info("possible section(type) names are:");
>> + for (i = 0; i < ARRAY_SIZE(section_names); i++) {
>> + pr_out(" %s", section_names[i].sec);
>> + }
>
> Nit: unnecessary brackets.
>
>> + pr_out("\n");
>> return -EINVAL;
>> }
>> @@ -2701,6 +2709,14 @@ int libbpf_attach_type_by_name(const char *name,
>> *attach_type = section_names[i].attach_type;
>> return 0;
>> }
>> +
>> + pr_warning("failed to guess attach type based on section name
>> '%s'\n", name);
>> + pr_info("attachable section(type) names are:");
>> + for (i = 0; i < ARRAY_SIZE(section_names); i++) {
>> + if (section_names[i].is_attachable)
>> + pr_out(" %s", section_names[i].sec);
>> + }
>
> Likewise.
>
>> + pr_out("\n");
>> return -EINVAL;
>> }
>> @@ -2907,8 +2923,6 @@ int bpf_prog_load_xattr(const struct
>> bpf_prog_load_attr *attr,
>> err = bpf_program__identify_section(prog, &prog_type,
>> &expected_attach_type);
>> if (err < 0) {
>> - pr_warning("failed to guess program type based on
>> section name %s\n",
>> - prog->section_name);
>> bpf_object__close(obj);
>> return -EINVAL;
>> }
>> diff --git a/tools/testing/selftests/bpf/test_socket_cookie.c
>> b/tools/testing/selftests/bpf/test_socket_cookie.c
>> index b6c2c605d8c0..27c1576198d7 100644
>> --- a/tools/testing/selftests/bpf/test_socket_cookie.c
>> +++ b/tools/testing/selftests/bpf/test_socket_cookie.c
>> @@ -158,10 +158,8 @@ static int run_test(int cgfd)
>> bpf_object__for_each_program(prog, pobj) {
>> prog_name = bpf_program__title(prog, /*needs_copy*/ false);
>> - if (libbpf_attach_type_by_name(prog_name, &attach_type)) {
>> - log_err("Unexpected prog: %s", prog_name);
>> + if (libbpf_attach_type_by_name(prog_name, &attach_type))
>> goto err;
>> - }
>> err = bpf_prog_attach(bpf_program__fd(prog), cgfd, attach_type,
>> BPF_F_ALLOW_OVERRIDE);
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type
2018-12-19 18:52 ` Jakub Kicinski
@ 2018-12-20 0:47 ` Taeung Song
2018-12-20 2:22 ` Jakub Kicinski
0 siblings, 1 reply; 6+ messages in thread
From: Taeung Song @ 2018-12-20 0:47 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, Andrey Ignatov
On 12/20/18 3:52 AM, Jakub Kicinski wrote:
> On Wed, 19 Dec 2018 17:52:40 +0900, Taeung Song wrote:
>> We need to let users check their wrong section name
>> with proper section names when failed to get proper type from it.
>> Because there is no knowing what kind of section name can be used.
>
> # bpftool prog help
> Usage: bpftool prog { show | list } [PROG]
> [...]
> TYPE := { socket | kprobe | kretprobe | classifier | action |
> tracepoint | raw_tracepoint | xdp | perf_event | cgroup/skb |
> cgroup/sock | cgroup/dev | lwt_in | lwt_out | lwt_xmit |
> lwt_seg6local | sockops | sk_skb | sk_msg | lirc_mode2 |
> sk_reuseport | flow_dissector |
> cgroup/bind4 | cgroup/bind6 | cgroup/post_bind4 |
> cgroup/post_bind6 | cgroup/connect4 | cgroup/connect6 |
> cgroup/sendmsg4 | cgroup/sendmsg6 }
> ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse |
> flow_dissector }
> OPTIONS := { {-j|--json} [{-p|--pretty}] | {-f|--bpffs} |
> {-m|--mapcompat} | {-n|--nomount} }
>
> Also in man bpftool-prog.
>
> Perhaps we could just point users to that info? IMHO having libraries
Right, I think when users used wrong section names
we need to let them realize libbpf guess prog/attach types from given
section names because users who are unfamiliar with BPF programming
mightn't even know that, right ?
And how about a function that return supported section names ?
I think it can be used for the 'TYPE'/'ATTACH_TYPE' help message
because it is just a string and couldn't have latest types.
> unconditionally print warnings is not great..
>
OK, need to change that, will send v2.
Thanks,
Taeung
>> For example, when a 'cgroup' section name of a BPF program is used,
>>
>> Before:
>>
>> $ bpftool prog load bpf-prog.o /sys/fs/bpf/prog1
>> Error: failed to guess program type based on section name cgroup
>>
>> After:
>>
>> libbpf: failed to guess program type based on section name 'cgroup'
>> libbpf: possible section(type) names are: socket kprobe/ kretprobe/ classifier action tracepoint/ raw_tracepoint/ xdp perf_event lwt_in lwt_out lwt_xmit lwt_seg6local cgroup_skb/ingress cgroup_skb/egress cgroup/skb cgroup/sock cgroup/post_bind4 cgroup/post_bind6 cgroup/dev sockops sk_skb/stream_parser sk_skb/stream_verdict sk_skb sk_msg lirc_mode2 flow_dissector cgroup/bind4 cgroup/bind6 cgroup/connect4 cgroup/connect6 cgroup/sendmsg4 cgroup/sendmsg6
>>
>> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
>> Cc: Andrey Ignatov <rdna@fb.com>
>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type
2018-12-20 0:47 ` Taeung Song
@ 2018-12-20 2:22 ` Jakub Kicinski
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2018-12-20 2:22 UTC (permalink / raw)
To: Taeung Song; +Cc: Daniel Borkmann, Alexei Starovoitov, netdev, Andrey Ignatov
On Thu, 20 Dec 2018 09:47:12 +0900, Taeung Song wrote:
> On 12/20/18 3:52 AM, Jakub Kicinski wrote:
> > On Wed, 19 Dec 2018 17:52:40 +0900, Taeung Song wrote:
> >> We need to let users check their wrong section name
> >> with proper section names when failed to get proper type from it.
> >> Because there is no knowing what kind of section name can be used.
> >
> > # bpftool prog help
> > Usage: bpftool prog { show | list } [PROG]
> > [...]
> > TYPE := { socket | kprobe | kretprobe | classifier | action |
> > tracepoint | raw_tracepoint | xdp | perf_event | cgroup/skb |
> > cgroup/sock | cgroup/dev | lwt_in | lwt_out | lwt_xmit |
> > lwt_seg6local | sockops | sk_skb | sk_msg | lirc_mode2 |
> > sk_reuseport | flow_dissector |
> > cgroup/bind4 | cgroup/bind6 | cgroup/post_bind4 |
> > cgroup/post_bind6 | cgroup/connect4 | cgroup/connect6 |
> > cgroup/sendmsg4 | cgroup/sendmsg6 }
> > ATTACH_TYPE := { msg_verdict | skb_verdict | skb_parse |
> > flow_dissector }
> > OPTIONS := { {-j|--json} [{-p|--pretty}] | {-f|--bpffs} |
> > {-m|--mapcompat} | {-n|--nomount} }
> >
> > Also in man bpftool-prog.
> >
> > Perhaps we could just point users to that info? IMHO having libraries
>
> Right, I think when users used wrong section names
> we need to let them realize libbpf guess prog/attach types from given
> section names because users who are unfamiliar with BPF programming
> mightn't even know that, right ?
The error messages says:
failed to guess program type based on section name X
I'm not good with user friendly error messages, unfortunately but to me
it clearly state we tried to guess the type based on section name..
Perhaps we could improve it? Say "ELF section name"? Take the section
name into single quotes? Not sure..
> And how about a function that return supported section names ?
> I think it can be used for the 'TYPE'/'ATTACH_TYPE' help message
> because it is just a string and couldn't have latest types.
The man page has to be updated manually, anyway, no? I don't have a
strong opinion as long as formatting doesn't suffer.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-12-20 2:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-19 8:52 [PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type Taeung Song
2018-12-19 11:26 ` Quentin Monnet
2018-12-20 0:17 ` Taeung Song
2018-12-19 18:52 ` Jakub Kicinski
2018-12-20 0:47 ` Taeung Song
2018-12-20 2:22 ` Jakub Kicinski
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).