Re: [PATCH bpf-next v5 7/7] selftests/bpf: Add selftest for XDP_REDIRECT in bpf_prog_run()

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Mon, Jan 03, 2022 at 04:08:12PM +0100, Toke Høiland-Jørgensen wrote:
>> +
>> +#define NUM_PKTS 3
>
> May be send a bit more than 3 packets?
> Just to test skb_list logic for XDP_PASS.

OK, can do.

>> +
>> +	/* We setup a veth pair that we can not only XDP_REDIRECT packets
>> +	 * between, but also route them. The test packet (defined above) has
>> +	 * address information so it will be routed back out the same interface
>> +	 * after it has been received, which will allow it to be picked up by
>> +	 * the XDP program on the destination interface.
>> +	 *
>> +	 * The XDP program we run with bpf_prog_run() will cycle through all
>> +	 * four return codes (DROP/PASS/TX/REDIRECT), so we should end up with
>> +	 * NUM_PKTS - 1 packets seen on the dst iface. We match the packets on
>> +	 * the UDP payload.
>> +	 */
>> +	SYS("ip link add veth_src type veth peer name veth_dst");
>> +	SYS("ip link set dev veth_src address 00:11:22:33:44:55");
>> +	SYS("ip link set dev veth_dst address 66:77:88:99:aa:bb");
>> +	SYS("ip link set dev veth_src up");
>> +	SYS("ip link set dev veth_dst up");
>> +	SYS("ip addr add dev veth_src fc00::1/64");
>> +	SYS("ip addr add dev veth_dst fc00::2/64");
>> +	SYS("ip neigh add fc00::2 dev veth_src lladdr 66:77:88:99:aa:bb");
>> +	SYS("sysctl -w net.ipv6.conf.all.forwarding=1");
>
> These commands pollute current netns. The test has to create its own netns
> like other tests do.

Right, will fix.

> The forwarding=1 is odd. Nothing in the comments or commit logs
> talks about it.

Hmm, yeah, should probably have added an explanation, sorry about that :)

> I'm guessing it's due to patch 6 limitation of picking loopback
> for XDP_PASS and XDP_TX, right?
> There is ingress_ifindex field in struct xdp_md.
> May be use that to setup dev and rxq in test_run in patch 6?
> Then there will be no need to hack through forwarding=1 ?

No, as you note there's already ingress_ifindex to set the device, and
the test does use that:

+	memcpy(skel->rodata->expect_dst, &pkt_udp.eth.h_dest, ETH_ALEN);
+	skel->rodata->ifindex_out = ifindex_src;
+	ctx_in.ingress_ifindex = ifindex_src;

I enable forwarding because the XDP program that counts the packets is
running on the other end of the veth pair (on veth_dst), while the
traffic gen is using veth_src as its ingress ifindex. So for XDP_TX and
XDP_REDIRECT we send the frame back out the veth device, and it ends up
being processed by the XDP program on veth_dst, and counted. But when
the test program returns XDP_PASS, the packet will go up the frame; so
to get it back to the counting program I enable forwarding and set the
packet dst IP so that the stack routes it back out the same interface.

I'll admit this is a bit hacky; I guess I can add a second TC ingress
program that will count the packets being XDP_PASS'ed instead...

-Toke