From 12ee287d0b25cc1fb498e54c4484c995613e9bcb Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Tue, 19 Apr 2022 15:01:49 +0300 Subject: [PATCH] Pull request: 3157 excessive ptrs Merge in DNS/adguard-home from 3157-excessive-ptrs to master Updates #3157. Squashed commit of the following: commit 6803988240dca2f147bb80a5b3f78d7749d2fa14 Author: Eugene Burkov Date: Tue Apr 19 14:50:01 2022 +0300 aghnet: and again commit 1a7f4d1dbc8fd4d3ae620349917526a75fa71b47 Author: Eugene Burkov Date: Tue Apr 19 14:49:20 2022 +0300 aghnet: docs again commit d88da1fc7135f3cd03aff10b02d9957c8ffdfd30 Author: Eugene Burkov Date: Tue Apr 19 14:47:36 2022 +0300 aghnet: imp docs commit c45dbc7800e882c6c4110aab640c32b03046f89a Author: Eugene Burkov Date: Tue Apr 19 14:41:19 2022 +0300 aghnet: keep alphabetical order commit b61781785d096ef43f60fb4f1905a4ed3cdf7c68 Author: Eugene Burkov Date: Tue Apr 19 13:50:56 2022 +0300 aghnet: imp code quality commit 578dbd71ed2f2089c69343d7d4bf8bbc29150ace Author: Eugene Burkov Date: Tue Apr 12 17:02:38 2022 +0300 aghnet: imp arp container --- CHANGELOG.md | 5 +++++ internal/aghnet/arpdb_bsd.go | 13 +++++++++---- internal/aghnet/arpdb_linux.go | 19 ++++++++++++------- internal/aghnet/arpdb_openbsd.go | 15 ++++++++++----- internal/aghnet/arpdb_windows.go | 2 +- internal/dnsforward/dns.go | 1 - internal/dnsforward/recursiondetector_test.go | 14 ++++---------- internal/home/rdns.go | 11 +++++------ 8 files changed, 46 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a80b4ea1..9d82fb57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,10 +87,15 @@ In this release, the schema version has changed from 12 to 13. - Go 1.17 support. v0.109.0 will require at least Go 1.18 to build. +### Fixed + +- ARP tables refreshing process causing excessive PTR requests ([#3157]). + [#1730]: https://github.com/AdguardTeam/AdGuardHome/issues/1730 [#2993]: https://github.com/AdguardTeam/AdGuardHome/issues/2993 [#3057]: https://github.com/AdguardTeam/AdGuardHome/issues/3057 [#3142]: https://github.com/AdguardTeam/AdGuardHome/issues/3142 +[#3157]: https://github.com/AdguardTeam/AdGuardHome/issues/3157 [#3367]: https://github.com/AdguardTeam/AdGuardHome/issues/3367 [#3381]: https://github.com/AdguardTeam/AdGuardHome/issues/3381 [#3503]: https://github.com/AdguardTeam/AdGuardHome/issues/3503 diff --git a/internal/aghnet/arpdb_bsd.go b/internal/aghnet/arpdb_bsd.go index a82da76c..26ac7758 100644 --- a/internal/aghnet/arpdb_bsd.go +++ b/internal/aghnet/arpdb_bsd.go @@ -13,19 +13,24 @@ import ( "github.com/AdguardTeam/golibs/netutil" ) -func newARPDB() *cmdARPDB { +func newARPDB() (arp *cmdARPDB) { return &cmdARPDB{ parse: parseArpA, ns: &neighs{ mu: &sync.RWMutex{}, ns: make([]Neighbor, 0), }, - cmd: "arp", - args: []string{"-a"}, + cmd: "arp", + // Use -n flag to avoid resolving the hostnames of the neighbors. By + // default ARP attempts to resolve the hostnames via DNS. See man 8 + // arp. + // + // See also https://github.com/AdguardTeam/AdGuardHome/issues/3157. + args: []string{"-a", "-n"}, } } -// parseArpA parses the output of the "arp -a" command on macOS and FreeBSD. +// parseArpA parses the output of the "arp -a -n" command on macOS and FreeBSD. // The expected input format: // // host.name (192.168.0.1) at ff:ff:ff:ff:ff:ff on en0 ifscope [ethernet] diff --git a/internal/aghnet/arpdb_linux.go b/internal/aghnet/arpdb_linux.go index 3d391f29..9cc38906 100644 --- a/internal/aghnet/arpdb_linux.go +++ b/internal/aghnet/arpdb_linux.go @@ -38,12 +38,17 @@ func newARPDB() (arp *arpdbs) { fsys: rootDirFS, filename: "proc/net/arp", }, - // Then, try "arp -a". + // Then, try "arp -a -n". &cmdARPDB{ parse: parseF, ns: ns, cmd: "arp", - args: []string{"-a"}, + // Use -n flag to avoid resolving the hostnames of the neighbors. + // By default ARP attempts to resolve the hostnames via DNS. See + // man 8 arp. + // + // See also https://github.com/AdguardTeam/AdGuardHome/issues/3157. + args: []string{"-a", "-n"}, }, // Finally, try "ip neigh". &cmdARPDB{ @@ -109,11 +114,11 @@ func (arp *fsysARPDB) Neighbors() (ns []Neighbor) { return arp.ns.clone() } -// parseArpAWrt parses the output of the "arp -a" command on OpenWrt. The +// parseArpAWrt parses the output of the "arp -a -n" command on OpenWrt. The // expected input format: // -// IP address HW type Flags HW address Mask Device -// 192.168.11.98 0x1 0x2 5a:92:df:a9:7e:28 * wan +// IP address HW type Flags HW address Mask Device +// 192.168.11.98 0x1 0x2 5a:92:df:a9:7e:28 * wan // func parseArpAWrt(sc *bufio.Scanner, lenHint int) (ns []Neighbor) { if !sc.Scan() { @@ -153,8 +158,8 @@ func parseArpAWrt(sc *bufio.Scanner, lenHint int) (ns []Neighbor) { return ns } -// parseArpA parses the output of the "arp -a" command on Linux. The expected -// input format: +// parseArpA parses the output of the "arp -a -n" command on Linux. The +// expected input format: // // hostname (192.168.1.1) at ab:cd:ef:ab:cd:ef [ether] on enp0s3 // diff --git a/internal/aghnet/arpdb_openbsd.go b/internal/aghnet/arpdb_openbsd.go index a00ffa85..d5ec5fea 100644 --- a/internal/aghnet/arpdb_openbsd.go +++ b/internal/aghnet/arpdb_openbsd.go @@ -12,20 +12,25 @@ import ( "github.com/AdguardTeam/golibs/log" ) -func newARPDB() *cmdARPDB { +func newARPDB() (arp *cmdARPDB) { return &cmdARPDB{ parse: parseArpA, ns: &neighs{ mu: &sync.RWMutex{}, ns: make([]Neighbor, 0), }, - cmd: "arp", - args: []string{"-a"}, + cmd: "arp", + // Use -n flag to avoid resolving the hostnames of the neighbors. By + // default ARP attempts to resolve the hostnames via DNS. See man 8 + // arp. + // + // See also https://github.com/AdguardTeam/AdGuardHome/issues/3157. + args: []string{"-a", "-n"}, } } -// parseArpA parses the output of the "arp -a" command on OpenBSD. The expected -// input format: +// parseArpA parses the output of the "arp -a -n" command on OpenBSD. The +// expected input format: // // Host Ethernet Address Netif Expire Flags // 192.168.1.1 ab:cd:ef:ab:cd:ef em0 19m59s diff --git a/internal/aghnet/arpdb_windows.go b/internal/aghnet/arpdb_windows.go index 2a70125f..8d5431eb 100644 --- a/internal/aghnet/arpdb_windows.go +++ b/internal/aghnet/arpdb_windows.go @@ -10,7 +10,7 @@ import ( "sync" ) -func newARPDB() *cmdARPDB { +func newARPDB() (arp *cmdARPDB) { return &cmdARPDB{ parse: parseArpA, ns: &neighs{ diff --git a/internal/dnsforward/dns.go b/internal/dnsforward/dns.go index 63f694bd..d423482a 100644 --- a/internal/dnsforward/dns.go +++ b/internal/dnsforward/dns.go @@ -135,7 +135,6 @@ func (s *Server) processRecursion(dctx *dnsContext) (rc resultCode) { pctx.Res = s.genNXDomain(pctx.Req) return resultCodeFinish - } return resultCodeSuccess diff --git a/internal/dnsforward/recursiondetector_test.go b/internal/dnsforward/recursiondetector_test.go index 7573b668..4edb3a37 100644 --- a/internal/dnsforward/recursiondetector_test.go +++ b/internal/dnsforward/recursiondetector_test.go @@ -83,7 +83,7 @@ func TestRecursionDetector_Suspect(t *testing.T) { testCases := []struct { name string msg dns.Msg - want bool + want int }{{ name: "simple", msg: dns.Msg{ @@ -95,24 +95,18 @@ func TestRecursionDetector_Suspect(t *testing.T) { Qtype: dns.TypeA, }}, }, - want: true, + want: 1, }, { name: "unencumbered", msg: dns.Msg{}, - want: false, + want: 0, }} for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { t.Cleanup(rd.clear) - rd.add(tc.msg) - - if tc.want { - assert.Equal(t, 1, rd.recentRequests.Stats().Count) - } else { - assert.Zero(t, rd.recentRequests.Stats().Count) - } + assert.Equal(t, tc.want, rd.recentRequests.Stats().Count) }) } } diff --git a/internal/home/rdns.go b/internal/home/rdns.go index 7a2d1bcd..924aff37 100644 --- a/internal/home/rdns.go +++ b/internal/home/rdns.go @@ -16,18 +16,17 @@ type RDNS struct { exchanger dnsforward.RDNSExchanger clients *clientsContainer - // usePrivate is used to store the state of current private RDNS - // resolving settings and to react to it's changes. + // usePrivate is used to store the state of current private RDNS resolving + // settings and to react to it's changes. usePrivate uint32 // ipCh used to pass client's IP to rDNS workerLoop. ipCh chan net.IP // ipCache caches the IP addresses to be resolved by rDNS. The resolved - // address stays here while it's inside clients. After leaving clients - // the address will be resolved once again. If the address couldn't be - // resolved, cache prevents further attempts to resolve it for some - // time. + // address stays here while it's inside clients. After leaving clients the + // address will be resolved once again. If the address couldn't be + // resolved, cache prevents further attempts to resolve it for some time. ipCache cache.Cache }