Re: [PATCH mptcp-next v5 2/5] Squash to "bpf: Extend bpf_skc_to_mptcp_sock to MPTCP sock"

[Mat Martineau <martineau@kernel.org>]

On Mon, 3 Mar 2025, Geliang Tang wrote:

> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Set msk->bpf_iter_task in bpf_mptcp_sock_from_sock() to allow
> mptcp_subflow bpt_iter can be used in cgroup/getsockopt,
> otherwise, the selftest in this set fails.
>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> net/mptcp/bpf.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> index be222fa5f308..aff92f458e7f 100644
> --- a/net/mptcp/bpf.c
> +++ b/net/mptcp/bpf.c
> @@ -197,14 +197,22 @@ static struct bpf_struct_ops bpf_mptcp_sched_ops = {
>
> struct mptcp_sock *bpf_mptcp_sock_from_sock(struct sock *sk)
> {
> +	struct mptcp_sock *msk;
> +
> 	if (unlikely(!sk || !sk_fullsock(sk)))
> 		return NULL;
>
> -	if (sk->sk_protocol == IPPROTO_MPTCP)
> -		return mptcp_sk(sk);
> +	if (sk->sk_protocol == IPPROTO_MPTCP) {
> +		msk = mptcp_sk(sk);
> +		mptcp_set_bpf_iter_task(msk);

Hi Geliang -

The important part of the bpf_iter_task approach is to only set the 
task_struct pointer when the msk lock is already held, and to clear it 
before the msk lock is released. When used this way, the check ensures 
that the iterator code is both *running with the socket locked* and *in 
the same context where the lock is held*.

The code above will set msk->bpf_iter_task even when the lock is not held, 
and then it is never cleared.

To set/clear as expected for get/setsockopt I think the task pointer would 
have to be set in places where the locks are acquired and released, like 
these:

https://elixir.bootlin.com/linux/v6.14-rc5/source/kernel/bpf/cgroup.c#L1955
https://elixir.bootlin.com/linux/v6.14-rc5/source/kernel/bpf/cgroup.c#L1846


- Mat

> +		return msk;
> +	}
>
> -	if (sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk))
> -		return mptcp_sk(mptcp_subflow_ctx(sk)->conn);
> +	if (sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)) {
> +		msk = mptcp_sk(mptcp_subflow_ctx(sk)->conn);
> +		mptcp_set_bpf_iter_task(msk);
> +		return msk;
> +	}
>
> 	return NULL;
> }
> -- 
> 2.43.0
>
>
>