Re: [PATCH mptcp-next 3/4] mptcp: handle join requests via pernet listen socket

[Florian Westphal <fw@strlen.de>]

Paolo Abeni <pabeni@redhat.com> wrote:
> On Thu, 2022-02-10 at 16:29 +0100, Florian Westphal wrote:
> > +
> > +	/* pernet listener to handle mptcp join requests
> > +	 * based on the mptcp token.
> > +	 *
> > +	 * Has to be pernet because tcp uses
> > +	 * sock_net(sk_listener) to obtain the net namespace for
> > +	 * the syn/ack route lookup.
> > +	 */
> 
> A possible alternative would be proving a __tcp_conn_request() variant
> which uses an exiplicit 'net' argument. tcp_conn_request() could be an
> inline on top of the above.

Yes, we could change tcp stack to allow explicit net arg.

> Or we can use a global per-cpu set of "listeners", setting the net
> field just before calling tcp_conn_request.

I would prefer to avoid that, might get messy at least for RT folks?
If you don't want pernet, then I think additional tcp surgery is better.

> > +	sock_put((struct sock *)msk);
> 
> This should be called under the RCU lock, right? we could have
> __mptcp_token_lookup_sock variant that does not touches the msk
> reference count.

Yes, we should make token api netns safe and then use the 'exist'
variant which takes no reference count.

> > +	msk = mptcp_sk(lsk);
> > +	ssock = __mptcp_nmpc_socket(msk);
> > +	lsk = ssock->sk;
> > +	sock_hold(lsk);
> 
> If I read correctly, at this point 'refcounted' should be 'false' in
> the caller (either tcp_v4_rcv or tcp_v6_rcv), so we don't need to
> acquire a reference to lsk ?!?

Yes, the refcount increase is wrong for sure, its not needed unless
we go for 'skb->sk = lsk' route.