[PATCH mptcp-net v4 11/23] selftests: mptcp: join: check re-using ID of closed subflow

Matthieu Baerts (NGI0) posted 23 patches 2 months, 3 weeks ago
There is a newer version of this series
[PATCH mptcp-net v4 11/23] selftests: mptcp: join: check re-using ID of closed subflow
Posted by Matthieu Baerts (NGI0) 2 months, 3 weeks ago
This test extends "delete and re-add" to validate the previous commit. A
new 'subflow' endpoint is added, but the subflow request will be
rejected. The result is that no subflow will be established from this
address.

Later, the endpoint is removed and re-added after having cleared the
firewall rule. Before the previous commit, the client would not have
been able to create this new subflow.

While at it, extra checks have been added to validate the expected
numbers of MPJ and RM_ADDR.

The 'Fixes' tag here below is the same as the one from the previous
commit: this patch here is not fixing anything wrong in the selftests,
but it validates the previous fix for an issue introduced by this commit
ID.

Fixes: b6c08380860b ("mptcp: remove addr and subflow in PM netlink")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 27 ++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index b4dc5f2772dc..c5aa745a36f5 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -436,9 +436,10 @@ reset_with_tcp_filter()
 	local ns="${!1}"
 	local src="${2}"
 	local target="${3}"
+	local chain="${4:-INPUT}"
 
 	if ! ip netns exec "${ns}" ${iptables} \
-			-A INPUT \
+			-A "${chain}" \
 			-s "${src}" \
 			-p tcp \
 			-j "${target}"; then
@@ -3527,10 +3528,10 @@ endpoint_tests()
 		mptcp_lib_kill_wait $tests_pid
 	fi
 
-	if reset "delete and re-add" &&
+	if reset_with_tcp_filter "delete and re-add" ns2 10.0.3.2 REJECT OUTPUT &&
 	   mptcp_lib_kallsyms_has "subflow_rebuild_header$"; then
-		pm_nl_set_limits $ns1 1 1
-		pm_nl_set_limits $ns2 1 1
+		pm_nl_set_limits $ns1 0 2
+		pm_nl_set_limits $ns2 0 2
 		pm_nl_add_endpoint $ns2 10.0.2.2 id 2 dev ns2eth2 flags subflow
 		test_linkfail=4 speed=20 \
 			run_tests $ns1 $ns2 10.0.1.1 &
@@ -3547,11 +3548,27 @@ endpoint_tests()
 		chk_subflow_nr "after delete" 1
 		chk_mptcp_info subflows 0 subflows 0
 
-		pm_nl_add_endpoint $ns2 10.0.2.2 dev ns2eth2 flags subflow
+		pm_nl_add_endpoint $ns2 10.0.2.2 id 2 dev ns2eth2 flags subflow
 		wait_mpj $ns2
 		chk_subflow_nr "after re-add" 2
 		chk_mptcp_info subflows 1 subflows 1
+
+		pm_nl_add_endpoint $ns2 10.0.3.2 id 3 flags subflow
+		wait_attempt_fail $ns2
+		chk_subflow_nr "after new reject" 2
+		chk_mptcp_info subflows 1 subflows 1
+
+		ip netns exec "${ns2}" ${iptables} -D OUTPUT -s "10.0.3.2" -p tcp -j REJECT
+		pm_nl_del_endpoint $ns2 3 10.0.3.2
+		pm_nl_add_endpoint $ns2 10.0.3.2 id 3 flags subflow
+		wait_mpj $ns2
+		chk_subflow_nr "after no reject" 3
+		chk_mptcp_info subflows 2 subflows 2
+
 		mptcp_lib_kill_wait $tests_pid
+
+		chk_join_nr 3 3 3
+		chk_rm_nr 1 1
 	fi
 
 	# remove and re-add

-- 
2.45.2
Re: [PATCH mptcp-net v4 11/23] selftests: mptcp: join: check re-using ID of closed subflow
Posted by Mat Martineau 2 months, 3 weeks ago
On Mon, 22 Jul 2024, Matthieu Baerts (NGI0) wrote:

> This test extends "delete and re-add" to validate the previous commit. A
> new 'subflow' endpoint is added, but the subflow request will be
> rejected. The result is that no subflow will be established from this
> address.
>
> Later, the endpoint is removed and re-added after having cleared the
> firewall rule. Before the previous commit, the client would not have
> been able to create this new subflow.
>
> While at it, extra checks have been added to validate the expected
> numbers of MPJ and RM_ADDR.
>
> The 'Fixes' tag here below is the same as the one from the previous
> commit: this patch here is not fixing anything wrong in the selftests,
> but it validates the previous fix for an issue introduced by this commit
> ID.
>
> Fixes: b6c08380860b ("mptcp: remove addr and subflow in PM netlink")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> tools/testing/selftests/net/mptcp/mptcp_join.sh | 27 ++++++++++++++++++++-----
> 1 file changed, 22 insertions(+), 5 deletions(-)

As we discussed in the meeting today, for patches 1-11:

Reviewed-by: Mat Martineau <martineau@kernel.org>
Re: [PATCH mptcp-net v4 11/23] selftests: mptcp: join: check re-using ID of closed subflow
Posted by Matthieu Baerts 2 months, 3 weeks ago
Hi Mat,

On 24/07/2024 19:14, Mat Martineau wrote:
> On Mon, 22 Jul 2024, Matthieu Baerts (NGI0) wrote:
> 
>> This test extends "delete and re-add" to validate the previous commit. A
>> new 'subflow' endpoint is added, but the subflow request will be
>> rejected. The result is that no subflow will be established from this
>> address.
>>
>> Later, the endpoint is removed and re-added after having cleared the
>> firewall rule. Before the previous commit, the client would not have
>> been able to create this new subflow.
>>
>> While at it, extra checks have been added to validate the expected
>> numbers of MPJ and RM_ADDR.
>>
>> The 'Fixes' tag here below is the same as the one from the previous
>> commit: this patch here is not fixing anything wrong in the selftests,
>> but it validates the previous fix for an issue introduced by this commit
>> ID.
>>
>> Fixes: b6c08380860b ("mptcp: remove addr and subflow in PM netlink")
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>> tools/testing/selftests/net/mptcp/mptcp_join.sh | 27 +++++++++++++++++
>> +++-----
>> 1 file changed, 22 insertions(+), 5 deletions(-)
> 
> As we discussed in the meeting today, for patches 1-11:
Thank you for the review!

These first 11 patches are now in our tree (fixes for -net):

New patches for t/upstream-net and t/upstream:
- 0810e3bfd645: mptcp: fully established after ADD_ADDR echo on MPJ
- fa82905dad6c: mptcp: pm: deny endp with signal + subflow + port
- 0bcd8eb5baf3: mptcp: pm: reduce indentation blocks
- 4446f3ce0f45: mptcp: pm: don't try to create sf if alloc failed
- 23dcae2e002f: mptcp: pm: do not ignore 'subflow' if 'signal' flag is
also set
- 8396284e59ae: selftests: mptcp: join: ability to invert ADD_ADDR check
- 97c32986ea33: selftests: mptcp: join: test both signal & subflow
- 1624f3f439c1: mptcp: pm: re-using ID of unused removed ADD_ADDR
- e85c614f5dae: selftests: mptcp: join: check re-using ID of unused ADD_ADDR
- 887b2064d1c1: mptcp: pm: re-using ID of unused removed subflows
- 6d8031641325: selftests: mptcp: join: check re-using ID of closed subflow
- Results: 0e547869a341..35d7208999c3 (export-net)
- Results: 9a749f3dd928..a9a9b3b154d1 (export)

Tests are now in progress:

- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/a2e8f006417d01deb51431292ae03179641b1e20/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/70a0d0dafa76f55e2d2e1ba5ae6067f832b1bcb0/checks

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.