Go: proposal: net: reconsider representation of IP

[bradfitz]

A net.IP is currently a []byte, which has the bad properties of:

• being mutable
• not being able to be a map key
• being large (the 24 byte slice header on 64-bit machines is bigger by itself than a 16 byte IPv6 address)

Reconsider for Go 2.

[jhorowitz]

I maintain an IP manipulation lib that relies heavily on ips being a byte array. Out of curiosity, what would the alternative be? Also, how can I make sure that I can give input on / contribute to the new implementation?

[bradfitz]

@jhorowitz, when the time comes, this will be the place. Just stay subscribed.

We're not soliciting design ideas or feedback yet.

[jhorowitz]

@bradfitz Thanks!

[mdlayher]

As much as I appreciate that the net.IP type can store both IPv4 and IPv6 addresses, I find the API awkward to use and the need to check for nil repeatedly annoying.

I almost wonder if it would make sense to make net.IP an interface in Go 2, with different IPv4 and IPv6 implementations. The "IPv4 mapped IPv6" address that net.ParseIP produces with an IPv4 address is almost certainly not what the caller wants.

As previously mentioned, the need to check for nil instead of an ok Boolean to determine if an operation was successful (ParseIP, To4, To16) is also unusual and not Go-like.

I am mobile at the moment but managed to find this issue, and wanted to leave some thoughts while they were fresh in my mind. I would be happy to go into more detail if needed.

[mikioh]

I guess what you really want is some help for dealing with IPv6 Addressing of IPv4/IPv6 Translators defined in RFC 6052 instead of IPv4 mapped IPv6 addresses on circumstances using XLAT, and more help for IPv6 Address Synthesis described in RFC 7050.

Is my understanding correct? I'm fine to make IP address handling API more flexible with user-supplied context; I'm a XYZ-mo user, here I have a translation prefix via some mechanism such as PvD: thus any IP address comes from the prefix must be considered as an IPv4 address.

[mdlayher]

Sorry for the delay, forgot to reply sooner.

I'm a little confused by the terminology you've used (and am still fairly new to IPv6 as a whole), but here's what I mean:

https://play.golang.org/p/2xleMiTvFIW

I don't understand why these functions would default to this 16 byte representation, when it's almost certainly not what the caller would want. Mapping IPv4 addresses like this seems like an advanced use case that I wouldn't think the stdlib should need to handle. Of course, I could be missing something obvious.

This has bitten me a few times in the past, and now I find myself creating small helpers to check things like "is X bytes or Y string actually an IPv4 or IPv6 address"?

https://play.golang.org/p/Udt8YQdO6f2

It seems like it'd be less error prone if they were two separate types, instead of having to make sure you remember to call "To4" and "To16" appropriately, and remember what a nil return value means for both.

[bradfitz]

Concretely, I want the representation to be like a time.Time: a small, opaque, immutable value type. It could then be copied and retained at will, like a Time.

We could add methods for the required operations.

[bradfitz]

@mdlayher, I presume from your thumbs-up on the prior comment that you're cool with a Time-like representation rather than an interface as you previously proposed?

[mdlayher]

I think I'd really just like a better way to disambiguate between IPv4 and IPv6 addresses. An interface seemed like a decent way to do that, but I'd be curious to see what kind of API you have in mind.

[jhorowitz]

@mdlayher Do you find calling ip.To4 != nil to be too cumbersome or have you found the reslicing to be a performance bottleneck? I do agree the method is not super intuitive.
In terms of the 16 byte representation, IPv6 reserves part of it's IP space to represent IPv4 addresses. If an IPv6 address has bytes 0-9 as zeros and 10-11 as 255 then the remaining 4 bytes represent an IPv4 address. I'm not sure why it defaults to the 16 byte representation but I have found that having that default makes comparisons easier when mixing IPv4 and IPv6 together.

@bradfitz What do you think of unifying IP and IPNet into one structure?

[mdlayher]

Apologies for the delay.

Do you find calling ip.To4 != nil to be too cumbersome or have you found the reslicing to be a performance bottleneck? I do agree the method is not super intuitive.

Yep, it's really non-obvious IMHO, and too easy to make silly mistakes like passing an IPv4 address as bytes to some system while stored in its packed, 16-byte IPv6 mapped representation.

In terms of the 16 byte representation, IPv6 reserves part of it's IP space to represent IPv4 addresses. If an IPv6 address has bytes 0-9 as zeros and 10-11 as 255 then the remaining 4 bytes represent an IPv4 address. I'm not sure why it defaults to the 16 byte representation but I have found that having that default makes comparisons easier when mixing IPv4 and IPv6 together.

I'm just not really sure why the standard library does this at all. Can't say I've ever come across a system where this representation was used. Does it even need to be supported by the standard library? (Honest question, I'm really not sure!) I suppose it depends on what the "Go 2" representation is like.

Even if it remains, I do feel strongly that net.IPv4 should return the 4-byte representation, as that's almost certainly what the caller wants.

---

To further reply to Brad's thread:

I presume from your thumbs-up on the prior comment that you're cool with a Time-like representation rather than an interface as you previously proposed?

@bradfitz have any ideas in mind for this yet? Were you thinking about something like:

type IP struct {
    high uint64
    low  uint64
}

... and then using this to represent both IPv4/6 addresses (IPv4 using the 16-byte mapped representation)? I'm okay with this, but I would argue in favor of more obvious methods of obtaining an IPv4 versus IPv6 address that don't require strange nil checks like the current To4 and To16 methods.

Here's a helper of sorts I usually end up copying and pasting between packages:

// checkIPv6 verifies that ip is an IPv6 address.
func checkIPv6(ip net.IP) error {
    if ip.To16() == nil || ip.To4() != nil {
        return fmt.Errorf("ndp: invalid IPv6 address: %q", ip.String())
    }

    return nil
}

... and I'd love to not have to do that. :)

[bradfitz]

Were you thinking about something like:

Something like that, or an array. But that can change over time (as time.Time has) so I'm more interested in discussing:

• a list of methods we'd have on an IP
• could we get rid of the existing Equal(IP) bool method? Would == be sufficient?
• If so, that means IP to 1.2.3.4 vs an IP to ::ff:ff:1.2.3.4 are not ==, so:
• which Is4() bool / Is6() bool / ToMapped6() IP / etc APIs would be sufficient?
• or can we say that an IP is never in IPv4-in-IPv6 mapped form and if you want that, you need to do, say:

// Copy16 copies 16 bytes into dst. It panics if dst is not at least 16 bytes. If ip is an IPv4 address, its IPv4-in-IPv6 representation is copied, lorem ipsum .... 
func (ip IP) Copy6(dst []byte)

[mdlayher]

Sorry, hit some keyboard shortcut. Typing up a more coherent response...

[mdlayher]

My vote is to simplify the existing API surface and move things that 95% of folks won't need to x/net or outside the stdlib. I realize I'm proposing some big changes, and I don't know if this is something folks are willing to consider, but here are my thoughts:

---

a list of methods we'd have on an IP

I'm not totally convinced all of the existing IsX (IsGlobalUnicast, IsLinkLinkUnicast, etc.) methods are necessary in the stdlib. Maybe it'd be best to stick to the simplest ones, like IsLoopback, IsMulticast, and IsUnspecified. I don't feel strongly about this, but I generally think small API surfaces are nicer.

DefaultMask feels unnecessary in stdlib especially because it only applies to IPv4. I'd vote x/net/ipv4 or remove.

I would be curious to see how much MarshalText and UnmarshalText are actually used in the wild; I've pretty much exclusively used ParseIP and IP.String. Maybe these are used in stdlib for optimizations?

So in theory, I wonder if we could get away with just:

• Equal (see below)
• Is4 (new)
• Is6 (new)
• IsLoopback
• IsMulticast
• IsUnspecified
• String

... and leave any other address classification logic to x/net or outside stdlib.

could we get rid of the existing Equal(IP) bool method? Would == be sufficient?

I think retaining Equal is a decent idea so that the internal representation can change later on if needed, but since you explicitly mentioned the goal of keeping it small and such, perhaps that escape hatch is no longer necessary.

If so, that means IP to 1.2.3.4 vs an IP to ::ff:ff:1.2.3.4 are not ==, so:

This has personally bitten me a few times in some code that was shipping around IP addresses as bytes in protobufs. I'd be okay with calling these two representations "not equal".

which Is4() bool / Is6() bool / ToMapped6() IP / etc APIs would be sufficient?

I like the explicitness of the first two methods. I personally would argue against supporting the notion of the mapped IPv4 in IPv6 representation unless there's some major use case for it that I'm not aware of.

or can we say that an IP is never in IPv4-in-IPv6 mapped form and if you want that, you need to do, say:

Feels like an x/net or outside of stdlib thing to me IMHO.

[bradfitz]

FYI: I've started implementing this at https://github.com/inetaf/netaddr because I needed it.