Re: [PATCH bpf-next v2] libbpf: fix probe code to return EPERM if encountered

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4 May 2020, at 20:43, Andrii Nakryiko wrote:

On Mon, May 4, 2020 at 2:13 AM Eelco Chaudron <echaudro@xxxxxxxxxx> wrote:

When the probe code was failing for any reason ENOTSUP was returned, even if this was due to no having enough lock space. This patch fixes this by returning EPERM to the user application, so it can respond and increase
the RLIMIT_MEMLOCK size.

Signed-off-by: Eelco Chaudron <echaudro@xxxxxxxxxx>
---
v2: Split bpf_object__probe_name() in two functions as suggested by Andrii

Yeah, looks good, and this is good enough, so consider you have my
ack. But I think we can further improve the experience by:

1. Changing existing "Couldn't load basic 'r0 = 0' BPF program."
message to be something more meaningful and actionable for user. E.g.,

"Couldn't load trivial BPF program. Make sure your kernel supports BPF
(CONFIG_BPF_SYSCALL=y) and/or that RLIMIT_MEMLOCK is set to big enough
value."

I had pr_perm_msg() in the previous patch, but I forgot to put it back in :( However your message looks way better, so I plan to send a v3 with the following:

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6838e6d431ce..ad3043c5db13 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3170,8 +3170,10 @@ bpf_object__probe_loading(struct bpf_object *obj)
        ret = bpf_load_program_xattr(&attr, NULL, 0);
        if (ret < 0) {
                cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
- pr_warn("Error in %s():%s(%d). Couldn't load basic 'r0 = 0' BPF program.\n",
-                       __func__, cp, errno);
+ pr_warn("Error in %s():%s(%d). Couldn't load trivial BPF "
+                       "program. Make sure your kernel supports BPF "
+ "(CONFIG_BPF_SYSCALL=y) and/or that RLIMIT_MEMLOCK is " + "set to big enough value.\n", __func__, cp, errno);
                return -errno;
        }
        close(ret);

Then even complete kernel newbies can search for CONFIG_BPF_SYSCALL or
RLIMIT_MEMLOCK and hopefully find useful discussions. We can/should
add RLIMIT_MEMLOCK examples to some FAQ, probably as well (if it's not
there already).

The xdp-tutorial repo has examples on how to set it to unlimited ;)
Also, the xdp-tool’s repo has some examples on how to dynamically try to increase it, for examples:

http://172.16.1.201:8080/source/xref/xdp-tools/xdp-loader/xdp-loader.c?r=1926fc3d#198

2. I'd do bpf_object__probe_loading() before obj->loaded is set, so
that user can have a loop of bpf_object__load() that bump
RLIMIT_MEMLOCK in steps. After setting obj->loaded = true, user won't
be able to attemp loading again and will get "object should not be
loaded twice\n".

Guess this was acked in Toke’s thread.

[...]





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux