proposal: net/netip: add netip.Addr.Map

[database64128]

netip.Addr has Unmap() to convert an IPv4-mapped IPv6 address to an IPv4 address:

// Unmap returns ip with any IPv4-mapped IPv6 address prefix removed.
//
// That is, if ip is an IPv6 address wrapping an IPv4 address, it
// returns the wrapped IPv4 address. Otherwise it returns ip unmodified.
func (ip Addr) Unmap() Addr {
	if ip.Is4In6() {
		ip.z = z4
	}
	return ip
}

To convert an IPv4 address to an IPv4-mapped IPv6 address, however, can only be achieved by netip.AddrFrom16(addr.As16()), which is rather cumbersome and inefficient. I propose we also add a corresponding Map() method:

// Map returns ip wrapped in an IPv6 address.
//
// That is, if ip is an IPv4 address, it returns an IPv6 address
// wrapping ip. Otherwise it returns ip unmodified.
func (ip Addr) Map() Addr {
	if ip.Is4() {
		ip.z = z6noz
	}
	return ip
}

/cc @bradfitz

[bradfitz]

which is rather cumbersome and inefficient

I agree it's cumbersome, but is it really inefficient? Got numbers?

But cumbersome-but-possible is usually a reasonable compromise for use cases that are very rare. It's often better than adding API surface.

[database64128]

But cumbersome-but-possible is usually a reasonable compromise for use cases that are very rare. It's often better than adding API surface.

One could also argue that the existing Unmap() method does not have to exist, because it is just as trivial to implement as Map().

And its use cases are not rare at all. I first adopted netip in https://github.com/database64128/swgp-go, which led me to open #52264. In swgp-go there is not a single use of Unmap(), but multiple uses of netip.AddrFrom16(addr.As16()) in many places.

Since you opened #45886, you might be interested to see that the relay routines in swgp-go for Linux also take advantage of the recvmmsg(2) and sendmmsg(2) syscalls.

[vincentbernat]

I also have a code base where I reduce code by handling everything as IPv6. I don't have a single use of Unmap() either, but I have to use netip.AddrFrom16(addr.As16()) often.

[database64128]

I agree it's cumbersome, but is it really inefficient? Got numbers?

AddrPortv4Mappedv6Unsafe simulates the proposed Map.

type addrPortHeader struct {
	ip   [16]byte
	z    unsafe.Pointer
	port uint16
}

// AddrPortv4Mappedv6 converts an IPv4 address to an IPv4-mapped IPv6 address.
// This function does nothing if addrPort is an IPv6 address.
func AddrPortv4Mappedv6(addrPort netip.AddrPort) netip.AddrPort {
	if addrPort.Addr().Is4() {
		addr6 := addrPort.Addr().As16()
		ip := netip.AddrFrom16(addr6)
		port := addrPort.Port()
		return netip.AddrPortFrom(ip, port)
	}
	return addrPort
}

func AddrPortv4Mappedv6Unsafe(addrPort netip.AddrPort) netip.AddrPort {
	if addrPort.Addr().Is4() {
		app := (*addrPortHeader)(unsafe.Pointer(&addrPort))
		app.z = unsafe.Add(app.z, 3*unsafe.Sizeof(uintptr(0)))
	}
	return addrPort
}

var (
	addrPort4    = netip.AddrPortFrom(netip.AddrFrom4([4]byte{127, 0, 0, 1}), 1080)
	addrPort4in6 = netip.AddrPortFrom(netip.AddrFrom16([16]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 127, 0, 0, 1}), 1080)
)

func BenchmarkAddrPortv4Mappedv6(b *testing.B) {
	b.Run("Is4", func(b *testing.B) {
		for i := 0; i < b.N; i++ {
			AddrPortv4Mappedv6(addrPort4)
		}
	})

	b.Run("Is4In6", func(b *testing.B) {
		for i := 0; i < b.N; i++ {
			AddrPortv4Mappedv6(addrPort4in6)
		}
	})
}

func BenchmarkAddrPortv4Mappedv6Unsafe(b *testing.B) {
	b.Run("Is4", func(b *testing.B) {
		for i := 0; i < b.N; i++ {
			AddrPortv4Mappedv6Unsafe(addrPort4)
		}
	})

	b.Run("Is4In6", func(b *testing.B) {
		for i := 0; i < b.N; i++ {
			AddrPortv4Mappedv6Unsafe(addrPort4in6)
		}
	})
}

func TestAddrPortv4Mappedv6Unsafe(t *testing.T) {
	app := (*addrPortHeader)(unsafe.Pointer(&addrPort4))
	t.Logf("addrPort4.z: %p", app.z)
	app = (*addrPortHeader)(unsafe.Pointer(&addrPort4in6))
	t.Logf("addrPort4in6.z: %p", app.z)

	if ap := AddrPortv4Mappedv6Unsafe(addrPort4); ap != addrPort4in6 {
		t.Errorf("AddrPortv4Mappedv6Unsafe(%s) returned %s, expected %s.", addrPort4, ap, addrPort4in6)
	}

	if ap := AddrPortv4Mappedv6Unsafe(addrPort4in6); ap != addrPort4in6 {
		t.Errorf("AddrPortv4Mappedv6Unsafe(%s) returned %s, expected %s.", addrPort4in6, ap, addrPort4in6)
	}
}

BenchmarkAddrPortv4Mappedv6/Is4-8               	100000000	        11.45 ns/op	       0 B/op	       0 allocs/op
BenchmarkAddrPortv4Mappedv6/Is4In6-8            	586448924	         2.021 ns/op	       0 B/op	       0 allocs/op
BenchmarkAddrPortv4Mappedv6Unsafe/Is4-8         	527418046	         2.294 ns/op	       0 B/op	       0 allocs/op
BenchmarkAddrPortv4Mappedv6Unsafe/Is4In6-8      	694173684	         1.760 ns/op	       0 B/op	       0 allocs/op

[favonia]

@database64128 @vincentbernat Hi all, this is a duplicate of my earlier proposal #51833 (but I don't mean we should close it---see below). Perhaps you will find interesting information there. My proposal was rejected because I could not present evidence of wide usage back then. I, too, have a codebase that could benefit from adding this function, though the benefit is small in my case. In any case, the Go team indicated they could revisit it when there's more information. 🙂

[vincentbernat]

How can we gather statistics about the use of As16 and AddrFrom16? It is a bit hard to gauge compared to the use of netip itself since it's quite new. Moreover, I would use the pattern more often myself, but as many IP addresses I handle come from net.IP, it's easier to use net.IP.To16 before doing the conversion. Using GitHub search, we see that the pattern exists in several projects.

As for the name, I would prefer Map, but otherwise, To16 would align better with existing methods (To16 in net.IP or As16/AddrFrom16).

[favonia]

As for the name, I would prefer Map, but otherwise, To16 would align better with existing methods (To16 in net.IP or As16/AddrFrom16).

Thanks for your investigation. I agree with everything you said except the naming---assuming that Map is unavailable, I think To6 will work better than To16 for netip. The netip packages uses two numbering systems: 4/16 and 4/6:

• The 4/16 in As4, AddrFrom4, As16 and AddrFrom16 refers to the size of byte arrays; these functions convert IP addresses to/from fixed-size byte arrays.
• The 4/6 in IPv4Unspecified, IPv6LinkLocalAllNodes, IPv6Unspecified, Is4In6 and Is6 refers to the protocol versions; they don't involve byte arrays and have clear meanings without referring to byte arrays.

In my opinion, the Map and Unmap are much closer to the latter and thus Map should be named To6 instead of To16.

[favonia]

Found a corner case that could support the inclusion of To6/Map. The zero value will be converted to :: by As16:

zero := netip.Addr{}
netip.AddrFrom16(zero.As16()) // becomes ::

while the proposed function will keep zero values as zeros. So, the longer code is not a full replacement.

[rsc]

BenchmarkAddrPortv4Mappedv6/Is4 shows that maybe there's an optimization available in that case. The optimization should not be to use unsafe.

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[rsc]

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

[favonia]

Based on the discussion above, this proposal seems like a likely decline.

@rsc Thanks for the update, but I'm a bit confused. It seems most comments in this PR are supportive. What does "the discussion above" refer to?

PS: Personally I am not super insistent, but I cannot speak for others.

[rsc]

It sounds like the main objection is performance, which we can address without new API. Do I have that right?

I see the comment about turning the zero Addr into a non-zero Addr (specifically, the non-zero Addr representing "::") but I'm not sure that's too compelling.

[vincentbernat]

It is unclear how the performance problem can be handled without a new API. In #54365 (comment), we see that there is a performance hit by using the existing API (unless using unsafe), doesn't we?

Maybe you would prefer a benchmark of new API vs existing API without unsafe?

[rsc]

@vincentbernat We'd be happy to see CLs that optimize this case without using unsafe.

[vincentbernat]

Do you mean a compiler optimization? This is out of my league. But I can provide a benchmark of Map() vs AddrFrom16(As16()):

 21:45 ❱ go test -bench .
goos: linux
goarch: amd64
pkg: titi
cpu: AMD Ryzen 5 5600X 6-Core Processor
BenchmarkNoMap/Is4-12           132831410                8.982 ns/op
BenchmarkNoMap/Is4In6-12        132333423                9.009 ns/op
BenchmarkMap/Is4-12             1000000000               0.6530 ns/op
BenchmarkMap/Is4In6-12          1000000000               0.4385 ns/op
PASS

Source

package main

import "testing"

// Copy/paste from netip

type uint128 struct {
	hi uint64
	lo uint64
}

type Addr struct {
	addr uint128
	z    *internValue
}

var (
	z0    = (*internValue)(nil)
	z4    = new(internValue)
	z6noz = new(internValue)
)

type internValue struct {
	_           [0]func()
	cmpVal      any
	resurrected bool
}

func AddrFrom16(addr [16]byte) Addr {
	return Addr{
		addr: uint128{
			beUint64(addr[:8]),
			beUint64(addr[8:]),
		},
		z: z6noz,
	}
}

func (ip Addr) As16() (a16 [16]byte) {
	bePutUint64(a16[:8], ip.addr.hi)
	bePutUint64(a16[8:], ip.addr.lo)
	return a16
}

func AddrFrom4(addr [4]byte) Addr {
	return Addr{
		addr: uint128{0, 0xffff00000000 | uint64(addr[0])<<24 | uint64(addr[1])<<16 | uint64(addr[2])<<8 | uint64(addr[3])},
		z:    z4,
	}
}

func (ip Addr) Is4() bool {
	return ip.z == z4
}

func bePutUint64(b []byte, v uint64) {
	_ = b[7] // early bounds check to guarantee safety of writes below
	b[0] = byte(v >> 56)
	b[1] = byte(v >> 48)
	b[2] = byte(v >> 40)
	b[3] = byte(v >> 32)
	b[4] = byte(v >> 24)
	b[5] = byte(v >> 16)
	b[6] = byte(v >> 8)
	b[7] = byte(v)
}

func beUint64(b []byte) uint64 {
	_ = b[7] // bounds check hint to compiler; see golang.org/issue/14808
	return uint64(b[7]) | uint64(b[6])<<8 | uint64(b[5])<<16 | uint64(b[4])<<24 |
		uint64(b[3])<<32 | uint64(b[2])<<40 | uint64(b[1])<<48 | uint64(b[0])<<56
}

// Add Map()

func (ip Addr) Map() Addr {
	if ip.Is4() {
		ip.z = z6noz
	}
	return ip
}

// Benchmark

var (
	addr4    = AddrFrom4([4]byte{127, 0, 0, 1})
	addr4in6 = AddrFrom16([16]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 127, 0, 0, 1})
)

func BenchmarkNoMap(b *testing.B) {
	b.Run("Is4", func(b *testing.B) {
		var z Addr
		for i := 0; i < b.N; i++ {
			z = AddrFrom16(addr4.As16())
		}
		if z != addr4in6 {
			b.Fatal("NO")
		}
	})

	b.Run("Is4In6", func(b *testing.B) {
		var z Addr
		for i := 0; i < b.N; i++ {
			z = AddrFrom16(addr4in6.As16())
		}
		if z != addr4in6 {
			b.Fatal("NO")
		}
	})
}

func BenchmarkMap(b *testing.B) {
	b.Run("Is4", func(b *testing.B) {
		var z Addr
		for i := 0; i < b.N; i++ {
			z = addr4.Map()
		}
		if z != addr4in6 {
			b.Fatal("NO")
		}
	})

	b.Run("Is4In6", func(b *testing.B) {
		var z Addr
		for i := 0; i < b.N; i++ {
			z = addr4in6.Map()
		}
		if z != addr4in6 {
			b.Fatal("NO")
		}
	})
}

[rsc]

No change in consensus, so declined.
— rsc for the proposal review group