v2.1.0: usnic cagent: correctly compute the "large" ping message size #2145
Conversation
This file contains hidden or 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
The (effective) "+42" computation was, in fact, the incorrect answer in this case (gasp!). We should just take the max_msg_size from the command (which came from the libfabric endpoint max_msg_size attribute in the client) and subtract off the max header size: 68 (which is explained in the comment). This will result in a "large" message size which is likely slightly smaller than the MTU, but still right up near the MTU, and therefore good enough. Also remove an old/misleading comment. Signed-off-by: Jeff Squyres <[email protected]> (cherry picked from commit dfc72e4)
|
@hppritcha usNIC will not function with Libfabric v1.4 without this commit. |
bturrubiates
approved these changes
Oct 1, 2016
|
@bturrubiates actually approved this, but he's apparently not a committer on this repo. I'll approve the review (since I am a committer) so that Github allows the merge. |
ompiteam
approved these changes
Oct 4, 2016
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.
The (effective) "+42" computation was, in fact, the incorrect answer in this case (gasp!).
We should just take the max_msg_size from the command (which came from the libfabric endpoint max_msg_size attribute in the client) and subtract off the max header size: 68 (which is explained in the comment). This will result in a "large" message size which is likely slightly smaller than the MTU, but still right up near the MTU, and therefore good enough.
Note: the old computation (i.e., -(68-42)) worked fine when we asked for Libfabric API v1.1 because the usnic provider would return a max_msg_size that was already less than the MTU due to FI_PREFIX behavior shenanigans. Once we started asking for Libfabric API v1.4, the usnic Libfabric provider started returning (MTU + prefix_size), and the -(68-42) computation started giving a value that was over the MTU. This caused sendto() on the connectivity checker UDP socket to fail.
This commit also removes an old/misleading comment.
Signed-off-by: Jeff Squyres [email protected]
(cherry picked from commit dfc72e4)
@bturrubiates please review