Re: [PATCH mptcp-next v3 3/5] Squash to "bpf: Add mptcp_subflow bpf_iter"

[Geliang Tang <geliang@kernel.org>]

Hi Mat,

Thanks for the review.

On Thu, 2025-02-20 at 19:25 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 17/02/2025 11:34, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > Drop the NULL check for 'msk' as Martin suggested, add more checks
> > for 'sk'.
> > 
> > Use the "struct sock *sk" instead of "struct mptcp-sock *msk" as
> > the
> > argument in the bpf_iter_mptcp_subflow_new as Martin suggested.
> > 
> > v3:
> >  - continue to use sock_owned_by_user_nocheck and spin_is_locked
> > checks instead of using msk_owned_by_me().
> 
> Why should we continue to do so?
> 
> From what I understood, with the current version, this new
> bpf_iter_mptcp_subflow_new kfunc can only be called from a cg sockopt
> bpf prog, which means these checks are not needed for the moment, no?

Thanks for the clarification. I re-read Martin's comments and he also
suggested removing these checks. So I removed it in v4.

> 
> So maybe we should only add these checks later, when extending its
> usage
> to struct_ops? In the meantime, msk_owned_by_me() could then be used
> instead?

Do I need to add a patch into the "use bpf_iter in bpf schedulers" set
that is under review to do these checks? Do you mean that using
msk_owned_by_me() is enough for struct_ops, and there is no need to use
sock_owned_by_user_nocheck() and spin_is_locked() checks?

Thanks,
-Geliang

> 
> Cheers,
> Matt