Skip to content

Commit

Permalink
Don't set O_NONBLOCK on sockets used for tests.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bmah888 committed Nov 8, 2023
1 parent 20a02b4 commit 6e75b07
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 25 deletions.
13 changes: 0 additions & 13 deletions src/iperf_client_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -662,12 +662,6 @@ iperf_run_client(struct iperf_test * test)
goto cleanup_and_fail;
}

// Set non-blocking for non-UDP tests
if (test->protocol->id != Pudp) {
SLIST_FOREACH(sp, &test->streams, streams) {
setnonblocking(sp->socket, 1);
}
}
}

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

// Unset non-blocking for non-UDP tests
if (test->protocol->id != Pudp) {
SLIST_FOREACH(sp, &test->streams, streams) {
setnonblocking(sp->socket, 0);
}
}

/* Yes, done! Send TEST_END. */
test->done = 1;
cpu_util(test->cpu_util);
Expand Down
11 changes: 0 additions & 11 deletions src/iperf_server_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -757,17 +757,6 @@ iperf_run_server(struct iperf_test *test)

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

/*
* If the protocol isn't UDP, or even if it is but
* we're the receiver, set nonblocking sockets.
* We need this to allow a server receiver to
* maintain interactivity with the control channel.
*/
if (test->protocol->id != Pudp ||
!sp->sender) {
setnonblocking(s, 1);
}

if (test->on_new_stream)
test->on_new_stream(sp);

Expand Down
5 changes: 4 additions & 1 deletion src/net.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* iperf, Copyright (c) 2014-2019, The Regents of the University of
* iperf, Copyright (c) 2014-2023, The Regents of the University of
* California, through Lawrence Berkeley National Laboratory (subject
* to receipt of any required approvals from the U.S. Dept. of
* Energy). All rights reserved.
Expand Down Expand Up @@ -405,6 +405,7 @@ Nread(int fd, char *buf, size_t count, int prot)
while (nleft > 0) {
r = read(fd, buf, nleft);
if (r < 0) {
/* XXX EWOULDBLOCK can't happen without non-blocking sockets */
if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK)
break;
else
Expand Down Expand Up @@ -469,6 +470,7 @@ Nwrite(int fd, const char *buf, size_t count, int prot)
case EINTR:
case EAGAIN:
#if (EAGAIN != EWOULDBLOCK)
/* XXX EWOULDBLOCK can't happen without non-blocking sockets */
case EWOULDBLOCK:
#endif
return count - nleft;
Expand Down Expand Up @@ -539,6 +541,7 @@ Nsendfile(int fromfd, int tofd, const char *buf, size_t count)
case EINTR:
case EAGAIN:
#if (EAGAIN != EWOULDBLOCK)
/* XXX EWOULDBLOCK can't happen without non-blocking sockets */
case EWOULDBLOCK:
#endif
if (count == nleft)
Expand Down

0 comments on commit 6e75b07

Please sign in to comment.