From 12435fc9c8d6cd59cca0b7e0dc43c5fe3730a154 Mon Sep 17 00:00:00 2001 From: sridhar Date: Thu, 2 Jan 2025 11:43:26 -0800 Subject: [PATCH 1/3] Modify setfilter to make it suitable for filtering ipsets dynamically --- felix/bpf/ipsets/ipsets.go | 10 ++++--- felix/dataplane/ipsets/ipsets_mgr.go | 3 ++- felix/dataplane/ipsets/mock_ipsets.go | 6 ++++- felix/dataplane/linux/int_dataplane.go | 14 ++++++++-- felix/dataplane/linux/policy_mgr.go | 11 ++++++++ felix/dataplane/windows/ipsets/ipsets.go | 6 +++-- felix/ipsets/ipsets.go | 34 +++++++++++------------- felix/ipsets/ipsets_test.go | 18 ++++++++++--- felix/nftables/ipsets.go | 20 +++++++------- 9 files changed, 81 insertions(+), 41 deletions(-) diff --git a/felix/bpf/ipsets/ipsets.go b/felix/bpf/ipsets/ipsets.go index ae075f87941..d5e0d22a0b9 100644 --- a/felix/bpf/ipsets/ipsets.go +++ b/felix/bpf/ipsets/ipsets.go @@ -57,6 +57,8 @@ type bpfIPSets struct { opRecorder logutils.OpRecorder lg *log.Entry + + filterIPSet func(string) bool } func NewBPFIPSets( @@ -370,9 +372,11 @@ func (m *bpfIPSets) markIPSetDirty(data *bpfIPSet) { m.dirtyIPSetIDs.Add(data.ID) } -func (m *bpfIPSets) SetFilter(ipSetNames set.Set[string]) { - // Not needed for this IP set dataplane. All known IP sets - // are written into the corresponding BPF map. +func (m *bpfIPSets) SetFilter(fn func(ipSetName string) bool) { + m.filterIPSet = fn +} + +func (m *bpfIPSets) MarkDirty(ipsetNames set.Set[string]) { } type bpfIPSet struct { diff --git a/felix/dataplane/ipsets/ipsets_mgr.go b/felix/dataplane/ipsets/ipsets_mgr.go index 2e9acbbb93a..39ae4a10c48 100644 --- a/felix/dataplane/ipsets/ipsets_mgr.go +++ b/felix/dataplane/ipsets/ipsets_mgr.go @@ -33,7 +33,8 @@ type IPSetsDataplane interface { QueueResync() ApplyUpdates() ApplyDeletions() (reschedule bool) - SetFilter(neededIPSets set.Set[string]) + SetFilter(fn func(string) bool) + MarkDirty(set.Set[string]) } // Except for domain IP sets, IPSetsManager simply passes through IP set updates from the datastore diff --git a/felix/dataplane/ipsets/mock_ipsets.go b/felix/dataplane/ipsets/mock_ipsets.go index e73629d358d..c9b8c82aba4 100644 --- a/felix/dataplane/ipsets/mock_ipsets.go +++ b/felix/dataplane/ipsets/mock_ipsets.go @@ -101,6 +101,10 @@ func (s *MockIPSets) ApplyDeletions() bool { return false } -func (s *MockIPSets) SetFilter(ipSetNames set.Set[string]) { +func (s *MockIPSets) SetFilter(fn func(string) bool) { + // Not implemented for UT. +} + +func (s *MockIPSets) MarkDirty(ipSetNames set.Set[string]) { // Not implemented for UT. } diff --git a/felix/dataplane/linux/int_dataplane.go b/felix/dataplane/linux/int_dataplane.go index e459988ff96..7e5d77027cc 100644 --- a/felix/dataplane/linux/int_dataplane.go +++ b/felix/dataplane/linux/int_dataplane.go @@ -770,7 +770,12 @@ func NewIntDataplaneDriver(config Config) *InternalDataplane { bpfutils.RemoveBPFSpecialDevices() } else { // In BPF mode we still use iptables for raw egress policy. - dp.RegisterManager(newRawEgressPolicyManager(rawTableV4, ruleRenderer, 4, ipSetsV4.SetFilter, config.RulesConfig.NFTables)) + mgr := newRawEgressPolicyManager(rawTableV4, ruleRenderer, 4, ipSetsV4.MarkDirty, config.RulesConfig.NFTables) + ipSetsV4.SetFilter(func(ipSetName string) bool { + neededIPSets := mgr.GetNeededIPSets() + return neededIPSets.Contains(ipSetName) + }) + dp.RegisterManager(mgr) } interfaceRegexes := make([]string, len(config.RulesConfig.WorkloadIfacePrefixes)) @@ -1035,7 +1040,12 @@ func NewIntDataplaneDriver(config Config) *InternalDataplane { config.MaxIPSetSize)) dp.RegisterManager(newPolicyManager(rawTableV6, mangleTableV6, filterTableV6, ruleRenderer, 6, config.RulesConfig.NFTables)) } else { - dp.RegisterManager(newRawEgressPolicyManager(rawTableV6, ruleRenderer, 6, ipSetsV6.SetFilter, config.RulesConfig.NFTables)) + mgr := newRawEgressPolicyManager(rawTableV6, ruleRenderer, 6, ipSetsV6.MarkDirty, config.RulesConfig.NFTables) + ipSetsV6.SetFilter(func(ipSetName string) bool { + neededIPSets := mgr.GetNeededIPSets() + return neededIPSets.Contains(ipSetName) + }) + dp.RegisterManager(mgr) } dp.RegisterManager(newEndpointManager( diff --git a/felix/dataplane/linux/policy_mgr.go b/felix/dataplane/linux/policy_mgr.go index cbb183f4d3f..88e693fa458 100644 --- a/felix/dataplane/linux/policy_mgr.go +++ b/felix/dataplane/linux/policy_mgr.go @@ -173,3 +173,14 @@ func (m *policyManager) CompleteDeferredWork() error { m.ipSetsCallback(merged) return nil } + +func (m *policyManager) GetNeededIPSets() set.Set[string] { + merged := set.New[string]() + for _, ipSets := range m.neededIPSets { + ipSets.Iter(func(item string) error { + merged.Add(item) + return nil + }) + } + return merged +} diff --git a/felix/dataplane/windows/ipsets/ipsets.go b/felix/dataplane/windows/ipsets/ipsets.go index 9fc6a0bf350..755e01aaba9 100644 --- a/felix/dataplane/windows/ipsets/ipsets.go +++ b/felix/dataplane/windows/ipsets/ipsets.go @@ -193,6 +193,8 @@ func (m *IPSets) ApplyDeletions() bool { return false } -func (s *IPSets) SetFilter(ipSetNames set.Set[string]) { - // Not needed for Windows. +func (m *IPSets) SetFilter(fn func(ipSetName string) bool) { +} + +func (m *IPSets) MarkDirty(ipsetNames set.Set[string]) { } diff --git a/felix/ipsets/ipsets.go b/felix/ipsets/ipsets.go index 8085504da74..2684a921f8f 100644 --- a/felix/ipsets/ipsets.go +++ b/felix/ipsets/ipsets.go @@ -99,7 +99,7 @@ type IPSets struct { // Optional filter. When non-nil, only these IP set IDs will be rendered into the dataplane // as Linux IP sets. - neededIPSetNames set.Set[string] + filterIPSet func(string) bool } func NewIPSets(ipVersionConfig *IPVersionConfig, recorder logutils.OpRecorder) *IPSets { @@ -1092,31 +1092,29 @@ func (s *IPSets) updateDirtiness(name string) { } } -func (s *IPSets) SetFilter(ipSetNames set.Set[string]) { - oldSetNames := s.neededIPSetNames - if oldSetNames == nil && ipSetNames == nil { - return - } - s.logCxt.Debugf("Filtering to needed IP set names: %v", ipSetNames) - s.neededIPSetNames = ipSetNames - for name, meta := range s.setNameToAllMetadata { - if s.ipSetNeeded(name) { - s.setNameToProgrammedMetadata.Desired().Set(name, meta) - } else { - s.setNameToProgrammedMetadata.Desired().Delete(name) - } - s.updateDirtiness(name) - } +func (s *IPSets) SetFilter(fn func(string) bool) { + s.filterIPSet = fn } func (s *IPSets) ipSetNeeded(name string) bool { - if s.neededIPSetNames == nil { + if s.filterIPSet == nil { // We're not filtering down to a "needed" set, so all IP sets are needed. return true } // We are filtering down, so compare against the needed set. - return s.neededIPSetNames.Contains(name) + return s.filterIPSet(name) +} + +func (s *IPSets) MarkDirty(ipsetNames set.Set[string]) { + for name, meta := range s.setNameToAllMetadata { + if ipsetNames.Contains(name) { + s.setNameToProgrammedMetadata.Desired().Set(name, meta) + } else { + s.setNameToProgrammedMetadata.Desired().Delete(name) + } + s.updateDirtiness(name) + } } // CanonicaliseMember converts the string representation of an IP set member to a canonical diff --git a/felix/ipsets/ipsets_test.go b/felix/ipsets/ipsets_test.go index acf6f6ee59b..a7eeb697eab 100644 --- a/felix/ipsets/ipsets_test.go +++ b/felix/ipsets/ipsets_test.go @@ -598,7 +598,10 @@ var _ = Describe("IP sets dataplane", func() { Context("with filtering to two IP sets", func() { BeforeEach(func() { - ipsets.SetFilter(set.From(v4MainIPSetName2, v4MainIPSetName)) + ipsets.SetFilter(func(ipSetName string) bool { + neededIPSets := set.From(v4MainIPSetName2, v4MainIPSetName) + return neededIPSets.Contains(ipSetName) + }) ipsets.QueueResync() apply() }) @@ -699,7 +702,11 @@ var _ = Describe("IP sets dataplane", func() { Context("with filtering to single IP set", func() { BeforeEach(func() { - ipsets.SetFilter(set.From(v4MainIPSetName2)) + ipsets.SetFilter(func(ipSetName string) bool { + neededIPSets := set.From(v4MainIPSetName2) + return neededIPSets.Contains(ipSetName) + }) + ipsets.MarkDirty(set.From(v4MainIPSetName2)) apply() }) @@ -714,7 +721,12 @@ var _ = Describe("IP sets dataplane", func() { Context("with filtering to both known IP sets", func() { BeforeEach(func() { - ipsets.SetFilter(set.From(v4MainIPSetName2, v4MainIPSetName)) + ipsets.SetFilter(func(ipSetName string) bool { + neededIPSets := set.From(v4MainIPSetName2, v4MainIPSetName) + return neededIPSets.Contains(ipSetName) + }) + ipsets.MarkDirty(set.From(v4MainIPSetName2, v4MainIPSetName)) + apply() }) diff --git a/felix/nftables/ipsets.go b/felix/nftables/ipsets.go index 89bdc1a1394..6848fe81ede 100644 --- a/felix/nftables/ipsets.go +++ b/felix/nftables/ipsets.go @@ -97,7 +97,7 @@ type IPSets struct { // Optional filter. When non-nil, only these IP set IDs will be rendered into the dataplane // as Linux IP sets. - neededIPSetNames set.Set[string] + filterIPSet func(string) bool nft knftables.Interface } @@ -781,15 +781,13 @@ func (s *IPSets) updateDirtiness(name string) { } } -func (s *IPSets) SetFilter(ipSetNames set.Set[string]) { - oldSetNames := s.neededIPSetNames - if oldSetNames == nil && ipSetNames == nil { - return - } - s.logCxt.Debugf("Filtering to needed IP set names: %v", ipSetNames) - s.neededIPSetNames = ipSetNames +func (s *IPSets) SetFilter(fn func(ipSetName string) bool) { + s.filterIPSet = fn +} + +func (s *IPSets) MarkDirty(ipsetNames set.Set[string]) { for name, meta := range s.setNameToAllMetadata { - if s.ipSetNeeded(name) { + if ipsetNames.Contains(name) { s.setNameToProgrammedMetadata.Desired().Set(name, meta) } else { s.setNameToProgrammedMetadata.Desired().Delete(name) @@ -799,13 +797,13 @@ func (s *IPSets) SetFilter(ipSetNames set.Set[string]) { } func (s *IPSets) ipSetNeeded(name string) bool { - if s.neededIPSetNames == nil { + if s.filterIPSet == nil { // We're not filtering down to a "needed" set, so all IP sets are needed. return true } // We are filtering down, so compare against the needed set. - return s.neededIPSetNames.Contains(name) + return s.filterIPSet(name) } // CanonicaliseMember converts the string representation of an nftables set member to a canonical From 44b74d00ee972b14041daa078cd4341448cc6e3d Mon Sep 17 00:00:00 2001 From: sridhar Date: Thu, 2 Jan 2025 16:57:37 -0800 Subject: [PATCH 2/3] Filter BPF ipsets --- felix/bpf/ipsets/ipsets.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/felix/bpf/ipsets/ipsets.go b/felix/bpf/ipsets/ipsets.go index d5e0d22a0b9..7bc143f01d5 100644 --- a/felix/bpf/ipsets/ipsets.go +++ b/felix/bpf/ipsets/ipsets.go @@ -135,6 +135,14 @@ func (m *bpfIPSets) deleteIPSetAndReleaseID(ipSet *bpfIPSet) { // to ApplyUpdates(), the IP sets will be replaced with the new contents and the set's metadata // will be updated as appropriate. func (m *bpfIPSets) AddOrReplaceIPSet(setMetadata ipsets.IPSetMetadata, members []string) { + if m.filterIPSet != nil && !m.filterIPSet(setMetadata.SetID) { + ipSet := m.getExistingIPSetString(setMetadata.SetID) + if ipSet != nil { + ipSet.Deleted = true + m.markIPSetDirty(ipSet) + } + return + } ipSet := m.getOrCreateIPSet(setMetadata.SetID) ipSet.Type = setMetadata.Type m.lg.WithFields(log.Fields{"stringID": setMetadata.SetID, "uint64ID": ipSet.ID, "members": members}).Info("IP set added") From 11548606633ef1ea34b8ca166f1f938c4799f1ec Mon Sep 17 00:00:00 2001 From: sridhar Date: Fri, 3 Jan 2025 15:54:03 -0800 Subject: [PATCH 3/3] Address review comments --- felix/bpf/ipsets/ipsets.go | 29 +++++++++++++++++++++++- felix/dataplane/ipsets/ipsets_mgr.go | 2 +- felix/dataplane/ipsets/mock_ipsets.go | 2 +- felix/dataplane/linux/int_dataplane.go | 20 ++++++++-------- felix/dataplane/linux/policy_mgr.go | 28 +++++++++++------------ felix/dataplane/linux/policy_mgr_test.go | 26 ++++++++++----------- felix/dataplane/windows/ipsets/ipsets.go | 4 +++- felix/ipsets/ipsets.go | 17 ++++++++++---- felix/ipsets/ipsets_test.go | 5 ++-- felix/nftables/ipsets.go | 17 ++++++++++---- 10 files changed, 96 insertions(+), 54 deletions(-) diff --git a/felix/bpf/ipsets/ipsets.go b/felix/bpf/ipsets/ipsets.go index 7bc143f01d5..d12508bae4b 100644 --- a/felix/bpf/ipsets/ipsets.go +++ b/felix/bpf/ipsets/ipsets.go @@ -58,6 +58,11 @@ type bpfIPSets struct { lg *log.Entry + // filterIPSet is set to filter the needed ipsets based on the + // ipset name. + // when not set (nil), all the ipsets will be programmed. + // when the filter returns true, the ipset will be programmed. + // when the filter returns false, the ipset will be skipped. filterIPSet func(string) bool } @@ -380,11 +385,33 @@ func (m *bpfIPSets) markIPSetDirty(data *bpfIPSet) { m.dirtyIPSetIDs.Add(data.ID) } +// SetFilter updates the ipset filter function but does +// not scan the existing ipsets and apply the filter. func (m *bpfIPSets) SetFilter(fn func(ipSetName string) bool) { m.filterIPSet = fn } -func (m *bpfIPSets) MarkDirty(ipsetNames set.Set[string]) { +func (m *bpfIPSets) isIPSetNeeded(name string) bool { + if m.filterIPSet == nil { + // We're not filtering down to a "needed" set, so all IP sets are needed. + return true + } + + // We are filtering down, so compare against the needed set. + return m.filterIPSet(name) +} + +// ApplyFilter applies the ipset filter to the existing +// ipsets. The caller should call ApplyFilter after updating +// the filter program to make sure the filter is applied to +// the existing ipsets. +func (m *bpfIPSets) ApplyFilter() { + for _, ipset := range m.ipSets { + if !m.isIPSetNeeded(ipset.OriginalID) { + ipset.Deleted = true + m.markIPSetDirty(ipset) + } + } } type bpfIPSet struct { diff --git a/felix/dataplane/ipsets/ipsets_mgr.go b/felix/dataplane/ipsets/ipsets_mgr.go index 39ae4a10c48..aefe5e9a777 100644 --- a/felix/dataplane/ipsets/ipsets_mgr.go +++ b/felix/dataplane/ipsets/ipsets_mgr.go @@ -34,7 +34,7 @@ type IPSetsDataplane interface { ApplyUpdates() ApplyDeletions() (reschedule bool) SetFilter(fn func(string) bool) - MarkDirty(set.Set[string]) + ApplyFilter() } // Except for domain IP sets, IPSetsManager simply passes through IP set updates from the datastore diff --git a/felix/dataplane/ipsets/mock_ipsets.go b/felix/dataplane/ipsets/mock_ipsets.go index c9b8c82aba4..cffd0284760 100644 --- a/felix/dataplane/ipsets/mock_ipsets.go +++ b/felix/dataplane/ipsets/mock_ipsets.go @@ -105,6 +105,6 @@ func (s *MockIPSets) SetFilter(fn func(string) bool) { // Not implemented for UT. } -func (s *MockIPSets) MarkDirty(ipSetNames set.Set[string]) { +func (s *MockIPSets) ApplyFilter() { // Not implemented for UT. } diff --git a/felix/dataplane/linux/int_dataplane.go b/felix/dataplane/linux/int_dataplane.go index 7e5d77027cc..2a73e0e7581 100644 --- a/felix/dataplane/linux/int_dataplane.go +++ b/felix/dataplane/linux/int_dataplane.go @@ -770,11 +770,11 @@ func NewIntDataplaneDriver(config Config) *InternalDataplane { bpfutils.RemoveBPFSpecialDevices() } else { // In BPF mode we still use iptables for raw egress policy. - mgr := newRawEgressPolicyManager(rawTableV4, ruleRenderer, 4, ipSetsV4.MarkDirty, config.RulesConfig.NFTables) - ipSetsV4.SetFilter(func(ipSetName string) bool { - neededIPSets := mgr.GetNeededIPSets() - return neededIPSets.Contains(ipSetName) - }) + mgr := newRawEgressPolicyManager(rawTableV4, ruleRenderer, 4, ipSetsV4.ApplyFilter, config.RulesConfig.NFTables) + // When in BPF mode, we still program egress do-not-track rules into + // (ip|nf)tables; set a filter on the IP sets we program so that only the + // IP sets needed for do-not-track are programmed. + ipSetsV4.SetFilter(mgr.IPSetNeeded) dp.RegisterManager(mgr) } @@ -1040,11 +1040,11 @@ func NewIntDataplaneDriver(config Config) *InternalDataplane { config.MaxIPSetSize)) dp.RegisterManager(newPolicyManager(rawTableV6, mangleTableV6, filterTableV6, ruleRenderer, 6, config.RulesConfig.NFTables)) } else { - mgr := newRawEgressPolicyManager(rawTableV6, ruleRenderer, 6, ipSetsV6.MarkDirty, config.RulesConfig.NFTables) - ipSetsV6.SetFilter(func(ipSetName string) bool { - neededIPSets := mgr.GetNeededIPSets() - return neededIPSets.Contains(ipSetName) - }) + mgr := newRawEgressPolicyManager(rawTableV6, ruleRenderer, 6, ipSetsV6.ApplyFilter, config.RulesConfig.NFTables) + // When in BPF mode, we still program egress do-not-track rules into + // (ip|nf)tables; set a filter on the IP sets we program so that only the + // IP sets needed for do-not-track are programmed. + ipSetsV6.SetFilter(mgr.IPSetNeeded) dp.RegisterManager(mgr) } diff --git a/felix/dataplane/linux/policy_mgr.go b/felix/dataplane/linux/policy_mgr.go index 88e693fa458..4921a1ffb1f 100644 --- a/felix/dataplane/linux/policy_mgr.go +++ b/felix/dataplane/linux/policy_mgr.go @@ -37,7 +37,8 @@ type policyManager struct { rawEgressOnly bool ipSetFilterDirty bool // Only used in "raw only" mode. neededIPSets map[types.PolicyID]set.Set[string] - ipSetsCallback func(neededIPSets set.Set[string]) + ipSetsCache set.Set[string] + ipSetsCallback func() nftablesEnabled bool } @@ -58,7 +59,7 @@ func newPolicyManager(rawTable, mangleTable, filterTable Table, ruleRenderer pol } func newRawEgressPolicyManager(rawTable Table, ruleRenderer policyRenderer, ipVersion uint8, - ipSetsCallback func(neededIPSets set.Set[string]), + ipSetsCallback func(), nft bool, ) *policyManager { return &policyManager{ @@ -71,6 +72,7 @@ func newRawEgressPolicyManager(rawTable Table, ruleRenderer policyRenderer, ipVe // Make sure we set the filter at start-of-day, even if there are no policies. ipSetFilterDirty: true, neededIPSets: make(map[types.PolicyID]set.Set[string]), + ipSetsCache: set.New[string](), ipSetsCallback: ipSetsCallback, nftablesEnabled: nft, } @@ -151,6 +153,7 @@ func (m *policyManager) updateNeededIPSets(id *types.PolicyID, neededIPSets set. } else { delete(m.neededIPSets, *id) } + m.syncIPSetCache() m.ipSetFilterDirty = true } @@ -162,25 +165,20 @@ func (m *policyManager) CompleteDeferredWork() error { return nil } m.ipSetFilterDirty = false - - merged := set.New[string]() - for _, ipSets := range m.neededIPSets { - ipSets.Iter(func(item string) error { - merged.Add(item) - return nil - }) - } - m.ipSetsCallback(merged) + m.ipSetsCallback() return nil } -func (m *policyManager) GetNeededIPSets() set.Set[string] { - merged := set.New[string]() +func (m *policyManager) syncIPSetCache() { + m.ipSetsCache.Clear() for _, ipSets := range m.neededIPSets { ipSets.Iter(func(item string) error { - merged.Add(item) + m.ipSetsCache.Add(item) return nil }) } - return merged +} + +func (m *policyManager) IPSetNeeded(name string) bool { + return m.ipSetsCache.Contains(name) } diff --git a/felix/dataplane/linux/policy_mgr_test.go b/felix/dataplane/linux/policy_mgr_test.go index f319afd2dd4..dd860040f73 100644 --- a/felix/dataplane/linux/policy_mgr_test.go +++ b/felix/dataplane/linux/policy_mgr_test.go @@ -253,12 +253,10 @@ var _ = Describe("Raw egress policy manager", func() { var ( policyMgr *policyManager rawTable *mockTable - neededIPSets set.Set[string] numCallbackCalls int ) BeforeEach(func() { - neededIPSets = nil numCallbackCalls = 0 rawTable = newMockTable("raw") ruleRenderer := rules.NewRenderer(rules.Config{ @@ -275,8 +273,7 @@ var _ = Describe("Raw egress policy manager", func() { rawTable, ruleRenderer, 4, - func(ipSets set.Set[string]) { - neededIPSets = ipSets + func() { numCallbackCalls++ }, false) }) @@ -284,8 +281,6 @@ var _ = Describe("Raw egress policy manager", func() { It("correctly reports no IP sets at start of day", func() { err := policyMgr.CompleteDeferredWork() Expect(err).NotTo(HaveOccurred()) - Expect(neededIPSets).ToNot(BeNil()) - Expect(neededIPSets.Len()).To(BeZero()) Expect(numCallbackCalls).To(Equal(1)) By("Not repeating the callback.") @@ -294,7 +289,7 @@ var _ = Describe("Raw egress policy manager", func() { Expect(numCallbackCalls).To(Equal(1)) }) - It("correctly reports needed IP sets", func() { + It("correctly caches the needed IP sets", func() { By("defining one untracked policy with an IP set") policyMgr.OnUpdate(&proto.ActivePolicyUpdate{ Id: &proto.PolicyID{Tier: "default", Name: "pol1"}, @@ -311,7 +306,7 @@ var _ = Describe("Raw egress policy manager", func() { err := policyMgr.CompleteDeferredWork() Expect(err).NotTo(HaveOccurred()) - Expect(neededIPSets).To(MatchIPSets("ipsetA")) + Expect(policyMgr.IPSetNeeded("cali40ipsetA")).To(BeTrue()) By("defining another untracked policy with a different IP set") policyMgr.OnUpdate(&proto.ActivePolicyUpdate{ @@ -329,7 +324,8 @@ var _ = Describe("Raw egress policy manager", func() { err = policyMgr.CompleteDeferredWork() Expect(err).NotTo(HaveOccurred()) - Expect(neededIPSets).To(MatchIPSets("ipsetA", "ipsetB")) + Expect(policyMgr.IPSetNeeded("cali40ipsetB")).To(BeTrue()) + Expect(policyMgr.IPSetNeeded("cali40ipsetA")).To(BeTrue()) By("defining a non-untracked policy with a third IP set") policyMgr.OnUpdate(&proto.ActivePolicyUpdate{ @@ -346,8 +342,8 @@ var _ = Describe("Raw egress policy manager", func() { err = policyMgr.CompleteDeferredWork() Expect(err).NotTo(HaveOccurred()) - // The non-untracked policy IP set is not needed. - Expect(neededIPSets).To(MatchIPSets("ipsetA", "ipsetB")) + // The non-untracked policy IP set is not cached. + Expect(policyMgr.IPSetNeeded("cali40ipsetC")).To(BeFalse()) By("removing the first untracked policy") policyMgr.OnUpdate(&proto.ActivePolicyRemove{ @@ -356,7 +352,8 @@ var _ = Describe("Raw egress policy manager", func() { err = policyMgr.CompleteDeferredWork() Expect(err).NotTo(HaveOccurred()) - Expect(neededIPSets).To(MatchIPSets("ipsetB")) + Expect(policyMgr.IPSetNeeded("cali40ipsetB")).To(BeTrue()) + Expect(policyMgr.IPSetNeeded("cali40ipsetA")).To(BeFalse()) By("removing the second untracked policy") policyMgr.OnUpdate(&proto.ActivePolicyRemove{ @@ -364,8 +361,9 @@ var _ = Describe("Raw egress policy manager", func() { }) err = policyMgr.CompleteDeferredWork() Expect(err).NotTo(HaveOccurred()) - - Expect(neededIPSets).To(MatchIPSets()) + Expect(policyMgr.IPSetNeeded("cali40ipsetB")).To(BeFalse()) + Expect(policyMgr.IPSetNeeded("cali40ipsetA")).To(BeFalse()) + Expect(policyMgr.IPSetNeeded("cali40ipsetC")).To(BeFalse()) }) }) diff --git a/felix/dataplane/windows/ipsets/ipsets.go b/felix/dataplane/windows/ipsets/ipsets.go index 755e01aaba9..e76bf8b0eb6 100644 --- a/felix/dataplane/windows/ipsets/ipsets.go +++ b/felix/dataplane/windows/ipsets/ipsets.go @@ -194,7 +194,9 @@ func (m *IPSets) ApplyDeletions() bool { } func (m *IPSets) SetFilter(fn func(ipSetName string) bool) { + // Not needed for Windows. } -func (m *IPSets) MarkDirty(ipsetNames set.Set[string]) { +func (m *IPSets) ApplyFilter() { + // Not needed for Windows. } diff --git a/felix/ipsets/ipsets.go b/felix/ipsets/ipsets.go index 2684a921f8f..fe0caa5e4d0 100644 --- a/felix/ipsets/ipsets.go +++ b/felix/ipsets/ipsets.go @@ -97,8 +97,11 @@ type IPSets struct { opReporter logutils.OpRecorder - // Optional filter. When non-nil, only these IP set IDs will be rendered into the dataplane - // as Linux IP sets. + // filterIPSet is set to filter the needed ipsets based on the + // ipset name. + // when not set (nil), all the ipsets will be programmed. + // when the filter returns true, the ipset will be programmed. + // when the filter returns false, the ipset will be skipped. filterIPSet func(string) bool } @@ -1092,6 +1095,8 @@ func (s *IPSets) updateDirtiness(name string) { } } +// SetFilter updates the ipset filter function but does +// not scan the existing ipsets and apply the filter. func (s *IPSets) SetFilter(fn func(string) bool) { s.filterIPSet = fn } @@ -1106,9 +1111,13 @@ func (s *IPSets) ipSetNeeded(name string) bool { return s.filterIPSet(name) } -func (s *IPSets) MarkDirty(ipsetNames set.Set[string]) { +// ApplyFilter applies the ipset filter to the existing +// ipsets. The caller should call ApplyFilter after updating +// the filter program to make sure the filter is applied to +// the existing ipsets. +func (s *IPSets) ApplyFilter() { for name, meta := range s.setNameToAllMetadata { - if ipsetNames.Contains(name) { + if s.ipSetNeeded(name) { s.setNameToProgrammedMetadata.Desired().Set(name, meta) } else { s.setNameToProgrammedMetadata.Desired().Delete(name) diff --git a/felix/ipsets/ipsets_test.go b/felix/ipsets/ipsets_test.go index a7eeb697eab..1a78e6a45f7 100644 --- a/felix/ipsets/ipsets_test.go +++ b/felix/ipsets/ipsets_test.go @@ -706,7 +706,7 @@ var _ = Describe("IP sets dataplane", func() { neededIPSets := set.From(v4MainIPSetName2) return neededIPSets.Contains(ipSetName) }) - ipsets.MarkDirty(set.From(v4MainIPSetName2)) + ipsets.ApplyFilter() apply() }) @@ -725,8 +725,7 @@ var _ = Describe("IP sets dataplane", func() { neededIPSets := set.From(v4MainIPSetName2, v4MainIPSetName) return neededIPSets.Contains(ipSetName) }) - ipsets.MarkDirty(set.From(v4MainIPSetName2, v4MainIPSetName)) - + ipsets.ApplyFilter() apply() }) diff --git a/felix/nftables/ipsets.go b/felix/nftables/ipsets.go index 6848fe81ede..7f884f5c06d 100644 --- a/felix/nftables/ipsets.go +++ b/felix/nftables/ipsets.go @@ -95,8 +95,11 @@ type IPSets struct { logCxt *log.Entry - // Optional filter. When non-nil, only these IP set IDs will be rendered into the dataplane - // as Linux IP sets. + // filterIPSet is set to filter the needed ipsets based on the + // ipset name. + // when not set (nil), all the ipsets will be programmed. + // when the filter returns true, the ipset will be programmed. + // when the filter returns false, the ipset will be skipped. filterIPSet func(string) bool nft knftables.Interface @@ -781,13 +784,19 @@ func (s *IPSets) updateDirtiness(name string) { } } +// SetFilter updates the ipset filter function but does +// not scan the existing ipsets and apply the filter. func (s *IPSets) SetFilter(fn func(ipSetName string) bool) { s.filterIPSet = fn } -func (s *IPSets) MarkDirty(ipsetNames set.Set[string]) { +// ApplyFilter applies the ipset filter to the existing +// ipsets. The caller should call ApplyFilter after updating +// the filter program to make sure the filter is applied to +// the existing ipsets. +func (s *IPSets) ApplyFilter() { for name, meta := range s.setNameToAllMetadata { - if ipsetNames.Contains(name) { + if s.ipSetNeeded(name) { s.setNameToProgrammedMetadata.Desired().Set(name, meta) } else { s.setNameToProgrammedMetadata.Desired().Delete(name)