Re: [PATCH v2 bpf-next] libbpf: always specify expected_attach_type on program load if supported

[Andrey Ignatov <rdna@fb.com>]

Andrii Nakryiko <andrii.nakryiko@gmail.com> [Tue, 2020-04-14 11:25 -0700]:
> On Tue, Apr 14, 2020 at 10:56 AM Andrey Ignatov <rdna@fb.com> wrote:
> >
> > Andrii Nakryiko <andriin@fb.com> [Mon, 2020-04-13 21:56 -0700]:

...

> > > v1->v2:
> > > - fixed prog_type/expected_attach_type combo (Andrey);
> > > - added comment explaining what we are doing in probe_exp_attach_type (Andrey).
> >
> > Thanks for changes.
> >
> > I built the patch (removing the double .sec Song mentioned since it
> > breaks compilation) and tested it: it fixes the problem with NET_XMIT_CN
> 
> Wait, what? How does it break compilation? I compiled and tested
> before submitting and just cleaned and built again, no compilation
> errors or even warnings. Can you share compilation error you got,
> please?

Sure:

	11:37:28 1 rdna@dev082.prn2:~/bpf-next$>/home/rdna/bin/clang --version
	clang version 9.0.20190721
	Target: x86_64-unknown-linux-gnu
	Thread model: posix
	InstalledDir: /home/rdna/bin
	11:37:32 0 rdna@dev082.prn2:~/bpf-next$>env GCC=~/bin/gcc CLANG=~/bin/clang CC=~/bin/clang LLC=~/bin/llc LLVM_STRIP=~/bin/llvm-strip  make V=1 -C tools/bpf/bpftool/
	
	Auto-detecting system features:
	...                        libbfd: [ on  ]
	...        disassembler-four-args: [ OFF ]
	...                          zlib: [ on  ]
	...          clang-bpf-global-var: [ on  ]
	
	make: Entering directory `/home/rdna/bpf-next/tools/bpf/bpftool'
	make -C /home/rdna/bpf-next/tools/lib/bpf/ OUTPUT= libbpf.a
	make[1]: Entering directory `/home/rdna/bpf-next/tools/lib/bpf'
	make -f /home/rdna/bpf-next/tools/build/Makefile.build dir=. obj=libbpf OUTPUT=staticobjs/
	  /home/rdna/bin/clang -Wp,-MD,staticobjs/.libbpf.o.d -Wp,-MT,staticobjs/libbpf.o -g -Wall -DHAVE_LIBELF_MMAP_SUPPORT -DCOMPAT_NEED_REALLOCARRAY -Wbad-function-cast -Wdeclaration-after-statement -Wformat-security -Wformat-y2k -Winit-self -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wno-system-headers -Wold-style-definition -Wpacked -Wredundant-decls -Wstrict-prototypes -Wswitch-default -Wswitch-enum -Wundef -Wwrite-strings -Wformat -fno-strict-aliasing -Wno-shadow -Werror -Wall -fPIC -I. -I/home/rdna/bpf-next/tools/include -I/home/rdna/bpf-next/tools/arch/x86/include/uapi -I/home/rdna/bpf-next/tools/include/uapi -fvisibility=hidden -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D"BUILD_STR(s)=#s" -c -o staticobjs/libbpf.o libbpf.c
	libbpf.c:6329:15: error: initializer overrides prior initialization of this subobject [-Werror,-Winitializer-overrides]
	        BPF_PROG_SEC("socket",                  BPF_PROG_TYPE_SOCKET_FILTER),
	                     ^~~~~~~~
	libbpf.c:6291:55: note: expanded from macro 'BPF_PROG_SEC'
	#define BPF_PROG_SEC(string, ptype) BPF_PROG_SEC_IMPL(string, ptype, 0, 0, 0, 0)
	                                                      ^~~~~~
	libbpf.c:6283:10: note: expanded from macro 'BPF_PROG_SEC_IMPL'
	                .sec = string,                                              \
	                       ^~~~~~
	libbpf.c:6329:15: note: previous initialization is here
	        BPF_PROG_SEC("socket",                  BPF_PROG_TYPE_SOCKET_FILTER),
	                     ^~~~~~~~
	libbpf.c:6291:55: note: expanded from macro 'BPF_PROG_SEC'
	#define BPF_PROG_SEC(string, ptype) BPF_PROG_SEC_IMPL(string, ptype, 0, 0, 0, 0)
	                                                      ^~~~~~
	libbpf.c:6280:10: note: expanded from macro 'BPF_PROG_SEC_IMPL'
	                .sec = string,                                              \
	                       ^~~~~~
	libbpf.c:6330:15: error: initializer overrides prior initialization of this subobject [-Werror,-Winitializer-overrides]
	        BPF_PROG_SEC("sk_reuseport",            BPF_PROG_TYPE_SK_REUSEPORT),
	                     ^~~~~~~~~~~~~~
	libbpf.c:6291:55: note: expanded from macro 'BPF_PROG_SEC'
	#define BPF_PROG_SEC(string, ptype) BPF_PROG_SEC_IMPL(string, ptype, 0, 0, 0, 0)
	                                                      ^~~~~~
	libbpf.c:6283:10: note: expanded from macro 'BPF_PROG_SEC_IMPL'
	                .sec = string,                                              \
	                       ^~~~~~
	libbpf.c:6330:15: note: previous initialization is here
	        BPF_PROG_SEC("sk_reuseport",            BPF_PROG_TYPE_SK_REUSEPORT),
	                     ^~~~~~~~~~~~~~
	libbpf.c:6291:55: note: expanded from macro 'BPF_PROG_SEC'
	#define BPF_PROG_SEC(string, ptype) BPF_PROG_SEC_IMPL(string, ptype, 0, 0, 0, 0)
	                                                      ^~~~~~
	libbpf.c:6280:10: note: expanded from macro 'BPF_PROG_SEC_IMPL'
	                .sec = string,                                              \
	                       ^~~~~~
	
	... and so on ...
	
	fatal error: too many errors emitted, stopping now [-ferror-limit=]
	20 errors generated.
	   ld -r -o staticobjs/libbpf-in.o  staticobjs/libbpf.o staticobjs/bpf.o staticobjs/nlattr.o staticobjs/btf.o staticobjs/libbpf_errno.o staticobjs/str_error.o staticobjs/netlink.o staticobjs/bpf_prog_linfo.o staticobjs/libbpf_probes.o staticobjs/xsk.o staticobjs/hashmap.o staticobjs/btf_dump.o
	ld: cannot find staticobjs/libbpf.o: No such file or directory
	make[2]: *** [staticobjs/libbpf-in.o] Error 1
	make[1]: *** [staticobjs/libbpf-in.o] Error 2
	make[1]: Leaving directory `/home/rdna/bpf-next/tools/lib/bpf'
	make: *** [/home/rdna/bpf-next/tools/lib/bpf/libbpf.a] Error 2
	make: Leaving directory `/home/rdna/bpf-next/tools/bpf/bpftool'
	11:37:43 2 rdna@dev082.prn2:~/bpf-next$>

> > I guess we can deal with selftest separately in the original thread.
> 
> Sure, if this is going to be applied to bpf as a fix, I'd rather
> follow-up with selftests separately.

Sounds good.

> > Also a question about bpf vs bpf-next: since this fixes real problem
> > with loading cgroup skb programs, should it go to bpf tree instead?
> 
> It will be up to maintainers, it's not so clear whether it's a new
> feature or a bug fix.. I don't mind either way.

Sounds good. Thanks.

-- 
Andrey Ignatov