Re: [MPTCP][PATCH v5 mptcp-next 0/5] MP_FAIL support

[Geliang Tang <geliangtang@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2837 bytes --]

Hi Mat,

Mat Martineau <mathew.j.martineau@linux.intel.com> 于2021年7月28日周三 上午8:27写道：
>
> On Mon, 26 Jul 2021, Geliang Tang wrote:
>
> > From: Geliang Tang <geliangtang@xiaomi.com>
> >
> > v5:
> > - patch 1, change "ret = true;" to "return true;"
> > - patch 3, in the single-subflow case, send MP_FAIL and receive the
> >   echo, then temporarily handled by reset.
> >
>
> Hi Geliang -
>
> Thanks for the v5 changes. The changes are looking good in terms of manual
> code review. Since the selftest changes in patch 5 only confirm that
> MP_FAIL was not sent, I'm going to do some testing tomorrow to force
> checksum failures in single and multiple subflow cases so I can check some
> packet traces. It would be good to have some packetdrill checksum test
> cases to cover that, but we don't yet.

I use a similar test for MP_FAIL to force checksum failures. I have put
it in the attachment.

Thanks,
-Geliang

>
>
> - Mat
>
>
> > v4:
> > - just deal with the multiple subflows case, put the single subflow
> >   case into the new 'infinite mapping' part.
> >
> > v3:
> > - respond with MP_FAIL
> > - add single subflow check
> > - add infinite mapping sending and receiving
> > - export/20210626T054902
> >
> > v2:
> > - MP_FAIL logic:
> >   * Peer B send a DSS to peer A, and the data has been modify by the
> >  middleboxes, then peer A detects the bad checksum.
> >   * In the multiple subflows case, peer A sends MP_FAIL+RST back to peer B,
> >  and peer A discards the data following the bad data sequence number. Peer
> >  B receives this MP_FAIL+RST, and close this subflow.
> >   * In the single subflow case, using the simple implementation, peer A
> >  sends MP_FAIL back to peer B, and peer A fallback to a regular TCP. Peer
> >  B receives this MP_FAIL, and fallback to a regular TCP.
> >
> > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/52
> >
> > Geliang Tang (5):
> >  mptcp: MP_FAIL suboption sending
> >  mptcp: MP_FAIL suboption receiving
> >  mptcp: send out MP_FAIL when data checksum fails
> >  mptcp: add the mibs for MP_FAIL
> >  selftests: mptcp: add MP_FAIL mibs check
> >
> > include/net/mptcp.h                           |  5 +-
> > net/mptcp/mib.c                               |  2 +
> > net/mptcp/mib.h                               |  2 +
> > net/mptcp/options.c                           | 78 ++++++++++++++++++-
> > net/mptcp/pm.c                                | 20 +++++
> > net/mptcp/protocol.h                          | 20 +++++
> > net/mptcp/subflow.c                           | 18 +++++
> > .../testing/selftests/net/mptcp/mptcp_join.sh | 38 +++++++++
> > 8 files changed, 178 insertions(+), 5 deletions(-)
> >
> > --
> > 2.31.1
> >
> >
> >
>
> --
> Mat Martineau
> Intel

[-- Attachment #2: 0001-mptcp-mp_fail-test.patch --]
[-- Type: text/x-patch, Size: 2192 bytes --]

From 0bd5d26b8cdeb296de3fdf93bd4f08781e02a853 Mon Sep 17 00:00:00 2001
From: Geliang Tang <geliangtang@gmail.com>
Date: Wed, 14 Jul 2021 15:25:28 +0800
Subject: [PATCH] mptcp: mp_fail test

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 8eb2626503d7..59e04f9c6906 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1275,6 +1275,8 @@ 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 int i;
+
 static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 			      struct mptcp_data_frag *dfrag,
 			      struct mptcp_sendmsg_info *info)
@@ -1376,6 +1378,13 @@ static int mptcp_sendmsg_frag(struct sock *sk, struct sock *ssk,
 out:
 	if (READ_ONCE(msk->csum_enabled))
 		mptcp_update_data_checksum(tail, ret);
+
+	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 551fcce7c2f2..8495ca54d563 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -927,6 +927,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
+
+	# 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
+
+	exit
+
 	reset
 	run_tests $ns1 $ns2 10.0.1.1
 	chk_join_nr "no JOIN" "0" "0" "0"
-- 
2.31.1