On Tue, May 5, 2020 at 3:07 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: > > Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> writes: > > > 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." > > > > 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). > > Always on board with improving documentation; and yeah I agree that > "Couldn't load basic 'r0 = 0' BPF program." could be a bit friendlier ;) > > > 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". > > In practice this is not going to be enough, though. The memlock error > only triggers on initial load if the limit is already exceeded (by other > BPF programs); but often what will happen is that the program being > loaded will have a map definition that's big enough to exhaust the > memlimit by itself. In which case the memlock error will trigger while > creating maps, not on initial probe. > > Since you can't predict where the error will happen, you need to be > prepared to close the bpf object and start over anyway, so I'm not sure > it adds much value to move bpf_object__probe_loading() earlier? > True that. Ok, sounds fine to me (error message would be still nice to improve, though). > -Toke >
