[PATCH iperf3 v3] add MPTCPv1 support

Geliang Tang posted 1 patch 1 month, 2 weeks ago
Failed in applying to current master (apply log)
src/iperf.h        |  3 +++
src/iperf3.1       |  4 ++++
src/iperf_api.c    | 15 ++++++++++++++-
src/iperf_locale.c |  3 +++
src/iperf_tcp.c    | 18 +++++++++++++++---
src/net.c          | 10 +++++-----
src/net.h          |  2 +-
7 files changed, 45 insertions(+), 10 deletions(-)
[PATCH iperf3 v3] add MPTCPv1 support
Posted by Geliang Tang 1 month, 2 weeks ago
The Multipath TCP (MPTCP) protocol (v1 / RFC 8684) has been added in
the upstream Linux kernel since v5.6.

MPTCP is strongly tied to TCP, and the kernel APIs are almost the same.
The only required dependency is the 'IPPROTO_MPTCP' protocol number
definition, which should be provided by the netinet/in.h header if it
is recent enough.

This patch adds a new flag '-m' or '--multipath' to support MPTCPv1.
It can be used like this:

 > iperf3 -m -s
 > iperf3 -m -c 127.0.0.1

There is no need to check for IPPROTO_MPTCP support in configure.ac
at build time, it is at runtime we will see if the kernel being use
supports or not MPTCP.

If IPPROTO_MPTCP is not supported by the kernel being tested, it is
normal to fail because the feature is not available and the user
explicitly asked to use MPTCP.

Closes: https://github.com/esnet/iperf/pull/1659
Co-developed-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Geliang Tang <geliang@kernel.org>
---
 src/iperf.h        |  3 +++
 src/iperf3.1       |  4 ++++
 src/iperf_api.c    | 15 ++++++++++++++-
 src/iperf_locale.c |  3 +++
 src/iperf_tcp.c    | 18 +++++++++++++++---
 src/net.c          | 10 +++++-----
 src/net.h          |  2 +-
 7 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/src/iperf.h b/src/iperf.h
index dc3c0d1..80dda24 100644
--- a/src/iperf.h
+++ b/src/iperf.h
@@ -339,6 +339,9 @@ struct iperf_test
     int	      udp_counters_64bit;		/* --use-64-bit-udp-counters */
     int       forceflush; /* --forceflush - flushing output at every interval */
     int	      multisend;
+#if defined(linux)
+    int       multipath;
+#endif
     int	      repeating_payload;                /* --repeating-payload */
     int       timestamps;			/* --timestamps */
     char     *timestamp_format;
diff --git a/src/iperf3.1 b/src/iperf3.1
index 2efd53d..d847f37 100644
--- a/src/iperf3.1
+++ b/src/iperf3.1
@@ -193,6 +193,10 @@ parameter is specified in ms, and defaults to the system settings.
 This functionality depends on the TCP_USER_TIMEOUT socket option, and
 will not work on systems that do not support it.
 .TP
+.BR -m ", " --multipath " "
+use multipath variant for the current protocol. This only applies to
+TCP and enables MPTCP usage.
+.TP
 .BR -d ", " --debug " "
 emit debugging output.
 Primarily (perhaps exclusively) of use to developers.
diff --git a/src/iperf_api.c b/src/iperf_api.c
index 1dcfaab..a064c5c 100644
--- a/src/iperf_api.c
+++ b/src/iperf_api.c
@@ -1144,6 +1144,9 @@ iperf_parse_arguments(struct iperf_test *test, int argc, char **argv)
         {"idle-timeout", required_argument, NULL, OPT_IDLE_TIMEOUT},
         {"rcv-timeout", required_argument, NULL, OPT_RCV_TIMEOUT},
         {"snd-timeout", required_argument, NULL, OPT_SND_TIMEOUT},
+#if defined(linux)
+        {"multipath", no_argument, NULL, 'm'},
+#endif
         {"debug", optional_argument, NULL, 'd'},
         {"help", no_argument, NULL, 'h'},
         {NULL, 0, NULL, 0}
@@ -1169,7 +1172,7 @@ iperf_parse_arguments(struct iperf_test *test, int argc, char **argv)
     FILE *ptr_file;
 #endif /* HAVE_SSL */
 
-    while ((flag = getopt_long(argc, argv, "p:f:i:D1VJvsc:ub:t:n:k:l:P:Rw:B:M:N46S:L:ZO:F:A:T:C:dI:hX:", longopts, NULL)) != -1) {
+    while ((flag = getopt_long(argc, argv, "p:f:i:D1VJvsc:ub:t:n:k:l:P:Rw:B:mM:N46S:L:ZO:F:A:T:C:dI:hX:", longopts, NULL)) != -1) {
         switch (flag) {
             case 'p':
 		portno = atoi(optarg);
@@ -1639,6 +1642,12 @@ iperf_parse_arguments(struct iperf_test *test, int argc, char **argv)
 		test->settings->connect_timeout = unit_atoi(optarg);
 		client_flag = 1;
 		break;
+#if defined(linux)
+	    case 'm':
+		set_protocol(test, Ptcp);
+		test->multipath = 1;
+		break;
+#endif
 	    case 'h':
 		usage_long(stdout);
 		exit(0);
@@ -2216,6 +2225,8 @@ send_parameters(struct iperf_test *test)
 	    cJSON_AddTrueToObject(j, "reverse");
 	if (test->bidirectional)
 	            cJSON_AddTrueToObject(j, "bidirectional");
+	if (test->multipath)
+	    cJSON_AddTrueToObject(j, "multipath");
 	if (test->settings->socket_bufsize)
 	    cJSON_AddNumberToObject(j, "window", test->settings->socket_bufsize);
 	if (test->settings->blksize)
@@ -2332,6 +2343,8 @@ get_parameters(struct iperf_test *test)
 	    iperf_set_test_reverse(test, 1);
         if ((j_p = cJSON_GetObjectItem(j, "bidirectional")) != NULL)
             iperf_set_test_bidirectional(test, 1);
+	if ((j_p = cJSON_GetObjectItem(j, "multipath")) != NULL)
+	    test->multipath = 1;
 	if ((j_p = cJSON_GetObjectItem(j, "window")) != NULL)
 	    test->settings->socket_bufsize = j_p->valueint;
 	if ((j_p = cJSON_GetObjectItem(j, "len")) != NULL)
diff --git a/src/iperf_locale.c b/src/iperf_locale.c
index ae0f63a..4f680ff 100644
--- a/src/iperf_locale.c
+++ b/src/iperf_locale.c
@@ -128,6 +128,9 @@ const char usage_longstr[] = "Usage: iperf3 [-s|-c host] [options]\n"
                            "  --snd-timeout #           timeout for unacknowledged TCP data\n"
                            "                            (in ms, default is system settings)\n"
 #endif /* HAVE_TCP_USER_TIMEOUT */
+#if defined(linux)
+                           "  -m, --multipath           use MPTCP rather than plain TCP\n"
+#endif
                            "  -d, --debug[=#]           emit debugging output\n"
                            "                            (optional optional \"=\" and debug level: 1-4. Default is 4 - all messages)\n"
                            "  -v, --version             show version information and quit\n"
diff --git a/src/iperf_tcp.c b/src/iperf_tcp.c
index 184a195..84ff646 100644
--- a/src/iperf_tcp.c
+++ b/src/iperf_tcp.c
@@ -44,6 +44,10 @@
 #include "net.h"
 #include "cjson.h"
 
+#ifndef IPPROTO_MPTCP
+#define IPPROTO_MPTCP 262
+#endif
+
 #if defined(HAVE_FLOWLABEL)
 #include "flowlabel.h"
 #endif /* HAVE_FLOWLABEL */
@@ -182,9 +186,10 @@ iperf_tcp_listen(struct iperf_test *test)
      *
      * It's not clear whether this is a requirement or a convenience.
      */
-    if (test->no_delay || test->settings->mss || test->settings->socket_bufsize) {
+    if (test->no_delay || test->multipath || test->settings->mss || test->settings->socket_bufsize) {
 	struct addrinfo hints, *res;
 	char portstr[6];
+	int proto = 0;
 
         FD_CLR(s, &test->read_set);
         close(s);
@@ -210,7 +215,10 @@ iperf_tcp_listen(struct iperf_test *test)
             return -1;
         }
 
-        if ((s = socket(res->ai_family, SOCK_STREAM, 0)) < 0) {
+	if (test->multipath)
+	    proto = IPPROTO_MPTCP;
+
+        if ((s = socket(res->ai_family, SOCK_STREAM, proto)) < 0) {
 	    freeaddrinfo(res);
             i_errno = IESTREAMLISTEN;
             return -1;
@@ -375,8 +383,12 @@ iperf_tcp_connect(struct iperf_test *test)
     socklen_t optlen;
     int saved_errno;
     int rcvbuf_actual, sndbuf_actual;
+    int proto = 0;
+
+    if (test->multipath)
+        proto = IPPROTO_MPTCP;
 
-    s = create_socket(test->settings->domain, SOCK_STREAM, test->bind_address, test->bind_dev, test->bind_port, test->server_hostname, test->server_port, &server_res);
+    s = create_socket(test->settings->domain, SOCK_STREAM, proto, test->bind_address, test->bind_dev, test->bind_port, test->server_hostname, test->server_port, &server_res);
     if (s < 0) {
 	i_errno = IESTREAMCONNECT;
 	return -1;
diff --git a/src/net.c b/src/net.c
index c82caff..849e919 100644
--- a/src/net.c
+++ b/src/net.c
@@ -124,7 +124,7 @@ timeout_connect(int s, const struct sockaddr *name, socklen_t namelen,
 
 /* create a socket */
 int
-create_socket(int domain, int proto, const char *local, const char *bind_dev, int local_port, const char *server, int port, struct addrinfo **server_res_out)
+create_socket(int domain, int type, int proto, const char *local, const char *bind_dev, int local_port, const char *server, int port, struct addrinfo **server_res_out)
 {
     struct addrinfo hints, *local_res = NULL, *server_res = NULL;
     int s, saved_errno;
@@ -133,14 +133,14 @@ create_socket(int domain, int proto, const char *local, const char *bind_dev, in
     if (local) {
         memset(&hints, 0, sizeof(hints));
         hints.ai_family = domain;
-        hints.ai_socktype = proto;
+        hints.ai_socktype = type;
         if ((gerror = getaddrinfo(local, NULL, &hints, &local_res)) != 0)
             return -1;
     }
 
     memset(&hints, 0, sizeof(hints));
     hints.ai_family = domain;
-    hints.ai_socktype = proto;
+    hints.ai_socktype = type;
     snprintf(portstr, sizeof(portstr), "%d", port);
     if ((gerror = getaddrinfo(server, portstr, &hints, &server_res)) != 0) {
 	if (local)
@@ -148,7 +148,7 @@ create_socket(int domain, int proto, const char *local, const char *bind_dev, in
         return -1;
     }
 
-    s = socket(server_res->ai_family, proto, 0);
+    s = socket(server_res->ai_family, type, proto);
     if (s < 0) {
 	if (local)
 	    freeaddrinfo(local_res);
@@ -238,7 +238,7 @@ netdial(int domain, int proto, const char *local, const char *bind_dev, int loca
     struct addrinfo *server_res = NULL;
     int s, saved_errno;
 
-    s = create_socket(domain, proto, local, bind_dev, local_port, server, port, &server_res);
+    s = create_socket(domain, proto, 0, local, bind_dev, local_port, server, port, &server_res);
     if (s < 0) {
       return -1;
     }
diff --git a/src/net.h b/src/net.h
index f0e1b4f..1f5cc4d 100644
--- a/src/net.h
+++ b/src/net.h
@@ -28,7 +28,7 @@
 #define __NET_H
 
 int timeout_connect(int s, const struct sockaddr *name, socklen_t namelen, int timeout);
-int create_socket(int domain, int proto, const char *local, const char *bind_dev, int local_port, const char *server, int port, struct addrinfo **server_res_out);
+int create_socket(int domain, int type, int proto, const char *local, const char *bind_dev, int local_port, const char *server, int port, struct addrinfo **server_res_out);
 int netdial(int domain, int proto, const char *local, const char *bind_dev, int local_port, const char *server, int port, int timeout);
 int netannounce(int domain, int proto, const char *local, const char *bind_dev, int port);
 int Nread(int fd, char *buf, size_t count, int prot);
-- 
2.40.1
Re: [PATCH iperf3 v3] add MPTCPv1 support
Posted by Matthieu Baerts 1 month, 2 weeks ago
Hi Geliang,

11 Mar 2024 08:04:05 Geliang Tang <geliang@kernel.org>:

> The Multipath TCP (MPTCP) protocol (v1 / RFC 8684) has been added in
> the upstream Linux kernel since v5.6.
>
> MPTCP is strongly tied to TCP, and the kernel APIs are almost the same.
> The only required dependency is the 'IPPROTO_MPTCP' protocol number
> definition, which should be provided by the netinet/in.h header if it
> is recent enough.
>
> This patch adds a new flag '-m' or '--multipath' to support MPTCPv1.

I just noticed you switched back to "multipath" instead of "mptcp".
I think it is best to stick with "mptcp", because "multipath"
is too vague, thus confusing: it could be multipath as in multiple
flows, multicast, with other protocols like UDP, etc.

Best to use "mptcp" everywhere: variables, strings, description, etc.


> It can be used like this:
>
>> iperf3 -m -s
>> iperf3 -m -c 127.0.0.1

If the server doesn't have "-m", only the client, does it use MPTCP?

Cheers,
Matt
Re: [PATCH iperf3 v3] add MPTCPv1 support
Posted by Geliang Tang 1 month, 2 weeks ago
Hi Matt,

On Tue, Mar 12, 2024 at 08:10:28AM +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> 11 Mar 2024 08:04:05 Geliang Tang <geliang@kernel.org>:
> 
> > The Multipath TCP (MPTCP) protocol (v1 / RFC 8684) has been added in
> > the upstream Linux kernel since v5.6.
> >
> > MPTCP is strongly tied to TCP, and the kernel APIs are almost the same.
> > The only required dependency is the 'IPPROTO_MPTCP' protocol number
> > definition, which should be provided by the netinet/in.h header if it
> > is recent enough.
> >
> > This patch adds a new flag '-m' or '--multipath' to support MPTCPv1.
> 
> I just noticed you switched back to "multipath" instead of "mptcp".
> I think it is best to stick with "mptcp", because "multipath"
> is too vague, thus confusing: it could be multipath as in multiple
> flows, multicast, with other protocols like UDP, etc.
> 
> Best to use "mptcp" everywhere: variables, strings, description, etc.

Sure, I'll send a v4 to update the flag to '--mptcp'. But I don't know
how to update the pull request on github. Should I send another pull
request?

> 
> 
> > It can be used like this:
> >
> >> iperf3 -m -s
> >> iperf3 -m -c 127.0.0.1
> 
> If the server doesn't have "-m", only the client, does it use MPTCP?

Yes, it use MPTCP in this case. I just tested it.

Thanks,
-Geliang

> 
> Cheers,
> Matt
Re: [PATCH iperf3 v3] add MPTCPv1 support
Posted by Matthieu Baerts 1 month, 2 weeks ago
Hi Geliang,

On 12/03/2024 08:27, Geliang Tang wrote:
> Hi Matt,
> 
> On Tue, Mar 12, 2024 at 08:10:28AM +0100, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> 11 Mar 2024 08:04:05 Geliang Tang <geliang@kernel.org>:
>>
>>> The Multipath TCP (MPTCP) protocol (v1 / RFC 8684) has been added in
>>> the upstream Linux kernel since v5.6.
>>>
>>> MPTCP is strongly tied to TCP, and the kernel APIs are almost the same.
>>> The only required dependency is the 'IPPROTO_MPTCP' protocol number
>>> definition, which should be provided by the netinet/in.h header if it
>>> is recent enough.
>>>
>>> This patch adds a new flag '-m' or '--multipath' to support MPTCPv1.
>>
>> I just noticed you switched back to "multipath" instead of "mptcp".
>> I think it is best to stick with "mptcp", because "multipath"
>> is too vague, thus confusing: it could be multipath as in multiple
>> flows, multicast, with other protocols like UDP, etc.
>>
>> Best to use "mptcp" everywhere: variables, strings, description, etc.
> 
> Sure, I'll send a v4 to update the flag to '--mptcp'.

Thanks!

> But I don't know
> how to update the pull request on github. Should I send another pull
> request?

No, please don't open a new pull request, nor close the old one, simply
override your remote branch on GitHub (`git push --force`).

>>> It can be used like this:>>>
>>>> iperf3 -m -s
>>>> iperf3 -m -c 127.0.0.1
>>
>> If the server doesn't have "-m", only the client, does it use MPTCP?
> 
> Yes, it use MPTCP in this case. I just tested it.

Great, thank you for having tested that!

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH iperf3 v3] add MPTCPv1 support
Posted by Geliang Tang 1 month, 2 weeks ago
On Tue, Mar 12, 2024 at 09:09:27AM +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 12/03/2024 08:27, Geliang Tang wrote:
> > Hi Matt,
> > 
> > On Tue, Mar 12, 2024 at 08:10:28AM +0100, Matthieu Baerts wrote:
> >> Hi Geliang,
> >>
> >> 11 Mar 2024 08:04:05 Geliang Tang <geliang@kernel.org>:
> >>
> >>> The Multipath TCP (MPTCP) protocol (v1 / RFC 8684) has been added in
> >>> the upstream Linux kernel since v5.6.
> >>>
> >>> MPTCP is strongly tied to TCP, and the kernel APIs are almost the same.
> >>> The only required dependency is the 'IPPROTO_MPTCP' protocol number
> >>> definition, which should be provided by the netinet/in.h header if it
> >>> is recent enough.
> >>>
> >>> This patch adds a new flag '-m' or '--multipath' to support MPTCPv1.
> >>
> >> I just noticed you switched back to "multipath" instead of "mptcp".
> >> I think it is best to stick with "mptcp", because "multipath"
> >> is too vague, thus confusing: it could be multipath as in multiple
> >> flows, multicast, with other protocols like UDP, etc.
> >>
> >> Best to use "mptcp" everywhere: variables, strings, description, etc.
> > 
> > Sure, I'll send a v4 to update the flag to '--mptcp'.
> 
> Thanks!
> 
> > But I don't know
> > how to update the pull request on github. Should I send another pull
> > request?
> 
> No, please don't open a new pull request, nor close the old one, simply
> override your remote branch on GitHub (`git push --force`).

Sorry, I open a new one before reading this:

https://github.com/esnet/iperf/pull/1661

> 
> >>> It can be used like this:>>>
> >>>> iperf3 -m -s
> >>>> iperf3 -m -c 127.0.0.1
> >>
> >> If the server doesn't have "-m", only the client, does it use MPTCP?
> > 
> > Yes, it use MPTCP in this case. I just tested it.
> 
> Great, thank you for having tested that!
> 
> Cheers,
> Matt
> -- 
> Sponsored by the NGI0 Core fund.