* [MPTCP] Re: [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows
@ 2020-10-06 16:50 Paolo Abeni
0 siblings, 0 replies; 8+ messages in thread
From: Paolo Abeni @ 2020-10-06 16:50 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 5749 bytes --]
On Tue, 2020-10-06 at 18:35 +0200, Matthieu Baerts wrote:
> Hi Davide,
>
> On 06/10/2020 16:03, Matthieu Baerts wrote:
> > Hi Davide, Paolo,
> >
> > On 06/10/2020 15:07, Paolo Abeni wrote:
> > > On Tue, 2020-10-06 at 14:22 +0200, Davide Caratti wrote:
> > > > using packetdrill it's possible to observe the same MPTCP DSN being
> > > > acked
> > > > by different subflows with DACK4 and DACK8. This is in contrast with
> > > > what
> > > > specified in RFC8684 §3.3.2: if an MPTCP endpoint transmits a 64-bit
> > > > wide
> > > > DSN, it MUST be acknowledged with a 64-bit wide DACK. Fix
> > > > 'use_64bit_ack'
> > > > variable to make it a property of MPTCP sockets, not TCP subflows.
> > > >
> > > > Fixes: a0c1d0eafd1e ("mptcp: Use 32-bit DATA_ACK when possible")
> > > > Signed-off-by: Davide Caratti <dcaratti(a)redhat.com>
> > >
> > > LGTM, thanks Davide!
> >
> > Thank you for the patch and the review!
> >
> > @Davide: be careful that this patch doesn't apply as is on -net (or
> > net-next). I had a small conflict when applying it on top of net-next,
> > not at the end of the "export" branch?
> >
> > - 79519fc3f520: net: mptcp: make DACK4/DACK8 usage consistent among all
> > subflows
> > - be078f4e24b5: conflict in t/mptcp-refactor-shutdown-and-close
> > - Results: b74dfa0c4018..9984fc6a69ca
> >
> > Tests + export are in progress!
>
> It looks unrelated but one selftest -- mptcp_join.sh, test 11 -- was
> failing with a debug kernel:
>
>
> # selftests: net/mptcp: mptcp_join.sh
> # Created /tmp/tmp.TXFPMQuIYt (size 1 KB) containing data sent by client
> # Created /tmp/tmp.rZmBg6Ql4I (size 1 KB) containing data sent by server
> # 01 no JOIN syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # 02 single subflow, limited by client syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # 03 single subflow, limited by server syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # 04 single subflow syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # 05 multiple subflows syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # 06 multiple subflows, limited by server syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # 07 unused signal address syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # add[ ok ] - echo [ ok ]
> # 08 signal address syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # add[ ok ] - echo [ ok ]
> # 09 subflow and signal syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # add[ ok ] - echo [ ok ]
> # 10 multiple subflows and signal syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # add[ ok ] - echo [ ok ]
> # 11 signal address, ADD_ADDR timeout syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # add[fail] got 2 ADD_ADDR[s]
> expected 4
> # - echo [ ok ]
> # Server ns stats
> # MPTcpExtMPCapableSYNRX 1 0.0
> # MPTcpExtMPCapableACKRX 1 0.0
> # MPTcpExtMPTCPRetrans 1 0.0
> # MPTcpExtMPJoinSynRx 1 0.0
> # MPTcpExtMPJoinAckRx 1 0.0
> # MPTcpExtDuplicateData 1 0.0
> # Client ns stats
> # MPTcpExtMPTCPRetrans 1 0.0
> # MPTcpExtMPJoinSynAckRx 1 0.0
> # MPTcpExtDuplicateData 1 0.0
> # MPTcpExtAddAddr 2 0.0
> # 12 remove single subflow syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # rm [ ok ] - sf [ ok ]
> # 13 remove multiple subflows syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # rm [ ok ] - sf [ ok ]
> # 14 remove single address syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # add[ ok ] - echo [ ok ]
> # rm [ ok ] - sf [ ok ]
> # 15 remove subflow and signal syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # add[ ok ] - echo [ ok ]
> # rm [ ok ] - sf [ ok ]
> # 16 remove subflows and signal syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # add[ ok ] - echo [ ok ]
> # rm [ ok ] - sf [ ok ]
> # 17 single subflow with syn cookies syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # 18 multiple subflows with syn cookies syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # 19 subflows limited by server w cookies syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # 20 signal address with syn cookies syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # add[ ok ] - echo [ ok ]
> # 21 subflow and signal w cookies syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # add[ ok ] - echo [ ok ]
> # 22 subflows and signal w. cookies syn[ ok ] - synack[ ok ] -
> ack[ ok ]
> # add[ ok ] - echo [ ok ]
> not ok 3 selftests: net/mptcp: mptcp_join.sh # exit=1
>
>
> I don't think it is due to your modification. I didn't have the issue
> without the debug kconfig.
uhmm... that self-test is failing here, too...
... just because my VM don't have iptables (required by said test:)
iptables is dead! long live to nftables!!! :)))
/P
^ permalink raw reply [flat|nested] 8+ messages in thread
* [MPTCP] Re: [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows
@ 2020-10-09 15:35 Jakub Kicinski
0 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2020-10-09 15:35 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 631 bytes --]
On Tue, 6 Oct 2020 18:26:17 +0200 Davide Caratti wrote:
> using packetdrill it's possible to observe the same MPTCP DSN being acked
> by different subflows with DACK4 and DACK8. This is in contrast with what
> specified in RFC8684 §3.3.2: if an MPTCP endpoint transmits a 64-bit wide
> DSN, it MUST be acknowledged with a 64-bit wide DACK. Fix 'use_64bit_ack'
> variable to make it a property of MPTCP sockets, not TCP subflows.
>
> Fixes: a0c1d0eafd1e ("mptcp: Use 32-bit DATA_ACK when possible")
> Acked-by: Paolo Abeni <pabeni(a)redhat.com>
> Signed-off-by: Davide Caratti <dcaratti(a)redhat.com>
Applied, thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread
* [MPTCP] Re: [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows
@ 2020-10-06 23:58 Mat Martineau
0 siblings, 0 replies; 8+ messages in thread
From: Mat Martineau @ 2020-10-06 23:58 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 849 bytes --]
On Tue, 6 Oct 2020, Davide Caratti wrote:
> using packetdrill it's possible to observe the same MPTCP DSN being acked
> by different subflows with DACK4 and DACK8. This is in contrast with what
> specified in RFC8684 §3.3.2: if an MPTCP endpoint transmits a 64-bit wide
> DSN, it MUST be acknowledged with a 64-bit wide DACK. Fix 'use_64bit_ack'
> variable to make it a property of MPTCP sockets, not TCP subflows.
>
> Fixes: a0c1d0eafd1e ("mptcp: Use 32-bit DATA_ACK when possible")
> Acked-by: Paolo Abeni <pabeni(a)redhat.com>
> Signed-off-by: Davide Caratti <dcaratti(a)redhat.com>
> ---
> net/mptcp/options.c | 2 +-
> net/mptcp/protocol.h | 2 +-
> net/mptcp/subflow.c | 3 +--
> 3 files changed, 3 insertions(+), 4 deletions(-)
Reviewed-by: Mat Martineau <mathew.j.martineau(a)linux.intel.com>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 8+ messages in thread
* [MPTCP] Re: [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows
@ 2020-10-06 16:35 Matthieu Baerts
0 siblings, 0 replies; 8+ messages in thread
From: Matthieu Baerts @ 2020-10-06 16:35 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 5325 bytes --]
Hi Davide,
On 06/10/2020 16:03, Matthieu Baerts wrote:
> Hi Davide, Paolo,
>
> On 06/10/2020 15:07, Paolo Abeni wrote:
>> On Tue, 2020-10-06 at 14:22 +0200, Davide Caratti wrote:
>>> using packetdrill it's possible to observe the same MPTCP DSN being
>>> acked
>>> by different subflows with DACK4 and DACK8. This is in contrast with
>>> what
>>> specified in RFC8684 §3.3.2: if an MPTCP endpoint transmits a 64-bit
>>> wide
>>> DSN, it MUST be acknowledged with a 64-bit wide DACK. Fix
>>> 'use_64bit_ack'
>>> variable to make it a property of MPTCP sockets, not TCP subflows.
>>>
>>> Fixes: a0c1d0eafd1e ("mptcp: Use 32-bit DATA_ACK when possible")
>>> Signed-off-by: Davide Caratti <dcaratti(a)redhat.com>
>>
>> LGTM, thanks Davide!
>
> Thank you for the patch and the review!
>
> @Davide: be careful that this patch doesn't apply as is on -net (or
> net-next). I had a small conflict when applying it on top of net-next,
> not at the end of the "export" branch?
>
> - 79519fc3f520: net: mptcp: make DACK4/DACK8 usage consistent among all
> subflows
> - be078f4e24b5: conflict in t/mptcp-refactor-shutdown-and-close
> - Results: b74dfa0c4018..9984fc6a69ca
>
> Tests + export are in progress!
It looks unrelated but one selftest -- mptcp_join.sh, test 11 -- was
failing with a debug kernel:
# selftests: net/mptcp: mptcp_join.sh
# Created /tmp/tmp.TXFPMQuIYt (size 1 KB) containing data sent by client
# Created /tmp/tmp.rZmBg6Ql4I (size 1 KB) containing data sent by server
# 01 no JOIN syn[ ok ] - synack[ ok ] -
ack[ ok ]
# 02 single subflow, limited by client syn[ ok ] - synack[ ok ] -
ack[ ok ]
# 03 single subflow, limited by server syn[ ok ] - synack[ ok ] -
ack[ ok ]
# 04 single subflow syn[ ok ] - synack[ ok ] -
ack[ ok ]
# 05 multiple subflows syn[ ok ] - synack[ ok ] -
ack[ ok ]
# 06 multiple subflows, limited by server syn[ ok ] - synack[ ok ] -
ack[ ok ]
# 07 unused signal address syn[ ok ] - synack[ ok ] -
ack[ ok ]
# add[ ok ] - echo [ ok ]
# 08 signal address syn[ ok ] - synack[ ok ] -
ack[ ok ]
# add[ ok ] - echo [ ok ]
# 09 subflow and signal syn[ ok ] - synack[ ok ] -
ack[ ok ]
# add[ ok ] - echo [ ok ]
# 10 multiple subflows and signal syn[ ok ] - synack[ ok ] -
ack[ ok ]
# add[ ok ] - echo [ ok ]
# 11 signal address, ADD_ADDR timeout syn[ ok ] - synack[ ok ] -
ack[ ok ]
# add[fail] got 2 ADD_ADDR[s]
expected 4
# - echo [ ok ]
# Server ns stats
# MPTcpExtMPCapableSYNRX 1 0.0
# MPTcpExtMPCapableACKRX 1 0.0
# MPTcpExtMPTCPRetrans 1 0.0
# MPTcpExtMPJoinSynRx 1 0.0
# MPTcpExtMPJoinAckRx 1 0.0
# MPTcpExtDuplicateData 1 0.0
# Client ns stats
# MPTcpExtMPTCPRetrans 1 0.0
# MPTcpExtMPJoinSynAckRx 1 0.0
# MPTcpExtDuplicateData 1 0.0
# MPTcpExtAddAddr 2 0.0
# 12 remove single subflow syn[ ok ] - synack[ ok ] -
ack[ ok ]
# rm [ ok ] - sf [ ok ]
# 13 remove multiple subflows syn[ ok ] - synack[ ok ] -
ack[ ok ]
# rm [ ok ] - sf [ ok ]
# 14 remove single address syn[ ok ] - synack[ ok ] -
ack[ ok ]
# add[ ok ] - echo [ ok ]
# rm [ ok ] - sf [ ok ]
# 15 remove subflow and signal syn[ ok ] - synack[ ok ] -
ack[ ok ]
# add[ ok ] - echo [ ok ]
# rm [ ok ] - sf [ ok ]
# 16 remove subflows and signal syn[ ok ] - synack[ ok ] -
ack[ ok ]
# add[ ok ] - echo [ ok ]
# rm [ ok ] - sf [ ok ]
# 17 single subflow with syn cookies syn[ ok ] - synack[ ok ] -
ack[ ok ]
# 18 multiple subflows with syn cookies syn[ ok ] - synack[ ok ] -
ack[ ok ]
# 19 subflows limited by server w cookies syn[ ok ] - synack[ ok ] -
ack[ ok ]
# 20 signal address with syn cookies syn[ ok ] - synack[ ok ] -
ack[ ok ]
# add[ ok ] - echo [ ok ]
# 21 subflow and signal w cookies syn[ ok ] - synack[ ok ] -
ack[ ok ]
# add[ ok ] - echo [ ok ]
# 22 subflows and signal w. cookies syn[ ok ] - synack[ ok ] -
ack[ ok ]
# add[ ok ] - echo [ ok ]
not ok 3 selftests: net/mptcp: mptcp_join.sh # exit=1
I don't think it is due to your modification. I didn't have the issue
without the debug kconfig.
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 8+ messages in thread
* [MPTCP] Re: [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows
@ 2020-10-06 15:11 Matthieu Baerts
0 siblings, 0 replies; 8+ messages in thread
From: Matthieu Baerts @ 2020-10-06 15:11 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 1837 bytes --]
Hi Davide,
On 06/10/2020 16:30, Davide Caratti wrote:
> On Tue, 2020-10-06 at 16:03 +0200, Matthieu Baerts wrote:
>> Hi Davide, Paolo,
>>
>> On 06/10/2020 15:07, Paolo Abeni wrote:
>>> On Tue, 2020-10-06 at 14:22 +0200, Davide Caratti wrote:
>>>> using packetdrill it's possible to observe the same MPTCP DSN being acked
>>>> by different subflows with DACK4 and DACK8. This is in contrast with what
>>>> specified in RFC8684 §3.3.2: if an MPTCP endpoint transmits a 64-bit wide
>>>> DSN, it MUST be acknowledged with a 64-bit wide DACK. Fix 'use_64bit_ack'
>>>> variable to make it a property of MPTCP sockets, not TCP subflows.
>>>>
>>>> Fixes: a0c1d0eafd1e ("mptcp: Use 32-bit DATA_ACK when possible")
>>>> Signed-off-by: Davide Caratti <dcaratti(a)redhat.com>
>>>
>>> LGTM, thanks Davide!
>>
>> Thank you for the patch and the review!
>
> hello Matthieu, thanks for looking at this.
>
>> @Davide: be careful that this patch doesn't apply as is on -net (or
>> net-next). I had a small conflict when applying it on top of net-next,
>> not at the end of the "export" branch?
>
> right, I did that on top of export/20201006T083856 - but it requires
> some adjustment to apply to "net", because context mismatches in
> protocol.h.
It looks like if you cherry-pick your patch that is going to be in the
next "export" branch (or 79519fc3f520 from the TopGit tree), you will
not have any conflicts! :-)
> FTR, this is another strange thing I noticed while working on
> https://github.com/multipath-tcp/mptcp_net-next/issues/94 : after a call
> to close(), subflow-0 does DSS (DACK4+DATAFIN), while subflow-1 does
> DSS(DACK8+DATAFIN) :)
Good catch! Always good to play with packetdrill to fix new bugs :-)
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 8+ messages in thread
* [MPTCP] Re: [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows
@ 2020-10-06 14:30 Davide Caratti
0 siblings, 0 replies; 8+ messages in thread
From: Davide Caratti @ 2020-10-06 14:30 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 1445 bytes --]
On Tue, 2020-10-06 at 16:03 +0200, Matthieu Baerts wrote:
> Hi Davide, Paolo,
>
> On 06/10/2020 15:07, Paolo Abeni wrote:
> > On Tue, 2020-10-06 at 14:22 +0200, Davide Caratti wrote:
> > > using packetdrill it's possible to observe the same MPTCP DSN being acked
> > > by different subflows with DACK4 and DACK8. This is in contrast with what
> > > specified in RFC8684 §3.3.2: if an MPTCP endpoint transmits a 64-bit wide
> > > DSN, it MUST be acknowledged with a 64-bit wide DACK. Fix 'use_64bit_ack'
> > > variable to make it a property of MPTCP sockets, not TCP subflows.
> > >
> > > Fixes: a0c1d0eafd1e ("mptcp: Use 32-bit DATA_ACK when possible")
> > > Signed-off-by: Davide Caratti <dcaratti(a)redhat.com>
> >
> > LGTM, thanks Davide!
>
> Thank you for the patch and the review!
hello Matthieu, thanks for looking at this.
> @Davide: be careful that this patch doesn't apply as is on -net (or
> net-next). I had a small conflict when applying it on top of net-next,
> not at the end of the "export" branch?
right, I did that on top of export/20201006T083856 - but it requires
some adjustment to apply to "net", because context mismatches in
protocol.h.
FTR, this is another strange thing I noticed while working on
https://github.com/multipath-tcp/mptcp_net-next/issues/94 : after a call
to close(), subflow-0 does DSS (DACK4+DATAFIN), while subflow-1 does
DSS(DACK8+DATAFIN) :)
--
davide
^ permalink raw reply [flat|nested] 8+ messages in thread
* [MPTCP] Re: [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows
@ 2020-10-06 14:03 Matthieu Baerts
0 siblings, 0 replies; 8+ messages in thread
From: Matthieu Baerts @ 2020-10-06 14:03 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 1202 bytes --]
Hi Davide, Paolo,
On 06/10/2020 15:07, Paolo Abeni wrote:
> On Tue, 2020-10-06 at 14:22 +0200, Davide Caratti wrote:
>> using packetdrill it's possible to observe the same MPTCP DSN being acked
>> by different subflows with DACK4 and DACK8. This is in contrast with what
>> specified in RFC8684 §3.3.2: if an MPTCP endpoint transmits a 64-bit wide
>> DSN, it MUST be acknowledged with a 64-bit wide DACK. Fix 'use_64bit_ack'
>> variable to make it a property of MPTCP sockets, not TCP subflows.
>>
>> Fixes: a0c1d0eafd1e ("mptcp: Use 32-bit DATA_ACK when possible")
>> Signed-off-by: Davide Caratti <dcaratti(a)redhat.com>
>
> LGTM, thanks Davide!
Thank you for the patch and the review!
@Davide: be careful that this patch doesn't apply as is on -net (or
net-next). I had a small conflict when applying it on top of net-next,
not at the end of the "export" branch?
- 79519fc3f520: net: mptcp: make DACK4/DACK8 usage consistent among all
subflows
- be078f4e24b5: conflict in t/mptcp-refactor-shutdown-and-close
- Results: b74dfa0c4018..9984fc6a69ca
Tests + export are in progress!
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 8+ messages in thread
* [MPTCP] Re: [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows
@ 2020-10-06 13:07 Paolo Abeni
0 siblings, 0 replies; 8+ messages in thread
From: Paolo Abeni @ 2020-10-06 13:07 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 596 bytes --]
On Tue, 2020-10-06 at 14:22 +0200, Davide Caratti wrote:
> using packetdrill it's possible to observe the same MPTCP DSN being acked
> by different subflows with DACK4 and DACK8. This is in contrast with what
> specified in RFC8684 §3.3.2: if an MPTCP endpoint transmits a 64-bit wide
> DSN, it MUST be acknowledged with a 64-bit wide DACK. Fix 'use_64bit_ack'
> variable to make it a property of MPTCP sockets, not TCP subflows.
>
> Fixes: a0c1d0eafd1e ("mptcp: Use 32-bit DATA_ACK when possible")
> Signed-off-by: Davide Caratti <dcaratti(a)redhat.com>
LGTM, thanks Davide!
/P
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-10-09 15:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-06 16:50 [MPTCP] Re: [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows Paolo Abeni
-- strict thread matches above, loose matches on Subject: below --
2020-10-09 15:35 Jakub Kicinski
2020-10-06 23:58 Mat Martineau
2020-10-06 16:35 Matthieu Baerts
2020-10-06 15:11 Matthieu Baerts
2020-10-06 14:30 Davide Caratti
2020-10-06 14:03 Matthieu Baerts
2020-10-06 13:07 Paolo Abeni
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.