[PATCH mptcp-next v5 0/8] The infinite mapping support

[PATCH mptcp-next v5 0/8] The infinite mapping support

[Geliang Tang]

v5:
 - move last_retrans_seq from msk to mptcp_subflow_context

v4:
 - update patch 1 and patch 2

v3:
 - drop MPTCP_INFINITE_DONE flag
 - drop MAPPING_INFINITE
 - add mptcp_is_data_contiguous helper
 - add the fallback check
 - The u32 target testcase has not been completed yet.

v2:
 - add MPTCP_INFINITE_DONE flag
 - add MAPPING_INFINITE mapping status
 - add start_seq in the msk

v1:
 - add noncontiguous flag
 - add the mibs check
 - tag: export/20210904T080009

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/216

Geliang Tang (8):
  mptcp: track and update contiguous data status
  mptcp: add last_fully_acked_dss_start_seq in the msk
  mptcp: infinite mapping sending
  mptcp: add the fallback check
  mptcp: infinite mapping receiving
  mptcp: add mib for infinite map sending
  selftests: mptcp: add infinite map mibs check
  DO-NOT-MERGE: mptcp: mp_fail test

 include/net/mptcp.h                           |  3 +-
 net/mptcp/mib.c                               |  1 +
 net/mptcp/mib.h                               |  1 +
 net/mptcp/options.c                           |  2 +-
 net/mptcp/pm.c                                |  5 ++
 net/mptcp/protocol.c                          | 38 +++++++++++++
 net/mptcp/protocol.h                          | 21 +++++++
 net/mptcp/subflow.c                           | 55 +++++++++---------
 .../testing/selftests/net/mptcp/mptcp_join.sh | 56 +++++++++++++++++++
 9 files changed, 154 insertions(+), 28 deletions(-)

-- 
2.31.1

[PATCH mptcp-next v5 1/8] mptcp: track and update contiguous data status

[Geliang Tang]

This patch added a new member last_retrans_seq in mptcp_subflow_context
to track the last retransmitting sequence number.

Add a new helper named mptcp_is_data_contiguous() to check whether the
data is contiguous on a subflow.

When a bad checksum is detected and a single contiguous subflow is in use,
don't send RST + MP_FAIL, send data_ack + MP_FAIL instead.

Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 net/mptcp/protocol.c |  6 ++++++
 net/mptcp/protocol.h |  8 ++++++++
 net/mptcp/subflow.c  | 12 ++++++------
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index f8ad049d6941..cf8cccfefb51 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -729,6 +729,9 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk)
 	if (unlikely(subflow->disposable))
 		return;
 
+	if (!subflow->last_retrans_seq || mptcp_is_data_contiguous(ssk))
+		subflow->last_retrans_seq = tcp_sk(ssk)->snd_una - 1;
+
 	ssk_rbuf = READ_ONCE(ssk->sk_rcvbuf);
 	sk_rbuf = READ_ONCE(sk->sk_rcvbuf);
 	if (unlikely(ssk_rbuf > sk_rbuf))
@@ -2415,6 +2418,7 @@ static void mptcp_check_fastclose(struct mptcp_sock *msk)
 static void __mptcp_retrans(struct sock *sk)
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);
+	struct mptcp_subflow_context *subflow;
 	struct mptcp_sendmsg_info info = {};
 	struct mptcp_data_frag *dfrag;
 	size_t copied = 0;
@@ -2464,6 +2468,8 @@ static void __mptcp_retrans(struct sock *sk)
 		dfrag->already_sent = max(dfrag->already_sent, info.sent);
 		tcp_push(ssk, 0, info.mss_now, tcp_sk(ssk)->nonagle,
 			 info.size_goal);
+		subflow = mptcp_subflow_ctx(ssk);
+		subflow->last_retrans_seq = tcp_sk(ssk)->snd_nxt;
 	}
 
 	release_sock(ssk);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 7379ab580a7e..e090a9244f8b 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -416,6 +416,7 @@ struct mptcp_subflow_context {
 	u32	map_data_len;
 	__wsum	map_data_csum;
 	u32	map_csum_len;
+	u32	last_retrans_seq;
 	u32	request_mptcp : 1,  /* send MP_CAPABLE */
 		request_join : 1,   /* send MP_JOIN */
 		request_bkup : 1,
@@ -625,6 +626,13 @@ static inline bool mptcp_has_another_subflow(struct sock *ssk)
 	return false;
 }
 
+static inline bool mptcp_is_data_contiguous(struct sock *ssk)
+{
+	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
+
+	return before(subflow->last_retrans_seq, tcp_sk(ssk)->snd_una);
+}
+
 void __init mptcp_proto_init(void);
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
 int __init mptcp_proto_v6_init(void);
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 6172f380dfb7..a8f46a48feb1 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1166,15 +1166,15 @@ static bool subflow_check_data_avail(struct sock *ssk)
 fallback:
 	/* RFC 8684 section 3.7. */
 	if (subflow->send_mp_fail) {
-		if (mptcp_has_another_subflow(ssk)) {
+		if (mptcp_has_another_subflow(ssk) || !mptcp_is_data_contiguous(ssk)) {
+			ssk->sk_err = EBADMSG;
+			tcp_set_state(ssk, TCP_CLOSE);
+			subflow->reset_transient = 0;
+			subflow->reset_reason = MPTCP_RST_EMIDDLEBOX;
+			tcp_send_active_reset(ssk, GFP_ATOMIC);
 			while ((skb = skb_peek(&ssk->sk_receive_queue)))
 				sk_eat_skb(ssk, skb);
 		}
-		ssk->sk_err = EBADMSG;
-		tcp_set_state(ssk, TCP_CLOSE);
-		subflow->reset_transient = 0;
-		subflow->reset_reason = MPTCP_RST_EMIDDLEBOX;
-		tcp_send_active_reset(ssk, GFP_ATOMIC);
 		WRITE_ONCE(subflow->data_avail, 0);
 		return true;
 	}
-- 
2.31.1

[PATCH mptcp-next v5 2/8] mptcp: add last_fully_acked_dss_start_seq in the msk

[Geliang Tang]

This patch added a new member named last_fully_acked_dss_start_seq to the
msk to keep track of the beginning of the last fully-acked data segment.
This would be updated in __mptcp_clean_una.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 net/mptcp/protocol.c | 3 +++
 net/mptcp/protocol.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index cf8cccfefb51..1cf43073845a 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1074,6 +1074,7 @@ static void __mptcp_clean_una(struct sock *sk)
 			WRITE_ONCE(msk->first_pending, mptcp_send_next(sk));
 		}
 
+		msk->last_fully_acked_dss_start_seq = dfrag->data_seq;
 		dfrag_clear(sk, dfrag);
 		cleaned = true;
 	}
@@ -2895,6 +2896,7 @@ struct sock *mptcp_sk_clone(const struct sock *sk,
 	msk->snd_una = msk->write_seq;
 	msk->wnd_end = msk->snd_nxt + req->rsk_rcv_wnd;
 	msk->setsockopt_seq = mptcp_sk(sk)->setsockopt_seq;
+	msk->last_fully_acked_dss_start_seq = subflow_req->idsn - 1;
 
 	if (mp_opt->suboptions & OPTIONS_MPTCP_MPC) {
 		msk->can_ack = true;
@@ -3151,6 +3153,7 @@ void mptcp_finish_connect(struct sock *ssk)
 	WRITE_ONCE(msk->rcv_wnd_sent, ack_seq);
 	WRITE_ONCE(msk->can_ack, 1);
 	WRITE_ONCE(msk->snd_una, msk->write_seq);
+	WRITE_ONCE(msk->last_fully_acked_dss_start_seq, subflow->idsn - 1);
 
 	mptcp_pm_new_connection(msk, ssk, 0);
 
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index e090a9244f8b..e9064fec0ed2 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -224,6 +224,7 @@ struct mptcp_sock {
 	u64		remote_key;
 	u64		write_seq;
 	u64		snd_nxt;
+	u64		last_fully_acked_dss_start_seq;
 	u64		ack_seq;
 	u64		rcv_wnd_sent;
 	u64		rcv_data_fin_seq;
-- 
2.31.1

[PATCH mptcp-next v5 3/8] mptcp: infinite mapping sending

[Geliang Tang]

This patch added the infinite mapping sending logic.

Added a new flag send_infinite_map in struct mptcp_subflow_context. Set
it true when a single contiguous subflow is in use in
mptcp_pm_mp_fail_received.

In mptcp_sendmsg_frag, if this flag is true, call the new function
mptcp_update_infinite_map to set the infinite mapping.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 include/net/mptcp.h  |  3 ++-
 net/mptcp/options.c  |  2 +-
 net/mptcp/pm.c       |  5 +++++
 net/mptcp/protocol.c | 19 +++++++++++++++++++
 net/mptcp/protocol.h | 12 ++++++++++++
 5 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index f83fa48408b3..29e930540ea2 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -35,7 +35,8 @@ struct mptcp_ext {
 			frozen:1,
 			reset_transient:1;
 	u8		reset_reason:4,
-			csum_reqd:1;
+			csum_reqd:1,
+			infinite_map:1;
 };
 
 #define MPTCP_RM_IDS_MAX	8
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 422f4acfb3e6..f4591421ed22 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -816,7 +816,7 @@ bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
 
 	opts->suboptions = 0;
 
-	if (unlikely(__mptcp_check_fallback(msk)))
+	if (unlikely(__mptcp_check_fallback(msk) && !mptcp_check_infinite_map(skb)))
 		return false;
 
 	if (unlikely(skb && TCP_SKB_CB(skb)->tcp_flags & TCPHDR_RST)) {
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 6ab386ff3294..27d43a613e9a 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -251,7 +251,12 @@ void mptcp_pm_mp_prio_received(struct sock *sk, u8 bkup)
 
 void mptcp_pm_mp_fail_received(struct sock *sk, u64 fail_seq)
 {
+	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
+
 	pr_debug("fail_seq=%llu", fail_seq);
+
+	if (!mptcp_has_another_subflow(sk) && mptcp_is_data_contiguous(sk))
+		subflow->send_infinite_map = 1;
 }
 
 /* path manager helpers */
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 1cf43073845a..60953b61b3c9 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1277,6 +1277,23 @@ static void mptcp_update_data_checksum(struct sk_buff *skb, int added)
 	mpext->csum = csum_fold(csum_block_add(csum, skb_checksum(skb, offset, added, 0), offset));
 }
 
+static void mptcp_update_infinite_map(struct mptcp_sock *msk, struct sock *ssk,
+				      struct mptcp_ext *mpext)
+{
+	if (!mpext)
+		return;
+
+	mpext->infinite_map = 1;
+	mpext->data_seq = READ_ONCE(msk->last_fully_acked_dss_start_seq);
+	mpext->subflow_seq = 0;
+	mpext->data_len = 0;
+	mpext->csum = 0;
+
+	mptcp_subflow_ctx(ssk)->send_infinite_map = 0;
+	pr_fallback(msk);
+	__mptcp_do_fallback(msk);
+}
+
 static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 			      struct mptcp_data_frag *dfrag,
 			      struct mptcp_sendmsg_info *info)
@@ -1409,6 +1426,8 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 out:
 	if (READ_ONCE(msk->csum_enabled))
 		mptcp_update_data_checksum(skb, copy);
+	if (mptcp_subflow_ctx(ssk)->send_infinite_map)
+		mptcp_update_infinite_map(msk, ssk, mpext);
 	mptcp_subflow_ctx(ssk)->rel_write_seq += copy;
 	return copy;
 }
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index e9064fec0ed2..005c18e81e18 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -433,6 +433,7 @@ struct mptcp_subflow_context {
 		backup : 1,
 		send_mp_prio : 1,
 		send_mp_fail : 1,
+		send_infinite_map : 1,
 		rx_eof : 1,
 		can_ack : 1,        /* only after processing the remote a key */
 		disposable : 1,	    /* ctx can be free at ulp release time */
@@ -876,6 +877,17 @@ static inline void mptcp_do_fallback(struct sock *sk)
 
 #define pr_fallback(a) pr_debug("%s:fallback to TCP (msk=%p)", __func__, a)
 
+static inline bool mptcp_check_infinite_map(struct sk_buff *skb)
+{
+	struct mptcp_ext *mpext;
+
+	mpext = skb ? mptcp_get_ext(skb) : NULL;
+	if (mpext && mpext->infinite_map)
+		return true;
+
+	return false;
+}
+
 static inline bool subflow_simultaneous_connect(struct sock *sk)
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
-- 
2.31.1

[PATCH mptcp-next v5 4/8] mptcp: add the fallback check

[Geliang Tang]

This patch added the fallback check in subflow_check_data_avail. Only do
the fallback when the msk isn't fallen back yet.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 net/mptcp/subflow.c | 46 +++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index a8f46a48feb1..023fb95e1fa2 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1164,35 +1164,37 @@ static bool subflow_check_data_avail(struct sock *ssk)
 	return false;
 
 fallback:
-	/* RFC 8684 section 3.7. */
-	if (subflow->send_mp_fail) {
-		if (mptcp_has_another_subflow(ssk) || !mptcp_is_data_contiguous(ssk)) {
+	if (!__mptcp_check_fallback(msk)) {
+		/* RFC 8684 section 3.7. */
+		if (subflow->send_mp_fail) {
+			if (mptcp_has_another_subflow(ssk) || !mptcp_is_data_contiguous(ssk)) {
+				ssk->sk_err = EBADMSG;
+				tcp_set_state(ssk, TCP_CLOSE);
+				subflow->reset_transient = 0;
+				subflow->reset_reason = MPTCP_RST_EMIDDLEBOX;
+				tcp_send_active_reset(ssk, GFP_ATOMIC);
+				while ((skb = skb_peek(&ssk->sk_receive_queue)))
+					sk_eat_skb(ssk, skb);
+			}
+			WRITE_ONCE(subflow->data_avail, 0);
+			return true;
+		}
+
+		if (subflow->mp_join || subflow->fully_established) {
+			/* fatal protocol error, close the socket.
+			 * subflow_error_report() will introduce the appropriate barriers
+			 */
 			ssk->sk_err = EBADMSG;
 			tcp_set_state(ssk, TCP_CLOSE);
 			subflow->reset_transient = 0;
-			subflow->reset_reason = MPTCP_RST_EMIDDLEBOX;
+			subflow->reset_reason = MPTCP_RST_EMPTCP;
 			tcp_send_active_reset(ssk, GFP_ATOMIC);
-			while ((skb = skb_peek(&ssk->sk_receive_queue)))
-				sk_eat_skb(ssk, skb);
+			WRITE_ONCE(subflow->data_avail, 0);
+			return false;
 		}
-		WRITE_ONCE(subflow->data_avail, 0);
-		return true;
-	}
 
-	if (subflow->mp_join || subflow->fully_established) {
-		/* fatal protocol error, close the socket.
-		 * subflow_error_report() will introduce the appropriate barriers
-		 */
-		ssk->sk_err = EBADMSG;
-		tcp_set_state(ssk, TCP_CLOSE);
-		subflow->reset_transient = 0;
-		subflow->reset_reason = MPTCP_RST_EMPTCP;
-		tcp_send_active_reset(ssk, GFP_ATOMIC);
-		WRITE_ONCE(subflow->data_avail, 0);
-		return false;
+		__mptcp_do_fallback(msk);
 	}
-
-	__mptcp_do_fallback(msk);
 	skb = skb_peek(&ssk->sk_receive_queue);
 	subflow->map_valid = 1;
 	subflow->map_seq = READ_ONCE(msk->ack_seq);
-- 
2.31.1

[PATCH mptcp-next v5 5/8] mptcp: infinite mapping receiving

[Geliang Tang]

This patch added the infinite mapping receiving logic. When the infinite
mapping is received, set the map_data_len of the subflow to 0.

In subflow_check_data_avail, only reset the subflow when the map_data_len
of the subflow is non-zero.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 net/mptcp/subflow.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 023fb95e1fa2..81ebefdef88d 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -968,6 +968,7 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
 	data_len = mpext->data_len;
 	if (data_len == 0) {
 		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_INFINITEMAPRX);
+		subflow->map_data_len = 0;
 		return MAPPING_INVALID;
 	}
 
@@ -1180,7 +1181,7 @@ static bool subflow_check_data_avail(struct sock *ssk)
 			return true;
 		}
 
-		if (subflow->mp_join || subflow->fully_established) {
+		if ((subflow->mp_join || subflow->fully_established) && subflow->map_data_len) {
 			/* fatal protocol error, close the socket.
 			 * subflow_error_report() will introduce the appropriate barriers
 			 */
-- 
2.31.1

[PATCH mptcp-next v5 6/8] mptcp: add mib for infinite map sending

[Geliang Tang]

This patch added a new mib named MPTCP_MIB_INFINITEMAPTX, increase it
when a infinite mapping has been sent out.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 net/mptcp/mib.c      | 1 +
 net/mptcp/mib.h      | 1 +
 net/mptcp/protocol.c | 1 +
 3 files changed, 3 insertions(+)

diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
index b21ff9be04c6..ab55afdcae22 100644
--- a/net/mptcp/mib.c
+++ b/net/mptcp/mib.c
@@ -24,6 +24,7 @@ static const struct snmp_mib mptcp_snmp_list[] = {
 	SNMP_MIB_ITEM("MPJoinAckRx", MPTCP_MIB_JOINACKRX),
 	SNMP_MIB_ITEM("MPJoinAckHMacFailure", MPTCP_MIB_JOINACKMAC),
 	SNMP_MIB_ITEM("DSSNotMatching", MPTCP_MIB_DSSNOMATCH),
+	SNMP_MIB_ITEM("InfiniteMapTx", MPTCP_MIB_INFINITEMAPTX),
 	SNMP_MIB_ITEM("InfiniteMapRx", MPTCP_MIB_INFINITEMAPRX),
 	SNMP_MIB_ITEM("DSSNoMatchTCP", MPTCP_MIB_DSSTCPMISMATCH),
 	SNMP_MIB_ITEM("DataCsumErr", MPTCP_MIB_DATACSUMERR),
diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
index ecd3d8b117e0..7901f1338d15 100644
--- a/net/mptcp/mib.h
+++ b/net/mptcp/mib.h
@@ -17,6 +17,7 @@ enum linux_mptcp_mib_field {
 	MPTCP_MIB_JOINACKRX,		/* Received an ACK + MP_JOIN */
 	MPTCP_MIB_JOINACKMAC,		/* HMAC was wrong on ACK + MP_JOIN */
 	MPTCP_MIB_DSSNOMATCH,		/* Received a new mapping that did not match the previous one */
+	MPTCP_MIB_INFINITEMAPTX,	/* Sent an infinite mapping */
 	MPTCP_MIB_INFINITEMAPRX,	/* Received an infinite mapping */
 	MPTCP_MIB_DSSTCPMISMATCH,	/* DSS-mapping did not map with TCP's sequence numbers */
 	MPTCP_MIB_DATACSUMERR,		/* The data checksum fail */
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 60953b61b3c9..c735adc6b835 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1289,6 +1289,7 @@ static void mptcp_update_infinite_map(struct mptcp_sock *msk, struct sock *ssk,
 	mpext->data_len = 0;
 	mpext->csum = 0;
 
+	MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_INFINITEMAPTX);
 	mptcp_subflow_ctx(ssk)->send_infinite_map = 0;
 	pr_fallback(msk);
 	__mptcp_do_fallback(msk);
-- 
2.31.1

[PATCH mptcp-next v5 7/8] selftests: mptcp: add infinite map mibs check

[Geliang Tang]

This patch added a function chk_infi_nr to check the mibs for the
infinite mapping.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 255793c5ac4f..fe0c8f3164a7 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -615,6 +615,43 @@ chk_fail_nr()
 	fi
 }
 
+chk_infi_nr()
+{
+	local mp_infi_nr_tx=$1
+	local mp_infi_nr_rx=$2
+	local count
+	local dump_stats
+
+	printf "%-39s %s" " " "itx"
+	count=`ip netns exec $ns1 nstat -as | grep InfiniteMapTx | awk '{print $2}'`
+	[ -z "$count" ] && count=0
+	if [ "$count" != "$mp_infi_nr_tx" ]; then
+		echo "[fail] got $count infinite map[s] TX expected $mp_infi_nr_tx"
+		ret=1
+		dump_stats=1
+	else
+		echo -n "[ ok ]"
+	fi
+
+	echo -n " - irx   "
+	count=`ip netns exec $ns2 nstat -as | grep InfiniteMapRx | awk '{print $2}'`
+	[ -z "$count" ] && count=0
+	if [ "$count" != "$mp_infi_nr_rx" ]; then
+		echo "[fail] got $count infinite map[s] RX expected $mp_infi_nr_rx"
+		ret=1
+		dump_stats=1
+	else
+		echo "[ ok ]"
+	fi
+
+	if [ "${dump_stats}" = 1 ]; then
+		echo Server ns stats
+		ip netns exec $ns1 nstat -as | grep MPTcp
+		echo Client ns stats
+		ip netns exec $ns2 nstat -as | grep MPTcp
+	fi
+}
+
 chk_join_nr()
 {
 	local msg="$1"
@@ -665,6 +702,7 @@ chk_join_nr()
 	if [ $checksum -eq 1 ]; then
 		chk_csum_nr
 		chk_fail_nr 0 0
+		chk_infi_nr 0 0
 	fi
 }
 
-- 
2.31.1

[PATCH mptcp-next v5 8/8] DO-NOT-MERGE: mptcp: mp_fail test

[Geliang Tang]

./mptcp_join.sh -Cf

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
 net/mptcp/protocol.c                           |  9 +++++++++
 .../testing/selftests/net/mptcp/mptcp_join.sh  | 18 ++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index c735adc6b835..dec87c05361b 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1295,6 +1295,8 @@ static void mptcp_update_infinite_map(struct mptcp_sock *msk, struct sock *ssk,
 	__mptcp_do_fallback(msk);
 }
 
+static int j;
+
 static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 			      struct mptcp_data_frag *dfrag,
 			      struct mptcp_sendmsg_info *info)
@@ -1429,6 +1431,13 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 		mptcp_update_data_checksum(skb, copy);
 	if (mptcp_subflow_ctx(ssk)->send_infinite_map)
 		mptcp_update_infinite_map(msk, ssk, mpext);
+
+	pr_debug("%s j=%d", __func__, j++);
+	if (j == 20)
+		skb->data_len = 1;
+	if (j > 40)
+		j = 0;
+
 	mptcp_subflow_ctx(ssk)->rel_write_seq += copy;
 	return copy;
 }
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index fe0c8f3164a7..38663f6373b8 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -977,6 +977,24 @@ chk_link_usage()
 
 subflows_tests()
 {
+	# 1 subflow
+	reset
+	ip netns exec $ns1 ./pm_nl_ctl limits 0 2
+	ip netns exec $ns2 ./pm_nl_ctl limits 0 2
+	run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow
+	chk_join_nr "1 subflow" 0 0 0
+
+	exit
+
+	# multiple subflows
+	reset
+	ip netns exec $ns1 ./pm_nl_ctl limits 0 2
+	ip netns exec $ns2 ./pm_nl_ctl limits 0 2
+	ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
+	ip netns exec $ns2 ./pm_nl_ctl add 10.0.2.2 flags subflow
+	run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow
+	chk_join_nr "multiple subflows" 2 2 2
+
 	reset
 	run_tests $ns1 $ns2 10.0.1.1
 	chk_join_nr "no JOIN" "0" "0" "0"
-- 
2.31.1

Re: [PATCH mptcp-next v5 3/8] mptcp: infinite mapping sending

[Mat Martineau]

On Sun, 26 Sep 2021, Geliang Tang wrote:

> This patch added the infinite mapping sending logic.
>
> Added a new flag send_infinite_map in struct mptcp_subflow_context. Set
> it true when a single contiguous subflow is in use in
> mptcp_pm_mp_fail_received.
>
> In mptcp_sendmsg_frag, if this flag is true, call the new function
> mptcp_update_infinite_map to set the infinite mapping.
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
> include/net/mptcp.h  |  3 ++-
> net/mptcp/options.c  |  2 +-
> net/mptcp/pm.c       |  5 +++++
> net/mptcp/protocol.c | 19 +++++++++++++++++++
> net/mptcp/protocol.h | 12 ++++++++++++
> 5 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index f83fa48408b3..29e930540ea2 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -35,7 +35,8 @@ struct mptcp_ext {
> 			frozen:1,
> 			reset_transient:1;
> 	u8		reset_reason:4,
> -			csum_reqd:1;
> +			csum_reqd:1,
> +			infinite_map:1;
> };
>
> #define MPTCP_RM_IDS_MAX	8
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 422f4acfb3e6..f4591421ed22 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -816,7 +816,7 @@ bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
>
> 	opts->suboptions = 0;
>
> -	if (unlikely(__mptcp_check_fallback(msk)))
> +	if (unlikely(__mptcp_check_fallback(msk) && !mptcp_check_infinite_map(skb)))
> 		return false;
>
> 	if (unlikely(skb && TCP_SKB_CB(skb)->tcp_flags & TCPHDR_RST)) {
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 6ab386ff3294..27d43a613e9a 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -251,7 +251,12 @@ void mptcp_pm_mp_prio_received(struct sock *sk, u8 bkup)
>
> void mptcp_pm_mp_fail_received(struct sock *sk, u64 fail_seq)
> {
> +	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
> +
> 	pr_debug("fail_seq=%llu", fail_seq);
> +
> +	if (!mptcp_has_another_subflow(sk) && mptcp_is_data_contiguous(sk))
> +		subflow->send_infinite_map = 1;
> }
>
> /* path manager helpers */
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 1cf43073845a..60953b61b3c9 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1277,6 +1277,23 @@ static void mptcp_update_data_checksum(struct sk_buff *skb, int added)
> 	mpext->csum = csum_fold(csum_block_add(csum, skb_checksum(skb, offset, added, 0), offset));
> }
>
> +static void mptcp_update_infinite_map(struct mptcp_sock *msk, struct sock *ssk,
> +				      struct mptcp_ext *mpext)
> +{
> +	if (!mpext)
> +		return;
> +
> +	mpext->infinite_map = 1;
> +	mpext->data_seq = READ_ONCE(msk->last_fully_acked_dss_start_seq);

I went back to double check what the RFC says about the contents of an 
infinite mapping DSS option, since I had asked for the data_seq to be 
assigned this way based on this text in section 3.7:

"This infinite mapping will be a DSS option (Section 3.3) on the first new 
packet, containing a Data Sequence Mapping that acts retroactively, 
referring to the start of the subflow sequence number of the most recent 
segment that was known to be delivered intact (i.e., was successfully 
DATA_ACKed)."

The meaning of the last half of that sentence is not 100% obvious. I 
initially thought it was trying to specify a sequence number to place in 
the DSS option, but maybe it's only defining what the "infinite mapping" 
means to the receiver. The very end of section 3.3.1 says:

"An infinite mapping can be used to fall back to regular TCP by mapping 
the subflow-level data to the connection-level data for the remainder of 
the connection (see Section 3.7).  This is achieved by setting the 
Data-Level Length field of the DSS option to the reserved value of 0. 
The checksum, in such a case, will also be set to 0."

Considering that, and the multipath-tcp.org code, the only required 
fields to set in an infinite mapping DSS would be the data_len (0), and 
the checksum if enabled.

Christoph (or any other protocol gurus), can you confirm this 
interpretation? That would let us drop patch 2 in this series.

Thanks!

Mat


> +	mpext->subflow_seq = 0;
> +	mpext->data_len = 0;
> +	mpext->csum = 0;
> +
> +	mptcp_subflow_ctx(ssk)->send_infinite_map = 0;
> +	pr_fallback(msk);
> +	__mptcp_do_fallback(msk);
> +}
> +
> static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
> 			      struct mptcp_data_frag *dfrag,
> 			      struct mptcp_sendmsg_info *info)
> @@ -1409,6 +1426,8 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
> out:
> 	if (READ_ONCE(msk->csum_enabled))
> 		mptcp_update_data_checksum(skb, copy);
> +	if (mptcp_subflow_ctx(ssk)->send_infinite_map)
> +		mptcp_update_infinite_map(msk, ssk, mpext);
> 	mptcp_subflow_ctx(ssk)->rel_write_seq += copy;
> 	return copy;
> }
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index e9064fec0ed2..005c18e81e18 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -433,6 +433,7 @@ struct mptcp_subflow_context {
> 		backup : 1,
> 		send_mp_prio : 1,
> 		send_mp_fail : 1,
> +		send_infinite_map : 1,
> 		rx_eof : 1,
> 		can_ack : 1,        /* only after processing the remote a key */
> 		disposable : 1,	    /* ctx can be free at ulp release time */
> @@ -876,6 +877,17 @@ static inline void mptcp_do_fallback(struct sock *sk)
>
> #define pr_fallback(a) pr_debug("%s:fallback to TCP (msk=%p)", __func__, a)
>
> +static inline bool mptcp_check_infinite_map(struct sk_buff *skb)
> +{
> +	struct mptcp_ext *mpext;
> +
> +	mpext = skb ? mptcp_get_ext(skb) : NULL;
> +	if (mpext && mpext->infinite_map)
> +		return true;
> +
> +	return false;
> +}
> +
> static inline bool subflow_simultaneous_connect(struct sock *sk)
> {
> 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
> -- 
> 2.31.1
>
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next v5 1/8] mptcp: track and update contiguous data status

[Mat Martineau]

On Sun, 26 Sep 2021, Geliang Tang wrote:

> This patch added a new member last_retrans_seq in mptcp_subflow_context
> to track the last retransmitting sequence number.
>
> Add a new helper named mptcp_is_data_contiguous() to check whether the
> data is contiguous on a subflow.
>
> When a bad checksum is detected and a single contiguous subflow is in use,
> don't send RST + MP_FAIL, send data_ack + MP_FAIL instead.
>
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>

Hi Geliang -

I know there's been a lot of churn in this part of the patch series, 
hopefully we can simplify a little to allow us to merge the first step of 
infinite mapping send/recv.

In v3 and v4 reviews, I suggested tracking the subflow sequence number 
like you do in this patch. After the discussion in the 2021-09-23 meeting, 
proper tracking of contiguous data appeared even more complicated, so 
Paolo had a suggestion to allow fallback in one simple-to-check case: Only 
do infinite mapping fallback if there is a single subflow AND there have 
been no retransmissions AND there have never been any MP_JOINs.

That can be tracked with a "allow_infinite_fallback" bool in mptcp_sock, 
which gets initialized to 'true' when the connection begins and is set to 
'false' on any retransmit or successful MP_JOIN.

Paolo, is that an accurate summary?


It's important that this type of fallback only happens when it is 
guaranteed that the stream is fully in-sequence, otherwise the 
application's data gets corrupted. The above suggestion lets us merge 
working infinite mapping code, and then figure out the more complicated 
"fallback after retransmissions or join" logic in a separate patch series.


Thanks,

Mat



> ---
> net/mptcp/protocol.c |  6 ++++++
> net/mptcp/protocol.h |  8 ++++++++
> net/mptcp/subflow.c  | 12 ++++++------
> 3 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index f8ad049d6941..cf8cccfefb51 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -729,6 +729,9 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk)
> 	if (unlikely(subflow->disposable))
> 		return;
>
> +	if (!subflow->last_retrans_seq || mptcp_is_data_contiguous(ssk))
> +		subflow->last_retrans_seq = tcp_sk(ssk)->snd_una - 1;
> +
> 	ssk_rbuf = READ_ONCE(ssk->sk_rcvbuf);
> 	sk_rbuf = READ_ONCE(sk->sk_rcvbuf);
> 	if (unlikely(ssk_rbuf > sk_rbuf))
> @@ -2415,6 +2418,7 @@ static void mptcp_check_fastclose(struct mptcp_sock *msk)
> static void __mptcp_retrans(struct sock *sk)
> {
> 	struct mptcp_sock *msk = mptcp_sk(sk);
> +	struct mptcp_subflow_context *subflow;
> 	struct mptcp_sendmsg_info info = {};
> 	struct mptcp_data_frag *dfrag;
> 	size_t copied = 0;
> @@ -2464,6 +2468,8 @@ static void __mptcp_retrans(struct sock *sk)
> 		dfrag->already_sent = max(dfrag->already_sent, info.sent);
> 		tcp_push(ssk, 0, info.mss_now, tcp_sk(ssk)->nonagle,
> 			 info.size_goal);
> +		subflow = mptcp_subflow_ctx(ssk);
> +		subflow->last_retrans_seq = tcp_sk(ssk)->snd_nxt;
> 	}
>
> 	release_sock(ssk);
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 7379ab580a7e..e090a9244f8b 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -416,6 +416,7 @@ struct mptcp_subflow_context {
> 	u32	map_data_len;
> 	__wsum	map_data_csum;
> 	u32	map_csum_len;
> +	u32	last_retrans_seq;
> 	u32	request_mptcp : 1,  /* send MP_CAPABLE */
> 		request_join : 1,   /* send MP_JOIN */
> 		request_bkup : 1,
> @@ -625,6 +626,13 @@ static inline bool mptcp_has_another_subflow(struct sock *ssk)
> 	return false;
> }
>
> +static inline bool mptcp_is_data_contiguous(struct sock *ssk)
> +{
> +	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> +
> +	return before(subflow->last_retrans_seq, tcp_sk(ssk)->snd_una);
> +}
> +
> void __init mptcp_proto_init(void);
> #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> int __init mptcp_proto_v6_init(void);
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 6172f380dfb7..a8f46a48feb1 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -1166,15 +1166,15 @@ static bool subflow_check_data_avail(struct sock *ssk)
> fallback:
> 	/* RFC 8684 section 3.7. */
> 	if (subflow->send_mp_fail) {
> -		if (mptcp_has_another_subflow(ssk)) {
> +		if (mptcp_has_another_subflow(ssk) || !mptcp_is_data_contiguous(ssk)) {
> +			ssk->sk_err = EBADMSG;
> +			tcp_set_state(ssk, TCP_CLOSE);
> +			subflow->reset_transient = 0;
> +			subflow->reset_reason = MPTCP_RST_EMIDDLEBOX;
> +			tcp_send_active_reset(ssk, GFP_ATOMIC);
> 			while ((skb = skb_peek(&ssk->sk_receive_queue)))
> 				sk_eat_skb(ssk, skb);
> 		}
> -		ssk->sk_err = EBADMSG;
> -		tcp_set_state(ssk, TCP_CLOSE);
> -		subflow->reset_transient = 0;
> -		subflow->reset_reason = MPTCP_RST_EMIDDLEBOX;
> -		tcp_send_active_reset(ssk, GFP_ATOMIC);
> 		WRITE_ONCE(subflow->data_avail, 0);
> 		return true;
> 	}
> -- 
> 2.31.1
>
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next v5 1/8] mptcp: track and update contiguous data status

[Geliang Tang]

Mat Martineau <mathew.j.martineau@linux.intel.com> 于2021年9月28日周二 上午8:52写道：
>
> On Sun, 26 Sep 2021, Geliang Tang wrote:
>
> > This patch added a new member last_retrans_seq in mptcp_subflow_context
> > to track the last retransmitting sequence number.
> >
> > Add a new helper named mptcp_is_data_contiguous() to check whether the
> > data is contiguous on a subflow.
> >
> > When a bad checksum is detected and a single contiguous subflow is in use,
> > don't send RST + MP_FAIL, send data_ack + MP_FAIL instead.
> >
> > Suggested-by: Paolo Abeni <pabeni@redhat.com>
> > Signed-off-by: Geliang Tang <geliangtang@gmail.com>
>
> Hi Geliang -
>
> I know there's been a lot of churn in this part of the patch series,
> hopefully we can simplify a little to allow us to merge the first step of
> infinite mapping send/recv.

Hi Mat, Paolo, Matt,

I prefer to continue doing more iterations of this series instead of
merging part of them first. There's still some work to do in it, but it's
nearly finished. I'll send a v6 recently.

And the infinite mapping support is the last issue assigned to me on
GitHub. So I'm thinking about what should I do next. Could you give me
some suggestions?

Thanks,
-Geliang

>
> In v3 and v4 reviews, I suggested tracking the subflow sequence number
> like you do in this patch. After the discussion in the 2021-09-23 meeting,
> proper tracking of contiguous data appeared even more complicated, so
> Paolo had a suggestion to allow fallback in one simple-to-check case: Only
> do infinite mapping fallback if there is a single subflow AND there have
> been no retransmissions AND there have never been any MP_JOINs.
>
> That can be tracked with a "allow_infinite_fallback" bool in mptcp_sock,
> which gets initialized to 'true' when the connection begins and is set to
> 'false' on any retransmit or successful MP_JOIN.
>
> Paolo, is that an accurate summary?
>
>
> It's important that this type of fallback only happens when it is
> guaranteed that the stream is fully in-sequence, otherwise the
> application's data gets corrupted. The above suggestion lets us merge
> working infinite mapping code, and then figure out the more complicated
> "fallback after retransmissions or join" logic in a separate patch series.
>
>
> Thanks,
>
> Mat
>
>
>
> > ---
> > net/mptcp/protocol.c |  6 ++++++
> > net/mptcp/protocol.h |  8 ++++++++
> > net/mptcp/subflow.c  | 12 ++++++------
> > 3 files changed, 20 insertions(+), 6 deletions(-)
> >
> > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> > index f8ad049d6941..cf8cccfefb51 100644
> > --- a/net/mptcp/protocol.c
> > +++ b/net/mptcp/protocol.c
> > @@ -729,6 +729,9 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk)
> >       if (unlikely(subflow->disposable))
> >               return;
> >
> > +     if (!subflow->last_retrans_seq || mptcp_is_data_contiguous(ssk))
> > +             subflow->last_retrans_seq = tcp_sk(ssk)->snd_una - 1;
> > +
> >       ssk_rbuf = READ_ONCE(ssk->sk_rcvbuf);
> >       sk_rbuf = READ_ONCE(sk->sk_rcvbuf);
> >       if (unlikely(ssk_rbuf > sk_rbuf))
> > @@ -2415,6 +2418,7 @@ static void mptcp_check_fastclose(struct mptcp_sock *msk)
> > static void __mptcp_retrans(struct sock *sk)
> > {
> >       struct mptcp_sock *msk = mptcp_sk(sk);
> > +     struct mptcp_subflow_context *subflow;
> >       struct mptcp_sendmsg_info info = {};
> >       struct mptcp_data_frag *dfrag;
> >       size_t copied = 0;
> > @@ -2464,6 +2468,8 @@ static void __mptcp_retrans(struct sock *sk)
> >               dfrag->already_sent = max(dfrag->already_sent, info.sent);
> >               tcp_push(ssk, 0, info.mss_now, tcp_sk(ssk)->nonagle,
> >                        info.size_goal);
> > +             subflow = mptcp_subflow_ctx(ssk);
> > +             subflow->last_retrans_seq = tcp_sk(ssk)->snd_nxt;
> >       }
> >
> >       release_sock(ssk);
> > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> > index 7379ab580a7e..e090a9244f8b 100644
> > --- a/net/mptcp/protocol.h
> > +++ b/net/mptcp/protocol.h
> > @@ -416,6 +416,7 @@ struct mptcp_subflow_context {
> >       u32     map_data_len;
> >       __wsum  map_data_csum;
> >       u32     map_csum_len;
> > +     u32     last_retrans_seq;
> >       u32     request_mptcp : 1,  /* send MP_CAPABLE */
> >               request_join : 1,   /* send MP_JOIN */
> >               request_bkup : 1,
> > @@ -625,6 +626,13 @@ static inline bool mptcp_has_another_subflow(struct sock *ssk)
> >       return false;
> > }
> >
> > +static inline bool mptcp_is_data_contiguous(struct sock *ssk)
> > +{
> > +     struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> > +
> > +     return before(subflow->last_retrans_seq, tcp_sk(ssk)->snd_una);
> > +}
> > +
> > void __init mptcp_proto_init(void);
> > #if IS_ENABLED(CONFIG_MPTCP_IPV6)
> > int __init mptcp_proto_v6_init(void);
> > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> > index 6172f380dfb7..a8f46a48feb1 100644
> > --- a/net/mptcp/subflow.c
> > +++ b/net/mptcp/subflow.c
> > @@ -1166,15 +1166,15 @@ static bool subflow_check_data_avail(struct sock *ssk)
> > fallback:
> >       /* RFC 8684 section 3.7. */
> >       if (subflow->send_mp_fail) {
> > -             if (mptcp_has_another_subflow(ssk)) {
> > +             if (mptcp_has_another_subflow(ssk) || !mptcp_is_data_contiguous(ssk)) {
> > +                     ssk->sk_err = EBADMSG;
> > +                     tcp_set_state(ssk, TCP_CLOSE);
> > +                     subflow->reset_transient = 0;
> > +                     subflow->reset_reason = MPTCP_RST_EMIDDLEBOX;
> > +                     tcp_send_active_reset(ssk, GFP_ATOMIC);
> >                       while ((skb = skb_peek(&ssk->sk_receive_queue)))
> >                               sk_eat_skb(ssk, skb);
> >               }
> > -             ssk->sk_err = EBADMSG;
> > -             tcp_set_state(ssk, TCP_CLOSE);
> > -             subflow->reset_transient = 0;
> > -             subflow->reset_reason = MPTCP_RST_EMIDDLEBOX;
> > -             tcp_send_active_reset(ssk, GFP_ATOMIC);
> >               WRITE_ONCE(subflow->data_avail, 0);
> >               return true;
> >       }
> > --
> > 2.31.1
> >
> >
> >
>
> --
> Mat Martineau
> Intel
>

Re: [PATCH mptcp-next v5 3/8] mptcp: infinite mapping sending

[Christoph Paasch]

Hello,

On 09/27/21 - 17:32, Mat Martineau wrote:
> On Sun, 26 Sep 2021, Geliang Tang wrote:
> 
> > This patch added the infinite mapping sending logic.
> > 
> > Added a new flag send_infinite_map in struct mptcp_subflow_context. Set
> > it true when a single contiguous subflow is in use in
> > mptcp_pm_mp_fail_received.
> > 
> > In mptcp_sendmsg_frag, if this flag is true, call the new function
> > mptcp_update_infinite_map to set the infinite mapping.
> > 
> > Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> > ---
> > include/net/mptcp.h  |  3 ++-
> > net/mptcp/options.c  |  2 +-
> > net/mptcp/pm.c       |  5 +++++
> > net/mptcp/protocol.c | 19 +++++++++++++++++++
> > net/mptcp/protocol.h | 12 ++++++++++++
> > 5 files changed, 39 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> > index f83fa48408b3..29e930540ea2 100644
> > --- a/include/net/mptcp.h
> > +++ b/include/net/mptcp.h
> > @@ -35,7 +35,8 @@ struct mptcp_ext {
> > 			frozen:1,
> > 			reset_transient:1;
> > 	u8		reset_reason:4,
> > -			csum_reqd:1;
> > +			csum_reqd:1,
> > +			infinite_map:1;
> > };
> > 
> > #define MPTCP_RM_IDS_MAX	8
> > diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> > index 422f4acfb3e6..f4591421ed22 100644
> > --- a/net/mptcp/options.c
> > +++ b/net/mptcp/options.c
> > @@ -816,7 +816,7 @@ bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
> > 
> > 	opts->suboptions = 0;
> > 
> > -	if (unlikely(__mptcp_check_fallback(msk)))
> > +	if (unlikely(__mptcp_check_fallback(msk) && !mptcp_check_infinite_map(skb)))
> > 		return false;
> > 
> > 	if (unlikely(skb && TCP_SKB_CB(skb)->tcp_flags & TCPHDR_RST)) {
> > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> > index 6ab386ff3294..27d43a613e9a 100644
> > --- a/net/mptcp/pm.c
> > +++ b/net/mptcp/pm.c
> > @@ -251,7 +251,12 @@ void mptcp_pm_mp_prio_received(struct sock *sk, u8 bkup)
> > 
> > void mptcp_pm_mp_fail_received(struct sock *sk, u64 fail_seq)
> > {
> > +	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
> > +
> > 	pr_debug("fail_seq=%llu", fail_seq);
> > +
> > +	if (!mptcp_has_another_subflow(sk) && mptcp_is_data_contiguous(sk))
> > +		subflow->send_infinite_map = 1;
> > }
> > 
> > /* path manager helpers */
> > diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> > index 1cf43073845a..60953b61b3c9 100644
> > --- a/net/mptcp/protocol.c
> > +++ b/net/mptcp/protocol.c
> > @@ -1277,6 +1277,23 @@ static void mptcp_update_data_checksum(struct sk_buff *skb, int added)
> > 	mpext->csum = csum_fold(csum_block_add(csum, skb_checksum(skb, offset, added, 0), offset));
> > }
> > 
> > +static void mptcp_update_infinite_map(struct mptcp_sock *msk, struct sock *ssk,
> > +				      struct mptcp_ext *mpext)
> > +{
> > +	if (!mpext)
> > +		return;
> > +
> > +	mpext->infinite_map = 1;
> > +	mpext->data_seq = READ_ONCE(msk->last_fully_acked_dss_start_seq);
> 
> I went back to double check what the RFC says about the contents of an
> infinite mapping DSS option, since I had asked for the data_seq to be
> assigned this way based on this text in section 3.7:
> 
> "This infinite mapping will be a DSS option (Section 3.3) on the first new
> packet, containing a Data Sequence Mapping that acts retroactively,
> referring to the start of the subflow sequence number of the most recent
> segment that was known to be delivered intact (i.e., was successfully
> DATA_ACKed)."
> 
> The meaning of the last half of that sentence is not 100% obvious. I
> initially thought it was trying to specify a sequence number to place in the
> DSS option, but maybe it's only defining what the "infinite mapping" means
> to the receiver. The very end of section 3.3.1 says:
> 
> "An infinite mapping can be used to fall back to regular TCP by mapping the
> subflow-level data to the connection-level data for the remainder of the
> connection (see Section 3.7).  This is achieved by setting the Data-Level
> Length field of the DSS option to the reserved value of 0. The checksum, in
> such a case, will also be set to 0."
> 
> Considering that, and the multipath-tcp.org code, the only required fields
> to set in an infinite mapping DSS would be the data_len (0), and the
> checksum if enabled.

I do believe that the relative subflow-sequence number is needed as well.
Otherwise the receiver may not be able to properly map the DSS to the actual
bytes in the payload.


Christoph

> 
> Christoph (or any other protocol gurus), can you confirm this
> interpretation? That would let us drop patch 2 in this series.
> 
> Thanks!
> 
> Mat
> 
> 
> > +	mpext->subflow_seq = 0;
> > +	mpext->data_len = 0;
> > +	mpext->csum = 0;
> > +
> > +	mptcp_subflow_ctx(ssk)->send_infinite_map = 0;
> > +	pr_fallback(msk);
> > +	__mptcp_do_fallback(msk);
> > +}
> > +
> > static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
> > 			      struct mptcp_data_frag *dfrag,
> > 			      struct mptcp_sendmsg_info *info)
> > @@ -1409,6 +1426,8 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
> > out:
> > 	if (READ_ONCE(msk->csum_enabled))
> > 		mptcp_update_data_checksum(skb, copy);
> > +	if (mptcp_subflow_ctx(ssk)->send_infinite_map)
> > +		mptcp_update_infinite_map(msk, ssk, mpext);
> > 	mptcp_subflow_ctx(ssk)->rel_write_seq += copy;
> > 	return copy;
> > }
> > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> > index e9064fec0ed2..005c18e81e18 100644
> > --- a/net/mptcp/protocol.h
> > +++ b/net/mptcp/protocol.h
> > @@ -433,6 +433,7 @@ struct mptcp_subflow_context {
> > 		backup : 1,
> > 		send_mp_prio : 1,
> > 		send_mp_fail : 1,
> > +		send_infinite_map : 1,
> > 		rx_eof : 1,
> > 		can_ack : 1,        /* only after processing the remote a key */
> > 		disposable : 1,	    /* ctx can be free at ulp release time */
> > @@ -876,6 +877,17 @@ static inline void mptcp_do_fallback(struct sock *sk)
> > 
> > #define pr_fallback(a) pr_debug("%s:fallback to TCP (msk=%p)", __func__, a)
> > 
> > +static inline bool mptcp_check_infinite_map(struct sk_buff *skb)
> > +{
> > +	struct mptcp_ext *mpext;
> > +
> > +	mpext = skb ? mptcp_get_ext(skb) : NULL;
> > +	if (mpext && mpext->infinite_map)
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> > static inline bool subflow_simultaneous_connect(struct sock *sk)
> > {
> > 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
> > -- 
> > 2.31.1
> > 
> > 
> > 
> 
> --
> Mat Martineau
> Intel