[PATCH mptcp-next v5 0/4] add pm listener events

[PATCH mptcp-next v5 0/4] add pm listener events

[Geliang Tang]

v5:
- use GFP_KERNEL instead of GFP_ATOMIC in patch 1.
- add the listener event checking in the existing test in patch 4.
- update subjects and commit logs.

v4:
- split the selftests into two patches.
- do some cleanups.

v3:
- add selftests.

v2:
- send created event from mptcp_listen.
- add a new patch, use entry->lsk directly.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/313

Geliang Tang (4):
  mptcp: add pm listener events
  selftests: mptcp: enhance userspace pm tests
  selftests: mptcp: listener test for userspace PM
  selftests: mptcp: listener test for in-kernel PM

 include/uapi/linux/mptcp.h                    |  9 ++
 net/mptcp/pm_netlink.c                        | 57 ++++++++++++
 net/mptcp/protocol.c                          |  3 +
 net/mptcp/protocol.h                          |  2 +
 .../testing/selftests/net/mptcp/mptcp_join.sh | 65 +++++++++++++-
 .../selftests/net/mptcp/userspace_pm.sh       | 89 ++++++++++++++++++-
 6 files changed, 221 insertions(+), 4 deletions(-)

-- 
2.35.3

[PATCH mptcp-next v5 1/4] mptcp: add pm listener events

[Geliang Tang]

This patch adds two new MPTCP netlink event types for PM listening
socket create and close, named MPTCP_EVENT_LISTENER_CREATED and
MPTCP_EVENT_LISTENER_CLOSED.

Add a new function mptcp_event_pm_listener() to push the new events
with family, port and addr to userspace.

Invoke mptcp_event_pm_listener() with MPTCP_EVENT_LISTENER_CREATED in
mptcp_listen() and mptcp_pm_nl_create_listen_socket(), invoke it with
MPTCP_EVENT_LISTENER_CLOSED in __mptcp_close_ssk().

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 include/uapi/linux/mptcp.h |  9 ++++++
 net/mptcp/pm_netlink.c     | 57 ++++++++++++++++++++++++++++++++++++++
 net/mptcp/protocol.c       |  3 ++
 net/mptcp/protocol.h       |  2 ++
 4 files changed, 71 insertions(+)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index dfe19bf13f4c..32af2d278cb4 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -160,6 +160,12 @@ struct mptcp_info {
  *                           daddr4 | daddr6, sport, dport, backup, if_idx
  *                           [, error]
  * The priority of a subflow has changed. 'error' should not be set.
+ *
+ * MPTCP_EVENT_LISTENER_CREATED: family, sport, saddr4 | saddr6
+ * A new PM listener is created.
+ *
+ * MPTCP_EVENT_LISTENER_CLOSED: family, sport, saddr4 | saddr6
+ * A PM listener is closed.
  */
 enum mptcp_event_type {
 	MPTCP_EVENT_UNSPEC = 0,
@@ -174,6 +180,9 @@ enum mptcp_event_type {
 	MPTCP_EVENT_SUB_CLOSED = 11,
 
 	MPTCP_EVENT_SUB_PRIORITY = 13,
+
+	MPTCP_EVENT_LISTENER_CREATED = 15,
+	MPTCP_EVENT_LISTENER_CLOSED = 16,
 };
 
 enum mptcp_event_attr {
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 08806f97c8fb..93ae0fc892df 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1026,6 +1026,8 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
 	if (err)
 		return err;
 
+	mptcp_event_pm_listener(ssock->sk, MPTCP_EVENT_LISTENER_CREATED);
+
 	return 0;
 }
 
@@ -2149,6 +2151,58 @@ void mptcp_event_addr_announced(const struct sock *ssk,
 	kfree_skb(skb);
 }
 
+void mptcp_event_pm_listener(const struct sock *ssk,
+			     enum mptcp_event_type event)
+{
+	const struct inet_sock *issk = inet_sk(ssk);
+	struct net *net = sock_net(ssk);
+	struct nlmsghdr *nlh;
+	struct sk_buff *skb;
+
+	if (!genl_has_listeners(&mptcp_genl_family, net, MPTCP_PM_EV_GRP_OFFSET))
+		return;
+
+	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!skb)
+		return;
+
+	nlh = genlmsg_put(skb, 0, 0, &mptcp_genl_family, 0, event);
+	if (!nlh)
+		goto nla_put_failure;
+
+	if (nla_put_u16(skb, MPTCP_ATTR_FAMILY, ssk->sk_family))
+		goto nla_put_failure;
+
+	if (nla_put_be16(skb, MPTCP_ATTR_SPORT, issk->inet_sport))
+		goto nla_put_failure;
+
+	switch (ssk->sk_family) {
+	case AF_INET:
+		if (nla_put_in_addr(skb, MPTCP_ATTR_SADDR4, issk->inet_saddr))
+			goto nla_put_failure;
+		break;
+#if IS_ENABLED(CONFIG_MPTCP_IPV6)
+	case AF_INET6: {
+		const struct ipv6_pinfo *np = inet6_sk(ssk);
+
+		if (nla_put_in6_addr(skb, MPTCP_ATTR_SADDR6, &np->saddr))
+			goto nla_put_failure;
+		break;
+	}
+#endif
+	default:
+		WARN_ON_ONCE(1);
+		goto nla_put_failure;
+	}
+
+	genlmsg_end(skb, nlh);
+	mptcp_nl_mcast_send(net, skb, GFP_KERNEL);
+	return;
+
+nla_put_failure:
+	kfree_skb(skb);
+}
+
 void mptcp_event(enum mptcp_event_type type, const struct mptcp_sock *msk,
 		 const struct sock *ssk, gfp_t gfp)
 {
@@ -2194,6 +2248,9 @@ void mptcp_event(enum mptcp_event_type type, const struct mptcp_sock *msk,
 		if (mptcp_event_sub_closed(skb, msk, ssk) < 0)
 			goto nla_put_failure;
 		break;
+	case MPTCP_EVENT_LISTENER_CREATED:
+	case MPTCP_EVENT_LISTENER_CLOSED:
+		break;
 	}
 
 	genlmsg_end(skb, nlh);
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 1f9b72b62998..c2bb4255969e 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2366,6 +2366,7 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
 			tcp_set_state(ssk, TCP_CLOSE);
 			mptcp_subflow_queue_clean(ssk);
 			inet_csk_listen_stop(ssk);
+			mptcp_event_pm_listener(ssk, MPTCP_EVENT_LISTENER_CLOSED);
 		}
 		__tcp_close(ssk, 0);
 
@@ -3682,6 +3683,8 @@ static int mptcp_listen(struct socket *sock, int backlog)
 	if (!err)
 		mptcp_copy_inaddrs(sock->sk, ssock->sk);
 
+	mptcp_event_pm_listener(ssock->sk, MPTCP_EVENT_LISTENER_CREATED);
+
 unlock:
 	release_sock(sock->sk);
 	return err;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index ffecd103cc50..bae216bff6e4 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -846,6 +846,8 @@ void mptcp_event(enum mptcp_event_type type, const struct mptcp_sock *msk,
 		 const struct sock *ssk, gfp_t gfp);
 void mptcp_event_addr_announced(const struct sock *ssk, const struct mptcp_addr_info *info);
 void mptcp_event_addr_removed(const struct mptcp_sock *msk, u8 id);
+void mptcp_event_pm_listener(const struct sock *ssk,
+			     enum mptcp_event_type event);
 bool mptcp_userspace_pm_active(const struct mptcp_sock *msk);
 
 void mptcp_fastopen_gen_msk_ackseq(struct mptcp_sock *msk, struct mptcp_subflow_context *subflow,
-- 
2.35.3

[PATCH mptcp-next v5 2/4] selftests: mptcp: enhance userspace pm tests

[Geliang Tang]

Some userspace pm tests failed since pm listener events have been added.
Now MPTCP_EVENT_LISTENER_CREATED event becomes the first item in the
events list like this:

 type:15,family:2,sport:10006,saddr4:0.0.0.0
 type:1,token:3701282876,server_side:1,family:2,saddr4:10.0.1.1,...

And no token value in this MPTCP_EVENT_LISTENER_CREATED event.

This patch fixes this by specifying the type 1 item to search for token
values.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh   | 3 ++-
 tools/testing/selftests/net/mptcp/userspace_pm.sh | 7 ++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 2a402b3b771f..f10ef65a7009 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -830,7 +830,8 @@ do_transfer()
 			if [ $userspace_pm -eq 0 ]; then
 				pm_nl_add_endpoint $ns1 $addr flags signal
 			else
-				tk=$(sed -n 's/.*\(token:\)\([[:digit:]]*\).*$/\2/p;q' "$evts_ns1")
+				tk=$(grep "type:1," "$evts_ns1" |
+				     sed -n 's/.*\(token:\)\([[:digit:]]*\).*$/\2/p;q')
 				ip netns exec ${listener_ns} ./pm_nl_ctl ann $addr token $tk id $id
 				sleep 1
 				ip netns exec ${listener_ns} ./pm_nl_ctl rem token $tk id $id
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 5dfc3ee74b98..08a88ea47a29 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -172,9 +172,10 @@ make_connection()
 	client_serverside=$(sed --unbuffered -n 's/.*\(server_side:\)\([[:digit:]]*\).*$/\2/p;q'\
 				      "$client_evts")
 	kill_wait $server_evts_pid
-	server_token=$(sed --unbuffered -n 's/.*\(token:\)\([[:digit:]]*\).*$/\2/p;q' "$server_evts")
-	server_serverside=$(sed --unbuffered -n 's/.*\(server_side:\)\([[:digit:]]*\).*$/\2/p;q'\
-				      "$server_evts")
+	server_token=$(grep "type:1," "$server_evts" |
+		       sed --unbuffered -n 's/.*\(token:\)\([[:digit:]]*\).*$/\2/p;q')
+	server_serverside=$(grep "type:1," "$server_evts" |
+			    sed --unbuffered -n 's/.*\(server_side:\)\([[:digit:]]*\).*$/\2/p;q')
 	rm -f "$client_evts" "$server_evts" "$file"
 
 	if [ "$client_token" != "" ] && [ "$server_token" != "" ] && [ "$client_serverside" = 0 ] &&
-- 
2.35.3

[PATCH mptcp-next v5 3/4] selftests: mptcp: listener test for userspace PM

[Geliang Tang]

This patch adds test coverage for listening sockets created by the
userspace path manager in userspace_pm.sh.

It adds a new test named test_listener() and a new verifying helper
verify_listener_events(). The new output looks like this:

 CREATE_SUBFLOW 10.0.2.2 (ns2) => 10.0.2.1 (ns1)              [OK]
 DESTROY_SUBFLOW 10.0.2.2 (ns2) => 10.0.2.1 (ns1)             [OK]
 MP_PRIO TX                                                   [OK]
 MP_PRIO RX                                                   [OK]
 CREATE_LISTENER 10.0.2.2:37106				      [OK]
 CLOSE_LISTENER 10.0.2.2:37106				      [OK]

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

diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 08a88ea47a29..40801279ebdf 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -11,6 +11,8 @@ ANNOUNCED=6        # MPTCP_EVENT_ANNOUNCED
 REMOVED=7          # MPTCP_EVENT_REMOVED
 SUB_ESTABLISHED=10 # MPTCP_EVENT_SUB_ESTABLISHED
 SUB_CLOSED=11      # MPTCP_EVENT_SUB_CLOSED
+LISTENER_CREATED=15 #MPTCP_EVENT_LISTENER_CREATED
+LISTENER_CLOSED=16  #MPTCP_EVENT_LISTENER_CLOSED
 
 AF_INET=2
 AF_INET6=10
@@ -808,11 +810,91 @@ test_prio()
 	fi
 }
 
+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 "CREATE_LISTENER %s:%s\t\t\t\t\t"\
+			$e_saddr $e_sport
+	elif [ $e_type = $LISTENER_CLOSED ]; then
+		stdbuf -o0 -e0 printf "CLOSE_LISTENER %s:%s\t\t\t\t\t"\
+			$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"
+	exit 1
+}
+
+test_listener()
+{
+	local evts
+	evts=$(mktemp)
+	# Capture events on the network namespace running the server
+	:>$evts
+	ip netns exec $ns2 ./pm_nl_ctl events >> $evts 2>&1 &
+	evts_pid=$!
+	sleep 0.5
+
+	# Attempt to add a listener at 10.0.2.2:<subflow-port>
+	ip netns exec $ns2 ./pm_nl_ctl listen 10.0.2.2\
+		$client4_port > /dev/null 2>&1 &
+	local listener_pid=$!
+
+	verify_listener_events $evts 15 $AF_INET 10.0.2.2 $client4_port
+
+	# ADD_ADDR from client to server machine reusing the subflow port
+	ip netns exec $ns2 ./pm_nl_ctl ann 10.0.2.2 token $client4_token id\
+		$client_addr_id > /dev/null 2>&1
+	sleep 0.5
+
+	# CREATE_SUBFLOW from server to client machine
+	ip netns exec $ns1 ./pm_nl_ctl csf lip 10.0.2.1 lid 23 rip 10.0.2.2\
+		rport $client4_port token $server4_token > /dev/null 2>&1
+	sleep 0.5
+
+	# Delete the listener from the client ns, if one was created
+	kill_wait $listener_pid
+
+	verify_listener_events $evts 16 $AF_INET 10.0.2.2 $client4_port
+
+	kill_wait $evts_pid
+	rm -f $evts
+}
+
 make_connection
 make_connection "v6"
 test_announce
 test_remove
 test_subflows
 test_prio
+test_listener
 
 exit 0
-- 
2.35.3

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

[Geliang Tang]

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: selftests: mptcp: listener test for in-kernel PM: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6305794950430720
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6305794950430720/summary/summary.txt

- KVM Validation: debug:
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5653956353851392
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5653956353851392/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/49c535583971


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)

Re: [PATCH mptcp-next v5 0/4] add pm listener events

[Mat Martineau]

On Thu, 17 Nov 2022, Geliang Tang wrote:

> v5:
> - use GFP_KERNEL instead of GFP_ATOMIC in patch 1.
> - add the listener event checking in the existing test in patch 4.
> - update subjects and commit logs.
>

v5 looks good to me (one minor commit message change noted on patch 3, if 
it's ok with Geliang):

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

> v4:
> - split the selftests into two patches.
> - do some cleanups.
>
> v3:
> - add selftests.
>
> v2:
> - send created event from mptcp_listen.
> - add a new patch, use entry->lsk directly.
>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/313
>
> Geliang Tang (4):
>  mptcp: add pm listener events
>  selftests: mptcp: enhance userspace pm tests
>  selftests: mptcp: listener test for userspace PM
>  selftests: mptcp: listener test for in-kernel PM
>
> include/uapi/linux/mptcp.h                    |  9 ++
> net/mptcp/pm_netlink.c                        | 57 ++++++++++++
> net/mptcp/protocol.c                          |  3 +
> net/mptcp/protocol.h                          |  2 +
> .../testing/selftests/net/mptcp/mptcp_join.sh | 65 +++++++++++++-
> .../selftests/net/mptcp/userspace_pm.sh       | 89 ++++++++++++++++++-
> 6 files changed, 221 insertions(+), 4 deletions(-)
>
> -- 
> 2.35.3
>
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next v5 3/4] selftests: mptcp: listener test for userspace PM

[Mat Martineau]

On Thu, 17 Nov 2022, Geliang Tang wrote:

> This patch adds test coverage for listening sockets created by the
> userspace path manager in userspace_pm.sh.

I suggest changing this sentence to: "This patch adds test coverage for 
listening sockets created by userspace processes."

(the events are created for any MPTCP listener socket, not only the ones 
created by a path manager daemon)

- Mat

>
> It adds a new test named test_listener() and a new verifying helper
> verify_listener_events(). The new output looks like this:
>
> CREATE_SUBFLOW 10.0.2.2 (ns2) => 10.0.2.1 (ns1)              [OK]
> DESTROY_SUBFLOW 10.0.2.2 (ns2) => 10.0.2.1 (ns1)             [OK]
> MP_PRIO TX                                                   [OK]
> MP_PRIO RX                                                   [OK]
> CREATE_LISTENER 10.0.2.2:37106				      [OK]
> CLOSE_LISTENER 10.0.2.2:37106				      [OK]
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> .../selftests/net/mptcp/userspace_pm.sh       | 82 +++++++++++++++++++
> 1 file changed, 82 insertions(+)
>
> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> index 08a88ea47a29..40801279ebdf 100755
> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> @@ -11,6 +11,8 @@ ANNOUNCED=6        # MPTCP_EVENT_ANNOUNCED
> REMOVED=7          # MPTCP_EVENT_REMOVED
> SUB_ESTABLISHED=10 # MPTCP_EVENT_SUB_ESTABLISHED
> SUB_CLOSED=11      # MPTCP_EVENT_SUB_CLOSED
> +LISTENER_CREATED=15 #MPTCP_EVENT_LISTENER_CREATED
> +LISTENER_CLOSED=16  #MPTCP_EVENT_LISTENER_CLOSED
>
> AF_INET=2
> AF_INET6=10
> @@ -808,11 +810,91 @@ test_prio()
> 	fi
> }
>
> +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 "CREATE_LISTENER %s:%s\t\t\t\t\t"\
> +			$e_saddr $e_sport
> +	elif [ $e_type = $LISTENER_CLOSED ]; then
> +		stdbuf -o0 -e0 printf "CLOSE_LISTENER %s:%s\t\t\t\t\t"\
> +			$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"
> +	exit 1
> +}
> +
> +test_listener()
> +{
> +	local evts
> +	evts=$(mktemp)
> +	# Capture events on the network namespace running the server
> +	:>$evts
> +	ip netns exec $ns2 ./pm_nl_ctl events >> $evts 2>&1 &
> +	evts_pid=$!
> +	sleep 0.5
> +
> +	# Attempt to add a listener at 10.0.2.2:<subflow-port>
> +	ip netns exec $ns2 ./pm_nl_ctl listen 10.0.2.2\
> +		$client4_port > /dev/null 2>&1 &
> +	local listener_pid=$!
> +
> +	verify_listener_events $evts 15 $AF_INET 10.0.2.2 $client4_port
> +
> +	# ADD_ADDR from client to server machine reusing the subflow port
> +	ip netns exec $ns2 ./pm_nl_ctl ann 10.0.2.2 token $client4_token id\
> +		$client_addr_id > /dev/null 2>&1
> +	sleep 0.5
> +
> +	# CREATE_SUBFLOW from server to client machine
> +	ip netns exec $ns1 ./pm_nl_ctl csf lip 10.0.2.1 lid 23 rip 10.0.2.2\
> +		rport $client4_port token $server4_token > /dev/null 2>&1
> +	sleep 0.5
> +
> +	# Delete the listener from the client ns, if one was created
> +	kill_wait $listener_pid
> +
> +	verify_listener_events $evts 16 $AF_INET 10.0.2.2 $client4_port
> +
> +	kill_wait $evts_pid
> +	rm -f $evts
> +}
> +
> make_connection
> make_connection "v6"
> test_announce
> test_remove
> test_subflows
> test_prio
> +test_listener
>
> exit 0
> -- 
> 2.35.3
>
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next v5 0/4] add pm listener events

[Geliang Tang]

On Thu, Nov 17, 2022 at 05:44:10PM -0800, Mat Martineau wrote:
> On Thu, 17 Nov 2022, Geliang Tang wrote:
> 
> > v5:
> > - use GFP_KERNEL instead of GFP_ATOMIC in patch 1.
> > - add the listener event checking in the existing test in patch 4.
> > - update subjects and commit logs.
> > 
> 
> v5 looks good to me (one minor commit message change noted on patch 3, if
> it's ok with Geliang):

Thanks, I agree. Please help me update this commit message when merging
it.

-Geliang

> 
> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> 
> > v4:
> > - split the selftests into two patches.
> > - do some cleanups.
> > 
> > v3:
> > - add selftests.
> > 
> > v2:
> > - send created event from mptcp_listen.
> > - add a new patch, use entry->lsk directly.
> > 
> > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/313
> > 
> > Geliang Tang (4):
> >  mptcp: add pm listener events
> >  selftests: mptcp: enhance userspace pm tests
> >  selftests: mptcp: listener test for userspace PM
> >  selftests: mptcp: listener test for in-kernel PM
> > 
> > include/uapi/linux/mptcp.h                    |  9 ++
> > net/mptcp/pm_netlink.c                        | 57 ++++++++++++
> > net/mptcp/protocol.c                          |  3 +
> > net/mptcp/protocol.h                          |  2 +
> > .../testing/selftests/net/mptcp/mptcp_join.sh | 65 +++++++++++++-
> > .../selftests/net/mptcp/userspace_pm.sh       | 89 ++++++++++++++++++-
> > 6 files changed, 221 insertions(+), 4 deletions(-)
> > 
> > -- 
> > 2.35.3
> > 
> > 
> > 
> 
> --
> Mat Martineau
> Intel

Re: selftests: mptcp: listener test for in-kernel PM: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Unstable: 1 failed test(s): selftest_simult_flows 🔴:
  - Task: https://cirrus-ci.com/task/6178938796376064
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6178938796376064/summary/summary.txt

- KVM Validation: debug:
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5924801957593088
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5924801957593088/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/9b92ff8007de


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)

Re: [PATCH mptcp-next v5 0/4] add pm listener events

[Matthieu Baerts]

Hi Geliang, Mat,

On 17/11/2022 06:31, Geliang Tang wrote:
> v5:
> - use GFP_KERNEL instead of GFP_ATOMIC in patch 1.
> - add the listener event checking in the existing test in patch 4.
> - update subjects and commit logs.
> 
> v4:
> - split the selftests into two patches.
> - do some cleanups.
> 
> v3:
> - add selftests.
> 
> v2:
> - send created event from mptcp_listen.
> - add a new patch, use entry->lsk directly.
> 
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/313
> 
> Geliang Tang (4):
>   mptcp: add pm listener events
>   selftests: mptcp: enhance userspace pm tests
>   selftests: mptcp: listener test for userspace PM
>   selftests: mptcp: listener test for in-kernel PM

Thank you for the patches and the reviews!

Now in our tree (feat. for net-next)

New patches for t/upstream:
- a6871c7e014a: mptcp: add pm listener events
- 4aabc20960d4: selftests: mptcp: enhance userspace pm tests
- 83027c9e1d0c: selftests: mptcp: listener test for userspace PM
- e33052bbf56f: selftests: mptcp: listener test for in-kernel PM
- Results: 48478f54612d..077c54d47661 (export)

- e9fa9c5f6951: tg:msg: events are for any MPTCP listener socket
- Results: 077c54d47661..69720c6cd168 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20221118T102434

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

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

[Paolo Abeni]

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 3/4] selftests: mptcp: listener test for userspace PM

[Paolo Abeni]

On Thu, 2022-11-17 at 13:31 +0800, Geliang Tang wrote:
> This patch adds test coverage for listening sockets created by the
> userspace path manager in userspace_pm.sh.
> 
> It adds a new test named test_listener() and a new verifying helper
> verify_listener_events(). The new output looks like this:
> 
>  CREATE_SUBFLOW 10.0.2.2 (ns2) => 10.0.2.1 (ns1)              [OK]
>  DESTROY_SUBFLOW 10.0.2.2 (ns2) => 10.0.2.1 (ns1)             [OK]
>  MP_PRIO TX                                                   [OK]
>  MP_PRIO RX                                                   [OK]
>  CREATE_LISTENER 10.0.2.2:37106				      [OK]
>  CLOSE_LISTENER 10.0.2.2:37106				      [OK]
> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  .../selftests/net/mptcp/userspace_pm.sh       | 82 +++++++++++++++++++
>  1 file changed, 82 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> index 08a88ea47a29..40801279ebdf 100755
> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> @@ -11,6 +11,8 @@ ANNOUNCED=6        # MPTCP_EVENT_ANNOUNCED
>  REMOVED=7          # MPTCP_EVENT_REMOVED
>  SUB_ESTABLISHED=10 # MPTCP_EVENT_SUB_ESTABLISHED
>  SUB_CLOSED=11      # MPTCP_EVENT_SUB_CLOSED
> +LISTENER_CREATED=15 #MPTCP_EVENT_LISTENER_CREATED
> +LISTENER_CLOSED=16  #MPTCP_EVENT_LISTENER_CLOSED
>  
>  AF_INET=2
>  AF_INET6=10
> @@ -808,11 +810,91 @@ test_prio()
>  	fi
>  }
>  
> +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 "CREATE_LISTENER %s:%s\t\t\t\t\t"\
> +			$e_saddr $e_sport
> +	elif [ $e_type = $LISTENER_CLOSED ]; then
> +		stdbuf -o0 -e0 printf "CLOSE_LISTENER %s:%s\t\t\t\t\t"\
> +			$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"
> +	exit 1

This has similar issues to the patch 4/4, with the main difference that
we dont need the test_fail call here: exit 1 is enough.


@Geliang: could you please take care of the follow-up here, too?

Thanks!

Paolo

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

[Geliang Tang]

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
>