mptcp.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH] tcp: consistently disable header prediction for mptcp
@ 2021-06-11 17:57 Paolo Abeni
  2021-06-15 22:45 ` Mat Martineau
  2021-06-24 16:02 ` Matthieu Baerts
  0 siblings, 2 replies; 3+ messages in thread
From: Paolo Abeni @ 2021-06-11 17:57 UTC (permalink / raw)
  To: mptcp; +Cc: fwestpha

The MPTCP receive path is hooked only into the TCP slow-path.
The DSS presence allows plain MPTCP traffic to hit that
consistently.

Since commit e1ff9e82e2ea ("net: mptcp: improve fallback to TCP"),
when and MPTCP socket falls back to TCP, we can the receive
fast path, and delay or stop triggering the event notification.

Address the issue explicitly disabling the header prediction
for MPTCP sockets.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/200
Fixes: e1ff9e82e2ea ("net: mptcp: improve fallback to TCP")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
I could not find a reliable way to disable header prediction
touching the MPTCP code only: the TCP stack can flip header
prediction on in a bunch of points and I could not find existing
MPTCP hooks after such check to revert the change.

So I opted for this TCP change, unless we are able to really
drop TCP fast path ?!? (see commit
31770e34e43d6c8dee129bfee77e56c34e61f0e5)

@Florian: WDYT?
---
 include/net/tcp.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index e668f1bf780d..17df9b047ee4 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -686,6 +686,10 @@ static inline u32 __tcp_set_rto(const struct tcp_sock *tp)
 
 static inline void __tcp_fast_path_on(struct tcp_sock *tp, u32 snd_wnd)
 {
+	/* mptcp hooks are only on the slow path */
+	if (sk_is_mptcp((struct sock *)tp))
+		return;
+
 	tp->pred_flags = htonl((tp->tcp_header_len << 26) |
 			       ntohl(TCP_FLAG_ACK) |
 			       snd_wnd);
-- 
2.26.3


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] tcp: consistently disable header prediction for mptcp
  2021-06-11 17:57 [RFC PATCH] tcp: consistently disable header prediction for mptcp Paolo Abeni
@ 2021-06-15 22:45 ` Mat Martineau
  2021-06-24 16:02 ` Matthieu Baerts
  1 sibling, 0 replies; 3+ messages in thread
From: Mat Martineau @ 2021-06-15 22:45 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: mptcp, Florian Westphal

On Fri, 11 Jun 2021, Paolo Abeni wrote:

> The MPTCP receive path is hooked only into the TCP slow-path.
> The DSS presence allows plain MPTCP traffic to hit that
> consistently.
>
> Since commit e1ff9e82e2ea ("net: mptcp: improve fallback to TCP"),
> when and MPTCP socket falls back to TCP, we can the receive
> fast path, and delay or stop triggering the event notification.
>
> Address the issue explicitly disabling the header prediction
> for MPTCP sockets.
>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/200
> Fixes: e1ff9e82e2ea ("net: mptcp: improve fallback to TCP")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> I could not find a reliable way to disable header prediction
> touching the MPTCP code only: the TCP stack can flip header
> prediction on in a bunch of points and I could not find existing
> MPTCP hooks after such check to revert the change.
>
> So I opted for this TCP change, unless we are able to really
> drop TCP fast path ?!? (see commit
> 31770e34e43d6c8dee129bfee77e56c34e61f0e5)
>
> @Florian: WDYT?

I'm also interested in Florian's thoughts.

We need to fix MPTCP fallback whether regular TCP fast path is still 
present or not, so I'm in favor of this patch. Looking at Florian's 
comments on those previous patches (31770e34e43 and the patch it reverts), 
I wonder if anything in the last four years has changed tcp_ack() calls 
enough to affect performance.


Mat

> ---
> include/net/tcp.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index e668f1bf780d..17df9b047ee4 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -686,6 +686,10 @@ static inline u32 __tcp_set_rto(const struct tcp_sock *tp)
>
> static inline void __tcp_fast_path_on(struct tcp_sock *tp, u32 snd_wnd)
> {
> +	/* mptcp hooks are only on the slow path */
> +	if (sk_is_mptcp((struct sock *)tp))
> +		return;
> +
> 	tp->pred_flags = htonl((tp->tcp_header_len << 26) |
> 			       ntohl(TCP_FLAG_ACK) |
> 			       snd_wnd);
> -- 
> 2.26.3
>
>
>

--
Mat Martineau
Intel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] tcp: consistently disable header prediction for mptcp
  2021-06-11 17:57 [RFC PATCH] tcp: consistently disable header prediction for mptcp Paolo Abeni
  2021-06-15 22:45 ` Mat Martineau
@ 2021-06-24 16:02 ` Matthieu Baerts
  1 sibling, 0 replies; 3+ messages in thread
From: Matthieu Baerts @ 2021-06-24 16:02 UTC (permalink / raw)
  To: Paolo Abeni, mptcp; +Cc: fwestpha

Hi Paolo,

On 11/06/2021 19:57, Paolo Abeni wrote:
> The MPTCP receive path is hooked only into the TCP slow-path.
> The DSS presence allows plain MPTCP traffic to hit that
> consistently.
> 
> Since commit e1ff9e82e2ea ("net: mptcp: improve fallback to TCP"),
> when and MPTCP socket falls back to TCP, we can the receive
> fast path, and delay or stop triggering the event notification.
> 
> Address the issue explicitly disabling the header prediction
> for MPTCP sockets.
> 
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/200
> Fixes: e1ff9e82e2ea ("net: mptcp: improve fallback to TCP")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Thank you for the patch!

I just applied it in our tree so we have some tests and we can easily
work on top of it while it is going to be discussed upstream when you
are going to send it.

New patches:

- c33c021a9394: tcp: consistently disable header prediction for mptcp

- Results: 07b006118fa3..1f935a2c8db1

Builds and tests are now in progress:



https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20210624T160212

https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export/20210624T160212

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-06-24 16:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-11 17:57 [RFC PATCH] tcp: consistently disable header prediction for mptcp Paolo Abeni
2021-06-15 22:45 ` Mat Martineau
2021-06-24 16:02 ` Matthieu Baerts

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).