1. Patchew
  2. MPTCP
  3. [PATCH mptcp-next v5 0/4] add pm listener events
  4. View patch

[PATCH mptcp-next v5 4/4] selftests: mptcp: listener test for in-kernel PM

Geliang Tang posted 4 patches 3 years, 2 months ago
Maintainers: Mat Martineau <mathew.j.martineau@linux.intel.com>, Matthieu Baerts <matthieu.baerts@tessares.net>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Shuah Khan <shuah@kernel.org>
[PATCH mptcp-next v5 4/4] selftests: mptcp: listener test for in-kernel PM
Posted by Geliang Tang 3 years, 2 months ago 
This patch adds test coverage for listening sockets created by the
in-kernel path manager in mptcp_join.sh.

It adds the listener event checking in the existing "remove single
address with port" test. The output looks like this:

 003 remove single address with port syn[ ok ] - synack[ ok ] - ack[ ok ]
                                     add[ ok ] - echo  [ ok ] - pt [ ok ]
                                     syn[ ok ] - synack[ ok ] - ack[ ok ]
                                     syn[ ok ] - ack   [ ok ]
                                     rm [ ok ] - rmsf  [ ok ]   invert
                                     CREATE_LISTENER 10.0.2.1:10100[ ok ]
                                     CLOSE_LISTENER 10.0.2.1:10100 [ ok ]

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 62 +++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index f10ef65a7009..89a30225bbd7 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -2509,6 +2509,54 @@ backup_tests()
 	fi
 }
 
+LISTENER_CREATED=15 #MPTCP_EVENT_LISTENER_CREATED
+LISTENER_CLOSED=16  #MPTCP_EVENT_LISTENER_CLOSED
+
+AF_INET=2
+AF_INET6=10
+
+verify_listener_events()
+{
+	local evt=$1
+	local e_type=$2
+	local e_family=$3
+	local e_saddr=$4
+	local e_sport=$5
+	local type
+	local family
+	local saddr
+	local sport
+
+	if [ $e_type = $LISTENER_CREATED ]; then
+		stdbuf -o0 -e0 printf "\t\t\t\t\t CREATE_LISTENER %s:%s"\
+			$e_saddr $e_sport
+	elif [ $e_type = $LISTENER_CLOSED ]; then
+		stdbuf -o0 -e0 printf "\t\t\t\t\t CLOSE_LISTENER %s:%s "\
+			$e_saddr $e_sport
+	fi
+
+	type=$(grep "type:$e_type," $evt |
+	       sed --unbuffered -n 's/.*\(type:\)\([[:digit:]]*\).*$/\2/p;q')
+	family=$(grep "type:$e_type," $evt |
+		 sed --unbuffered -n 's/.*\(family:\)\([[:digit:]]*\).*$/\2/p;q')
+	sport=$(grep "type:$e_type," $evt |
+		sed --unbuffered -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q')
+	if [ $family = $AF_INET6 ]; then
+		saddr=$(grep "type:$e_type," $evt |
+			sed --unbuffered -n 's/.*\(saddr6:\)\([0-9a-f:.]*\).*$/\2/p;q')
+	else
+		saddr=$(grep "type:$e_type," $evt |
+			sed --unbuffered -n 's/.*\(saddr4:\)\([0-9.]*\).*$/\2/p;q')
+	fi
+
+	if [ $type = $e_type ] && [ $family = $e_family ] &&
+	   [ $saddr = $e_saddr ] && [ $sport = $e_sport ]; then
+		stdbuf -o0 -e0 printf "[ ok ]\n"
+		return 0
+	fi
+	stdbuf -o0 -e0 printf "[fail]\n"
+}
+
 add_addr_ports_tests()
 {
 	# signal address with port
@@ -2533,7 +2581,16 @@ add_addr_ports_tests()
 	fi
 
 	# single address with port, remove
+	# pm listener events
 	if reset "remove single address with port"; then
+		local evts
+		local pid
+
+		evts=$(mktemp)
+		:> $evts
+		ip netns exec $ns1 ./pm_nl_ctl events >> $evts 2>&1 &
+		pid=$!
+
 		pm_nl_set_limits $ns1 0 1
 		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal port 10100
 		pm_nl_set_limits $ns2 1 1
@@ -2541,6 +2598,11 @@ add_addr_ports_tests()
 		chk_join_nr 1 1 1
 		chk_add_nr 1 1 1
 		chk_rm_nr 1 1 invert
+
+		verify_listener_events $evts 15 $AF_INET 10.0.2.1 10100
+		verify_listener_events $evts 16 $AF_INET 10.0.2.1 10100
+		kill_wait $pid
+		rm -rf $evts
 	fi
 
 	# subflow and signal with port, remove
-- 
2.35.3
Re: [PATCH mptcp-next v5 4/4] selftests: mptcp: listener test for in-kernel PM
Posted by Paolo Abeni 3 years, 2 months ago 
Hello,

sorry for the late feedback.

On Thu, 2022-11-17 at 13:31 +0800, Geliang Tang wrote:
> This patch adds test coverage for listening sockets created by the
> in-kernel path manager in mptcp_join.sh.
> 
> It adds the listener event checking in the existing "remove single
> address with port" test. The output looks like this:
> 
>  003 remove single address with port syn[ ok ] - synack[ ok ] - ack[ ok ]
>                                      add[ ok ] - echo  [ ok ] - pt [ ok ]
>                                      syn[ ok ] - synack[ ok ] - ack[ ok ]
>                                      syn[ ok ] - ack   [ ok ]
>                                      rm [ ok ] - rmsf  [ ok ]   invert
>                                      CREATE_LISTENER 10.0.2.1:10100[ ok ]
>                                      CLOSE_LISTENER 10.0.2.1:10100 [ ok ]
> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  .../testing/selftests/net/mptcp/mptcp_join.sh | 62 +++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index f10ef65a7009..89a30225bbd7 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -2509,6 +2509,54 @@ backup_tests()
>  	fi
>  }
>  
> +LISTENER_CREATED=15 #MPTCP_EVENT_LISTENER_CREATED
> +LISTENER_CLOSED=16  #MPTCP_EVENT_LISTENER_CLOSED
> +
> +AF_INET=2
> +AF_INET6=10
> +
> +verify_listener_events()
> +{
> +	local evt=$1
> +	local e_type=$2
> +	local e_family=$3
> +	local e_saddr=$4
> +	local e_sport=$5
> +	local type
> +	local family
> +	local saddr
> +	local sport
> +
> +	if [ $e_type = $LISTENER_CREATED ]; then
> +		stdbuf -o0 -e0 printf "\t\t\t\t\t CREATE_LISTENER %s:%s"\
> +			$e_saddr $e_sport
> +	elif [ $e_type = $LISTENER_CLOSED ]; then
> +		stdbuf -o0 -e0 printf "\t\t\t\t\t CLOSE_LISTENER %s:%s "\
> +			$e_saddr $e_sport
> +	fi
> +
> +	type=$(grep "type:$e_type," $evt |
> +	       sed --unbuffered -n 's/.*\(type:\)\([[:digit:]]*\).*$/\2/p;q')
> +	family=$(grep "type:$e_type," $evt |
> +		 sed --unbuffered -n 's/.*\(family:\)\([[:digit:]]*\).*$/\2/p;q')
> +	sport=$(grep "type:$e_type," $evt |
> +		sed --unbuffered -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q')

This does not handle properly error conditions: $family, $type or
$sport could be undef in case of bugs, you need to quote them
everywhere.

> +	if [ $family = $AF_INET6 ]; then
> +		saddr=$(grep "type:$e_type," $evt |
> +			sed --unbuffered -n 's/.*\(saddr6:\)\([0-9a-f:.]*\).*$/\2/p;q')
> +	else
> +		saddr=$(grep "type:$e_type," $evt |
> +			sed --unbuffered -n 's/.*\(saddr4:\)\([0-9.]*\).*$/\2/p;q')
> +	fi
> +
> +	if [ $type = $e_type ] && [ $family = $e_family ] &&
> +	   [ $saddr = $e_saddr ] && [ $sport = $e_sport ]; then
> +		stdbuf -o0 -e0 printf "[ ok ]\n"
> +		return 0
> +	fi
> +	stdbuf -o0 -e0 printf "[fail]\n"

This is not enough, you must additionally call

	fail_test

to properly report the failure to the CI, otherwise the test will be
considered successful even when the condition is not meet.

> +}
> +
>  add_addr_ports_tests()
>  {
>  	# signal address with port
> @@ -2533,7 +2581,16 @@ add_addr_ports_tests()
>  	fi
>  
>  	# single address with port, remove
> +	# pm listener events
>  	if reset "remove single address with port"; then
> +		local evts
> +		local pid
> +
> +		evts=$(mktemp)
> +		:> $evts
> +		ip netns exec $ns1 ./pm_nl_ctl events >> $evts 2>&1 &
> +		pid=$!
> +
>  		pm_nl_set_limits $ns1 0 1
>  		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal port 10100
>  		pm_nl_set_limits $ns2 1 1
> @@ -2541,6 +2598,11 @@ add_addr_ports_tests()
>  		chk_join_nr 1 1 1
>  		chk_add_nr 1 1 1
>  		chk_rm_nr 1 1 invert
> +
> +		verify_listener_events $evts 15 $AF_INET 10.0.2.1 10100
> +		verify_listener_events $evts 16 $AF_INET 10.0.2.1 10100
> +		kill_wait $pid
> +		rm -rf $evts

is better if you make the 'evts' variable global, moving its definition
in the initial global variable declaration and move the 'rm -rf $evts'
in cleanup() 

@Geliang: could you please send a squash-to follow tacking care of
the above?

Thanks!

Paolo
Re: [PATCH mptcp-next v5 4/4] selftests: mptcp: listener test for in-kernel PM
Posted by Geliang Tang 3 years, 2 months ago 
On Wed, Nov 23, 2022 at 12:49:58PM +0100, Paolo Abeni wrote:
> Hello,
> 
> sorry for the late feedback.
> 
> On Thu, 2022-11-17 at 13:31 +0800, Geliang Tang wrote:
> > This patch adds test coverage for listening sockets created by the
> > in-kernel path manager in mptcp_join.sh.
> > 
> > It adds the listener event checking in the existing "remove single
> > address with port" test. The output looks like this:
> > 
> >  003 remove single address with port syn[ ok ] - synack[ ok ] - ack[ ok ]
> >                                      add[ ok ] - echo  [ ok ] - pt [ ok ]
> >                                      syn[ ok ] - synack[ ok ] - ack[ ok ]
> >                                      syn[ ok ] - ack   [ ok ]
> >                                      rm [ ok ] - rmsf  [ ok ]   invert
> >                                      CREATE_LISTENER 10.0.2.1:10100[ ok ]
> >                                      CLOSE_LISTENER 10.0.2.1:10100 [ ok ]
> > 
> > Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> > ---
> >  .../testing/selftests/net/mptcp/mptcp_join.sh | 62 +++++++++++++++++++
> >  1 file changed, 62 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > index f10ef65a7009..89a30225bbd7 100755
> > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > @@ -2509,6 +2509,54 @@ backup_tests()
> >  	fi
> >  }
> >  
> > +LISTENER_CREATED=15 #MPTCP_EVENT_LISTENER_CREATED
> > +LISTENER_CLOSED=16  #MPTCP_EVENT_LISTENER_CLOSED
> > +
> > +AF_INET=2
> > +AF_INET6=10
> > +
> > +verify_listener_events()
> > +{
> > +	local evt=$1
> > +	local e_type=$2
> > +	local e_family=$3
> > +	local e_saddr=$4
> > +	local e_sport=$5
> > +	local type
> > +	local family
> > +	local saddr
> > +	local sport
> > +
> > +	if [ $e_type = $LISTENER_CREATED ]; then
> > +		stdbuf -o0 -e0 printf "\t\t\t\t\t CREATE_LISTENER %s:%s"\
> > +			$e_saddr $e_sport
> > +	elif [ $e_type = $LISTENER_CLOSED ]; then
> > +		stdbuf -o0 -e0 printf "\t\t\t\t\t CLOSE_LISTENER %s:%s "\
> > +			$e_saddr $e_sport
> > +	fi
> > +
> > +	type=$(grep "type:$e_type," $evt |
> > +	       sed --unbuffered -n 's/.*\(type:\)\([[:digit:]]*\).*$/\2/p;q')
> > +	family=$(grep "type:$e_type," $evt |
> > +		 sed --unbuffered -n 's/.*\(family:\)\([[:digit:]]*\).*$/\2/p;q')
> > +	sport=$(grep "type:$e_type," $evt |
> > +		sed --unbuffered -n 's/.*\(sport:\)\([[:digit:]]*\).*$/\2/p;q')
> 
> This does not handle properly error conditions: $family, $type or
> $sport could be undef in case of bugs, you need to quote them
> everywhere.
> 
> > +	if [ $family = $AF_INET6 ]; then
> > +		saddr=$(grep "type:$e_type," $evt |
> > +			sed --unbuffered -n 's/.*\(saddr6:\)\([0-9a-f:.]*\).*$/\2/p;q')
> > +	else
> > +		saddr=$(grep "type:$e_type," $evt |
> > +			sed --unbuffered -n 's/.*\(saddr4:\)\([0-9.]*\).*$/\2/p;q')
> > +	fi
> > +
> > +	if [ $type = $e_type ] && [ $family = $e_family ] &&
> > +	   [ $saddr = $e_saddr ] && [ $sport = $e_sport ]; then
> > +		stdbuf -o0 -e0 printf "[ ok ]\n"
> > +		return 0
> > +	fi
> > +	stdbuf -o0 -e0 printf "[fail]\n"
> 
> This is not enough, you must additionally call
> 
> 	fail_test
> 
> to properly report the failure to the CI, otherwise the test will be
> considered successful even when the condition is not meet.
> 
> > +}
> > +
> >  add_addr_ports_tests()
> >  {
> >  	# signal address with port
> > @@ -2533,7 +2581,16 @@ add_addr_ports_tests()
> >  	fi
> >  
> >  	# single address with port, remove
> > +	# pm listener events
> >  	if reset "remove single address with port"; then
> > +		local evts
> > +		local pid
> > +
> > +		evts=$(mktemp)
> > +		:> $evts
> > +		ip netns exec $ns1 ./pm_nl_ctl events >> $evts 2>&1 &
> > +		pid=$!
> > +
> >  		pm_nl_set_limits $ns1 0 1
> >  		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal port 10100
> >  		pm_nl_set_limits $ns2 1 1
> > @@ -2541,6 +2598,11 @@ add_addr_ports_tests()
> >  		chk_join_nr 1 1 1
> >  		chk_add_nr 1 1 1
> >  		chk_rm_nr 1 1 invert
> > +
> > +		verify_listener_events $evts 15 $AF_INET 10.0.2.1 10100
> > +		verify_listener_events $evts 16 $AF_INET 10.0.2.1 10100
> > +		kill_wait $pid
> > +		rm -rf $evts

Thanks Paolo, I just sent the squash-to patches to fix them.

> 
> is better if you make the 'evts' variable global, moving its definition
> in the initial global variable declaration and move the 'rm -rf $evts'
> in cleanup() 

Here I prefer to keep 'evts' as a local variable, since all other 'evts'
both in userspace_pm.sh and in do_transfer() in mptcp_join.sh are local
variables.

Thanks,
-Geliang

> 
> @Geliang: could you please send a squash-to follow tacking care of
> the above?
> 
> Thanks!
> 
> Paolo
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.