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

[alexcrichton]

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
• Ipv6::is_documentation

---

• Ip::is_global
• Ipv4::is_global
• Ipv6::is_global

---

• Ipv6::is_unicast_global
• Ipv6::is_unicast_link_local
• Ipv6::is_unicast_site_local
• Ipv6::is_unique_local
• Ipv6::multicast_scope

[aturon]

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

[alexcrichton]

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

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.

[briansmith]

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

[SimonSapin]

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

[briansmith]

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.

[steveklabnik]

@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.

[huonw]

FWIW, I filed #29221 a while ago, which would make tracking down what these tracking issues refer to slightly easier.

[alexcrichton]

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.

[ollie27]

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?

[alexcrichton]

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!

[alexcrichton]

🔔 This issue is now yet again entering its final comment period 🔔

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

[alexcrichton]

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:

• Ipv4Addr::is_unspecified - appears to be an ipv6 property, not ipv4?
• Ipv4Addr::is_loopback - ref
• Ipv4Addr::is_private - ref1, ref2, ref3
• Ipv4Addr::is_link_local - ref
• Ipv4Addr::is_global - not found, this is a property, but haven't found exhaustive documentation
• Ipv4Addr::is_multicast - ref
• Ipv4Addr::is_broadcast - ref
• 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:

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

[vinipsmaker]

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.

[alexcrichton]

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

[little-dude]

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.

[little-dude]

@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.

[therealbstern]

@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.

[little-dude]

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?

[little-dude]

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

[therealbstern]

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

[jens1o]

Why is this still unstable?

[little-dude]

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.

[jens1o]

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.

[kpp]

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

[little-dude]

@kpp you can start from my pr if you want #51832

[little-dude]

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.

[kpp]

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

[frewsxcv]

FYI, a bug was just filed related to is_global: #57558

[therealbstern]

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.

[kpp]

@little-dude please finish #56050