net/netip: missing odds and ends for 1.18 [freeze exception]

[zx2c4]

Now that netip has landed, I've started porting code to it. I'm hoping to get a bit of experience doing this before Go 1.18 in order to catch issues before it ships. So far I've identified six things that are missing, which I was hoping we could get into Go 1.18, as a followup to #46518.

Items with ⏳ have CLs still pending, and items with ✅ have CLs that have been submitted.

---

1. MarshalBinary/UnmarshalBinary on AddrPort and Prefix ✅

Addr.MarshalBinary and Addr.UnmarshalBinary both exist. However, the same is not true for the remaining two types, AddrPort and Prefix. This makes it very hard to use with encoding/gob, because not only are these marshals missing, but the inner addr + port/cidr members are unexported. So, I propose adding these two methods, with implementations that simply concatenate Addr.MarshalBinary output with little-endian encoding of the port (uint16) for AddrPort or cidr (uint8) for Prefix.

2. Addr.AsSlice ✅

There is netip.AddrFromSlice() and netip.AddrFrom4, but there is only netip.As4 and no netip.AsSlice. This may seem kind of trifling, but it winds up being kind of annoying when coding things; I find myself writing a lot of:

var s []byte
if addr.Is4() {
    arr := addr.As4()
    s = arr[:]
} else addr.Is16() {
    arr := addr.As16()
    s = arr[:]
}

You can't slice an rvalue, so that temporary is needed. And sometimes I just want a slice, regardless of what kind of IP it is, and having to open code dispatch for that every time is a bit of a pain. The above snippet would be simplified to:

s := addr.AsSlice()

3. IPv4Unspecified() ✅

There is IPv6Unspecified(), but not IPv4Unspecified(). This is sort of confusing, and code would be more clear and consistent if I could write netip.IPv4Unspecified() instead of netip.AddrFrom4([4]byte{}).

4. One remaining allocation in UDP path

The "net" package has one remaining allocation in the UDP packet path, which @josharian has a change to fix, so that UDP sends/receives can be allocation-free. Being alloc-free is one of the original motivations behind netip in the first place.

5. Missing UDPAddr and TCPAddr analogs ❌ (partially accepted, partially punted to go 1.19)

We have ReadFromUDPAddrPort and such, for reads and writes of UDP and TCP sockets, but the listener/dialer functions still take the old TCPAddr and UDPAddr structs. So, we'll need new functions there, to avoid unwanted type conversions by users.

6. AddrFromSlice should return only one value ❌ (rejected)

The AddrFrom4 and AddrFrom6 functions never fail, and that makes it easy to pass them around with function composition. AddrFromSlice, however, awkwardly returns an ok value. But, when ok==false, the returned Addr isn't valid anyway. So folks who want to check for validity can just do addr.IsValid() like normal, instead of having to learn something new and odd. So, we should drop the ok return value, especially as most users already ignore it.

---

(I'll update this issue if I find anything new.)

CC @bradfitz @crawshaw @josharian @danderson @ianlancetaylor

[zx2c4]

golang.zx2c4.com/go118/netip contains an extraction of Go 1.18's netip, so folks can experiment with this before Go 1.18 ships.

import "golang.zx2c4.com/go118/netip"

[bradfitz]

(1) is just an omission. I think @josharian was going to send a change for that.

(2) doesn't seem objectionable to me at least. We had that originally as https://pkg.go.dev/inet.af/netaddr#IP.IPAddr but for circular dependency reasons we couldn't include it in net/netip.

@rsc, opinions?

[dave-andersen]

(psst, I think you may have meant to tag @danderson instead of me. :-)

[gopherbot]

Change https://golang.org/cl/360874 mentions this issue: net/netip: add Addr.AsSlice() method

[DeedleFake]

Unless I'm misunderstanding something, AsSlice()'s functionality seems to already be covered by IPAddrParts(), MarshalBinary(), and the unexported ipZone().

[seankhliao]

Maybe we can drop IPAddrParts()? the discoverability of AsSlice() is better and there's already a Zone().
MarshalBinary() also encodes the zone, so it's something different

[gopherbot]

Change https://golang.org/cl/360875 mentions this issue: net/netip: add missing encoding.BinaryUnmarshaler to AddrPort and Prefix

[zx2c4]

The idea of AsSlice would be to return a single return value, like As4 and As6, so that it can easily and ergonomically be passed to functions and such.

The CL has been updated with @seankhliao's suggestion.

[zx2c4]

Top post of issue updated to contain mention of IPv4Unspecified.

[gopherbot]

Change https://golang.org/cl/361056 mentions this issue: net/netip: add IPv4Unspecified

[bradfitz]

Also, @josharian has a change to remove the remaining 1 alloc in the UDP paths so UDP sends/receives can be allocation-free, which is the main motivation of netip in the first place. It'd be a better story if the package & its benefit came together in the same release, rather than waiting for Go 1.19 to make use of netip in net be zero-alloc.

In the same spirit of the #49073 freeze exception proposal, this is a proposal that we finish the misc loose ends on netip for 1.18.

[ianlancetaylor]

CC @golang/release

[zx2c4]

Also, @josharian has a change to remove the remaining 1 alloc in the UDP paths so UDP sends/receives can be allocation-free

I've now updated the top post of this issue to include that.

[zx2c4]

Added to top post entry about missing UDPAddr and TCPAddr analogs.

[gopherbot]

Change https://golang.org/cl/361477 mentions this issue: net: add missing AddrPort functions to TCP and UDP

[zx2c4]

Added to top post entry about AddrFromSlice should return only one value.

[gopherbot]

Change https://golang.org/cl/361479 mentions this issue: net/netip: drop 'ok' argument in AddrFromSlice

[dmitshur]

We've discussed this on the release team. Freeze exception granted. Please be mindful of timing and stability as you proceed.

(Thanks for already documenting this new package in 1.18 release notes, it helps a lot!)

[zx2c4]

Great. There are currently CLs submitted for 5/6 of the items (afaik). The relation chain begins here: https://go-review.googlesource.com/c/go/+/360874/

[zx2c4]

One thing I was hoping to reach some consensus about was item 6:

---

6. AddrFromSlice should return only one value ⏳

The AddrFrom4 and AddrFrom6 functions never fail, and that makes it easy to pass them around with function composition. AddrFromSlice, however, awkwardly returns an ok value. But, when ok==false, the returned Addr isn't valid anyway. So folks who want to check for validity can just do addr.IsValid() like normal, instead of having to learn something new and odd. So, we should drop the ok return value, especially as most users already ignore it.

---

Brad mentioned in the issue that forcing people to check the ok value was a feature, not a bug. I see it differently, as a nuisance. Thought I should ask here how others feel, or if I should abandon that CL. (The CL's diff shows the simplification in other parts of the code afforded by this improvement.)

[gopherbot]

Change https://golang.org/cl/362596 mentions this issue: net: add conversion from AddrPort to TCPAddr to complement existing inverse

[gopherbot]

Change https://golang.org/cl/362595 mentions this issue: net: add missing AddrPort functions to UDP

[gopherbot]

Change https://golang.org/cl/362597 mentions this issue: net: add AddrPort Listen and Dial functions to TCP