From 4e888c72b645e03b3dd3237ec78175e56cfc44e7 Mon Sep 17 00:00:00 2001
From: Antonin Bas <abas@vmware.com>
Date: Fri, 8 Jul 2022 12:53:30 -0700
Subject: [PATCH] Add missing rules when NodePort support is disabled

* the rules that need to be installed for NodePort support and SNAT
  support are very similar. The same traffic mark is needed for both. As
  a result, rules that are currently installed only when NodePort
  support is enabled should also be installed when external SNAT is
  disabled, which is the case by default.
* remove a useless iptables rule in the nat table (for restoring the
  traffic mark); the rule would only apply to the first packet of each
  connection. The mangle table should be in charge of restoring the
  traffic mark.
* remove "-m state --state NEW" from a rule in the nat table. This is
  always true for packets that traverse the nat table.
* fix typo in one rule's name (extra whitespace).

Fixes #2025

Co-authored-by: Quan Tian <qtian@vmware.com>

Signed-off-by: Antonin Bas <abas@vmware.com>
---
 pkg/networkutils/network.go | 35 ++++++-----------------------------
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/pkg/networkutils/network.go b/pkg/networkutils/network.go
index a49935aecb0..d9904dc5574 100644
--- a/pkg/networkutils/network.go
+++ b/pkg/networkutils/network.go
@@ -276,7 +276,7 @@ func (n *linuxNetwork) SetupHostNetwork(vpcv4CIDRs []string, primaryMAC string,
 	var err error
 	primaryIntf := "eth0"
 	//RP Filter setting is only needed if IPv4 mode is enabled.
-	if v4Enabled && n.nodePortSupportEnabled {
+	if v4Enabled && (n.nodePortSupportEnabled || !n.useExternalSNAT) {
 		primaryIntf, err = findPrimaryInterfaceName(primaryMAC)
 		if err != nil {
 			return errors.Wrapf(err, "failed to SetupHostNetwork")
@@ -340,7 +340,7 @@ func (n *linuxNetwork) SetupHostNetwork(vpcv4CIDRs []string, primaryMAC string,
 		return errors.Wrapf(err, "host network setup: failed to delete old main ENI rule")
 	}
 
-	if n.nodePortSupportEnabled {
+	if n.nodePortSupportEnabled || !n.useExternalSNAT {
 		err = n.netLink.RuleAdd(mainENIRule)
 		if err != nil {
 			log.Errorf("Failed to add host main ENI rule: %v", err)
@@ -528,7 +528,7 @@ func (n *linuxNetwork) buildIptablesSNATRules(vpcCIDRs []string, primaryAddr *ne
 
 	iptableRules = append(iptableRules, iptablesRule{
 		name:        "connmark restore for primary ENI",
-		shouldExist: n.nodePortSupportEnabled,
+		shouldExist: n.nodePortSupportEnabled || !n.useExternalSNAT,
 		table:       "mangle",
 		chain:       "PREROUTING",
 		rule: []string{
@@ -539,7 +539,7 @@ func (n *linuxNetwork) buildIptablesSNATRules(vpcCIDRs []string, primaryAddr *ne
 
 	iptableRules = append(iptableRules, iptablesRule{
 		name:        "connmark restore for primary ENI from vlan",
-		shouldExist: n.nodePortSupportEnabled,
+		shouldExist: n.nodePortSupportEnabled || !n.useExternalSNAT,
 		table:       "mangle",
 		chain:       "PREROUTING",
 		rule: []string{
@@ -579,7 +579,7 @@ func (n *linuxNetwork) buildIptablesConnmarkRules(vpcCIDRs []string, ipt iptable
 		chain:       "PREROUTING",
 		rule: []string{
 			"-i", n.vethPrefix + "+", "-m", "comment", "--comment", "AWS, outbound connections",
-			"-m", "state", "--state", "NEW", "-j", "AWS-CONNMARK-CHAIN-0",
+			"-m", "state", "-j", "AWS-CONNMARK-CHAIN-0",
 		}})
 
 	for i, cidr := range allCIDRs {
@@ -603,7 +603,7 @@ func (n *linuxNetwork) buildIptablesConnmarkRules(vpcCIDRs []string, ipt iptable
 	}
 
 	iptableRules = append(iptableRules, iptablesRule{
-		name:        "connmark rule for external  outbound traffic",
+		name:        "connmark rule for external outbound traffic",
 		shouldExist: !n.useExternalSNAT,
 		table:       "nat",
 		chain:       chains[len(chains)-1],
@@ -613,29 +613,6 @@ func (n *linuxNetwork) buildIptablesConnmarkRules(vpcCIDRs []string, ipt iptable
 		},
 	})
 
-	// Force delete existing restore mark rule so that the subsequent rule gets added to the end
-	iptableRules = append(iptableRules, iptablesRule{
-		name:        "connmark to fwmark copy",
-		shouldExist: false,
-		table:       "nat",
-		chain:       "PREROUTING",
-		rule: []string{
-			"-m", "comment", "--comment", "AWS, CONNMARK", "-j", "CONNMARK",
-			"--restore-mark", "--mask", fmt.Sprintf("%#x", n.mainENIMark),
-		},
-	})
-
-	iptableRules = append(iptableRules, iptablesRule{
-		name:        "connmark to fwmark copy",
-		shouldExist: !n.useExternalSNAT,
-		table:       "nat",
-		chain:       "PREROUTING",
-		rule: []string{
-			"-m", "comment", "--comment", "AWS, CONNMARK", "-j", "CONNMARK",
-			"--restore-mark", "--mask", fmt.Sprintf("%#x", n.mainENIMark),
-		},
-	})
-
 	connmarkStaleRules, err := computeStaleIptablesRules(ipt, "nat", "AWS-CONNMARK-CHAIN", iptableRules, chains)
 	if err != nil {
 		return []iptablesRule{}, err