Hi Alexis,
On Mon, 09 Sep 2024 22:02:07 +0200 Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com wrote:
test_xdp_features.sh is a shell script allowing to test that xdp features advertised by an interface are indeed delivered. The test works by starting two instance of the same program, both attaching specific xdp programs to each side of a veth link, and then make those programs manage packets and collect stats to check whether tested XDP feature is indeed delivered or not. However this test is not integrated in test_progs framework and so can not run automatically in CI.
Rewrite test_xdp_features to integrate it in test_progs so it can run automatically in CI. The main changes brought by the rewrite are the following:
- instead of running to separated processes (each one managing either the tester veth or the DUT vet), run a single process
- slightly change testing direction (v0 is the tester in local namespace, v1 is the Device Under Test in remote namespace)
- group all tests previously managed by test_xdp_features as subtests (one per tested XDP feature). As a consequence, run only once some steps instead of once per subtest (eg: starting/stopping the udp server). On the contrary, make sure that each subtest properly cleans up its state (ie detach xdp programs, reset test stats, etc)
- since there is now a single process, get rid of the "control" tcp channel used to configure DUT. Configuring the DUT now only consists in switching to DUT network namespace and run the relevant commands
- since there is no more control channel, get rid of TLVs, keep only the CMD_ECHO packet type, and set it as a magic
- simplify network setup: use only ipv6 instead of both ipv4 and ipv6, force static neighbours instead of waiting for autoconfiguration, do not force gro (fetch xdp features only once xdp programs are loaded instead)
The existing XDP programs are reused, with some minor changes:
- tester and dut stats maps are converted to global variables for easier usage
- programs do not use TLV struct anymore but the magic replacing the echo command
- avoid to accidentally make tests pass: drop packets instead of forwarding them to userspace when they do not match the expected payload
Signed-off-by: Alexis Lothoré (eBPF Foundation) alexis.lothore@bootlin.com
I'm far for having reviewed the whole patch, but I do have one comment below :)
[...]
+static void *run_dut_echo_thread(void *arg) +{
- struct test_data *t = (struct test_data *)arg;
- __u32 magic;
- while (!t->quit_dut_echo_thread) {
struct sockaddr_storage addr;socklen_t addrlen;size_t n;n = recvfrom(t->echo_server_sock, &magic, sizeof(magic),MSG_WAITALL, (struct sockaddr *)&addr, &addrlen);if (n != sizeof(magic)) {usleep(LOOP_DELAY_US);continue;}if (htonl(magic) != CMD_ECHO)continue;
Shouldn't it be ntohl here ? The former code used the ntohs helper for that command, and you're sending the magic in send_echo_msg with a htonl() so I guess here you might want to convert the value back to host endianness.
Thanks,
Maxime