From b2542899ae59cd386bb9afb437929c0c1b67007b Mon Sep 17 00:00:00 2001 From: Casey Davenport Date: Wed, 27 Nov 2024 11:05:01 -0800 Subject: [PATCH] Use longer (and thus more descriptive) nftables chain names (#9528) * Use longer (and thus more descriptive) nftables chain names * Fix build --- felix/dataplane/linux/int_dataplane.go | 8 ++++---- felix/dataplane/linux/policy_mgr.go | 24 +++++++++++++---------- felix/dataplane/linux/policy_mgr_test.go | 12 ++++++------ felix/rules/endpoints.go | 4 +++- felix/rules/policy.go | 25 ++++++++++++++++-------- 5 files changed, 44 insertions(+), 29 deletions(-) diff --git a/felix/dataplane/linux/int_dataplane.go b/felix/dataplane/linux/int_dataplane.go index 0d1945a4b40..9ccafd9fa7d 100644 --- a/felix/dataplane/linux/int_dataplane.go +++ b/felix/dataplane/linux/int_dataplane.go @@ -758,7 +758,7 @@ func NewIntDataplaneDriver(config Config) *InternalDataplane { rules.IPSetIDThisHostIPs, ipSetsV4, config.MaxIPSetSize)) - dp.RegisterManager(newPolicyManager(rawTableV4, mangleTableV4, filterTableV4, ruleRenderer, 4)) + dp.RegisterManager(newPolicyManager(rawTableV4, mangleTableV4, filterTableV4, ruleRenderer, 4, config.RulesConfig.NFTables)) // Clean up any leftover BPF state. err := bpfnat.RemoveConnectTimeLoadBalancer("") @@ -769,7 +769,7 @@ 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)) + dp.RegisterManager(newRawEgressPolicyManager(rawTableV4, ruleRenderer, 4, ipSetsV4.SetFilter, config.RulesConfig.NFTables)) } interfaceRegexes := make([]string, len(config.RulesConfig.WorkloadIfacePrefixes)) @@ -1032,9 +1032,9 @@ func NewIntDataplaneDriver(config Config) *InternalDataplane { rules.IPSetIDThisHostIPs, ipSetsV6, config.MaxIPSetSize)) - dp.RegisterManager(newPolicyManager(rawTableV6, mangleTableV6, filterTableV6, ruleRenderer, 6)) + dp.RegisterManager(newPolicyManager(rawTableV6, mangleTableV6, filterTableV6, ruleRenderer, 6, config.RulesConfig.NFTables)) } else { - dp.RegisterManager(newRawEgressPolicyManager(rawTableV6, ruleRenderer, 6, ipSetsV6.SetFilter)) + dp.RegisterManager(newRawEgressPolicyManager(rawTableV6, ruleRenderer, 6, ipSetsV6.SetFilter, config.RulesConfig.NFTables)) } dp.RegisterManager(newEndpointManager( diff --git a/felix/dataplane/linux/policy_mgr.go b/felix/dataplane/linux/policy_mgr.go index 5e97855425d..050dd7368c7 100644 --- a/felix/dataplane/linux/policy_mgr.go +++ b/felix/dataplane/linux/policy_mgr.go @@ -37,6 +37,7 @@ type policyManager struct { ipSetFilterDirty bool // Only used in "raw only" mode. neededIPSets map[proto.PolicyID]set.Set[string] ipSetsCallback func(neededIPSets set.Set[string]) + nftablesEnabled bool } type policyRenderer interface { @@ -44,18 +45,20 @@ type policyRenderer interface { ProfileToIptablesChains(profileID *proto.ProfileID, policy *proto.Profile, ipVersion uint8) (inbound, outbound *generictables.Chain) } -func newPolicyManager(rawTable, mangleTable, filterTable Table, ruleRenderer policyRenderer, ipVersion uint8) *policyManager { +func newPolicyManager(rawTable, mangleTable, filterTable Table, ruleRenderer policyRenderer, ipVersion uint8, nft bool) *policyManager { return &policyManager{ - rawTable: rawTable, - mangleTable: mangleTable, - filterTable: filterTable, - ruleRenderer: ruleRenderer, - ipVersion: ipVersion, + rawTable: rawTable, + mangleTable: mangleTable, + filterTable: filterTable, + ruleRenderer: ruleRenderer, + ipVersion: ipVersion, + nftablesEnabled: nft, } } func newRawEgressPolicyManager(rawTable Table, ruleRenderer policyRenderer, ipVersion uint8, ipSetsCallback func(neededIPSets set.Set[string]), + nft bool, ) *policyManager { return &policyManager{ rawTable: rawTable, @@ -68,6 +71,7 @@ func newRawEgressPolicyManager(rawTable Table, ruleRenderer policyRenderer, ipVe ipSetFilterDirty: true, neededIPSets: make(map[proto.PolicyID]set.Set[string]), ipSetsCallback: ipSetsCallback, + nftablesEnabled: nft, } } @@ -113,8 +117,8 @@ func (m *policyManager) OnUpdate(msg interface{}) { m.mangleTable.UpdateChains([]*generictables.Chain{outbound}) case *proto.ActiveProfileRemove: log.WithField("id", msg.Id).Debug("Removing profile chains") - inName := rules.ProfileChainName(rules.ProfileInboundPfx, msg.Id) - outName := rules.ProfileChainName(rules.ProfileOutboundPfx, msg.Id) + inName := rules.ProfileChainName(rules.ProfileInboundPfx, msg.Id, m.nftablesEnabled) + outName := rules.ProfileChainName(rules.ProfileOutboundPfx, msg.Id, m.nftablesEnabled) m.filterTable.RemoveChainByName(inName) m.filterTable.RemoveChainByName(outName) m.mangleTable.RemoveChainByName(outName) @@ -125,8 +129,8 @@ func (m *policyManager) cleanUpPolicy(id *proto.PolicyID) { if m.rawEgressOnly { m.updateNeededIPSets(id, nil) } - inName := rules.PolicyChainName(rules.PolicyInboundPfx, id) - outName := rules.PolicyChainName(rules.PolicyOutboundPfx, id) + inName := rules.PolicyChainName(rules.PolicyInboundPfx, id, m.nftablesEnabled) + outName := rules.PolicyChainName(rules.PolicyOutboundPfx, id, m.nftablesEnabled) // As above, we need to clean up in all the tables. m.filterTable.RemoveChainByName(inName) m.filterTable.RemoveChainByName(outName) diff --git a/felix/dataplane/linux/policy_mgr_test.go b/felix/dataplane/linux/policy_mgr_test.go index 6b715913004..596f02a6b16 100644 --- a/felix/dataplane/linux/policy_mgr_test.go +++ b/felix/dataplane/linux/policy_mgr_test.go @@ -41,7 +41,7 @@ var _ = Describe("Policy manager", func() { mangleTable = newMockTable("mangle") filterTable = newMockTable("filter") ruleRenderer = newMockPolRenderer() - policyMgr = newPolicyManager(rawTable, mangleTable, filterTable, ruleRenderer, 4) + policyMgr = newPolicyManager(rawTable, mangleTable, filterTable, ruleRenderer, 4, false) }) It("shouldn't touch iptables", func() { @@ -277,7 +277,7 @@ var _ = Describe("Raw egress policy manager", func() { func(ipSets set.Set[string]) { neededIPSets = ipSets numCallbackCalls++ - }) + }, false) }) It("correctly reports no IP sets at start of day", func() { @@ -399,8 +399,8 @@ func (m *ipSetsMatcher) NegatedFailureMessage(actual interface{}) (message strin type mockPolRenderer struct{} func (r *mockPolRenderer) PolicyToIptablesChains(policyID *proto.PolicyID, policy *proto.Policy, ipVersion uint8) []*generictables.Chain { - inName := rules.PolicyChainName(rules.PolicyInboundPfx, policyID) - outName := rules.PolicyChainName(rules.PolicyOutboundPfx, policyID) + inName := rules.PolicyChainName(rules.PolicyInboundPfx, policyID, false) + outName := rules.PolicyChainName(rules.PolicyOutboundPfx, policyID, false) return []*generictables.Chain{ {Name: inName}, {Name: outName}, @@ -409,10 +409,10 @@ func (r *mockPolRenderer) PolicyToIptablesChains(policyID *proto.PolicyID, polic func (r *mockPolRenderer) ProfileToIptablesChains(profID *proto.ProfileID, policy *proto.Profile, ipVersion uint8) (inbound, outbound *generictables.Chain) { inbound = &generictables.Chain{ - Name: rules.ProfileChainName(rules.ProfileInboundPfx, profID), + Name: rules.ProfileChainName(rules.ProfileInboundPfx, profID, false), } outbound = &generictables.Chain{ - Name: rules.ProfileChainName(rules.ProfileOutboundPfx, profID), + Name: rules.ProfileChainName(rules.ProfileOutboundPfx, profID, false), } return } diff --git a/felix/rules/endpoints.go b/felix/rules/endpoints.go index c45beda056d..a181ff51be1 100644 --- a/felix/rules/endpoints.go +++ b/felix/rules/endpoints.go @@ -362,6 +362,7 @@ func (r *DefaultRuleRenderer) PolicyGroupToIptablesChains(group *PolicyGroup) [] chainToJumpTo := PolicyChainName( polChainPrefix, &proto.PolicyID{Tier: group.Tier, Name: polName}, + r.NFTables, ) rules = append(rules, generictables.Rule{ Match: match, @@ -473,6 +474,7 @@ func (r *DefaultRuleRenderer) endpointIptablesChain( chainsToJumpTo = append(chainsToJumpTo, PolicyChainName( policyPrefix, &proto.PolicyID{Tier: tier.Name, Name: p}, + r.NFTables, )) } } else { @@ -541,7 +543,7 @@ func (r *DefaultRuleRenderer) endpointIptablesChain( if chainType == chainTypeNormal { // Then, jump to each profile in turn. for _, profileID := range profileIds { - profChainName := ProfileChainName(profilePrefix, &proto.ProfileID{Name: profileID}) + profChainName := ProfileChainName(profilePrefix, &proto.ProfileID{Name: profileID}, r.NFTables) rules = append(rules, generictables.Rule{Match: r.NewMatch(), Action: r.Jump(profChainName)}, // If policy marked packet as accepted, it returns, setting the diff --git a/felix/rules/policy.go b/felix/rules/policy.go index bcd7023ccc8..16bc4d87895 100644 --- a/felix/rules/policy.go +++ b/felix/rules/policy.go @@ -24,6 +24,7 @@ import ( "github.com/projectcalico/calico/felix/hashutils" "github.com/projectcalico/calico/felix/ipsets" "github.com/projectcalico/calico/felix/iptables" + "github.com/projectcalico/calico/felix/nftables" "github.com/projectcalico/calico/felix/proto" ) @@ -31,12 +32,12 @@ import ( func (r *DefaultRuleRenderer) PolicyToIptablesChains(policyID *proto.PolicyID, policy *proto.Policy, ipVersion uint8) []*generictables.Chain { inbound := generictables.Chain{ - Name: PolicyChainName(PolicyInboundPfx, policyID), + Name: PolicyChainName(PolicyInboundPfx, policyID, r.NFTables), // Note that the policy name includes the tier, so it does not need to be separately specified. Rules: r.ProtoRulesToIptablesRules(policy.InboundRules, ipVersion, fmt.Sprintf("Policy %s ingress", policyID.Name)), } outbound := generictables.Chain{ - Name: PolicyChainName(PolicyOutboundPfx, policyID), + Name: PolicyChainName(PolicyOutboundPfx, policyID, r.NFTables), // Note that the policy name also includes the tier, so it does not need to be separately specified. Rules: r.ProtoRulesToIptablesRules(policy.OutboundRules, ipVersion, fmt.Sprintf("Policy %s egress", policyID.Name)), } @@ -45,11 +46,11 @@ func (r *DefaultRuleRenderer) PolicyToIptablesChains(policyID *proto.PolicyID, p func (r *DefaultRuleRenderer) ProfileToIptablesChains(profileID *proto.ProfileID, profile *proto.Profile, ipVersion uint8) (inbound, outbound *generictables.Chain) { inbound = &generictables.Chain{ - Name: ProfileChainName(ProfileInboundPfx, profileID), + Name: ProfileChainName(ProfileInboundPfx, profileID, r.NFTables), Rules: r.ProtoRulesToIptablesRules(profile.InboundRules, ipVersion, fmt.Sprintf("Profile %s ingress", profileID.Name)), } outbound = &generictables.Chain{ - Name: ProfileChainName(ProfileOutboundPfx, profileID), + Name: ProfileChainName(ProfileOutboundPfx, profileID, r.NFTables), Rules: r.ProtoRulesToIptablesRules(profile.OutboundRules, ipVersion, fmt.Sprintf("Profile %s egress", profileID.Name)), } return @@ -797,18 +798,26 @@ func (r *DefaultRuleRenderer) CalculateRuleMatch(pRule *proto.Rule, ipVersion ui return match } -func PolicyChainName(prefix PolicyChainNamePrefix, polID *proto.PolicyID) string { +func PolicyChainName(prefix PolicyChainNamePrefix, polID *proto.PolicyID, nft bool) string { + maxLen := iptables.MaxChainNameLength + if nft { + maxLen = nftables.MaxChainNameLength + } return hashutils.GetLengthLimitedID( string(prefix), polID.Tier+"/"+polID.Name, - iptables.MaxChainNameLength, + maxLen, ) } -func ProfileChainName(prefix ProfileChainNamePrefix, profID *proto.ProfileID) string { +func ProfileChainName(prefix ProfileChainNamePrefix, profID *proto.ProfileID, nft bool) string { + maxLen := iptables.MaxChainNameLength + if nft { + maxLen = nftables.MaxChainNameLength + } return hashutils.GetLengthLimitedID( string(prefix), profID.Name, - iptables.MaxChainNameLength, + maxLen, ) }