diff mbox series

[net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows

Message ID 70c96303d6d9931aae1b1028aed016d807df0e20.1602001119.git.dcaratti@redhat.com
State New
Headers show
Series [net] net: mptcp: make DACK4/DACK8 usage consistent among all subflows | expand

Commit Message

Davide Caratti Oct. 6, 2020, 4:26 p.m. UTC
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(-)

Comments

Mat Martineau Oct. 6, 2020, 11:58 p.m. UTC | #1
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
Jakub Kicinski Oct. 9, 2020, 3:35 p.m. UTC | #2
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!
diff mbox series

Patch

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 */