netdev - Re: [PATCH bpf-next 3/4] selftests/bpf: Add mptcp subflow example

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <843ea6eb-a28d-437c-9c98-0b8c8816c518@kernel.org>
Date: Tue, 7 May 2024 18:03:30 +0200
From: Matthieu Baerts <matttbe@...nel.org>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: MPTCP Upstream <mptcp@...ts.linux.dev>,
 Mat Martineau <martineau@...nel.org>, Geliang Tang <geliang@...nel.org>,
 Andrii Nakryiko <andrii@...nel.org>, Eduard Zingerman <eddyz87@...il.com>,
 Mykola Lysenko <mykolal@...com>, Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>,
 Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>,
 Yonghong Song <yonghong.song@...ux.dev>,
 John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>,
 Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>,
 Jiri Olsa <jolsa@...nel.org>, Shuah Khan <shuah@...nel.org>,
 LKML <linux-kernel@...r.kernel.org>,
 Network Development <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
 "open list:KERNEL SELFTEST FRAMEWORK" <linux-kselftest@...r.kernel.org>,
 Geliang Tang <tanggeliang@...inos.cn>
Subject: Re: [PATCH bpf-next 3/4] selftests/bpf: Add mptcp subflow example

Hi Alexei,

Thank you for the review!

On 07/05/2024 16:49, Alexei Starovoitov wrote:
> On Tue, May 7, 2024 at 3:53 AM Matthieu Baerts (NGI0)
> <matttbe@...nel.org> wrote:
>>
>> From: Nicolas Rybowski <nicolas.rybowski@...sares.net>
>>
>> Move Nicolas's patch into bpf selftests directory. This example added a
>> test that was adding a different mark (SO_MARK) on each subflow, and
>> changing the TCP CC only on the first subflow.
>>
>> This example shows how it is possible to:
>>
>>     Identify the parent msk of an MPTCP subflow.
>>     Put different sockopt for each subflow of a same MPTCP connection.
>>
>> Here especially, we implemented two different behaviours:
>>
>>     A socket mark (SOL_SOCKET SO_MARK) is put on each subflow of a same
>>     MPTCP connection. The order of creation of the current subflow defines
>>     its mark.
> 
>> The TCP CC algorithm of the very first subflow of an MPTCP
>>     connection is set to "reno".
> 
> why?
> What does it test?
> That bpf_setsockopt() can actually do it?

Correct.

Here is a bit of context: from the userspace, an application can do a
setsockopt() on an MPTCP socket, and typically the same value will be
set on all subflows (paths). If someone wants to have different values
per subflow, the recommanded way is to use BPF.

We can indeed restrict this test to changing the MARK only. I think the
CC has been modified just not to check one thing, but also to change
something at the TCP level, because it is managed differently on MPTCP
side -- but only when the userspace set something, or when new subflows
are created. The result of this operation is easy to check with 'ss',
and it was to show an exemple where this is set only on one subflow.

> But the next patch doesn't check that it's reno.

No, I think it is checked: 'reno' is not hardcoded, but 'skel->data->cc'
is used instead:

  run_subflow(skel->data->cc);

> It looks to me that dropping this "set to reno" part
> won't change the purpose of the rest of selftest.

Yes, up to you. If you still think it is better without it, we can
remove the modification of the CC in patch 3/4, and the validation in
patch 4/4.

> pw-bot: cr

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.