From 8842b2df90bbadbba17daeea0fa73525f2d7de9d Mon Sep 17 00:00:00 2001 From: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon, 9 Oct 2023 13:15:51 +0300 Subject: [PATCH] Pull request 2027: 6233-ipset-cached-entries Updates #6233. Squashed commit of the following: commit ef7692fb78a287a51a6b50c4ac0f1c33857a9ff0 Merge: b3ef5de41 8b6c260de Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Oct 9 13:07:10 2023 +0300 Merge branch 'master' into 6233-ipset-cached-entries commit b3ef5de411d2ebb2f344430daf81e05a33ae4e78 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Mon Oct 9 13:06:23 2023 +0300 all: fix typo commit d42a970336d1d7e8a2f7c8459bf862762cdac8f6 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Fri Oct 6 19:26:51 2023 +0300 all: imp chlog commit 818931a136c7b851820f8ff8e05ada5360da2090 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Fri Oct 6 18:30:52 2023 +0300 all: upd chlog commit af3dc60c038f04690882eca30a6f9c7d23f7c371 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Fri Oct 6 18:03:01 2023 +0300 ipset: imp docs commit 2c9d6c0c88ba2c2185b4d29212272ad5d48ae474 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Fri Oct 6 16:53:42 2023 +0300 all: add tests commit 0d41eaabf7a275c6a9eb4a1d64aa551d4d8de367 Author: Stanislav Chzhen <s.chzhen@adguard.com> Date: Fri Oct 6 15:12:54 2023 +0300 ipset: rm cache --- CHANGELOG.md | 2 + .../{ipset_test.go => ipset_internal_test.go} | 71 +++++++++++++++++++ internal/ipset/ipset_linux.go | 38 ---------- internal/ipset/ipset_linux_internal_test.go | 1 + 4 files changed, 74 insertions(+), 38 deletions(-) rename internal/dnsforward/{ipset_test.go => ipset_internal_test.go} (66%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 831e15c9..b128128d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ NOTE: Add new changes BELOW THIS COMMENT. ### Changed +- ipset entries are updated more often ([6233]). - Node.JS 16 is now required to build the frontend. ### Fixed @@ -44,6 +45,7 @@ NOTE: Add new changes BELOW THIS COMMENT. [#4569]: https://github.com/AdguardTeam/AdGuardHome/issues/4569 [#6226]: https://github.com/AdguardTeam/AdGuardHome/issues/6226 [#6231]: https://github.com/AdguardTeam/AdGuardHome/issues/6231 +[#6233]: https://github.com/AdguardTeam/AdGuardHome/issues/6233 [#6280]: https://github.com/AdguardTeam/AdGuardHome/issues/6280 <!-- diff --git a/internal/dnsforward/ipset_test.go b/internal/dnsforward/ipset_internal_test.go similarity index 66% rename from internal/dnsforward/ipset_test.go rename to internal/dnsforward/ipset_internal_test.go index 66185a5c..cdeed21b 100644 --- a/internal/dnsforward/ipset_test.go +++ b/internal/dnsforward/ipset_internal_test.go @@ -114,3 +114,74 @@ func TestIpsetCtx_process(t *testing.T) { assert.NoError(t, err) }) } + +func TestIpsetCtx_SkipIpsetProcessing(t *testing.T) { + req4 := createTestMessage("example.com") + resp4 := &dns.Msg{ + Answer: []dns.RR{&dns.A{ + A: net.IP{1, 2, 3, 4}, + }}, + } + + m := &fakeIpsetMgr{} + ictx := &ipsetCtx{ + ipsetMgr: m, + } + + testCases := []struct { + dctx *dnsContext + name string + want bool + }{{ + name: "basic", + want: false, + dctx: &dnsContext{ + proxyCtx: &proxy.DNSContext{ + Req: req4, + Res: resp4, + }, + + responseFromUpstream: true, + }, + }, { + name: "rewrite", + want: true, + dctx: &dnsContext{ + proxyCtx: &proxy.DNSContext{ + Req: req4, + Res: resp4, + }, + + responseFromUpstream: false, + }, + }, { + name: "empty_req", + want: true, + dctx: &dnsContext{ + proxyCtx: &proxy.DNSContext{ + Req: nil, + Res: resp4, + }, + + responseFromUpstream: true, + }, + }, { + name: "empty_res", + want: true, + dctx: &dnsContext{ + proxyCtx: &proxy.DNSContext{ + Req: req4, + Res: nil, + }, + + responseFromUpstream: true, + }, + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := ictx.skipIpsetProcessing(tc.dctx) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/internal/ipset/ipset_linux.go b/internal/ipset/ipset_linux.go index 84560897..06b6923c 100644 --- a/internal/ipset/ipset_linux.go +++ b/internal/ipset/ipset_linux.go @@ -62,18 +62,6 @@ type props struct { family netfilter.ProtoFamily } -// unit is a convenient alias for struct{}. -type unit = struct{} - -// ipsInIpset is the type of a set of IP-address-to-ipset mappings. -type ipsInIpset map[ipInIpsetEntry]unit - -// ipInIpsetEntry is the type for entries in an ipsInIpset set. -type ipInIpsetEntry struct { - ipsetName string - ipArr [net.IPv6len]byte -} - // manager is the Linux Netfilter ipset manager. type manager struct { nameToIpset map[string]props @@ -84,13 +72,6 @@ type manager struct { // mu protects all properties below. mu *sync.Mutex - // TODO(a.garipov): Currently, the ipset list is static, and we don't - // read the IPs already in sets, so we can assume that all incoming IPs - // are either added to all corresponding ipsets or not. When that stops - // being the case, for example if we add dynamic reconfiguration of - // ipsets, this map will need to become a per-ipset-name one. - addedIPs ipsInIpset - ipv4Conn ipsetConn ipv6Conn ipsetConn } @@ -205,8 +186,6 @@ func newManagerWithDialer(ipsetConf []string, dial dialer) (mgr Manager, err err domainToIpsets: make(map[string][]props), dial: dial, - - addedIPs: make(ipsInIpset), } err = m.dialNetfilter(&netlink.Config{}) @@ -280,19 +259,8 @@ func (m *manager) addIPs(host string, set props, ips []net.IP) (n int, err error } var entries []*ipset.Entry - var newAddedEntries []ipInIpsetEntry for _, ip := range ips { - e := ipInIpsetEntry{ - ipsetName: set.name, - } - copy(e.ipArr[:], ip.To16()) - - if _, added := m.addedIPs[e]; added { - continue - } - entries = append(entries, ipset.NewEntry(ipset.EntryIP(ip))) - newAddedEntries = append(newAddedEntries, e) } n = len(entries) @@ -315,12 +283,6 @@ func (m *manager) addIPs(host string, set props, ips []net.IP) (n int, err error return 0, fmt.Errorf("adding %q%s to ipset %q: %w", host, ips, set.name, err) } - // Only add these to the cache once we're sure that all of them were - // actually sent to the ipset. - for _, e := range newAddedEntries { - m.addedIPs[e] = unit{} - } - return n, nil } diff --git a/internal/ipset/ipset_linux_internal_test.go b/internal/ipset/ipset_linux_internal_test.go index 260c507e..97c5b8bb 100644 --- a/internal/ipset/ipset_linux_internal_test.go +++ b/internal/ipset/ipset_linux_internal_test.go @@ -114,6 +114,7 @@ func TestManager_Add(t *testing.T) { assert.NoError(t, err) } +// ipsetPropsSink is the typed sink for benchmark results. var ipsetPropsSink []props func BenchmarkManager_LookupHost(b *testing.B) {