Re: [PATCH mptcp-next v8 0/8] BPF packet scheduler

From: Matthieu Baerts <matthieu.baerts@tessares.net>
To: Geliang Tang <geliang.tang@suse.com>,
	Mat Martineau <mathew.j.martineau@linux.intel.com>
Cc: mptcp@lists.linux.dev
Subject: Re: [PATCH mptcp-next v8 0/8] BPF packet scheduler
Date: Wed, 30 Mar 2022 16:57:56 +0200	[thread overview]
Message-ID: <74bc5fda-ab48-6bfa-dbae-a5783f9a327d@tessares.net> (raw)
In-Reply-To: <20220330141211.GA345@localhost>

Hi Geliang,

On 30/03/2022 16:12, Geliang Tang wrote:
> Hi Mat & Matt,
> 
> On Tue, Mar 29, 2022 at 04:13:02PM -0700, Mat Martineau wrote:
>>
>> On Tue, 29 Mar 2022, Geliang Tang wrote:
>>
>>> v8:
>>> - use global sched_list instead of pernet sched_list.
>>
>> Yes, I think this is fine. I had initially asked about pernet configuration
>> with respect to registering BPF schedulers, but the important thing is that
>> the sysctl can be set per-namespace.
>>
>>> - drop synchronize_rcu() in mptcp_unregister_scheduler().
>>> - update mptcp_init_sched and mptcp_release_sched as Mat and Florian
>>>   suggested.
>>> - fix the build break in patch 8.
>>> - depends on: "add skc_to_mptcp_sock" v14.
>>> - export/20220325T055307
>>
>> Thanks for updating. Builds and runs fine here.
>>
>> Before we add this to the export branch, have you thought about how subflow
>> data (including the backup bit and throughput/latency data used by the
>> default scheduler) can be accessed in the BPF get_subflow hook? Do you think
>> that can be cleanly added after this series, or is there anything that may
>> need to be changed in this series?
> 
> I plan to do that in the next series, BPF round-robin scheduler.
> 
> I had implemented round-robin in kernel before:
> 
> https://patchwork.kernel.org/project/mptcp/cover/cover.1631011068.git.geliangtang@xiaomi.com/
> 
> This time I plan to implement it using BPF.

Sounds like a good idea to exercise his new BPF packet scheduler
infrastructure, thanks!

> In order to support bpf_rr, I think we need to do two more things
> based on this series:
> 
> 1. Get the subflow data from BPF, maybe iterate over the subflows
> from mptcp_sock.

Yes, sounds good to me. What is blocking to do that?
Maybe some new BPF helpers will be needed?

> 2. Make same members of struct mptcp_sock writable in BPF, say
> last_snd, then we can set it like this:
> 
>         msk->last_snd = ssk.

I don't think this is a direction to take with BPF. I mean: not
directly. You can have helper doing that with extra verifications. How
is it done with TCP CA?

Or maybe for this specific case here with RR, you can also store the
last snd subflow in a MAP storage, no?

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net