Skip to content

Commit

Permalink
mptcp: avoid blocking in tcp_sendpages
Browse files Browse the repository at this point in the history
The transmit loop continues to xmit new data until an
error is returned or all data was transmitted.

For the blocking i/o case, this means that tcp_sendpages() may
block on the subflow until more space becomes available, i.e. we end up
sleeping with the mptcp socket lock held.

Instead we should check if a different subflow is ready to be used.

This restarts the subflow sk lookup when the tx operation succeeded
and the tcp subflow can't accept more data or if tcp_sendpages
indicates -EAGAIN on a blocking mptcp socket.

In case all subflows are busy, the existing logic will wait until
a subflow is ready, releasing the mptcp level socket while doing so.

The mptcp worker already sets DONTWAIT, so no need to make
changes there.

Reported-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
  • Loading branch information
Florian Westphal authored and jenkins-tessares committed May 8, 2020
1 parent 636ef28 commit 61a9f18
Showing 1 changed file with 20 additions and 3 deletions.
23 changes: 20 additions & 3 deletions net/mptcp/protocol.c
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
* access the skb after the sendpages call
*/
ret = do_tcp_sendpages(ssk, page, offset, psize,
msg->msg_flags | MSG_SENDPAGE_NOTLAST);
msg->msg_flags | MSG_SENDPAGE_NOTLAST | MSG_DONTWAIT);
if (ret <= 0)
return ret;

Expand Down Expand Up @@ -714,6 +714,7 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
struct socket *ssock;
size_t copied = 0;
struct sock *ssk;
bool tx_ok;
long timeo;

if (msg->msg_flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL))
Expand All @@ -738,6 +739,7 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
return ret >= 0 ? ret + copied : (copied ? copied : ret);
}

restart:
mptcp_clean_una(sk);

__mptcp_flush_join_list(msk);
Expand All @@ -759,11 +761,17 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
pr_debug("conn_list->subflow=%p", ssk);

lock_sock(ssk);
while (msg_data_left(msg)) {
tx_ok = msg_data_left(msg);
while (tx_ok) {
ret = mptcp_sendmsg_frag(sk, ssk, msg, NULL, &timeo, &mss_now,
&size_goal);
if (ret < 0)
if (ret < 0) {
if (ret == -EAGAIN && timeo > 0) {
release_sock(ssk);
goto restart;
}
break;
}
if (ret == 0 && unlikely(__mptcp_needs_tcp_fallback(msk))) {
/* Can happen for passive sockets:
* 3WHS negotiated MPTCP, but first packet after is
Expand All @@ -777,6 +785,15 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
}

copied += ret;
tx_ok = msg_data_left(msg);
if (!tx_ok)
break;
if (!sk_stream_memory_free(ssk)) {
tcp_push(ssk, msg->msg_flags, mss_now,
tcp_sk(ssk)->nonagle, size_goal);
release_sock(ssk);
goto restart;
}
}

mptcp_set_timeout(sk, ssk);
Expand Down

0 comments on commit 61a9f18

Please sign in to comment.