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

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

[Geliang Tang]

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: update userspace pm tests
  selftests: mptcp: pm_listener tests for userspace_pm
  selftests: mptcp: pm_listener tests for mptcp_join

 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 | 75 +++++++++++++++-
 .../selftests/net/mptcp/userspace_pm.sh       | 89 ++++++++++++++++++-
 6 files changed, 231 insertions(+), 4 deletions(-)

-- 
2.35.3

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

[Geliang Tang]

This patch adds MPTCP netlink events for PM listening socket create and
close.

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..685240f6bc80 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_ATOMIC);
+	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_ATOMIC);
+	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 v4 2/4] selftests: mptcp: update userspace pm tests

[Geliang Tang]

Some userspace pm tests failed since pm listener events have been added.
Fix them.

patch 3 in "add pm listener events" series.

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 v4 3/4] selftests: mptcp: pm_listener tests for userspace_pm

[Geliang Tang]

Add pm_listener selftests for userspace_pm.sh.

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 v4 4/4] selftests: mptcp: pm_listener tests for mptcp_join

[Geliang Tang]

Add pm_listener selftests for mptcp_join.sh.

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

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index f10ef65a7009..0810e9c12fd5 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
@@ -2589,6 +2637,30 @@ add_addr_ports_tests()
 		chk_join_nr 2 2 2
 		chk_add_nr 2 2 2
 	fi
+
+	# pm listener events
+	if reset "pm listener events"; 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
+		run_tests $ns1 $ns2 10.0.1.1 0 -1 0 slow
+		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
 }
 
 syncookies_tests()
-- 
2.35.3

Re: selftests: mptcp: pm_listener tests for mptcp_join: 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/5271409589485568
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5271409589485568/summary/summary.txt

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

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


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 v4 1/4] mptcp: add pm listener events

[Mat Martineau]

On Wed, 16 Nov 2022, Geliang Tang wrote:

> This patch adds MPTCP netlink events for PM listening socket create and
> close.
>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>

Hi Geliang -

The functionality here looks good, thanks for adding the tests. I have 
some questions below and on patch 4.

> ---
> 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..685240f6bc80 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_ATOMIC);

This doesn't appear to run in atomic context, so GFP_KERNEL would be 
better.

> +	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_ATOMIC);

GFP_KERNEL here too.

- Mat

> +	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
>
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next v4 4/4] selftests: mptcp: pm_listener tests for mptcp_join

[Mat Martineau]

On Wed, 16 Nov 2022, Geliang Tang wrote:

> Add pm_listener selftests for mptcp_join.sh.

Hi Geliang -

Also mention in the commit message that this adds test coverage for 
listening sockets created by the in-kernel path manager.

>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> .../testing/selftests/net/mptcp/mptcp_join.sh | 72 +++++++++++++++++++
> 1 file changed, 72 insertions(+)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index f10ef65a7009..0810e9c12fd5 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
> @@ -2589,6 +2637,30 @@ add_addr_ports_tests()
> 		chk_join_nr 2 2 2
> 		chk_add_nr 2 2 2
> 	fi
> +
> +	# pm listener events
> +	if reset "pm listener events"; 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
> +		run_tests $ns1 $ns2 10.0.1.1 0 -1 0 slow
> +		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

This duplicates the "remove single address with port" test. I think it's 
better to modify that existing test to add the listener event checking 
instead of increasing the execution time for mptcp_join.sh


> }
>
> syncookies_tests()
> -- 
> 2.35.3
>
>
>

--
Mat Martineau
Intel