Re: [PATCH bpf-next 2/2] selftests/bpf: Migrate test_xdp_meta.sh into xdp_context_test_run.c

[Martin KaFai Lau <martin.lau@xxxxxxxxx>]

On 12/6/24 12:12 AM, Bastien Curutchet wrote:
> +void test_xdp_context_functional(void)
> +{
> +	LIBBPF_OPTS(bpf_tc_hook, tc_hook, .attach_point = BPF_TC_INGRESS);
> +	LIBBPF_OPTS(bpf_tc_opts, tc_opts, .handle = 1, .priority = 1);
> +	struct bpf_program *tc_prog, *xdp_prog;
> +	struct netns_obj *rx_ns, *tx_ns;
> +	struct test_xdp_meta *skel;
> +	struct nstoken *nstoken;
> +	int rx_ifindex;
> +	int ret;
> +
> +	tx_ns = netns_new(TX_NETNS, false);
> +	if (!ASSERT_OK_PTR(tx_ns, "create tx_ns"))
> +		return;
> +
> +	rx_ns = netns_new(RX_NETNS, false);
> +	if (!ASSERT_OK_PTR(rx_ns, "create rx_ns"))
> +		goto free_txns;
> +
> +	SYS(free_rxns, "ip link add " RX_NAME " netns " RX_NETNS
> +	    " type veth peer name " TX_NAME " netns " TX_NETNS);
> +
> +	nstoken = open_netns(RX_NETNS);

close_netns(nstoken) is needed.

> +	if (!ASSERT_OK_PTR(nstoken, "setns rx_ns"))
> +		goto free_rxns;
> +
> +	SYS(free_rxns, "ip addr add " RX_ADDR "/24 dev " RX_NAME);
> +	SYS(free_rxns, "ip link set dev " RX_NAME " up");
> +
> +	skel = test_xdp_meta__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "open and load skeleton"))
> +		goto free_rxns;
> +
> +	rx_ifindex = if_nametoindex(RX_NAME);
> +	if (!ASSERT_GE(rx_ifindex, 0, "if_nametoindex rx"))
> +		goto destroy_skel;
> +
> +	tc_hook.ifindex = rx_ifindex;
> +	ret = bpf_tc_hook_create(&tc_hook);
> +	if (!ASSERT_OK(ret, "bpf_tc_hook_create"))
> +		goto destroy_skel;
> +
> +	tc_prog = bpf_object__find_program_by_name(skel->obj, "ing_cls");
> +	if (!ASSERT_OK_PTR(tc_prog, "open ing_cls prog"))
> +		goto destroy_skel;
> +
> +	tc_opts.prog_fd = bpf_program__fd(tc_prog);
> +	ret = bpf_tc_attach(&tc_hook, &tc_opts);
> +	if (!ASSERT_OK(ret, "bpf_tc_attach"))
> +		goto destroy_skel;
> +
> +	xdp_prog = bpf_object__find_program_by_name(skel->obj, "ing_xdp");
> +	if (!ASSERT_OK_PTR(xdp_prog, "open ing_xdp prog"))
> +		goto destroy_skel;
> +
> +	ret = bpf_xdp_attach(rx_ifindex,
> +			     bpf_program__fd(xdp_prog),
> +			     0, NULL);
> +	if (!ASSERT_GE(ret, 0, "bpf_xdp_attach"))
> +		goto destroy_skel;
> +
> +	nstoken = open_netns(TX_NETNS);

Same here.

pw-bot: cr

> +	if (!ASSERT_OK_PTR(nstoken, "setns tx_ns"))
> +		goto destroy_skel;
> +
> +	SYS(destroy_skel, "ip addr add " TX_ADDR "/24 dev " TX_NAME);
> +	SYS(destroy_skel, "ip link set dev " TX_NAME " up");
> +	SYS(destroy_skel, "ping -c 1 " RX_ADDR);
> +
> +destroy_skel:
> +	test_xdp_meta__destroy(skel);
> +free_rxns:
> +	netns_free(rx_ns);
> +free_txns:

nit. test_xdp_meta__destroy, netns_free, and the to-be-added close_netns can 
handle NULL. Init the variables to NULL at the beginning could save a few goto 
labels, probably only one label is needed.

> +	netns_free(tx_ns);
> +}