lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fa9ca4f6-f34d-4957-9100-ba006500116e@bootlin.com>
Date: Wed, 17 Sep 2025 17:37:32 +0200
From: Bastien Curutchet <bastien.curutchet@...tlin.com>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc: Björn Töpel <bjorn@...nel.org>,
 Magnus Karlsson <magnus.karlsson@...el.com>,
 Jonathan Lemon <jonathan.lemon@...il.com>,
 Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
 Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau
 <martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>,
 Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>,
 John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>,
 Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>,
 Jiri Olsa <jolsa@...nel.org>, Mykola Lysenko <mykolal@...com>,
 Shuah Khan <shuah@...nel.org>, "David S. Miller" <davem@...emloft.net>,
 Jakub Kicinski <kuba@...nel.org>, Jesper Dangaard Brouer <hawk@...nel.org>,
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
 Alexis Lothore <alexis.lothore@...tlin.com>, netdev@...r.kernel.org,
 bpf@...r.kernel.org, linux-kselftest@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v3 14/14] selftests/bpf: test_xsk: Integrate
 test_xsk.c to test_progs framework

Hi Maciej,

On 9/16/25 8:16 PM, Maciej Fijalkowski wrote:
> On Thu, Sep 04, 2025 at 12:10:29PM +0200, Bastien Curutchet (eBPF Foundation) wrote:
>> test_xsk.c isn't part of the test_progs framework.
>>
>> Integrate the tests defined by test_xsk.c into the test_progs framework
>> through a new file : prog_tests/xsk.c. ZeroCopy mode isn't tested in it
>> as veth peers don't support it.
>>
>> Move test_xsk{.c/.h} to prog_tests/.
>>
>> Add the find_bit library to test_progs sources in the Makefile as it is
>> is used by test_xsk.c
>>
>> Signed-off-by: Bastien Curutchet (eBPF Foundation) <bastien.curutchet@...tlin.com>
>> ---
>>   tools/testing/selftests/bpf/Makefile               |  13 +-
>>   .../selftests/bpf/{ => prog_tests}/test_xsk.c      |   0
>>   .../selftests/bpf/{ => prog_tests}/test_xsk.h      |   0
>>   tools/testing/selftests/bpf/prog_tests/xsk.c       | 146 +++++++++++++++++++++
>>   tools/testing/selftests/bpf/xskxceiver.c           |   2 +-
>>   5 files changed, 158 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>> index 4bb4f3ee822c1adce0fbd82725b40983695d38b9..1af7d4b9fe54b777131bce0cbb8ca328c885c23a 100644
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>> @@ -541,6 +541,8 @@ TRUNNER_TEST_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.test.o,	\
>>   				 $$(notdir $$(wildcard $(TRUNNER_TESTS_DIR)/*.c)))
>>   TRUNNER_EXTRA_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o,		\
>>   				 $$(filter %.c,$(TRUNNER_EXTRA_SOURCES)))
>> +TRUNNER_LIB_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o,		\
>> +				 $$(filter %.c,$(TRUNNER_LIB_SOURCES)))
>>   TRUNNER_EXTRA_HDRS := $$(filter %.h,$(TRUNNER_EXTRA_SOURCES))
>>   TRUNNER_TESTS_HDR := $(TRUNNER_TESTS_DIR)/tests.h
>>   TRUNNER_BPF_SRCS := $$(notdir $$(wildcard $(TRUNNER_BPF_PROGS_DIR)/*.c))
>> @@ -672,6 +674,10 @@ $(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o:				\
>>   	$$(call msg,EXT-OBJ,$(TRUNNER_BINARY),$$@)
>>   	$(Q)$$(CC) $$(CFLAGS) -c $$< $$(LDLIBS) -o $$@
>>   
>> +$(TRUNNER_LIB_OBJS): $(TRUNNER_OUTPUT)/%.o:$(TOOLSDIR)/lib/%.c
>> +	$$(call msg,LIB-OBJ,$(TRUNNER_BINARY),$$@)
>> +	$(Q)$$(CC) $$(CFLAGS) -c $$< $$(LDLIBS) -o $$@
>> +
>>   # non-flavored in-srctree builds receive special treatment, in particular, we
>>   # do not need to copy extra resources (see e.g. test_btf_dump_case())
>>   $(TRUNNER_BINARY)-extras: $(TRUNNER_EXTRA_FILES) | $(TRUNNER_OUTPUT)
>> @@ -685,6 +691,7 @@ $(OUTPUT)/$(TRUNNER_BINARY): | $(TRUNNER_BPF_OBJS)
>>   
>>   $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS)			\
>>   			     $(TRUNNER_EXTRA_OBJS) $$(BPFOBJ)		\
>> +			     $(TRUNNER_LIB_OBJS)			\
>>   			     $(RESOLVE_BTFIDS)				\
>>   			     $(TRUNNER_BPFTOOL)				\
>>   			     $(OUTPUT)/veristat				\
>> @@ -718,6 +725,7 @@ TRUNNER_EXTRA_SOURCES := test_progs.c		\
>>   			 json_writer.c 		\
>>   			 flow_dissector_load.h	\
>>   			 ip_check_defrag_frags.h
>> +TRUNNER_LIB_SOURCES := find_bit.c
>>   TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read				\
>>   		       $(OUTPUT)/liburandom_read.so			\
>>   		       $(OUTPUT)/xdp_synproxy				\
>> @@ -755,6 +763,7 @@ endif
>>   TRUNNER_TESTS_DIR := map_tests
>>   TRUNNER_BPF_PROGS_DIR := progs
>>   TRUNNER_EXTRA_SOURCES := test_maps.c
>> +TRUNNER_LIB_SOURCES :=
>>   TRUNNER_EXTRA_FILES :=
>>   TRUNNER_BPF_BUILD_RULE := $$(error no BPF objects should be built)
>>   TRUNNER_BPF_CFLAGS :=
>> @@ -776,8 +785,8 @@ $(OUTPUT)/test_verifier: test_verifier.c verifier/tests.h $(BPFOBJ) | $(OUTPUT)
>>   	$(Q)$(CC) $(CFLAGS) $(filter %.a %.o %.c,$^) $(LDLIBS) -o $@
>>   
>>   # Include find_bit.c to compile xskxceiver.
>> -EXTRA_SRC := $(TOOLSDIR)/lib/find_bit.c
>> -$(OUTPUT)/xskxceiver: $(EXTRA_SRC) test_xsk.c test_xsk.h xskxceiver.c xskxceiver.h $(OUTPUT)/network_helpers.o $(OUTPUT)/xsk.o $(OUTPUT)/xsk_xdp_progs.skel.h $(BPFOBJ) | $(OUTPUT)
>> +EXTRA_SRC := $(TOOLSDIR)/lib/find_bit.c prog_tests/test_xsk.c prog_tests/test_xsk.h
>> +$(OUTPUT)/xskxceiver: $(EXTRA_SRC) xskxceiver.c xskxceiver.h $(OUTPUT)/network_helpers.o $(OUTPUT)/xsk.o $(OUTPUT)/xsk_xdp_progs.skel.h $(BPFOBJ) | $(OUTPUT)
>>   	$(call msg,BINARY,,$@)
>>   	$(Q)$(CC) $(CFLAGS) $(filter %.a %.o %.c,$^) $(LDLIBS) -o $@
>>   
>> diff --git a/tools/testing/selftests/bpf/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
>> similarity index 100%
>> rename from tools/testing/selftests/bpf/test_xsk.c
>> rename to tools/testing/selftests/bpf/prog_tests/test_xsk.c
>> diff --git a/tools/testing/selftests/bpf/test_xsk.h b/tools/testing/selftests/bpf/prog_tests/test_xsk.h
>> similarity index 100%
>> rename from tools/testing/selftests/bpf/test_xsk.h
>> rename to tools/testing/selftests/bpf/prog_tests/test_xsk.h
>> diff --git a/tools/testing/selftests/bpf/prog_tests/xsk.c b/tools/testing/selftests/bpf/prog_tests/xsk.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..7ce5ddd7d3fc848df27534f00a6a9f82fbc797c5
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/xsk.c
>> @@ -0,0 +1,146 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#include <net/if.h>
>> +#include <stdarg.h>
>> +
>> +#include "network_helpers.h"
>> +#include "test_progs.h"
>> +#include "test_xsk.h"
>> +#include "xsk_xdp_progs.skel.h"
>> +
>> +#define VETH_RX "veth0"
>> +#define VETH_TX "veth1"
>> +#define MTU	1500
>> +
>> +int setup_veth(bool busy_poll)
>> +{
>> +	SYS(fail,
>> +	"ip link add %s numtxqueues 4 numrxqueues 4 type veth peer name %s numtxqueues 4 numrxqueues 4",
>> +	VETH_RX, VETH_TX);
>> +	SYS(fail, "sysctl -wq net.ipv6.conf.%s.disable_ipv6=1", VETH_RX);
>> +	SYS(fail, "sysctl -wq net.ipv6.conf.%s.disable_ipv6=1", VETH_TX);
>> +
>> +	if (busy_poll) {
>> +		SYS(fail, "echo 2 > /sys/class/net/%s/napi_defer_hard_irqs", VETH_RX);
>> +		SYS(fail, "echo 200000 > /sys/class/net/%s/gro_flush_timeout", VETH_RX);
>> +		SYS(fail, "echo 2 > /sys/class/net/%s/napi_defer_hard_irqs", VETH_TX);
>> +		SYS(fail, "echo 200000 > /sys/class/net/%s/gro_flush_timeout", VETH_TX);
>> +	}
>> +
>> +	SYS(fail, "ip link set %s mtu %d", VETH_RX, MTU);
>> +	SYS(fail, "ip link set %s mtu %d", VETH_TX, MTU);
>> +	SYS(fail, "ip link set %s up", VETH_RX);
>> +	SYS(fail, "ip link set %s up", VETH_TX);
>> +
>> +	return 0;
>> +
>> +fail:
>> +	return -1;
>> +}
>> +
>> +void delete_veth(void)
>> +{
>> +	SYS_NOFAIL("ip link del %s", VETH_RX);
>> +	SYS_NOFAIL("ip link del %s", VETH_TX);
>> +}
>> +
>> +int configure_ifobj(struct ifobject *tx, struct ifobject *rx)
>> +{
>> +	rx->ifindex = if_nametoindex(VETH_RX);
>> +	if (!ASSERT_OK_FD(rx->ifindex, "get RX ifindex"))
>> +		return -1;
>> +
>> +	tx->ifindex = if_nametoindex(VETH_TX);
>> +	if (!ASSERT_OK_FD(tx->ifindex, "get TX ifindex"))
>> +		return -1;
>> +
>> +	tx->shared_umem = false;
>> +	rx->shared_umem = false;
>> +
>> +
>> +	return 0;
>> +}
>> +
>> +static void test_xsk(const struct test_spec *test_to_run, enum test_mode mode)
>> +{
>> +	struct ifobject *ifobj_tx, *ifobj_rx;
>> +	struct test_spec test;
>> +	int ret;
>> +
>> +	ifobj_tx = ifobject_create();
>> +	if (!ASSERT_OK_PTR(ifobj_tx, "create ifobj_tx"))
>> +		return;
>> +
>> +	ifobj_rx = ifobject_create();
>> +	if (!ASSERT_OK_PTR(ifobj_rx, "create ifobj_rx"))
>> +		goto delete_tx;
>> +
>> +	if (!ASSERT_OK(setup_veth(false), "setup veth"))
>> +		goto delete_rx;
>> +
>> +	if (!ASSERT_OK(configure_ifobj(ifobj_tx, ifobj_rx), "conigure ifobj"))
>> +		goto delete_veth;
>> +
>> +	ret = get_hw_ring_size(ifobj_tx->ifname, &ifobj_tx->ring);
>> +	if (!ret) {
>> +		ifobj_tx->hw_ring_size_supp = true;
>> +		ifobj_tx->set_ring.default_tx = ifobj_tx->ring.tx_pending;
>> +		ifobj_tx->set_ring.default_rx = ifobj_tx->ring.rx_pending;
>> +	}
>> +
>> +	if (!ASSERT_OK(init_iface(ifobj_rx, worker_testapp_validate_rx), "init RX"))
>> +		goto delete_veth;
>> +	if (!ASSERT_OK(init_iface(ifobj_tx, worker_testapp_validate_tx), "init TX"))
>> +		goto delete_veth;
>> +
>> +	test_init(&test, ifobj_tx, ifobj_rx, 0, &tests[0]);
>> +
>> +	test.tx_pkt_stream_default = pkt_stream_generate(DEFAULT_PKT_CNT, MIN_PKT_SIZE);
>> +	if (!ASSERT_OK_PTR(test.tx_pkt_stream_default, "TX pkt generation"))
>> +		goto delete_veth;
>> +	test.rx_pkt_stream_default = pkt_stream_generate(DEFAULT_PKT_CNT, MIN_PKT_SIZE);
>> +	if (!ASSERT_OK_PTR(test.rx_pkt_stream_default, "RX pkt generation"))
>> +		goto delete_veth;
>> +
>> +
>> +	test_init(&test, ifobj_tx, ifobj_rx, mode, test_to_run);
>> +	ret = test.test_func(&test);
>> +	if (ret != TEST_SKIP)
>> +		ASSERT_OK(ret, "Run test");
>> +	pkt_stream_restore_default(&test);
>> +
>> +	if (ifobj_tx->hw_ring_size_supp)
>> +		hw_ring_size_reset(ifobj_tx);
>> +
>> +	pkt_stream_delete(test.tx_pkt_stream_default);
>> +	pkt_stream_delete(test.rx_pkt_stream_default);
>> +	xsk_xdp_progs__destroy(ifobj_tx->xdp_progs);
>> +	xsk_xdp_progs__destroy(ifobj_rx->xdp_progs);
>> +
>> +delete_veth:
>> +	delete_veth();
>> +delete_rx:
>> +	ifobject_delete(ifobj_rx);
>> +delete_tx:
>> +	ifobject_delete(ifobj_tx);
>> +}
>> +
>> +void test_ns_xsk_skb(void)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(tests); i++) {
>> +		if (test__start_subtest(tests[i].name))
>> +			test_xsk(&tests[i], TEST_MODE_SKB);
> 
> Ouff. Feels not optimal to setup everything per each test case. Was there
> any stopper from keeping the veth pair per each test mode?
> 

Indeed, it can feel heavy, but IMHO, creating new veth pairs for each 
test case is good way to ensure that the test cases don't collide with 
each other.

Best regards,
-- 
Bastien Curutchet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ