Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IPv6 Unicast Interface #85604

Open
CDirkx opened this issue May 23, 2021 · 3 comments
Open

IPv6 Unicast Interface #85604

CDirkx opened this issue May 23, 2021 · 3 comments
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@CDirkx
Copy link
Contributor

CDirkx commented May 23, 2021

This issue is split out of the larger discussion around stabilization of the ip feature (tracking issue: #27709). The focus of this issue on the question of what interface Rust should provide for IPv6 unicast adresses.

The current unstable interface is as follows:

impl Ipv6Addr {
    fn is_unicast_global(&self) -> bool;
    fn is_unicast_link_local(&self) -> bool;
    fn is_unicast_link_local_strict(&self) -> bool;
    fn is_unicast_site_local(&self) -> bool;
}

Open Problems

Behaviour of is_unicast_link_local/is_unicast_link_local_strict

Concern was raised about the need for both is_unicast_link_local and is_unicast_link_local_strict(#66584 (comment)). is_unicast_link_local tests if an address is in FE80::/10, is_unicast_link_local_strict in the stricter FE80::/64. For a time it was unclear which interpretation was the correct one, see #76098 (comment) for a more complete overview. However, it was mentioned (#76098 (comment)) that IETF RFC #5156 Section 2.4 defines FE80::/10 as the link-local unicast addresses, and that other programming languages and the linux kernel all use FE80::/10 (#76098 (comment), #76098 (comment)). The conclusion seems to be that the current implementation of is_unicast_link_local is correct and consistent with other implementations, is_unicast_link_local_strict could be removed.

Unresolved: Should is_unicast_link_local_strict be removed?

Deprecation of site-local addresses

Unicast site-local addresses were deprecated in IETF RFC #3879, see also RFC #4291 Section 2.5.7. Any new implementation must no longer support the special behaviour of site-local addresses. This is mentioned in the docs of is_unicast_site_local and already implemented in is_unicast_global, which considers site-local addresses to be global.

For reference; .NET has IsIPv6SiteLocal, Python has is_site_local but mentions that is has been deprecated.

Unresolved: Should is_unicast_site_local be removed? Should SiteLocal be included in a possible IPv6UnicastScope?

Introduce IPv6UnicastScope

It was suggested (#76098 (comment)) to replace the existing unicast interface with an enum IPv6UnicastScope, similar to IPv6MulticastScope:

impl Ipv6Addr {
    fn unicast_scope(&self) -> Option<IPv6UnicastScope>
}

Positive reaction (#76098 (comment), #76098 (comment))

Unresolved: What would be the definition of IPv6UnicastScope?

RFCs

Previous Discussion

@CDirkx
Copy link
Contributor Author

CDirkx commented May 25, 2021

With site-local unicast addresses deprecated, are the only two unicast scopes now link-local and global? If a unicast address is not-link-local, is it always global (and vice versa)?.

@CDirkx
Copy link
Contributor Author

CDirkx commented May 25, 2021

If I read the following table and the rest of RFC 4291 correctly:

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)
  1. Any address is either unicast or multicast
  2. The unspecified and loopback address are unicast (They are defined under section 2.5 Unicast Addresses)
  3. The unspecified and loopback address are not link-local unicast (link-local addresses have prefix FE80::/10), although of the loopback address it is stated that "it is treated as having Link-Local scope".
  4. The unspecified and loopback address are not global unicast, as per the above table.

This leads to a problem if we would want to model unicast addresses similar to multicast addresses:

enum Ipv6UnicastScope {
    LinkLocal,
    Global
}

impl Ipv6Addr {
    fn is_unicast(&self) -> bool;
    fn unicast_scope(&self) -> Option<Ipv6UnicastScope>
}

Ipv6Addr::UNSPECIFIED.is_unicast() would have to be true (2), but what would
Ipv6Addr::UNSPECIFIED.unicast_scope() return? It can not return Some(LinkLocal) (3), and not Some(Global) (4). None?

Edit: I checked and for multicast there are also instances where is_multicast() is true, but multicast_scope() returns None, so that is less of a problem then I thought.

rust/library/std/src/net/ip.rs

Lines 1498 to 1513 in ff2c947

pub const fn multicast_scope(&self) -> Option<Ipv6MulticastScope> {
if self.is_multicast() {
match self.segments()[0] & 0x000f {
1 => Some(Ipv6MulticastScope::InterfaceLocal),
2 => Some(Ipv6MulticastScope::LinkLocal),
3 => Some(Ipv6MulticastScope::RealmLocal),
4 => Some(Ipv6MulticastScope::AdminLocal),
5 => Some(Ipv6MulticastScope::SiteLocal),
8 => Some(Ipv6MulticastScope::OrganizationLocal),
14 => Some(Ipv6MulticastScope::Global),
_ => None,
}
} else {
None
}
}

@CDirkx
Copy link
Contributor Author

CDirkx commented May 25, 2021

My plan forward for the IPv6 unicast interface:

After those steps the interface will look like this:

impl Ipv6Addr {
    fn is_unicast(&self) -> bool;
    fn is_unicast_global(&self) -> bool;
    fn is_unicast_link_local(&self) -> bool;
}

and we can then consider replacing is_unicast_global and is_unicast_link_local with unicast_scope and

#[non_exhaustive]
enum Ipv6UnicastScope {
    LinkLocal,
    Global
}

bors added a commit to rust-lang-ci/rust that referenced this issue May 31, 2021
…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
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jun 9, 2021
Add `Ipv6Addr::is_unicast`

Adds an unstable utility method `Ipv6Addr::is_unicast` under the feature flag `ip` (tracking issue: rust-lang#27709).

Added for completeness with the other unicast methods (see also rust-lang#85604 (comment)) and opposite of `is_multicast`.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jun 16, 2021
…htriplett

Remove `Ipv6Addr::is_unicast_site_local`

Removes the unstable method `Ipv6Addr::is_unicast_site_local`, see also rust-lang#85604 where I have tried to summarize related discussion so far.

Unicast site-local addresses (`fec0::/10`) were deprecated in [IETF RFC rust-lang#3879](https://datatracker.ietf.org/doc/html/rfc3879), see also [RFC rust-lang#4291 Section 2.5.7](https://datatracker.ietf.org/doc/html/rfc4291#section-2.5.7). Any new implementation must no longer support the special behaviour of site-local addresses. This is mentioned in the docs of `is_unicast_site_local` and already implemented in `is_unicast_global`, which considers addresses in `fec0::/10` to have global scope, thus overlapping with `is_unicast_site_local`.

Given that RFC rust-lang#3879 was published in 2004, long before Rust existed, and it is specified that any new implementation must no longer support the special behaviour of site-local addresses, I don't see how a user would ever have a need for `is_unicast_site_local`. It is also confusing that currently both `is_unicast_site_local` and `is_unicast_global` can be `true` for an address, but an address can actually only have a single scope. The deprecating RFC mentions that Site-Local scope was confusing to work with and that the classification of an address as either Link-Local or Global better matches the mental model of users.

There has been earlier discussion of removing `is_unicast_site_local` (rust-lang#60145 (comment)) which decided against it, but that had the incorrect assumption that the method was already stable; it is not. (This confusion arose from the placement of the unstable attribute on the entire module, instead of on individual methods, resolved in rust-lang#85672)

r? `@joshtriplett` as reviewer of all the related PRs
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 16, 2021
…riplett

Remove `Ipv6Addr::is_unicast_site_local`

Removes the unstable method `Ipv6Addr::is_unicast_site_local`, see also rust-lang#85604 where I have tried to summarize related discussion so far.

Unicast site-local addresses (`fec0::/10`) were deprecated in [IETF RFC rust-lang#3879](https://datatracker.ietf.org/doc/html/rfc3879), see also [RFC rust-lang#4291 Section 2.5.7](https://datatracker.ietf.org/doc/html/rfc4291#section-2.5.7). Any new implementation must no longer support the special behaviour of site-local addresses. This is mentioned in the docs of `is_unicast_site_local` and already implemented in `is_unicast_global`, which considers addresses in `fec0::/10` to have global scope, thus overlapping with `is_unicast_site_local`.

Given that RFC rust-lang#3879 was published in 2004, long before Rust existed, and it is specified that any new implementation must no longer support the special behaviour of site-local addresses, I don't see how a user would ever have a need for `is_unicast_site_local`. It is also confusing that currently both `is_unicast_site_local` and `is_unicast_global` can be `true` for an address, but an address can actually only have a single scope. The deprecating RFC mentions that Site-Local scope was confusing to work with and that the classification of an address as either Link-Local or Global better matches the mental model of users.

There has been earlier discussion of removing `is_unicast_site_local` (rust-lang#60145 (comment)) which decided against it, but that had the incorrect assumption that the method was already stable; it is not. (This confusion arose from the placement of the unstable attribute on the entire module, instead of on individual methods, resolved in rust-lang#85672)

r? `@joshtriplett` as reviewer of all the related PRs
@Dylan-DPC Dylan-DPC added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants