[PATCH mptcp-net v6 10/11] selftests: mptcp: join: cannot rm sf if closed

Matthieu Baerts (NGI0) posted 11 patches 4 months ago
There is a newer version of this series
[PATCH mptcp-net v6 10/11] selftests: mptcp: join: cannot rm sf if closed
Posted by Matthieu Baerts (NGI0) 4 months ago
Thanks to the previous commit, the MPTCP subflows are now closed on both
directions even when only the MPTCP path-manager of one peer asks for
their closure.

In the two tests modified here -- "userspace pm add & remove address"
and "userspace pm create destroy subflow" -- one peer is controlled by
the userspace PM, and the other one by in-kernel PM. When the userspace
PM sends a RM_ADDR notification, the in-kernel PM will automatically
react by closing all subflows using this address. Now that the subflows
are properly closed on both direction, the userspace PM can no longer
closes the same subflows if they are already closed. In this case, an
error will be returned to the userspace. So no need to run this command,
which mean that the linked counters will then not be incremented.

These tests are then no longer sending both a RM_ADDR, then closing the
linked subflow just after. The test with the userspace PM on the server
side is now removing one subflow linked to one address, then sending
a RM_ADDR for another address. The test with the userspace PM on the
client side is now only removing the subflow that was previously
created.

Fixes: 4369c198e599 ("selftests: mptcp: test userspace pm out of transfer")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index ea954ba85969..4129952fd9ec 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -3408,14 +3408,12 @@ userspace_tests()
 			"signal"
 		userspace_pm_chk_get_addr "${ns1}" "10" "id 10 flags signal 10.0.2.1"
 		userspace_pm_chk_get_addr "${ns1}" "20" "id 20 flags signal 10.0.3.1"
-		userspace_pm_rm_addr $ns1 10
 		userspace_pm_rm_sf $ns1 "::ffff:10.0.2.1" $MPTCP_LIB_EVENT_SUB_ESTABLISHED
 		userspace_pm_chk_dump_addr "${ns1}" \
-			"id 20 flags signal 10.0.3.1" "after rm_addr 10"
+			"id 20 flags signal 10.0.3.1" "after rm_sf 10"
 		userspace_pm_rm_addr $ns1 20
-		userspace_pm_rm_sf $ns1 10.0.3.1 $MPTCP_LIB_EVENT_SUB_ESTABLISHED
 		userspace_pm_chk_dump_addr "${ns1}" "" "after rm_addr 20"
-		chk_rm_nr 2 2 invert
+		chk_rm_nr 1 1 invert
 		chk_mptcp_info subflows 0 subflows 0
 		chk_subflows_total 1 1
 		kill_events_pids
@@ -3439,12 +3437,11 @@ userspace_tests()
 			"id 20 flags subflow 10.0.3.2" \
 			"subflow"
 		userspace_pm_chk_get_addr "${ns2}" "20" "id 20 flags subflow 10.0.3.2"
-		userspace_pm_rm_addr $ns2 20
 		userspace_pm_rm_sf $ns2 10.0.3.2 $MPTCP_LIB_EVENT_SUB_ESTABLISHED
 		userspace_pm_chk_dump_addr "${ns2}" \
 			"" \
-			"after rm_addr 20"
-		chk_rm_nr 1 1
+			"after rm_sf 20"
+		chk_rm_nr 0 1
 		chk_mptcp_info subflows 0 subflows 0
 		chk_subflows_total 1 1
 		kill_events_pids

-- 
2.45.2
Re: [PATCH mptcp-net v6 10/11] selftests: mptcp: join: cannot rm sf if closed
Posted by Mat Martineau 3 months, 3 weeks ago
On Fri, 2 Aug 2024, Matthieu Baerts (NGI0) wrote:

> Thanks to the previous commit, the MPTCP subflows are now closed on both
> directions even when only the MPTCP path-manager of one peer asks for
> their closure.
>
> In the two tests modified here -- "userspace pm add & remove address"
> and "userspace pm create destroy subflow" -- one peer is controlled by
> the userspace PM, and the other one by in-kernel PM. When the userspace
> PM sends a RM_ADDR notification, the in-kernel PM will automatically
> react by closing all subflows using this address. Now that the subflows
> are properly closed on both direction, the userspace PM can no longer
> closes the same subflows if they are already closed. In this case, an
> error will be returned to the userspace. So no need to run this command,
> which mean that the linked counters will then not be incremented.
>
> These tests are then no longer sending both a RM_ADDR, then closing the
> linked subflow just after. The test with the userspace PM on the server
> side is now removing one subflow linked to one address, then sending
> a RM_ADDR for another address. The test with the userspace PM on the
> client side is now only removing the subflow that was previously
> created.
>

Hi Matthieu -

How does this selftest behave on older kernels without the fix in the 
previous patch? Would be good to include that information in the commit 
message.

- Mat


> Fixes: 4369c198e599 ("selftests: mptcp: test userspace pm out of transfer")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> tools/testing/selftests/net/mptcp/mptcp_join.sh | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index ea954ba85969..4129952fd9ec 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -3408,14 +3408,12 @@ userspace_tests()
> 			"signal"
> 		userspace_pm_chk_get_addr "${ns1}" "10" "id 10 flags signal 10.0.2.1"
> 		userspace_pm_chk_get_addr "${ns1}" "20" "id 20 flags signal 10.0.3.1"
> -		userspace_pm_rm_addr $ns1 10
> 		userspace_pm_rm_sf $ns1 "::ffff:10.0.2.1" $MPTCP_LIB_EVENT_SUB_ESTABLISHED
> 		userspace_pm_chk_dump_addr "${ns1}" \
> -			"id 20 flags signal 10.0.3.1" "after rm_addr 10"
> +			"id 20 flags signal 10.0.3.1" "after rm_sf 10"
> 		userspace_pm_rm_addr $ns1 20
> -		userspace_pm_rm_sf $ns1 10.0.3.1 $MPTCP_LIB_EVENT_SUB_ESTABLISHED
> 		userspace_pm_chk_dump_addr "${ns1}" "" "after rm_addr 20"
> -		chk_rm_nr 2 2 invert
> +		chk_rm_nr 1 1 invert
> 		chk_mptcp_info subflows 0 subflows 0
> 		chk_subflows_total 1 1
> 		kill_events_pids
> @@ -3439,12 +3437,11 @@ userspace_tests()
> 			"id 20 flags subflow 10.0.3.2" \
> 			"subflow"
> 		userspace_pm_chk_get_addr "${ns2}" "20" "id 20 flags subflow 10.0.3.2"
> -		userspace_pm_rm_addr $ns2 20
> 		userspace_pm_rm_sf $ns2 10.0.3.2 $MPTCP_LIB_EVENT_SUB_ESTABLISHED
> 		userspace_pm_chk_dump_addr "${ns2}" \
> 			"" \
> -			"after rm_addr 20"
> -		chk_rm_nr 1 1
> +			"after rm_sf 20"
> +		chk_rm_nr 0 1
> 		chk_mptcp_info subflows 0 subflows 0
> 		chk_subflows_total 1 1
> 		kill_events_pids
>
> -- 
> 2.45.2
>
>
Re: [PATCH mptcp-net v6 10/11] selftests: mptcp: join: cannot rm sf if closed
Posted by Matthieu Baerts 3 months, 3 weeks ago
Hi Mat,

On 09/08/2024 02:33, Mat Martineau wrote:
> On Fri, 2 Aug 2024, Matthieu Baerts (NGI0) wrote:
> 
>> Thanks to the previous commit, the MPTCP subflows are now closed on both
>> directions even when only the MPTCP path-manager of one peer asks for
>> their closure.
>>
>> In the two tests modified here -- "userspace pm add & remove address"
>> and "userspace pm create destroy subflow" -- one peer is controlled by
>> the userspace PM, and the other one by in-kernel PM. When the userspace
>> PM sends a RM_ADDR notification, the in-kernel PM will automatically
>> react by closing all subflows using this address. Now that the subflows
>> are properly closed on both direction, the userspace PM can no longer
>> closes the same subflows if they are already closed. In this case, an
>> error will be returned to the userspace. So no need to run this command,
>> which mean that the linked counters will then not be incremented.
>>
>> These tests are then no longer sending both a RM_ADDR, then closing the
>> linked subflow just after. The test with the userspace PM on the server
>> side is now removing one subflow linked to one address, then sending
>> a RM_ADDR for another address. The test with the userspace PM on the
>> client side is now only removing the subflow that was previously
>> created.
>>
> 
> Hi Matthieu -
> 
> How does this selftest behave on older kernels without the fix in the
> previous patch? Would be good to include that information in the commit
> message.

I tried to explain that in the commit message. Let me try to clarify
this in the v7 :)

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