Pull request: 3166 fix dhcp

Updates #3166.

Squashed commit of the following:

commit cac62f842886f06a70fb50d251938e0feaa06846
Author: Ildar Kamalov <ik@adguard.com>
Date:   Thu Jun 10 14:35:40 2021 +0300

    client: get dhcp status on static lease submit

commit 2ae4e3ab40041fbc0f53f4902a00263425409a6a
Merge: 1e0f9c15 299ea88c
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Jun 10 14:09:05 2021 +0300

    Merge branch 'master' into 3166-fix-dhcp

commit 1e0f9c15d98f40a499c75b74bca48fe58512e300
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Jun 10 14:03:58 2021 +0300

    dhcpd: imp code

commit e4b9626b60a11cd2491c675a297f527f7e12f9b4
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Thu Jun 10 13:03:47 2021 +0300

    dhcpd: fix static lease load

commit dc449f21986a8ea300d07c081f1e416b2f08bc25
Author: Eugene Burkov <e.burkov@adguard.com>
Date:   Tue Jun 1 17:21:10 2021 +0300

    home: change clients sources priority

CHANGELOG.md

@@ -47,6 +47,8 @@ released by then.
 
 ### Fixed
 
+- Redundant hostname generating while loading static leases with empty hostname
+  ([#3166]).
 - Domain name case in responses ([#3194]).
 - Custom upstreams selection for clients with client IDs in DNS-over-TLS and
   DNS-over-HTTP ([#3186]).
@@ -61,6 +63,7 @@ released by then.
 [#2443]: https://github.com/AdguardTeam/AdGuardHome/issues/2443
 [#2763]: https://github.com/AdguardTeam/AdGuardHome/issues/2763
 [#3136]: https://github.com/AdguardTeam/AdGuardHome/issues/3136
+[#3166]: https://github.com/AdguardTeam/AdGuardHome/issues/3166
 [#3172]: https://github.com/AdguardTeam/AdGuardHome/issues/3172
 [#3184]: https://github.com/AdguardTeam/AdGuardHome/issues/3184
 [#3185]: https://github.com/AdguardTeam/AdGuardHome/issues/3185

client/src/actions/index.js

@@ -561,6 +561,7 @@ export const addStaticLease = (config) => async (dispatch) => {
         dispatch(addStaticLeaseSuccess(config));
         dispatch(addSuccessToast(i18next.t('dhcp_lease_added', { key: name })));
         dispatch(toggleLeaseModal());
+        dispatch(getDhcpStatus());
     } catch (error) {
         dispatch(addErrorToast({ error }));
         dispatch(addStaticLeaseFailure());

internal/aghnet/addr.go

@@ -127,7 +127,7 @@ func ValidateDomainName(name string) (err error) {
 
 // The maximum lengths of generated hostnames for different IP versions.
 const (
-	ipv4HostnameMaxLen = len("192-168-100-10-")
+	ipv4HostnameMaxLen = len("192-168-100-100")
 	ipv6HostnameMaxLen = len("ff80-f076-0000-0000-0000-0000-0000-0010")
 )
 

internal/dhcpd/v4.go

@@ -110,7 +110,9 @@ func (s *v4Server) ResetLeases(leases []*Lease) {
 	s.leases = nil
 
 	for _, l := range leases {
-		l.Hostname = s.validHostnameForClient(l.Hostname, l.IP)
+		if !l.IsStatic() {
+			l.Hostname = s.validHostnameForClient(l.Hostname, l.IP)
+		}
 		err = s.addLease(l)
 		if err != nil {
 			// TODO(a.garipov): Wrap and bubble up the error.
@@ -339,9 +341,8 @@ func (s *v4Server) AddStaticLease(l Lease) (err error) {
 		return err
 	}
 
-	if l.Hostname != "" {
+	if hostname := l.Hostname; hostname != "" {
-		var hostname string
+		hostname, err = normalizeHostname(hostname)
-		hostname, err = normalizeHostname(l.Hostname)
 		if err != nil {
 			return err
 		}
@@ -635,6 +636,33 @@ func (o *optFQDN) ToBytes() []byte {
 	return b
 }
 
+// checkLease checks if the pair of mac and ip is already leased.  The mismatch
+// is true when the existing lease has the same hardware address but differs in
+// its IP address.
+func (s *v4Server) checkLease(mac net.HardwareAddr, ip net.IP) (lease *Lease, mismatch bool) {
+	s.leasesLock.Lock()
+	defer s.leasesLock.Unlock()
+
+	for _, l := range s.leases {
+		if !bytes.Equal(l.HWAddr, mac) {
+			continue
+		}
+
+		if l.IP.Equal(ip) {
+			return l, false
+		}
+
+		log.Debug(
+			`dhcpv4: mismatched OptionRequestedIPAddress in req msg for %s`,
+			mac,
+		)
+
+		return nil, true
+	}
+
+	return nil, false
+}
+
 // processRequest is the handler for the DHCP Request request.
 func (s *v4Server) processRequest(req, resp *dhcpv4.DHCPv4) (lease *Lease, needsReply bool) {
 	mac := req.ClientHWAddr
@@ -656,33 +684,8 @@ func (s *v4Server) processRequest(req, resp *dhcpv4.DHCPv4) (lease *Lease, needs
 		return nil, false
 	}
 
-	mismatch := false
+	var mismatch bool
-	func() {
+	if lease, mismatch = s.checkLease(mac, reqIP); mismatch {
-		s.leasesLock.Lock()
-		defer s.leasesLock.Unlock()
-
-		for _, l := range s.leases {
-			if !bytes.Equal(l.HWAddr, mac) {
-				continue
-			}
-
-			if l.IP.Equal(reqIP) {
-				lease = l
-
-				return
-			}
-
-			log.Debug(
-				`dhcpv4: mismatched OptionRequestedIPAddress in req msg for %s`,
-				mac,
-			)
-
-			mismatch = true
-
-			return
-		}
-	}()
-	if mismatch {
 		return nil, true
 	}
 

internal/home/clients.go

@@ -53,13 +53,11 @@ type Client struct {
 type clientSource uint
 
 // Client sources.  The order determines the priority.
-//
-// TODO(e.burkov): Is ARP a higher priority source than DHCP?
 const (
 	ClientSourceWHOIS clientSource = iota
 	ClientSourceRDNS
-	ClientSourceDHCP
 	ClientSourceARP
+	ClientSourceDHCP
 	ClientSourceHostsFile
 )
 

internal/home/clients_test.go

@@ -151,6 +151,20 @@ func TestClients(t *testing.T) {
 		assert.True(t, clients.Exists("1.1.1.1", ClientSourceHostsFile))
 	})
 
+	t.Run("dhcp_replaces_arp", func(t *testing.T) {
+		ok, err := clients.AddHost("1.2.3.4", "from_arp", ClientSourceARP)
+		require.NoError(t, err)
+		assert.True(t, ok)
+
+		assert.True(t, clients.Exists("1.2.3.4", ClientSourceARP))
+
+		ok, err = clients.AddHost("1.2.3.4", "from_dhcp", ClientSourceDHCP)
+		require.NoError(t, err)
+		assert.True(t, ok)
+
+		assert.True(t, clients.Exists("1.2.3.4", ClientSourceDHCP))
+	})
+
 	t.Run("addhost_fail", func(t *testing.T) {
 		ok, err := clients.AddHost("1.1.1.1", "host1", ClientSourceRDNS)
 		require.NoError(t, err)