From cebbb69a4c8803a670a117fee0bc79075f101e5a Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Mon, 24 Oct 2022 17:49:52 +0300 Subject: [PATCH] Pull request: 5035-netip-maps-clients Updates #5035. Squashed commit of the following: commit c2d38fe75b8aa2f00b19892724984ed3bb843db5 Author: Ainar Garipov Date: Mon Oct 24 16:50:14 2022 +0300 home: move clients to netip.Addr --- internal/dnsforward/dnsforward.go | 4 +- internal/home/clients.go | 67 ++++++++++++++++--------------- internal/home/clients_test.go | 30 +++++--------- internal/home/clientshttp.go | 16 ++------ internal/home/rdns_test.go | 5 ++- 5 files changed, 53 insertions(+), 69 deletions(-) diff --git a/internal/dnsforward/dnsforward.go b/internal/dnsforward/dnsforward.go index 914ec0a9..f6507b03 100644 --- a/internal/dnsforward/dnsforward.go +++ b/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, rule + return blocked, aghalg.Coalesce(rule, clientID) } diff --git a/internal/home/clients.go b/internal/home/clients.go index 28b6eccb..bb1a3210 100644 --- a/internal/home/clients.go +++ b/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. - // - // TODO(e.burkov): Use map[netip.Addr]struct{} instead. - ipToRC *netutil.IPMap + ipToRC map[netip.Addr]*RuntimeClient 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 - v, ok = clients.ipToRC.Get(ip) - if !ok { - return nil, false - } - - rc, ok = v.(*RuntimeClient) - if !ok { - log.Error("clients: bad type %T in ipToRC for %s", v, ip) + // 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 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 + } + + clients.ipToRC[ipAddr] = rc } - log.Debug("clients: added %s -> %q [%d]", ip, host, clients.ipToRC.Len()) + 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) { - rc, ok := v.(*RuntimeClient) - if !ok { - log.Error("clients: bad type %T in ipToRC for %s", v, ip) - - return true - } - + for ip, rc := range clients.ipToRC { 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 } diff --git a/internal/home/clients_test.go b/internal/home/clients_test.go index 00148c26..407b2513 100644 --- a/internal/home/clients_test.go +++ b/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} - clients.SetWHOISInfo(ip, whois) - v, _ := clients.ipToRC.Get(ip) - require.NotNil(t, v) - - rc, ok := v.(*RuntimeClient) - require.True(t, ok) + ip := netip.MustParseAddr("1.1.1.255") + clients.SetWHOISInfo(ip.AsSlice(), whois) + rc := clients.ipToRC[ip] 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} - ok, err := clients.AddHost(ip, "host", ClientSourceRDNS) + ip := netip.MustParseAddr("1.1.1.1") + ok, err := clients.AddHost(ip.AsSlice(), "host", ClientSourceRDNS) require.NoError(t, err) assert.True(t, ok) - clients.SetWHOISInfo(ip, whois) - v, _ := clients.ipToRC.Get(ip) - require.NotNil(t, v) - - rc, ok := v.(*RuntimeClient) - require.True(t, ok) + clients.SetWHOISInfo(ip.AsSlice(), whois) + rc := clients.ipToRC[ip] 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) - v, _ := clients.ipToRC.Get(ip) - require.Nil(t, v) + clients.SetWHOISInfo(ip.AsSlice(), whois) + rc := clients.ipToRC[ip] + require.Nil(t, rc) assert.True(t, clients.Del("client1")) }) diff --git a/internal/home/clientshttp.go b/internal/home/clientshttp.go index 4e4d22f2..30b0608e 100644 --- a/internal/home/clientshttp.go +++ b/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) { - rc, ok := v.(*RuntimeClient) - if !ok { - log.Error("dns: bad type %T in ipToRC for %s", v, ip) - - return true - } - + for ip, rc := range clients.ipToRC { 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 diff --git a/internal/home/rdns_test.go b/internal/home/rdns_test.go index 870d0f04..3f6ce4c7 100644 --- a/internal/home/rdns_test.go +++ b/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)