pm: use `signal` + `subflow` + `fullmesh`

[anthonyryan1]

Pre-requisites

• A similar question has not been reported before.
• mptcp.dev website does not cover my case.
• The wiki doesn't cover my case.
• This is not a question related to the current behavior, an issue or a feature requst: if it is, please use another template even if it is a question: we will need details about your system: kernel version, config, etc.

My question

As I understand it:

• To have a server advertise alternative paths: Path-Manager requires signal
• To have a cilent advertise additional paths: Path-Manager requires subflow
• fullmesh can optionally be added to clients to tell them to establish & maintain redundant connections

In a scenario like a MariaDB Galera Cluster, or a spanning tree network of IRC servers, each node at any given moment could be either a server or a client to each other, and sometimes a server to one node and a client to another at the same time.

Is it possible to tell MPTCP that I want the benefits of a fullmesh (establishing redundant connections preemptively) when it happens to be operating as a client, while still sharing IPs as a server?

Currently the docs say fullmesh can't be combined with signal and my attempts to go off script with ip mptcp confirm that there's guardrails to prevent it. But signal and subflow can both be combined.

How can I establish and maintain redundant paths preemptively in this sort of bi-directional / reversible client-server model?

[matttbe]

Hi @anthonyryan1

Sorry for the delay, and for having missed the question on IRC.

signal + fullmesh flags don't make sense, but indeed, I guess it can when the subflow flag is also used.

After a very quick look, I don't really see anything blocking this case. Is it easy for you to do some tests on your side with a modified kernel? If yes, please apply this patch and try using signal + subflow + fullmesh:

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 7ec81d5195d4..59a375cc4e21 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -804,13 +804,6 @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
                return -EINVAL;
        }
 
-       if (addr.flags & MPTCP_PM_ADDR_FLAG_SIGNAL &&
-           addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) {
-               NL_SET_ERR_MSG_ATTR(info->extack, attr,
-                                   "flags mustn't have both signal and fullmesh");
-               return -EINVAL;
-       }
-
        if (addr.flags & MPTCP_PM_ADDR_FLAG_IMPLICIT) {
                NL_SET_ERR_MSG_ATTR(info->extack, attr,
                                    "can't create IMPLICIT endpoint");

(just in case you want to modify the flag for existing endpoints, but likely more chances to hit bugs there)

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 7ec81d5195d4..59a375cc4e21 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -1310,7 +1303,7 @@ int mptcp_pm_nl_set_flags(struct mptcp_pm_addr_entry *local,
                return -EINVAL;
        }
        if ((local->flags & MPTCP_PM_ADDR_FLAG_FULLMESH) &&
-           (entry->flags & (MPTCP_PM_ADDR_FLAG_SIGNAL |
+           (entry->flags & (
                             MPTCP_PM_ADDR_FLAG_IMPLICIT))) {
                spin_unlock_bh(&pernet->lock);
                NL_SET_ERR_MSG_ATTR(info->extack, attr, "invalid addr flags");

Please note that the fullmesh flag will likely create "too many" subflows in most cases. Maybe in your case, it will not. But if both side creates subflows in fullmesh, this likely means each path will have two subflows, no?

Also, the current limit of subflows per MPTCP connection is 8, we were trying to find good use-cases before looking at increase this. This might be "too low" for you.

[anthonyryan1]

I tested it today with:

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 7ec81d5195d4..59a375cc4e21 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -804,13 +804,6 @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
                return -EINVAL;
        }
 
-       if (addr.flags & MPTCP_PM_ADDR_FLAG_SIGNAL &&
-           addr.flags & MPTCP_PM_ADDR_FLAG_FULLMESH) {
-               NL_SET_ERR_MSG_ATTR(info->extack, attr,
-                                   "flags mustn't have both signal and fullmesh");
-               return -EINVAL;
-       }
-
        if (addr.flags & MPTCP_PM_ADDR_FLAG_IMPLICIT) {
                NL_SET_ERR_MSG_ATTR(info->extack, attr,
                                    "can't create IMPLICIT endpoint");

& this patch for iproute2:

diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
index 118bac4a..16793651 100644
--- a/ip/ipmptcp.c
+++ b/ip/ipmptcp.c
@@ -120,11 +120,6 @@ static int mptcp_parse_opt(int argc, char **argv, struct nlmsghdr *n, int cmd)
 	ll_init_map(&rth);
 	while (argc > 0) {
 		if (get_flags(*argv, &flags) == 0) {
-			if (adding &&
-			    (flags & MPTCP_PM_ADDR_FLAG_SIGNAL) &&
-			    (flags & MPTCP_PM_ADDR_FLAG_FULLMESH))
-				invarg("flags mustn't have both signal and fullmesh", *argv);
-
 			/* allow changing the 'backup' and 'fullmesh' flags only */
 			if (setting &&
 			    (flags & ~(MPTCP_PM_ADDR_FLAG_BACKUP |

I can confirm everything works exceedingly well. I was able to connect and disconnect connections without any disruption.

I also did some synthetic packet loss testing, and observed that when one link had 0% packet loss, and the other had 20% packet loss, MPTCP appeared to have 10% packet loss, which I think is reasonably expected that it averages packet loss over all the connections.

Do you feel like this is a use case that you'd be content to support going forward? If so, let me know how I can help.

It does seem like a reasonably extension of the support for signal + subflow, the docs would need to be updated to clarify that fullmesh only applies to subflow connections, and has no effect on signal.

[matttbe]

& this patch for iproute2:

Ah yes, good point, it was indeed needed.

Do you plan to send this one upstream? Older kernels will continue to block this, but that's OK, iproute2 will continue to report an error with them (maybe less clear, but that's OK).

I can confirm everything works exceedingly well. I was able to connect and disconnect connections without any disruption.

Thank you for having tested!

Do you then have 2 established subflows per available path?
So if you have 2 addresses on each side, thus 4 available paths, you end up with 8 subflows, right?

I also did some synthetic packet loss testing, and observed that when one link had 0% packet loss, and the other had 20% packet loss, MPTCP appeared to have 10% packet loss, which I think is reasonably expected that it averages packet loss over all the connections.

Interesting. There are maybe some improvements to do on the scheduler side if such packet loss impacts the throughput, but that's a different topic.

Do you feel like this is a use case that you'd be content to support going forward? If so, let me know how I can help.

There is always a risk of introducing more issues, and maybe some confusions for the users (the signal + subflow is already confusing some :) ), but if we only have to drop this limitation, that might be fine.

Before removing this limitation, at least three things will be needed:

• Validate the flag change for existing endpoints with ongoing connections. In other words, applying my second patch, use ip mptcp endpoint change id X (no)fullmesh on the client and server, and see the impact.
• Validate this subflow + signal + fullmesh case in the selftests (and the possibility to change the fullmesh flag afterward): for the moment, we have the "subflow and signal together" test in tools/testing/selftests/net/mptcp/mptcp_join.sh
• Having the IPRoute2 patch ready (the diff you shared + commit message)
• (have CI + syzkaller checking everything is OK)

If you can help with any of them, that would help to accelerate things.

It does seem like a reasonably extension of the support for signal + subflow, the docs would need to be updated to clarify that fullmesh only applies to subflow connections, and has no effect on signal.

Indeed, hopefully not confusing users :)

[anthonyryan1]

Do you then have 2 established subflows per available path?
So if you have 2 addresses on each side, thus 4 available paths, you end up with 8 subflows, right?

In my initial test I used 1x IPv4 and 1x IPv6 address on each end, because they're not able to connect to each other, I only got 2 subflows. I hope to do some further testing in a few more days, but I can't promise a timeline as my work direction has shifted away from MPTCP. :(