Add Time Flag For The Iperf Server Process #1684
Conversation
manedurphy
force-pushed
the
server_timeout
branch
from
ad3e789 to
c407fcf
Compare
manedurphy
force-pushed
the
server_timeout
branch
from
c407fcf to
98e915e
Compare
manedurphy
force-pushed
the
server_timeout
branch
from
0740526 to
49063c8
Compare
|
Update (05/22/2025) @bmah888 expressed concern over the value for the added |
manedurphy
force-pushed
the
server_timeout
branch
from
65e9786 to
d0a1936
Compare
|
Thanks for the new version of the PR! I'll have to play around with happens w.r.t. interoperability between different versions of software. Ideally yes the control protocol would allow the client and server to exchange version numbers or API versions and behave in some compatible manner accordingly. If I was going to redesign iperf3 that'd be a part of the control protocol, for sure. I'm haven't thought about what's the right thing to do in this situation. |
manedurphy
force-pushed
the
server_timeout
branch
from
d0a1936 to
091986a
Compare
|
@bmah888 have you had some time to consider the approach we should take here given the constraints? @davidBar-On your input on this would also be appreciated. It appears that it has always been an issue that an So while a more sophisticated approach for providing more context to older versions of the client would be ideal, I'm not sure that it is absolutely necessary given the historical context. At least not necessary to move forward with this feature. An enhancement for this can be added to the code base separately. |
I agree with you here...iperf3 wasn't designed to support perfect forward/backward compatibility between arbitrary versions of code. It was intended for deployments within a single administrative domain (or at least between cooperating administrative domains), where the requirements for compatibility are lessened a bit. In this case all we're missing is the ability to display a meaningful error message to the end-user on the client side. |
|
I've been testing this and it seems to be working as designed. One thing just occurred to me that if you run the server as |
|
@bmah888 I'm open to adding those protections. I think having a flag per protection is fine since the goal is to give the administrator of the
For me, personally, as an I'd want to know if these additional changes should be implemented as part of these changes, or if I should raise them in a separate PR. I'm open to either, but tend to prefer raising separate PRs for separation of concern for the PR template. |
I assume that means that the duration that a specific client is using the server is the most important limit. In this case, instead of limiting bytes/blocks, at the beginning of the test the server can start a timer that will expire after the Actually, the server already does something similar, in max_duration = test->duration + test->omit + grace_period;
if (test->max_server_duration > 0 && test->max_server_duration < max_duration) {
max_duration = test->max_server_duration;
}
test->timer = tmr_create(&now, server_timer_proc, cd, max_duration * SEC_TO_US, 0); |
Thanks @manedurphy! Herein we're running into a difference in use cases between what iperf3 was originally designed for, and what people would (quite reasonably) like it to do. :-) iperf3 was originally designed to test between two specific hosts (i.e. client and server) at a time. In the networking environment it was designed for, it's very possible that the end hosts, rather than the network, are the bottlenecks to performance. So only one test per server process can be running at once (and ideally only one test per host at a time), to avoid interference between tests. There was also a general assumption that the client and server were loosely affiliated, or at least co-operating. This is very different from people who want to run large-scale iperf3 speed-test type services that serve dozens or hundreds of potentially unknown clients, some of which might need some guardrails. I understand that the PR we're discussing is an attempt to adapt iperf3 to this use case, which is great. I'm just trying to poke (in a friendly way, or at least I'm trying!) at the solution to see what else might be needed to fit the requirements. I appreciate wanting to support (server-side) limits on parameters as well, although I am not sure how these options should interact, particularly when the client side can only specify one of the ending criteria. So...my question back to you is do you think this PR is still useful as-is? I haven't quite made up my mind on this. (I see where @davidBar-On has some feedback also, which I haven't read yet.) |
|
@bmah888 I appreciate the additional context for @davidBar-On your proposed change of using the value of Guidelines:
Since it is always possible for the server to determine if the client's duration will violate it's protection, it will either reject or permit with success. For other protections such as bit rate or bytes, the server may not be able to determine if its protections will be violated, and will have to react accordingly. |
|
@manedurphy, thanks for the detailed response. In general, I fully agree with what you wrote.
I agree that both checks should be done , both when receiving the client's parameters and during the test and that the parameters check should be added regarding
I believe that I found a reasonable solution (actually a workaround) to this problem in PR #1909. Once this PR (or a variant of it) will be merged, the client will (also) print the correct error message. The idea is that if the server finds a problem with a parameter received from the client, it will send the error code as the next State to client (after making sure that no State and error code have the same value). When the client receive the "State", instead of just printing |
manedurphy
force-pushed
the
server_timeout
branch
from
4ebe02a to
cd87599
Compare
|
@davidBar-On I was thinking of following the same pattern to convey the message. I've pasted a snippet below, but I'll open a new PR which has both the error handling and input validation for the sake of keeping this PR focused on handling the time parameter. snippet// Check if average transfer rate was exceeded (condition set in the callback routines)
if (test->bitrate_limit_exceeded) {
i_errno = IETOTALRATE;
if (iperf_set_send_state(test, SERVER_ERROR) != 0) {
cleanup_server(test);
return -1;
}
err = htonl(i_errno);
if (Nwrite(test->ctrl_sck, (char*) &err, sizeof(err), Ptcp) < 0) {
cleanup_server(test);
i_errno = IECTRLWRITE;
return -1;
}
err = htonl(errno);
if (Nwrite(test->ctrl_sck, (char*) &err, sizeof(err), Ptcp) < 0) {
cleanup_server(test);
i_errno = IECTRLWRITE;
return -1;
}
cleanup_server(test);
return -1;
}My latest push here includes writing |
|
@manedurphy thanks for your patience in dealing with feedback on this PR! I'm pretty happy with this in the state it's in, except there's a merge conflict with Also I noticed your PR #1931, you probably want that in as well before we do iperf-3.20, right? |
manedurphy
force-pushed
the
server_timeout
branch
from
cd87599 to
215ef3b
Compare
|
I think all of the PRs that were asked for |
bmah888
left a comment
bmah888
left a comment
There was a problem hiding this comment.
Looks good, quick testing seems to do the right thing.
I'm intending to do a squash and merge on the whole thing since it's more or less hitting the master branch as a single body of work.
3 participants
PLEASE NOTE the following text from the iperf3 license. Submitting a
pull request to the iperf3 repository constitutes "[making]
Enhancements available...publicly":
The complete iperf3 license is available in the
LICENSEfile in thetop directory of the iperf3 source tree.
Version of iperf3 (or development branch, such as
masteror3.1-STABLE) to which this pull request applies: masterIssues fixed (if any): Enable '-t X' option in server mode #354, Server-side limiting switches #615
Brief description of code changes (suitable for use as a commit message):
These changes add the functionality as requested by the linked issues. Specifically, this functionality allows the server to determine the maximum duration of an
iperfmeasurement. The implementation is similar to the--server-bitrate-limitflag, where the measurement is forcibly stopped when an interval has exceeded the server's configured bitrate. In this case, the measurement is forcibly stopped when the duration exceeds the server's configured time, which is the value of--server-time.Example
From the dropdowns below, we can see that the server has enforced a measurement time of
3seconds. The client attempts to run the measurement for4seconds. When the 3rd second is exceeded, the socket is forcibly closed.server
client
client_json
{ "start": { "connected": [{ "socket": 5, "local_host": "127.0.0.1", "local_port": 53314, "remote_host": "127.0.0.1", "remote_port": 5201 }], "version": "iperf 3.16+", "system_info": "Linux windows-nexus 5.15.146.1-microsoft-standard-WSL2 #1 SMP Thu Jan 11 04:09:03 UTC 2024 x86_64", "timestamp": { "time": "Thu, 18 Apr 2024 04:38:09 GMT", "timesecs": 1713415089 }, "connecting_to": { "host": "127.0.0.1", "port": 5201 }, "cookie": "wtuhwwetnqayseccmqno6dutzjf3zly3op5p", "tcp_mss_default": 32768, "target_bitrate": 0, "fq_rate": 0, "sock_bufsize": 0, "sndbuf_actual": 16384, "rcvbuf_actual": 131072, "test_start": { "protocol": "TCP", "num_streams": 1, "blksize": 131072, "omit": 3, "duration": 4, "bytes": 0, "blocks": 0, "reverse": 0, "tos": 0, "target_bitrate": 0, "bidir": 0, "fqrate": 0, "interval": 1 } }, "intervals": [{ "streams": [{ "socket": 5, "start": 0, "end": 1.001066, "seconds": 1.0010659694671631, "bytes": 3578265600, "bits_per_second": 28595642717.968742, "retransmits": 0, "snd_cwnd": 2029973, "snd_wnd": 3145088, "rtt": 51, "rttvar": 5, "pmtu": 65535, "omitted": true, "sender": true }], "sum": { "start": 0, "end": 1.001066, "seconds": 1.0010659694671631, "bytes": 3578265600, "bits_per_second": 28595642717.968742, "retransmits": 0, "omitted": true, "sender": true } }, { "streams": [{ "socket": 5, "start": 1.001066, "end": 2.001046, "seconds": 0.99997997283935547, "bytes": 3452698624, "bits_per_second": 27622142185.078888, "retransmits": 0, "snd_cwnd": 2029973, "snd_wnd": 3145088, "rtt": 88, "rttvar": 67, "pmtu": 65535, "omitted": true, "sender": true }], "sum": { "start": 1.001066, "end": 2.001046, "seconds": 0.99997997283935547, "bytes": 3452698624, "bits_per_second": 27622142185.078888, "retransmits": 0, "omitted": true, "sender": true } }, { "streams": [{ "socket": 5, "start": 2.001046, "end": 3.001049, "seconds": 1.0000029802322388, "bytes": 3500015616, "bits_per_second": 28000041481.373692, "retransmits": 0, "snd_cwnd": 2029973, "snd_wnd": 3145088, "rtt": 63, "rttvar": 17, "pmtu": 65535, "omitted": true, "sender": true }], "sum": { "start": 2.001046, "end": 3.001049, "seconds": 1.0000029802322388, "bytes": 3500015616, "bits_per_second": 28000041481.373692, "retransmits": 0, "omitted": true, "sender": true } }, { "streams": [{ "socket": 5, "start": 0.012641, "end": 0.988366, "seconds": 1.0010069608688354, "bytes": 3449421824, "bits_per_second": 27567615082.364941, "retransmits": 0, "snd_cwnd": 2029973, "snd_wnd": 3145088, "rtt": 58, "rttvar": 7, "pmtu": 65535, "omitted": false, "sender": true }], "sum": { "start": 0.012641, "end": 0.988366, "seconds": 1.0010069608688354, "bytes": 3449421824, "bits_per_second": 27567615082.364941, "retransmits": 0, "omitted": false, "sender": true } }, { "streams": [{ "socket": 5, "start": 0.988366, "end": 1.988398, "seconds": 1.0000319480895996, "bytes": 3414818816, "bits_per_second": 27317677780.382618, "retransmits": 0, "snd_cwnd": 2029973, "snd_wnd": 3145088, "rtt": 61, "rttvar": 9, "pmtu": 65535, "omitted": false, "sender": true }], "sum": { "start": 0.988366, "end": 1.988398, "seconds": 1.0000319480895996, "bytes": 3414818816, "bits_per_second": 27317677780.382618, "retransmits": 0, "omitted": false, "sender": true } }, { "streams": [{ "socket": 5, "start": 0.988366, "end": 1.988398, "seconds": 1.0000319480895996, "bytes": 3414818816, "bits_per_second": 27317677780.382618, "retransmits": 0, "snd_cwnd": 2029973, "snd_wnd": 3145088, "rtt": 61, "rttvar": 9, "pmtu": 65535, "omitted": false, "sender": true }], "sum": { "start": 0.988366, "end": 1.988398, "seconds": 1.0000319480895996, "bytes": 3414818816, "bits_per_second": 27317677780.382618, "retransmits": 0, "omitted": false, "sender": true } }], "end": { }, "error": "control socket has closed unexpectedly" }Implementation Details
With this implementation, the server overrides the value of
test->durationwith the value that was passed in via the--server-timeflag. We can see from the drop downs that the client's experience is similar to when the bitrate limit is exceeded with the error:iperf3: error - control socket has closed unexpectedly. However, the server side is less friendly, with an error ofiperf3: error - select failed: Bad file descriptor. I can already envision myself encountering this error and not knowing why I am getting it, as well as forgetting that I had set the--server-timeflag to begin with.Another thing to note is that nothing is stopping the user from setting a high value for
--omit. Given that this feature is meant to protect the server, is it reasonable to include a--server-omitoption to enforce a maximum value?Update (04/18/2024)
Since the first commit, I've introduced a solution for the error
iperf3: error - select failed: Bad file descriptorwhich provides a more friendly experience. Let's consider the scenario where the--server-timeflag has been set on the server to a value of3:3for the server time3. If the client's duration is greater than the server's, then the server's value is selected for the timer.The JSON output also has a new field which specifies the server's value for duration:
server_durationclient_json
{ "start": { "connected": [{ "socket": 5, "local_host": "127.0.0.1", "local_port": 36906, "remote_host": "127.0.0.1", "remote_port": 5201 }], "version": "iperf 3.16+", "system_info": "Linux windows-nexus 5.15.146.1-microsoft-standard-WSL2 #1 SMP Thu Jan 11 04:09:03 UTC 2024 x86_64", "timestamp": { "time": "Mon, 22 Apr 2024 02:23:53 GMT", "timesecs": 1713752633 }, "connecting_to": { "host": "127.0.0.1", "port": 5201 }, "cookie": "m4n5p26auofjvyda2x2ba56kexzsly7mz5k7", "tcp_mss_default": 32768, "target_bitrate": 5000000, "fq_rate": 0, "sock_bufsize": 0, "sndbuf_actual": 16384, "rcvbuf_actual": 131072, "test_start": { "protocol": "TCP", "num_streams": 1, "blksize": 131072, "omit": 0, "duration": 6, "server_duration": 3, "bytes": 0, "blocks": 0, "reverse": 0, "tos": 0, "target_bitrate": 5000000, "bidir": 0, "fqrate": 0, "interval": 1 } }, "intervals": [{ "streams": [{ "socket": 5, "start": 0, "end": 1.000087, "seconds": 1.0000870227813721, "bytes": 655360, "bits_per_second": 5242423.78970069, "retransmits": 0, "snd_cwnd": 654830, "snd_wnd": 1375232, "rtt": 38, "rttvar": 24, "pmtu": 65535, "omitted": false, "sender": true }], "sum": { "start": 0, "end": 1.000087, "seconds": 1.0000870227813721, "bytes": 655360, "bits_per_second": 5242423.78970069, "retransmits": 0, "omitted": false, "sender": true } }, { "streams": [{ "socket": 5, "start": 1.000087, "end": 2.000084, "seconds": 0.99999701976776123, "bytes": 655360, "bits_per_second": 5242895.6250465661, "retransmits": 0, "snd_cwnd": 654830, "snd_wnd": 2684928, "rtt": 45, "rttvar": 22, "pmtu": 65535, "omitted": false, "sender": true }], "sum": { "start": 1.000087, "end": 2.000084, "seconds": 0.99999701976776123, "bytes": 655360, "bits_per_second": 5242895.6250465661, "retransmits": 0, "omitted": false, "sender": true } }, { "streams": [{ "socket": 5, "start": 2.000084, "end": 3.000148, "seconds": 1.0000640153884888, "bytes": 655360, "bits_per_second": 5242544.39648379, "retransmits": 0, "snd_cwnd": 654830, "snd_wnd": 3112448, "rtt": 62, "rttvar": 44, "pmtu": 65535, "omitted": false, "sender": true }], "sum": { "start": 2.000084, "end": 3.000148, "seconds": 1.0000640153884888, "bytes": 655360, "bits_per_second": 5242544.39648379, "retransmits": 0, "omitted": false, "sender": true } }], "end": { "streams": [{ "sender": { "socket": 5, "start": 0, "end": 3.000148, "seconds": 3.000148, "bytes": 1966080, "bits_per_second": 5242621.36401271, "retransmits": 0, "max_snd_cwnd": 654830, "max_snd_wnd": 3112448, "max_rtt": 62, "min_rtt": 38, "mean_rtt": 48, "sender": true }, "receiver": { "socket": 5, "start": 0, "end": 3.000258, "seconds": 3.000148, "bytes": 1966080, "bits_per_second": 5242429.1510930061, "sender": true } }], "sum_sent": { "start": 0, "end": 3.000148, "seconds": 3.000148, "bytes": 1966080, "bits_per_second": 5242621.36401271, "retransmits": 0, "sender": true }, "sum_received": { "start": 0, "end": 3.000258, "seconds": 3.000258, "bytes": 1966080, "bits_per_second": 5242429.1510930061, "sender": true }, "cpu_utilization_percent": { "host_total": 100.87966904530634, "host_user": 100.8797023673657, "host_system": 0, "remote_total": 0.037496475331318856, "remote_user": 0.037496475331318856, "remote_system": 0 }, "sender_tcp_congestion": "cubic", "receiver_tcp_congestion": "cubic" } }Update (06/05/2024)
A valid question was brought up by @TheRealDJ:
What happens when the server has this feature but the client doesn't?. With the current implementation, there is no backwards compatability. Further discussions will need to be had in order to handle this case properly.Update (05/17/2025)
A new implementation has been made to address the backwards compatibility issue mentioned above.
Implementation Details
The server has a new flag,
--server-max-duration, represents the maximum permitted duration for aniperf3test in seconds. When the client initiates a test, it sends its desired duration to the server. The server then compares the client's requested duration with its own maximum duration. If the client's requested duration exceeds the server's maximum, the server rejects the test and sends an error message back to the client. Since older client's do not have the newly addedIEMAXSERVERDURATIONEXCEEDEDerror, the handler forint_errnofalls into the default case. The output for the server and both outputs for the client are shown below.server
client (new)
client (old)
Questions
Is this output acceptable? The error message is not very user friendly, but I'm not sure of a way to relay more information when the error message is based on the
int_errnovalue. Ideally, the server and the client should exchange information on their respective versions, but since that functionality does not currently exist, I am uncertain of the best way to handle this.Update (05/19/2025)
@TheRealDJ mentioned that the sum of the client's duration and omits must not exceed the server's maximum duration. This change has been implemented.