Re: [PATCH bpf-next v4 3/3] selftests/bpf: xdp: test dev_xdp_install() rejects device-bound program

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 4050 bytes --]

> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_dev_bound_only.c b/tools/testing/selftests/bpf/prog_tests/xdp_dev_bound_only.c
> index 7dd18c6d06c6..4a13f8ec4300 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_dev_bound_only.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_dev_bound_only.c
> @@ -1,9 +1,11 @@
>  // SPDX-License-Identifier: GPL-2.0
>  #include <net/if.h>
> +#include <linux/if_link.h>
>  #include <test_progs.h>
>  #include <network_helpers.h>
>
>  #define LOCAL_NETNS "xdp_dev_bound_only_netns"
> +#define LINK_UPDATE_NETNS "xdp_dev_bound_only_lu_netns"
>
>  static int load_dummy_prog(char *name, __u32 ifindex, __u32 flags)
>  {
> @@ -59,3 +61,50 @@ void test_xdp_dev_bound_only_offdev(void)
>  	 */
>  	SYS_NOFAIL("ip netns del " LOCAL_NETNS);
>  }
> +
> +/* A device-bound program must not run on the XDP software path.
> + * dev_xdp_attach() rejected such programs, but bpf_xdp_link_update() reaches
> + * dev_xdp_install() directly and bypasses it, so the check has to live in
> + * dev_xdp_install(). Create a generic (SKB) XDP link with a normal program,
> + * then try to swap in a device-bound program via BPF_LINK_UPDATE.
> + */
> +void test_xdp_dev_bound_only_link_update(void)
> +{
> +	LIBBPF_OPTS(bpf_link_create_opts, lopts, .flags = XDP_FLAGS_SKB_MODE);
> +	int base_fd = -1, devbound_fd = -1, link_fd = -1;
> +	struct nstoken *tok = NULL;
> +	__u32 ifindex;
> +	int err;
> +
> +	SYS(out, "ip netns add " LINK_UPDATE_NETNS);
> +	tok = open_netns(LINK_UPDATE_NETNS);
> +	if (!ASSERT_OK_PTR(tok, "open_netns"))
> +		goto out;
> +
> +	SYS(out, "ip link add eth42 type veth");

A subsystem pattern flags this as potentially concerning: the new test
hand-rolls the network-namespace lifecycle with SYS("ip netns add") plus
open_netns() here and close_netns()/SYS_NOFAIL("ip netns del") at the
teardown, but the BPF selftest framework already provides this as a
single pair netns_new(name, true) and netns_free(netns), declared at
tools/testing/selftests/bpf/test_progs.h and implemented at
tools/testing/selftests/bpf/test_progs.c.

The shared version additionally brings lo up and wires up traffic-monitor
start/stop for -m runs, neither of which the hand-rolled sequence does.
Both LINK_UPDATE_NETNS and the paired add/del strings are also duplicated
state that netns_new() would own.

The new function is a deliberate copy of the sibling
test_xdp_dev_bound_only_offdev() at lines 27-63 of the same file, which
uses exactly the same SYS/open_netns/close_netns/SYS_NOFAIL sequence,
and 28 other files under prog_tests/ create namespaces the same way
rather than via netns_new(). Keeping the two functions in one file
structurally symmetric has clear readability value.

The functional deltas (lo not brought up, no traffic monitor) do not
affect this test, which never sends traffic. Would you prefer
netns_new()/netns_free() here for consistency with the framework helpers,
or is the structural symmetry with the sibling function the better choice?

> +	ifindex = if_nametoindex("eth42");
> +	if (!ASSERT_NEQ(ifindex, 0, "if_nametoindex"))
> +		goto out;
> +
> +	devbound_fd = load_dummy_prog("devbound", ifindex, BPF_F_XDP_DEV_BOUND_ONLY);
> +	if (!ASSERT_GE(devbound_fd, 0, "load_dummy_prog devbound"))
> +		goto out;
> +
> +	base_fd = load_dummy_prog("base", 0, 0);
> +	if (!ASSERT_GE(base_fd, 0, "load_dummy_prog base"))
> +		goto out;
> +
> +	link_fd = bpf_link_create(base_fd, ifindex, BPF_XDP, &lopts);
> +	if (!ASSERT_GE(link_fd, 0, "bpf_link_create"))
> +		goto out;
> +
> +	err = bpf_link_update(link_fd, devbound_fd, NULL);
> +	ASSERT_EQ(err, -EINVAL, "link_update device-bound rejected");
> +
> +out:
> +	close(link_fd);
> +	close(base_fd);
> +	close(devbound_fd);
> +	close_netns(tok);
> +	SYS_NOFAIL("ip netns del " LINK_UPDATE_NETNS);
> +}

---
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

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31358779411