Commit 8e2b8a9f authored by Davide Caratti's avatar Davide Caratti Committed by David S. Miller
Browse files

mptcp: don't overwrite sock_ops in mptcp_is_tcpsk() 

Eric Dumazet suggests:

 > The fact that mptcp_is_tcpsk() was able to write over sock->ops was a
 > bit strange to me.
 > mptcp_is_tcpsk() should answer a question, with a read-only argument.

re-factor code to avoid overwriting sock_ops inside that function. Also,
change the helper name to reflect the semantics and to disambiguate from
its dual, sk_is_mptcp(). While at it, collapse mptcp_stream_accept() and
mptcp_accept() into a single function, where fallback / non-fallback are
separated into a single sk_is_mptcp() conditional.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/432


Suggested-by: default avatarEric Dumazet <edumazet@google.com>
Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
Acked-by: default avatarPaolo Abeni <pabeni@redhat.com>
Signed-off-by: default avatarMatthieu Baerts <matttbe@kernel.org>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 7961ef1f

net/mptcp/protocol.c

+44 −64
Original line number Diff line number Diff line
@@ -55,28 +55,14 @@ static u64 mptcp_wnd_end(const struct mptcp_sock *msk) 
	return READ_ONCE(msk->wnd_end);

}



static bool mptcp_is_tcpsk(struct sock *sk)

static const struct proto_ops *mptcp_fallback_tcp_ops(const struct sock *sk)

{

	struct socket *sock = sk->sk_socket;



	if (unlikely(sk->sk_prot == &tcp_prot)) {

		/* we are being invoked after mptcp_accept() has

		 * accepted a non-mp-capable flow: sk is a tcp_sk,

		 * not an mptcp one.

		 *

		 * Hand the socket over to tcp so all further socket ops

		 * bypass mptcp.

		 */

		WRITE_ONCE(sock->ops, &inet_stream_ops);

		return true;

#if IS_ENABLED(CONFIG_MPTCP_IPV6)

	} else if (unlikely(sk->sk_prot == &tcpv6_prot)) {

		WRITE_ONCE(sock->ops, &inet6_stream_ops);

		return true;

	if (sk->sk_prot == &tcpv6_prot)

		return &inet6_stream_ops;

#endif

	}



	return false;

	WARN_ON_ONCE(sk->sk_prot != &tcp_prot);

	return &inet_stream_ops;

}



static int __mptcp_socket_create(struct mptcp_sock *msk)
@@ -3258,44 +3244,6 @@ void mptcp_rcv_space_init(struct mptcp_sock *msk, const struct sock *ssk) 
	WRITE_ONCE(msk->wnd_end, msk->snd_nxt + tcp_sk(ssk)->snd_wnd);

}



static struct sock *mptcp_accept(struct sock *ssk, int flags, int *err,

				 bool kern)

{

	struct sock *newsk;



	pr_debug("ssk=%p, listener=%p", ssk, mptcp_subflow_ctx(ssk));

	newsk = inet_csk_accept(ssk, flags, err, kern);

	if (!newsk)

		return NULL;



	pr_debug("newsk=%p, subflow is mptcp=%d", newsk, sk_is_mptcp(newsk));

	if (sk_is_mptcp(newsk)) {

		struct mptcp_subflow_context *subflow;

		struct sock *new_mptcp_sock;



		subflow = mptcp_subflow_ctx(newsk);

		new_mptcp_sock = subflow->conn;



		/* is_mptcp should be false if subflow->conn is missing, see

		 * subflow_syn_recv_sock()

		 */

		if (WARN_ON_ONCE(!new_mptcp_sock)) {

			tcp_sk(newsk)->is_mptcp = 0;

			goto out;

		}



		newsk = new_mptcp_sock;

		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_MPCAPABLEPASSIVEACK);

	} else {

		MPTCP_INC_STATS(sock_net(ssk),

				MPTCP_MIB_MPCAPABLEPASSIVEFALLBACK);

	}



out:

	newsk->sk_kern_sock = kern;

	return newsk;

}



void mptcp_destroy_common(struct mptcp_sock *msk, unsigned int flags)

{

	struct mptcp_subflow_context *subflow, *tmp;
@@ -3739,7 +3687,6 @@ static struct proto mptcp_prot = { 
	.connect	= mptcp_connect,

	.disconnect	= mptcp_disconnect,

	.close		= mptcp_close,

	.accept		= mptcp_accept,

	.setsockopt	= mptcp_setsockopt,

	.getsockopt	= mptcp_getsockopt,

	.shutdown	= mptcp_shutdown,
@@ -3849,18 +3796,36 @@ static int mptcp_stream_accept(struct socket *sock, struct socket *newsock, 
	if (!ssk)

		return -EINVAL;



	newsk = mptcp_accept(ssk, flags, &err, kern);

	pr_debug("ssk=%p, listener=%p", ssk, mptcp_subflow_ctx(ssk));

	newsk = inet_csk_accept(ssk, flags, &err, kern);

	if (!newsk)

		return err;



	lock_sock(newsk);

	pr_debug("newsk=%p, subflow is mptcp=%d", newsk, sk_is_mptcp(newsk));

	if (sk_is_mptcp(newsk)) {

		struct mptcp_subflow_context *subflow;

		struct sock *new_mptcp_sock;



		subflow = mptcp_subflow_ctx(newsk);

		new_mptcp_sock = subflow->conn;



		/* is_mptcp should be false if subflow->conn is missing, see

		 * subflow_syn_recv_sock()

		 */

		if (WARN_ON_ONCE(!new_mptcp_sock)) {

			tcp_sk(newsk)->is_mptcp = 0;

			goto tcpfallback;

		}



		newsk = new_mptcp_sock;

		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_MPCAPABLEPASSIVEACK);



		newsk->sk_kern_sock = kern;

		lock_sock(newsk);

		__inet_accept(sock, newsock, newsk);

	if (!mptcp_is_tcpsk(newsock->sk)) {

		struct mptcp_sock *msk = mptcp_sk(newsk);

		struct mptcp_subflow_context *subflow;



		set_bit(SOCK_CUSTOM_SOCKOPT, &newsock->flags);

		msk = mptcp_sk(newsk);

		msk->in_accept_queue = 0;



		/* set ssk->sk_socket of accept()ed flows to mptcp socket.
@@ -3882,6 +3847,21 @@ static int mptcp_stream_accept(struct socket *sock, struct socket *newsock, 
			if (unlikely(list_is_singular(&msk->conn_list)))

				inet_sk_state_store(newsk, TCP_CLOSE);

		}

	} else {

		MPTCP_INC_STATS(sock_net(ssk),

				MPTCP_MIB_MPCAPABLEPASSIVEFALLBACK);

tcpfallback:

		newsk->sk_kern_sock = kern;

		lock_sock(newsk);

		__inet_accept(sock, newsock, newsk);

		/* we are being invoked after accepting a non-mp-capable

		 * flow: sk is a tcp_sk, not an mptcp one.

		 *

		 * Hand the socket over to tcp so all further socket ops

		 * bypass mptcp.

		 */

		WRITE_ONCE(newsock->sk->sk_socket->ops,

			   mptcp_fallback_tcp_ops(newsock->sk));

	}

	release_sock(newsk);