BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/2] handle errno ENOTSUPP
@ 2024-07-12 10:14 Geliang Tang
  2024-07-12 10:14 ` [PATCH bpf-next 1/2] bpf: verifier: Fix return value of fixup_call_args Geliang Tang
  2024-07-12 10:14 ` [PATCH bpf-next 2/2] libbpf: handle ENOTSUPP in libbpf_strerror_r Geliang Tang
  0 siblings, 2 replies; 6+ messages in thread
From: Geliang Tang @ 2024-07-12 10:14 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa
  Cc: Geliang Tang, bpf

From: Geliang Tang <tanggeliang@kylinos.cn>

This patchset contains two fixes for handling errno ENOTSUPP. Patch 1
fixes the return value of fixup_call_args() to make sure ENOTSUPP
is returned to user space correctly. Patch 2 handles ENOTSUPP in
libbpf_strerror_r() in libbpf.

Geliang Tang (2):
  bpf: verifier: Fix return value of fixup_call_args
  libbpf: handle ENOTSUPP in libbpf_strerror_r

 kernel/bpf/verifier.c     |  6 +++---
 tools/lib/bpf/str_error.c | 11 +++++++----
 tools/lib/bpf/str_error.h |  4 ++++
 3 files changed, 14 insertions(+), 7 deletions(-)

-- 
2.43.0


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

* [PATCH bpf-next 1/2] bpf: verifier: Fix return value of fixup_call_args
  2024-07-12 10:14 [PATCH bpf-next 0/2] handle errno ENOTSUPP Geliang Tang
@ 2024-07-12 10:14 ` Geliang Tang
  2024-07-12 15:56   ` Alexei Starovoitov
  2024-07-12 10:14 ` [PATCH bpf-next 2/2] libbpf: handle ENOTSUPP in libbpf_strerror_r Geliang Tang
  1 sibling, 1 reply; 6+ messages in thread
From: Geliang Tang @ 2024-07-12 10:14 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa
  Cc: Geliang Tang, bpf

From: Geliang Tang <tanggeliang@kylinos.cn>

Run bloom_filter_map selftests (./test_progs -t bloom_filter_map) on a
Loongarch platform, an error message "JIT doesn't support bpf-to-bpf calls"
is got in user space, together with an unexpected errno EINVAL (22), not
ENOTSUPP (524):

 libbpf: prog 'inner_map': BPF program load failed: Invalid argument
 libbpf: prog 'inner_map': -- BEGIN PROG LOAD LOG --
 JIT doesn't support bpf-to-bpf calls
 callbacks are not allowed in non-JITed programs
 processed 37 insns (limit 1000000) max_states_per_insn 1 total_states
 -- END PROG LOAD LOG --
 libbpf: prog 'inner_map': failed to load: -22
 libbpf: failed to load object 'bloom_filter_map'
 libbpf: failed to load BPF skeleton 'bloom_filter_map': -22
 setup_progs:FAIL:bloom_filter_map__open_and_load unexpected error: -22
 #16      bloom_filter_map:FAIL

Although in jit_subprogs(), the error number does be set as "ENOTSUPP":

	verbose(env, "JIT doesn't support bpf-to-bpf calls\n");
	err = -ENOTSUPP;
	goto out_free;

But afterwards in fixup_call_args(), such error number is ignored, and
overwritten as "-EINVAL":

	verbose(env, "callbacks are not allowed in non-JITed programs\n");
	return -EINVAL;

This patch fixes this by changing return values of fixup_call_args() from
"-EINVAL" to "err ?: -EINVAL". With this change, errno 524 is got in user
space now:

 libbpf: prog 'inner_map': BPF program load failed: unknown error (-524)
 libbpf: prog 'inner_map': -- BEGIN PROG LOAD LOG --
 JIT doesn't support bpf-to-bpf calls
 processed 37 insns (limit 1000000) max_states_per_insn 1 total_states
 -- END PROG LOAD LOG --
 libbpf: prog 'inner_map': failed to load: -524
 libbpf: failed to load object 'bloom_filter_map'
 libbpf: failed to load BPF skeleton 'bloom_filter_map': -524
 setup_progs:FAIL:bloom_filter_map__open_and_load unexpected error: -524

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 kernel/bpf/verifier.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c0263fb5ca4b..aa589fedd036 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -19717,14 +19717,14 @@ static int fixup_call_args(struct bpf_verifier_env *env)
 #ifndef CONFIG_BPF_JIT_ALWAYS_ON
 	if (has_kfunc_call) {
 		verbose(env, "calling kernel functions are not allowed in non-JITed programs\n");
-		return -EINVAL;
+		return err ?: -EINVAL;
 	}
 	if (env->subprog_cnt > 1 && env->prog->aux->tail_call_reachable) {
 		/* When JIT fails the progs with bpf2bpf calls and tail_calls
 		 * have to be rejected, since interpreter doesn't support them yet.
 		 */
 		verbose(env, "tail_calls are not allowed in non-JITed programs with bpf-to-bpf calls\n");
-		return -EINVAL;
+		return err ?: -EINVAL;
 	}
 	for (i = 0; i < prog->len; i++, insn++) {
 		if (bpf_pseudo_func(insn)) {
@@ -19732,7 +19732,7 @@ static int fixup_call_args(struct bpf_verifier_env *env)
 			 * have to be rejected, since interpreter doesn't support them yet.
 			 */
 			verbose(env, "callbacks are not allowed in non-JITed programs\n");
-			return -EINVAL;
+			return err ?: -EINVAL;
 		}
 
 		if (!bpf_pseudo_call(insn))
-- 
2.43.0


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

* [PATCH bpf-next 2/2] libbpf: handle ENOTSUPP in libbpf_strerror_r
  2024-07-12 10:14 [PATCH bpf-next 0/2] handle errno ENOTSUPP Geliang Tang
  2024-07-12 10:14 ` [PATCH bpf-next 1/2] bpf: verifier: Fix return value of fixup_call_args Geliang Tang
@ 2024-07-12 10:14 ` Geliang Tang
  2024-07-12 14:10   ` Geliang Tang
  1 sibling, 1 reply; 6+ messages in thread
From: Geliang Tang @ 2024-07-12 10:14 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa
  Cc: Geliang Tang, bpf

From: Geliang Tang <tanggeliang@kylinos.cn>

The errno 95 (ENOTSUP or EOPNOTSUPP) can be recognized by strerror_r(),
but 524 (ENOTSUPP) can't:

 prog 'basic_alloc3': BPF program load failed: Operation not supported
 prog 'basic_alloc3': failed to load: -95
 failed to load object 'verifier_arena'
 FAIL:unexpected_load_failure unexpected error: -95 (errno 95)

 prog 'inner_map': BPF program load failed: unknown error (-524)
 prog 'inner_map': failed to load: -524
 failed to load object 'bloom_filter_map'
 failed to load BPF skeleton 'bloom_filter_map': -524
 FAIL:bloom_filter_map__open_and_load unexpected error: -524

This patch fixes this by handling ENOTSUPP in libbpf_strerror_r(). With
this change, the new error string looks like:

 prog 'inner_map': BPF program load failed: Operation not supported (-524)

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/lib/bpf/str_error.c | 11 +++++++----
 tools/lib/bpf/str_error.h |  4 ++++
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/lib/bpf/str_error.c b/tools/lib/bpf/str_error.c
index 5e6a1e27ddf9..10597d5124cd 100644
--- a/tools/lib/bpf/str_error.c
+++ b/tools/lib/bpf/str_error.c
@@ -23,10 +23,13 @@ char *libbpf_strerror_r(int err, char *dst, int len)
 	if (ret == -1)
 		ret = errno;
 	if (ret) {
-		if (ret == EINVAL)
-			/* strerror_r() doesn't recognize this specific error */
-			snprintf(dst, len, "unknown error (%d)", err < 0 ? err : -err);
-		else
+		if (ret == EINVAL) {
+			if (err == ENOTSUPP)
+				snprintf(dst, len, "Operation not supported (%d)", -err);
+			else
+				/* strerror_r() doesn't recognize this specific error */
+				snprintf(dst, len, "unknown error (%d)", err < 0 ? err : -err);
+		} else
 			snprintf(dst, len, "ERROR: strerror_r(%d)=%d", err, ret);
 	}
 	return dst;
diff --git a/tools/lib/bpf/str_error.h b/tools/lib/bpf/str_error.h
index 626d7ffb03d6..c41f6ba133cf 100644
--- a/tools/lib/bpf/str_error.h
+++ b/tools/lib/bpf/str_error.h
@@ -4,6 +4,10 @@
 
 #define STRERR_BUFSIZE  128
 
+#ifndef ENOTSUPP
+#define ENOTSUPP 524
+#endif
+
 char *libbpf_strerror_r(int err, char *dst, int len);
 
 #endif /* __LIBBPF_STR_ERROR_H */
-- 
2.43.0


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

* Re: [PATCH bpf-next 2/2] libbpf: handle ENOTSUPP in libbpf_strerror_r
  2024-07-12 10:14 ` [PATCH bpf-next 2/2] libbpf: handle ENOTSUPP in libbpf_strerror_r Geliang Tang
@ 2024-07-12 14:10   ` Geliang Tang
  2024-07-12 15:57     ` Alexei Starovoitov
  0 siblings, 1 reply; 6+ messages in thread
From: Geliang Tang @ 2024-07-12 14:10 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa
  Cc: Geliang Tang, bpf

On Fri, 2024-07-12 at 18:14 +0800, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> The errno 95 (ENOTSUP or EOPNOTSUPP) can be recognized by
> strerror_r(),
> but 524 (ENOTSUPP) can't:
> 
>  prog 'basic_alloc3': BPF program load failed: Operation not
> supported
>  prog 'basic_alloc3': failed to load: -95
>  failed to load object 'verifier_arena'
>  FAIL:unexpected_load_failure unexpected error: -95 (errno 95)
> 
>  prog 'inner_map': BPF program load failed: unknown error (-524)
>  prog 'inner_map': failed to load: -524
>  failed to load object 'bloom_filter_map'
>  failed to load BPF skeleton 'bloom_filter_map': -524
>  FAIL:bloom_filter_map__open_and_load unexpected error: -524
> 
> This patch fixes this by handling ENOTSUPP in libbpf_strerror_r().
> With
> this change, the new error string looks like:
> 
>  prog 'inner_map': BPF program load failed: Operation not supported
> (-524)
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  tools/lib/bpf/str_error.c | 11 +++++++----
>  tools/lib/bpf/str_error.h |  4 ++++
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/lib/bpf/str_error.c b/tools/lib/bpf/str_error.c
> index 5e6a1e27ddf9..10597d5124cd 100644
> --- a/tools/lib/bpf/str_error.c
> +++ b/tools/lib/bpf/str_error.c
> @@ -23,10 +23,13 @@ char *libbpf_strerror_r(int err, char *dst, int
> len)
>  	if (ret == -1)
>  		ret = errno;
>  	if (ret) {
> -		if (ret == EINVAL)
> -			/* strerror_r() doesn't recognize this
> specific error */
> -			snprintf(dst, len, "unknown error (%d)", err
> < 0 ? err : -err);
> -		else
> +		if (ret == EINVAL) {
> +			if (err == ENOTSUPP)

Shouldn't use "err" directly here, should use "err < 0 ? -err : err"
instead. I prefer to add an unsigned variable "no" in v2:

	unsigned int no = err < 0 ? -err : err;

And use "switch-case" for future expansion:
	
	switch (no) {
	case ENOTSUPP:

> +				snprintf(dst, len, "Operation not
> supported (%d)", -err);

No need to use "Operation not supported (-524)" here, just "Operation
not supported" is better.

		snprintf(dst, len, "Operation not supported");

> +			else
> +				/* strerror_r() doesn't recognize
> this specific error */
> +				snprintf(dst, len, "unknown error
> (%d)", err < 0 ? err : -err);

Then here:

		snprintf(dst, len, "unknown error (-%u)", no);

Changes Requested.

Will send a v2 soon.

Thanks,
-Geliang

> +		} else
>  			snprintf(dst, len, "ERROR:
> strerror_r(%d)=%d", err, ret);
>  	}
>  	return dst;
> diff --git a/tools/lib/bpf/str_error.h b/tools/lib/bpf/str_error.h
> index 626d7ffb03d6..c41f6ba133cf 100644
> --- a/tools/lib/bpf/str_error.h
> +++ b/tools/lib/bpf/str_error.h
> @@ -4,6 +4,10 @@
>  
>  #define STRERR_BUFSIZE  128
>  
> +#ifndef ENOTSUPP
> +#define ENOTSUPP 524
> +#endif
> +
>  char *libbpf_strerror_r(int err, char *dst, int len);
>  
>  #endif /* __LIBBPF_STR_ERROR_H */


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

* Re: [PATCH bpf-next 1/2] bpf: verifier: Fix return value of fixup_call_args
  2024-07-12 10:14 ` [PATCH bpf-next 1/2] bpf: verifier: Fix return value of fixup_call_args Geliang Tang
@ 2024-07-12 15:56   ` Alexei Starovoitov
  0 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2024-07-12 15:56 UTC (permalink / raw)
  To: Geliang Tang
  Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Geliang Tang, bpf

On Fri, Jul 12, 2024 at 3:14 AM Geliang Tang <geliang@kernel.org> wrote:
>
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Run bloom_filter_map selftests (./test_progs -t bloom_filter_map) on a
> Loongarch platform, an error message "JIT doesn't support bpf-to-bpf calls"
> is got in user space, together with an unexpected errno EINVAL (22), not
> ENOTSUPP (524):
>
>  libbpf: prog 'inner_map': BPF program load failed: Invalid argument
>  libbpf: prog 'inner_map': -- BEGIN PROG LOAD LOG --
>  JIT doesn't support bpf-to-bpf calls
>  callbacks are not allowed in non-JITed programs
>  processed 37 insns (limit 1000000) max_states_per_insn 1 total_states
>  -- END PROG LOAD LOG --
>  libbpf: prog 'inner_map': failed to load: -22
>  libbpf: failed to load object 'bloom_filter_map'
>  libbpf: failed to load BPF skeleton 'bloom_filter_map': -22
>  setup_progs:FAIL:bloom_filter_map__open_and_load unexpected error: -22
>  #16      bloom_filter_map:FAIL
>
> Although in jit_subprogs(), the error number does be set as "ENOTSUPP":
>
>         verbose(env, "JIT doesn't support bpf-to-bpf calls\n");
>         err = -ENOTSUPP;
>         goto out_free;
>
> But afterwards in fixup_call_args(), such error number is ignored, and
> overwritten as "-EINVAL":
>
>         verbose(env, "callbacks are not allowed in non-JITed programs\n");
>         return -EINVAL;
>
> This patch fixes this by changing return values of fixup_call_args() from
> "-EINVAL" to "err ?: -EINVAL". With this change, errno 524 is got in user
> space now:
>
>  libbpf: prog 'inner_map': BPF program load failed: unknown error (-524)
>  libbpf: prog 'inner_map': -- BEGIN PROG LOAD LOG --
>  JIT doesn't support bpf-to-bpf calls
>  processed 37 insns (limit 1000000) max_states_per_insn 1 total_states
>  -- END PROG LOAD LOG --
>  libbpf: prog 'inner_map': failed to load: -524
>  libbpf: failed to load object 'bloom_filter_map'
>  libbpf: failed to load BPF skeleton 'bloom_filter_map': -524
>  setup_progs:FAIL:bloom_filter_map__open_and_load unexpected error: -524
>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  kernel/bpf/verifier.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index c0263fb5ca4b..aa589fedd036 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -19717,14 +19717,14 @@ static int fixup_call_args(struct bpf_verifier_env *env)
>  #ifndef CONFIG_BPF_JIT_ALWAYS_ON
>         if (has_kfunc_call) {
>                 verbose(env, "calling kernel functions are not allowed in non-JITed programs\n");
> -               return -EINVAL;
> +               return err ?: -EINVAL;
>         }
>         if (env->subprog_cnt > 1 && env->prog->aux->tail_call_reachable) {
>                 /* When JIT fails the progs with bpf2bpf calls and tail_calls
>                  * have to be rejected, since interpreter doesn't support them yet.
>                  */
>                 verbose(env, "tail_calls are not allowed in non-JITed programs with bpf-to-bpf calls\n");
> -               return -EINVAL;
> +               return err ?: -EINVAL;
>         }
>         for (i = 0; i < prog->len; i++, insn++) {
>                 if (bpf_pseudo_func(insn)) {
> @@ -19732,7 +19732,7 @@ static int fixup_call_args(struct bpf_verifier_env *env)
>                          * have to be rejected, since interpreter doesn't support them yet.
>                          */
>                         verbose(env, "callbacks are not allowed in non-JITed programs\n");
> -                       return -EINVAL;
> +                       return err ?: -EINVAL;

Nack.
We're not changing this.
pw-bot: cr

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

* Re: [PATCH bpf-next 2/2] libbpf: handle ENOTSUPP in libbpf_strerror_r
  2024-07-12 14:10   ` Geliang Tang
@ 2024-07-12 15:57     ` Alexei Starovoitov
  0 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2024-07-12 15:57 UTC (permalink / raw)
  To: Geliang Tang
  Cc: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Geliang Tang, bpf

On Fri, Jul 12, 2024 at 7:20 AM Geliang Tang <geliang@kernel.org> wrote:
>
> Changes Requested.
>
> Will send a v2 soon.

Do NOT. Stop this spam of patches.

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

end of thread, other threads:[~2024-07-12 15:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-12 10:14 [PATCH bpf-next 0/2] handle errno ENOTSUPP Geliang Tang
2024-07-12 10:14 ` [PATCH bpf-next 1/2] bpf: verifier: Fix return value of fixup_call_args Geliang Tang
2024-07-12 15:56   ` Alexei Starovoitov
2024-07-12 10:14 ` [PATCH bpf-next 2/2] libbpf: handle ENOTSUPP in libbpf_strerror_r Geliang Tang
2024-07-12 14:10   ` Geliang Tang
2024-07-12 15:57     ` Alexei Starovoitov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox