Re: [PATCH 2/3] selftests/bpf: Migrate test_xdp_redirect.sh to xdp_do_redirect.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Alexis,

On 1/3/25 1:54 PM, Alexis Lothoré wrote:
Hi Bastien,

On 1/3/25 11:10, Bastien Curutchet (eBPF Foundation) wrote:

[...]

+		SYS(fail, "ip link add veth%d index %d%d%d type veth peer name veth0 netns %s",
+		    i, i, i, i, ns_name);

nit: since you have to run an ip command through SYS anyway, you can reduce the
open ns/run command/close ns dance (and all the resulting error checks) by
running directly `SYS("ip netns exec %s ip link add [...]", NS0, [...])`

[...]


True, thanks.

+	ret = bpf_xdp_attach(if_nametoindex("veth2"),
+			     bpf_program__fd(prog_to_111),
+			     data->xdp_flags, NULL);

nit: since we are setting static if index at veth creation (which looks needed
for this test), the if_nametoindex could be replaced by the corresponding index,
which could be directly a define


Also true, thanks.

+	if (!ASSERT_GE(ret, 0, "bpf_xdp_attach"))
+		goto close;
+
+	ret = bpf_xdp_attach(if_nametoindex("veth1"),
+			     bpf_program__fd(prog_to_222),
+			     data->xdp_flags, NULL);
+	if (!ASSERT_GE(ret, 0, "bpf_xdp_attach"))
+		goto close;
+
+	close_netns(nstoken);
+
+	nstoken = open_netns(NS1);
+	if (!ASSERT_OK_PTR(nstoken, "open NS1"))
+		goto close;
+
+	SYS(close, "ping -c 1 %s.2", IPV4_NETWORK);
+
+	close_netns(nstoken);
+
+	nstoken = open_netns(NS2);
+	if (!ASSERT_OK_PTR(nstoken, "open NS2"))
+		goto close;
+
+	SYS(close, "ping -c 1 %s.1", IPV4_NETWORK);

Is it really useful to check ping originating from both interfaces, isn´t a
single ping able to stimulate programs attached to both veth0 ?


Indeed, I think a single ping would be enough, I just wanted to stick with what test_xdp_redirect.sh does.

Aside from those minor points, LGTM :)


Best regards,
Bastien




[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux