Automated backport of #3979: Fix IPv6 protocol conflict in nftables rules

.github/workflows/e2e.yml

@@ -60,6 +60,7 @@ jobs:
           - extra-toggles: dual-stack, globalnet
           - extra-toggles: ipv6-stack
           - extra-toggles: nftables
+          - extra-toggles: nftables, dual-stack
           - extra-toggles: nftables, ovn
           - extra-toggles: nftables, globalnet
           - extra-toggles: nftables, globalnet, ovn

pkg/packetfilter/nftables/nftables.go

@@ -58,13 +58,12 @@ var (
 	}
 
 	ruleActionToStr = map[packetfilter.RuleAction][]string{
-		packetfilter.RuleActionAccept:   {"accept"},
-		packetfilter.RuleActionMss:      {"tcp", "option", "maxseg"},
-		packetfilter.RuleActionMark:     {"meta", "mark"},
-		packetfilter.RuleActionSNAT:     {"snat"},
-		packetfilter.RuleActionDNAT:     {"dnat"},
-		packetfilter.RuleActionJump:     {"jump"},
-		packetfilter.RuleActionSelfSNAT: {"snat to ip saddr"},
+		packetfilter.RuleActionAccept: {"accept"},
+		packetfilter.RuleActionMss:    {"tcp", "option", "maxseg"},
+		packetfilter.RuleActionMark:   {"meta", "mark"},
+		packetfilter.RuleActionSNAT:   {"snat"},
+		packetfilter.RuleActionDNAT:   {"dnat"},
+		packetfilter.RuleActionJump:   {"jump"},
 	}
 
 	nftFamilies = map[k8snet.IPFamily]knftables.Family{

pkg/packetfilter/nftables/nftables_test.go

@@ -400,6 +400,57 @@ var _ = Describe("Interface", func() {
 	})
 })
 
+var _ = Describe("IPv6 rules", func() {
+	const chainName6 = "test-chain"
+
+	var (
+		fakeNft6 *fakeKnftablesWrapper
+		pf6      packetfilter.Driver
+	)
+
+	BeforeEach(func() {
+		fakeNft6 = &fakeKnftablesWrapper{knftables.NewFake(knftables.IPv6Family, "submariner-ipv6")}
+		pf6 = nftables.NewWithNft(fakeNft6, k8snet.IPv6)
+
+		Expect(pf6.CreateChainIfNotExists(packetfilter.TableTypeNAT, &packetfilter.Chain{Name: chainName6})).To(Succeed())
+	})
+
+	getRuleSpec6 := func() string {
+		rules, err := fakeNft6.Fake.ListRules(context.TODO(), chainName6)
+		Expect(err).To(Succeed())
+		Expect(rules).To(HaveLen(1))
+
+		return rules[0].Rule
+	}
+
+	Specify("should use ip6 nexthdr for UDP", func() {
+		Expect(pf6.Append(packetfilter.TableTypeNAT, chainName6, &packetfilter.Rule{
+			Proto: packetfilter.RuleProtoUDP, DPort: "4500", Action: packetfilter.RuleActionAccept,
+		})).To(Succeed())
+		Expect(getRuleSpec6()).To(And(
+			ContainSubstring("ip6 nexthdr udp"),
+			Not(ContainSubstring("ip protocol udp")),
+		))
+	})
+
+	Specify("should use ip6 nexthdr icmpv6 for ICMP", func() {
+		Expect(pf6.Append(packetfilter.TableTypeNAT, chainName6, &packetfilter.Rule{
+			Proto: packetfilter.RuleProtoICMP, Action: packetfilter.RuleActionAccept,
+		})).To(Succeed())
+		Expect(getRuleSpec6()).To(And(
+			ContainSubstring("ip6 nexthdr icmpv6"),
+			Not(ContainSubstring("ip nexthdr icmp")),
+		))
+	})
+
+	Specify("should use ip6 saddr for SelfSNAT", func() {
+		Expect(pf6.Append(packetfilter.TableTypeNAT, chainName6, &packetfilter.Rule{
+			Action: packetfilter.RuleActionSelfSNAT,
+		})).To(Succeed())
+		Expect(getRuleSpec6()).To(ContainSubstring("snat to ip6 saddr"))
+	})
+})
+
 func testRuleConversion(rule *packetfilter.Rule) {
 	serialized := nftables.SerializeRule(rule)
 	Expect(len(serialized)).To(BeNumerically("<=", 128))

pkg/packetfilter/nftables/rule_conversion.go

@@ -29,22 +29,35 @@ import (
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 )
 
-const serializedVersion = "1.0"
+const (
+	serializedVersion = "1.0"
+	ipv6Family        = "ip6"
+)
+
+func protoToRuleSpec(ruleSpec []string, proto packetfilter.RuleProto, dPort string, isIPv6 bool) []string {
+	addrFamily, protoKW := "ip", "protocol"
+	if isIPv6 {
+		addrFamily, protoKW = ipv6Family, "nexthdr"
+	}
 
-func protoToRuleSpec(ruleSpec []string, proto packetfilter.RuleProto, dPort string) []string {
 	switch proto {
 	case packetfilter.RuleProtoUDP:
-		ruleSpec = append(ruleSpec, "ip", "protocol", "udp")
+		ruleSpec = append(ruleSpec, addrFamily, protoKW, "udp")
 		if dPort != "" {
 			ruleSpec = append(ruleSpec, "udp", "dport", dPort)
 		}
 	case packetfilter.RuleProtoTCP:
-		ruleSpec = append(ruleSpec, "ip", "protocol", "tcp")
+		ruleSpec = append(ruleSpec, addrFamily, protoKW, "tcp")
 		if dPort != "" {
 			ruleSpec = append(ruleSpec, "tcp", "dport", dPort)
 		}
 	case packetfilter.RuleProtoICMP:
-		ruleSpec = append(ruleSpec, "ip", "protocol", "icmp")
+		icmpProto := "icmp"
+		if isIPv6 {
+			icmpProto = "icmpv6"
+		}
+
+		ruleSpec = append(ruleSpec, addrFamily, protoKW, icmpProto)
 	case packetfilter.RuleProtoAll:
 	case packetfilter.RuleProtoUndefined:
 	}
@@ -64,11 +77,11 @@ func mssClampToRuleSpec(ruleSpec []string, clampType packetfilter.MssClampType,
 	return ruleSpec
 }
 
-func setToRuleSpec(ruleSpec []string, srcSetName, destSetName string, isIPv6Set bool) []string {
+func setToRuleSpec(ruleSpec []string, srcSetName, destSetName string, isIPv6 bool) []string {
 	setPrefix := "ip"
 
-	if isIPv6Set {
-		setPrefix = "ip6"
+	if isIPv6 {
+		setPrefix = ipv6Family
 	}
 
 	if srcSetName != "" {
@@ -82,13 +95,13 @@ func setToRuleSpec(ruleSpec []string, srcSetName, destSetName string, isIPv6Set
 	return ruleSpec
 }
 
-func toNftRuleSpec(rule *packetfilter.Rule, isIPv6Set bool) string {
-	ruleSpec := protoToRuleSpec([]string{}, rule.Proto, rule.DPort)
+func toNftRuleSpec(rule *packetfilter.Rule, isIPv6 bool) string {
+	ruleSpec := protoToRuleSpec([]string{}, rule.Proto, rule.DPort, isIPv6)
 
 	setPrefix := "ip"
 
-	if isIPv6Set {
-		setPrefix = "ip6"
+	if isIPv6 {
+		setPrefix = ipv6Family
 	}
 
 	if rule.SrcCIDR != "" {
@@ -104,7 +117,7 @@ func toNftRuleSpec(rule *packetfilter.Rule, isIPv6Set bool) string {
 		ruleSpec = append(ruleSpec, "meta", "mark", "&", rule.MarkValue, "==", rule.MarkValue)
 	}
 
-	ruleSpec = setToRuleSpec(ruleSpec, rule.SrcSetName, rule.DestSetName, isIPv6Set)
+	ruleSpec = setToRuleSpec(ruleSpec, rule.SrcSetName, rule.DestSetName, isIPv6)
 
 	if rule.OutInterface != "" {
 		ruleSpec = append(ruleSpec, "oifname", rule.OutInterface)
@@ -119,7 +132,12 @@ func toNftRuleSpec(rule *packetfilter.Rule, isIPv6Set bool) string {
 	}
 
 	ruleSpec = append(ruleSpec, "counter")
-	ruleSpec = append(ruleSpec, ruleActionToStr[rule.Action]...)
+
+	if rule.Action == packetfilter.RuleActionSelfSNAT {
+		ruleSpec = append(ruleSpec, "snat", "to", setPrefix, "saddr")
+	} else {
+		ruleSpec = append(ruleSpec, ruleActionToStr[rule.Action]...)
+	}
 
 	if rule.Action == packetfilter.RuleActionJump {
 		ruleSpec = append(ruleSpec, rule.TargetChain)