Re: [RFC PATCH v2 08/10] selftests: conditionally enable XDP support in udpgso_bench_rx

[Paolo Abeni <pabeni@redhat.com>]

On Sun, 2018-10-21 at 16:09 -0400, Willem de Bruijn wrote:
> On Fri, Oct 19, 2018 at 10:31 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > 
> > XDP support will be used by a later patch to test the GRO path
> > in a net namespace, leveraging the veth XDP implementation.
> > To avoid breaking existing setup, XDP support is conditionally
> > enabled and build only if llc is locally available.
> > 
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> > index 256d82d5fa87..176459b7c4d6 100644
> > --- a/tools/testing/selftests/net/Makefile
> > +++ b/tools/testing/selftests/net/Makefile
> > @@ -16,8 +16,77 @@ TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
> >  TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict tls
> > 
> >  KSFT_KHDR_INSTALL := 1
> > +
> > +# Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
> > +#  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
> > +LLC ?= llc
> > +CLANG ?= clang
> > +LLVM_OBJCOPY ?= llvm-objcopy
> > +BTF_PAHOLE ?= pahole
> > +HAS_LLC := $(shell which $(LLC) 2>/dev/null)
> > +
> > +# conditional enable testes requiring llc
> > +ifneq (, $(HAS_LLC))
> > +TEST_GEN_FILES += xdp_dummy.o
> > +endif
> > +
> >  include ../lib.mk
> > 
> > +ifneq (, $(HAS_LLC))
> > +
> > +# Detect that we're cross compiling and use the cross compiler
> > +ifdef CROSS_COMPILE
> > +CLANG_ARCH_ARGS = -target $(ARCH)
> > +endif
> > +
> > +PROBE := $(shell $(LLC) -march=bpf -mcpu=probe -filetype=null /dev/null 2>&1)
> > +
> > +# Let newer LLVM versions transparently probe the kernel for availability
> > +# of full BPF instruction set.
> > +ifeq ($(PROBE),)
> > +  CPU ?= probe
> > +else
> > +  CPU ?= generic
> > +endif
> > +
> > +SRC_PATH := $(abspath ../../../..)
> > +LIB_PATH := $(SRC_PATH)/tools/lib
> > +XDP_CFLAGS := -D SUPPORT_XDP=1 -I$(LIB_PATH)
> > +LIBBPF = $(LIB_PATH)/bpf/libbpf.a
> > +BTF_LLC_PROBE := $(shell $(LLC) -march=bpf -mattr=help 2>&1 | grep dwarfris)
> > +BTF_PAHOLE_PROBE := $(shell $(BTF_PAHOLE) --help 2>&1 | grep BTF)
> > +BTF_OBJCOPY_PROBE := $(shell $(LLVM_OBJCOPY) --help 2>&1 | grep -i 'usage.*llvm')
> > +CLANG_SYS_INCLUDES := $(shell $(CLANG) -v -E - </dev/null 2>&1 \
> > +        | sed -n '/<...> search starts here:/,/End of search list./{ s| \(/.*\)|-idirafter \1|p }')
> > +CLANG_FLAGS = -I. -I$(SRC_PATH)/include -I../bpf/ \
> > +             $(CLANG_SYS_INCLUDES) -Wno-compare-distinct-pointer-types
> > +
> > +ifneq ($(and $(BTF_LLC_PROBE),$(BTF_PAHOLE_PROBE),$(BTF_OBJCOPY_PROBE)),)
> > +       CLANG_CFLAGS += -g
> > +       LLC_FLAGS += -mattr=dwarfris
> > +       DWARF2BTF = y
> > +endif
> > +
> > +$(LIBBPF): FORCE
> > +# Fix up variables inherited from Kbuild that tools/ build system won't like
> > +       $(MAKE) -C $(dir $@) RM='rm -rf' LDFLAGS= srctree=$(SRC_PATH) O= $(nodir $@)
> > +
> 
> This is a lot of XDP specific code. Not for this patchset, per se, but
> would be nice if we can reuse the logic in selftests/bpf for all this.

Agreed. A very similar code is already present almost duplicated in 3
different places (samples/bpf/Makefile, tools/testing/selftests/tc-
testing/bpf/Makefile and tools/testing/selftests/bpf/Makefile). A
bfp_lib.mk or the like would be nice ;). But I felt it a bit out of
scope for this patch, and I'm new to XDP/ebpf, so I preferred avoid
additional issues.

> > --- a/tools/testing/selftests/net/udpgso_bench_rx.c
> > @@ -227,6 +234,13 @@ static void parse_opts(int argc, char **argv)
> >                         cfg_verify = true;
> >                         cfg_read_all = true;
> >                         break;
> > +#ifdef SUPPORT_XDP
> > +               case 'x':
> > +                       cfg_xdp_iface = if_nametoindex(optarg);
> > +                       if (!cfg_xdp_iface)
> > +                               error(1, errno, "unknown interface %s", optarg);
> > +                       break;
> > +#endif
> 
> nit: needs to be added to getopt string in this patch.

Thanks, will do in next iteration.

Cheers,

Paolo