[MPTCP] Re: [RFC 7/7] make accept not allocate kernel socket struct

[Florian Westphal <fw at strlen.de>]

[-- Attachment #1: Type: text/plain, Size: 1621 bytes --]

Paolo Abeni <pabeni(a)redhat.com> wrote:
> On Mon, 2019-11-25 at 03:15 +0100, Florian Westphal wrote:
> > @@ -924,10 +924,17 @@ static void mptcp_close(struct sock *sk, long timeout)
> >  
> >  	list_for_each_entry_safe(subflow, tmp, &msk->conn_list, node) {
> >  		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
> > +		struct socket *sock = READ_ONCE(ssk->sk_socket);
> >  
> >  		pr_debug("conn_list->subflow=%p", subflow);
> >  		list_del(&subflow->node);
> > -		sock_release(ssk->sk_socket);
> > +
> > +		if (sock && sock != sk->sk_socket) {
> > +			sock_release(sock);
> 
> Double checking I read the above correctly: the condition 'sock && sock
> != sk->sk_socket' only for the first subflow of a client socket, right?
> If so, can we use the msk socket even for that one?

No, its also for outgoing connections, we use in kernel sockets for
those.

> > @@ -1443,6 +1470,20 @@ static int mptcp_stream_accept(struct socket *sock, struct socket *newsock,
> >  		return -EINVAL;
> >  
> >  	err = ssock->ops->accept(sock, newsock, flags, kern);
> > +	if (err == 0 && (newsock->sk->sk_prot == &mptcp_prot ||
> > +			 is_mptcp_v6(newsock))) {
> 
> Is not clear to me why/how we can hit the condition '!(newsock->sk-
> >sk_prot == &mptcp_prot || is_mptcp_v6(newsock))' ... Can you please
> explain?

We will hit it when we get connection from non-mptcp peer, i.e. the
!mp_capable part of mptcp_accept().

In that case sk_prot is &tcp_prot.

It might make sense to change this to
    err == 0 && newsock->sk->sk_prot != &tcp_prot

perhaps that would clarify this a bit.  WDYT?