[PATCH net 0/3] selftests: net: a few fixes

[PATCH net 0/3] selftests: net: a few fixes

[Paolo Abeni]

This series address self-tests failures for udp gro-related tests.

The first patch addresses the main problem I observe locally - the XDP
program required by such tests, xdp_dummy, is currently build in the
ebpf self-tests directory, not available if/when the user targets net
only. Arguably is more a refactor than a fix, but still targeting net
to hopefully 

The second patch fixes the integration of such tests with the build
system.

Patch 3/3 fixes sporadic failures due to races.

Tested with:

make -C tools/testing/selftests/ TARGETS=net install
./tools/testing/selftests/kselftest_install/run_kselftest.sh \
	-t "net:udpgro_bench.sh net:udpgro.sh net:udpgro_fwd.sh \
	    net:udpgro_frglist.sh net:veth.sh"

no failures.

Paolo Abeni (3):
  selftests: net: remove dependency on ebpf tests
  selftests: net: included needed helper in the install targets
  selftests: net: explicitly wait for listener ready

 tools/testing/selftests/net/Makefile          |  6 ++++--
 tools/testing/selftests/net/udpgro.sh         |  4 ++--
 tools/testing/selftests/net/udpgro_bench.sh   |  4 ++--
 tools/testing/selftests/net/udpgro_frglist.sh |  6 +++---
 tools/testing/selftests/net/udpgro_fwd.sh     |  8 +++++---
 tools/testing/selftests/net/veth.sh           |  4 ++--
 tools/testing/selftests/net/xdp_dummy.c       | 13 +++++++++++++
 7 files changed, 31 insertions(+), 14 deletions(-)
 create mode 100644 tools/testing/selftests/net/xdp_dummy.c

-- 
2.43.0

[PATCH net 1/3] selftests: net: remove dependency on ebpf tests

[Paolo Abeni]

Several net tests requires an XDP program build under the ebpf
directory, and error out if such program is not available.

That makes running successful net test hard, let's duplicate into the
net dir the [very small] program, re-using the existing rules to build
it, and finally dropping the bogus dependency.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 tools/testing/selftests/net/Makefile          |  5 +++--
 tools/testing/selftests/net/udpgro.sh         |  4 ++--
 tools/testing/selftests/net/udpgro_bench.sh   |  4 ++--
 tools/testing/selftests/net/udpgro_frglist.sh |  6 +++---
 tools/testing/selftests/net/udpgro_fwd.sh     |  2 +-
 tools/testing/selftests/net/veth.sh           |  4 ++--
 tools/testing/selftests/net/xdp_dummy.c       | 13 +++++++++++++
 7 files changed, 26 insertions(+), 12 deletions(-)
 create mode 100644 tools/testing/selftests/net/xdp_dummy.c

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 50818075e566..304d8b852ef0 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -84,6 +84,7 @@ TEST_PROGS += sctp_vrf.sh
 TEST_GEN_FILES += sctp_hello
 TEST_GEN_FILES += csum
 TEST_GEN_FILES += nat6to4.o
+TEST_GEN_FILES += xdp_dummy.o
 TEST_GEN_FILES += ip_local_port_range
 TEST_GEN_FILES += bind_wildcard
 TEST_PROGS += test_vxlan_mdb.sh
@@ -104,7 +105,7 @@ $(OUTPUT)/tcp_inq: LDLIBS += -lpthread
 $(OUTPUT)/bind_bhash: LDLIBS += -lpthread
 $(OUTPUT)/io_uring_zerocopy_tx: CFLAGS += -I../../../include/
 
-# Rules to generate bpf obj nat6to4.o
+# Rules to generate bpf objs
 CLANG ?= clang
 SCRATCH_DIR := $(OUTPUT)/tools
 BUILD_DIR := $(SCRATCH_DIR)/build
@@ -139,7 +140,7 @@ endif
 
 CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
 
-$(OUTPUT)/nat6to4.o: nat6to4.c $(BPFOBJ) | $(MAKE_DIRS)
+$(OUTPUT)/nat6to4.o $(OUTPUT)/xdp_dummy.o: $(OUTPUT)/%.o : %.c $(BPFOBJ) | $(MAKE_DIRS)
 	$(CLANG) -O2 --target=bpf -c $< $(CCINCLUDE) $(CLANG_SYS_INCLUDES) -o $@
 
 $(BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile)		       \
diff --git a/tools/testing/selftests/net/udpgro.sh b/tools/testing/selftests/net/udpgro.sh
index af5dc57c8ce9..8802604148dd 100755
--- a/tools/testing/selftests/net/udpgro.sh
+++ b/tools/testing/selftests/net/udpgro.sh
@@ -7,7 +7,7 @@ source net_helper.sh
 
 readonly PEER_NS="ns-peer-$(mktemp -u XXXXXX)"
 
-BPF_FILE="../bpf/xdp_dummy.bpf.o"
+BPF_FILE="xdp_dummy.o"
 
 # set global exit status, but never reset nonzero one.
 check_err()
@@ -197,7 +197,7 @@ run_all() {
 }
 
 if [ ! -f ${BPF_FILE} ]; then
-	echo "Missing ${BPF_FILE}. Build bpf selftest first"
+	echo "Missing ${BPF_FILE}. Run 'make' first"
 	exit -1
 fi
 
diff --git a/tools/testing/selftests/net/udpgro_bench.sh b/tools/testing/selftests/net/udpgro_bench.sh
index cb664679b434..7080eae5312b 100755
--- a/tools/testing/selftests/net/udpgro_bench.sh
+++ b/tools/testing/selftests/net/udpgro_bench.sh
@@ -7,7 +7,7 @@ source net_helper.sh
 
 readonly PEER_NS="ns-peer-$(mktemp -u XXXXXX)"
 
-BPF_FILE="../bpf/xdp_dummy.bpf.o"
+BPF_FILE="xdp_dummy.o"
 
 cleanup() {
 	local -r jobs="$(jobs -p)"
@@ -84,7 +84,7 @@ run_all() {
 }
 
 if [ ! -f ${BPF_FILE} ]; then
-	echo "Missing ${BPF_FILE}. Build bpf selftest first"
+	echo "Missing ${BPF_FILE}. Run 'make' first"
 	exit -1
 fi
 
diff --git a/tools/testing/selftests/net/udpgro_frglist.sh b/tools/testing/selftests/net/udpgro_frglist.sh
index dd47fa96f6b3..e1ff645bd3d1 100755
--- a/tools/testing/selftests/net/udpgro_frglist.sh
+++ b/tools/testing/selftests/net/udpgro_frglist.sh
@@ -7,7 +7,7 @@ source net_helper.sh
 
 readonly PEER_NS="ns-peer-$(mktemp -u XXXXXX)"
 
-BPF_FILE="../bpf/xdp_dummy.bpf.o"
+BPF_FILE="xdp_dummy.o"
 
 cleanup() {
 	local -r jobs="$(jobs -p)"
@@ -85,12 +85,12 @@ run_all() {
 }
 
 if [ ! -f ${BPF_FILE} ]; then
-	echo "Missing ${BPF_FILE}. Build bpf selftest first"
+	echo "Missing ${BPF_FILE}. Run 'make' first"
 	exit -1
 fi
 
 if [ ! -f nat6to4.o ]; then
-	echo "Missing nat6to4 helper. Build bpf nat6to4.o selftest first"
+	echo "Missing nat6to4 helper. Run 'make' first"
 	exit -1
 fi
 
diff --git a/tools/testing/selftests/net/udpgro_fwd.sh b/tools/testing/selftests/net/udpgro_fwd.sh
index c079565add39..5fa8659ab13d 100755
--- a/tools/testing/selftests/net/udpgro_fwd.sh
+++ b/tools/testing/selftests/net/udpgro_fwd.sh
@@ -1,7 +1,7 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 
-BPF_FILE="../bpf/xdp_dummy.bpf.o"
+BPF_FILE="xdp_dummy.o"
 readonly BASE="ns-$(mktemp -u XXXXXX)"
 readonly SRC=2
 readonly DST=1
diff --git a/tools/testing/selftests/net/veth.sh b/tools/testing/selftests/net/veth.sh
index 2d073595c620..27574bbf2d63 100755
--- a/tools/testing/selftests/net/veth.sh
+++ b/tools/testing/selftests/net/veth.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 # SPDX-License-Identifier: GPL-2.0
 
-BPF_FILE="../bpf/xdp_dummy.bpf.o"
+BPF_FILE="xdp_dummy.o"
 readonly STATS="$(mktemp -p /tmp ns-XXXXXX)"
 readonly BASE=`basename $STATS`
 readonly SRC=2
@@ -218,7 +218,7 @@ while getopts "hs:" option; do
 done
 
 if [ ! -f ${BPF_FILE} ]; then
-	echo "Missing ${BPF_FILE}. Build bpf selftest first"
+	echo "Missing ${BPF_FILE}. Run 'make' first"
 	exit 1
 fi
 
diff --git a/tools/testing/selftests/net/xdp_dummy.c b/tools/testing/selftests/net/xdp_dummy.c
new file mode 100644
index 000000000000..d988b2e0cee8
--- /dev/null
+++ b/tools/testing/selftests/net/xdp_dummy.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define KBUILD_MODNAME "xdp_dummy"
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+SEC("xdp")
+int xdp_dummy_prog(struct xdp_md *ctx)
+{
+	return XDP_PASS;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.43.0

Re: [PATCH net 1/3] selftests: net: remove dependency on ebpf tests

[Willem de Bruijn]

Paolo Abeni wrote:
> Several net tests requires an XDP program build under the ebpf
> directory, and error out if such program is not available.
> 
> That makes running successful net test hard, let's duplicate into the
> net dir the [very small] program, re-using the existing rules to build
> it, and finally dropping the bogus dependency.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  tools/testing/selftests/net/Makefile          |  5 +++--
>  tools/testing/selftests/net/udpgro.sh         |  4 ++--
>  tools/testing/selftests/net/udpgro_bench.sh   |  4 ++--
>  tools/testing/selftests/net/udpgro_frglist.sh |  6 +++---
>  tools/testing/selftests/net/udpgro_fwd.sh     |  2 +-
>  tools/testing/selftests/net/veth.sh           |  4 ++--
>  tools/testing/selftests/net/xdp_dummy.c       | 13 +++++++++++++
>  7 files changed, 26 insertions(+), 12 deletions(-)
>  create mode 100644 tools/testing/selftests/net/xdp_dummy.c
> 
> diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> index 50818075e566..304d8b852ef0 100644
> --- a/tools/testing/selftests/net/Makefile
> +++ b/tools/testing/selftests/net/Makefile
> @@ -84,6 +84,7 @@ TEST_PROGS += sctp_vrf.sh
>  TEST_GEN_FILES += sctp_hello
>  TEST_GEN_FILES += csum
>  TEST_GEN_FILES += nat6to4.o
> +TEST_GEN_FILES += xdp_dummy.o
>  TEST_GEN_FILES += ip_local_port_range
>  TEST_GEN_FILES += bind_wildcard
>  TEST_PROGS += test_vxlan_mdb.sh
> @@ -104,7 +105,7 @@ $(OUTPUT)/tcp_inq: LDLIBS += -lpthread
>  $(OUTPUT)/bind_bhash: LDLIBS += -lpthread
>  $(OUTPUT)/io_uring_zerocopy_tx: CFLAGS += -I../../../include/
>  
> -# Rules to generate bpf obj nat6to4.o
> +# Rules to generate bpf objs
>  CLANG ?= clang
>  SCRATCH_DIR := $(OUTPUT)/tools
>  BUILD_DIR := $(SCRATCH_DIR)/build
> @@ -139,7 +140,7 @@ endif
>  
>  CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
>  
> -$(OUTPUT)/nat6to4.o: nat6to4.c $(BPFOBJ) | $(MAKE_DIRS)
> +$(OUTPUT)/nat6to4.o $(OUTPUT)/xdp_dummy.o: $(OUTPUT)/%.o : %.c $(BPFOBJ) | $(MAKE_DIRS)
>  	$(CLANG) -O2 --target=bpf -c $< $(CCINCLUDE) $(CLANG_SYS_INCLUDES) -o $@

is the "$(OUTPUT)/%.o :" intentional or a leftover from editing?

Re: [PATCH net 1/3] selftests: net: remove dependency on ebpf tests

[Paolo Abeni]

On Wed, 2024-01-24 at 20:10 -0500, Willem de Bruijn wrote:
> Paolo Abeni wrote:
> > Several net tests requires an XDP program build under the ebpf
> > directory, and error out if such program is not available.
> > 
> > That makes running successful net test hard, let's duplicate into the
> > net dir the [very small] program, re-using the existing rules to build
> > it, and finally dropping the bogus dependency.
> > 
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> >  tools/testing/selftests/net/Makefile          |  5 +++--
> >  tools/testing/selftests/net/udpgro.sh         |  4 ++--
> >  tools/testing/selftests/net/udpgro_bench.sh   |  4 ++--
> >  tools/testing/selftests/net/udpgro_frglist.sh |  6 +++---
> >  tools/testing/selftests/net/udpgro_fwd.sh     |  2 +-
> >  tools/testing/selftests/net/veth.sh           |  4 ++--
> >  tools/testing/selftests/net/xdp_dummy.c       | 13 +++++++++++++
> >  7 files changed, 26 insertions(+), 12 deletions(-)
> >  create mode 100644 tools/testing/selftests/net/xdp_dummy.c
> > 
> > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> > index 50818075e566..304d8b852ef0 100644
> > --- a/tools/testing/selftests/net/Makefile
> > +++ b/tools/testing/selftests/net/Makefile
> > @@ -84,6 +84,7 @@ TEST_PROGS += sctp_vrf.sh
> >  TEST_GEN_FILES += sctp_hello
> >  TEST_GEN_FILES += csum
> >  TEST_GEN_FILES += nat6to4.o
> > +TEST_GEN_FILES += xdp_dummy.o
> >  TEST_GEN_FILES += ip_local_port_range
> >  TEST_GEN_FILES += bind_wildcard
> >  TEST_PROGS += test_vxlan_mdb.sh
> > @@ -104,7 +105,7 @@ $(OUTPUT)/tcp_inq: LDLIBS += -lpthread
> >  $(OUTPUT)/bind_bhash: LDLIBS += -lpthread
> >  $(OUTPUT)/io_uring_zerocopy_tx: CFLAGS += -I../../../include/
> >  
> > -# Rules to generate bpf obj nat6to4.o
> > +# Rules to generate bpf objs
> >  CLANG ?= clang
> >  SCRATCH_DIR := $(OUTPUT)/tools
> >  BUILD_DIR := $(SCRATCH_DIR)/build
> > @@ -139,7 +140,7 @@ endif
> >  
> >  CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
> >  
> > -$(OUTPUT)/nat6to4.o: nat6to4.c $(BPFOBJ) | $(MAKE_DIRS)
> > +$(OUTPUT)/nat6to4.o $(OUTPUT)/xdp_dummy.o: $(OUTPUT)/%.o : %.c $(BPFOBJ) | $(MAKE_DIRS)
> >  	$(CLANG) -O2 --target=bpf -c $< $(CCINCLUDE) $(CLANG_SYS_INCLUDES) -o $@
> 
> is the "$(OUTPUT)/%.o :" intentional or a leftover from editing?

Is intentional and AFAICS required to let this rule being selected when
the output directory is not an empty string (the target and the pre-req
will be in different directories).

Cheers,

Paolo

Re: [PATCH net 1/3] selftests: net: remove dependency on ebpf tests

[Willem de Bruijn]

Paolo Abeni wrote:
> On Wed, 2024-01-24 at 20:10 -0500, Willem de Bruijn wrote:
> > Paolo Abeni wrote:
> > > Several net tests requires an XDP program build under the ebpf
> > > directory, and error out if such program is not available.
> > > 
> > > That makes running successful net test hard, let's duplicate into the
> > > net dir the [very small] program, re-using the existing rules to build
> > > it, and finally dropping the bogus dependency.
> > > 
> > > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > > ---
> > >  tools/testing/selftests/net/Makefile          |  5 +++--
> > >  tools/testing/selftests/net/udpgro.sh         |  4 ++--
> > >  tools/testing/selftests/net/udpgro_bench.sh   |  4 ++--
> > >  tools/testing/selftests/net/udpgro_frglist.sh |  6 +++---
> > >  tools/testing/selftests/net/udpgro_fwd.sh     |  2 +-
> > >  tools/testing/selftests/net/veth.sh           |  4 ++--
> > >  tools/testing/selftests/net/xdp_dummy.c       | 13 +++++++++++++
> > >  7 files changed, 26 insertions(+), 12 deletions(-)
> > >  create mode 100644 tools/testing/selftests/net/xdp_dummy.c
> > > 
> > > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> > > index 50818075e566..304d8b852ef0 100644
> > > --- a/tools/testing/selftests/net/Makefile
> > > +++ b/tools/testing/selftests/net/Makefile
> > > @@ -84,6 +84,7 @@ TEST_PROGS += sctp_vrf.sh
> > >  TEST_GEN_FILES += sctp_hello
> > >  TEST_GEN_FILES += csum
> > >  TEST_GEN_FILES += nat6to4.o
> > > +TEST_GEN_FILES += xdp_dummy.o
> > >  TEST_GEN_FILES += ip_local_port_range
> > >  TEST_GEN_FILES += bind_wildcard
> > >  TEST_PROGS += test_vxlan_mdb.sh
> > > @@ -104,7 +105,7 @@ $(OUTPUT)/tcp_inq: LDLIBS += -lpthread
> > >  $(OUTPUT)/bind_bhash: LDLIBS += -lpthread
> > >  $(OUTPUT)/io_uring_zerocopy_tx: CFLAGS += -I../../../include/
> > >  
> > > -# Rules to generate bpf obj nat6to4.o
> > > +# Rules to generate bpf objs
> > >  CLANG ?= clang
> > >  SCRATCH_DIR := $(OUTPUT)/tools
> > >  BUILD_DIR := $(SCRATCH_DIR)/build
> > > @@ -139,7 +140,7 @@ endif
> > >  
> > >  CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
> > >  
> > > -$(OUTPUT)/nat6to4.o: nat6to4.c $(BPFOBJ) | $(MAKE_DIRS)
> > > +$(OUTPUT)/nat6to4.o $(OUTPUT)/xdp_dummy.o: $(OUTPUT)/%.o : %.c $(BPFOBJ) | $(MAKE_DIRS)
> > >  	$(CLANG) -O2 --target=bpf -c $< $(CCINCLUDE) $(CLANG_SYS_INCLUDES) -o $@
> > 
> > is the "$(OUTPUT)/%.o :" intentional or a leftover from editing?
> 
> Is intentional and AFAICS required to let this rule being selected when
> the output directory is not an empty string (the target and the pre-req
> will be in different directories).

Thanks. I don't understand why. Sorry to harp on this small point, but
you've verified that the build fails without? Is it perhaps due to that
"$(MAKE_DIRS)" order-only-prerequisite? But nat6to4 on its own did not
need this.

Substition references could add a second colon in a rule, but
otherwise I cannot find a reference to this repeated colon syntax.

Don't waste time on my behalf if you're sure this is correct. I just
can't add a reviewed tag if I don't understand it -- but that tag is
hardly essential.

Re: [PATCH net 1/3] selftests: net: remove dependency on ebpf tests

[Paolo Abeni]

On Thu, 2024-01-25 at 09:27 -0500, Willem de Bruijn wrote:
> Paolo Abeni wrote:
> > On Wed, 2024-01-24 at 20:10 -0500, Willem de Bruijn wrote:
> > > Paolo Abeni wrote:
> > > > Several net tests requires an XDP program build under the ebpf
> > > > directory, and error out if such program is not available.
> > > > 
> > > > That makes running successful net test hard, let's duplicate into the
> > > > net dir the [very small] program, re-using the existing rules to build
> > > > it, and finally dropping the bogus dependency.
> > > > 
> > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > > > ---
> > > >  tools/testing/selftests/net/Makefile          |  5 +++--
> > > >  tools/testing/selftests/net/udpgro.sh         |  4 ++--
> > > >  tools/testing/selftests/net/udpgro_bench.sh   |  4 ++--
> > > >  tools/testing/selftests/net/udpgro_frglist.sh |  6 +++---
> > > >  tools/testing/selftests/net/udpgro_fwd.sh     |  2 +-
> > > >  tools/testing/selftests/net/veth.sh           |  4 ++--
> > > >  tools/testing/selftests/net/xdp_dummy.c       | 13 +++++++++++++
> > > >  7 files changed, 26 insertions(+), 12 deletions(-)
> > > >  create mode 100644 tools/testing/selftests/net/xdp_dummy.c
> > > > 
> > > > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> > > > index 50818075e566..304d8b852ef0 100644
> > > > --- a/tools/testing/selftests/net/Makefile
> > > > +++ b/tools/testing/selftests/net/Makefile
> > > > @@ -84,6 +84,7 @@ TEST_PROGS += sctp_vrf.sh
> > > >  TEST_GEN_FILES += sctp_hello
> > > >  TEST_GEN_FILES += csum
> > > >  TEST_GEN_FILES += nat6to4.o
> > > > +TEST_GEN_FILES += xdp_dummy.o
> > > >  TEST_GEN_FILES += ip_local_port_range
> > > >  TEST_GEN_FILES += bind_wildcard
> > > >  TEST_PROGS += test_vxlan_mdb.sh
> > > > @@ -104,7 +105,7 @@ $(OUTPUT)/tcp_inq: LDLIBS += -lpthread
> > > >  $(OUTPUT)/bind_bhash: LDLIBS += -lpthread
> > > >  $(OUTPUT)/io_uring_zerocopy_tx: CFLAGS += -I../../../include/
> > > >  
> > > > -# Rules to generate bpf obj nat6to4.o
> > > > +# Rules to generate bpf objs
> > > >  CLANG ?= clang
> > > >  SCRATCH_DIR := $(OUTPUT)/tools
> > > >  BUILD_DIR := $(SCRATCH_DIR)/build
> > > > @@ -139,7 +140,7 @@ endif
> > > >  
> > > >  CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
> > > >  
> > > > -$(OUTPUT)/nat6to4.o: nat6to4.c $(BPFOBJ) | $(MAKE_DIRS)
> > > > +$(OUTPUT)/nat6to4.o $(OUTPUT)/xdp_dummy.o: $(OUTPUT)/%.o : %.c $(BPFOBJ) | $(MAKE_DIRS)
> > > >  	$(CLANG) -O2 --target=bpf -c $< $(CCINCLUDE) $(CLANG_SYS_INCLUDES) -o $@
> > > 
> > > is the "$(OUTPUT)/%.o :" intentional or a leftover from editing?
> > 
> > Is intentional and AFAICS required to let this rule being selected when
> > the output directory is not an empty string (the target and the pre-req
> > will be in different directories).
> 
> Thanks. I don't understand why. Sorry to harp on this small point, but
> you've verified that the build fails without? Is it perhaps due to that
> "$(MAKE_DIRS)" order-only-prerequisite? But nat6to4 on its own did not
> need this.

I tried quite a bit of permutation (all others failing) before
selecting this one (shame on me, with a stackoverflow hint [!!!]).

But I finally found the relevant documentation reference:

https://www.gnu.org/software/make/manual/make.html#Static-Pattern

A simpler wildcard rule would not be enough, as the already existing
wildcard used to build plain c files will take precedence.

nat6to4 did not need this fancy syntax, as it was a simple, single
target single pre-req rule - that takes precedence on the mentioned
wildcard.

Please let me know if the above clarifies a bit the scenario.

Cheers,

Paolo

Re: [PATCH net 1/3] selftests: net: remove dependency on ebpf tests

[Willem de Bruijn]

Paolo Abeni wrote:
> On Thu, 2024-01-25 at 09:27 -0500, Willem de Bruijn wrote:
> > Paolo Abeni wrote:
> > > On Wed, 2024-01-24 at 20:10 -0500, Willem de Bruijn wrote:
> > > > Paolo Abeni wrote:
> > > > > Several net tests requires an XDP program build under the ebpf
> > > > > directory, and error out if such program is not available.
> > > > > 
> > > > > That makes running successful net test hard, let's duplicate into the
> > > > > net dir the [very small] program, re-using the existing rules to build
> > > > > it, and finally dropping the bogus dependency.
> > > > > 
> > > > > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > > > > ---
> > > > >  tools/testing/selftests/net/Makefile          |  5 +++--
> > > > >  tools/testing/selftests/net/udpgro.sh         |  4 ++--
> > > > >  tools/testing/selftests/net/udpgro_bench.sh   |  4 ++--
> > > > >  tools/testing/selftests/net/udpgro_frglist.sh |  6 +++---
> > > > >  tools/testing/selftests/net/udpgro_fwd.sh     |  2 +-
> > > > >  tools/testing/selftests/net/veth.sh           |  4 ++--
> > > > >  tools/testing/selftests/net/xdp_dummy.c       | 13 +++++++++++++
> > > > >  7 files changed, 26 insertions(+), 12 deletions(-)
> > > > >  create mode 100644 tools/testing/selftests/net/xdp_dummy.c
> > > > > 
> > > > > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> > > > > index 50818075e566..304d8b852ef0 100644
> > > > > --- a/tools/testing/selftests/net/Makefile
> > > > > +++ b/tools/testing/selftests/net/Makefile
> > > > > @@ -84,6 +84,7 @@ TEST_PROGS += sctp_vrf.sh
> > > > >  TEST_GEN_FILES += sctp_hello
> > > > >  TEST_GEN_FILES += csum
> > > > >  TEST_GEN_FILES += nat6to4.o
> > > > > +TEST_GEN_FILES += xdp_dummy.o
> > > > >  TEST_GEN_FILES += ip_local_port_range
> > > > >  TEST_GEN_FILES += bind_wildcard
> > > > >  TEST_PROGS += test_vxlan_mdb.sh
> > > > > @@ -104,7 +105,7 @@ $(OUTPUT)/tcp_inq: LDLIBS += -lpthread
> > > > >  $(OUTPUT)/bind_bhash: LDLIBS += -lpthread
> > > > >  $(OUTPUT)/io_uring_zerocopy_tx: CFLAGS += -I../../../include/
> > > > >  
> > > > > -# Rules to generate bpf obj nat6to4.o
> > > > > +# Rules to generate bpf objs
> > > > >  CLANG ?= clang
> > > > >  SCRATCH_DIR := $(OUTPUT)/tools
> > > > >  BUILD_DIR := $(SCRATCH_DIR)/build
> > > > > @@ -139,7 +140,7 @@ endif
> > > > >  
> > > > >  CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG),$(CLANG_TARGET_ARCH))
> > > > >  
> > > > > -$(OUTPUT)/nat6to4.o: nat6to4.c $(BPFOBJ) | $(MAKE_DIRS)
> > > > > +$(OUTPUT)/nat6to4.o $(OUTPUT)/xdp_dummy.o: $(OUTPUT)/%.o : %.c $(BPFOBJ) | $(MAKE_DIRS)
> > > > >  	$(CLANG) -O2 --target=bpf -c $< $(CCINCLUDE) $(CLANG_SYS_INCLUDES) -o $@
> > > > 
> > > > is the "$(OUTPUT)/%.o :" intentional or a leftover from editing?
> > > 
> > > Is intentional and AFAICS required to let this rule being selected when
> > > the output directory is not an empty string (the target and the pre-req
> > > will be in different directories).
> > 
> > Thanks. I don't understand why. Sorry to harp on this small point, but
> > you've verified that the build fails without? Is it perhaps due to that
> > "$(MAKE_DIRS)" order-only-prerequisite? But nat6to4 on its own did not
> > need this.
> 
> I tried quite a bit of permutation (all others failing) before
> selecting this one (shame on me, with a stackoverflow hint [!!!]).
> 
> But I finally found the relevant documentation reference:
> 
> https://www.gnu.org/software/make/manual/make.html#Static-Pattern
> 
> A simpler wildcard rule would not be enough, as the already existing
> wildcard used to build plain c files will take precedence.
> 
> nat6to4 did not need this fancy syntax, as it was a simple, single
> target single pre-req rule - that takes precedence on the mentioned
> wildcard.
> 
> Please let me know if the above clarifies a bit the scenario.

Reviewed-by: Willem de Bruijn <willemb@google.com>

Thanks for looking that up!

So the wildcard is needed for the %.c in the new rule. Makes sense.

[PATCH net 2/3] selftests: net: included needed helper in the install targets

[Paolo Abeni]

The blamed commit below introduce a dependency in some net self-tests
towards a newly introduce helper script.

Such script is currently not included into the TEST_PROGS_EXTENDED list
and thus is not installed, causing failure for the relevant tests when
executed from the install dir.

Fix the issue updating the install targets.

Fixes: 3bdd9fd29cb0 ("selftests/net: synchronize udpgro tests' tx and rx connection")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 tools/testing/selftests/net/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 304d8b852ef0..48c6f93b8149 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -55,6 +55,7 @@ TEST_PROGS += rps_default_mask.sh
 TEST_PROGS += big_tcp.sh
 TEST_PROGS_EXTENDED := in_netns.sh setup_loopback.sh setup_veth.sh
 TEST_PROGS_EXTENDED += toeplitz_client.sh toeplitz.sh lib.sh
+TEST_PROGS_EXTENDED += net_helper.sh
 TEST_GEN_FILES =  socket nettest
 TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy reuseport_addr_any
 TEST_GEN_FILES += tcp_mmap tcp_inq psock_snd txring_overwrite
-- 
2.43.0

Re: [PATCH net 2/3] selftests: net: included needed helper in the install targets

[Willem de Bruijn]

Paolo Abeni wrote:
> The blamed commit below introduce a dependency in some net self-tests
> towards a newly introduce helper script.
> 
> Such script is currently not included into the TEST_PROGS_EXTENDED list
> and thus is not installed, causing failure for the relevant tests when
> executed from the install dir.
> 
> Fix the issue updating the install targets.
> 
> Fixes: 3bdd9fd29cb0 ("selftests/net: synchronize udpgro tests' tx and rx connection")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

[PATCH net 3/3] selftests: net: explicitly wait for listener ready

[Paolo Abeni]

The UDP GRO forwarding test still hard-code an arbitrary pause
to wait for the UDP listener becoming ready in background.

That causes sporadic failures depending on the host load.

Replace the sleep with the existing helper waiting for the desired
port being exposed.

Fixes: a062260a9d5f ("selftests: net: add UDP GRO forwarding self-tests")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 tools/testing/selftests/net/udpgro_fwd.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/udpgro_fwd.sh b/tools/testing/selftests/net/udpgro_fwd.sh
index 5fa8659ab13d..d6b9c759043c 100755
--- a/tools/testing/selftests/net/udpgro_fwd.sh
+++ b/tools/testing/selftests/net/udpgro_fwd.sh
@@ -1,6 +1,8 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 
+source net_helper.sh
+
 BPF_FILE="xdp_dummy.o"
 readonly BASE="ns-$(mktemp -u XXXXXX)"
 readonly SRC=2
@@ -119,7 +121,7 @@ run_test() {
 	ip netns exec $NS_DST $ipt -A INPUT -p udp --dport 8000
 	ip netns exec $NS_DST ./udpgso_bench_rx -C 1000 -R 10 -n 10 -l 1300 $rx_args &
 	local spid=$!
-	sleep 0.1
+	wait_local_port_listen "$NS_DST" 8000 udp
 	ip netns exec $NS_SRC ./udpgso_bench_tx $family -M 1 -s 13000 -S 1300 -D $dst
 	local retc=$?
 	wait $spid
@@ -168,7 +170,7 @@ run_bench() {
 	ip netns exec $NS_DST bash -c "echo 2 > /sys/class/net/veth$DST/queues/rx-0/rps_cpus"
 	ip netns exec $NS_DST taskset 0x2 ./udpgso_bench_rx -C 1000 -R 10  &
 	local spid=$!
-	sleep 0.1
+	wait_local_port_listen "$NS_DST" 8000 udp
 	ip netns exec $NS_SRC taskset 0x1 ./udpgso_bench_tx $family -l 3 -S 1300 -D $dst
 	local retc=$?
 	wait $spid
-- 
2.43.0

Re: [PATCH net 3/3] selftests: net: explicitly wait for listener ready

[Willem de Bruijn]

Paolo Abeni wrote:
> The UDP GRO forwarding test still hard-code an arbitrary pause
> to wait for the UDP listener becoming ready in background.
> 
> That causes sporadic failures depending on the host load.
> 
> Replace the sleep with the existing helper waiting for the desired
> port being exposed.
> 
> Fixes: a062260a9d5f ("selftests: net: add UDP GRO forwarding self-tests")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Re: [PATCH net 0/3] selftests: net: a few fixes

[Jakub Kicinski]

On Wed, 24 Jan 2024 22:33:19 +0100 Paolo Abeni wrote:
> This series address self-tests failures for udp gro-related tests.
> 
> The first patch addresses the main problem I observe locally - the XDP
> program required by such tests, xdp_dummy, is currently build in the
> ebpf self-tests directory, not available if/when the user targets net
> only. Arguably is more a refactor than a fix, but still targeting net
> to hopefully 
> 
> The second patch fixes the integration of such tests with the build
> system.
> 
> Patch 3/3 fixes sporadic failures due to races.
> 
> Tested with:
> 
> make -C tools/testing/selftests/ TARGETS=net install
> ./tools/testing/selftests/kselftest_install/run_kselftest.sh \
> 	-t "net:udpgro_bench.sh net:udpgro.sh net:udpgro_fwd.sh \
> 	    net:udpgro_frglist.sh net:veth.sh"
> 
> no failures.

This series got into net-next-2024-01-25--00-00, looking at the outputs
it turns out that the iproute2 was crashing somewhere in libbpf :(
I rebuilt latest iproute2 without libbpf support. It loads xdp_dummy.o
just fine, so hopefully that's good enough for now, we'll see for sure
in about 3 hours :)

Re: [PATCH net 0/3] selftests: net: a few fixes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 24 Jan 2024 22:33:19 +0100 you wrote:
> This series address self-tests failures for udp gro-related tests.
> 
> The first patch addresses the main problem I observe locally - the XDP
> program required by such tests, xdp_dummy, is currently build in the
> ebpf self-tests directory, not available if/when the user targets net
> only. Arguably is more a refactor than a fix, but still targeting net
> to hopefully
> 
> [...]

Here is the summary with links:
  - [net,1/3] selftests: net: remove dependency on ebpf tests
    https://git.kernel.org/netdev/net/c/98cb12eb52a7
  - [net,2/3] selftests: net: included needed helper in the install targets
    https://git.kernel.org/netdev/net/c/f5173fe3e13b
  - [net,3/3] selftests: net: explicitly wait for listener ready
    https://git.kernel.org/netdev/net/c/4acffb66630a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html