Skip to content

Commit 6be6a8e

Browse files
committed
erts: Only save the priority around setting IP_TOS
Every setsockopt went through a save and restore of both SO_PRIORITY and IP_TOS, on the grounds that "if any other option is set after tos, tos might be zeroed" - a comment carried over from the inet driver. That costs two getsockopt calls and up to two more setsockopt calls on every option set. Only half of it holds. Setting IP_TOS does make the kernel derive a new SO_PRIORITY from it, so that one option has to put the priority back. Setting SO_PRIORITY leaves the tos alone, and so does setting anything else: verified on Linux 7.0 with TCP_NODELAY, SO_RCVBUF and SO_LINGER, none of which disturbed either value. So do it only for IP_TOS, and only for the priority. Setting an option on an accepted connection goes from four system calls to one. The socket options a connection sets still look independent to the user, which is what the dance was there for.
1 parent ea8f22b commit 6be6a8e

1 file changed

Lines changed: 30 additions & 43 deletions

File tree

erts/emulator/nifs/common/prim_socket_nif.c

Lines changed: 30 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -11471,57 +11471,44 @@ int socket_setopt(int sock, int level, int opt,
1147111471
int res;
1147211472

1147311473
#if defined(IP_TOS) && defined(SOL_IP) && defined(SO_PRIORITY)
11474-
int tmpIValPRIO = 0;
11475-
int tmpIValTOS = 0;
11476-
int resPRIO;
11477-
int resTOS;
11478-
SOCKOPTLEN_T tmpArgSzPRIO = sizeof(tmpIValPRIO);
11479-
SOCKOPTLEN_T tmpArgSzTOS = sizeof(tmpIValTOS);
11480-
11481-
resPRIO = sock_getopt(sock, SOL_SOCKET, SO_PRIORITY,
11482-
&tmpIValPRIO, &tmpArgSzPRIO);
11483-
resTOS = sock_getopt(sock, SOL_IP, IP_TOS,
11484-
&tmpIValTOS, &tmpArgSzTOS);
11474+
/* Setting IP_TOS makes the kernel derive a new SO_PRIORITY from it,
11475+
* so that one option - and only that one - has to have the priority
11476+
* saved and put back to keep the two looking independent to the
11477+
* user. Setting anything else leaves both alone, and paying a
11478+
* getsockopt for each of them plus a setsockopt to restore on every
11479+
* option set was most of the system calls a connection made.
11480+
*/
11481+
if ((level == SOL_IP) && (opt == IP_TOS)) {
11482+
int savedPRIO;
11483+
int resPRIO;
11484+
SOCKOPTLEN_T savedSzPRIO = sizeof(savedPRIO);
1148511485

11486-
res = sock_setopt(sock, level, opt, optVal, optLen);
11487-
if (res == 0) {
11486+
resPRIO = sock_getopt(sock, SOL_SOCKET, SO_PRIORITY,
11487+
&savedPRIO, &savedSzPRIO);
1148811488

11489-
/* Ok, now we *maybe* need to "maybe" restore PRIO and TOS...
11490-
* maybe, possibly, ...
11491-
*/
11489+
res = sock_setopt(sock, level, opt, optVal, optLen);
1149211490

11493-
if (opt != SO_PRIORITY) {
11494-
if ((opt != IP_TOS) && (resTOS == 0)) {
11495-
resTOS = sock_setopt(sock, SOL_IP, IP_TOS,
11496-
(void *) &tmpIValTOS,
11497-
tmpArgSzTOS);
11498-
res = resTOS;
11499-
}
11500-
if ((res == 0) && (resPRIO == 0)) {
11501-
resPRIO = sock_setopt(sock, SOL_SOCKET, SO_PRIORITY,
11502-
&tmpIValPRIO,
11503-
tmpArgSzPRIO);
11504-
11505-
/* Some kernels set a SO_PRIORITY by default
11506-
* that you are not permitted to reset,
11507-
* silently ignore this error condition.
11508-
*/
11509-
11510-
if ((resPRIO != 0) && (sock_errno() == EPERM)) {
11511-
res = 0;
11512-
} else {
11513-
res = resPRIO;
11514-
}
11515-
}
11516-
}
11517-
}
11491+
if ((res == 0) && (resPRIO == 0)) {
11492+
resPRIO = sock_setopt(sock, SOL_SOCKET, SO_PRIORITY,
11493+
&savedPRIO, savedSzPRIO);
1151811494

11519-
#else
11495+
/* Some kernels set a SO_PRIORITY by default
11496+
* that you are not permitted to reset,
11497+
* silently ignore this error condition.
11498+
*/
1152011499

11521-
res = sock_setopt(sock, level, opt, optVal, optLen);
11500+
if ((resPRIO != 0) && (sock_errno() == EPERM))
11501+
res = 0;
11502+
else
11503+
res = resPRIO;
11504+
}
1152211505

11506+
return res;
11507+
}
1152311508
#endif
1152411509

11510+
res = sock_setopt(sock, level, opt, optVal, optLen);
11511+
1152511512
return res;
1152611513
}
1152711514

0 commit comments

Comments
 (0)