src/iperf.h | 5 +++++ src/iperf_api.c | 4 ++++ src/iperf_api.h | 1 + src/iperf_client_api.c | 2 +- src/iperf_locale.c | 1 + src/iperf_tcp.c | 4 ++-- src/iperf_tcp.h | 1 - src/iperf_udp.c | 2 +- src/net.c | 14 +++++++------- src/net.h | 4 ++-- 10 files changed, 24 insertions(+), 14 deletions(-)
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
Linked: https://github.com/esnet/iperf/pull/1166
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
src/iperf.h | 5 +++++
src/iperf_api.c | 4 ++++
src/iperf_api.h | 1 +
src/iperf_client_api.c | 2 +-
src/iperf_locale.c | 1 +
src/iperf_tcp.c | 4 ++--
src/iperf_tcp.h | 1 -
src/iperf_udp.c | 2 +-
src/net.c | 14 +++++++-------
src/net.h | 4 ++--
10 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/src/iperf.h b/src/iperf.h
index 0051a30..89af39b 100644
--- a/src/iperf.h
+++ b/src/iperf.h
@@ -63,6 +63,10 @@
# endif
#endif
+#ifndef IPPROTO_MPTCP
+#define IPPROTO_MPTCP 262
+#endif
+
#include "timer.h"
#include "queue.h"
#include "cjson.h"
@@ -167,6 +171,7 @@ 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 */
+ int mptcp;
};
struct iperf_test;
diff --git a/src/iperf_api.c b/src/iperf_api.c
index a2cd53a..e19ca38 100644
--- a/src/iperf_api.c
+++ b/src/iperf_api.c
@@ -1104,6 +1104,7 @@ 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},
+ {"mptcp", no_argument, NULL, OPT_MPTCP},
{"debug", optional_argument, NULL, 'd'},
{"help", no_argument, NULL, 'h'},
{NULL, 0, NULL, 0}
@@ -1594,6 +1595,9 @@ iperf_parse_arguments(struct iperf_test *test, int argc, char **argv)
test->settings->connect_timeout = unit_atoi(optarg);
client_flag = 1;
break;
+ case OPT_MPTCP:
+ test->settings->mptcp = 1;
+ break;
case 'h':
usage_long(stdout);
exit(0);
diff --git a/src/iperf_api.h b/src/iperf_api.h
index 171006a..79d19cc 100644
--- a/src/iperf_api.h
+++ b/src/iperf_api.h
@@ -90,6 +90,7 @@ typedef uint64_t iperf_size_t;
#define OPT_DONT_FRAGMENT 26
#define OPT_RCV_TIMEOUT 27
#define OPT_SND_TIMEOUT 28
+#define OPT_MPTCP 29
/* states */
#define TEST_START 1
diff --git a/src/iperf_client_api.c b/src/iperf_client_api.c
index 5833068..83909eb 100644
--- a/src/iperf_client_api.c
+++ b/src/iperf_client_api.c
@@ -378,7 +378,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 9eec77b..0f854f3 100644
--- a/src/iperf_locale.c
+++ b/src/iperf_locale.c
@@ -124,6 +124,7 @@ 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 */
+ " --mptcp Multipath TCP support\n"
" -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 ce6a522..954c299 100644
--- a/src/iperf_tcp.c
+++ b/src/iperf_tcp.c
@@ -194,7 +194,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;
@@ -375,7 +375,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_tcp.h b/src/iperf_tcp.h
index d53b052..3071cbf 100644
--- a/src/iperf_tcp.h
+++ b/src/iperf_tcp.h
@@ -27,7 +27,6 @@
#ifndef IPERF_TCP_H
#define IPERF_TCP_H
-
/**
* iperf_tcp_accept -- accepts a new TCP connection
* on tcp_listener_socket for TCP data and param/result
diff --git a/src/iperf_udp.c b/src/iperf_udp.c
index 5dc1422..ec1163f 100644
--- a/src/iperf_udp.c
+++ b/src/iperf_udp.c
@@ -514,7 +514,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 1a88155..c0d185d 100644
--- a/src/net.c
+++ b/src/net.c
@@ -121,7 +121,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;
@@ -130,14 +130,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)
@@ -145,7 +145,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);
@@ -230,12 +230,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;
}
@@ -286,7 +286,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.35.3Hi Geliang,
Thank you for this, that would be helpful!
The code looks good to me, I just have one suggestion for the commit
message, one "typo" with the alignment and one small suggestion in the
code. Please see below.
I think you can create a new PR on esnet/iperf repo (and mention it
replaces the one of Paolo).
On 27/06/2023 07:20, 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.
I quickly looked at the comments that have been posted on Paolo's PR on
Github and it looks like the blocking one was that the configure.ac file
has not been modified to check if the feature was available or not.
I think you should mention in the commit message that 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.
The only thing I see we could do is to restrict this feature to Linux
builds. But this can be done later if they ask for it I guess.
> 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
>
> Linked: https://github.com/esnet/iperf/pull/1166
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> src/iperf.h | 5 +++++
> src/iperf_api.c | 4 ++++
> src/iperf_api.h | 1 +
> src/iperf_client_api.c | 2 +-
> src/iperf_locale.c | 1 +
> src/iperf_tcp.c | 4 ++--
> src/iperf_tcp.h | 1 -
> src/iperf_udp.c | 2 +-
> src/net.c | 14 +++++++-------
> src/net.h | 4 ++--
> 10 files changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/src/iperf.h b/src/iperf.h
> index 0051a30..89af39b 100644
> --- a/src/iperf.h
> +++ b/src/iperf.h
> @@ -63,6 +63,10 @@
> # endif
> #endif
>
> +#ifndef IPPROTO_MPTCP
> +#define IPPROTO_MPTCP 262
> +#endif
> +
> #include "timer.h"
> #include "queue.h"
> #include "cjson.h"
> @@ -167,6 +171,7 @@ 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 */
> + int mptcp;
> };
>
> struct iperf_test;
> diff --git a/src/iperf_api.c b/src/iperf_api.c
> index a2cd53a..e19ca38 100644
> --- a/src/iperf_api.c
> +++ b/src/iperf_api.c
> @@ -1104,6 +1104,7 @@ 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},
> + {"mptcp", no_argument, NULL, OPT_MPTCP},
> {"debug", optional_argument, NULL, 'd'},
> {"help", no_argument, NULL, 'h'},
> {NULL, 0, NULL, 0}
> @@ -1594,6 +1595,9 @@ iperf_parse_arguments(struct iperf_test *test, int argc, char **argv)
> test->settings->connect_timeout = unit_atoi(optarg);
> client_flag = 1;
> break;
> + case OPT_MPTCP:
> + test->settings->mptcp = 1;
> + break;
It looks like you used spaces instead of tabs here.
(the whole iperf3 code doesn't look like they are using one or the other
in a consistent way but I guess it is best to imitate what is done here
just above/below)
> case 'h':
> usage_long(stdout);
> exit(0);
(...)
> --- 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);
I don't know if you should rename proto to type in netannounce() as well
just for consistency reasons. Up to you
> 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)) */;
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
Hi Geliang, On 30/06/2023 18:03, Matthieu Baerts wrote: > Hi Geliang, > > Thank you for this, that would be helpful! > > The code looks good to me, I just have one suggestion for the commit > message, one "typo" with the alignment and one small suggestion in the > code. Please see below. > > I think you can create a new PR on esnet/iperf repo (and mention it > replaces the one of Paolo). What do you plan to do with this patch? There is no hurry at all to suggest it upstream (esnet/iperf) but it is mainly to know if you are expecting something from one of us, just to avoid a deadlock :) Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
On Wed, Jul 12, 2023 at 10:14:31AM +0200, Matthieu Baerts wrote: > Hi Geliang, > > On 30/06/2023 18:03, Matthieu Baerts wrote: > > Hi Geliang, > > > > Thank you for this, that would be helpful! > > > > The code looks good to me, I just have one suggestion for the commit > > message, one "typo" with the alignment and one small suggestion in the > > code. Please see below. > > > > I think you can create a new PR on esnet/iperf repo (and mention it > > replaces the one of Paolo). > > What do you plan to do with this patch? > > There is no hurry at all to suggest it upstream (esnet/iperf) but it is > mainly to know if you are expecting something from one of us, just to > avoid a deadlock :) Hi Matt, Thanks for the review, a v2 will be sent later. -Geliang > > Cheers, > Matt > -- > Tessares | Belgium | Hybrid Access Solutions > www.tessares.net
Hi Geliang, On 13/07/2023 09:03, Geliang Tang wrote: > On Wed, Jul 12, 2023 at 10:14:31AM +0200, Matthieu Baerts wrote: >> Hi Geliang, >> >> On 30/06/2023 18:03, Matthieu Baerts wrote: >>> Hi Geliang, >>> >>> Thank you for this, that would be helpful! >>> >>> The code looks good to me, I just have one suggestion for the commit >>> message, one "typo" with the alignment and one small suggestion in the >>> code. Please see below. >>> >>> I think you can create a new PR on esnet/iperf repo (and mention it >>> replaces the one of Paolo). >> >> What do you plan to do with this patch? >> >> There is no hurry at all to suggest it upstream (esnet/iperf) but it is >> mainly to know if you are expecting something from one of us, just to >> avoid a deadlock :) > > Hi Matt, > > Thanks for the review, a v2 will be sent later. Thank you, no hurry at all, take your time! If you prefer, you can also directly send the v2 to esnet/iperf repo, up to you, I don't mind reviewing it before or after. Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
Hi Matt, > > Hi Geliang, > > On 13/07/2023 09:03, Geliang Tang wrote: > > On Wed, Jul 12, 2023 at 10:14:31AM +0200, Matthieu Baerts wrote: > >> Hi Geliang, > >> > >> On 30/06/2023 18:03, Matthieu Baerts wrote: > >>> Hi Geliang, > >>> > >>> Thank you for this, that would be helpful! > >>> > >>> The code looks good to me, I just have one suggestion for the commit > >>> message, one "typo" with the alignment and one small suggestion in the > >>> code. Please see below. > >>> > >>> I think you can create a new PR on esnet/iperf repo (and mention it > >>> replaces the one of Paolo). > >> > >> What do you plan to do with this patch? > >> > >> There is no hurry at all to suggest it upstream (esnet/iperf) but it is > >> mainly to know if you are expecting something from one of us, just to > >> avoid a deadlock :) > > > > Hi Matt, > > > > Thanks for the review, a v2 will be sent later. > > Thank you, no hurry at all, take your time! > > If you prefer, you can also directly send the v2 to repo, up > to you, I don't mind reviewing it before or after. I sent out v2 to MPTCP ML and the iperf maintainer today. Sorry, v2 came too late. You can now update the status on patchwork. Thanks, -Geliang > > Cheers, > Matt > -- > Tessares | Belgium | Hybrid Access Solutions > www.tessares.net >
---------- Forwarded message --------- 发件人： Geliang Tang <geliangtang@gmail.com> Date: 2024年3月6日周三 11:42 Subject: Re: [PATCH iperf3] add MPTCPv1 support To: Matthieu Baerts <matthieu.baerts@tessares.net> Cc: Geliang Tang <geliang.tang@suse.com>, <mptcp@lists.linux.dev> Hi Matt, > > Hi Geliang, > > On 13/07/2023 09:03, Geliang Tang wrote: > > On Wed, Jul 12, 2023 at 10:14:31AM +0200, Matthieu Baerts wrote: > >> Hi Geliang, > >> > >> On 30/06/2023 18:03, Matthieu Baerts wrote: > >>> Hi Geliang, > >>> > >>> Thank you for this, that would be helpful! > >>> > >>> The code looks good to me, I just have one suggestion for the commit > >>> message, one "typo" with the alignment and one small suggestion in the > >>> code. Please see below. > >>> > >>> I think you can create a new PR on esnet/iperf repo (and mention it > >>> replaces the one of Paolo). > >> > >> What do you plan to do with this patch? > >> > >> There is no hurry at all to suggest it upstream (esnet/iperf) but it is > >> mainly to know if you are expecting something from one of us, just to > >> avoid a deadlock :) > > > > Hi Matt, > > > > Thanks for the review, a v2 will be sent later. > > Thank you, no hurry at all, take your time! > > If you prefer, you can also directly send the v2 to repo, up > to you, I don't mind reviewing it before or after. I sent out v2 to MPTCP ML and the iperf maintainer today. Sorry, v2 came too late. You can now update the status on patchwork. Thanks, -Geliang > > Cheers, > Matt > -- > Tessares | Belgium | Hybrid Access Solutions > www.tessares.net >
© 2016 - 2024 Red Hat, Inc.