Rust: Tracking issue for Ipv{4,6}Addr convenience methods

Created on 12 Aug 2015  Â·  73Comments  Â·  Source: rust-lang/rust

The below is a list of methods left to be stabilized under the ip feature. The path forward today is unclear; if you'd like to push through any method on here the libs team is interested in a PR with links to the associated RFCs or other official documentation. Let us know!

  • [ ] [Ip::is_documentation](https://doc.rust-lang.org/nightly/std/net/enum.IpAddr.html#method.is_documentation)
  • [ ] [Ipv6::is_documentation](https://doc.rust-lang.org/nightly/std/net/struct.Ipv6Addr.html#method.is_documentation)

  • [ ] [Ip::is_global](https://doc.rust-lang.org/nightly/std/net/enum.IpAddr.html#method.is_global)
  • [ ] [Ipv4::is_global](https://doc.rust-lang.org/nightly/std/net/struct.Ipv4Addr.html#method.is_global)
  • [ ] [Ipv6::is_global](https://doc.rust-lang.org/nightly/std/net/struct.Ipv6Addr.html#method.is_global)

  • [ ] [Ipv4::is_shared](https://doc.rust-lang.org/nightly/std/net/struct.Ipv4Addr.html#method.is_shared)
  • [ ] [Ipv4::is_ietf_protocol_assignment](https://doc.rust-lang.org/nightly/std/net/struct.Ipv4Addr.html#method.is_ietf_protocol_assignment)
  • [ ] [Ipv4::is_benchmarking](https://doc.rust-lang.org/nightly/std/net/struct.Ipv4Addr.html#method.is_benchmarking)

    • [ ] [Ipv4::is_reserved](https://doc.rust-lang.org/nightly/std/net/struct.Ipv4Addr.html#method.is_reserved)


  • [ ] [Ipv6::is_unicast_global](https://doc.rust-lang.org/nightly/std/net/struct.Ipv6Addr.html#method.is_unicast_global)
  • [ ] [Ipv6::is_unicast_link_local](https://doc.rust-lang.org/nightly/std/net/struct.Ipv6Addr.html#method.is_unicast_link_local)
  • [ ] [Ipv6::is_unicast_link_local_strict](https://doc.rust-lang.org/nightly/std/net/struct.Ipv6Addr.html#method.is_unicast_link_local_strict)
  • [ ] [Ipv6::is_unicast_site_local](https://doc.rust-lang.org/nightly/std/net/struct.Ipv6Addr.html#method.is_unicast_site_local)
  • [ ] [Ipv6::is_unique_local](https://doc.rust-lang.org/nightly/std/net/struct.Ipv6Addr.html#method.is_unique_local)
  • [ ] [Ipv6::multicast_scope](https://doc.rust-lang.org/nightly/std/net/struct.Ipv6Addr.html#method.multicast_scope)

  • [ ] [Ipv6Addr::to_ipv4_mapped](https://doc.rust-lang.org/nightly/std/net/struct.Ipv6Addr.html#method.to_ipv4_mapped)
A-io B-unstable C-tracking-issue Libs-Tracked T-libs

Most helpful comment

Not much has happened since https://github.com/rust-lang/rust/pull/60145. Should we go ahead an stabilize the Ipv4Addr and Ipv6Addr that are behind the ip feature?

All 73 comments

I think we should consider this for stabilization in 1.6. Nominating.

:bell: This issue is now entering its cycle-long final comment period for stabilization :bell:

Concretely, we discussed this in the libs meeting and the conclusion was that the boolean accessors are likely ready for stabilization after verifying that they're all the canonical definitions, but the enum-returning variants will likely remain unstable for now.

What, exactly, is the "ip feature"? Could you link to the RustDoc(s) of the specific things that are supposed to be reviewed?

I think "ip feature" in this context refers to things annotated with #![unstable(feature = "ip", …)], which require #![feature(ip)] to be used.

I think "ip feature" in this context refers to things annotated with #![unstable(feature = "ip", …)], which require #![feature(ip)] to be used.

I don't mean to be rude, but that's just a restating my question as the answer. if you want people to actually give feedback on the proposal, it should be easier to understand what the proposal is. In this case, it is pretty difficult to tell what is being proposed because the module mixes stable and unstable features.

I noticed that a large part of this module could work in #![no_std] mode. I suggest moving the #![no_std]-compatible parts to core::net, or at least consider how making it work with #![no_std] would affect the API.

It also seems odd that Ipv4Addr and Ipv6Addr have things like is_multicast and is_global but there's no trait that allows code to make these queries generically over those types of addresses. If such a trait were to bad added later, would the existence of these non-trait methods cause problems? If so, it might be worth considering building the trait first.

@briansmith I don't think that's being rude, there's just tension here between "we've been doing this a while so we're a bit short" and "newer people might not know what that is." @SimonSapin leaned a bit towards a literal explanation, but you're right to point out that more detail is good.

I read @alexcrichton 's comment as:

that the boolean accessors are likely ready for stabilization after verifying that they're all the canonical definitions,

http://doc.rust-lang.org/std/net/struct.Ipv4Addr.html and http://doc.rust-lang.org/std/net/struct.Ipv6Addr.html <- all the stuff here that is -> bool

but the enum-returning variants will likely remain unstable for now.

http://doc.rust-lang.org/std/net/struct.Ipv6Addr.html#method.multicast_scope is the only one I see that's unstable.

FWIW, I filed https://github.com/rust-lang/rust/issues/29221 a while ago, which would make tracking down what these tracking issues refer to slightly easier.

Yes, to be concrete, I was proposing stabilizing:

  • Ipv4Addr::is_unspecified
  • Ipv4Addr::is_loopback
  • Ipv4Addr::is_private
  • Ipv4Addr::is_link_local
  • Ipv4Addr::is_global
  • Ipv4Addr::is_multicast
  • Ipv4Addr::is_broadcast
  • Ipv4Addr::is_documentation
  • Ipv6Addr::is_unspecified
  • Ipv6Addr::is_loopback
  • Ipv6Addr::is_global
  • Ipv6Addr::is_unique_local
  • Ipv6Addr::is_unicast_link_local
  • Ipv6Addr::is_unicast_site_local
  • Ipv6Addr::is_unicast_global
  • Ipv6Addr::is_multicast

Note that this is all pending actually verifying that these are standard properties of the respective IP address space and are well known with canonical implementations. I believe they fit this requirement already but would like to double-check.


@briansmith

I suggest moving the #![no_std]-compatible parts to core::net

Yeah these things can certainly move around over time (it's backwards compatible to move them at a later date). I'd be a little wary of putting things in core "just because" without a concrete purpose, and these kinda fall into the category I'd be wary of. For example the internal representation of each of these primitives is the libc equivalent (e.g. libc::sockaddr_in6 or libc::in_addr) which unfortunately isn't available in libcore, so if we move it to core we'd have to invent our own storage format.

It also seems odd that Ipv4Addr and Ipv6Addr have things like is_multicast and is_global but there's no trait that allows code to make these queries generically over those types of addresses. If such a trait were to bad added later, would the existence of these non-trait methods cause problems?

Method resolution favors inherent methods (methods defined on the type itself) over trait methods (e.g. impl'd traits plus the trait being in scope), so in that sense we're covered to add a trait at a future date. That being said the standard library doesn't have too many traits like this for abstracting between one or two types, so I would personally want to hold off on this extension for now.

A possible alternative, however, could be adding the common set of methods to IpAddr if we end up stabilizing that as well.

A couple of issues I have noticed with what we have:

  • 0.0.0.0/8, :: and many more ranges shouldn't return true for is_global()
  • Ipv6Addr::is_documentation is missing and should be 2001:db8::/32

On a more general note, might some of these functions need to be updated in the future if new ranges are assigned? How would that be handled?

Ah I unfortunately did not have time to do an audit of these APIs this cycle, so when the libs team talked about this during triage today the conclusion was to punt this to next cycle, I hope to have the time to investigate it then and incorportate @ollie27's suggestions!

:bell: This issue is now yet again entering its final comment period :bell:

Hopefully I get a chance to researching this API this time around!

Better late than never! -- my analysis:

Of the ipv4 properties, there's more listed on wikipedia at least, for example:

  • Current network
  • Shared
  • protocol assignments (and DS-Lite)
  • ipv6 to ipv4 relay
  • benchmark tests
  • reserved

These sound relatively obscure (at least to me) though, so it seems fine that we don't add them just yet. In terms of what we currently have:

  • [x] Ipv4Addr::is_unspecified - appears to be an ipv6 property, not ipv4?
  • [x] Ipv4Addr::is_loopback - ref
  • [x] Ipv4Addr::is_private - ref1, ref2, ref3
  • [x] Ipv4Addr::is_link_local - ref
  • [x] Ipv4Addr::is_global - _not found_, this is a property, but haven't found exhaustive documentation
  • [x] Ipv4Addr::is_multicast - ref
  • [x] Ipv4Addr::is_broadcast - ref
  • [x] Ipv4Addr::is_documentation - ref1, ref2, ref3

Like with ipv4 we're missing some ipv6 properties (according to RFC 6890):

  • ipv4-ipv6 transit
  • ipv4-mapped (although we have a method to access this, so probably ok)
  • discard only
  • protocol assignments
  • TEREDO (wut?)
  • benchmarking
  • documentation
  • ORCHID
  • 6to4

Of the ipv6 methods:

  • [x] Ipv6Addr::is_unspecified - ref
  • [x] Ipv6Addr::is_loopback - ref
  • [x] Ipv6Addr::is_global - couldn't find a reference online quickly, seems to have some interesting logic though?
  • [x] Ipv6Addr::is_unique_local - ref
  • [x] Ipv6Addr::is_unicast_link_local - ref, called "Linked-Scope Unicast" in the RFC at least, not sure about the name in that case.
  • [x] Ipv6Addr::is_unicast_site_local - didn't find a reference in this RFC at least
  • [x] Ipv6Addr::is_unicast_global - same as above, didn't find a reference (doesn't mean it's wrong though!)
  • [x] Ipv6Addr::is_multicast - ref

From this I'm comfortable stabilizing the checked methods (they've got verified names and implementations at least), but I would personally want some more verification of the unchecked methods before stabilizing.

From this I'm comfortable stabilizing the checked methods (they've got verified names and implementations at least), but I would personally want some more verification of the unchecked methods before stabilizing.

Sounds like a good start. The is_loopback would be already useful for me. Having to use only one standard type to represent an ip, no matter whether v4 or v6, would be very useful already.

The libs team discussed this during triage yesterday and the decision was to stabilize the methods I've checked above

Removing from FCP as the initial round of stabilization has happened.

I know I'm late to the party on this, but I just got bitten by this issue when trying to implement a generic IpAddr is_unspecified method for my own use.

The IPv4 unspecified address (which is not checked above at the time of this writing) definitely exists and is standard. It is defined on p891 of Stevens, Volume 1, Second Edition, which notes that it is all zeroes and is also referred to as INADDR_ANY. (There are numerous other references to it, but this is the most compact.) If you want an online reference, http://man7.org/linux/man-pages/man7/ip.7.html also confirms this.

@therealbstern oh interesting! I think we're fine stabilizing if it's actually a property documented somewhere, I just couldn't find it at the time. Want to send a PR updating the docs accordingly? We may be able to consider it for stabilization next cycle.

I think PR #34739 does what you want, though I wasn't 100% sure what to put into the stable block. I defer future commentary to that PR.

:bell: This issue is now entering a cycle-long final comment period 🔔

Specifically, the libs team is considering stabilizing Ipv4Addr::is_unspecified as proposed in https://github.com/rust-lang/rust/issues/34739. The other methods on these types will remain unstable.

IPv6 site local is also in Stevens, p895. I'll look for an online reference. I take it that IPv4Addr.is_global will stay unstable, despite the (admittedly inelegant) discussion at the iana-ipv4-special-registry? [That link is already documented.]

I apologize; IPv6 site local is deprecated by RFC 3879, so I don't think stabilizing it is as good an idea.

@therealbstern probably, yeah. Most of the instability here is just because of a lack of motivation to push them towards stable, but if they're well accepted concepts elsewhere that can be documented, we'd be fine stabilizing!

The libs team has also decided to throw the methods introduced in https://github.com/rust-lang/rust/pull/34694 into FCP, which is just all of these methods that are shared between v4 and v6 addrs also being exported from IpAddr

Discussed recently, the libs team decided to stabilize the shared methods and the is_unspecified method on Ipv4Addr

@rfcbot fcp close

I feel like the remaining methods have languished long enough in the standard library. Anything that's not stable I'd think we should just deprecate/remove at this point.

Team member @alexcrichton has proposed to close this. The next step is review by the rest of the tagged teams:

  • [ ] @BurntSushi
  • [ ] @Kimundi
  • [x] @alexcrichton
  • [ ] @aturon
  • [ ] @brson
  • [ ] @sfackler

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

Seems like perhaps is_documentation should be stabilized, everything else deprecated.

@brson I was able to find documentation for Ipv4Addr::is_documentation but couldn't find documentation on the Ipv6Addr. That being said, briefly taking a look at the ipv4 docs points me to https://tools.ietf.org/html/rfc3849 which has documentation for ipv6. In that sense I'd be fine considering stabilization of that and deprecation of everything else.

Some more docs:
Ipv4Addr::is_global https://tools.ietf.org/html/rfc5735#page-3
Ipv6Addr::is_global https://tools.ietf.org/html/rfc4291#section-2.5.4
Ipv6Addr::is_unicast_link_local https://tools.ietf.org/html/rfc4291#section-2.5.6
Ipv6Addr::is_unicast_site_local https://tools.ietf.org/html/rfc4291#section-2.5.7
Ipv6Addr::is_unicast_global https://tools.ietf.org/html/rfc4291#section-2.5.4
Ipv6Addr::multicast_scope and Ipv6MulticastScope https://tools.ietf.org/html/rfc4291#section-2.7

@rfcbot fcp cancel

ok, seems like we shouldn't deprecate!

Can we fcp to stabilize then (assuming that the implementations correspond to what they should be)?

@alexcrichton proposal cancelled.

@sfackler presumably, yeah, but I don't really have the time right now to audit everything much less deal with Ipv6MulticastScope, so I'll leave that to someone else to propose FCP :)

@alexcrichton let me know if I can help with anything here

@achanda sure yeah the next step for stabilizing this would be to audit all implementations to ensure they match precisely what the RFCs mention, and then ensure all document references the relevant RFC as well. We may want to leave multicast_scope out for now as it has an enum, but it'd be good to read up on that and ensure it matches the RFC.

ping @achanda, any updates? I just came across a use for is_global and possibly is_link_local, would be nice to get them in stable.

@kamalmarhubi sorry, I forgot about this one. Let me get back to this as soon as I can (should be a few days).

@achanda no worries!

Any update on this?

I'd also love to have is_global stabilized.

I have updated the original issue description to include a list of the methods currently unstable under the ip feature. If anyone is interested in pushing any given method to stability, I believe a PR with associated documentation (as in the description above) would be welcomed.

The implementation of Ipv4::is_global is not complete, according to the IANA IPv4 Special-Purpose Address Registry.

  • It compares the address to 0.0.0.0, but anything in 0.0.0.0/8 should not be considered global.

    • 0/8 is not global and is currently forbidden because some systems used to treat it as the local network.

    • The implementation of Ipv4::is_unspecified is correct. 0.0.0.0 is the unspecified address.

  • It does not examine 100.64.0.0/10, which is "Shared Address Space" and not global.
  • Ditto 192.0.0.0/24 (IETF Protocol Assignments), except for 192.0.0.9/32 and 192.0.0.10/32, which are carved out as globally reachable.
  • 198.18.0.0/15 is for "Benchmarking" and should not be globally reachable.
  • 240.0.0.0/4 is reserved and not currently reachable

There are also no methods that identify the netblocks identified above.

I may be wrong but Ipv6::is_unicast_link_local() seems wrong to me.
The current implementation is:

pub fn is_unicast_link_local(&self) -> bool {
    (self.segments()[0] & 0xffc0) == 0xfe80
}

But the RFC says:

   Link-Local addresses are for use on a single link.  Link-Local
   addresses have the following format:

   |   10     |
   |  bits    |         54 bits         |          64 bits           |
   +----------+-------------------------+----------------------------+
   |1111111010|           0             |       interface ID         |
   +----------+-------------------------+----------------------------+

   Link-Local addresses are designed to be used for addressing on a
   single link for purposes such as automatic address configuration,
   neighbor discovery, or when no routers are present.

   Routers must not forward any packets with Link-Local source or
   destination addresses to other links.

So I think this should be:

pub fn is_unicast_link_local(&self) -> bool {
    (self.segments()[0] & 0xffff) == 0xfe80
        && (self.segments()[1] & 0xffff) == 0
        && (self.segments()[2] & 0xffff) == 0
        && (self.segments()[3] & 0xffff) == 0
}

If you agree I'll open a PR.

Also, I think it would be worth having Ipv6::is_ipv4_compatible and Ipv6::is_ipv4_mapped(). I can also open a PR for that.

@Mark-Simulacrum

I have updated the original issue description to include a list of the methods currently unstable under the ip feature. If anyone is interested in pushing any given method to stability, I believe a PR with associated documentation (as in the description above) would be welcomed.

I'd like to work on that. I'll try to send a first PR tonight.

@little-dude, The errata for RFC 4291, and more specifically erratum 4406 make this much less clear than one might think. /10 is reserved for link-local and similar uses, though only /64 is defined right now, and they don't name what the rest of it for. RFC 7371 also takes the opportunity to mention that the link-local /10 exists and then appears to discuss multicast without regard to whether or not it's a new chunk of the /10 or not.

Anyway, the point is, although I agreed with your suggestion so much that I initially thought I had written it, I'm no longer so sure how to stand on this.

I see, it's debatable indeed. To sum up:

  • fe80::/10 is reserved
  • fe80::/64 is the currenttly defined link local unicast

I have slight preference for validating the second range since that as of today, the only valid unicast link local addresses are in fe80::/64.

Otherwise, why not having two methods with a detailed documentation of how they differ: is_link_local() for fe80/10 and is_link_local_strict() for fe80/64?

@therealbstern I've started working on fixing Ipv4Addr::is_global as per your comment.

Obviously, I'm not in charge of the API here, but I personally think your validation plan is the way to fly. Also, the looser method would help more with the "is this globally routable" question.

Did you understand what RFC 7371 was saying? I thought it meant that some of the /10 link-local space was going to become multicast, but I might have completely misunderstood. :-P

Why is this still unstable?

This PR has not been finished. You can pick it up if you want, or I can try finishing it but I won't be able to this week.

No, take your time. :) I'm terribly sorry for pressing hard, I just wondered, by no means did I want to attack you and your work. I do not know a lot about the RFC process here and marking things as stable. My laptop is too weak to test out the changes.

Hey, how do I stabilize these methods? What is the procedure?

@kpp you can start from my pr if you want https://github.com/rust-lang/rust/pull/51832

Or I'll try to finish it off this weekend if you don't. I'm a bit disappointed that I did not push it to the finish line.

I won't =) Push it to the finish line!

FYI, a bug was just filed related to is_global: https://github.com/rust-lang/rust/issues/57558

As above, this has been a problem for a while and is not a regression. I did not fix it because it looked like trying to figure how to name the reason that an IPv4 address might not be global looked like a maze of twisty little passages all alike.

If naming the various blocks or otherwise identifying them in the API is not significant, then I can pick this up and fix it for IPv4.

ping?

I'm stuck on nightly because of ip6.is_unicast_link_local(). Is this one ready to become stable yet?

Sorry for the delay, I'll re-open the PR this afternoon.
Looking at it, it seems some tests were missing/failing.
I had a hard time debugging so I wanted to refactor the tests a bit.

Not much has happened since https://github.com/rust-lang/rust/pull/60145. Should we go ahead an stabilize the Ipv4Addr and Ipv6Addr that are behind the ip feature?

Do we know what rust release the stabilization would occur in? I'd love to know when I can move to stable for one of my apps that relies on this.

@chills42 I just opened a PR to stabilize this.

Stabilization concern: There are motions under way[0][1] to slowly free up some of the reserved ipv4 ranges to make them useable as unicast addresses at some point in the future.

We discussed a similar concerns in #60145. For ipv6 the spec already says that is_global is a heuristic and must include reserved ranges per spec, so it wasn't a concern there while IPv4 was presumed set in stone. I wasn't aware of the above-mentioned plans, so we should reconsider.

Perhaps we could leave Ipv4Addr::is_global unstable or use the ipv6 approach (only exclude known non-global uses) instead. Or we could add a warning that these methods are subject to expected future standard changes, in fact there is an issue open for this #60239, perhaps that should be done before stabilizing.

[0] https://www.netdevconf.info/0x13/session.html?talk-ipv4-unicast-expansions
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=96125bf9985a

I'd be in favor of stabilizing everything, because if I understand correctly Ipv4Addr::is_global is correct, as per the current state of the RFCs (Linux allows 0.x.y.z addresses, but it hasn't been enacted by an RFC yet).

I prefer your second proposal: adding the disclaimer discussed in https://github.com/rust-lang/rust/issues/60239.

@little-dude you can send a stabilisation PR that stabilises the functions / module and then we can see what's missing or left to be done.

Add Ipv6Addr::to_ipv4_mapped: #75019

Hey so I think PR #76098 covers this stuff but I'm not sure? It's basically just an extension of #66584.

By the way, why not have a trait Ip for common feature on both ipv4 and ipv6 ?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

GuillaumeGomez picture GuillaumeGomez  Â·  300Comments

nikomatsakis picture nikomatsakis  Â·  210Comments

nikomatsakis picture nikomatsakis  Â·  268Comments

withoutboats picture withoutboats  Â·  308Comments

withoutboats picture withoutboats  Â·  213Comments