| Message ID | 20200821153824.27468-1-fw@strlen.de |
|---|---|
| State | Accepted, archived |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | [mptcp-next,1/1] mptcp: fix stale subflow->writeable caching | expand |
On Fri, 21 Aug 2020, Florian Westphal wrote: > Even with fixed subflow_write_space(), we may still indicate EPOLLOUT > even if no subflow is writeable. > > We need to re-check the last ssk that was used after the final tcp_push(). > tcp_push() can allocate new skbs that get charged to the subflow sk > which may bring it above the wmem limit. > > Signed-off-by: Florian Westphal <fw@strlen.de> > --- > net/mptcp/protocol.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c > index ad91f4588216..4dd5d35a8f39 100644 > --- a/net/mptcp/protocol.c > +++ b/net/mptcp/protocol.c > @@ -1239,7 +1239,7 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len) > msk->snd_burst -= ret; > copied += ret; > > - if (!sk_stream_memory_free(ssk)) > + if (!sk_stream_is_writeable(ssk)) > WRITE_ONCE(subflow->writable, false); > > tx_ok = msg_data_left(msg); > @@ -1294,6 +1294,9 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len) > /* start the timer, if it's not pending */ > if (!mptcp_timer_pending(sk)) > mptcp_reset_timer(sk); > + > + if (!sk_stream_is_writeable(ssk)) > + WRITE_ONCE(mptcp_subflow_ctx(ssk)->writable, false); > } > > release_sock(ssk); > -- > 2.26.2 Thanks Florian. All three patches look good to me. -- Mat Martineau Intel
Hi Florian, Mat, On 21/08/2020 21:10, Mat Martineau wrote: > On Fri, 21 Aug 2020, Florian Westphal wrote: > >> Even with fixed subflow_write_space(), we may still indicate EPOLLOUT >> even if no subflow is writeable. >> >> We need to re-check the last ssk that was used after the final >> tcp_push(). >> tcp_push() can allocate new skbs that get charged to the subflow sk >> which may bring it above the wmem limit. >> >> Signed-off-by: Florian Westphal <fw@strlen.de> > > Thanks Florian. All three patches look good to me. Thank you for the patch and the review! Applied at the end of the series. Tests + export are in progress! Cheers, Matt
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index ad91f4588216..4dd5d35a8f39 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -1239,7 +1239,7 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len) msk->snd_burst -= ret; copied += ret; - if (!sk_stream_memory_free(ssk)) + if (!sk_stream_is_writeable(ssk)) WRITE_ONCE(subflow->writable, false); tx_ok = msg_data_left(msg); @@ -1294,6 +1294,9 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len) /* start the timer, if it's not pending */ if (!mptcp_timer_pending(sk)) mptcp_reset_timer(sk); + + if (!sk_stream_is_writeable(ssk)) + WRITE_ONCE(mptcp_subflow_ctx(ssk)->writable, false); } release_sock(ssk);
Even with fixed subflow_write_space(), we may still indicate EPOLLOUT even if no subflow is writeable. We need to re-check the last ssk that was used after the final tcp_push(). tcp_push() can allocate new skbs that get charged to the subflow sk which may bring it above the wmem limit. Signed-off-by: Florian Westphal <fw@strlen.de> --- net/mptcp/protocol.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)