Skip to content

Commit 6e75b07

Browse files
committed
Don't set O_NONBLOCK on sockets used for tests.
This fixes a problem where every thread would essentially burn a CPU core busy-waiting, when it didn't need to. It's believed that this excess CPU usage might contribute to packet loss and poor performance. Non-blocking sockets were a necessity with the original single- thread process model of iperf3, in order to allow for concurrency between different test streams. With multiple threads, this is no longer necesary (it's perfectly fine for a thread to block on I/O, as it only services a single test stream and won't affect any others). Problem pointed out by @bltierney. Code review by @swlars. Fixes IPERF-177.
1 parent 20a02b4 commit 6e75b07

File tree

3 files changed

+4
-25
lines changed

3 files changed

+4
-25
lines changed

src/iperf_client_api.c

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -662,12 +662,6 @@ iperf_run_client(struct iperf_test * test)
662662
goto cleanup_and_fail;
663663
}
664664

665-
// Set non-blocking for non-UDP tests
666-
if (test->protocol->id != Pudp) {
667-
SLIST_FOREACH(sp, &test->streams, streams) {
668-
setnonblocking(sp->socket, 1);
669-
}
670-
}
671665
}
672666

673667
/* Run the timers. */
@@ -706,13 +700,6 @@ iperf_run_client(struct iperf_test * test)
706700
iperf_printf(test, "Sender threads stopped\n");
707701
}
708702

709-
// Unset non-blocking for non-UDP tests
710-
if (test->protocol->id != Pudp) {
711-
SLIST_FOREACH(sp, &test->streams, streams) {
712-
setnonblocking(sp->socket, 0);
713-
}
714-
}
715-
716703
/* Yes, done! Send TEST_END. */
717704
test->done = 1;
718705
cpu_util(test->cpu_util);

src/iperf_server_api.c

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -757,17 +757,6 @@ iperf_run_server(struct iperf_test *test)
757757

758758
if (s > test->max_fd) test->max_fd = s;
759759

760-
/*
761-
* If the protocol isn't UDP, or even if it is but
762-
* we're the receiver, set nonblocking sockets.
763-
* We need this to allow a server receiver to
764-
* maintain interactivity with the control channel.
765-
*/
766-
if (test->protocol->id != Pudp ||
767-
!sp->sender) {
768-
setnonblocking(s, 1);
769-
}
770-
771760
if (test->on_new_stream)
772761
test->on_new_stream(sp);
773762

src/net.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* iperf, Copyright (c) 2014-2019, The Regents of the University of
2+
* iperf, Copyright (c) 2014-2023, The Regents of the University of
33
* California, through Lawrence Berkeley National Laboratory (subject
44
* to receipt of any required approvals from the U.S. Dept. of
55
* Energy). All rights reserved.
@@ -405,6 +405,7 @@ Nread(int fd, char *buf, size_t count, int prot)
405405
while (nleft > 0) {
406406
r = read(fd, buf, nleft);
407407
if (r < 0) {
408+
/* XXX EWOULDBLOCK can't happen without non-blocking sockets */
408409
if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK)
409410
break;
410411
else
@@ -469,6 +470,7 @@ Nwrite(int fd, const char *buf, size_t count, int prot)
469470
case EINTR:
470471
case EAGAIN:
471472
#if (EAGAIN != EWOULDBLOCK)
473+
/* XXX EWOULDBLOCK can't happen without non-blocking sockets */
472474
case EWOULDBLOCK:
473475
#endif
474476
return count - nleft;
@@ -539,6 +541,7 @@ Nsendfile(int fromfd, int tofd, const char *buf, size_t count)
539541
case EINTR:
540542
case EAGAIN:
541543
#if (EAGAIN != EWOULDBLOCK)
544+
/* XXX EWOULDBLOCK can't happen without non-blocking sockets */
542545
case EWOULDBLOCK:
543546
#endif
544547
if (count == nleft)

0 commit comments

Comments
 (0)