[MPTCP][PATCH RFC 0/5] The infinite mapping support

[MPTCP][PATCH RFC 0/5] The infinite mapping support

[Geliang Tang]

From: Geliang Tang <geliangtang@xiaomi.com>

This patchset doesn't work yet. I sent this RFC version for some
suggestions and help.

tag: export/20210901T094059

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

Geliang Tang (5):
  mptcp: don't send RST for single subflow
  mptcp: infinite mapping sending
  mptcp: infinite mapping receiving
  mptcp: add a mib for the infinite mapping sending
  DO-NOT-MERGE: mptcp: mp_fail test

 net/mptcp/mib.c                               |  1 +
 net/mptcp/mib.h                               |  1 +
 net/mptcp/options.c                           | 13 ++++++++
 net/mptcp/pm.c                                |  6 ++++
 net/mptcp/protocol.c                          | 32 +++++++++++++++++++
 net/mptcp/protocol.h                          |  3 ++
 net/mptcp/subflow.c                           | 27 +++++++++++++---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 18 +++++++++++
 8 files changed, 96 insertions(+), 5 deletions(-)

-- 
2.31.1

[MPTCP][PATCH RFC 1/5] mptcp: don't send RST for single subflow

[Geliang Tang]

From: Geliang Tang <geliangtang@xiaomi.com>

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

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

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 1de7ce883c37..dfcd84abc13e 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1167,14 +1167,14 @@ static bool subflow_check_data_avail(struct sock *ssk)
 	/* RFC 8684 section 3.7. */
 	if (subflow->send_mp_fail) {
 		if (mptcp_has_another_subflow(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

[MPTCP][PATCH RFC 2/5] mptcp: infinite mapping sending

[Geliang Tang]

From: Geliang Tang <geliangtang@xiaomi.com>

This patch added the infinite mapping sending logic.

Added a new flag snd_infinite_mapping_enable in mptcp_sock. Set it true
when a single 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_mapping to set the infinite mapping.

In mptcp_write_options, send out the infinite mapping and fallback to
a regular TCP.

Signed-off-by: Geliang Tang <geliangtang@xiaomi.com>
---
 net/mptcp/options.c  | 13 +++++++++++++
 net/mptcp/pm.c       |  6 ++++++
 net/mptcp/protocol.c | 21 +++++++++++++++++++++
 net/mptcp/protocol.h |  1 +
 4 files changed, 41 insertions(+)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 1ec6529c4326..e2df21246be7 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -1326,6 +1326,19 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
 				put_unaligned_be32(mpext->data_len << 16 |
 						   TCPOPT_NOP << 8 | TCPOPT_NOP, ptr);
 			}
+
+			if (mpext->data_len == 0) {
+				const struct sock *ssk = (const struct sock *)tp;
+				struct mptcp_subflow_context *subflow;
+				struct mptcp_sock *msk;
+
+				subflow = mptcp_subflow_ctx(ssk);
+				msk = mptcp_sk(subflow->conn);
+
+				pr_debug("write infinite mapping!");
+				pr_fallback(msk);
+				__mptcp_do_fallback(msk);
+			}
 		}
 	} else if ((OPTION_MPTCP_MPC_SYN | OPTION_MPTCP_MPC_SYNACK |
 		    OPTION_MPTCP_MPC_ACK) & opts->suboptions) {
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 6ab386ff3294..39f2dcda53bf 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -251,7 +251,13 @@ 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);
+	struct mptcp_sock *msk = mptcp_sk(subflow->conn);
+
 	pr_debug("fail_seq=%llu", fail_seq);
+
+	if (!mptcp_has_another_subflow(sk))
+		WRITE_ONCE(msk->snd_infinite_mapping_enable, true);
 }
 
 /* path manager helpers */
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index faf6e7000d18..dd7738a6b7f5 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1282,6 +1282,22 @@ 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_mapping(struct mptcp_sock *msk, struct mptcp_ext *mpext)
+{
+	if (!mpext)
+		return;
+
+	mpext->data_seq = READ_ONCE(msk->ack_seq);
+	mpext->data_len = 0;
+	if (READ_ONCE(msk->csum_enabled))
+		mpext->csum = 0;
+
+	WRITE_ONCE(msk->snd_infinite_mapping_enable, false);
+
+	pr_debug("infinite mapping: data_seq=%llu subflow_seq=%u data_len=%u dsn64=%d",
+		 mpext->data_seq, mpext->subflow_seq, mpext->data_len, mpext->dsn64);
+}
+
 static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 			      struct mptcp_data_frag *dfrag,
 			      struct mptcp_sendmsg_info *info)
@@ -1390,6 +1406,10 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 out:
 	if (READ_ONCE(msk->csum_enabled))
 		mptcp_update_data_checksum(tail, ret);
+
+	if (READ_ONCE(msk->snd_infinite_mapping_enable))
+		mptcp_update_infinite_mapping(msk, mpext);
+
 	mptcp_subflow_ctx(ssk)->rel_write_seq += ret;
 	return ret;
 }
@@ -2858,6 +2878,7 @@ struct sock *mptcp_sk_clone(const struct sock *sk,
 	WRITE_ONCE(msk->fully_established, false);
 	if (mp_opt->suboptions & OPTION_MPTCP_CSUMREQD)
 		WRITE_ONCE(msk->csum_enabled, true);
+	WRITE_ONCE(msk->snd_infinite_mapping_enable, false);
 
 	msk->write_seq = subflow_req->idsn + 1;
 	msk->snd_nxt = msk->write_seq;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 99a23fff7b03..33400fcdf1b1 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -246,6 +246,7 @@ struct mptcp_sock {
 	bool		fully_established;
 	bool		rcv_data_fin;
 	bool		snd_data_fin_enable;
+	bool		snd_infinite_mapping_enable;
 	bool		rcv_fastclose;
 	bool		use_64bit_ack; /* Set when we received a 64-bit DSN */
 	bool		csum_enabled;
-- 
2.31.1

[MPTCP][PATCH RFC 3/5] mptcp: infinite mapping receiving

[Geliang Tang]

From: Geliang Tang <geliangtang@xiaomi.com>

This patch added the infinite mapping receiving logic.

Added a new struct member infinite_mapping_received in struct
mptcp_subflow_context, set it true when the infinite mapping is
received. And added another new struct member infinite_rcv_seq to save
the received infinite mapping sequence number.

In subflow_check_data_avail, if the infinite_mapping_received flag is
set, fallback to a regular TCP.

Signed-off-by: Geliang Tang <geliangtang@xiaomi.com>
---
 net/mptcp/protocol.h |  2 ++
 net/mptcp/subflow.c  | 17 +++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 33400fcdf1b1..ad26e7c0a18c 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,
+		infinite_mapping_received : 1,
 		rx_eof : 1,
 		can_ack : 1,        /* only after processing the remote a key */
 		disposable : 1,	    /* ctx can be free at ulp release time */
@@ -449,6 +450,7 @@ struct mptcp_subflow_context {
 	u8	reset_transient:1;
 	u8	reset_reason:4;
 	u8	stale_count;
+	u64	infinite_rcv_seq;
 
 	long	delegated_status;
 	struct	list_head delegated_node;   /* link into delegated_action, protected by local BH */
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index dfcd84abc13e..95ea01694bb3 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -967,7 +967,10 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
 
 	data_len = mpext->data_len;
 	if (data_len == 0) {
+		pr_debug("infinite mapping received, data_seq=%llu", mpext->data_seq);
 		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_INFINITEMAPRX);
+		subflow->infinite_mapping_received = 1;
+		subflow->infinite_rcv_seq = mpext->data_seq;
 		return MAPPING_INVALID;
 	}
 
@@ -1179,6 +1182,20 @@ static bool subflow_check_data_avail(struct sock *ssk)
 		return true;
 	}
 
+	if (subflow->infinite_mapping_received) {
+		pr_fallback(msk);
+		__mptcp_do_fallback(msk);
+		skb = skb_peek(&ssk->sk_receive_queue);
+		subflow->map_valid = 1;
+		subflow->map_seq = subflow->infinite_rcv_seq;
+		subflow->map_data_len = skb->len;
+		subflow->map_subflow_seq = tcp_sk(ssk)->copied_seq - subflow->ssn_offset;
+		WRITE_ONCE(subflow->data_avail, MPTCP_SUBFLOW_DATA_AVAIL);
+		subflow->infinite_mapping_received = 0;
+		subflow->infinite_rcv_seq = 0;
+		return true;
+	}
+
 	if (subflow->mp_join || subflow->fully_established) {
 		/* fatal protocol error, close the socket.
 		 * subflow_error_report() will introduce the appropriate barriers
-- 
2.31.1

[MPTCP][PATCH RFC 4/5] mptcp: add a mib for the infinite mapping sending

[Geliang Tang]

From: Geliang Tang <geliangtang@xiaomi.com>

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@xiaomi.com>
---
 net/mptcp/mib.c      | 1 +
 net/mptcp/mib.h      | 1 +
 net/mptcp/protocol.c | 3 +++
 3 files changed, 5 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 dd7738a6b7f5..4987909991cd 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1284,6 +1284,8 @@ static void mptcp_update_data_checksum(struct sk_buff *skb, int added)
 
 static void mptcp_update_infinite_mapping(struct mptcp_sock *msk, struct mptcp_ext *mpext)
 {
+	struct sock *sk = (struct sock *)msk;
+
 	if (!mpext)
 		return;
 
@@ -1292,6 +1294,7 @@ static void mptcp_update_infinite_mapping(struct mptcp_sock *msk, struct mptcp_e
 	if (READ_ONCE(msk->csum_enabled))
 		mpext->csum = 0;
 
+	MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_INFINITEMAPTX);
 	WRITE_ONCE(msk->snd_infinite_mapping_enable, false);
 
 	pr_debug("infinite mapping: data_seq=%llu subflow_seq=%u data_len=%u dsn64=%d",
-- 
2.31.1

[MPTCP][PATCH RFC 5/5] DO-NOT-MERGE: mptcp: mp_fail test

[Geliang Tang]

From: Geliang Tang <geliangtang@xiaomi.com>

./mptcp_join.sh -Cf

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

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 4987909991cd..2d05236ad759 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1301,6 +1301,8 @@ static void mptcp_update_infinite_mapping(struct mptcp_sock *msk, struct mptcp_e
 		 mpext->data_seq, mpext->subflow_seq, mpext->data_len, mpext->dsn64);
 }
 
+static int i;
+
 static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 			      struct mptcp_data_frag *dfrag,
 			      struct mptcp_sendmsg_info *info)
@@ -1413,6 +1415,12 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 	if (READ_ONCE(msk->snd_infinite_mapping_enable))
 		mptcp_update_infinite_mapping(msk, mpext);
 
+	pr_debug("%s i=%d", __func__, i++);
+	if (i == 20)
+		tail->data_len = 1;
+	if (i > 40)
+		i = 0;
+
 	mptcp_subflow_ctx(ssk)->rel_write_seq += ret;
 	return ret;
 }
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 255793c5ac4f..16fa761607e4 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -939,6 +939,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: [MPTCP][PATCH RFC 3/5] mptcp: infinite mapping receiving

[Mat Martineau]

On Fri, 3 Sep 2021, Geliang Tang wrote:

> From: Geliang Tang <geliangtang@xiaomi.com>
>
> This patch added the infinite mapping receiving logic.
>
> Added a new struct member infinite_mapping_received in struct
> mptcp_subflow_context, set it true when the infinite mapping is
> received. And added another new struct member infinite_rcv_seq to save
> the received infinite mapping sequence number.
>
> In subflow_check_data_avail, if the infinite_mapping_received flag is
> set, fallback to a regular TCP.
>
> Signed-off-by: Geliang Tang <geliangtang@xiaomi.com>
> ---
> net/mptcp/protocol.h |  2 ++
> net/mptcp/subflow.c  | 17 +++++++++++++++++
> 2 files changed, 19 insertions(+)
>
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 33400fcdf1b1..ad26e7c0a18c 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,
> +		infinite_mapping_received : 1,
> 		rx_eof : 1,
> 		can_ack : 1,        /* only after processing the remote a key */
> 		disposable : 1,	    /* ctx can be free at ulp release time */
> @@ -449,6 +450,7 @@ struct mptcp_subflow_context {
> 	u8	reset_transient:1;
> 	u8	reset_reason:4;
> 	u8	stale_count;
> +	u64	infinite_rcv_seq;
>
> 	long	delegated_status;
> 	struct	list_head delegated_node;   /* link into delegated_action, protected by local BH */
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index dfcd84abc13e..95ea01694bb3 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -967,7 +967,10 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
>
> 	data_len = mpext->data_len;
> 	if (data_len == 0) {
> +		pr_debug("infinite mapping received, data_seq=%llu", mpext->data_seq);
> 		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_INFINITEMAPRX);
> +		subflow->infinite_mapping_received = 1;
> +		subflow->infinite_rcv_seq = mpext->data_seq;

RFC 8684 doesn't seem to call for any validation of the data_seq in an 
infinite mapping. I'm not sure we need to do anything with it, we would 
have to keep track of recent mapping values related to the DATA_ACKs we 
sent in order to compare. Does anyone else see this in the RFC?

> 		return MAPPING_INVALID;
> 	}
>
> @@ -1179,6 +1182,20 @@ static bool subflow_check_data_avail(struct sock *ssk)
> 		return true;
> 	}
>
> +	if (subflow->infinite_mapping_received) {
> +		pr_fallback(msk);
> +		__mptcp_do_fallback(msk);
> +		skb = skb_peek(&ssk->sk_receive_queue);
> +		subflow->map_valid = 1;
> +		subflow->map_seq = subflow->infinite_rcv_seq;
> +		subflow->map_data_len = skb->len;
> +		subflow->map_subflow_seq = tcp_sk(ssk)->copied_seq - subflow->ssn_offset;
> +		WRITE_ONCE(subflow->data_avail, MPTCP_SUBFLOW_DATA_AVAIL);
> +		subflow->infinite_mapping_received = 0;
> +		subflow->infinite_rcv_seq = 0;
> +		return true;
> +	}
> +

I don't think we should duplicate the code from later in the function, 
maybe the code below:

> 	if (subflow->mp_join || subflow->fully_established) {

could be changed to this:

if (subflow->mp_join || (subflow->fully_established && 
!subflow->infinite_mapping_received)

?


I think the intent of this fallback mechanism is to not throw away any 
data in the contiguous stream. Instead, it seems like the assumption is 
that the checksum problem was in the MPTCP header and the 
MP_FAIL/infinite-mapping signaling is supposed to let MPTCP do fallback 
while still delivering all the data to the user. Anyone else read the RFC 
the same way, or know better?


> 		/* fatal protocol error, close the socket.
> 		 * subflow_error_report() will introduce the appropriate barriers
> -- 
> 2.31.1
>
>
>

--
Mat Martineau
Intel

Re: [MPTCP][PATCH RFC 2/5] mptcp: infinite mapping sending

[Mat Martineau]

On Fri, 3 Sep 2021, Geliang Tang wrote:

> From: Geliang Tang <geliangtang@xiaomi.com>
>
> This patch added the infinite mapping sending logic.
>
> Added a new flag snd_infinite_mapping_enable in mptcp_sock. Set it true
> when a single 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_mapping to set the infinite mapping.
>
> In mptcp_write_options, send out the infinite mapping and fallback to
> a regular TCP.
>
> Signed-off-by: Geliang Tang <geliangtang@xiaomi.com>
> ---
> net/mptcp/options.c  | 13 +++++++++++++
> net/mptcp/pm.c       |  6 ++++++
> net/mptcp/protocol.c | 21 +++++++++++++++++++++
> net/mptcp/protocol.h |  1 +
> 4 files changed, 41 insertions(+)
>
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 1ec6529c4326..e2df21246be7 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -1326,6 +1326,19 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
> 				put_unaligned_be32(mpext->data_len << 16 |
> 						   TCPOPT_NOP << 8 | TCPOPT_NOP, ptr);
> 			}
> +
> +			if (mpext->data_len == 0) {
> +				const struct sock *ssk = (const struct sock *)tp;
> +				struct mptcp_subflow_context *subflow;
> +				struct mptcp_sock *msk;
> +
> +				subflow = mptcp_subflow_ctx(ssk);
> +				msk = mptcp_sk(subflow->conn);
> +
> +				pr_debug("write infinite mapping!");
> +				pr_fallback(msk);
> +				__mptcp_do_fallback(msk);
> +			}

The fallback action shouldn't happen inside mptcp_write_options(). I have 
a suggestion below.

> 		}
> 	} else if ((OPTION_MPTCP_MPC_SYN | OPTION_MPTCP_MPC_SYNACK |
> 		    OPTION_MPTCP_MPC_ACK) & opts->suboptions) {
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 6ab386ff3294..39f2dcda53bf 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -251,7 +251,13 @@ 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);
> +	struct mptcp_sock *msk = mptcp_sk(subflow->conn);
> +
> 	pr_debug("fail_seq=%llu", fail_seq);
> +
> +	if (!mptcp_has_another_subflow(sk))
> +		WRITE_ONCE(msk->snd_infinite_mapping_enable, true);

Like patch 1, there needs to be a check for both "single subflow" and 
"contiguous data".

> }
>
> /* path manager helpers */
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index faf6e7000d18..dd7738a6b7f5 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -1282,6 +1282,22 @@ 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_mapping(struct mptcp_sock *msk, struct mptcp_ext *mpext)
> +{
> +	if (!mpext)
> +		return;
> +
> +	mpext->data_seq = READ_ONCE(msk->ack_seq);

RFC 8684 says the data_seq to use is "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)".

In other words, the data_seq from the mapping that was for the *beginning* 
of the last fully-acked data segment.

This is something else that we don't specifically keep track of yet. The 
necessary information is (all?) in the msk->rtx_queue - I think we will 
have to add something to the msk to keep track of the 64-bit sequence 
number of each mapping as they are acked. This would be updated in 
__mptcp_data_acked() or __mptcp_clean_una().

> +	mpext->data_len = 0;

I think it might work well to add an infinite_mapping field to struct 
mptcp_ext, and that could be checked in mptcp_established_options() to 
allow the DSS option to be written even if the fallback flag is set.

> +	if (READ_ONCE(msk->csum_enabled))
> +		mpext->csum = 0;
> +
> +	WRITE_ONCE(msk->snd_infinite_mapping_enable, false);
> +
> +	pr_debug("infinite mapping: data_seq=%llu subflow_seq=%u data_len=%u dsn64=%d",
> +		 mpext->data_seq, mpext->subflow_seq, mpext->data_len, mpext->dsn64);
> +}
> +
> static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
> 			      struct mptcp_data_frag *dfrag,
> 			      struct mptcp_sendmsg_info *info)
> @@ -1390,6 +1406,10 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
> out:
> 	if (READ_ONCE(msk->csum_enabled))
> 		mptcp_update_data_checksum(tail, ret);
> +
> +	if (READ_ONCE(msk->snd_infinite_mapping_enable))
> +		mptcp_update_infinite_mapping(msk, mpext);
> +
> 	mptcp_subflow_ctx(ssk)->rel_write_seq += ret;
> 	return ret;
> }
> @@ -2858,6 +2878,7 @@ struct sock *mptcp_sk_clone(const struct sock *sk,
> 	WRITE_ONCE(msk->fully_established, false);
> 	if (mp_opt->suboptions & OPTION_MPTCP_CSUMREQD)
> 		WRITE_ONCE(msk->csum_enabled, true);
> +	WRITE_ONCE(msk->snd_infinite_mapping_enable, false);
>
> 	msk->write_seq = subflow_req->idsn + 1;
> 	msk->snd_nxt = msk->write_seq;
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 99a23fff7b03..33400fcdf1b1 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -246,6 +246,7 @@ struct mptcp_sock {
> 	bool		fully_established;
> 	bool		rcv_data_fin;
> 	bool		snd_data_fin_enable;
> +	bool		snd_infinite_mapping_enable;
> 	bool		rcv_fastclose;
> 	bool		use_64bit_ack; /* Set when we received a 64-bit DSN */
> 	bool		csum_enabled;
> -- 
> 2.31.1
>
>
>

--
Mat Martineau
Intel

Re: [MPTCP][PATCH RFC 1/5] mptcp: don't send RST for single subflow

[Mat Martineau]

On Fri, 3 Sep 2021, Geliang Tang wrote:

> From: Geliang Tang <geliangtang@xiaomi.com>
>
> When a bad checksum is detected and a single subflow is in use, don't
> send RST + MP_FAIL, send data_ack + MP_FAIL instead.
>
> Signed-off-by: Geliang Tang <geliangtang@xiaomi.com>
> ---
> net/mptcp/subflow.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 1de7ce883c37..dfcd84abc13e 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -1167,14 +1167,14 @@ static bool subflow_check_data_avail(struct sock *ssk)
> 	/* RFC 8684 section 3.7. */
> 	if (subflow->send_mp_fail) {
> 		if (mptcp_has_another_subflow(ssk)) {

Hi Geliang -

Thanks for sharing these RFC patches.


RFC8684 says that this infinite mapping case is when there is one subflow, 
*and* all the data is contiguous. So this condition needs to check for 
contiguous data on the subflow.

We don't currently track whether the data is contiguous on a subflow, so 
that tracking needs to be added. When retransmission happens, we could set 
a "noncontiguous" flag in the msk, and once all retransmissions are 
DATA_ACK'd that flag could be cleared.

> +			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