Skip to content

Commit

Permalink
Pull request 2027: 6233-ipset-cached-entries
Browse files Browse the repository at this point in the history
Updates #6233.

Squashed commit of the following:

commit ef7692f
Merge: b3ef5de 8b6c260
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 b3ef5de
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Mon Oct 9 13:06:23 2023 +0300

    all: fix typo

commit d42a970
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Fri Oct 6 19:26:51 2023 +0300

    all: imp chlog

commit 818931a
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Fri Oct 6 18:30:52 2023 +0300

    all: upd chlog

commit af3dc60
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Fri Oct 6 18:03:01 2023 +0300

    ipset: imp docs

commit 2c9d6c0
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Fri Oct 6 16:53:42 2023 +0300

    all: add tests

commit 0d41eaa
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Fri Oct 6 15:12:54 2023 +0300

    ipset: rm cache
  • Loading branch information
schzhn committed Oct 9, 2023
1 parent 8b6c260 commit 8842b2d
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 38 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

<!--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
38 changes: 0 additions & 38 deletions internal/ipset/ipset_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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{})
Expand Down Expand Up @@ -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)
Expand All @@ -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
}

Expand Down
1 change: 1 addition & 0 deletions internal/ipset/ipset_linux_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 8842b2d

Please sign in to comment.