Skip to content

Conversation

@kernel-patches-daemon-bpf
Copy link

Pull request for series with
subject: selftests/bpf: convert test_tc_tunnel.sh to test_progs
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1014381

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: e758657
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1014381
version: 2

@kernel-patches-review-bot
Copy link

commit e98dbe270f1e4290d72c329febbc828b7a99acb0
Author: Alexis Lothoré (eBPF Foundation) <[email protected]>

selftests/bpf: integrate test_tc_tunnel.sh tests into test_progs

This commit integrates test_tc_tunnel.sh into test_progs framework,
creating a new test_tc_tunnel.c that validates BPF-based tunnel
encapsulation and decapsulation for various tunnel types.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c b/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c
> new file mode 100644
> index 000000000..bf60ffcfe
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/test_tc_tunnel.c
> @@ -0,0 +1,660 @@

[ ... ]

> +static struct connection *connect_client_to_server(struct subtest_cfg *cfg)
> +{
> +	struct network_helper_opts opts = {.timeout_ms = 500};
> +	int family = cfg->ipproto == 6 ? AF_INET6 : AF_INET;
> +	struct connection *conn = NULL;
> +	int client_fd, server_fd;
> +
> +	client_fd = connect_to_addr_str(family, SOCK_STREAM, cfg->server_addr,
> +					TEST_PORT, &opts);
> +
> +	if (client_fd < 0)
> +		return NULL;
> +
> +	server_fd = accept(*cfg->server_fd, NULL, NULL);
> +	if (server_fd < 0)
> +		return NULL;
> +
> +	conn = malloc(sizeof(struct connection));
> +	if (conn) {
> +		conn->server_fd = server_fd;
> +		conn->client_fd = client_fd;
> +	}
> +
> +	return conn;
> +}

In connect_client_to_server(), can the file descriptors leak when
allocation fails? If malloc() returns NULL, both client_fd and
server_fd are not closed before returning NULL to the caller.

> +
> +static void disconnect_client_from_server(struct subtest_cfg *cfg,
> +					  struct connection *conn)
> +{
> +	close(conn->server_fd);
> +	close(conn->client_fd);
> +	free(conn);
> +}
> +
> +static int send_and_test_data(struct subtest_cfg *cfg, bool must_succeed)
> +{
> +	struct connection *conn;
> +	int err, res = -1;
> +
> +	conn = connect_client_to_server(cfg);
> +	if (!must_succeed && !ASSERT_EQ(conn, NULL, "connection that must fail"))
> +		goto end;
> +	else if (!must_succeed)
> +		return 0;
> +
> +	if (!ASSERT_NEQ(conn, NULL, "connection that must succeed"))
> +		return 1;
> +
> +	err = send(conn->client_fd, tx_buffer, DEFAULT_TEST_DATA_SIZE, 0);
> +	if (!ASSERT_EQ(err, DEFAULT_TEST_DATA_SIZE, "send data from client"))
> +		goto end;
> +	if (check_server_rx_data(cfg, conn, DEFAULT_TEST_DATA_SIZE))
> +		goto end;
> +
> +	if (!cfg->test_gso) {
> +		res = 0;
> +		goto end;
> +	}
> +
> +	err = send(conn->client_fd, tx_buffer, GSO_TEST_DATA_SIZE, 0);
> +	if (!ASSERT_EQ(err, GSO_TEST_DATA_SIZE, "send (large) data from client"))
> +		goto end;
> +	if (check_server_rx_data(cfg, conn, DEFAULT_TEST_DATA_SIZE))
> +		goto end;
> +
> +	res = 0;
> +end:
> +	disconnect_client_from_server(cfg, conn);
> +	return res;
> +}

In send_and_test_data(), when must_succeed is false and the connection
fails as expected, can this dereference NULL? The path
connect_client_to_server()->returns NULL, ASSERT_EQ(conn, NULL)
succeeds, goto end, then disconnect_client_from_server() is called
with a NULL conn pointer.

[ ... ]

> +static void run_test(struct subtest_cfg *cfg)
> +{
> +	struct nstoken *nstoken = open_netns(CLIENT_NS);
> +
> +	if (!ASSERT_OK(run_server(cfg), "run server"))
> +		goto fail;
> +
> +	/* Basic communication must work */
> +	if (!ASSERT_OK(send_and_test_data(cfg, true), "connect without any encap"))
> +		goto fail;
> +
> +	/* Attach encapsulation program to client, communication must fail */
> +	if (!ASSERT_OK(configure_encapsulation(cfg), "configure encapsulation"))
> +		return;

In run_test(), does this early return leak resources? The server was
started successfully at run_server(), and the network namespace was
opened at the function entry. This return skips both stop_server()
and close_netns() that other error paths reach via the fail label.

> +	if (!ASSERT_OK(send_and_test_data(cfg, false), "connect with encap prog only"))
> +		goto fail;
> +
> +	/* Insert kernel decap module, connection must succeed */
> +	if (!ASSERT_OK(configure_kernel_decapsulation(cfg), "configure kernel decapsulation"))
> +		goto fail;
> +	if (!ASSERT_OK(send_and_test_data(cfg, !cfg->expect_kern_decap_failure),
> +		       "connect with encap prog and kern decap"))
> +		goto fail;
> +
> +	/* Replace kernel module with BPF decap, test must pass */
> +	if (!ASSERT_OK(configure_ebpf_decapsulation(cfg), "configure ebpf decapsulation"))
> +		goto fail;
> +	ASSERT_OK(send_and_test_data(cfg, true), "connect with encap and decap progs");
> +
> +fail:
> +	stop_server(cfg);
> +	close_netns(nstoken);
> +}

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> index b509e4c99..7330c61b5 100644
> --- a/tools/testing/selftests/bpf/progs/test_tc_tunnel.c
> +++ b/tools/testing/selftests/bpf/progs/test_tc_tunnel.c

[ ... ]



AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: selftests/bpf: integrate test_tc_tunnel.sh tests into test_progs
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18709189094

@kernel-patches-daemon-bpf
Copy link
Author

Forwarding comment 3430930886 via email
In-Reply-To: [email protected]
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: e758657
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1014381
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: e758657
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1014381
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: e758657
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1014381
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: e758657
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1014381
version: 2

The test_tunnel.c file defines small fonctions to easily attach eBPF
programs to tc hooks, either on egress, ingress or both.

Create a shared helper in network_helpers.c so that other tests can
benefit from it.

Signed-off-by: Alexis Lothoré (eBPF Foundation) <[email protected]>
…latforms

When trying to run bpf-based encapsulation in a s390x environment, some
parts of test_tc_tunnel.bpf.o do not encapsulate correctly the traffic,
leading to tests failures. Adding some logs shows for example that
packets about to be sent on an interface with the ip6vxlan_eth program
attached do not have the expected value 5 in the ip header ihl field,
and so are ignored by the program.

This phenomenon appears when trying to cross-compile the selftests,
rather than compiling it from a virtualized host: the selftests build
system may then wrongly pick some host headers. If <asm/byteorder.h>
ends up being picked on the host (and if the host has a endianness
different from the target one), it will then expose wrong endianness
defines (e.g __LITTLE_ENDIAN_BITFIELD instead of __BIT_ENDIAN_BITFIELD),
and it will for example mess up the iphdr structure layout used in the
ebpf program.

To prevent this, directly use the vmlinux.h header generated by the
selftests build system rather than including directly specific kernel
headers. As a consequence, add some missing definitions that are not
exposed by vmlinux.h, and adapt the bitfield manipulations to allow
building and using the program on both types of platforms.

Signed-off-by: Alexis Lothoré (eBPF Foundation) <[email protected]>
The test_tc_tunnel.sh script checks that a large variety of tunneling
mechanisms handled by the kernel can be handled as well by eBPF
programs. While this test shares similarities with test_tunnel.c (which
is already integrated in test_progs), those are testing slightly
different things:
- test_tunnel.c creates a tunnel interface, and then get and set tunnel
  keys in packet metadata, from BPF programs.
- test_tc_tunnels.sh manually parses/crafts packets content

Bring the tests covered by test_tc_tunnel.sh into the test_progs
framework, by creating a dedicated test_tc_tunnel.sh. This new test
defines a "generic" runner which, for each test configuration:
- will configure the relevant veth pair, each of those isolated in a
  dedicated namespace
- will check that traffic will fail if there is only an encapsulating
  program attached to one veth egress
- will check that traffic succeed if we enable some decapsulation module
  on kernel side
- will check that traffic still succeeds if we replace the kernel
  decapsulation with some eBPF ingress decapsulation.

Example of the new test execution:

  # ./test_progs -a tc_tunnel
  #447/1   tc_tunnel/ipip_none:OK
  #447/2   tc_tunnel/ipip6_none:OK
  #447/3   tc_tunnel/ip6tnl_none:OK
  #447/4   tc_tunnel/sit_none:OK
  #447/5   tc_tunnel/vxlan_eth:OK
  #447/6   tc_tunnel/ip6vxlan_eth:OK
  #447/7   tc_tunnel/gre_none:OK
  #447/8   tc_tunnel/gre_eth:OK
  #447/9   tc_tunnel/gre_mpls:OK
  #447/10  tc_tunnel/ip6gre_none:OK
  #447/11  tc_tunnel/ip6gre_eth:OK
  #447/12  tc_tunnel/ip6gre_mpls:OK
  #447/13  tc_tunnel/udp_none:OK
  #447/14  tc_tunnel/udp_eth:OK
  #447/15  tc_tunnel/udp_mpls:OK
  #447/16  tc_tunnel/ip6udp_none:OK
  #447/17  tc_tunnel/ip6udp_eth:OK
  #447/18  tc_tunnel/ip6udp_mpls:OK
  #447     tc_tunnel:OK
  Summary: 1/18 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Alexis Lothoré (eBPF Foundation) <[email protected]>
Now that test_tc_tunnel.sh scope has been ported to the test_progs
framework, remove it.

Signed-off-by: Alexis Lothoré (eBPF Foundation) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant