Fix `Ipv6Addr::is_unicast_global` to check for unicast global scope #85696

CDirkx · 2021-05-25T20:24:11Z

See #85604 for previous discussion.

The current behaviour of Ipv6Addr::is_unicast_global is

/// Returns [`true`] if the address is a globally routable unicast address.

This is inconsistent with methods like is_unicast_link_local and is_unicast_site_local, which check if an address has unicast link-local scope or site-local scope respectively. Similarly one would expect is_unicast_global to check if an address has unicast global scope (which is not the same thing as being globally routable, for that we already have Ipv6Addr::is_global which is reworked in #86634).

This PR changes the behaviour to

/// Returns `true` if the address is a unicast address with global scope, as defined in [RFC 4291].
///
/// Any unicast address has global scope if it is not:
///  - the [unspecified address] (`::`)
///  - the [loopback address] (`::1`)
///  - a [link-local address] (`fe80::/10`)

Basing itself on the following table from RFC 4291:

Address type         Binary prefix        IPv6 notation   Section
------------         -------------        -------------   -------
Unspecified          00...0  (128 bits)   ::/128          2.5.2
Loopback             00...1  (128 bits)   ::1/128         2.5.3
Multicast            11111111             FF00::/8        2.7
Link-Local unicast   1111111010           FE80::/10       2.5.6
Global Unicast       (everything else)

The documentation is also updated to reflect that future RFCs may change the behaviour of this method: "Future specifications may redefine one or more sub-ranges of the Global Unicast space for other purposes".

The behaviour of is_global is kept the same, it used to delegate to is_unicast_global but now performs the checks for global reachability itself.

To avoid further confusion between is_unicast_global and is_global, and the concepts of "global scope" and "globally reachable", the folowing methods have been renamed:

is_unicast_global -> has_unicast_global_scope
is_unicast_link_local -> has_unicast_link_local_scope
~~is_unicast_link_local_strict -> has_unicast_link_local_scope_strict~~ (removed in Remove Ipv6Addr::is_unicast_link_local_strict #85819)
~~is_unicast_site_local -> has_unicast_site_local_scope~~ (removed in Remove Ipv6Addr::is_unicast_site_local #85820)

rust-highfive · 2021-05-25T20:24:15Z

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

the8472 · 2021-05-25T21:06:37Z

What does that mean for unique local addresses now? Those clearly are not global and RFC 4291 doesn't even acknowledge their existence even though RFC 4193 predates it by a year.

Edit: Nevermind, I see is_global handles that now.

CDirkx · 2021-05-25T21:20:55Z

Yes that is where the difference between "global scope" and "globally reachable" comes into play. RFC 4193 has the following:

In practice, applications may treat these addresses like global scoped addresses.

Application and other higher level protocols can treat [Unique Local] IPv6 addresses in the same manner as other types of global unicast addresses. No special handling is required. This type of address may not be reachable, but that is no different from other types of IPv6 global unicast address.

So unique local addresses are indeed not globally reachable (which is implemented in is_global) but do have global scope.

CDirkx · 2021-05-25T21:29:01Z

Perhaps it would be less confusing to rename is_unicast_global (and the link-local and site-local versions) to has_unicast_global_scope etc. in order to distinguish it from is_global. Or alternatively rename is_global to is_globally_reachable.

I have already included in the documentation of is_unicast_global:

/// Note that an address that has global scope may still not be globally reachable.
/// If you want to check if an address appears to be globally reachable, use [`is_global`].

…r=joshtriplett Remove `Ipv6Addr::is_unicast_link_local_strict` Removes the unstable method `Ipv6Addr::is_unicast_link_local_strict` and keeps the behaviour of `Ipv6Addr::is_unicast_link_local`, see also rust-lang#85604 where I have tried to summarize related discussion so far. My intent is for `is_unicast_link_local`, `is_unicast_site_local` and `is_unicast_global` to have the semantics of checking if an address has Link-Local, Site-Local or Global scope, see also rust-lang#85696 which changes the behaviour of `is_unicast_global` and renames these methods to `has_unicast_XXX_scope` to reflect this. For checking Link-Local scope we currently have two methods: `is_unicast_link_local` and `is_unicast_link_local_strict`. This is because of what appears to be conflicting definitions in [IETF RFC 4291](https://datatracker.ietf.org/doc/html/rfc4291). From [IETF RFC 4291 section 2.4](https://datatracker.ietf.org/doc/html/rfc4291#section-2.4): "Link-Local unicast" (`FE80::/10`) ```text Address type Binary prefix IPv6 notation Section ------------ ------------- ------------- ------- Unspecified 00...0 (128 bits) ::/128 2.5.2 Loopback 00...1 (128 bits) ::1/128 2.5.3 Multicast 11111111 FF00::/8 2.7 Link-Local unicast 1111111010 FE80::/10 2.5.6 Global Unicast (everything else) ``` From [IETF RFC 4291 section 2.5.6](https://datatracker.ietf.org/doc/html/rfc4291#section-2.5.6): "Link-Local IPv6 Unicast Addresses" (`FE80::/64`) ```text | 10 bits | 54 bits | 64 bits | +----------+-------------------------+----------------------------+ |1111111010| 0 | interface ID | +----------+-------------------------+----------------------------+ ``` With `is_unicast_link_local` checking `FE80::/10` and `is_unicast_link_local_strict` checking `FE80::/64`. There is also [IETF RFC 5156 section 2.4](https://datatracker.ietf.org/doc/html/rfc5156#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` (rust-lang#76098 (comment), rust-lang#76098 (comment)). Given all of this I believe the correct interpretation to be the following: All addresses in `FE80::/10` are defined as having Link-Local scope, however currently only the block `FE80::/64` has been allocated for "Link-Local IPv6 Unicast Addresses". This might change in the future however; more addresses in `FE80::/10` could be allocated and those will have Link-Local scope. I therefore believe the current behaviour of `is_unicast_link_local` to be correct (if interpreting it to have the semantics of `has_unicast_link_local_scope`) and `is_unicast_link_local_strict` to be unnecessary, confusing and even a potential source of future bugs: Currently there is no real difference in checking `FE80::/10` or `FE80::/64`, since any address in practice will be `FE80::/64`. However if an application uses `is_unicast_link_local_strict` to implement link-local (so non-global) behaviour, it will be incorrect in the future if addresses outside of `FE80::/64` are allocated. r? `@joshtriplett` as reviewer of all the related PRs

bors · 2021-05-31T08:04:45Z

☔ The latest upstream changes (presumably #85819) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2021-06-09T06:14:47Z

☔ The latest upstream changes (presumably #86160) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2021-06-16T04:33:12Z

☔ The latest upstream changes (presumably #85820) made this pull request unmergeable. Please resolve the merge conflicts.

…st global scope

…_unicast_global_scope` are together

CDirkx · 2021-06-16T04:59:37Z

I rebased and reordered the methods so now has_unicast_global_scope and has_unicast_link_local_scope are together (might look weird in the diff).

CDirkx · 2021-07-08T12:42:25Z

library/std/src/net/ip.rs

-            None => self.is_unicast_global(),
+            None => {
+                self.has_unicast_global_scope()
+                    && !(self.is_unique_local() || self.is_documentation())
+            }


This implementation of is_global is changed completely in #86634

crlf0710 · 2021-07-24T08:09:43Z

Block on #86634

bors · 2021-10-04T10:19:18Z

☔ The latest upstream changes (presumably #89512) made this pull request unmergeable. Please resolve the merge conflicts.

chotchki · 2022-07-30T13:59:55Z

@CDirkx I'm trying to figure out where the "ip" feature left off and I think its on this rebase need. Am I correct?

Mark-Simulacrum · 2022-08-21T15:13:57Z

Approved #99947, so going to go ahead and close this PR. #99947 contains the commits from here though.

rust-highfive assigned joshtriplett May 25, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 25, 2021

CDirkx mentioned this pull request May 25, 2021

IPv6 Unicast Interface #85604

Open

CDirkx mentioned this pull request May 29, 2021

Remove Ipv6Addr::is_unicast_link_local_strict #85819

Merged

CDirkx changed the title ~~Fix is_unicast_global to check for unicast global scope~~ Fix Ipv6Addr::is_unicast_global to check for unicast global scope May 29, 2021

CDirkx force-pushed the unicast_global branch from dc74b0b to ba8d112 Compare May 31, 2021 08:36

CDirkx force-pushed the unicast_global branch from ba8d112 to 9e15081 Compare June 9, 2021 08:02

CDirkx added 3 commits June 16, 2021 06:43

Change Ipv6Addr::is_unicast_global to check if an address has unica…

e7fc4c8

…st global scope

Rename is_unicast_global etc. to has_unicast_global_scope

fda30e4

Reorder Ipv6Addr methods so has_unicast_link_local_scope and `has…

85931ea

…_unicast_global_scope` are together

CDirkx force-pushed the unicast_global branch from 9e15081 to 85931ea Compare June 16, 2021 04:58

This comment has been minimized.

Sign in to view

rustbot added A-io Area: std::io, std::fs, std::net and std::path T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jun 23, 2021

This was referenced Jun 25, 2021

Rework Ipv6Addr::is_global to check for global reachability rather than global scope #86634

Closed

IP Utility Methods #85612

Open

CDirkx commented Jul 8, 2021

View reviewed changes

crlf0710 added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 24, 2021

chotchki mentioned this pull request Jul 30, 2022

Fix Ipv6Addr::is_unicast_global to check for unicast global scope rebase #99947

Merged

Mark-Simulacrum closed this Aug 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `Ipv6Addr::is_unicast_global` to check for unicast global scope #85696

Fix `Ipv6Addr::is_unicast_global` to check for unicast global scope #85696

CDirkx commented May 25, 2021 •

edited

Loading

rust-highfive commented May 25, 2021

the8472 commented May 25, 2021 •

edited

Loading

CDirkx commented May 25, 2021

CDirkx commented May 25, 2021 •

edited

Loading

bors commented May 31, 2021

bors commented Jun 9, 2021

bors commented Jun 16, 2021

CDirkx commented Jun 16, 2021 •

edited

Loading

This comment has been minimized.

CDirkx Jul 8, 2021

crlf0710 commented Jul 24, 2021

bors commented Oct 4, 2021

chotchki commented Jul 30, 2022

Mark-Simulacrum commented Aug 21, 2022

Fix Ipv6Addr::is_unicast_global to check for unicast global scope #85696

Fix Ipv6Addr::is_unicast_global to check for unicast global scope #85696

Conversation

CDirkx commented May 25, 2021 • edited Loading

rust-highfive commented May 25, 2021

the8472 commented May 25, 2021 • edited Loading

CDirkx commented May 25, 2021

CDirkx commented May 25, 2021 • edited Loading

bors commented May 31, 2021

bors commented Jun 9, 2021

bors commented Jun 16, 2021

CDirkx commented Jun 16, 2021 • edited Loading

This comment has been minimized.

CDirkx Jul 8, 2021

Choose a reason for hiding this comment

crlf0710 commented Jul 24, 2021

bors commented Oct 4, 2021

chotchki commented Jul 30, 2022

Mark-Simulacrum commented Aug 21, 2022

Fix `Ipv6Addr::is_unicast_global` to check for unicast global scope #85696

Fix `Ipv6Addr::is_unicast_global` to check for unicast global scope #85696

CDirkx commented May 25, 2021 •

edited

Loading

the8472 commented May 25, 2021 •

edited

Loading

CDirkx commented May 25, 2021 •

edited

Loading

CDirkx commented Jun 16, 2021 •

edited

Loading