* [MPTCP] [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows
@ 2020-10-06 16:26 ` Davide Caratti
0 siblings, 0 replies; 12+ messages in thread
From: Davide Caratti @ 2020-10-06 16:26 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 2549 bytes --]
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(-)
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 888bbbbb3e8a..9d7fa93fe0cf 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -516,7 +516,7 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
return ret;
}
- if (subflow->use_64bit_ack) {
+ if (READ_ONCE(msk->use_64bit_ack)) {
ack_size = TCPOLEN_MPTCP_DSS_ACK64;
opts->ext_copy.data_ack = READ_ONCE(msk->ack_seq);
opts->ext_copy.ack64 = 1;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 20f04ac85409..285dd8b2b43a 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -202,6 +202,7 @@ struct mptcp_sock {
bool fully_established;
bool rcv_data_fin;
bool snd_data_fin_enable;
+ bool use_64bit_ack; /* Set when we received a 64-bit DSN */
spinlock_t join_list_lock;
struct work_struct work;
struct list_head conn_list;
@@ -294,7 +295,6 @@ struct mptcp_subflow_context {
backup : 1,
data_avail : 1,
rx_eof : 1,
- use_64bit_ack : 1, /* Set when we received a 64-bit DSN */
can_ack : 1; /* only after processing the remote a key */
u32 remote_nonce;
u64 thmac;
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 6f035af1c9d2..91bef7bfffa6 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -769,12 +769,11 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
if (!mpext->dsn64) {
map_seq = expand_seq(subflow->map_seq, subflow->map_data_len,
mpext->data_seq);
- subflow->use_64bit_ack = 0;
pr_debug("expanded seq=%llu", subflow->map_seq);
} else {
map_seq = mpext->data_seq;
- subflow->use_64bit_ack = 1;
}
+ WRITE_ONCE(mptcp_sk(subflow->conn)->use_64bit_ack, !!mpext->dsn64);
if (subflow->map_valid) {
/* Allow replacing only with an identical map */
--
2.26.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows
@ 2020-10-06 16:26 ` Davide Caratti
0 siblings, 0 replies; 12+ messages in thread
From: Davide Caratti @ 2020-10-06 16:26 UTC (permalink / raw)
To: netdev, David S. Miller; +Cc: mptcp, Christoph Paasch, pabeni, Matthieu Baerts
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@redhat.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/mptcp/options.c | 2 +-
net/mptcp/protocol.h | 2 +-
net/mptcp/subflow.c | 3 +--
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 888bbbbb3e8a..9d7fa93fe0cf 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -516,7 +516,7 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
return ret;
}
- if (subflow->use_64bit_ack) {
+ if (READ_ONCE(msk->use_64bit_ack)) {
ack_size = TCPOLEN_MPTCP_DSS_ACK64;
opts->ext_copy.data_ack = READ_ONCE(msk->ack_seq);
opts->ext_copy.ack64 = 1;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 20f04ac85409..285dd8b2b43a 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -202,6 +202,7 @@ struct mptcp_sock {
bool fully_established;
bool rcv_data_fin;
bool snd_data_fin_enable;
+ bool use_64bit_ack; /* Set when we received a 64-bit DSN */
spinlock_t join_list_lock;
struct work_struct work;
struct list_head conn_list;
@@ -294,7 +295,6 @@ struct mptcp_subflow_context {
backup : 1,
data_avail : 1,
rx_eof : 1,
- use_64bit_ack : 1, /* Set when we received a 64-bit DSN */
can_ack : 1; /* only after processing the remote a key */
u32 remote_nonce;
u64 thmac;
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 6f035af1c9d2..91bef7bfffa6 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -769,12 +769,11 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
if (!mpext->dsn64) {
map_seq = expand_seq(subflow->map_seq, subflow->map_data_len,
mpext->data_seq);
- subflow->use_64bit_ack = 0;
pr_debug("expanded seq=%llu", subflow->map_seq);
} else {
map_seq = mpext->data_seq;
- subflow->use_64bit_ack = 1;
}
+ WRITE_ONCE(mptcp_sk(subflow->conn)->use_64bit_ack, !!mpext->dsn64);
if (subflow->map_valid) {
/* Allow replacing only with an identical map */
--
2.26.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [MPTCP] Re: [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows
2020-10-06 16:26 ` Davide Caratti
@ 2020-10-06 23:58 ` Mat Martineau
-1 siblings, 0 replies; 12+ 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] 12+ messages in thread
* Re: [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows
@ 2020-10-06 23:58 ` Mat Martineau
0 siblings, 0 replies; 12+ messages in thread
From: Mat Martineau @ 2020-10-06 23:58 UTC (permalink / raw)
To: Davide Caratti
Cc: netdev, David S. Miller, mptcp, Christoph Paasch, pabeni,
Matthieu Baerts
[-- Attachment #1: Type: text/plain, Size: 820 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@redhat.com>
> Signed-off-by: Davide Caratti <dcaratti@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@linux.intel.com>
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [MPTCP] Re: [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows
2020-10-06 16:26 ` Davide Caratti
@ 2020-10-09 15:35 ` Jakub Kicinski
-1 siblings, 0 replies; 12+ 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] 12+ messages in thread
* Re: [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows
@ 2020-10-09 15:35 ` Jakub Kicinski
0 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2020-10-09 15:35 UTC (permalink / raw)
To: Davide Caratti
Cc: netdev, David S. Miller, mptcp, Christoph Paasch, pabeni,
Matthieu Baerts
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@redhat.com>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Applied, thanks!
^ permalink raw reply [flat|nested] 12+ messages in thread
* [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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ 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; 12+ 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] 12+ messages in thread
end of thread, other threads:[~2020-10-09 15:36 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-06 23:58 [MPTCP] Re: [PATCH net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows Mat Martineau
2020-10-06 23:58 ` Mat Martineau
-- strict thread matches above, loose matches on Subject: below --
2020-10-09 15:35 [MPTCP] " Jakub Kicinski
2020-10-09 15:35 ` Jakub Kicinski
2020-10-06 16:50 [MPTCP] " Paolo Abeni
2020-10-06 16:35 Matthieu Baerts
2020-10-06 16:26 [MPTCP] " Davide Caratti
2020-10-06 16:26 ` Davide Caratti
2020-10-06 15:11 [MPTCP] " 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.