Increase GRO segments to 64 #1354
Merged
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
|
Good catch! Could you include this rationale in the comment so it's not lost? |
Done! |
The GRO segment size (what we call stride), is set dynamically depending on the first datagram that starts a GRO list [0]. Unless the receive buffer passed to recvmmsg() is an exact multiple of the segment size, segments could be truncated since recvmmsg() will happily fill the whole buffer it's given regardless of whether it's aligned to segment boundaries or not. Bump the value returned by gro::gro_segments() to the maximum allowed by the kernel so that get_max_udp_payload_size() * gro::gro_segments() will always be large enough to hold the largest GRO list the kernel might produce, so that recvmmsg() will never truncate. [0]: https://github.com/torvalds/linux/blob/1e8a3f0d2a1ef544611a7ea4a7c1512c732e0e43/net/core/gro.c#L543
Ralith
approved these changes
May 10, 2022
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.
Welp, this was a bit of a head scratcher.
While running
perf_serverandperf_clientfrom two different hosts (and so not on loopback), I noticed that occasionally I was getting the following debug logs:It turns out that these logs are due to truncated GRO segments.
After some kernel spelunking, I noticed this: https://github.com/torvalds/linux/blob/1e8a3f0d2a1ef544611a7ea4a7c1512c732e0e43/net/core/gro.c#L543. The GRO segment size (what we call
stride), is set dynamically depending on the first datagram that starts a GRO list. Unless the receive buffer passed torecvmmsg()is an exact multiple of the segment size, segments can be truncated sincerecvmmsg()will happily fill the whole buffer it's given regardless of whether it's aligned to segment boundaries or not (this is a wart of the API, since in some cases it is ok for the last segment to be shorter than all the others).The fix is to bump the value we return from
gro::gro_segments()to the maximum allowed by the kernel (64, which incidentally is also the value we set for GSO). This wayget_max_udp_payload_size() * gro::gro_segments()will always be strictly larger than the GRO lists the kernel produces, sorecvmmsg()won't truncate.