Pull request: aghnet: fix adding entry into multiple ipsets

Updates #3638.

Squashed commit of the following:

commit f9c52176806051c2e3d5e34a440a919ca022c319
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Wed Sep 22 14:31:46 2021 +0300

    aghnet: fix docs

commit 1167806d73ba14d0145a2d1e11cece5dbb7958aa
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Wed Sep 22 14:26:28 2021 +0300

    all: imp docs, names

commit ba08f5c759fe4d83a4709f619fa65dffe3e9e164
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Wed Sep 22 14:14:05 2021 +0300

    aghnet: fix adding entry into multiple ipsets
This commit is contained in:
Ainar Garipov 2021-09-22 14:37:40 +03:00
parent b1242ee93f
commit e1e064db59
5 changed files with 31 additions and 12 deletions

View File

@ -116,6 +116,7 @@ In this release, the schema version has changed from 10 to 12.
### Fixed ### Fixed
- Adding an IP into only one of the matching ipsets on Linux ([#3638]).
- Removal of temporary filter files ([#3567]). - Removal of temporary filter files ([#3567]).
- Panic when an upstream server responds with an empty question section - Panic when an upstream server responds with an empty question section
([#3551]). ([#3551]).
@ -201,6 +202,7 @@ In this release, the schema version has changed from 10 to 12.
[#3568]: https://github.com/AdguardTeam/AdGuardHome/issues/3568 [#3568]: https://github.com/AdguardTeam/AdGuardHome/issues/3568
[#3579]: https://github.com/AdguardTeam/AdGuardHome/issues/3579 [#3579]: https://github.com/AdguardTeam/AdGuardHome/issues/3579
[#3607]: https://github.com/AdguardTeam/AdGuardHome/issues/3607 [#3607]: https://github.com/AdguardTeam/AdGuardHome/issues/3607
[#3638]: https://github.com/AdguardTeam/AdGuardHome/issues/3638

View File

@ -10,6 +10,7 @@ import (
"sync" "sync"
"github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/log"
"github.com/digineo/go-ipset/v2" "github.com/digineo/go-ipset/v2"
"github.com/mdlayher/netlink" "github.com/mdlayher/netlink"
"github.com/ti-mo/netfilter" "github.com/ti-mo/netfilter"
@ -67,6 +68,15 @@ type ipsetProps struct {
// unit is a convenient alias for struct{}. // unit is a convenient alias for struct{}.
type unit = 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
}
// ipsetMgr is the Linux Netfilter ipset manager. // ipsetMgr is the Linux Netfilter ipset manager.
type ipsetMgr struct { type ipsetMgr struct {
nameToIpset map[string]ipsetProps nameToIpset map[string]ipsetProps
@ -82,7 +92,7 @@ type ipsetMgr struct {
// are either added to all corresponding ipsets or not. When that stops // are either added to all corresponding ipsets or not. When that stops
// being the case, for example if we add dynamic reconfiguration of // being the case, for example if we add dynamic reconfiguration of
// ipsets, this map will need to become a per-ipset-name one. // ipsets, this map will need to become a per-ipset-name one.
addedIPs map[[16]byte]unit addedIPs ipsInIpset
ipv4Conn ipsetConn ipv4Conn ipsetConn
ipv6Conn ipsetConn ipv6Conn ipsetConn
@ -199,7 +209,7 @@ func newIpsetMgrWithDialer(ipsetConf []string, dial ipsetDialer) (mgr IpsetManag
dial: dial, dial: dial,
addedIPs: make(map[[16]byte]unit), addedIPs: make(ipsInIpset),
} }
err = m.dialNetfilter(&netlink.Config{}) err = m.dialNetfilter(&netlink.Config{})
@ -265,16 +275,19 @@ func (m *ipsetMgr) addIPs(host string, set ipsetProps, ips []net.IP) (n int, err
} }
var entries []*ipset.Entry var entries []*ipset.Entry
var newAddedIPs [][16]byte var newAddedEntries []ipInIpsetEntry
for _, ip := range ips { for _, ip := range ips {
var iparr [16]byte e := ipInIpsetEntry{
copy(iparr[:], ip.To16()) ipsetName: set.name,
if _, added := m.addedIPs[iparr]; added { }
copy(e.ipArr[:], ip.To16())
if _, added := m.addedIPs[e]; added {
continue continue
} }
entries = append(entries, ipset.NewEntry(ipset.EntryIP(ip))) entries = append(entries, ipset.NewEntry(ipset.EntryIP(ip)))
newAddedIPs = append(newAddedIPs, iparr) newAddedEntries = append(newAddedEntries, e)
} }
n = len(entries) n = len(entries)
@ -299,8 +312,8 @@ func (m *ipsetMgr) addIPs(host string, set ipsetProps, ips []net.IP) (n int, err
// Only add these to the cache once we're sure that all of them were // Only add these to the cache once we're sure that all of them were
// actually sent to the ipset. // actually sent to the ipset.
for _, iparr := range newAddedIPs { for _, e := range newAddedEntries {
m.addedIPs[iparr] = unit{} m.addedIPs[e] = unit{}
} }
return n, nil return n, nil
@ -330,6 +343,8 @@ func (m *ipsetMgr) addToSets(
return n, fmt.Errorf("unexpected family %s for ipset %q", set.family, set.name) return n, fmt.Errorf("unexpected family %s for ipset %q", set.family, set.name)
} }
log.Debug("ipset: added %d ips to set %s", nn, set.name)
n += nn n += nn
} }
@ -346,6 +361,8 @@ func (m *ipsetMgr) Add(host string, ip4s, ip6s []net.IP) (n int, err error) {
return 0, nil return 0, nil
} }
log.Debug("ipset: found %d sets", len(sets))
return m.addToSets(host, ip4s, ip6s, sets) return m.addToSets(host, ip4s, ip6s, sets)
} }

View File

@ -131,7 +131,7 @@ func (c *ipsetCtx) process(dctx *dnsContext) (rc resultCode) {
return resultCodeSuccess return resultCodeSuccess
} }
log.Debug("ipset: added %d new ips", n) log.Debug("ipset: added %d new ipset entries", n)
return resultCodeSuccess return resultCodeSuccess
} }

View File

@ -15,7 +15,7 @@ type fakeIpsetMgr struct {
ip6s []net.IP ip6s []net.IP
} }
// Add implements the aghnet.IpsetManager inteface for *fakeIpsetMgr. // Add implements the aghnet.IpsetManager interface for *fakeIpsetMgr.
func (m *fakeIpsetMgr) Add(host string, ip4s, ip6s []net.IP) (n int, err error) { func (m *fakeIpsetMgr) Add(host string, ip4s, ip6s []net.IP) (n int, err error) {
m.ip4s = append(m.ip4s, ip4s...) m.ip4s = append(m.ip4s, ip4s...)
m.ip6s = append(m.ip6s, ip6s...) m.ip6s = append(m.ip6s, ip6s...)

View File

@ -69,7 +69,7 @@ type homeContext struct {
configFilename string // Config filename (can be overridden via the command line arguments) configFilename string // Config filename (can be overridden via the command line arguments)
workDir string // Location of our directory, used to protect against CWD being somewhere else workDir string // Location of our directory, used to protect against CWD being somewhere else
firstRun bool // if set to true, don't run any services except HTTP web inteface, and serve only first-run html firstRun bool // if set to true, don't run any services except HTTP web interface, and serve only first-run html
pidFileName string // PID file name. Empty if no PID file was created. pidFileName string // PID file name. Empty if no PID file was created.
disableUpdate bool // If set, don't check for updates disableUpdate bool // If set, don't check for updates
controlLock sync.Mutex controlLock sync.Mutex