netdev - Re: [PATCH 2/3] selftests/bpf: Migrate test_xdp_redirect.sh to xdp_do

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9a18fec7-8f73-48c6-81d4-5baa5b701f76@bootlin.com>
Date: Mon, 6 Jan 2025 08:54:18 +0100
From: Bastien Curutchet <bastien.curutchet@...tlin.com>
To: Alexis Lothoré <alexis.lothore@...tlin.com>,
 Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
 "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
 Jesper Dangaard Brouer <hawk@...nel.org>,
 John Fastabend <john.fastabend@...il.com>,
 Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau
 <martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>,
 Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>,
 KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
 Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
 Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>
Cc: Thomas Petazzoni <thomas.petazzoni@...tlin.com>, netdev@...r.kernel.org,
 bpf@...r.kernel.org, linux-kselftest@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] selftests/bpf: Migrate test_xdp_redirect.sh to
 xdp_do_redirect.c

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