TUN-6007: Implement new edge discovery algorithm

(cherry picked from commit 4f468b8a5d)

connection/errors.go

@@ -18,6 +18,15 @@ func (e DupConnRegisterTunnelError) Error() string {
 	return "already connected to this server, trying another address"
 }
 
+// Dial to edge server with quic failed
+type EdgeQuicDialError struct {
+	Cause error
+}
+
+func (e *EdgeQuicDialError) Error() string {
+	return "failed to dial to edge with quic: " + e.Cause.Error()
+}
+
 // RegisterTunnel error from server
 type ServerRegisterTunnelError struct {
 	Cause     error

connection/quic.go

@@ -57,7 +57,7 @@ func NewQUICConnection(
 ) (*QUICConnection, error) {
 	session, err := quic.DialAddr(edgeAddr.String(), tlsConfig, quicConfig)
 	if err != nil {
-		return nil, fmt.Errorf("failed to dial to edge: %w", err)
+		return nil, &EdgeQuicDialError{Cause: err}
 	}
 
 	datagramMuxer, err := quicpogs.NewDatagramMuxer(session, logger)

edgediscovery/allregions/address.go

@@ -0,0 +1,64 @@
+package allregions
+
+// Region contains cloudflared edge addresses. The edge is partitioned into several regions for
+// redundancy purposes.
+type AddrSet map[*EdgeAddr]UsedBy
+
+// AddrUsedBy finds the address used by the given connection in this region.
+// Returns nil if the connection isn't using any IP.
+func (a AddrSet) AddrUsedBy(connID int) *EdgeAddr {
+	for addr, used := range a {
+		if used.Used && used.ConnID == connID {
+			return addr
+		}
+	}
+	return nil
+}
+
+// AvailableAddrs counts how many unused addresses this region contains.
+func (a AddrSet) AvailableAddrs() int {
+	n := 0
+	for _, usedby := range a {
+		if !usedby.Used {
+			n++
+		}
+	}
+	return n
+}
+
+// GetUnusedIP returns a random unused address in this region.
+// Returns nil if all addresses are in use.
+func (a AddrSet) GetUnusedIP(excluding *EdgeAddr) *EdgeAddr {
+	for addr, usedby := range a {
+		if !usedby.Used && addr != excluding {
+			return addr
+		}
+	}
+	return nil
+}
+
+// Use the address, assigning it to a proxy connection.
+func (a AddrSet) Use(addr *EdgeAddr, connID int) {
+	if addr == nil {
+		return
+	}
+	a[addr] = InUse(connID)
+}
+
+// GetAnyAddress returns an arbitrary address from the region.
+func (a AddrSet) GetAnyAddress() *EdgeAddr {
+	for addr := range a {
+		return addr
+	}
+	return nil
+}
+
+// GiveBack the address, ensuring it is no longer assigned to an IP.
+// Returns true if the address is in this region.
+func (a AddrSet) GiveBack(addr *EdgeAddr) (ok bool) {
+	if _, ok := a[addr]; !ok {
+		return false
+	}
+	a[addr] = Unused()
+	return true
+}

edgediscovery/allregions/address_test.go

@@ -0,0 +1,247 @@
+package allregions
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestAddrSet_AddrUsedBy(t *testing.T) {
+	type args struct {
+		connID int
+	}
+	tests := []struct {
+		name    string
+		addrSet AddrSet
+		args    args
+		want    *EdgeAddr
+	}{
+		{
+			name: "happy trivial test",
+			addrSet: AddrSet{
+				&addr0: InUse(0),
+			},
+			args: args{connID: 0},
+			want: &addr0,
+		},
+		{
+			name: "sad trivial test",
+			addrSet: AddrSet{
+				&addr0: InUse(0),
+			},
+			args: args{connID: 1},
+			want: nil,
+		},
+		{
+			name: "sad test",
+			addrSet: AddrSet{
+				&addr0: InUse(0),
+				&addr1: InUse(1),
+				&addr2: InUse(2),
+			},
+			args: args{connID: 3},
+			want: nil,
+		},
+		{
+			name: "happy test",
+			addrSet: AddrSet{
+				&addr0: InUse(0),
+				&addr1: InUse(1),
+				&addr2: InUse(2),
+			},
+			args: args{connID: 1},
+			want: &addr1,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.addrSet.AddrUsedBy(tt.args.connID); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("Region.AddrUsedBy() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestAddrSet_AvailableAddrs(t *testing.T) {
+	tests := []struct {
+		name    string
+		addrSet AddrSet
+		want    int
+	}{
+		{
+			name: "contains addresses",
+			addrSet: AddrSet{
+				&addr0: InUse(0),
+				&addr1: Unused(),
+				&addr2: InUse(2),
+			},
+			want: 1,
+		},
+		{
+			name: "all free",
+			addrSet: AddrSet{
+				&addr0: Unused(),
+				&addr1: Unused(),
+				&addr2: Unused(),
+			},
+			want: 3,
+		},
+		{
+			name: "all used",
+			addrSet: AddrSet{
+				&addr0: InUse(0),
+				&addr1: InUse(1),
+				&addr2: InUse(2),
+			},
+			want: 0,
+		},
+		{
+			name:    "empty",
+			addrSet: AddrSet{},
+			want:    0,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.addrSet.AvailableAddrs(); got != tt.want {
+				t.Errorf("Region.AvailableAddrs() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestAddrSet_GetUnusedIP(t *testing.T) {
+	type args struct {
+		excluding *EdgeAddr
+	}
+	tests := []struct {
+		name    string
+		addrSet AddrSet
+		args    args
+		want    *EdgeAddr
+	}{
+		{
+			name: "happy test with excluding set",
+			addrSet: AddrSet{
+				&addr0: Unused(),
+				&addr1: Unused(),
+				&addr2: InUse(2),
+			},
+			args: args{excluding: &addr0},
+			want: &addr1,
+		},
+		{
+			name: "happy test with no excluding",
+			addrSet: AddrSet{
+				&addr0: InUse(0),
+				&addr1: Unused(),
+				&addr2: InUse(2),
+			},
+			args: args{excluding: nil},
+			want: &addr1,
+		},
+		{
+			name: "sad test with no excluding",
+			addrSet: AddrSet{
+				&addr0: InUse(0),
+				&addr1: InUse(1),
+				&addr2: InUse(2),
+			},
+			args: args{excluding: nil},
+			want: nil,
+		},
+		{
+			name: "sad test with excluding",
+			addrSet: AddrSet{
+				&addr0: Unused(),
+				&addr1: InUse(1),
+				&addr2: InUse(2),
+			},
+			args: args{excluding: &addr0},
+			want: nil,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.addrSet.GetUnusedIP(tt.args.excluding); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("Region.GetUnusedIP() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestAddrSet_GiveBack(t *testing.T) {
+	type args struct {
+		addr *EdgeAddr
+	}
+	tests := []struct {
+		name           string
+		addrSet        AddrSet
+		args           args
+		wantOk         bool
+		availableAfter int
+	}{
+		{
+			name: "sad test with excluding",
+			addrSet: AddrSet{
+				&addr1: InUse(1),
+			},
+			args:           args{addr: &addr1},
+			wantOk:         true,
+			availableAfter: 1,
+		},
+		{
+			name: "sad test with excluding",
+			addrSet: AddrSet{
+				&addr1: InUse(1),
+			},
+			args:           args{addr: &addr2},
+			wantOk:         false,
+			availableAfter: 0,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if gotOk := tt.addrSet.GiveBack(tt.args.addr); gotOk != tt.wantOk {
+				t.Errorf("Region.GiveBack() = %v, want %v", gotOk, tt.wantOk)
+			}
+			if tt.availableAfter != tt.addrSet.AvailableAddrs() {
+				t.Errorf("Region.AvailableAddrs() = %v, want %v", tt.addrSet.AvailableAddrs(), tt.availableAfter)
+			}
+		})
+	}
+}
+
+func TestAddrSet_GetAnyAddress(t *testing.T) {
+	tests := []struct {
+		name    string
+		addrSet AddrSet
+		wantNil bool
+	}{
+		{
+			name:    "Sad test -- GetAnyAddress should only fail if the region is empty",
+			addrSet: AddrSet{},
+			wantNil: true,
+		},
+		{
+			name: "Happy test (all addresses unused)",
+			addrSet: AddrSet{
+				&addr0: Unused(),
+			},
+			wantNil: false,
+		},
+		{
+			name: "Happy test (GetAnyAddress can still return addresses used by proxy conns)",
+			addrSet: AddrSet{
+				&addr0: InUse(2),
+			},
+			wantNil: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.addrSet.GetAnyAddress(); tt.wantNil != (got == nil) {
+				t.Errorf("Region.GetAnyAddress() = %v, but should it return nil? %v", got, tt.wantNil)
+			}
+		})
+	}
+}

edgediscovery/allregions/discovery.go

@@ -13,7 +13,7 @@ import (
 
 const (
 	// Used to discover HA origintunneld servers
-	srvService = "origintunneld"
+	srvService = "v2-origintunneld"
 	srvProto   = "tcp"
 	srvName    = "argotunnel.com"
 
@@ -115,6 +115,9 @@ func edgeDiscovery(log *zerolog.Logger, srvService string) ([][]*EdgeAddr, error
 		if err != nil {
 			return nil, err
 		}
+		for _, e := range edgeAddrs {
+			log.Debug().Msgf("Edge Address: %+v", *e)
+		}
 		resolvedAddrPerCNAME = append(resolvedAddrPerCNAME, edgeAddrs)
 	}
 
@@ -187,7 +190,6 @@ func ResolveAddrs(addrs []string, log *zerolog.Logger) (resolved []*EdgeAddr) {
 			UDP:       udpAddr,
 			IPVersion: version,
 		})
-
 	}
 	return
 }

edgediscovery/allregions/mocks_for_test.go

@@ -9,6 +9,115 @@ import (
 	"testing/quick"
 )
 
+var (
+	v4Addrs = []*EdgeAddr{&addr0, &addr1, &addr2, &addr3}
+	v6Addrs = []*EdgeAddr{&addr4, &addr5, &addr6, &addr7}
+	addr0   = EdgeAddr{
+		TCP: &net.TCPAddr{
+			IP:   net.ParseIP("123.4.5.0"),
+			Port: 8000,
+			Zone: "",
+		},
+		UDP: &net.UDPAddr{
+			IP:   net.ParseIP("123.4.5.0"),
+			Port: 8000,
+			Zone: "",
+		},
+		IPVersion: V4,
+	}
+	addr1 = EdgeAddr{
+		TCP: &net.TCPAddr{
+			IP:   net.ParseIP("123.4.5.1"),
+			Port: 8000,
+			Zone: "",
+		},
+		UDP: &net.UDPAddr{
+			IP:   net.ParseIP("123.4.5.1"),
+			Port: 8000,
+			Zone: "",
+		},
+		IPVersion: V4,
+	}
+	addr2 = EdgeAddr{
+		TCP: &net.TCPAddr{
+			IP:   net.ParseIP("123.4.5.2"),
+			Port: 8000,
+			Zone: "",
+		},
+		UDP: &net.UDPAddr{
+			IP:   net.ParseIP("123.4.5.2"),
+			Port: 8000,
+			Zone: "",
+		},
+		IPVersion: V4,
+	}
+	addr3 = EdgeAddr{
+		TCP: &net.TCPAddr{
+			IP:   net.ParseIP("123.4.5.3"),
+			Port: 8000,
+			Zone: "",
+		},
+		UDP: &net.UDPAddr{
+			IP:   net.ParseIP("123.4.5.3"),
+			Port: 8000,
+			Zone: "",
+		},
+		IPVersion: V4,
+	}
+	addr4 = EdgeAddr{
+		TCP: &net.TCPAddr{
+			IP:   net.ParseIP("2606:4700:a0::1"),
+			Port: 8000,
+			Zone: "",
+		},
+		UDP: &net.UDPAddr{
+			IP:   net.ParseIP("2606:4700:a0::1"),
+			Port: 8000,
+			Zone: "",
+		},
+		IPVersion: V6,
+	}
+	addr5 = EdgeAddr{
+		TCP: &net.TCPAddr{
+			IP:   net.ParseIP("2606:4700:a0::2"),
+			Port: 8000,
+			Zone: "",
+		},
+		UDP: &net.UDPAddr{
+			IP:   net.ParseIP("2606:4700:a0::2"),
+			Port: 8000,
+			Zone: "",
+		},
+		IPVersion: V6,
+	}
+	addr6 = EdgeAddr{
+		TCP: &net.TCPAddr{
+			IP:   net.ParseIP("2606:4700:a0::3"),
+			Port: 8000,
+			Zone: "",
+		},
+		UDP: &net.UDPAddr{
+			IP:   net.ParseIP("2606:4700:a0::3"),
+			Port: 8000,
+			Zone: "",
+		},
+		IPVersion: V6,
+	}
+	addr7 = EdgeAddr{
+		TCP: &net.TCPAddr{
+			IP:   net.ParseIP("2606:4700:a0::4"),
+			Port: 8000,
+			Zone: "",
+		},
+		UDP: &net.UDPAddr{
+			IP:   net.ParseIP("2606:4700:a0::4"),
+			Port: 8000,
+			Zone: "",
+		},
+		IPVersion: V6,
+	}
+)
+
 type mockAddrs struct {
 	// a set of synthetic SRV records
 	addrMap map[net.SRV][]*EdgeAddr

edgediscovery/allregions/region.go

@@ -1,79 +1,155 @@
 package allregions
 
+import "time"
+
+const (
+	timeoutDuration = 10 * time.Minute
+)
+
 // Region contains cloudflared edge addresses. The edge is partitioned into several regions for
 // redundancy purposes.
 type Region struct {
-	connFor map[*EdgeAddr]UsedBy
+	primaryIsActive bool
+	active          AddrSet
+	primary         AddrSet
+	secondary       AddrSet
+	primaryTimeout  time.Time
+	timeoutDuration time.Duration
 }
 
 // NewRegion creates a region with the given addresses, which are all unused.
-func NewRegion(addrs []*EdgeAddr) Region {
+func NewRegion(addrs []*EdgeAddr, overrideIPVersion ConfigIPVersion) Region {
 	// The zero value of UsedBy is Unused(), so we can just initialize the map's values with their
 	// zero values.
-	connFor := make(map[*EdgeAddr]UsedBy)
+	connForv4 := make(AddrSet)
-	for _, addr := range addrs {
+	connForv6 := make(AddrSet)
-		connFor[addr] = Unused()
+	systemPreference := V6
+	for i, addr := range addrs {
+		if i == 0 {
+			// First family of IPs returned is system preference of IP
+			systemPreference = addr.IPVersion
 		}
+		switch addr.IPVersion {
+		case V4:
+			connForv4[addr] = Unused()
+		case V6:
+			connForv6[addr] = Unused()
+		}
+	}
+
+	// Process as system preference
+	var primary AddrSet
+	var secondary AddrSet
+	switch systemPreference {
+	case V4:
+		primary = connForv4
+		secondary = connForv6
+	case V6:
+		primary = connForv6
+		secondary = connForv4
+	}
+
+	// Override with provided preference
+	switch overrideIPVersion {
+	case IPv4Only:
+		primary = connForv4
+		secondary = make(AddrSet) // empty
+	case IPv6Only:
+		primary = connForv6
+		secondary = make(AddrSet) // empty
+	case Auto:
+		// no change
+	default:
+		// no change
+	}
+
 	return Region{
-		connFor: connFor,
+		primaryIsActive: true,
+		active:          primary,
+		primary:         primary,
+		secondary:       secondary,
+		timeoutDuration: timeoutDuration,
 	}
 }
 
 // AddrUsedBy finds the address used by the given connection in this region.
 // Returns nil if the connection isn't using any IP.
 func (r *Region) AddrUsedBy(connID int) *EdgeAddr {
-	for addr, used := range r.connFor {
+	edgeAddr := r.primary.AddrUsedBy(connID)
-		if used.Used && used.ConnID == connID {
+	if edgeAddr == nil {
-			return addr
+		edgeAddr = r.secondary.AddrUsedBy(connID)
 	}
-	}
+	return edgeAddr
-	return nil
 }
 
 // AvailableAddrs counts how many unused addresses this region contains.
 func (r Region) AvailableAddrs() int {
-	n := 0
+	return r.active.AvailableAddrs()
-	for _, usedby := range r.connFor {
-		if !usedby.Used {
-			n++
-		}
-	}
-	return n
 }
 
-// GetUnusedIP returns a random unused address in this region.
+// AssignAnyAddress returns a random unused address in this region now
-// Returns nil if all addresses are in use.
+// assigned to the connID excluding the provided EdgeAddr.
-func (r Region) GetUnusedIP(excluding *EdgeAddr) *EdgeAddr {
+// Returns nil if all addresses are in use for the region.
-	for addr, usedby := range r.connFor {
+func (r Region) AssignAnyAddress(connID int, excluding *EdgeAddr) *EdgeAddr {
-		if !usedby.Used && addr != excluding {
+	if addr := r.active.GetUnusedIP(excluding); addr != nil {
+		r.active.Use(addr, connID)
 		return addr
 	}
-	}
 	return nil
 }
 
-// Use the address, assigning it to a proxy connection.
-func (r Region) Use(addr *EdgeAddr, connID int) {
-	if addr == nil {
-		return
-	}
-	r.connFor[addr] = InUse(connID)
-}
-
 // GetAnyAddress returns an arbitrary address from the region.
 func (r Region) GetAnyAddress() *EdgeAddr {
-	for addr := range r.connFor {
+	return r.active.GetAnyAddress()
-		return addr
-	}
-	return nil
 }
 
 // GiveBack the address, ensuring it is no longer assigned to an IP.
 // Returns true if the address is in this region.
-func (r Region) GiveBack(addr *EdgeAddr) (ok bool) {
+func (r *Region) GiveBack(addr *EdgeAddr, hasConnectivityError bool) (ok bool) {
-	if _, ok := r.connFor[addr]; !ok {
+	if ok = r.primary.GiveBack(addr); !ok {
-		return false
+		// Attempt to give back the address in the secondary set
+		if ok = r.secondary.GiveBack(addr); !ok {
+			// Address is not in this region
+			return
 		}
-	r.connFor[addr] = Unused()
+	}
-	return true
+
+	// No connectivity error: no worry
+	if !hasConnectivityError {
+		return
+	}
+
+	// If using primary and returned address is IPv6 and secondary is available
+	if r.primaryIsActive && addr.IPVersion == V6 && len(r.secondary) > 0 {
+		r.active = r.secondary
+		r.primaryIsActive = false
+		r.primaryTimeout = time.Now().Add(r.timeoutDuration)
+		return
+	}
+
+	// Do nothing for IPv4 or if secondary is empty
+	if r.primaryIsActive {
+		return
+	}
+
+	// Immediately return to primary pool, regardless of current primary timeout
+	if addr.IPVersion == V4 {
+		activatePrimary(r)
+		return
+	}
+
+	// Timeout exceeded and can be reset to primary pool
+	if r.primaryTimeout.Before(time.Now()) {
+		activatePrimary(r)
+		return
+	}
+
+	return
+}
+
+// activatePrimary sets the primary set to the active set and resets the timeout.
+func activatePrimary(r *Region) {
+	r.active = r.primary
+	r.primaryIsActive = true
+	r.primaryTimeout = time.Now() // reset timeout
 }

edgediscovery/allregions/region_test.go

@@ -1,284 +1,357 @@
 package allregions
 
 import (
-	"reflect"
+	"net"
 	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
 )
 
+func makeAddrSet(addrs []*EdgeAddr) AddrSet {
+	addrSet := make(AddrSet, len(addrs))
+	for _, addr := range addrs {
+		addrSet[addr] = Unused()
+	}
+	return addrSet
+}
+
 func TestRegion_New(t *testing.T) {
-	r := NewRegion([]*EdgeAddr{&addr0, &addr1, &addr2})
-	if r.AvailableAddrs() != 3 {
-		t.Errorf("r.AvailableAddrs() == %v but want 3", r.AvailableAddrs())
-	}
-}
-
-func TestRegion_AddrUsedBy(t *testing.T) {
-	type fields struct {
-		connFor map[*EdgeAddr]UsedBy
-	}
-	type args struct {
-		connID int
-	}
 	tests := []struct {
 		name          string
-		fields fields
+		addrs         []*EdgeAddr
-		args   args
+		mode          ConfigIPVersion
-		want   *EdgeAddr
+		expectedAddrs int
+		primary       AddrSet
+		secondary     AddrSet
 	}{
 		{
-			name: "happy trivial test",
+			name:          "IPv4 addresses with IPv4Only",
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{
+			addrs:         v4Addrs,
-				&addr0: InUse(0),
+			mode:          IPv4Only,
-			}},
+			expectedAddrs: len(v4Addrs),
-			args: args{connID: 0},
+			primary:       makeAddrSet(v4Addrs),
-			want: &addr0,
+			secondary:     AddrSet{},
 		},
 		{
-			name: "sad trivial test",
+			name:          "IPv6 addresses with IPv4Only",
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{
+			addrs:         v6Addrs,
-				&addr0: InUse(0),
+			mode:          IPv4Only,
-			}},
+			expectedAddrs: 0,
-			args: args{connID: 1},
+			primary:       AddrSet{},
-			want: nil,
+			secondary:     AddrSet{},
 		},
 		{
-			name: "sad test",
+			name:          "IPv6 addresses with IPv6Only",
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{
+			addrs:         v6Addrs,
-				&addr0: InUse(0),
+			mode:          IPv6Only,
-				&addr1: InUse(1),
+			expectedAddrs: len(v6Addrs),
-				&addr2: InUse(2),
+			primary:       makeAddrSet(v6Addrs),
-			}},
+			secondary:     AddrSet{},
-			args: args{connID: 3},
-			want: nil,
 		},
 		{
-			name: "happy test",
+			name:          "IPv6 addresses with IPv4Only",
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{
+			addrs:         v6Addrs,
-				&addr0: InUse(0),
+			mode:          IPv4Only,
-				&addr1: InUse(1),
+			expectedAddrs: 0,
-				&addr2: InUse(2),
+			primary:       AddrSet{},
-			}},
+			secondary:     AddrSet{},
-			args: args{connID: 1},
+		},
-			want: &addr1,
+		{
+			name:          "IPv4 (first) and IPv6 addresses with Auto",
+			addrs:         append(v4Addrs, v6Addrs...),
+			mode:          Auto,
+			expectedAddrs: len(v4Addrs),
+			primary:       makeAddrSet(v4Addrs),
+			secondary:     makeAddrSet(v6Addrs),
+		},
+		{
+			name:          "IPv6 (first) and IPv4 addresses with Auto",
+			addrs:         append(v6Addrs, v4Addrs...),
+			mode:          Auto,
+			expectedAddrs: len(v6Addrs),
+			primary:       makeAddrSet(v6Addrs),
+			secondary:     makeAddrSet(v4Addrs),
+		},
+		{
+			name:          "IPv4 addresses with Auto",
+			addrs:         v4Addrs,
+			mode:          Auto,
+			expectedAddrs: len(v4Addrs),
+			primary:       makeAddrSet(v4Addrs),
+			secondary:     AddrSet{},
+		},
+		{
+			name:          "IPv6 addresses with Auto",
+			addrs:         v6Addrs,
+			mode:          Auto,
+			expectedAddrs: len(v6Addrs),
+			primary:       makeAddrSet(v6Addrs),
+			secondary:     AddrSet{},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			r := &Region{
+			r := NewRegion(tt.addrs, tt.mode)
-				connFor: tt.fields.connFor,
+			assert.Equal(t, tt.expectedAddrs, r.AvailableAddrs())
+			assert.Equal(t, tt.primary, r.primary)
+			assert.Equal(t, tt.secondary, r.secondary)
+		})
 	}
-			if got := r.AddrUsedBy(tt.args.connID); !reflect.DeepEqual(got, tt.want) {
+}
-				t.Errorf("Region.AddrUsedBy() = %v, want %v", got, tt.want)
+
+func TestRegion_AnyAddress_EmptyActiveSet(t *testing.T) {
+	tests := []struct {
+		name  string
+		addrs []*EdgeAddr
+		mode  ConfigIPVersion
+	}{
+		{
+			name:  "IPv6 addresses with IPv4Only",
+			addrs: v6Addrs,
+			mode:  IPv4Only,
+		},
+		{
+			name:  "IPv4 addresses with IPv6Only",
+			addrs: v4Addrs,
+			mode:  IPv6Only,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			r := NewRegion(tt.addrs, tt.mode)
+			addr := r.GetAnyAddress()
+			assert.Nil(t, addr)
+			addr = r.AssignAnyAddress(0, nil)
+			assert.Nil(t, addr)
+		})
+	}
+}
+
+func TestRegion_AssignAnyAddress_FullyUsedActiveSet(t *testing.T) {
+	tests := []struct {
+		name  string
+		addrs []*EdgeAddr
+		mode  ConfigIPVersion
+	}{
+		{
+			name:  "IPv6 addresses with IPv6Only",
+			addrs: v6Addrs,
+			mode:  IPv6Only,
+		},
+		{
+			name:  "IPv4 addresses with IPv4Only",
+			addrs: v4Addrs,
+			mode:  IPv4Only,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			r := NewRegion(tt.addrs, tt.mode)
+			total := r.active.AvailableAddrs()
+			for i := 0; i < total; i++ {
+				addr := r.AssignAnyAddress(i, nil)
+				assert.NotNil(t, addr)
+			}
+			addr := r.AssignAnyAddress(9, nil)
+			assert.Nil(t, addr)
+		})
+	}
+}
+
+var giveBackTests = []struct {
+	name          string
+	addrs         []*EdgeAddr
+	mode          ConfigIPVersion
+	expectedAddrs int
+	primary       AddrSet
+	secondary     AddrSet
+	primarySwap   bool
+}{
+	{
+		name:          "IPv4 addresses with IPv4Only",
+		addrs:         v4Addrs,
+		mode:          IPv4Only,
+		expectedAddrs: len(v4Addrs),
+		primary:       makeAddrSet(v4Addrs),
+		secondary:     AddrSet{},
+		primarySwap:   false,
+	},
+	{
+		name:          "IPv6 addresses with IPv6Only",
+		addrs:         v6Addrs,
+		mode:          IPv6Only,
+		expectedAddrs: len(v6Addrs),
+		primary:       makeAddrSet(v6Addrs),
+		secondary:     AddrSet{},
+		primarySwap:   false,
+	},
+	{
+		name:          "IPv4 (first) and IPv6 addresses with Auto",
+		addrs:         append(v4Addrs, v6Addrs...),
+		mode:          Auto,
+		expectedAddrs: len(v4Addrs),
+		primary:       makeAddrSet(v4Addrs),
+		secondary:     makeAddrSet(v6Addrs),
+		primarySwap:   false,
+	},
+	{
+		name:          "IPv6 (first) and IPv4 addresses with Auto",
+		addrs:         append(v6Addrs, v4Addrs...),
+		mode:          Auto,
+		expectedAddrs: len(v6Addrs),
+		primary:       makeAddrSet(v6Addrs),
+		secondary:     makeAddrSet(v4Addrs),
+		primarySwap:   true,
+	},
+	{
+		name:          "IPv4 addresses with Auto",
+		addrs:         v4Addrs,
+		mode:          Auto,
+		expectedAddrs: len(v4Addrs),
+		primary:       makeAddrSet(v4Addrs),
+		secondary:     AddrSet{},
+		primarySwap:   false,
+	},
+	{
+		name:          "IPv6 addresses with Auto",
+		addrs:         v6Addrs,
+		mode:          Auto,
+		expectedAddrs: len(v6Addrs),
+		primary:       makeAddrSet(v6Addrs),
+		secondary:     AddrSet{},
+		primarySwap:   false,
+	},
+}
+
+func TestRegion_GiveBack_NoConnectivityError(t *testing.T) {
+	for _, tt := range giveBackTests {
+		t.Run(tt.name, func(t *testing.T) {
+			r := NewRegion(tt.addrs, tt.mode)
+			addr := r.AssignAnyAddress(0, nil)
+			assert.NotNil(t, addr)
+			assert.True(t, r.GiveBack(addr, false))
+		})
+	}
+}
+
+func TestRegion_GiveBack_ForeignAddr(t *testing.T) {
+	invalid := EdgeAddr{
+		TCP: &net.TCPAddr{
+			IP:   net.ParseIP("123.4.5.0"),
+			Port: 8000,
+			Zone: "",
+		},
+		UDP: &net.UDPAddr{
+			IP:   net.ParseIP("123.4.5.0"),
+			Port: 8000,
+			Zone: "",
+		},
+		IPVersion: V4,
+	}
+	for _, tt := range giveBackTests {
+		t.Run(tt.name, func(t *testing.T) {
+			r := NewRegion(tt.addrs, tt.mode)
+			assert.False(t, r.GiveBack(&invalid, false))
+			assert.False(t, r.GiveBack(&invalid, true))
+		})
+	}
+}
+
+func TestRegion_GiveBack_SwapPrimary(t *testing.T) {
+	for _, tt := range giveBackTests {
+		t.Run(tt.name, func(t *testing.T) {
+			r := NewRegion(tt.addrs, tt.mode)
+			addr := r.AssignAnyAddress(0, nil)
+			assert.NotNil(t, addr)
+			assert.True(t, r.GiveBack(addr, true))
+			assert.Equal(t, tt.primarySwap, !r.primaryIsActive)
+			if tt.primarySwap {
+				assert.Equal(t, r.secondary, r.active)
+				assert.False(t, r.primaryTimeout.IsZero())
+			} else {
+				assert.Equal(t, r.primary, r.active)
+				assert.True(t, r.primaryTimeout.IsZero())
 			}
 		})
 	}
 }
 
-func TestRegion_AvailableAddrs(t *testing.T) {
+func TestRegion_GiveBack_IPv4_ResetPrimary(t *testing.T) {
-	type fields struct {
+	r := NewRegion(append(v6Addrs, v4Addrs...), Auto)
-		connFor map[*EdgeAddr]UsedBy
+	// Exhaust all IPv6 addresses
-	}
+	a0 := r.AssignAnyAddress(0, nil)
-	tests := []struct {
+	a1 := r.AssignAnyAddress(1, nil)
-		name   string
+	a2 := r.AssignAnyAddress(2, nil)
-		fields fields
+	a3 := r.AssignAnyAddress(3, nil)
-		want   int
+	assert.NotNil(t, a0)
-	}{
+	assert.NotNil(t, a1)
-		{
+	assert.NotNil(t, a2)
-			name: "contains addresses",
+	assert.NotNil(t, a3)
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{
+	// Give back the first IPv6 address to fallback to secondary IPv4 address set
-				&addr0: InUse(0),
+	assert.True(t, r.GiveBack(a0, true))
-				&addr1: Unused(),
+	assert.False(t, r.primaryIsActive)
-				&addr2: InUse(2),
+	// Give back another IPv6 address
-			}},
+	assert.True(t, r.GiveBack(a1, true))
-			want: 1,
+	// Primary shouldn't change
-		},
+	assert.False(t, r.primaryIsActive)
-		{
+	// Request an address (should be IPv4 from secondary)
-			name: "all free",
+	a4_v4 := r.AssignAnyAddress(4, nil)
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{
+	assert.NotNil(t, a4_v4)
-				&addr0: Unused(),
+	assert.Equal(t, V4, a4_v4.IPVersion)
-				&addr1: Unused(),
+	a5_v4 := r.AssignAnyAddress(5, nil)
-				&addr2: Unused(),
+	assert.NotNil(t, a5_v4)
-			}},
+	assert.Equal(t, V4, a5_v4.IPVersion)
-			want: 3,
+	a6_v4 := r.AssignAnyAddress(6, nil)
-		},
+	assert.NotNil(t, a6_v4)
-		{
+	assert.Equal(t, V4, a6_v4.IPVersion)
-			name: "all used",
+	// Return IPv4 address (without failure)
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{
+	// Primary shouldn't change because it is not a connectivity failure
-				&addr0: InUse(0),
+	assert.True(t, r.GiveBack(a4_v4, false))
-				&addr1: InUse(1),
+	assert.False(t, r.primaryIsActive)
-				&addr2: InUse(2),
+	// Return IPv4 address (with failure)
-			}},
+	// Primary should change because it is a connectivity failure
-			want: 0,
+	assert.True(t, r.GiveBack(a5_v4, true))
-		},
+	assert.True(t, r.primaryIsActive)
-		{
+	// Return IPv4 address (with failure)
-			name:   "empty",
+	// Primary shouldn't change because the address is returned to the inactive
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{}},
+	// secondary address set
-			want:   0,
+	assert.True(t, r.GiveBack(a6_v4, true))
-		},
+	assert.True(t, r.primaryIsActive)
-	}
+	// Return IPv6 address (without failure)
-	for _, tt := range tests {
+	// Primary shoudn't change because it is not a connectivity failure
-		t.Run(tt.name, func(t *testing.T) {
+	assert.True(t, r.GiveBack(a2, false))
-			r := Region{
+	assert.True(t, r.primaryIsActive)
-				connFor: tt.fields.connFor,
-			}
-			if got := r.AvailableAddrs(); got != tt.want {
-				t.Errorf("Region.AvailableAddrs() = %v, want %v", got, tt.want)
-			}
-		})
-	}
 }
 
-func TestRegion_GetUnusedIP(t *testing.T) {
+func TestRegion_GiveBack_Timeout(t *testing.T) {
-	type fields struct {
+	r := NewRegion(append(v6Addrs, v4Addrs...), Auto)
-		connFor map[*EdgeAddr]UsedBy
+	a0 := r.AssignAnyAddress(0, nil)
-	}
+	a1 := r.AssignAnyAddress(1, nil)
-	type args struct {
+	a2 := r.AssignAnyAddress(2, nil)
-		excluding *EdgeAddr
+	assert.NotNil(t, a0)
-	}
+	assert.NotNil(t, a1)
-	tests := []struct {
+	assert.NotNil(t, a2)
-		name   string
+	// Give back IPv6 address to set timeout
-		fields fields
+	assert.True(t, r.GiveBack(a0, true))
-		args   args
+	assert.False(t, r.primaryIsActive)
-		want   *EdgeAddr
+	assert.False(t, r.primaryTimeout.IsZero())
-	}{
+	// Request an address (should be IPv4 from secondary)
-		{
+	a3_v4 := r.AssignAnyAddress(3, nil)
-			name: "happy test with excluding set",
+	assert.NotNil(t, a3_v4)
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{
+	assert.Equal(t, V4, a3_v4.IPVersion)
-				&addr0: Unused(),
+	assert.False(t, r.primaryIsActive)
-				&addr1: Unused(),
+	// Give back IPv6 address inside timeout (no change)
-				&addr2: InUse(2),
+	assert.True(t, r.GiveBack(a2, true))
-			}},
+	assert.False(t, r.primaryIsActive)
-			args: args{excluding: &addr0},
+	assert.False(t, r.primaryTimeout.IsZero())
-			want: &addr1,
+	// Accelerate timeout
-		},
+	r.primaryTimeout = time.Now().Add(-time.Minute)
-		{
+	// Return IPv6 address
-			name: "happy test with no excluding",
+	assert.True(t, r.GiveBack(a1, true))
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{
+	assert.True(t, r.primaryIsActive)
-				&addr0: InUse(0),
+	// Returning an IPv4 address after primary is active shouldn't change primary
-				&addr1: Unused(),
+	// even with a connectivity error
-				&addr2: InUse(2),
+	assert.True(t, r.GiveBack(a3_v4, true))
-			}},
+	assert.True(t, r.primaryIsActive)
-			args: args{excluding: nil},
-			want: &addr1,
-		},
-		{
-			name: "sad test with no excluding",
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{
-				&addr0: InUse(0),
-				&addr1: InUse(1),
-				&addr2: InUse(2),
-			}},
-			args: args{excluding: nil},
-			want: nil,
-		},
-		{
-			name: "sad test with excluding",
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{
-				&addr0: Unused(),
-				&addr1: InUse(1),
-				&addr2: InUse(2),
-			}},
-			args: args{excluding: &addr0},
-			want: nil,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			r := Region{
-				connFor: tt.fields.connFor,
-			}
-			if got := r.GetUnusedIP(tt.args.excluding); !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("Region.GetUnusedIP() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
-func TestRegion_GiveBack(t *testing.T) {
-	type fields struct {
-		connFor map[*EdgeAddr]UsedBy
-	}
-	type args struct {
-		addr *EdgeAddr
-	}
-	tests := []struct {
-		name           string
-		fields         fields
-		args           args
-		wantOk         bool
-		availableAfter int
-	}{
-		{
-			name: "sad test with excluding",
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{
-				&addr1: InUse(1),
-			}},
-			args:           args{addr: &addr1},
-			wantOk:         true,
-			availableAfter: 1,
-		},
-		{
-			name: "sad test with excluding",
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{
-				&addr1: InUse(1),
-			}},
-			args:           args{addr: &addr2},
-			wantOk:         false,
-			availableAfter: 0,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			r := Region{
-				connFor: tt.fields.connFor,
-			}
-			if gotOk := r.GiveBack(tt.args.addr); gotOk != tt.wantOk {
-				t.Errorf("Region.GiveBack() = %v, want %v", gotOk, tt.wantOk)
-			}
-			if tt.availableAfter != r.AvailableAddrs() {
-				t.Errorf("Region.AvailableAddrs() = %v, want %v", r.AvailableAddrs(), tt.availableAfter)
-			}
-		})
-	}
-}
-
-func TestRegion_GetAnyAddress(t *testing.T) {
-	type fields struct {
-		connFor map[*EdgeAddr]UsedBy
-	}
-	tests := []struct {
-		name    string
-		fields  fields
-		wantNil bool
-	}{
-		{
-			name:    "Sad test -- GetAnyAddress should only fail if the region is empty",
-			fields:  fields{connFor: map[*EdgeAddr]UsedBy{}},
-			wantNil: true,
-		},
-		{
-			name: "Happy test (all addresses unused)",
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{
-				&addr0: Unused(),
-			}},
-			wantNil: false,
-		},
-		{
-			name: "Happy test (GetAnyAddress can still return addresses used by proxy conns)",
-			fields: fields{connFor: map[*EdgeAddr]UsedBy{
-				&addr0: InUse(2),
-			}},
-			wantNil: false,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			r := Region{
-				connFor: tt.fields.connFor,
-			}
-			if got := r.GetAnyAddress(); tt.wantNil != (got == nil) {
-				t.Errorf("Region.GetAnyAddress() = %v, but should it return nil? %v", got, tt.wantNil)
-			}
-		})
-	}
 }

edgediscovery/allregions/regions.go

@@ -18,7 +18,7 @@ type Regions struct {
 // ------------------------------------
 
 // ResolveEdge resolves the Cloudflare edge, returning all regions discovered.
-func ResolveEdge(log *zerolog.Logger, region string) (*Regions, error) {
+func ResolveEdge(log *zerolog.Logger, region string, overrideIPVersion ConfigIPVersion) (*Regions, error) {
 	edgeAddrs, err := edgeDiscovery(log, getRegionalServiceName(region))
 	if err != nil {
 		return nil, err
@@ -27,8 +27,8 @@ func ResolveEdge(log *zerolog.Logger, region string) (*Regions, error) {
 		return nil, fmt.Errorf("expected at least 2 Cloudflare Regions regions, but SRV only returned %v", len(edgeAddrs))
 	}
 	return &Regions{
-		region1: NewRegion(edgeAddrs[0]),
+		region1: NewRegion(edgeAddrs[0], overrideIPVersion),
-		region2: NewRegion(edgeAddrs[1]),
+		region2: NewRegion(edgeAddrs[1], overrideIPVersion),
 	}, nil
 }
 
@@ -56,8 +56,8 @@ func NewNoResolve(addrs []*EdgeAddr) *Regions {
 	}
 
 	return &Regions{
-		region1: NewRegion(region1),
+		region1: NewRegion(region1, Auto),
-		region2: NewRegion(region2),
+		region2: NewRegion(region2, Auto),
 	}
 }
 
@@ -95,14 +95,12 @@ func (rs *Regions) GetUnusedAddr(excluding *EdgeAddr, connID int) *EdgeAddr {
 // getAddrs tries to grab address form `first` region, then `second` region
 // this is an unrolled loop over 2 element array
 func getAddrs(excluding *EdgeAddr, connID int, first *Region, second *Region) *EdgeAddr {
-	addr := first.GetUnusedIP(excluding)
+	addr := first.AssignAnyAddress(connID, excluding)
 	if addr != nil {
-		first.Use(addr, connID)
 		return addr
 	}
-	addr = second.GetUnusedIP(excluding)
+	addr = second.AssignAnyAddress(connID, excluding)
 	if addr != nil {
-		second.Use(addr, connID)
 		return addr
 	}
 
@@ -116,18 +114,18 @@ func (rs *Regions) AvailableAddrs() int {
 
 // GiveBack the address so that other connections can use it.
 // Returns true if the address is in this edge.
-func (rs *Regions) GiveBack(addr *EdgeAddr) bool {
+func (rs *Regions) GiveBack(addr *EdgeAddr, hasConnectivityError bool) bool {
-	if found := rs.region1.GiveBack(addr); found {
+	if found := rs.region1.GiveBack(addr, hasConnectivityError); found {
 		return found
 	}
-	return rs.region2.GiveBack(addr)
+	return rs.region2.GiveBack(addr, hasConnectivityError)
 }
 
 // Return regionalized service name if `region` isn't empty, otherwise return the global service name for origintunneld
 func getRegionalServiceName(region string) string {
 	if region != "" {
-		return region + "-" + srvService // Example: `us-origintunneld`
+		return region + "-" + srvService // Example: `us-v2-origintunneld`
 	}
 
-	return srvService // Global service is just `origintunneld`
+	return srvService // Global service is just `v2-origintunneld`
 }

edgediscovery/allregions/regions_test.go

@@ -1,134 +1,215 @@
 package allregions
 
 import (
-	"net"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
-var (
+func makeRegions(addrs []*EdgeAddr, mode ConfigIPVersion) Regions {
-	addr0 = EdgeAddr{
+	r1addrs := make([]*EdgeAddr, 0)
-		TCP: &net.TCPAddr{
+	r2addrs := make([]*EdgeAddr, 0)
-			IP:   net.ParseIP("123.4.5.0"),
+	for i, addr := range addrs {
-			Port: 8000,
+		if i%2 == 0 {
-			Zone: "",
+			r1addrs = append(r1addrs, addr)
-		},
+		} else {
-		UDP: &net.UDPAddr{
+			r2addrs = append(r2addrs, addr)
-			IP:   net.ParseIP("123.4.5.0"),
-			Port: 8000,
-			Zone: "",
-		},
 		}
-	addr1 = EdgeAddr{
-		TCP: &net.TCPAddr{
-			IP:   net.ParseIP("123.4.5.1"),
-			Port: 8000,
-			Zone: "",
-		},
-		UDP: &net.UDPAddr{
-			IP:   net.ParseIP("123.4.5.1"),
-			Port: 8000,
-			Zone: "",
-		},
 	}
-	addr2 = EdgeAddr{
+	r1 := NewRegion(r1addrs, mode)
-		TCP: &net.TCPAddr{
+	r2 := NewRegion(r2addrs, mode)
-			IP:   net.ParseIP("123.4.5.2"),
-			Port: 8000,
-			Zone: "",
-		},
-		UDP: &net.UDPAddr{
-			IP:   net.ParseIP("123.4.5.2"),
-			Port: 8000,
-			Zone: "",
-		},
-	}
-	addr3 = EdgeAddr{
-		TCP: &net.TCPAddr{
-			IP:   net.ParseIP("123.4.5.3"),
-			Port: 8000,
-			Zone: "",
-		},
-		UDP: &net.UDPAddr{
-			IP:   net.ParseIP("123.4.5.3"),
-			Port: 8000,
-			Zone: "",
-		},
-	}
-)
-
-func makeRegions() Regions {
-	r1 := NewRegion([]*EdgeAddr{&addr0, &addr1})
-	r2 := NewRegion([]*EdgeAddr{&addr2, &addr3})
 	return Regions{region1: r1, region2: r2}
 }
 
 func TestRegions_AddrUsedBy(t *testing.T) {
-	rs := makeRegions()
+	tests := []struct {
+		name  string
+		addrs []*EdgeAddr
+		mode  ConfigIPVersion
+	}{
+		{
+			name:  "IPv4 addresses with IPv4Only",
+			addrs: v4Addrs,
+			mode:  IPv4Only,
+		},
+		{
+			name:  "IPv6 addresses with IPv6Only",
+			addrs: v6Addrs,
+			mode:  IPv6Only,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			rs := makeRegions(tt.addrs, tt.mode)
 			addr1 := rs.GetUnusedAddr(nil, 1)
 			assert.Equal(t, addr1, rs.AddrUsedBy(1))
 			addr2 := rs.GetUnusedAddr(nil, 2)
 			assert.Equal(t, addr2, rs.AddrUsedBy(2))
 			addr3 := rs.GetUnusedAddr(nil, 3)
 			assert.Equal(t, addr3, rs.AddrUsedBy(3))
+		})
+	}
 }
 
 func TestRegions_Giveback_Region1(t *testing.T) {
-	rs := makeRegions()
+	tests := []struct {
-	rs.region1.Use(&addr0, 0)
+		name  string
-	rs.region1.Use(&addr1, 1)
+		addrs []*EdgeAddr
-	rs.region2.Use(&addr2, 2)
+		mode  ConfigIPVersion
-	rs.region2.Use(&addr3, 3)
+	}{
+		{
+			name:  "IPv4 addresses with IPv4Only",
+			addrs: v4Addrs,
+			mode:  IPv4Only,
+		},
+		{
+			name:  "IPv6 addresses with IPv6Only",
+			addrs: v6Addrs,
+			mode:  IPv6Only,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			rs := makeRegions(tt.addrs, tt.mode)
+			addr := rs.region1.AssignAnyAddress(0, nil)
+			rs.region1.AssignAnyAddress(1, nil)
+			rs.region2.AssignAnyAddress(2, nil)
+			rs.region2.AssignAnyAddress(3, nil)
 
 			assert.Equal(t, 0, rs.AvailableAddrs())
 
-	rs.GiveBack(&addr0)
+			rs.GiveBack(addr, false)
-	assert.Equal(t, &addr0, rs.GetUnusedAddr(nil, 3))
+			assert.Equal(t, addr, rs.GetUnusedAddr(nil, 0))
+		})
+	}
 }
 
 func TestRegions_Giveback_Region2(t *testing.T) {
-	rs := makeRegions()
+	tests := []struct {
-	rs.region1.Use(&addr0, 0)
+		name  string
-	rs.region1.Use(&addr1, 1)
+		addrs []*EdgeAddr
-	rs.region2.Use(&addr2, 2)
+		mode  ConfigIPVersion
-	rs.region2.Use(&addr3, 3)
+	}{
+		{
+			name:  "IPv4 addresses with IPv4Only",
+			addrs: v4Addrs,
+			mode:  IPv4Only,
+		},
+		{
+			name:  "IPv6 addresses with IPv6Only",
+			addrs: v6Addrs,
+			mode:  IPv6Only,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			rs := makeRegions(tt.addrs, tt.mode)
+			rs.region1.AssignAnyAddress(0, nil)
+			rs.region1.AssignAnyAddress(1, nil)
+			addr := rs.region2.AssignAnyAddress(2, nil)
+			rs.region2.AssignAnyAddress(3, nil)
 
 			assert.Equal(t, 0, rs.AvailableAddrs())
 
-	rs.GiveBack(&addr2)
+			rs.GiveBack(addr, false)
-	assert.Equal(t, &addr2, rs.GetUnusedAddr(nil, 2))
+			assert.Equal(t, addr, rs.GetUnusedAddr(nil, 2))
+		})
+	}
 }
 
 func TestRegions_GetUnusedAddr_OneAddrLeft(t *testing.T) {
-	rs := makeRegions()
+	tests := []struct {
-
+		name  string
-	rs.region1.Use(&addr0, 0)
+		addrs []*EdgeAddr
-	rs.region1.Use(&addr1, 1)
+		mode  ConfigIPVersion
-	rs.region2.Use(&addr2, 2)
+	}{
+		{
+			name:  "IPv4 addresses with IPv4Only",
+			addrs: v4Addrs,
+			mode:  IPv4Only,
+		},
+		{
+			name:  "IPv6 addresses with IPv6Only",
+			addrs: v6Addrs,
+			mode:  IPv6Only,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			rs := makeRegions(tt.addrs, tt.mode)
+			rs.region1.AssignAnyAddress(0, nil)
+			rs.region1.AssignAnyAddress(1, nil)
+			rs.region2.AssignAnyAddress(2, nil)
+			addr := rs.region2.active.GetUnusedIP(nil)
 
 			assert.Equal(t, 1, rs.AvailableAddrs())
-	assert.Equal(t, &addr3, rs.GetUnusedAddr(nil, 3))
+			assert.Equal(t, addr, rs.GetUnusedAddr(nil, 3))
+		})
+	}
 }
 
 func TestRegions_GetUnusedAddr_Excluding_Region1(t *testing.T) {
-	rs := makeRegions()
+	tests := []struct {
+		name  string
+		addrs []*EdgeAddr
+		mode  ConfigIPVersion
+	}{
+		{
+			name:  "IPv4 addresses with IPv4Only",
+			addrs: v4Addrs,
+			mode:  IPv4Only,
+		},
+		{
+			name:  "IPv6 addresses with IPv6Only",
+			addrs: v6Addrs,
+			mode:  IPv6Only,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			rs := makeRegions(tt.addrs, tt.mode)
 
-	rs.region1.Use(&addr0, 0)
+			rs.region1.AssignAnyAddress(0, nil)
-	rs.region1.Use(&addr1, 1)
+			rs.region1.AssignAnyAddress(1, nil)
+			addr := rs.region2.active.GetUnusedIP(nil)
+			a2 := rs.region2.active.GetUnusedIP(addr)
 
 			assert.Equal(t, 2, rs.AvailableAddrs())
-	assert.Equal(t, &addr3, rs.GetUnusedAddr(&addr2, 3))
+			assert.Equal(t, addr, rs.GetUnusedAddr(a2, 3))
+		})
+	}
 }
 
 func TestRegions_GetUnusedAddr_Excluding_Region2(t *testing.T) {
-	rs := makeRegions()
+	tests := []struct {
+		name  string
+		addrs []*EdgeAddr
+		mode  ConfigIPVersion
+	}{
+		{
+			name:  "IPv4 addresses with IPv4Only",
+			addrs: v4Addrs,
+			mode:  IPv4Only,
+		},
+		{
+			name:  "IPv6 addresses with IPv6Only",
+			addrs: v6Addrs,
+			mode:  IPv6Only,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			rs := makeRegions(tt.addrs, tt.mode)
 
-	rs.region2.Use(&addr2, 0)
+			rs.region2.AssignAnyAddress(0, nil)
-	rs.region2.Use(&addr3, 1)
+			rs.region2.AssignAnyAddress(1, nil)
+			addr := rs.region1.active.GetUnusedIP(nil)
+			a2 := rs.region1.active.GetUnusedIP(addr)
 
 			assert.Equal(t, 2, rs.AvailableAddrs())
-	assert.Equal(t, &addr1, rs.GetUnusedAddr(&addr0, 1))
+			assert.Equal(t, addr, rs.GetUnusedAddr(a2, 1))
+		})
+	}
 }
 
 func TestNewNoResolveBalancesRegions(t *testing.T) {

edgediscovery/edgediscovery.go

@@ -11,9 +11,10 @@ import (
 
 const (
 	LogFieldConnIndex = "connIndex"
+	LogFieldIPAddress = "ip"
 )
 
-var errNoAddressesLeft = fmt.Errorf("there are no free edge addresses left")
+var ErrNoAddressesLeft = fmt.Errorf("there are no free edge addresses left")
 
 // Edge finds addresses on the Cloudflare edge and hands them out to connections.
 type Edge struct {
@@ -28,8 +29,8 @@ type Edge struct {
 
 // ResolveEdge runs the initial discovery of the Cloudflare edge, finding Addrs that can be allocated
 // to connections.
-func ResolveEdge(log *zerolog.Logger, region string) (*Edge, error) {
+func ResolveEdge(log *zerolog.Logger, region string, edgeIpVersion allregions.ConfigIPVersion) (*Edge, error) {
-	regions, err := allregions.ResolveEdge(log, region)
+	regions, err := allregions.ResolveEdge(log, region, edgeIpVersion)
 	if err != nil {
 		return new(Edge), err
 	}
@@ -51,15 +52,6 @@ func StaticEdge(log *zerolog.Logger, hostnames []string) (*Edge, error) {
 	}, nil
 }
 
-// MockEdge creates a Cloudflare Edge from arbitrary TCP addresses. Used for testing.
-func MockEdge(log *zerolog.Logger, addrs []*allregions.EdgeAddr) *Edge {
-	regions := allregions.NewNoResolve(addrs)
-	return &Edge{
-		log:     log,
-		regions: regions,
-	}
-}
-
 // ------------------------------------
 // Methods
 // ------------------------------------
@@ -70,7 +62,7 @@ func (ed *Edge) GetAddrForRPC() (*allregions.EdgeAddr, error) {
 	defer ed.Unlock()
 	addr := ed.regions.GetAnyAddress()
 	if addr == nil {
-		return nil, errNoAddressesLeft
+		return nil, ErrNoAddressesLeft
 	}
 	return addr, nil
 }
@@ -91,14 +83,17 @@ func (ed *Edge) GetAddr(connIndex int) (*allregions.EdgeAddr, error) {
 	addr := ed.regions.GetUnusedAddr(nil, connIndex)
 	if addr == nil {
 		log.Debug().Msg("edgediscovery - GetAddr: No addresses left to give proxy connection")
-		return nil, errNoAddressesLeft
+		return nil, ErrNoAddressesLeft
 	}
-	log.Debug().Msg("edgediscovery - GetAddr: Giving connection its new address")
+	log = ed.log.With().
+		Int(LogFieldConnIndex, connIndex).
+		IPAddr(LogFieldIPAddress, addr.UDP.IP).Logger()
+	log.Debug().Msgf("edgediscovery - GetAddr: Giving connection its new address")
 	return addr, nil
 }
 
 // GetDifferentAddr gives back the proxy connection's edge Addr and uses a new one.
-func (ed *Edge) GetDifferentAddr(connIndex int) (*allregions.EdgeAddr, error) {
+func (ed *Edge) GetDifferentAddr(connIndex int, hasConnectivityError bool) (*allregions.EdgeAddr, error) {
 	log := ed.log.With().Int(LogFieldConnIndex, connIndex).Logger()
 
 	ed.Lock()
@@ -106,16 +101,18 @@ func (ed *Edge) GetDifferentAddr(connIndex int) (*allregions.EdgeAddr, error) {
 
 	oldAddr := ed.regions.AddrUsedBy(connIndex)
 	if oldAddr != nil {
-		ed.regions.GiveBack(oldAddr)
+		ed.regions.GiveBack(oldAddr, hasConnectivityError)
 	}
 	addr := ed.regions.GetUnusedAddr(oldAddr, connIndex)
 	if addr == nil {
 		log.Debug().Msg("edgediscovery - GetDifferentAddr: No addresses left to give proxy connection")
 		// note: if oldAddr were not nil, it will become available on the next iteration
-		return nil, errNoAddressesLeft
+		return nil, ErrNoAddressesLeft
 	}
-	log.Debug().Msgf("edgediscovery - GetDifferentAddr: Giving connection its new address: %v from the address list: %v",
+	log = ed.log.With().
-		addr, ed.regions.AvailableAddrs())
+		Int(LogFieldConnIndex, connIndex).
+		IPAddr(LogFieldIPAddress, addr.UDP.IP).Logger()
+	log.Debug().Msgf("edgediscovery - GetDifferentAddr: Giving connection its new address from the address list: %v", ed.regions.AvailableAddrs())
 	return addr, nil
 }
 
@@ -128,9 +125,11 @@ func (ed *Edge) AvailableAddrs() int {
 
 // GiveBack the address so that other connections can use it.
 // Returns true if the address is in this edge.
-func (ed *Edge) GiveBack(addr *allregions.EdgeAddr) bool {
+func (ed *Edge) GiveBack(addr *allregions.EdgeAddr, hasConnectivityError bool) bool {
 	ed.Lock()
 	defer ed.Unlock()
-	ed.log.Debug().Msg("edgediscovery - GiveBack: Address now unused")
+	log := ed.log.With().
-	return ed.regions.GiveBack(addr)
+		IPAddr(LogFieldIPAddress, addr.UDP.IP).Logger()
+	log.Debug().Msgf("edgediscovery - GiveBack: Address now unused")
+	return ed.regions.GiveBack(addr, hasConnectivityError)
 }

edgediscovery/edgediscovery_test.go

@@ -11,56 +11,113 @@ import (
 )
 
 var (
+	testLogger = zerolog.Nop()
+	v4Addrs    = []*allregions.EdgeAddr{&addr0, &addr1, &addr2, &addr3}
+	v6Addrs    = []*allregions.EdgeAddr{&addr4, &addr5, &addr6, &addr7}
 	addr0      = allregions.EdgeAddr{
 		TCP: &net.TCPAddr{
-			IP:   net.ParseIP("123.0.0.0"),
+			IP:   net.ParseIP("123.4.5.0"),
 			Port: 8000,
 			Zone: "",
 		},
 		UDP: &net.UDPAddr{
-			IP:   net.ParseIP("123.0.0.0"),
+			IP:   net.ParseIP("123.4.5.0"),
 			Port: 8000,
 			Zone: "",
 		},
+		IPVersion: allregions.V4,
 	}
 	addr1 = allregions.EdgeAddr{
 		TCP: &net.TCPAddr{
-			IP:   net.ParseIP("123.0.0.1"),
+			IP:   net.ParseIP("123.4.5.1"),
 			Port: 8000,
 			Zone: "",
 		},
 		UDP: &net.UDPAddr{
-			IP:   net.ParseIP("123.0.0.1"),
+			IP:   net.ParseIP("123.4.5.1"),
 			Port: 8000,
 			Zone: "",
 		},
+		IPVersion: allregions.V4,
 	}
 	addr2 = allregions.EdgeAddr{
 		TCP: &net.TCPAddr{
-			IP:   net.ParseIP("123.0.0.2"),
+			IP:   net.ParseIP("123.4.5.2"),
 			Port: 8000,
 			Zone: "",
 		},
 		UDP: &net.UDPAddr{
-			IP:   net.ParseIP("123.0.0.2"),
+			IP:   net.ParseIP("123.4.5.2"),
 			Port: 8000,
 			Zone: "",
 		},
+		IPVersion: allregions.V4,
 	}
 	addr3 = allregions.EdgeAddr{
 		TCP: &net.TCPAddr{
-			IP:   net.ParseIP("123.0.0.3"),
+			IP:   net.ParseIP("123.4.5.3"),
 			Port: 8000,
 			Zone: "",
 		},
 		UDP: &net.UDPAddr{
-			IP:   net.ParseIP("123.0.0.3"),
+			IP:   net.ParseIP("123.4.5.3"),
 			Port: 8000,
 			Zone: "",
 		},
+		IPVersion: allregions.V4,
+	}
+	addr4 = allregions.EdgeAddr{
+		TCP: &net.TCPAddr{
+			IP:   net.ParseIP("2606:4700:a0::1"),
+			Port: 8000,
+			Zone: "",
+		},
+		UDP: &net.UDPAddr{
+			IP:   net.ParseIP("2606:4700:a0::1"),
+			Port: 8000,
+			Zone: "",
+		},
+		IPVersion: allregions.V6,
+	}
+	addr5 = allregions.EdgeAddr{
+		TCP: &net.TCPAddr{
+			IP:   net.ParseIP("2606:4700:a0::2"),
+			Port: 8000,
+			Zone: "",
+		},
+		UDP: &net.UDPAddr{
+			IP:   net.ParseIP("2606:4700:a0::2"),
+			Port: 8000,
+			Zone: "",
+		},
+		IPVersion: allregions.V6,
+	}
+	addr6 = allregions.EdgeAddr{
+		TCP: &net.TCPAddr{
+			IP:   net.ParseIP("2606:4700:a0::3"),
+			Port: 8000,
+			Zone: "",
+		},
+		UDP: &net.UDPAddr{
+			IP:   net.ParseIP("2606:4700:a0::3"),
+			Port: 8000,
+			Zone: "",
+		},
+		IPVersion: allregions.V6,
+	}
+	addr7 = allregions.EdgeAddr{
+		TCP: &net.TCPAddr{
+			IP:   net.ParseIP("2606:4700:a0::4"),
+			Port: 8000,
+			Zone: "",
+		},
+		UDP: &net.UDPAddr{
+			IP:   net.ParseIP("2606:4700:a0::4"),
+			Port: 8000,
+			Zone: "",
+		},
+		IPVersion: allregions.V6,
 	}
-
-	testLogger = zerolog.Nop()
 )
 
 func TestGiveBack(t *testing.T) {
@@ -75,7 +132,7 @@ func TestGiveBack(t *testing.T) {
 	assert.Equal(t, 3, edge.AvailableAddrs())
 
 	// Get it back
-	edge.GiveBack(addr)
+	edge.GiveBack(addr, false)
 	assert.Equal(t, 4, edge.AvailableAddrs())
 }
 
@@ -107,7 +164,7 @@ func TestGetAddrForRPC(t *testing.T) {
 	assert.Equal(t, 4, edge.AvailableAddrs())
 
 	// Get it back
-	edge.GiveBack(addr)
+	edge.GiveBack(addr, false)
 	assert.Equal(t, 4, edge.AvailableAddrs())
 }
 
@@ -122,13 +179,13 @@ func TestOnePerRegion(t *testing.T) {
 	assert.NotNil(t, a1)
 
 	// if the first address is bad, get the second one
-	a2, err := edge.GetDifferentAddr(connID)
+	a2, err := edge.GetDifferentAddr(connID, false)
 	assert.NoError(t, err)
 	assert.NotNil(t, a2)
 	assert.NotEqual(t, a1, a2)
 
 	// now that second one is bad, get the first one again
-	a3, err := edge.GetDifferentAddr(connID)
+	a3, err := edge.GetDifferentAddr(connID, false)
 	assert.NoError(t, err)
 	assert.Equal(t, a1, a3)
 }
@@ -144,11 +201,11 @@ func TestOnlyOneAddrLeft(t *testing.T) {
 	assert.NotNil(t, addr)
 
 	// If that edge address is "bad", there's no alternative address.
-	_, err = edge.GetDifferentAddr(connID)
+	_, err = edge.GetDifferentAddr(connID, false)
 	assert.Error(t, err)
 
 	// previously bad address should become available again on next iteration.
-	addr, err = edge.GetDifferentAddr(connID)
+	addr, err = edge.GetDifferentAddr(connID, false)
 	assert.NoError(t, err)
 	assert.NotNil(t, addr)
 }
@@ -190,8 +247,17 @@ func TestGetDifferentAddr(t *testing.T) {
 	assert.Equal(t, 3, edge.AvailableAddrs())
 
 	// If the same connection requests another address, it should get the same one.
-	addr2, err := edge.GetDifferentAddr(connID)
+	addr2, err := edge.GetDifferentAddr(connID, false)
 	assert.NoError(t, err)
 	assert.NotEqual(t, addr, addr2)
 	assert.Equal(t, 3, edge.AvailableAddrs())
 }
+
+// MockEdge creates a Cloudflare Edge from arbitrary TCP addresses. Used for testing.
+func MockEdge(log *zerolog.Logger, addrs []*allregions.EdgeAddr) *Edge {
+	regions := allregions.NewNoResolve(addrs)
+	return &Edge{
+		log:     log,
+		regions: regions,
+	}
+}

supervisor/supervisor.go

@@ -7,7 +7,6 @@ import (
 	"time"
 
 	"github.com/google/uuid"
-	"github.com/lucas-clemente/quic-go"
 	"github.com/rs/zerolog"
 
 	"github.com/cloudflare/cloudflared/connection"
@@ -42,6 +41,7 @@ type Supervisor struct {
 	config            *TunnelConfig
 	orchestrator      *orchestration.Orchestrator
 	edgeIPs           *edgediscovery.Edge
+	edgeTunnelServer  EdgeTunnelServer
 	tunnelErrors      chan tunnelError
 	tunnelsConnecting map[int]chan struct{}
 	// nextConnectedIndex and nextConnectedSignal are used to wait for all
@@ -76,12 +76,34 @@ func NewSupervisor(config *TunnelConfig, orchestrator *orchestration.Orchestrato
 	if len(config.EdgeAddrs) > 0 {
 		edgeIPs, err = edgediscovery.StaticEdge(config.Log, config.EdgeAddrs)
 	} else {
-		edgeIPs, err = edgediscovery.ResolveEdge(config.Log, config.Region)
+		edgeIPs, err = edgediscovery.ResolveEdge(config.Log, config.Region, config.EdgeIPVersion)
 	}
 	if err != nil {
 		return nil, err
 	}
 
+	reconnectCredentialManager := newReconnectCredentialManager(connection.MetricsNamespace, connection.TunnelSubsystem, config.HAConnections)
+	log := NewConnAwareLogger(config.Log, config.Observer)
+
+	var edgeAddrHandler EdgeAddrHandler
+	if config.EdgeIPVersion == allregions.IPv6Only || config.EdgeIPVersion == allregions.Auto {
+		edgeAddrHandler = &IPAddrFallback{}
+	} else { // IPv4Only
+		edgeAddrHandler = &DefaultAddrFallback{}
+	}
+
+	edgeTunnelServer := EdgeTunnelServer{
+		config:            config,
+		cloudflaredUUID:   cloudflaredUUID,
+		orchestrator:      orchestrator,
+		credentialManager: reconnectCredentialManager,
+		edgeAddrs:         edgeIPs,
+		edgeAddrHandler:   edgeAddrHandler,
+		reconnectCh:       reconnectCh,
+		gracefulShutdownC: gracefulShutdownC,
+		connAwareLogger:   log,
+	}
+
 	useReconnectToken := false
 	if config.ClassicTunnel != nil {
 		useReconnectToken = config.ClassicTunnel.UseReconnectToken
@@ -92,11 +114,12 @@ func NewSupervisor(config *TunnelConfig, orchestrator *orchestration.Orchestrato
 		config:                     config,
 		orchestrator:               orchestrator,
 		edgeIPs:                    edgeIPs,
+		edgeTunnelServer:           edgeTunnelServer,
 		tunnelErrors:               make(chan tunnelError),
 		tunnelsConnecting:          map[int]chan struct{}{},
-		log:                        NewConnAwareLogger(config.Log, config.Observer),
+		log:                        log,
 		logTransport:               config.LogTransport,
-		reconnectCredentialManager: newReconnectCredentialManager(connection.MetricsNamespace, connection.TunnelSubsystem, config.HAConnections),
+		reconnectCredentialManager: reconnectCredentialManager,
 		useReconnectToken:          useReconnectToken,
 		reconnectCh:                reconnectCh,
 		gracefulShutdownC:          gracefulShutdownC,
@@ -143,11 +166,18 @@ func (s *Supervisor) Run(
 				tunnelsActive--
 			}
 			return nil
-		// startTunnel returned with error
+		// startTunnel completed with a response
 		// (note that this may also be caused by context cancellation)
 		case tunnelError := <-s.tunnelErrors:
 			tunnelsActive--
 			if tunnelError.err != nil && !shuttingDown {
+				switch tunnelError.err.(type) {
+				case ReconnectSignal:
+					// For tunnels that closed with reconnect signal, we reconnect immediately
+					go s.startTunnel(ctx, tunnelError.index, s.newConnectedTunnelSignal(tunnelError.index))
+					tunnelsActive++
+					continue
+				}
 				s.log.ConnAwareLogger().Err(tunnelError.err).Int(connection.LogFieldConnIndex, tunnelError.index).Msg("Connection terminated")
 				tunnelsWaiting = append(tunnelsWaiting, tunnelError.index)
 				s.waitForNextTunnel(tunnelError.index)
@@ -155,10 +185,9 @@ func (s *Supervisor) Run(
 				if backoffTimer == nil {
 					backoffTimer = backoff.BackoffTimer()
 				}
-
-				// Previously we'd mark the edge address as bad here, but now we'll just silently use another.
 			} else if tunnelsActive == 0 {
-				// all connected tunnels exited gracefully, no more work to do
+				s.log.ConnAwareLogger().Msg("no more connections active and exiting")
+				// All connected tunnels exited gracefully, no more work to do
 				return nil
 			}
 		// Backoff was set and its timer expired
@@ -192,6 +221,8 @@ func (s *Supervisor) Run(
 }
 
 // Returns nil if initialization succeeded, else the initialization error.
+// Attempts here will be made to connect one tunnel, if successful, it will
+// connect the available tunnels up to config.HAConnections.
 func (s *Supervisor) initialize(
 	ctx context.Context,
 	connectedSignal *signal.Signal,
@@ -203,6 +234,8 @@ func (s *Supervisor) initialize(
 	}
 
 	go s.startFirstTunnel(ctx, connectedSignal)
+
+	// Wait for response from first tunnel before proceeding to attempt other HA edge tunnels
 	select {
 	case <-ctx.Done():
 		<-s.tunnelErrors
@@ -213,6 +246,7 @@ func (s *Supervisor) initialize(
 		return errEarlyShutdown
 	case <-connectedSignal.Wait():
 	}
+
 	// At least one successful connection, so start the rest
 	for i := 1; i < s.config.HAConnections; i++ {
 		ch := signal.New(make(chan struct{}))
@@ -229,7 +263,6 @@ func (s *Supervisor) startFirstTunnel(
 	connectedSignal *signal.Signal,
 ) {
 	var (
-		addr *allregions.EdgeAddr
 		err error
 	)
 	const firstConnIndex = 0
@@ -237,94 +270,35 @@ func (s *Supervisor) startFirstTunnel(
 		s.tunnelErrors <- tunnelError{index: firstConnIndex, err: err}
 	}()
 
-	addr, err = s.edgeIPs.GetAddr(firstConnIndex)
+	err = s.edgeTunnelServer.Serve(ctx, firstConnIndex, connectedSignal)
-	if err != nil {
-		return
-	}
 
-	err = ServeTunnelLoop(
-		ctx,
-		s.reconnectCredentialManager,
-		s.config,
-		s.orchestrator,
-		addr,
-		s.log,
-		firstConnIndex,
-		connectedSignal,
-		s.cloudflaredUUID,
-		s.reconnectCh,
-		s.gracefulShutdownC,
-	)
 	// If the first tunnel disconnects, keep restarting it.
-	edgeErrors := 0
 	for s.unusedIPs() {
 		if ctx.Err() != nil {
 			return
 		}
-		switch err.(type) {
+		if err == nil {
-		case nil:
-			return
-		// try the next address if it was a quic.IdleTimeoutError, dialError(network problem) or
-		// dupConnRegisterTunnelError
-		case *quic.IdleTimeoutError, edgediscovery.DialError, connection.DupConnRegisterTunnelError:
-			edgeErrors++
-		default:
 			return
 		}
-		if edgeErrors >= 2 {
+		err = s.edgeTunnelServer.Serve(ctx, firstConnIndex, connectedSignal)
-			addr, err = s.edgeIPs.GetDifferentAddr(firstConnIndex)
-			if err != nil {
-				return
-			}
-		}
-		err = ServeTunnelLoop(
-			ctx,
-			s.reconnectCredentialManager,
-			s.config,
-			s.orchestrator,
-			addr,
-			s.log,
-			firstConnIndex,
-			connectedSignal,
-			s.cloudflaredUUID,
-			s.reconnectCh,
-			s.gracefulShutdownC,
-		)
 	}
 }
 
 // startTunnel starts a new tunnel connection. The resulting error will be sent on
-// s.tunnelErrors.
+// s.tunnelError as this is expected to run in a goroutine.
 func (s *Supervisor) startTunnel(
 	ctx context.Context,
 	index int,
 	connectedSignal *signal.Signal,
 ) {
 	var (
-		addr *allregions.EdgeAddr
 		err error
 	)
 	defer func() {
 		s.tunnelErrors <- tunnelError{index: index, err: err}
 	}()
 
-	addr, err = s.edgeIPs.GetDifferentAddr(index)
+	err = s.edgeTunnelServer.Serve(ctx, uint8(index), connectedSignal)
-	if err != nil {
-		return
-	}
-	err = ServeTunnelLoop(
-		ctx,
-		s.reconnectCredentialManager,
-		s.config,
-		s.orchestrator,
-		addr,
-		s.log,
-		uint8(index),
-		connectedSignal,
-		s.cloudflaredUUID,
-		s.reconnectCh,
-		s.gracefulShutdownC,
-	)
 }
 
 func (s *Supervisor) newConnectedTunnelSignal(index int) *signal.Signal {

supervisor/tunnel.go

@@ -122,28 +122,84 @@ func StartTunnelDaemon(
 	return s.Run(ctx, connectedSignal)
 }
 
-func ServeTunnelLoop(
+// EdgeAddrHandler provides a mechanism switch between behaviors in ServeTunnel
-	ctx context.Context,
+// for handling the errors when attempting to make edge connections.
-	credentialManager *reconnectCredentialManager,
+type EdgeAddrHandler interface {
-	config *TunnelConfig,
+	// ShouldGetNewAddress will check the edge connection error and determine if
-	orchestrator *orchestration.Orchestrator,
+	// the edge address should be replaced with a new one. Also, will return if the
-	addr *allregions.EdgeAddr,
+	// error should be recognized as a connectivity error, or otherwise, a general
-	connAwareLogger *ConnAwareLogger,
+	// application error.
-	connIndex uint8,
+	ShouldGetNewAddress(err error) (needsNewAddress bool, isConnectivityError bool)
-	connectedSignal *signal.Signal,
+}
-	cloudflaredUUID uuid.UUID,
+
-	reconnectCh chan ReconnectSignal,
+// DefaultAddrFallback will always return false for isConnectivityError since this
-	gracefulShutdownC <-chan struct{},
+// handler is a way to provide the legacy behavior in the new edge discovery algorithm.
-) error {
+type DefaultAddrFallback struct {
+	edgeErrors int
+}
+
+func (f DefaultAddrFallback) ShouldGetNewAddress(err error) (needsNewAddress bool, isConnectivityError bool) {
+	switch err.(type) {
+	case nil: // maintain current IP address
+	// Try the next address if it was a quic.IdleTimeoutError or
+	// dupConnRegisterTunnelError
+	case *quic.IdleTimeoutError,
+		connection.DupConnRegisterTunnelError,
+		edgediscovery.DialError,
+		*connection.EdgeQuicDialError:
+		// Wait for two failures before falling back to a new address
+		f.edgeErrors++
+		if f.edgeErrors >= 2 {
+			f.edgeErrors = 0
+			return true, false
+		}
+	default: // maintain current IP address
+	}
+	return false, false
+}
+
+// IPAddrFallback will have more conditions to fall back to a new address for certain
+// edge connection errors. This means that this handler will return true for isConnectivityError
+// for more cases like duplicate connection register and edge quic dial errors.
+type IPAddrFallback struct{}
+
+func (f IPAddrFallback) ShouldGetNewAddress(err error) (needsNewAddress bool, isConnectivityError bool) {
+	switch err.(type) {
+	case nil: // maintain current IP address
+	// Try the next address if it was a quic.IdleTimeoutError
+	// DupConnRegisterTunnelError needs to also receive a new ip address
+	case connection.DupConnRegisterTunnelError,
+		*quic.IdleTimeoutError:
+		return true, false
+	// Network problems should be retried with new address immediately and report
+	// as connectivity error
+	case edgediscovery.DialError, *connection.EdgeQuicDialError:
+		return true, true
+	default: // maintain current IP address
+	}
+	return false, false
+}
+
+type EdgeTunnelServer struct {
+	config            *TunnelConfig
+	cloudflaredUUID   uuid.UUID
+	orchestrator      *orchestration.Orchestrator
+	credentialManager *reconnectCredentialManager
+	edgeAddrHandler   EdgeAddrHandler
+	edgeAddrs         *edgediscovery.Edge
+	reconnectCh       chan ReconnectSignal
+	gracefulShutdownC <-chan struct{}
+
+	connAwareLogger *ConnAwareLogger
+}
+
+func (e EdgeTunnelServer) Serve(ctx context.Context, connIndex uint8, connectedSignal *signal.Signal) error {
 	haConnections.Inc()
 	defer haConnections.Dec()
 
-	logger := config.Log.With().Uint8(connection.LogFieldConnIndex, connIndex).Logger()
-	connLog := connAwareLogger.ReplaceLogger(&logger)
-
 	protocolFallback := &protocolFallback{
-		retry.BackoffHandler{MaxRetries: config.Retries},
+		retry.BackoffHandler{MaxRetries: e.config.Retries},
-		config.ProtocolSelector.Current(),
+		e.config.ProtocolSelector.Current(),
 		false,
 	}
 	connectedFuse := h2mux.NewBooleanFuse()
@@ -154,38 +210,64 @@ func ServeTunnelLoop(
 	}()
 	// Ensure the above goroutine will terminate if we return without connecting
 	defer connectedFuse.Fuse(false)
+
+	// Fetch IP address to associated connection index
+	addr, err := e.edgeAddrs.GetAddr(int(connIndex))
+	switch err {
+	case nil: // no error
+	case edgediscovery.ErrNoAddressesLeft:
+		return err
+	default:
+		return err
+	}
+
+	logger := e.config.Log.With().
+		IPAddr(connection.LogFieldIPAddress, addr.UDP.IP).
+		Uint8(connection.LogFieldConnIndex, connIndex).
+		Logger()
+	connLog := e.connAwareLogger.ReplaceLogger(&logger)
 	// Each connection to keep its own copy of protocol, because individual connections might fallback
 	// to another protocol when a particular metal doesn't support new protocol
-	for {
+	// Each connection can also have it's own IP version because individual connections might fallback
+	// to another IP version.
 	err, recoverable := ServeTunnel(
 		ctx,
 		connLog,
-			credentialManager,
+		e.credentialManager,
-			config,
+		e.config,
-			orchestrator,
+		e.orchestrator,
 		addr,
 		connIndex,
 		connectedFuse,
 		protocolFallback,
-			cloudflaredUUID,
+		e.cloudflaredUUID,
-			reconnectCh,
+		e.reconnectCh,
 		protocolFallback.protocol,
-			gracefulShutdownC,
+		e.gracefulShutdownC,
 	)
 
+	// If the connection is recoverable, we want to maintain the same IP
+	// but backoff a reconnect with some duration.
 	if recoverable {
 		duration, ok := protocolFallback.GetMaxBackoffDuration(ctx)
 		if !ok {
 			return err
 		}
-			config.Observer.SendReconnect(connIndex)
+		e.config.Observer.SendReconnect(connIndex)
 		connLog.Logger().Info().Msgf("Retrying connection in up to %s seconds", duration)
 	}
 
+	// Check if the connection error was from an IP issue with the host or
+	// establishing a connection to the edge and if so, rotate the IP address.
+	yes, hasConnectivityError := e.edgeAddrHandler.ShouldGetNewAddress(err)
+	if yes {
+		e.edgeAddrs.GetDifferentAddr(int(connIndex), hasConnectivityError)
+	}
+
 	select {
 	case <-ctx.Done():
 		return ctx.Err()
-		case <-gracefulShutdownC:
+	case <-e.gracefulShutdownC:
 		return nil
 	case <-protocolFallback.BackoffTimer():
 		if !recoverable {
@@ -195,13 +277,14 @@ func ServeTunnelLoop(
 		if !selectNextProtocol(
 			connLog.Logger(),
 			protocolFallback,
-				config.ProtocolSelector,
+			e.config.ProtocolSelector,
 			err,
 		) {
 			return err
 		}
 	}
-	}
+
+	return err
 }
 
 // protocolFallback is a wrapper around backoffHandler that will try fallback option when backoff reaches
@@ -233,6 +316,10 @@ func selectNextProtocol(
 ) bool {
 	var idleTimeoutError *quic.IdleTimeoutError
 	isNetworkActivityTimeout := errors.As(cause, &idleTimeoutError)
+	edgeQuicDialError, ok := cause.(*connection.EdgeQuicDialError)
+	if !isNetworkActivityTimeout && ok {
+		isNetworkActivityTimeout = errors.As(edgeQuicDialError.Cause, &idleTimeoutError)
+	}
 	_, hasFallback := selector.Fallback()
 
 	if protocolBackoff.ReachedMaxRetries() || (hasFallback && isNetworkActivityTimeout) {
@@ -241,7 +328,7 @@ func selectNextProtocol(
 				"Cloudflare Network with `quic` protocol, then most likely your machine/network is getting its egress " +
 				"UDP to port 7844 (or others) blocked or dropped. Make sure to allow egress connectivity as per " +
 				"https://developers.cloudflare.com/cloudflare-one/connections/connect-apps/configuration/ports-and-ips/\n" +
-				"If you are using private routing to this Tunnel, then UDP (and Private DNS Resolution) will not work" +
+				"If you are using private routing to this Tunnel, then UDP (and Private DNS Resolution) will not work " +
 				"unless your cloudflared can connect with Cloudflare Network with `quic`.")
 		}
 
@@ -326,8 +413,12 @@ func ServeTunnel(
 				connLog.ConnAwareLogger().Msg(activeIncidentsMsg(incidents))
 			}
 			return err.Cause, !err.Permanent
+		case *connection.EdgeQuicDialError:
+			// Don't retry connection for a dial error
+			return err, false
 		case ReconnectSignal:
 			connLog.Logger().Info().
+				IPAddr(connection.LogFieldIPAddress, addr.UDP.IP).
 				Uint8(connection.LogFieldConnIndex, connIndex).
 				Msgf("Restarting connection due to reconnect signal in %s", err.Delay)
 			err.DelayBeforeReconnect()
@@ -526,6 +617,7 @@ func ServeHTTP2(
 		err := listenReconnect(serveCtx, reconnectCh, gracefulShutdownC)
 		if err != nil {
 			// forcefully break the connection (this is only used for testing)
+			connLog.Logger().Debug().Msg("Forcefully breaking http2 connection")
 			_ = tlsServerConn.Close()
 		}
 		return err
@@ -584,6 +676,7 @@ func ServeQUIC(
 		err := listenReconnect(serveCtx, reconnectCh, gracefulShutdownC)
 		if err != nil {
 			// forcefully break the connection (this is only used for testing)
+			connLogger.Logger().Debug().Msg("Forcefully breaking quic connection")
 			quicConn.Close()
 		}
 		return err