[PATCH iperf3 v2] add MPTCPv1 support

Geliang Tang posted 1 patch 1 month, 3 weeks ago
Failed in applying to current master (apply log)
There is a newer version of this series
src/iperf.h            |  7 +++++++
src/iperf_api.c        |  8 ++++++++
src/iperf_api.h        |  1 +
src/iperf_client_api.c |  2 +-
src/iperf_locale.c     |  3 +++
src/iperf_tcp.c        |  4 ++--
src/iperf_udp.c        |  2 +-
src/net.c              | 14 +++++++-------
src/net.h              |  4 ++--
9 files changed, 32 insertions(+), 13 deletions(-)
[PATCH iperf3 v2] add MPTCPv1 support
Posted by Geliang Tang 1 month, 3 weeks ago
The Multipath TCP (MPTCP) protocol (v1 / RFC 8684) has been added in the
upstream Linux kernel since v5.16.

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.

This patch adds a new flag --mptcp to support MPTCPv1. It can be used
like this:

 > iperf3 --mptcp -s
 > iperf3 --mptcp -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.

Linked: https://github.com/esnet/iperf/pull/1166
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Geliang Tang <geliang@kernel.org>
---

Notes:
    restrict this feature to Linux builds

 src/iperf.h            |  7 +++++++
 src/iperf_api.c        |  8 ++++++++
 src/iperf_api.h        |  1 +
 src/iperf_client_api.c |  2 +-
 src/iperf_locale.c     |  3 +++
 src/iperf_tcp.c        |  4 ++--
 src/iperf_udp.c        |  2 +-
 src/net.c              | 14 +++++++-------
 src/net.h              |  4 ++--
 9 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/src/iperf.h b/src/iperf.h
index dc3c0d1..bc9fdf6 100644
--- a/src/iperf.h
+++ b/src/iperf.h
@@ -50,6 +50,10 @@
 #include <sys/cpuset.h>
 #endif /* HAVE_CPUSET_SETAFFINITY */
 
+#ifndef IPPROTO_MPTCP
+#define IPPROTO_MPTCP 262
+#endif
+
 #include "timer.h"
 #include "queue.h"
 #include "cjson.h"
@@ -175,6 +179,9 @@ struct iperf_settings
     int       idle_timeout;         /* server idle time timeout */
     unsigned int snd_timeout; /* Timeout for sending tcp messages in active mode, in us */
     struct iperf_time rcv_timeout;  /* Timeout for receiving messages in active mode, in us */
+#if defined(linux)
+    int       mptcp;
+#endif
 };
 
 struct iperf_test;
diff --git a/src/iperf_api.c b/src/iperf_api.c
index 1dcfaab..686b78b 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)
+        {"mptcp", no_argument, NULL, OPT_MPTCP},
+#endif
         {"debug", optional_argument, NULL, 'd'},
         {"help", no_argument, NULL, 'h'},
         {NULL, 0, NULL, 0}
@@ -1639,6 +1642,11 @@ 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 OPT_MPTCP:
+		test->settings->mptcp = 1;
+		break;
+#endif
 	    case 'h':
 		usage_long(stdout);
 		exit(0);
diff --git a/src/iperf_api.h b/src/iperf_api.h
index d2bbdfe..2ee29d9 100644
--- a/src/iperf_api.h
+++ b/src/iperf_api.h
@@ -100,6 +100,7 @@ typedef atomic_uint_fast64_t atomic_iperf_size_t;
 #define OPT_RCV_TIMEOUT 27
 #define OPT_JSON_STREAM 28
 #define OPT_SND_TIMEOUT 29
+#define OPT_MPTCP 30
 
 /* states */
 #define TEST_START 1
diff --git a/src/iperf_client_api.c b/src/iperf_client_api.c
index 7ad4c93..01dceb6 100644
--- a/src/iperf_client_api.c
+++ b/src/iperf_client_api.c
@@ -399,7 +399,7 @@ iperf_connect(struct iperf_test *test)
     /* Create and connect the control channel */
     if (test->ctrl_sck < 0)
 	// Create the control channel using an ephemeral port
-	test->ctrl_sck = netdial(test->settings->domain, Ptcp, test->bind_address, test->bind_dev, 0, test->server_hostname, test->server_port, test->settings->connect_timeout);
+	test->ctrl_sck = netdial(test->settings->domain, Ptcp, test->settings->mptcp ? IPPROTO_MPTCP : 0, test->bind_address, test->bind_dev, 0, test->server_hostname, test->server_port, test->settings->connect_timeout);
     if (test->ctrl_sck < 0) {
         i_errno = IECONNECT;
         return -1;
diff --git a/src/iperf_locale.c b/src/iperf_locale.c
index ae0f63a..7e00fb6 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)
+                           "  --mptcp                   Multipath TCP support\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..cb88317 100644
--- a/src/iperf_tcp.c
+++ b/src/iperf_tcp.c
@@ -210,7 +210,7 @@ iperf_tcp_listen(struct iperf_test *test)
             return -1;
         }
 
-        if ((s = socket(res->ai_family, SOCK_STREAM, 0)) < 0) {
+        if ((s = socket(res->ai_family, SOCK_STREAM, res->ai_protocol)) < 0) {
 	    freeaddrinfo(res);
             i_errno = IESTREAMLISTEN;
             return -1;
@@ -376,7 +376,7 @@ iperf_tcp_connect(struct iperf_test *test)
     int saved_errno;
     int rcvbuf_actual, sndbuf_actual;
 
-    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, test->settings->mptcp ? IPPROTO_MPTCP : 0, 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/iperf_udp.c b/src/iperf_udp.c
index a603236..4e651fe 100644
--- a/src/iperf_udp.c
+++ b/src/iperf_udp.c
@@ -507,7 +507,7 @@ iperf_udp_connect(struct iperf_test *test)
     int i, max_len_wait_for_reply;
 
     /* Create and bind our local socket. */
-    if ((s = netdial(test->settings->domain, Pudp, test->bind_address, test->bind_dev, test->bind_port, test->server_hostname, test->server_port, -1)) < 0) {
+    if ((s = netdial(test->settings->domain, Pudp, 0, test->bind_address, test->bind_dev, test->bind_port, test->server_hostname, test->server_port, -1)) < 0) {
         i_errno = IESTREAMCONNECT;
         return -1;
     }
diff --git a/src/net.c b/src/net.c
index c82caff..b8d3da5 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);
@@ -233,12 +233,12 @@ create_socket(int domain, int proto, const char *local, const char *bind_dev, in
 
 /* make connection to server */
 int
-netdial(int domain, int proto, const char *local, const char *bind_dev, int local_port, const char *server, int port, int timeout)
+netdial(int domain, int type, int proto, const char *local, const char *bind_dev, int local_port, const char *server, int port, int timeout)
 {
     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, type, proto, local, bind_dev, local_port, server, port, &server_res);
     if (s < 0) {
       return -1;
     }
@@ -289,7 +289,7 @@ netannounce(int domain, int proto, const char *local, const char *bind_dev, int
     if ((gerror = getaddrinfo(local, portstr, &hints, &res)) != 0)
         return -1;
 
-    s = socket(res->ai_family, proto, 0);
+    s = socket(res->ai_family, proto, res->ai_protocol);
     if (s < 0) {
 	freeaddrinfo(res);
         return -1;
diff --git a/src/net.h b/src/net.h
index f0e1b4f..cd0afb7 100644
--- a/src/net.h
+++ b/src/net.h
@@ -28,8 +28,8 @@
 #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 netdial(int domain, int proto, const char *local, const char *bind_dev, int local_port, const char *server, int port, int timeout);
+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 type, 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);
 int Nwrite(int fd, const char *buf, size_t count, int prot) /* __attribute__((hot)) */;
-- 
2.40.1
Re: [PATCH iperf3 v2] add MPTCPv1 support
Posted by Matthieu Baerts 1 month, 3 weeks ago
Hi Geliang,

On 06/03/2024 04:28, Geliang Tang wrote:
> The Multipath TCP (MPTCP) protocol (v1 / RFC 8684) has been added in the
> upstream Linux kernel since v5.16.
> 
> 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.
> 
> This patch adds a new flag --mptcp to support MPTCPv1. It can be used
> like this:
> 
>  > iperf3 --mptcp -s
>  > iperf3 --mptcp -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.
> 
> Linked: https://github.com/esnet/iperf/pull/1166

(...)

> diff --git a/src/iperf_api.c b/src/iperf_api.c
> index 1dcfaab..686b78b 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)
> +        {"mptcp", no_argument, NULL, OPT_MPTCP},
> +#endif

I guess you will need to add something in the man page (src/iperf3.1)
and in the usage menu (src/iperf_locale.c) as well.

>          {"debug", optional_argument, NULL, 'd'},
>          {"help", no_argument, NULL, 'h'},
>          {NULL, 0, NULL, 0}
> @@ -1639,6 +1642,11 @@ 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 OPT_MPTCP:
> +		test->settings->mptcp = 1;
> +		break;
> +#endif
>  	    case 'h':
>  		usage_long(stdout);
>  		exit(0);

I see in Paolo's version that he added the mptcp option in the JSON that
is sent to the server (send_parameters/get_parameters). Does that mean
that if only the client asks for MPTCP, the server will create a
listening socket for the data with MPTCP support? It might be good to do
that: so we would not need to ask people maintaining public IPerf3
servers to enable MPTCP support, MPTCP will be used if it is asked by
the client! We would only need to set the option on the client side:

  iperf3 -c <SERVER> --mptcp

And keep the server as it was (as long as it is up-to-date).

But would it work if we do that? I mean: when is created the listening
socket for the data connection? After the client has sent the JSON via
the control connection? Or before?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH iperf3 v2] add MPTCPv1 support
Posted by Matthieu Baerts 1 month, 3 weeks ago
Hi Geliang,

(Without the iperf3 maintainer in CC)

On 06/03/2024 04:28, Geliang Tang wrote:
> The Multipath TCP (MPTCP) protocol (v1 / RFC 8684) has been added in the
> upstream Linux kernel since v5.16.

since v5.6, not v5.16

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

Maybe add "if it is recent enough)

> This patch adds a new flag --mptcp to support MPTCPv1. It can be used
> like this:
> 
>  > iperf3 --mptcp -s
>  > iperf3 --mptcp -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.)

> Linked: https://github.com/esnet/iperf/pull/1166

Thank you for the new version!

The code looks good to me, but that's up to the IPerf maintainers to
accept it or not.

I guess the new patches should be proposed via a new GitHub pull
request. Do you mind opening a new one, and mentioning it is replacing
the one from Paolo and https://github.com/esnet/iperf/pull/1615 (just a
rebase from what I understood)

> Suggested-by: Paolo Abeni <pabeni@redhat.com>

Co-developed-by maybe?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.