[v1] mptcp: fix signal endpoint readd

[PATCH mptcp-net 2/3] selftests: mptcp: add explicit test case for remove/readd

Posted by Paolo Abeni 3 months, 3 weeks ago

Delete and re-create a signal endpoint and ensure that the PM
actually deletes and re-create the subflow.

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

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 108aeeb84ef1..228fecee61f9 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -3526,6 +3526,34 @@ endpoint_tests()
 		chk_mptcp_info subflows 1 subflows 1
 		mptcp_lib_kill_wait $tests_pid
 	fi
+
+	# remove and re-add
+	if reset "delete re-add signal"; then
+		pm_nl_set_limits $ns1 1 1
+		pm_nl_set_limits $ns2 1 1
+		pm_nl_add_endpoint $ns1 10.0.2.1 id 1 flags signal
+		test_linkfail=4 speed=20 \
+			run_tests $ns1 $ns2 10.0.1.1 &
+		local tests_pid=$!
+
+		wait_mpj $ns2
+		pm_nl_check_endpoint "creation" \
+			$ns1 10.0.2.1 id 1 flags signal
+		chk_subflow_nr "before delete" 2
+		chk_mptcp_info subflows 1 subflows 1
+
+		pm_nl_del_endpoint $ns1 1 10.0.2.1
+		sleep 0.5
+		chk_subflow_nr "after delete" 1
+		chk_mptcp_info subflows 0 subflows 0
+
+		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
+		wait_mpj $ns2
+		chk_subflow_nr "after re-add" 2
+		chk_mptcp_info subflows 1 subflows 1
+		mptcp_lib_kill_wait $tests_pid
+	fi
+
 }
 
 # [$1: error message]
-- 
2.45.1

Re: [PATCH mptcp-net 2/3] selftests: mptcp: add explicit test case for remove/readd

Posted by Matthieu Baerts 3 months, 2 weeks ago

Hi Paolo,

On 28/06/2024 17:54, Paolo Abeni wrote:
> Delete and re-create a signal endpoint and ensure that the PM
> actually deletes and re-create the subflow.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  .../testing/selftests/net/mptcp/mptcp_join.sh | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 108aeeb84ef1..228fecee61f9 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -3526,6 +3526,34 @@ endpoint_tests()
>  		chk_mptcp_info subflows 1 subflows 1
>  		mptcp_lib_kill_wait $tests_pid
>  	fi
> +
> +	# remove and re-add
> +	if reset "delete re-add signal"; then

In the previous tests -- "delete and re-add", we added:

  mptcp_lib_kallsyms_has "subflow_rebuild_header$"

At the beginning of the endpoint_tests() function there is this comment:

  # subflow_rebuild_header is needed to support the implicit flag

Maybe I put the same check on "delete and re-add" by mistake. I don't
think this test depends on the "implicit" type. Or maybe it is, because
its introduction is also linked to some big modifications on the PM
side. In this case, should we have the same check?

  && mptcp_lib_kallsyms_has "subflow_rebuild_header$"

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

Re: [PATCH mptcp-net 2/3] selftests: mptcp: add explicit test case for remove/readd

Posted by Paolo Abeni 3 months, 2 weeks ago

On Tue, 2024-07-02 at 11:01 +0200, Matthieu Baerts wrote:
> Hi Paolo,
> 
> On 28/06/2024 17:54, Paolo Abeni wrote:
> > Delete and re-create a signal endpoint and ensure that the PM
> > actually deletes and re-create the subflow.
> > 
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> >  .../testing/selftests/net/mptcp/mptcp_join.sh | 28 +++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > index 108aeeb84ef1..228fecee61f9 100755
> > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > @@ -3526,6 +3526,34 @@ endpoint_tests()
> >  		chk_mptcp_info subflows 1 subflows 1
> >  		mptcp_lib_kill_wait $tests_pid
> >  	fi
> > +
> > +	# remove and re-add
> > +	if reset "delete re-add signal"; then
> 
> In the previous tests -- "delete and re-add", we added:
> 
>   mptcp_lib_kallsyms_has "subflow_rebuild_header$"
> 
> At the beginning of the endpoint_tests() function there is this comment:
> 
>   # subflow_rebuild_header is needed to support the implicit flag
> 
> Maybe I put the same check on "delete and re-add" by mistake. I don't
> think this test depends on the "implicit" type. Or maybe it is, because
> its introduction is also linked to some big modifications on the PM
> side. In this case, should we have the same check?
> 
>   && mptcp_lib_kallsyms_has "subflow_rebuild_header$"

Uhmm... sounds a little too much defensive programming, but I guess I
can add the test here

Thanks!

Paolo