Fixes for socket buffer size #16982
Merged
Fixes for socket buffer size #16982
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
poettering
reviewed
Sep 8, 2020
| r = setsockopt_int(fd, SOL_SOCKET, SO_RCVBUFFORCE, n); | ||
| if (r < 0) | ||
| return r; | ||
| } |
|
|
||
| return 0; | ||
| return fd_inc_rcvbuf(m->sock, size); |
There was a problem hiding this comment.
so, hmm, is this right? fd_inc_recvbuf() only increases the socket buffer, while the pre-existing code might also lower it if the caller asks for it. Is this an intentional change? I guess in most cases this is called to increase the buffer, but do we want to encode that it can only be used for that?
| } | ||
| r = fd_inc_rcvbuf(fd, s->receive_buffer); | ||
| if (r < 0) | ||
| log_unit_warning_errno(UNIT(s), r, "SO_RCVBUF/SO_RCVBUFFORCE failed: %m"); |
There was a problem hiding this comment.
same as above, previous code could also lower buffer size, new code only increases.
yuwata
force-pushed
the
socket-buffer-size
branch
from
8fcc033
to
d400038
Compare
|
@poettering Thank you for your review. Updated. I hope all comments are addressed. PTAL. @keszybz I think it is worth to backport this to some stable branches. |
poettering
added
the
good-to-merge/waiting-for-ci 👍
yuwata
force-pushed
the
socket-buffer-size
branch
from
d400038
to
550ac69
Compare
…ger than the kernel limit The commit 10ce2e0 inverts the order of SO_{RCV,SND}BUFFORCE and SO_{RCV,SND}BUF. However, setting buffer size with SO_{RCV,SND}BUF does not fail even if the requested size is larger than the kernel limit. Hence, SO_{RCV,SND}BUFFORCE will not use anymore and the buffer size is always limited by the kernel limit even if we have the priviledge to ignore the limit. This makes the buffer size is checked after configuring it with SO_{RCV,SND}BUF, and if it is still not sufficient, then try to set it with FORCE command. With this commit, if we have enough priviledge, the requested buffer size is correctly set. Hopefully fixes systemd#14417.
If networkd creates huge amount of netdevs, then the buffer of device monitor becomes easily flowed. Hopefully fixes systemd#16865.
yuwata
force-pushed
the
socket-buffer-size
branch
from
550ac69
to
e77f52e
Compare
|
Is there a chance this could be included in a stable systemd release or do you think this is too big of a change for the current stable version? |
keszybz
removed
the
good-to-merge/waiting-for-ci 👍
|
Note. If this will be backported to a stable repository, then please do not forget to include 4934ba2. |
yuwata
added a commit
to yuwata/systemd
that referenced
this pull request
Sep 11, 2020
The issue seems already fixed by PR systemd#16982 and its follow-up commit 4934ba2.
|
This will be included in systemd-246.5. |
peckato1
pushed a commit
to peckato1/systemd
that referenced
this pull request
Oct 1, 2020
The issue seems already fixed by PR systemd#16982 and its follow-up commit 4934ba2. (cherry picked from commit 766f8f3)
ssahani
pushed a commit
to ssahani/systemd
that referenced
this pull request
Oct 5, 2020
The issue seems already fixed by PR systemd#16982 and its follow-up commit 4934ba2.
ssahani
pushed a commit
to ssahani/systemd
that referenced
this pull request
Oct 5, 2020
The issue seems already fixed by PR systemd#16982 and its follow-up commit 4934ba2.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hopefully fixes #14417.
Hopefully fixes #16865.