* [MPTCP] Re: [PATCH net v2] mptcp: fix length of ADD_ADDR with port suboption
@ 2021-03-08 10:49 Matthieu Baerts
0 siblings, 0 replies; 3+ messages in thread
From: Matthieu Baerts @ 2021-03-08 10:49 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 997 bytes --]
Hi Davide, Geliang, Mat,
On 04/03/2021 17:23, Davide Caratti wrote:
> in current Linux, MPTCP peers advertising endpoints with port numbers use
> a sub-option length that wrongly accounts for the trailing TCP NOP. Also,
> receivers will only process incoming ADD_ADDR with port having such wrong
> sub-option length. Fix this, making ADD_ADDR compliant to RFC8684 §3.4.1.
>
> this can be verified running tcpdump on the kselftests artifacts:
Thank you for the patch, review and testing!
Sorry for the delay to apply it, I was doing other modifications in the
tree to ease CI scripts and because I got a conflict with the export
branch, I didn't apply it before :)
- 268780afbf25: mptcp: fix length of ADD_ADDR with port sub-option
- d16c1995a017: conflict in t/mptcp-add-rm_list-in-mptcp_out_options
- Results: b6772f0348c6..ec0b131d1b83
Tests + export are going to be started soon!
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 3+ messages in thread
* [MPTCP] Re: [PATCH net v2] mptcp: fix length of ADD_ADDR with port suboption
@ 2021-03-05 2:14 Geliang Tang
0 siblings, 0 replies; 3+ messages in thread
From: Geliang Tang @ 2021-03-05 2:14 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 3875 bytes --]
Mat Martineau <mathew.j.martineau(a)linux.intel.com> 于2021年3月5日周五 上午8:28写道：
>
>
> On Thu, 4 Mar 2021, Davide Caratti wrote:
>
> > in current Linux, MPTCP peers advertising endpoints with port numbers use
> > a sub-option length that wrongly accounts for the trailing TCP NOP. Also,
> > receivers will only process incoming ADD_ADDR with port having such wrong
> > sub-option length. Fix this, making ADD_ADDR compliant to RFC8684 §3.4.1.
> >
> > this can be verified running tcpdump on the kselftests artifacts:
> >
> > unpatched kernel:
> > [root(a)bottarga mptcp]# tcpdump -tnnr unpatched.pcap | grep add-addr
> > reading from file unpatched.pcap, link-type LINUX_SLL (Linux cooked v1), snapshot length 65535
> > IP 10.0.1.1.10000 > 10.0.1.2.53078: Flags [.], ack 101, win 509, options [nop,nop,TS val 214459678 ecr 521312851,mptcp add-addr v1 id 1 a00:201:2774:2d88:7436:85c3:17fd:101], length 0
> > IP 10.0.1.2.53078 > 10.0.1.1.10000: Flags [.], ack 101, win 502, options [nop,nop,TS val 521312852 ecr 214459678,mptcp add-addr[bad opt]]
> >
> > patched kernel:
> > [root(a)bottarga mptcp]# tcpdump -tnnr patched.pcap | grep add-addr
> > reading from file patched.pcap, link-type LINUX_SLL (Linux cooked v1), snapshot length 65535
> > IP 10.0.1.1.10000 > 10.0.1.2.38178: Flags [.], ack 101, win 509, options [nop,nop,TS val 3728873902 ecr 2732713192,mptcp add-addr v1 id 1 10.0.2.1:10100 hmac 0xbccdfcbe59292a1f,nop,nop], length 0
> > IP 10.0.1.2.38178 > 10.0.1.1.10000: Flags [.], ack 101, win 502, options [nop,nop,TS val 2732713195 ecr 3728873902,mptcp add-addr v1-echo id 1 10.0.2.1:10100,nop,nop], length 0
> >
> > Fixes: 22fb85ffaefb ("mptcp: add port support for ADD_ADDR suboption writing")
> > CC: stable(a)vger.kernel.org # 5.11+
> > Signed-off-by: Davide Caratti <dcaratti(a)redhat.com>
> > ---
> > net/mptcp/protocol.h | 14 ++++++++------
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> > index 91827d949766..e21a5bc36cf0 100644
> > --- a/net/mptcp/protocol.h
> > +++ b/net/mptcp/protocol.h
> > @@ -52,14 +52,15 @@
> > #define TCPOLEN_MPTCP_DSS_MAP64 14
> > #define TCPOLEN_MPTCP_DSS_CHECKSUM 2
> > #define TCPOLEN_MPTCP_ADD_ADDR 16
> > -#define TCPOLEN_MPTCP_ADD_ADDR_PORT 20
> > +#define TCPOLEN_MPTCP_ADD_ADDR_PORT 18
> > #define TCPOLEN_MPTCP_ADD_ADDR_BASE 8
> > -#define TCPOLEN_MPTCP_ADD_ADDR_BASE_PORT 12
> > +#define TCPOLEN_MPTCP_ADD_ADDR_BASE_PORT 10
> > #define TCPOLEN_MPTCP_ADD_ADDR6 28
> > -#define TCPOLEN_MPTCP_ADD_ADDR6_PORT 32
> > +#define TCPOLEN_MPTCP_ADD_ADDR6_PORT 30
> > #define TCPOLEN_MPTCP_ADD_ADDR6_BASE 20
> > -#define TCPOLEN_MPTCP_ADD_ADDR6_BASE_PORT 24
> > -#define TCPOLEN_MPTCP_PORT_LEN 4
> > +#define TCPOLEN_MPTCP_ADD_ADDR6_BASE_PORT 22
> > +#define TCPOLEN_MPTCP_PORT_LEN 2
> > +#define TCPOLEN_MPTCP_PORT_ALIGN 2
> > #define TCPOLEN_MPTCP_RM_ADDR_BASE 4
> > #define TCPOLEN_MPTCP_PRIO 3
> > #define TCPOLEN_MPTCP_PRIO_ALIGN 4
> > @@ -701,8 +702,9 @@ static inline unsigned int mptcp_add_addr_len(int family, bool echo, bool port)
> > len = TCPOLEN_MPTCP_ADD_ADDR6_BASE;
> > if (!echo)
> > len += MPTCPOPT_THMAC_LEN;
> > + /* account for 2 trailing 'nop' options */
> > if (port)
> > - len += TCPOLEN_MPTCP_PORT_LEN;
> > + len += TCPOLEN_MPTCP_PORT_LEN + TCPOLEN_MPTCP_PORT_ALIGN;
> >
> > return len;
> > }
> > --
> > 2.29.2
>
> Thanks Davide - fix looks good to me for -net and stable.
>
> Reviewed-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
>
Acked-and-tested-by: Geliang Tang <geliangtang(a)gmail.com>
-Geliang
> --
> Mat Martineau
> Intel
^ permalink raw reply [flat|nested] 3+ messages in thread
* [MPTCP] Re: [PATCH net v2] mptcp: fix length of ADD_ADDR with port suboption
@ 2021-03-05 0:28 Mat Martineau
0 siblings, 0 replies; 3+ messages in thread
From: Mat Martineau @ 2021-03-05 0:28 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 3379 bytes --]
On Thu, 4 Mar 2021, Davide Caratti wrote:
> in current Linux, MPTCP peers advertising endpoints with port numbers use
> a sub-option length that wrongly accounts for the trailing TCP NOP. Also,
> receivers will only process incoming ADD_ADDR with port having such wrong
> sub-option length. Fix this, making ADD_ADDR compliant to RFC8684 §3.4.1.
>
> this can be verified running tcpdump on the kselftests artifacts:
>
> unpatched kernel:
> [root(a)bottarga mptcp]# tcpdump -tnnr unpatched.pcap | grep add-addr
> reading from file unpatched.pcap, link-type LINUX_SLL (Linux cooked v1), snapshot length 65535
> IP 10.0.1.1.10000 > 10.0.1.2.53078: Flags [.], ack 101, win 509, options [nop,nop,TS val 214459678 ecr 521312851,mptcp add-addr v1 id 1 a00:201:2774:2d88:7436:85c3:17fd:101], length 0
> IP 10.0.1.2.53078 > 10.0.1.1.10000: Flags [.], ack 101, win 502, options [nop,nop,TS val 521312852 ecr 214459678,mptcp add-addr[bad opt]]
>
> patched kernel:
> [root(a)bottarga mptcp]# tcpdump -tnnr patched.pcap | grep add-addr
> reading from file patched.pcap, link-type LINUX_SLL (Linux cooked v1), snapshot length 65535
> IP 10.0.1.1.10000 > 10.0.1.2.38178: Flags [.], ack 101, win 509, options [nop,nop,TS val 3728873902 ecr 2732713192,mptcp add-addr v1 id 1 10.0.2.1:10100 hmac 0xbccdfcbe59292a1f,nop,nop], length 0
> IP 10.0.1.2.38178 > 10.0.1.1.10000: Flags [.], ack 101, win 502, options [nop,nop,TS val 2732713195 ecr 3728873902,mptcp add-addr v1-echo id 1 10.0.2.1:10100,nop,nop], length 0
>
> Fixes: 22fb85ffaefb ("mptcp: add port support for ADD_ADDR suboption writing")
> CC: stable(a)vger.kernel.org # 5.11+
> Signed-off-by: Davide Caratti <dcaratti(a)redhat.com>
> ---
> net/mptcp/protocol.h | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 91827d949766..e21a5bc36cf0 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -52,14 +52,15 @@
> #define TCPOLEN_MPTCP_DSS_MAP64 14
> #define TCPOLEN_MPTCP_DSS_CHECKSUM 2
> #define TCPOLEN_MPTCP_ADD_ADDR 16
> -#define TCPOLEN_MPTCP_ADD_ADDR_PORT 20
> +#define TCPOLEN_MPTCP_ADD_ADDR_PORT 18
> #define TCPOLEN_MPTCP_ADD_ADDR_BASE 8
> -#define TCPOLEN_MPTCP_ADD_ADDR_BASE_PORT 12
> +#define TCPOLEN_MPTCP_ADD_ADDR_BASE_PORT 10
> #define TCPOLEN_MPTCP_ADD_ADDR6 28
> -#define TCPOLEN_MPTCP_ADD_ADDR6_PORT 32
> +#define TCPOLEN_MPTCP_ADD_ADDR6_PORT 30
> #define TCPOLEN_MPTCP_ADD_ADDR6_BASE 20
> -#define TCPOLEN_MPTCP_ADD_ADDR6_BASE_PORT 24
> -#define TCPOLEN_MPTCP_PORT_LEN 4
> +#define TCPOLEN_MPTCP_ADD_ADDR6_BASE_PORT 22
> +#define TCPOLEN_MPTCP_PORT_LEN 2
> +#define TCPOLEN_MPTCP_PORT_ALIGN 2
> #define TCPOLEN_MPTCP_RM_ADDR_BASE 4
> #define TCPOLEN_MPTCP_PRIO 3
> #define TCPOLEN_MPTCP_PRIO_ALIGN 4
> @@ -701,8 +702,9 @@ static inline unsigned int mptcp_add_addr_len(int family, bool echo, bool port)
> len = TCPOLEN_MPTCP_ADD_ADDR6_BASE;
> if (!echo)
> len += MPTCPOPT_THMAC_LEN;
> + /* account for 2 trailing 'nop' options */
> if (port)
> - len += TCPOLEN_MPTCP_PORT_LEN;
> + len += TCPOLEN_MPTCP_PORT_LEN + TCPOLEN_MPTCP_PORT_ALIGN;
>
> return len;
> }
> --
> 2.29.2
Thanks Davide - fix looks good to me for -net and stable.
Reviewed-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-08 10:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08 10:49 [MPTCP] Re: [PATCH net v2] mptcp: fix length of ADD_ADDR with port suboption Matthieu Baerts
-- strict thread matches above, loose matches on Subject: below --
2021-03-05 2:14 Geliang Tang
2021-03-05 0:28 Mat Martineau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.