On Fri, Dec 4, 2020 at 9:55 AM Yonghong Song <yhs@xxxxxx> wrote:
>
>
>
> On 12/4/20 1:34 AM, Toke Høiland-Jørgensen wrote:
> > Yonghong Song <yhs@xxxxxx> writes:
> >
> >> On 12/3/20 9:55 AM, Toke Høiland-Jørgensen wrote:
> >>> Hi Andrii
> >>>
> >>> I noticed that recent libbpf versions fail to load BPF files compiled
> >>> with old versions of LLVM. E.g., if I compile xdp-tools with LLVM 7 I
> >>> get:
> >>>
> >>> $ sudo ./xdp-loader load testns ../lib/testing/xdp_drop.o -vv
> >>> Loading 1 files on interface 'testns'.
> >>> libbpf: loading ../lib/testing/xdp_drop.o
> >>> libbpf: elf: section(3) prog, size 16, link 0, flags 6, type=1
> >>> libbpf: sec 'prog': failed to find program symbol at offset 0
> >>> Couldn't open file '../lib/testing/xdp_drop.o': BPF object format invalid
> >>>
> >>> The 'failed to find program symbol' error seems to have been introduced
> >>> with commit c112239272c6 ("libbpf: Parse multi-function sections into
> >>> multiple BPF programs").
> >>>
> >>> Looking at the object file in question, indeed it seems to not have any
> >>> function symbols defined:
> >>>
> >>> $ llvm-objdump --syms ../lib/testing/xdp_drop.o
> >>>
> >>> ../lib/testing/xdp_drop.o: file format elf64-bpf
> >>>
> >>> SYMBOL TABLE:
> >>> 0000000000000000 l .debug_str 0000000000000000
> >>> 0000000000000037 l .debug_str 0000000000000000
> >>> 0000000000000042 l .debug_str 0000000000000000
> >>> 0000000000000068 l .debug_str 0000000000000000
> >>> 0000000000000071 l .debug_str 0000000000000000
> >>> 0000000000000076 l .debug_str 0000000000000000
> >>> 000000000000008a l .debug_str 0000000000000000
> >>> 0000000000000097 l .debug_str 0000000000000000
> >>> 00000000000000a3 l .debug_str 0000000000000000
> >>> 00000000000000ac l .debug_str 0000000000000000
> >>> 00000000000000b5 l .debug_str 0000000000000000
> >>> 00000000000000bc l .debug_str 0000000000000000
> >>> 00000000000000c9 l .debug_str 0000000000000000
> >>> 00000000000000d4 l .debug_str 0000000000000000
> >>> 00000000000000dd l .debug_str 0000000000000000
> >>> 00000000000000e1 l .debug_str 0000000000000000
> >>> 00000000000000e5 l .debug_str 0000000000000000
> >>> 00000000000000ea l .debug_str 0000000000000000
> >>> 00000000000000f0 l .debug_str 0000000000000000
> >>> 00000000000000f9 l .debug_str 0000000000000000
> >>> 0000000000000103 l .debug_str 0000000000000000
> >>> 0000000000000113 l .debug_str 0000000000000000
> >>> 0000000000000122 l .debug_str 0000000000000000
> >>> 0000000000000131 l .debug_str 0000000000000000
> >>> 0000000000000000 l d prog 0000000000000000 prog
> >>> 0000000000000000 l d .debug_abbrev 0000000000000000 .debug_abbrev
> >>> 0000000000000000 l d .debug_info 0000000000000000 .debug_info
> >>> 0000000000000000 l d .debug_frame 0000000000000000 .debug_frame
> >>> 0000000000000000 l d .debug_line 0000000000000000 .debug_line
> >>> 0000000000000000 g license 0000000000000000 _license
> >>> 0000000000000000 g prog 0000000000000000 xdp_drop
> >>>
> >>>
> >>> I assume this is because old LLVM versions simply don't emit that symbol
> >>> information?
>
> Thanks for the below instruction and xdp_drop.c file. I can reproduce
> the issue now.
>
> I added another function 'xdp_drop1' in the same thing. Below is the
> symbol table with llvm7 vs. llvm12.
>
> -bash-4.4$ llvm-readelf -symbols xdp-7.o | grep xdp_drop
> 32: 0000000000000000 0 NOTYPE GLOBAL DEFAULT 3 xdp_drop
> 33: 0000000000000010 0 NOTYPE GLOBAL DEFAULT 3 xdp_drop1
>
> [ 3] prog PROGBITS 0000000000000000 000040 000020
> 00 AX 0 0 8
>
> -bash-4.4$ llvm-readelf -symbols xdp-12.o | grep xdp_drop
> 32: 0000000000000000 16 FUNC GLOBAL DEFAULT 3 xdp_drop
> 33: 0000000000000010 16 FUNC GLOBAL DEFAULT 3 xdp_drop1
> -bash-4.4$
>
> [ 3] prog PROGBITS 0000000000000000 000040 000020
> 00 AX 0 0 8
>
>
> Yes, llvm7 does not encode type and size for FUNC's. I guess libbpf can
> change to recognize NOTYPE and use the symbol value (representing the
> offset from the start of the section) and section size to
> calculate the individual function size. This is more complicated than
> elf file providing FUNC type and symbol size directly.
I think we should just face the fact that LLVM7 is way too old to
produce a sensible BPF ELF file layout. We can extend:
libbpf: sec 'prog': failed to find program symbol at offset 0
Couldn't open file '../lib/testing/xdp_drop.o': BPF object format invalid
with a suggestion to upgrade Clang/LLVM to something more recent, if
that would be helpful. But I don't want to add error-prone checks and
assumptions in the already quite complicated logic. Even the kernel
itself maintains that Clang 10+ needs to be used for its compilation.
BPF CO-RE is also not working with older than Clang10, so lots of
people have already upgraded way beyond that.
Speaking of legacy. Toke, can you please update all the samples in
your xdp-tools repo to not use arbitrary sections names. I see
SEC("prog"), where it should really be SEC("xdp"). It sets a bad
example for newcomers, IMO. I'm also going to emit warnings in libbpf
soon for section names that don't follow proper libbpf naming pattern,
so it would be good if you could get ahead of the curve.
>
> Maybe in this case, libbpf can do some sanity check. If there are more
> than one functions in the 'prog' section and they are not marked at FUNC
> type, simply recommend newer compiler and bail out saying this feature
> not available with old llvm?
>
> >>
> >> Could you share xdp_drop.c or other test which I can compile and check
> >> to understand the issue?
> >
> > It's just an empty program returning XDP_DROP:
> >
> > https://github.com/xdp-project/xdp-tools/blob/master/lib/testing/xdp_drop.c
> >
> > I basically just did this on Debian buster:
> >
> > $ sudo apt install gcc-multilib build-essential libpcap-dev libelf-dev git llc lld clang gcc-multilib pkt-config m4
> > $ git clone --recurse-submodules https://github.com/xdp-project/xdp-tools
> > $ cd xdp-tools
> > $ LLC=llc-7 ./configure
> > $ make -k
> > $ cd xdp-loader
> > $ sudo ip link add dev testns type veth
> > $ sudo ./xdp-loader load testns ../lib/testing/xdp_drop.o -vv
> >
> > (xdpdump will fail to build with llvm7, but the rest should work)
> >
> > -Toke
> >
