Re: [PATCH bpf-next 2/5] selftests/bpf: add tc helpers

[Martin KaFai Lau <martin.lau@linux.dev>]

On 10/17/25 7:29 AM, Alexis Lothoré (eBPF Foundation) wrote:
> diff --git a/tools/testing/selftests/bpf/tc_helpers.c b/tools/testing/selftests/bpf/tc_helpers.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..d668e10e3ebad8f8e04862f5c2b3ccd487fe8fa6
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/tc_helpers.c
> @@ -0,0 +1,87 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#define _GNU_SOURCE
> +
> +#include <net/if.h>
> +#include "tc_helpers.h"
> +#include "test_progs.h"
> +
> +static int attach_tc_prog(int ifindex, int igr_fd, int egr_fd)

This one looks good but change it to "int tc_prog_attach(const char 
*dev, int ingress_fd, int egress_fd)". Remove static. Take "const char 
*dev" as the arg. Add it to network_helpers.[ch] instead of creating a 
new source file.

> +{
> +	DECLARE_LIBBPF_OPTS(bpf_tc_hook, hook, .ifindex = ifindex,
> +			    .attach_point = BPF_TC_INGRESS | BPF_TC_EGRESS);
> +	DECLARE_LIBBPF_OPTS(bpf_tc_opts, opts1, .handle = 1,
> +			    .priority = 1, .prog_fd = igr_fd);
> +	DECLARE_LIBBPF_OPTS(bpf_tc_opts, opts2, .handle = 1,
> +			    .priority = 1, .prog_fd = egr_fd);
> +	int ret;
> +
> +	ret = bpf_tc_hook_create(&hook);
> +	if (!ASSERT_OK(ret, "create tc hook"))
> +		return ret;
> +
> +	if (igr_fd >= 0) {
> +		hook.attach_point = BPF_TC_INGRESS;
> +		ret = bpf_tc_attach(&hook, &opts1);
> +		if (!ASSERT_OK(ret, "bpf_tc_attach")) {
> +			bpf_tc_hook_destroy(&hook);
> +			return ret;
> +		}
> +	}
> +
> +	if (egr_fd >= 0) {
> +		hook.attach_point = BPF_TC_EGRESS;
> +		ret = bpf_tc_attach(&hook, &opts2);
> +		if (!ASSERT_OK(ret, "bpf_tc_attach")) {
> +			bpf_tc_hook_destroy(&hook);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +int generic_attach(const char *dev, int igr_fd, int egr_fd)
> +{
> +	int ifindex;
> +
> +	if (!ASSERT_OK_FD(igr_fd, "check ingress fd"))
> +		return -1;
> +	if (!ASSERT_OK_FD(egr_fd, "check egress fd"))
> +		return -1;
> +
> +	ifindex = if_nametoindex(dev);
> +	if (!ASSERT_NEQ(ifindex, 0, "get ifindex"))
> +		return -1;
> +
> +	return attach_tc_prog(ifindex, igr_fd, egr_fd);
> +}
> +
> +int generic_attach_igr(const char *dev, int igr_fd)
> +{
> +	int ifindex;
> +
> +	if (!ASSERT_OK_FD(igr_fd, "check ingress fd"))
> +		return -1;
> +
> +	ifindex = if_nametoindex(dev);
> +	if (!ASSERT_NEQ(ifindex, 0, "get ifindex"))
> +		return -1;
> +
> +	return attach_tc_prog(ifindex, igr_fd, -1);
> +}
> +
> +int generic_attach_egr(const char *dev, int egr_fd)
> +{
> +	int ifindex;
> +
> +	if (!ASSERT_OK_FD(egr_fd, "check egress fd"))
> +		return -1;
> +
> +	ifindex = if_nametoindex(dev);
> +	if (!ASSERT_NEQ(ifindex, 0, "get ifindex"))
> +		return -1;
> +
> +	return attach_tc_prog(ifindex, -1, egr_fd);
> +}

These three generic_attach_* is a bit overkill for network_helpers.c.

Change test_tunnel.c to directly use tc_prog_attach().

> +
> +
> diff --git a/tools/testing/selftests/bpf/tc_helpers.h b/tools/testing/selftests/bpf/tc_helpers.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..d31abe33f9d80dadd8f829bcf9a68cfd744c3b99
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/tc_helpers.h

This new file is not needed also. Use the network_helpers.h.