Remove Ipv6Addr::is_unicast_link_local_strict#85819
Merged
bors merged 1 commit intorust-lang:masterfrom May 31, 2021
Merged
Conversation
b466b2d to
c1f0c15
Compare
This was referenced May 29, 2021
Member
|
@bors r+ |
Collaborator
|
📌 Commit c1f0c15 has been approved by |
Collaborator
Collaborator
|
☀️ Test successful - checks-actions |
klensy
reviewed
May 31, 2021
Comment on lines
482
to
483
| let unicast_link_local: u16 = 1 << 4; | ||
| let unicast_link_local_strict: u16 = 1 << 5; | ||
| let unicast_site_local: u16 = 1 << 6; |
Contributor
Author
There was a problem hiding this comment.
It is a bitset of which properties are true, one of the bits being unused shouldn't really matter. I think that is less invasive than renumbering all of them.
29 tasks
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.
Removes the unstable method
Ipv6Addr::is_unicast_link_local_strictand keeps the behaviour ofIpv6Addr::is_unicast_link_local, see also #85604 where I have tried to summarize related discussion so far.My intent is for
is_unicast_link_local,is_unicast_site_localandis_unicast_globalto have the semantics of checking if an address has Link-Local, Site-Local or Global scope, see also #85696 which changes the behaviour ofis_unicast_globaland renames these methods tohas_unicast_XXX_scopeto reflect this.For checking Link-Local scope we currently have two methods:
is_unicast_link_localandis_unicast_link_local_strict. This is because of what appears to be conflicting definitions in IETF RFC 4291.From IETF RFC 4291 section 2.4: "Link-Local unicast" (
FE80::/10)From IETF RFC 4291 section 2.5.6: "Link-Local IPv6 Unicast Addresses" (
FE80::/64)With
is_unicast_link_localcheckingFE80::/10andis_unicast_link_local_strictcheckingFE80::/64.There is also IETF RFC 5156 section 2.4 which defines "Link-Scoped Unicast" as
FE80::/10.It has been pointed out that implementations in other languages and the linux kernel all use
FE80::/10(#76098 (comment), #76098 (comment)).Given all of this I believe the correct interpretation to be the following: All addresses in
FE80::/10are defined as having Link-Local scope, however currently only the blockFE80::/64has been allocated for "Link-Local IPv6 Unicast Addresses". This might change in the future however; more addresses inFE80::/10could be allocated and those will have Link-Local scope. I therefore believe the current behaviour ofis_unicast_link_localto be correct (if interpreting it to have the semantics ofhas_unicast_link_local_scope) andis_unicast_link_local_strictto be unnecessary, confusing and even a potential source of future bugs:Currently there is no real difference in checking
FE80::/10orFE80::/64, since any address in practice will beFE80::/64. However if an application usesis_unicast_link_local_strictto implement link-local (so non-global) behaviour, it will be incorrect in the future if addresses outside ofFE80::/64are allocated.r? @joshtriplett as reviewer of all the related PRs