Re: [PATCH mptcp-next v4 3/5] Squash to "bpf: Register mptcp common kfunc set"

[Matthieu Baerts <matttbe@kernel.org>]

On 09/12/2024 09:47, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Drop bpf_mptcp_sk() and bpf_mptcp_subflow_tcp_sock() definitions. Use
> bpf_skc_to_mptcp_sock() and mptcp_subflow_tcp_sock() in mptcp_subflow
> bpf_iter selftests instead.
> 
> Address Martin's comments in v1:
> 
> - add null-check for bpf_mptcp_subflow_ctx.
> - add KF_RET_NULL flags for bpf_mptcp_subflow_ctx.
> - register this kfunc set to BPF_PROG_TYPE_CGROUP_SOCKOPT only,
>   not BPF_PROG_TYPE_UNSPEC.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/bpf.c | 20 +++++---------------
>  1 file changed, 5 insertions(+), 15 deletions(-)
> 
> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> index a0f49af85d57..f9ba0a46a9f1 100644
> --- a/net/mptcp/bpf.c
> +++ b/net/mptcp/bpf.c
> @@ -218,21 +218,13 @@ struct bpf_iter_mptcp_subflow_kern {
>  
>  __bpf_kfunc_start_defs();
>  
> -__bpf_kfunc static struct mptcp_sock *bpf_mptcp_sk(struct sock *sk)
> -{
> -	return mptcp_sk(sk);
> -}
> -
>  __bpf_kfunc static struct mptcp_subflow_context *
>  bpf_mptcp_subflow_ctx(const struct sock *sk)
>  {
> -	return mptcp_subflow_ctx(sk);
> -}
> +	if (!sk)
> +		return NULL;

Is it enough for the restrictions?

I mean, from what I understood, this helper can be used from any CGROUP
SOCKOPT hooks, with any type of 'struct sock *': then it could be called
from a non MPTCP subflow socket, e.g. a socket with ULP data, no?

Then should we not check this?

  sk->sk_protocol == IPPROTO_TCP && sk_is_mptcp(sk)

(and even sk_fullsock(sk)?)

(...)

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