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

[Geliang Tang <geliang@kernel.org>]

Hi Matt,

On Tue, 2024-12-10 at 12:48 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 09/12/2024 09:47, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > Add "sizeof" and "alignof" checks.
> > 
> > Address Martin's comments in v1:
> > 
> > - bpf_iter_mptcp_subflow_new returns -EINVAL when msk socket lock
> > isn't
> >   held.
> > 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> >  net/mptcp/bpf.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> > index f9ba0a46a9f1..0563d3c6d9d3 100644
> > --- a/net/mptcp/bpf.c
> > +++ b/net/mptcp/bpf.c
> > @@ -232,12 +232,20 @@ bpf_iter_mptcp_subflow_new(struct
> > bpf_iter_mptcp_subflow *it,
> >  			   struct mptcp_sock *msk)
> >  {
> >  	struct bpf_iter_mptcp_subflow_kern *kit = (void *)it;
> > +	struct sock *sk = (struct sock *)msk;
> > +
> > +	BUILD_BUG_ON(sizeof(struct bpf_iter_mptcp_subflow_kern) >
> > +		     sizeof(struct bpf_iter_mptcp_subflow));
> > +	BUILD_BUG_ON(__alignof__(struct
> > bpf_iter_mptcp_subflow_kern) !=
> > +		     __alignof__(struct bpf_iter_mptcp_subflow));
> >  
> >  	kit->msk = msk;
> >  	if (!msk)
> >  		return -EINVAL;
> >  
> > -	msk_owned_by_me(msk);
> > +	if (!sock_owned_by_user_nocheck(sk) &&
> > +	    !spin_is_locked(&sk->sk_lock.slock))
> > +		return -EINVAL;
> 
> Just to be sure, did you check it was always the case with the WIP
> scheduler and path-manager BPF programs you have?

Yes, I checked, BPF packet scheduler and BPF path manager are working
fine. Thanks for your other suggestions on this set, I have updated to
the new version v5.

-Geliang

> 
> We don't want to be too restrictive. (Or we need different helpers)
> 
> Cheers,
> Matt