Re: [PATCH mptcp-next v2 05/11] mptcp: add mptcp_subflow_sched bpf_iter

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 07/03/2025 12:36, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Add a new bpt_iter mptcp_subflow_sched, it's a wrapper of mptcp_subflow.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/bpf.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> index 3921261b4fd4..c31f7b407b84 100644
> --- a/net/mptcp/bpf.c
> +++ b/net/mptcp/bpf.c
> @@ -288,6 +288,31 @@ bpf_iter_mptcp_subflow_destroy(struct bpf_iter_mptcp_subflow *it)
>  {
>  }
>  
> +struct bpf_iter_mptcp_subflow_sched {
> +	struct bpf_iter_mptcp_subflow it;
> +};
> +
> +__bpf_kfunc static int
> +bpf_iter_mptcp_subflow_sched_new(struct bpf_iter_mptcp_subflow_sched *it,
> +				 struct sock *sk, struct mptcp_sched_data *data)
> +{
> +	if (!mptcp_sched_check_bpf_iter_task(data))
> +		return -EINVAL;
> +
> +	return bpf_iter_mptcp_subflow_new(&it->it, sk);

We would need the same thing for the BPF PM I suppose, no?

Then, why not having something more generic, either:

 - add "struct task_struct *" in the msk structure

 - or pass a "const struct task_struct *bpf_iter_task" instead of
something sched specific?

For the first idea, I guess it should work because the helper will be
called when the msk lock will be owned by the caller. So we should not
have the PM and scheduler setting it at the same time from different
context, right?
Also, if this helper receives a msk in argument, no need to check that
it is a valid MPTCP socket, etc., right?

If one of these two ideas is used, no need to re-introduce
mptcp_sched_data. The API is not settled yet, so no need to worry about
that. It is even a good time to change it.

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