[PATCH mptcp-next 0/2] mptcp: some self-tests fixup

[PATCH mptcp-next 0/2] mptcp: some self-tests fixup

[Paolo Abeni]

The 2 squash-to patches try to address issues/221, at least
for the root cause reported in the github tracker.

Note, I can still reproduce some self-test failures, on other
link_failure test case, but with lower rate and different
cause - to be investigated.

Paolo Abeni (2):
  Squash-to: "selftests: mptcp: add testcase for active-back"
  Squash-to: "mptcp: move drop_other_suboptions check under pm lock"

 net/mptcp/options.c                             | 4 ++++
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 8 ++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

-- 
2.26.3

[PATCH mptcp-next 1/2] Squash-to: "selftests: mptcp: add testcase for active-back"

[Paolo Abeni]

we need to use big-enough client file on link failure test.

We compute the expected link usage assuming half of the client
data will be sent after the link failure. The link failure
happens after sending once the client file, and than the client
file is sent again. After the link failure, a whole cwin amount
of data can be re-inected: to be able to predict correctly
the link utilization the client file size must be much greater
then max cwin.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index ae406b56c684..255793c5ac4f 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -513,9 +513,13 @@ run_tests()
 	# create the input file for the failure test when
 	# the first failure test run
 	if [ "$test_linkfail" -ne 0 -a -z "$cinfail" ]; then
-		size=$((RANDOM%8))
+		# the client file must be considerably larger
+		# of the maximum expected cwin value, or the
+		# link utilization will be not predicable
+		size=$((RANDOM%2))
 		size=$((size+1))
-		size=$((size*2048))
+		size=$((size*8192))
+		size=$((size + ( $RANDOM % 8192) ))
 
 		cinfail=$(mktemp)
 		make_file "$cinfail" "client" $size
-- 
2.26.3

[PATCH mptcp-next 2/2] Squash-to: "mptcp: move drop_other_suboptions check under pm lock"

[Paolo Abeni]

MPJ ACK is a pure TCP ack on an established TCP connection, so is
eligble to carry MPTCP ADD_ADDR option. If that happens, the MPJ
subopt will be stripped and the MPJ handshake will be broken.

Avoid adding ADD_ADDR subopt if the current packet is MPJ ACK (or
MPC ACK, just to be safe: the added check it's cheap).

This fixes issues/221

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/options.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index b2af81515f20..b3d5547fdb61 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -678,7 +678,11 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
 	bool port;
 	int len;
 
+	/* add addr will strip the existing options, be sure to avoid breaking
+	 * MPC/MPJ handshakes
+	 */
 	if (!mptcp_pm_should_add_signal(msk) ||
+	    (opts->suboptions & (OPTION_MPTCP_MPJ_ACK | OPTION_MPTCP_MPC_ACK)) ||
 	    !mptcp_pm_add_addr_signal(msk, skb, opt_size, remaining, &opts->addr,
 		    &echo, &port, &drop_other_suboptions))
 		return false;
-- 
2.26.3

Re: [PATCH mptcp-next 0/2] mptcp: some self-tests fixup

[Matthieu Baerts]

Hi Paolo,

On 11/08/2021 11:25, Paolo Abeni wrote:
> The 2 squash-to patches try to address issues/221, at least
> for the root cause reported in the github tracker.

I confirm that it is much better, thank you!

Before the patches, it took me 2 attempts to have "mptcp_join.sh -l"
failing with a debug kernel.

With the patches, I have 30 successful runs so far!

Tested-by: Matthieu Baerts <matthieu.baerts@tessares.net>

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Re: [PATCH mptcp-next 0/2] mptcp: some self-tests fixup

[Mat Martineau]

On Wed, 11 Aug 2021, Matthieu Baerts wrote:

> Hi Paolo,
>
> On 11/08/2021 11:25, Paolo Abeni wrote:
>> The 2 squash-to patches try to address issues/221, at least
>> for the root cause reported in the github tracker.
>
> I confirm that it is much better, thank you!
>
> Before the patches, it took me 2 attempts to have "mptcp_join.sh -l"
> failing with a debug kernel.
>
> With the patches, I have 30 successful runs so far!
>
> Tested-by: Matthieu Baerts <matthieu.baerts@tessares.net>

Thanks for the patches Paolo, and thanks for testing Matthieu. Looks good 
to squash.

--
Mat Martineau
Intel

Re: [PATCH mptcp-next 0/2] mptcp: some self-tests fixup

[Matthieu Baerts]

Hi Paolo, Mat,

On 11/08/2021 11:25, Paolo Abeni wrote:
> The 2 squash-to patches try to address issues/221, at least
> for the root cause reported in the github tracker.

Thank you for the patches and the review!

Now in our tree:

- 3b6b8c96b67c: "squashed" patch 1/2 in "selftests: mptcp: add testcase
for active-back"
- Results: e41bc27ecfa9..c636d849945e

- 32c3e4d50806: "squashed" patch 2/2 in "mptcp: move
drop_other_suboptions check under pm lock"
- Results: c636d849945e..3e7443d82746

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20210812T065059
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export/20210812T065059


> Note, I can still reproduce some self-test failures, on other
> link_failure test case, but with lower rate and different
> cause - to be investigated.

Do you prefer to keep the issue #221 open or should we open a new one?

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net