[PATCH net-next] mptcp: drop all sub-options except ADD_ADDR when the echo bit is set

[PATCH net-next] mptcp: drop all sub-options except ADD_ADDR when the echo bit is set

[Davide Caratti]

Current Linux carries echo-ed ADD_ADDR over pure TCP ACKs, so there is no
need to add a DSS element that would fit only ADD_ADDR with IPv4 address.
Drop the DSS from echo-ed ADD_ADDR, regardless of the IP version.

Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 net/mptcp/options.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index c7eb61d0564c..d51c3ad54d9a 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -624,7 +624,8 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
 	int len;
 
 	if ((mptcp_pm_should_add_signal_ipv6(msk) ||
-	     mptcp_pm_should_add_signal_port(msk)) &&
+	     mptcp_pm_should_add_signal_port(msk) ||
+	     mptcp_pm_should_add_signal_echo(msk)) &&
 	    skb && skb_is_tcp_pure_ack(skb)) {
 		pr_debug("drop other suboptions");
 		opts->suboptions = 0;
-- 
2.30.2

Re: [PATCH net-next] mptcp: drop all sub-options except ADD_ADDR when the echo bit is set

[Davide Caratti]

On Fri, 2021-03-26 at 13:28 +0100, Davide Caratti wrote:
> Current Linux carries echo-ed ADD_ADDR over pure TCP ACKs, so there is no
> need to add a DSS element that would fit only ADD_ADDR with IPv4 address.
> Drop the DSS from echo-ed ADD_ADDR, regardless of the IP version.
> 
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>

please note: testing this commit with current packetdrill will fail,
because the expected format of outbound echoed ADD_ADDR will fail.

In parallel with this "harmonization" of IPv4 / IPv6 echo-ed ADD_ADDRs
the kernel, I'm working on packetdrill to fix the support for IPv6 in
ADD_ADDR [1]. Non-echoed ADD_ADDRs are still "non-harmonized", so you
will notice the DSS sub-option on IPv4 and no other sub-option except
ADD_ADDR on IPv6.

So, server scripts using ADD_ADDR will still need 2 separate files for
IPv4 and IPv6, while it will be possible to run the same client script
on IPv4 and IPv6.

@matthieu, just writing few notes here as probably the CI you are
running will be "glitching" because of these changes. Sorry for the
inconvenience :]

-- 
davide

[1] https://github.com/multipath-tcp/packetdrill/pull/49






> ---
>  net/mptcp/options.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index c7eb61d0564c..d51c3ad54d9a 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -624,7 +624,8 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
>  	int len;
>  
> 
> 
> 
>  	if ((mptcp_pm_should_add_signal_ipv6(msk) ||
> -	     mptcp_pm_should_add_signal_port(msk)) &&
> +	     mptcp_pm_should_add_signal_port(msk) ||
> +	     mptcp_pm_should_add_signal_echo(msk)) &&
>  	    skb && skb_is_tcp_pure_ack(skb)) {
>  		pr_debug("drop other suboptions");
>  		opts->suboptions = 0;

Re: [PATCH net-next] mptcp: drop all sub-options except ADD_ADDR when the echo bit is set

[Mat Martineau]

On Fri, 26 Mar 2021, Davide Caratti wrote:

> Current Linux carries echo-ed ADD_ADDR over pure TCP ACKs, so there is no
> need to add a DSS element that would fit only ADD_ADDR with IPv4 address.
> Drop the DSS from echo-ed ADD_ADDR, regardless of the IP version.
>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
> ---
> net/mptcp/options.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)

Thanks Davide, looks good to me. This didn't land in patchwork without the 
mptcp@lists.01.org cc, but we should have mptcp@lists.linux.dev routed in 
to patchwork by Monday (Australia time).

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>


>
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index c7eb61d0564c..d51c3ad54d9a 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -624,7 +624,8 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
> 	int len;
>
> 	if ((mptcp_pm_should_add_signal_ipv6(msk) ||
> -	     mptcp_pm_should_add_signal_port(msk)) &&
> +	     mptcp_pm_should_add_signal_port(msk) ||
> +	     mptcp_pm_should_add_signal_echo(msk)) &&
> 	    skb && skb_is_tcp_pure_ack(skb)) {
> 		pr_debug("drop other suboptions");
> 		opts->suboptions = 0;
> -- 
> 2.30.2

--
Mat Martineau
Intel

Re: [PATCH net-next] mptcp: drop all sub-options except ADD_ADDR when the echo bit is set

[Matthieu Baerts]

Hi Davide, Mat

On 26/03/2021 13:28, Davide Caratti wrote:
> Current Linux carries echo-ed ADD_ADDR over pure TCP ACKs, so there is no
> need to add a DSS element that would fit only ADD_ADDR with IPv4 address.
> Drop the DSS from echo-ed ADD_ADDR, regardless of the IP version.

Thank you for the patch!

Now applied in our repo with Mat's RvB tag:

- aa44e0298e25: mptcp: drop all sub-options except ADD_ADDR when the 
echo bit is set
- Results: 01e1b77c047c..ee61d54be9fd

Tests + export are in progress!

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