[PATCH mptcp-net 0/2] mptcp: support MP_PRIO signals with userspace PMs

[PATCH mptcp-net 0/2] mptcp: support MP_PRIO signals with userspace PMs

[Kishen Maloor]

This patch series updates MPTCP_PM_CMD_SET_FLAGS to allow userspace PMs
to issue MP_PRIO signals for a specified address ID over a chosen
MPTCP connection. It also adds self testing support for this change.

Kishen Maloor (2):
  mptcp: netlink: issue MP_PRIO signals from userspace PMs
  selftests: mptcp: userspace PM support for MP_PRIO signals

 net/mptcp/pm_netlink.c                        | 23 ++++++++++----
 net/mptcp/pm_userspace.c                      | 30 ++++++++++++++++++
 net/mptcp/protocol.h                          |  6 +++-
 tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 19 ++++++++++--
 .../selftests/net/mptcp/userspace_pm.sh       | 31 +++++++++++++++++++
 5 files changed, 100 insertions(+), 9 deletions(-)


base-commit: 68fec8b7f640037e4bf224ca4e924574098f5911
--
2.31.1

[PATCH mptcp-net 1/2] mptcp: netlink: issue MP_PRIO signals from userspace PMs

[Kishen Maloor]

This change updates MPTCP_PM_CMD_SET_FLAGS to allow userspace PMs
to issue MP_PRIO signals for a specified address ID over a chosen
(by token) MPTCP connection.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/286
Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow establishment")
Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
---
 net/mptcp/pm_netlink.c   | 23 +++++++++++++++++------
 net/mptcp/pm_userspace.c | 30 ++++++++++++++++++++++++++++++
 net/mptcp/protocol.h     |  6 +++++-
 3 files changed, 52 insertions(+), 7 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index e099f2a12504..063700ff8cbb 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -717,9 +717,9 @@ void mptcp_pm_nl_addr_send_ack(struct mptcp_sock *msk)
 	}
 }
 
-static int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk,
-					struct mptcp_addr_info *addr,
-					u8 bkup)
+int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk,
+				 struct mptcp_addr_info *addr,
+				 u8 bkup, bool use_id)
 {
 	struct mptcp_subflow_context *subflow;
 
@@ -730,10 +730,15 @@ static int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk,
 		struct sock *sk = (struct sock *)msk;
 		struct mptcp_addr_info local;
 
-		local_address((struct sock_common *)ssk, &local);
-		if (!mptcp_addresses_equal(&local, addr, addr->port))
+		if (use_id && subflow->local_id != addr->id)
 			continue;
 
+		if (!use_id) {
+			local_address((struct sock_common *)ssk, &local);
+			if (!mptcp_addresses_equal(&local, addr, addr->port))
+				continue;
+		}
+
 		if (subflow->backup != bkup)
 			msk->last_snd = NULL;
 		subflow->backup = bkup;
@@ -1837,7 +1842,7 @@ static int mptcp_nl_set_flags(struct net *net,
 		lock_sock(sk);
 		spin_lock_bh(&msk->pm.lock);
 		if (changed & MPTCP_PM_ADDR_FLAG_BACKUP)
-			ret = mptcp_pm_nl_mp_prio_send_ack(msk, addr, bkup);
+			ret = mptcp_pm_nl_mp_prio_send_ack(msk, addr, bkup, false);
 		if (changed & MPTCP_PM_ADDR_FLAG_FULLMESH)
 			mptcp_pm_nl_fullmesh(msk, addr);
 		spin_unlock_bh(&msk->pm.lock);
@@ -1854,6 +1859,7 @@ static int mptcp_nl_set_flags(struct net *net,
 static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
 {
 	struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, }, *entry;
+	struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
 	struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
 	u8 changed, mask = MPTCP_PM_ADDR_FLAG_BACKUP |
@@ -1868,6 +1874,11 @@ static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
 
 	if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
 		bkup = 1;
+
+	if (token)
+		return mptcp_userspace_pm_set_flags(sock_net(skb->sk),
+						    token, &addr, bkup);
+
 	if (addr.addr.family == AF_UNSPEC) {
 		lookup_by_id = 1;
 		if (!addr.addr.id)
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 3d1d365e9c6f..b9df969cd38a 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -429,3 +429,33 @@ int mptcp_nl_cmd_sf_destroy(struct sk_buff *skb, struct genl_info *info)
 	sock_put((struct sock *)msk);
 	return err;
 }
+
+int mptcp_userspace_pm_set_flags(struct net *net, struct nlattr *token,
+				 struct mptcp_pm_addr_entry *entry, u8 bkup)
+{
+	struct mptcp_sock *msk;
+	int ret = -EINVAL;
+	u32 token_val;
+
+	token_val = nla_get_u32(token);
+
+	msk = mptcp_token_get_sock(net, token_val);
+	if (!msk)
+		return ret;
+
+	if (!mptcp_pm_is_userspace(msk))
+		goto set_flags_err;
+
+	if (entry->addr.family != AF_UNSPEC)
+		goto set_flags_err;
+
+	lock_sock((struct sock *)msk);
+	spin_lock_bh(&msk->pm.lock);
+	ret = mptcp_pm_nl_mp_prio_send_ack(msk, &entry->addr, bkup, true);
+	spin_unlock_bh(&msk->pm.lock);
+	release_sock((struct sock *)msk);
+
+set_flags_err:
+	sock_put((struct sock *)msk);
+	return ret;
+}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index bef7dea9f358..9bb49d199fa5 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -783,6 +783,9 @@ void mptcp_pm_rm_addr_received(struct mptcp_sock *msk,
 			       const struct mptcp_rm_list *rm_list);
 void mptcp_pm_mp_prio_received(struct sock *sk, u8 bkup);
 void mptcp_pm_mp_fail_received(struct sock *sk, u64 fail_seq);
+int mptcp_pm_nl_mp_prio_send_ack(struct mptcp_sock *msk,
+				 struct mptcp_addr_info *addr,
+				 u8 bkup, bool use_id);
 bool mptcp_pm_alloc_anno_list(struct mptcp_sock *msk,
 			      const struct mptcp_pm_addr_entry *entry);
 void mptcp_pm_free_anno_list(struct mptcp_sock *msk);
@@ -799,7 +802,8 @@ int mptcp_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
 int mptcp_userspace_pm_get_flags_and_ifindex_by_id(struct mptcp_sock *msk,
 						   unsigned int id,
 						   u8 *flags, int *ifindex);
-
+int mptcp_userspace_pm_set_flags(struct net *net, struct nlattr *token,
+				 struct mptcp_pm_addr_entry *entry, u8 bkup);
 int mptcp_pm_announce_addr(struct mptcp_sock *msk,
 			   const struct mptcp_addr_info *addr,
 			   bool echo);
-- 
2.31.1

[PATCH mptcp-net 2/2] selftests: mptcp: userspace PM support for MP_PRIO signals

[Kishen Maloor]

This change updates the testing sample (pm_nl_ctl) to take a
connection token as an optional param for the MPTCP_PM_CMD_SET_FLAGS
command. This is used to test the userspace PM code path for issuing
MP_PRIO signals over a connection for the specified address ID.

E.g. ./pm_nl_ctl set id 0 flags backup token 823274047

userspace_pm.sh has new selftests which exercise this command.

Fixes: 259a834fadda ("selftests: mptcp: functional tests for the userspace PM type")
Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
---
 tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 19 ++++++++++--
 .../selftests/net/mptcp/userspace_pm.sh       | 31 +++++++++++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
index 4dd87bb9ee91..0512d64b1d11 100644
--- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
+++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
@@ -39,7 +39,7 @@ static void syntax(char *argv[])
 	fprintf(stderr, "\tdsf lip <local-ip> lport <local-port> rip <remote-ip> rport <remote-port> token <token>\n");
 	fprintf(stderr, "\tdel <id> [<ip>]\n");
 	fprintf(stderr, "\tget <id>\n");
-	fprintf(stderr, "\tset [<ip>] [id <nr>] flags [no]backup|[no]fullmesh [port <nr>]\n");
+	fprintf(stderr, "\tset [<ip>] [id <nr>] flags [no]backup|[no]fullmesh [port <nr>] [token <token>]\n");
 	fprintf(stderr, "\tflush\n");
 	fprintf(stderr, "\tdump\n");
 	fprintf(stderr, "\tlimits [<rcv addr max> <subflow max>]\n");
@@ -1279,6 +1279,7 @@ int set_flags(int fd, int pm_family, int argc, char *argv[])
 	struct rtattr *rta, *nest;
 	struct nlmsghdr *nh;
 	u_int32_t flags = 0;
+	u_int32_t token = 0;
 	u_int16_t family;
 	int nest_start;
 	int use_id = 0;
@@ -1339,7 +1340,13 @@ int set_flags(int fd, int pm_family, int argc, char *argv[])
 		error(1, 0, " missing flags keyword");
 
 	for (; arg < argc; arg++) {
-		if (!strcmp(argv[arg], "flags")) {
+		if (!strcmp(argv[arg], "token")) {
+			if (++arg >= argc)
+				error(1, 0, " missing token value");
+
+			/* token */
+			token = atoi(argv[arg]);
+		} else if (!strcmp(argv[arg], "flags")) {
 			char *tok, *str;
 
 			/* flags */
@@ -1384,6 +1391,14 @@ int set_flags(int fd, int pm_family, int argc, char *argv[])
 	}
 	nest->rta_len = off - nest_start;
 
+	if (token) {
+		rta = (void *)(data + off);
+		rta->rta_type = MPTCP_PM_ATTR_TOKEN;
+		rta->rta_len = RTA_LENGTH(4);
+		memcpy(RTA_DATA(rta), &token, 4);
+		off += NLMSG_ALIGN(rta->rta_len);
+	}
+
 	do_nl_req(fd, nh, off, 0);
 	return 0;
 }
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index d586bc5ffe01..387f5774d541 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -776,10 +776,41 @@ test_subflows()
 	rm -f "$evts"
 }
 
+test_prio()
+{
+	local count
+
+	# Send MP_PRIO signal from client to server machine
+	ip netns exec "$ns2" ./pm_nl_ctl set id 0 flags backup token "$client4_token"
+
+	# Check TX
+	stdbuf -o0 -e0 printf "MP_PRIO TX                                                 \t"
+	count=$(ip netns exec "$ns2" nstat -as | grep MPTcpExtMPPrioTx | awk '{print $2}')
+	[ -z "$count" ] && count=0
+	if [ $count != 1 ]; then
+		stdbuf -o0 -e0 printf "[FAIL]\n"
+		exit 1
+	else
+		stdbuf -o0 -e0 printf "[OK]\n"
+	fi
+
+	# Check RX
+	stdbuf -o0 -e0 printf "MP_PRIO RX                                                 \t"
+	count=$(ip netns exec "$ns1" nstat -as | grep MPTcpExtMPPrioRx | awk '{print $2}')
+	[ -z "$count" ] && count=0
+	if [ $count != 1 ]; then
+		stdbuf -o0 -e0 printf "[FAIL]\n"
+		exit 1
+	else
+		stdbuf -o0 -e0 printf "[OK]\n"
+	fi
+}
+
 make_connection
 make_connection "v6"
 test_announce
 test_remove
 test_subflows
+test_prio
 
 exit 0
-- 
2.31.1

Re: selftests: mptcp: userspace PM support for MP_PRIO signals: Tests Results

[MPTCP CI]

Hi Kishen,

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/5802668730875904
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5802668730875904/summary/summary.txt

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

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


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-net 2/2] selftests: mptcp: userspace PM support for MP_PRIO signals

[Mat Martineau]

On Wed, 22 Jun 2022, Kishen Maloor wrote:

> This change updates the testing sample (pm_nl_ctl) to take a
> connection token as an optional param for the MPTCP_PM_CMD_SET_FLAGS
> command. This is used to test the userspace PM code path for issuing
> MP_PRIO signals over a connection for the specified address ID.
>
> E.g. ./pm_nl_ctl set id 0 flags backup token 823274047
>
> userspace_pm.sh has new selftests which exercise this command.
>
> Fixes: 259a834fadda ("selftests: mptcp: functional tests for the userspace PM type")
> Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
> ---
> tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 19 ++++++++++--
> .../selftests/net/mptcp/userspace_pm.sh       | 31 +++++++++++++++++++
> 2 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> index 4dd87bb9ee91..0512d64b1d11 100644
> --- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> +++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> @@ -39,7 +39,7 @@ static void syntax(char *argv[])
> 	fprintf(stderr, "\tdsf lip <local-ip> lport <local-port> rip <remote-ip> rport <remote-port> token <token>\n");
> 	fprintf(stderr, "\tdel <id> [<ip>]\n");
> 	fprintf(stderr, "\tget <id>\n");
> -	fprintf(stderr, "\tset [<ip>] [id <nr>] flags [no]backup|[no]fullmesh [port <nr>]\n");
> +	fprintf(stderr, "\tset [<ip>] [id <nr>] flags [no]backup|[no]fullmesh [port <nr>] [token <token>]\n");
> 	fprintf(stderr, "\tflush\n");
> 	fprintf(stderr, "\tdump\n");
> 	fprintf(stderr, "\tlimits [<rcv addr max> <subflow max>]\n");
> @@ -1279,6 +1279,7 @@ int set_flags(int fd, int pm_family, int argc, char *argv[])
> 	struct rtattr *rta, *nest;
> 	struct nlmsghdr *nh;
> 	u_int32_t flags = 0;
> +	u_int32_t token = 0;
> 	u_int16_t family;
> 	int nest_start;
> 	int use_id = 0;
> @@ -1339,7 +1340,13 @@ int set_flags(int fd, int pm_family, int argc, char *argv[])
> 		error(1, 0, " missing flags keyword");
>
> 	for (; arg < argc; arg++) {
> -		if (!strcmp(argv[arg], "flags")) {
> +		if (!strcmp(argv[arg], "token")) {
> +			if (++arg >= argc)
> +				error(1, 0, " missing token value");
> +
> +			/* token */
> +			token = atoi(argv[arg]);
> +		} else if (!strcmp(argv[arg], "flags")) {
> 			char *tok, *str;
>
> 			/* flags */
> @@ -1384,6 +1391,14 @@ int set_flags(int fd, int pm_family, int argc, char *argv[])
> 	}
> 	nest->rta_len = off - nest_start;
>
> +	if (token) {
> +		rta = (void *)(data + off);
> +		rta->rta_type = MPTCP_PM_ATTR_TOKEN;
> +		rta->rta_len = RTA_LENGTH(4);
> +		memcpy(RTA_DATA(rta), &token, 4);
> +		off += NLMSG_ALIGN(rta->rta_len);
> +	}
> +
> 	do_nl_req(fd, nh, off, 0);
> 	return 0;
> }
> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> index d586bc5ffe01..387f5774d541 100755
> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> @@ -776,10 +776,41 @@ test_subflows()
> 	rm -f "$evts"
> }
>
> +test_prio()
> +{
> +	local count
> +
> +	# Send MP_PRIO signal from client to server machine
> +	ip netns exec "$ns2" ./pm_nl_ctl set id 0 flags backup token "$client4_token"

This test did pass for me with a debug kernel in a desktop VM.

Considering the occasional failures we have when running other tests on 
debug kernels in CI, and the delays inserted elsewhere in this script, I 
think there should be a "sleep 0.5" here to make sure the MIBs have time 
to update.

- Mat


> +
> +	# Check TX
> +	stdbuf -o0 -e0 printf "MP_PRIO TX                                                 \t"
> +	count=$(ip netns exec "$ns2" nstat -as | grep MPTcpExtMPPrioTx | awk '{print $2}')
> +	[ -z "$count" ] && count=0
> +	if [ $count != 1 ]; then
> +		stdbuf -o0 -e0 printf "[FAIL]\n"
> +		exit 1
> +	else
> +		stdbuf -o0 -e0 printf "[OK]\n"
> +	fi
> +
> +	# Check RX
> +	stdbuf -o0 -e0 printf "MP_PRIO RX                                                 \t"
> +	count=$(ip netns exec "$ns1" nstat -as | grep MPTcpExtMPPrioRx | awk '{print $2}')
> +	[ -z "$count" ] && count=0
> +	if [ $count != 1 ]; then
> +		stdbuf -o0 -e0 printf "[FAIL]\n"
> +		exit 1
> +	else
> +		stdbuf -o0 -e0 printf "[OK]\n"
> +	fi
> +}
> +
> make_connection
> make_connection "v6"
> test_announce
> test_remove
> test_subflows
> +test_prio
>
> exit 0
> -- 
> 2.31.1
>
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-net 0/2] mptcp: support MP_PRIO signals with userspace PMs

[Mat Martineau]

On Wed, 22 Jun 2022, Kishen Maloor wrote:

> This patch series updates MPTCP_PM_CMD_SET_FLAGS to allow userspace PMs
> to issue MP_PRIO signals for a specified address ID over a chosen
> MPTCP connection. It also adds self testing support for this change.
>
> Kishen Maloor (2):
>  mptcp: netlink: issue MP_PRIO signals from userspace PMs
>  selftests: mptcp: userspace PM support for MP_PRIO signals
>
> net/mptcp/pm_netlink.c                        | 23 ++++++++++----
> net/mptcp/pm_userspace.c                      | 30 ++++++++++++++++++
> net/mptcp/protocol.h                          |  6 +++-
> tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 19 ++++++++++--
> .../selftests/net/mptcp/userspace_pm.sh       | 31 +++++++++++++++++++
> 5 files changed, 100 insertions(+), 9 deletions(-)
>
>
> base-commit: 68fec8b7f640037e4bf224ca4e924574098f5911
> --
> 2.31.1

Thanks Kishen!

The selftest passed on my test system, but I have one comment on that 
patch.

Paolo, do agree this is best sent to the -net branch? This is an important 
chunk of the userspace PM that we accidentally overlooked, and it seems 
pretty important to get in to 5.19.

--
Mat Martineau
Intel

Re: [PATCH mptcp-net 0/2] mptcp: support MP_PRIO signals with userspace PMs

[Mat Martineau]

On Wed, 22 Jun 2022, Mat Martineau wrote:

> On Wed, 22 Jun 2022, Kishen Maloor wrote:
>
>> This patch series updates MPTCP_PM_CMD_SET_FLAGS to allow userspace PMs
>> to issue MP_PRIO signals for a specified address ID over a chosen
>> MPTCP connection. It also adds self testing support for this change.
>> 
>> Kishen Maloor (2):
>>  mptcp: netlink: issue MP_PRIO signals from userspace PMs
>>  selftests: mptcp: userspace PM support for MP_PRIO signals
>> 
>> net/mptcp/pm_netlink.c                        | 23 ++++++++++----
>> net/mptcp/pm_userspace.c                      | 30 ++++++++++++++++++
>> net/mptcp/protocol.h                          |  6 +++-
>> tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 19 ++++++++++--
>> .../selftests/net/mptcp/userspace_pm.sh       | 31 +++++++++++++++++++
>> 5 files changed, 100 insertions(+), 9 deletions(-)
>> 
>> 
>> base-commit: 68fec8b7f640037e4bf224ca4e924574098f5911
>> --
>> 2.31.1
>
> Thanks Kishen!
>
> The selftest passed on my test system, but I have one comment on that patch.
>
> Paolo, do agree this is best sent to the -net branch? This is an important 
> chunk of the userspace PM that we accidentally overlooked, and it seems 
> pretty important to get in to 5.19.
>

One other thing: I did gather some pcaps in a manual run, and verified 
that the MP_PRIO packet is generated for both setting and clearing the 
backup flag.

--
Mat Martineau
Intel