On Mon, Nov 19, 2018 at 04:46:25PM -0800, Stanislav Fomichev wrote: > [Recent commit 23499442c319 ("bpf: libbpf: retry map creation without > the name") fixed this issue for maps, let's do the same for programs.] > > Since commit 88cda1c9da02 ("bpf: libbpf: Provide basic API support > to specify BPF obj name"), libbpf unconditionally sets bpf_attr->name > for programs. Pre v4.14 kernels don't know about programs names and > return an error about unexpected non-zero data. Retry sys_bpf without > a program name to cover older kernels. > > Signed-off-by: Stanislav Fomichev <s...@google.com> > --- > tools/lib/bpf/bpf.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c > index 961e1b9fc592..cbe9d757c646 100644 > --- a/tools/lib/bpf/bpf.c > +++ b/tools/lib/bpf/bpf.c > @@ -212,6 +212,16 @@ int bpf_load_program_xattr(const struct > bpf_load_program_attr *load_attr, > if (fd >= 0 || !log_buf || !log_buf_sz) > return fd; > > + if (fd < 0 && errno == E2BIG && load_attr->name) { > + /* Retry the same syscall, but without the name. > + * Pre v4.14 kernels don't support prog names. > + */
I'm afraid that will put unnecessary stress on the kernel. This check needs to be tighter. Like E2BIG and anything in the log_buf probably means that E2BIG came from the verifier and nothing to do with prog_name. Asking kernel to repeat is an unnecessary work. In general we need to think beyond this single prog_name field. There are bunch of other fields in bpf_load_program_xattr() and older kernels won't support them. Are we going to zero them out one by one and retry? I don't think that would be practical. Also libbpf silently ignoring prog_name is not great for debugging. A warning is needed. But it cannot be done out of lib/bpf/bpf.c, since it's a set of syscall wrappers. Imo such "old kernel -> lets retry" feature should probably be done at lib/bpf/libbpf.c level. inside load_program().