[PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type

[PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type

[Taeung Song]

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

Re: [PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type

[Quentin Monnet]

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

Re: [PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type

[Taeung Song]

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

Re: [PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type

[Jakub Kicinski]

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>

Re: [PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type

[Taeung Song]

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>

Re: [PATCH bpf-next] libbpf: Show possible section(type) names on when failed to guess a type

[Jakub Kicinski]

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.