Pull request: 5035-netip-maps-clients

Updates #5035.

Squashed commit of the following:

commit c2d38fe75b8aa2f00b19892724984ed3bb843db5
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Oct 24 16:50:14 2022 +0300

    home: move clients to netip.Addr

internal/dnsforward/dnsforward.go

@@ -706,7 +706,5 @@ func (s *Server) IsBlockedClient(ip net.IP, clientID string) (blocked bool, rule
 		blocked = true
 	}
 
-	rule = aghalg.Coalesce(rule, clientID)
+	return blocked, aghalg.Coalesce(rule, clientID)
-
-	return blocked, rule
 }

internal/home/clients.go

@@ -5,6 +5,7 @@ import (
 	"encoding"
 	"fmt"
 	"net"
+	"net/netip"
 	"sort"
 	"strings"
 	"sync"
@@ -25,8 +26,6 @@ import (
 	"golang.org/x/exp/slices"
 )
 
-//lint:file-ignore SA1019 TODO(a.garipov):  Replace [*netutil.IPMap].
-
 const clientsUpdatePeriod = 10 * time.Minute
 
 var webHandlersRegistered = false
@@ -135,9 +134,7 @@ type clientsContainer struct {
 	idIndex map[string]*Client // ID -> client
 
 	// ipToRC is the IP address to *RuntimeClient map.
-	//
+	ipToRC map[netip.Addr]*RuntimeClient
-	// TODO(e.burkov):  Use map[netip.Addr]struct{} instead.
-	ipToRC *netutil.IPMap
 
 	lock sync.Mutex
 
@@ -173,7 +170,7 @@ func (clients *clientsContainer) Init(
 	}
 	clients.list = make(map[string]*Client)
 	clients.idIndex = make(map[string]*Client)
-	clients.ipToRC = netutil.NewIPMap(0)
+	clients.ipToRC = map[netip.Addr]*RuntimeClient{}
 
 	clients.allTags = stringutil.NewSet(clientTags...)
 
@@ -541,20 +538,17 @@ func (clients *clientsContainer) findLocked(id string) (c *Client, ok bool) {
 // findRuntimeClientLocked finds a runtime client by their IP address.  For
 // internal use only.
 func (clients *clientsContainer) findRuntimeClientLocked(ip net.IP) (rc *RuntimeClient, ok bool) {
-	var v any
+	// TODO(a.garipov):  Remove once we switch to netip.Addr more fully.
-	v, ok = clients.ipToRC.Get(ip)
+	ipAddr, err := netutil.IPToAddrNoMapped(ip)
-	if !ok {
+	if err != nil {
-		return nil, false
+		log.Error("clients: bad client ip %v: %s", ip, err)
-	}
-
-	rc, ok = v.(*RuntimeClient)
-	if !ok {
-		log.Error("clients: bad type %T in ipToRC for %s", v, ip)
 
 		return nil, false
 	}
 
-	return rc, true
+	rc, ok = clients.ipToRC[ipAddr]
+
+	return rc, ok
 }
 
 // FindRuntimeClient finds a runtime client by their IP.
@@ -782,7 +776,15 @@ func (clients *clientsContainer) SetWHOISInfo(ip net.IP, wi *RuntimeClientWHOISI
 
 	rc.WHOISInfo = wi
 
-	clients.ipToRC.Set(ip, rc)
+	// TODO(a.garipov):  Remove once we switch to netip.Addr more fully.
+	ipAddr, err := netutil.IPToAddrNoMapped(ip)
+	if err != nil {
+		log.Error("clients: bad client ip %v: %s", ip, err)
+
+		return
+	}
+
+	clients.ipToRC[ipAddr] = rc
 
 	log.Debug("clients: set whois info for runtime client with ip %s: %+v", ip, wi)
 }
@@ -813,10 +815,18 @@ func (clients *clientsContainer) addHostLocked(ip net.IP, host string, src clien
 			WHOISInfo: &RuntimeClientWHOISInfo{},
 		}
 
-		clients.ipToRC.Set(ip, rc)
+		// TODO(a.garipov):  Remove once we switch to netip.Addr more fully.
+		ipAddr, err := netutil.IPToAddrNoMapped(ip)
+		if err != nil {
+			log.Error("clients: bad client ip %v: %s", ip, err)
+
+			return false
 		}
 
-	log.Debug("clients: added %s -> %q [%d]", ip, host, clients.ipToRC.Len())
+		clients.ipToRC[ipAddr] = rc
+	}
+
+	log.Debug("clients: added %s -> %q [%d]", ip, host, len(clients.ipToRC))
 
 	return true
 }
@@ -824,27 +834,20 @@ func (clients *clientsContainer) addHostLocked(ip net.IP, host string, src clien
 // rmHostsBySrc removes all entries that match the specified source.
 func (clients *clientsContainer) rmHostsBySrc(src clientSource) {
 	n := 0
-	clients.ipToRC.Range(func(ip net.IP, v any) (cont bool) {
+	for ip, rc := range clients.ipToRC {
-		rc, ok := v.(*RuntimeClient)
-		if !ok {
-			log.Error("clients: bad type %T in ipToRC for %s", v, ip)
-
-			return true
-		}
-
 		if rc.Source == src {
-			clients.ipToRC.Del(ip)
+			delete(clients.ipToRC, ip)
 			n++
 		}
-
+	}
-		return true
-	})
 
 	log.Debug("clients: removed %d client aliases", n)
 }
 
 // addFromHostsFile fills the client-hostname pairing index from the system's
 // hosts files.
+//
+//lint:ignore SA1019 TODO(a.garipov):  Replace [*netutil.IPMap].
 func (clients *clientsContainer) addFromHostsFile(hosts *netutil.IPMap) {
 	clients.lock.Lock()
 	defer clients.lock.Unlock()
@@ -855,7 +858,7 @@ func (clients *clientsContainer) addFromHostsFile(hosts *netutil.IPMap) {
 	hosts.Range(func(ip net.IP, v any) (cont bool) {
 		rec, ok := v.(*aghnet.HostsRecord)
 		if !ok {
-			log.Error("dns: bad type %T in ipToRC for %s", v, ip)
+			log.Error("clients: bad type %T in hosts for %s", v, ip)
 
 			return true
 		}

internal/home/clients_test.go

@@ -202,38 +202,30 @@ func TestClientsWHOIS(t *testing.T) {
 	}
 
 	t.Run("new_client", func(t *testing.T) {
-		ip := net.IP{1, 1, 1, 255}
+		ip := netip.MustParseAddr("1.1.1.255")
-		clients.SetWHOISInfo(ip, whois)
+		clients.SetWHOISInfo(ip.AsSlice(), whois)
-		v, _ := clients.ipToRC.Get(ip)
+		rc := clients.ipToRC[ip]
-		require.NotNil(t, v)
-
-		rc, ok := v.(*RuntimeClient)
-		require.True(t, ok)
 		require.NotNil(t, rc)
 
 		assert.Equal(t, rc.WHOISInfo, whois)
 	})
 
 	t.Run("existing_auto-client", func(t *testing.T) {
-		ip := net.IP{1, 1, 1, 1}
+		ip := netip.MustParseAddr("1.1.1.1")
-		ok, err := clients.AddHost(ip, "host", ClientSourceRDNS)
+		ok, err := clients.AddHost(ip.AsSlice(), "host", ClientSourceRDNS)
 		require.NoError(t, err)
 
 		assert.True(t, ok)
 
-		clients.SetWHOISInfo(ip, whois)
+		clients.SetWHOISInfo(ip.AsSlice(), whois)
-		v, _ := clients.ipToRC.Get(ip)
+		rc := clients.ipToRC[ip]
-		require.NotNil(t, v)
-
-		rc, ok := v.(*RuntimeClient)
-		require.True(t, ok)
 		require.NotNil(t, rc)
 
 		assert.Equal(t, rc.WHOISInfo, whois)
 	})
 
 	t.Run("can't_set_manually-added", func(t *testing.T) {
-		ip := net.IP{1, 1, 1, 2}
+		ip := netip.MustParseAddr("1.1.1.2")
 
 		ok, err := clients.Add(&Client{
 			IDs:  []string{"1.1.1.2"},
@@ -242,9 +234,9 @@ func TestClientsWHOIS(t *testing.T) {
 		require.NoError(t, err)
 		assert.True(t, ok)
 
-		clients.SetWHOISInfo(ip, whois)
+		clients.SetWHOISInfo(ip.AsSlice(), whois)
-		v, _ := clients.ipToRC.Get(ip)
+		rc := clients.ipToRC[ip]
-		require.Nil(t, v)
+		require.Nil(t, rc)
 
 		assert.True(t, clients.Del("client1"))
 	})

internal/home/clientshttp.go

@@ -7,7 +7,6 @@ import (
 	"net/http"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
-	"github.com/AdguardTeam/golibs/log"
 )
 
 // clientJSON is a common structure used by several handlers to deal with
@@ -70,26 +69,17 @@ func (clients *clientsContainer) handleGetClients(w http.ResponseWriter, r *http
 		data.Clients = append(data.Clients, cj)
 	}
 
-	clients.ipToRC.Range(func(ip net.IP, v any) (cont bool) {
+	for ip, rc := range clients.ipToRC {
-		rc, ok := v.(*RuntimeClient)
-		if !ok {
-			log.Error("dns: bad type %T in ipToRC for %s", v, ip)
-
-			return true
-		}
-
 		cj := runtimeClientJSON{
 			WHOISInfo: rc.WHOISInfo,
 
 			Name:   rc.Host,
 			Source: rc.Source,
-			IP:     ip,
+			IP:     ip.AsSlice(),
 		}
 
 		data.RuntimeClients = append(data.RuntimeClients, cj)
-
+	}
-		return true
-	})
 
 	data.Tags = clientTags
 

internal/home/rdns_test.go

@@ -5,6 +5,7 @@ import (
 	"encoding/binary"
 	"fmt"
 	"net"
+	"net/netip"
 	"sync"
 	"testing"
 	"time"
@@ -88,7 +89,7 @@ func TestRDNS_Begin(t *testing.T) {
 			clients: &clientsContainer{
 				list:    map[string]*Client{},
 				idIndex: tc.cliIDIndex,
-				ipToRC:  netutil.NewIPMap(0),
+				ipToRC:  map[netip.Addr]*RuntimeClient{},
 				allTags: stringutil.NewSet(),
 			},
 		}
@@ -228,7 +229,7 @@ func TestRDNS_WorkerLoop(t *testing.T) {
 		cc := &clientsContainer{
 			list:    map[string]*Client{},
 			idIndex: map[string]*Client{},
-			ipToRC:  netutil.NewIPMap(0),
+			ipToRC:  map[netip.Addr]*RuntimeClient{},
 			allTags: stringutil.NewSet(),
 		}
 		ch := make(chan net.IP)