diff --git a/dist/images/vpcnatgateway/nat-gateway.sh b/dist/images/vpcnatgateway/nat-gateway.sh index 18974d9c577..1230debfa9a 100644 --- a/dist/images/vpcnatgateway/nat-gateway.sh +++ b/dist/images/vpcnatgateway/nat-gateway.sh @@ -39,6 +39,8 @@ function show_help() { echo " dnat-del - Delete DNAT rule" echo " snat-add - Add SNAT rule" echo " snat-del - Delete SNAT rule" + echo " hairpin-snat-add - Add hairpin SNAT rule for internal FIP access" + echo " hairpin-snat-del - Delete hairpin SNAT rule" echo " qos-add - Add QoS rule" echo " qos-del - Delete QoS rule" echo " eip-ingress-qos-add - Add EIP ingress QoS" @@ -124,6 +126,7 @@ function init() { $iptables_cmd -t nat -N SNAT_FILTER $iptables_cmd -t nat -N EXCLUSIVE_DNAT # floatingIp DNAT $iptables_cmd -t nat -N EXCLUSIVE_SNAT # floatingIp SNAT + $iptables_cmd -t nat -N HAIRPIN_SNAT # hairpin SNAT for internal FIP access $iptables_cmd -t nat -N SHARED_DNAT $iptables_cmd -t nat -N SHARED_SNAT @@ -134,6 +137,7 @@ function init() { $iptables_cmd -t nat -A POSTROUTING -j SNAT_FILTER $iptables_cmd -t nat -A SNAT_FILTER -j EXCLUSIVE_SNAT $iptables_cmd -t nat -A SNAT_FILTER -j SHARED_SNAT + $iptables_cmd -t nat -A SNAT_FILTER -j HAIRPIN_SNAT # Send gratuitous ARP for all the IPs on the external network interface at initialization # This is especially useful to update the MAC of the nexthop we announce to the BGP speaker @@ -153,6 +157,26 @@ function get_iptables_version() { exec_cmd "$iptables_cmd --version" } +# Check if the given CIDR exists in VPC_INTERFACE's routes (indicates it's an internal CIDR) +# This is used to determine if hairpin SNAT is needed for a given SNAT rule +# Args: $1 - CIDR to check (e.g., "10.0.1.0/24") +# Returns: 0 if the CIDR is found in VPC_INTERFACE routes, 1 otherwise +# Example: VPC_INTERFACE=eth0, route "10.0.1.0/24 dev eth0" exists +# is_internal_cidr "10.0.1.0/24" -> returns 0 (true) +# is_internal_cidr "192.168.1.0/24" -> returns 1 (false, not in routes) +function is_internal_cidr() { + local cidr="$1" + if [ -z "$cidr" ]; then + return 1 + fi + # Match CIDR at the start of line to ensure exact match + # e.g., "10.0.1.0/24 dev eth0" matches, but "10.0.1.0/25 ..." does not + if ip -4 route show dev "$VPC_INTERFACE" | grep -q "^$cidr "; then + return 0 + fi + return 1 +} + function add_vpc_internal_route() { # make sure inited check_inited @@ -270,11 +294,20 @@ function add_snat() { eip=(${arr[0]//\// }) internalCIDR=${arr[1]} randomFullyOption=${arr[2]} - # check if already exist - $iptables_save_cmd | grep SHARED_SNAT | grep "\-s $internalCIDR" | grep "source $eip" && exit 0 - exec_cmd "$iptables_cmd -t nat -A SHARED_SNAT -o $EXTERNAL_INTERFACE -s $internalCIDR -j SNAT --to-source $eip $randomFullyOption" + # check if already exist, skip adding if exists (idempotent) + ruleMatch=$($iptables_save_cmd | grep SHARED_SNAT | grep -w -- "-s $internalCIDR" | grep -E -- "--to-source $eip(\$| )") + if [ -z "$ruleMatch" ]; then + exec_cmd "$iptables_cmd -t nat -A SHARED_SNAT -o $EXTERNAL_INTERFACE -s $internalCIDR -j SNAT --to-source $eip $randomFullyOption" + fi + # Add hairpin SNAT when internalCIDR is routed via VPC_INTERFACE + # This enables internal VMs to access other internal VMs via FIP + if is_internal_cidr "$internalCIDR"; then + echo "SNAT cidr $internalCIDR is internal, adding hairpin SNAT with EIP $eip" + add_hairpin_snat "$eip,$internalCIDR" + fi done } + function del_snat() { # make sure inited check_inited @@ -290,6 +323,79 @@ function del_snat() { ruleMatch=$(echo $ruleMatch | sed 's/-A //') exec_cmd "$iptables_cmd -t nat -D $ruleMatch" fi + # Delete hairpin SNAT when internalCIDR is routed via VPC_INTERFACE + if is_internal_cidr "$internalCIDR"; then + echo "SNAT cidr $internalCIDR is internal, deleting hairpin SNAT with EIP $eip" + del_hairpin_snat "$eip,$internalCIDR" + fi + done +} + +# Hairpin SNAT: Enables internal VM to access another internal VM's FIP +# Packet flow when VM A accesses VM B's EIP: +# 1. VM A (10.0.1.6) -> EIP (10.1.69.216) arrives at NAT GW +# 2. DNAT translates destination to VM B's internal IP (10.0.1.11) +# 3. Without hairpin SNAT, reply from VM B goes directly to VM A (same subnet), +# but VM A expects reply from EIP, causing asymmetric routing failure +# 4. Hairpin SNAT translates source to EIP, ensuring symmetric return path via NAT GW +# +# RECOMMENDED: NAT-GW binds to a single VPC internal subnet. In this case, +# only one hairpin SNAT rule is needed (matching the VPC's directly connected route). +# +# Multi-subnet scenarios are supported but NOT recommended. For multiple subnets, +# create separate NAT gateways for each subnet to achieve more direct forwarding paths. +# Each CIDR can only have one hairpin rule to avoid conflicting SNAT sources. +# +# Rule format: eip,internalCIDR +# Example: 10.1.69.219,10.0.1.0/24 +# Creates: iptables -t nat -A HAIRPIN_SNAT -s 10.0.1.0/24 -d 10.0.1.0/24 -j SNAT --to-source 10.1.69.219 +function add_hairpin_snat() { + # make sure inited + check_inited + local all_hairpin_rules + all_hairpin_rules=$($iptables_save_cmd -t nat | grep HAIRPIN_SNAT) + for rule in $@ + do + arr=(${rule//,/ }) + eip=(${arr[0]//\// }) + internalCIDR=${arr[1]} + + # Filter from cached rules for this specific CIDR + local existing_rules + existing_rules=$(echo "$all_hairpin_rules" | grep -w -- "-s $internalCIDR" | grep -w -- "-d $internalCIDR") + + # Check if this exact rule already exists (idempotent) + if echo "$existing_rules" | grep -qE -- "--to-source $eip(\$| )"; then + echo "Hairpin SNAT rule for $internalCIDR with EIP $eip already exists, skipping" + continue + fi + + # Check if this CIDR already has a hairpin rule with a different EIP + if [ -n "$existing_rules" ]; then + echo "WARNING: Hairpin SNAT rule for $internalCIDR already exists with different EIP. Skipping." + continue + fi + + exec_cmd "$iptables_cmd -t nat -A HAIRPIN_SNAT -s $internalCIDR -d $internalCIDR -j SNAT --to-source $eip" + echo "Hairpin SNAT rule added: $internalCIDR -> $eip" + done +} + +function del_hairpin_snat() { + # make sure inited + check_inited + local all_hairpin_rules + all_hairpin_rules=$($iptables_save_cmd -t nat | grep HAIRPIN_SNAT) + for rule in $@ + do + arr=(${rule//,/ }) + eip=(${arr[0]//\// }) + internalCIDR=${arr[1]} + # check if rule exists (idempotent - skip if not found) + if echo "$all_hairpin_rules" | grep -w -- "-s $internalCIDR" | grep -w -- "-d $internalCIDR" | grep -qE -- "--to-source $eip(\$| )"; then + exec_cmd "$iptables_cmd -t nat -D HAIRPIN_SNAT -s $internalCIDR -d $internalCIDR -j SNAT --to-source $eip" + echo "Hairpin SNAT rule deleted: $internalCIDR -> $eip" + fi done } @@ -562,6 +668,14 @@ case $opt in echo "floating-ip-del $rules" del_floating_ip $rules ;; + hairpin-snat-add) + echo "hairpin-snat-add $rules" + add_hairpin_snat $rules + ;; + hairpin-snat-del) + echo "hairpin-snat-del $rules" + del_hairpin_snat $rules + ;; get-iptables-version) echo "get-iptables-version $rules" get_iptables_version $rules diff --git a/test/e2e/iptables-vpc-nat-gw/e2e_test.go b/test/e2e/iptables-vpc-nat-gw/e2e_test.go index 39a18585f52..d11b84b13ad 100644 --- a/test/e2e/iptables-vpc-nat-gw/e2e_test.go +++ b/test/e2e/iptables-vpc-nat-gw/e2e_test.go @@ -248,6 +248,64 @@ func verifySubnetStatusAfterEIPOperation(subnetClient *framework.SubnetClient, s } } +// checkHairpinSnatRuleExists checks if hairpin SNAT rule exists in the NAT gateway pod +// Returns true if rule exists, false otherwise (including when HAIRPIN_SNAT chain doesn't exist) +// This is used with gomega.Eventually for polling-based verification +func checkHairpinSnatRuleExists(natGwPodName, cidr, eip string) bool { + cmd := []string{"iptables-save", "-t", "nat"} + stdout, _, err := framework.KubectlExec(framework.KubeOvnNamespace, natGwPodName, cmd...) + if err != nil { + framework.Logf("Failed to exec iptables-save in NAT gateway pod %s: %v", natGwPodName, err) + return false + } + + iptablesOutput := string(stdout) + + // If HAIRPIN_SNAT chain doesn't exist, rule cannot exist + if !strings.Contains(iptablesOutput, ":HAIRPIN_SNAT") && !strings.Contains(iptablesOutput, "-N HAIRPIN_SNAT") { + return false + } + + hairpinRulePattern := fmt.Sprintf("-A HAIRPIN_SNAT -s %s -d %s -j SNAT --to-source %s", cidr, cidr, eip) + return strings.Contains(iptablesOutput, hairpinRulePattern) +} + +// verifyHairpinSnatRule verifies hairpin SNAT rule exists or not in the NAT gateway pod +// Hairpin SNAT enables internal VMs to access other internal VMs via their FIP/EIP +// The rule format: -A HAIRPIN_SNAT -s -d -j SNAT --to-source +// This feature was introduced in v1.15, so the function will skip verification +// if HAIRPIN_SNAT chain does not exist (for backward compatibility) +func verifyHairpinSnatRule(natGwPodName, cidr, eip string, shouldExist bool) { + ginkgo.GinkgoHelper() + + cmd := []string{"iptables-save", "-t", "nat"} + stdout, _, err := framework.KubectlExec(framework.KubeOvnNamespace, natGwPodName, cmd...) + framework.ExpectNoError(err, "failed to exec iptables-save in NAT gateway pod %s", natGwPodName) + + iptablesOutput := string(stdout) + + // Check if HAIRPIN_SNAT chain exists (feature introduced in v1.15) + // Skip verification if the chain doesn't exist for backward compatibility + if !strings.Contains(iptablesOutput, ":HAIRPIN_SNAT") && !strings.Contains(iptablesOutput, "-N HAIRPIN_SNAT") { + framework.Logf("HAIRPIN_SNAT chain not found, skipping hairpin SNAT verification (feature requires v1.15+)") + return + } + + // Check for hairpin SNAT rule pattern: -A HAIRPIN_SNAT -s -d -j SNAT --to-source + hairpinRulePattern := fmt.Sprintf("-A HAIRPIN_SNAT -s %s -d %s -j SNAT --to-source %s", cidr, cidr, eip) + ruleExists := strings.Contains(iptablesOutput, hairpinRulePattern) + + if shouldExist { + framework.ExpectTrue(ruleExists, + "Hairpin SNAT rule should exist: %s\niptables output:\n%s", hairpinRulePattern, iptablesOutput) + framework.Logf("Verified hairpin SNAT rule exists: %s", hairpinRulePattern) + } else { + framework.ExpectFalse(ruleExists, + "Hairpin SNAT rule should NOT exist: %s\niptables output:\n%s", hairpinRulePattern, iptablesOutput) + framework.Logf("Verified hairpin SNAT rule does not exist for CIDR %s", cidr) + } +} + var _ = framework.OrderedDescribe("[group:iptables-vpc-nat-gw]", func() { f := framework.NewDefaultFramework("iptables-vpc-nat-gw") @@ -456,6 +514,7 @@ var _ = framework.OrderedDescribe("[group:iptables-vpc-nat-gw]", func() { }) framework.ConformanceIt("[2] iptables EIP FIP SNAT DNAT", func() { + f.SkipVersionPriorTo(1, 15, "This feature was introduced in v1.15") // Test-specific variables randomSuffix := framework.RandomSuffix() fipVipName := "fip-vip-" + randomSuffix @@ -528,6 +587,12 @@ var _ = framework.OrderedDescribe("[group:iptables-vpc-nat-gw]", func() { iptablesSnatRuleClient.DeleteSync(snatName) }) + // Verify hairpin SNAT rule is automatically created for internal CIDR + ginkgo.By("Verifying hairpin SNAT rule exists in NAT gateway pod") + vpcNatGwPodName := util.GenNatGwPodName(vpcNatGwName) + snatEip = iptablesEIPClient.Get(snatEipName) + verifyHairpinSnatRule(vpcNatGwPodName, overlaySubnetV4Cidr, snatEip.Status.IP, true) + ginkgo.By("Creating iptables vip for dnat") dnatVip := framework.MakeVip(f.Namespace.Name, dnatVipName, overlaySubnetName, "", "", "") _ = vipClient.CreateSync(dnatVip) @@ -603,8 +668,13 @@ var _ = framework.OrderedDescribe("[group:iptables-vpc-nat-gw]", func() { iptablesSnatRuleClient.DeleteSync(sharedEipSnatName) }) - ginkgo.By("Get share eip") + // Verify hairpin SNAT rule is created for shared SNAT as well + ginkgo.By("Getting share eip for hairpin verification") shareEip = iptablesEIPClient.Get(sharedEipName) + framework.ExpectNotEmpty(shareEip.Status.IP, "shareEip.Status.IP should not be empty for hairpin verification") + ginkgo.By("Verifying hairpin SNAT rule exists for shared snat") + verifyHairpinSnatRule(vpcNatGwPodName, overlaySubnetV4Cidr, shareEip.Status.IP, true) + ginkgo.By("Get share dnat") shareDnat = iptablesDnatRuleClient.Get(sharedEipDnatName) ginkgo.By("Get share snat") @@ -628,6 +698,16 @@ var _ = framework.OrderedDescribe("[group:iptables-vpc-nat-gw]", func() { // make sure eip is shared nats := []string{util.DnatUsingEip, util.FipUsingEip, util.SnatUsingEip} framework.ExpectEqual(shareEip.Status.Nat, strings.Join(nats, ",")) + + // Verify hairpin SNAT rule cleanup when SNAT is deleted + ginkgo.By("Deleting snat to verify hairpin SNAT rule cleanup") + iptablesSnatRuleClient.DeleteSync(snatName) + ginkgo.By("Verifying hairpin SNAT rule is deleted after snat deletion") + gomega.Eventually(func() bool { + return checkHairpinSnatRuleExists(vpcNatGwPodName, overlaySubnetV4Cidr, snatEip.Status.IP) + }, 30*time.Second, 2*time.Second).Should(gomega.BeFalse(), + "Hairpin SNAT rule should be deleted after SNAT deletion") + // All cleanup is handled by DeferCleanup above, no need for manual cleanup })