Conversation
Both backends set and read IPV6_MULTICAST_HOPS at IPPROTO_IP. It is an IPv6 option, so the call fails with EINVAL and the multicast hop limit of an IPv6 socket cannot be set at all. This is the surviving half of the copy+pasto that bytecodealliance#639 fixed in April 2023. That commit corrected the option constant and left the protocol level. bytecodealliance#855 later moved the code to its present file, which hides the origin from git blame. The test did not catch it because it accepted Err(INVAL), which is the exact error a wrong protocol level returns. It also read the option on a stream socket, where the value is meaningless, so the Ok arm never ran. Read the option on a datagram socket instead, and add a round trip for the setter. The round trip uses 200 because no platform defaults to it, so it cannot pass on an untouched value. The Ok arm now runs, and the default is 1, not 0. The deleted comment read that 1 as a NetBSD quirk. NetBSD was the only system reporting the true value; elsewhere the call errored into the INVAL arm. The default of 1 is measured on Linux and macOS. NetBSD is untested here.
Author
|
This superseded https://github.com/bytecodealliance/rustix/pull/1662/changes which still contains bugs in the tests. It still tries to set hops on a stream which will silently run into the cases. |
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
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.
Both backends set and read
IPV6_MULTICAST_HOPSatIPPROTO_IP. It is an IPv6 option, so the call fails withEINVALand the multicast hop limit of an IPv6 socket cannot be set at all.This is the surviving half of #639 fixed in April 2023. That commit corrected the option constant and left the protocol level. #855 later moved the code to its present file, which hides the origin from git blame.
The test did not catch it because it accepted
Err(INVAL), which is the exact error a wrong protocol level returns. It also read the option on a stream socket, where the value is meaningless, so the Ok arm never ran.Read the option on a datagram socket instead, and add a round trip for the setter. The round trip uses 200 because no platform defaults to it, so it cannot pass on an untouched value.
The
Okarm now runs, and the default is 1, not 0. The deleted comment read that 1 as a NetBSD quirk. NetBSD was the only system reporting the true value; elsewhere the call errored into the INVAL arm. The default of 1 is measured on Linux and macOS. NetBSD is untested here.I am happy to bring back the NetBSD quirk cfg flag if CI says otherwise. I do not have a local NetBSD.