Repository: antrea-io/antrea
Pull request #2540 (2021-08-05 17:22:10)

Automated cherry pick of #2469: Bug fix in adding iptable rules * bug fix in
Cherry pick of #2469 on release-1.2.

#2469: Bug fix in adding iptable rules * bug fix in

For details on the cherry pick process, see the [cherry pick requests](https://git.k8s.io/community/contributors/devel/sig-release/cherry-picks.md) page.

codecov-commenter: # [Codecov](https://codecov.io/gh/antrea-io/antrea/pull/2540?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#2540](https://codecov.io/gh/antrea-io/antrea/pull/2540?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (ab20cf7) into [release-1.2](https://codecov.io/gh/antrea-io/antrea/commit/1a7658bebb5398cf25b148a939e1781bc1682973?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (1a7658b) will **decrease** coverage by `0.04%`.
> The diff coverage is `0.00%`.

[![Impacted file tree graph](https://codecov.io/gh/antrea-io/antrea/pull/2540/graphs/tree.svg?width=650&height=150&src=pr&token=TjWaLL2GoC&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)](https://codecov.io/gh/antrea-io/antrea/pull/2540?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

```diff
@@               Coverage Diff               @@
##           release-1.2    #2540      +/-   ##
===============================================
- Coverage        42.18%   42.14%   -0.05%     
===============================================
  Files              146      146              
  Lines            18200    18218      +18     
===============================================
  Hits              7678     7678              
- Misses            9834     9851      +17     
- Partials           688      689       +1     
```

| Flag | Coverage Δ | |
|---|---|---|
| unit-tests | `42.14% <0.00%> (-0.05%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/antrea-io/antrea/pull/2540?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [pkg/agent/util/iptables/iptables.go](https://codecov.io/gh/antrea-io/antrea/pull/2540/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cGtnL2FnZW50L3V0aWwvaXB0YWJsZXMvaXB0YWJsZXMuZ28=) | `0.00% <0.00%> (ø)` | |
| [pkg/apiserver/storage/ram/store.go](https://codecov.io/gh/antrea-io/antrea/pull/2540/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cGtnL2FwaXNlcnZlci9zdG9yYWdlL3JhbS9zdG9yZS5nbw==) | `77.44% <0.00%> (-1.51%)` | :arrow_down: |
| [pkg/apiserver/storage/ram/watch.go](https://codecov.io/gh/antrea-io/antrea/pull/2540/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-cGtnL2FwaXNlcnZlci9zdG9yYWdlL3JhbS93YXRjaC5nbw==) | `88.46% <0.00%> (+3.84%)` | :arrow_up: |

antoninbas: /test-all

Patch:
diff --git a/build/yamls/antrea-aks.yml b/build/yamls/antrea-aks.yml
index c9acd831a..8f5247eea 100644
--- a/build/yamls/antrea-aks.yml
+++ b/build/yamls/antrea-aks.yml
@@ -4190,7 +4190,7 @@ spec:
           name: api
           protocol: TCP
         readinessProbe:
-          failureThreshold: 5
+          failureThreshold: 8
           httpGet:
             host: localhost
             path: /readyz
diff --git a/build/yamls/antrea-eks.yml b/build/yamls/antrea-eks.yml
index dddc4f5f5..6a012b8fe 100644
--- a/build/yamls/antrea-eks.yml
+++ b/build/yamls/antrea-eks.yml
@@ -4192,7 +4192,7 @@ spec:
           name: api
           protocol: TCP
         readinessProbe:
-          failureThreshold: 5
+          failureThreshold: 8
           httpGet:
             host: localhost
             path: /readyz
diff --git a/build/yamls/antrea-gke.yml b/build/yamls/antrea-gke.yml
index 21aaab06b..0c57fd02c 100644
--- a/build/yamls/antrea-gke.yml
+++ b/build/yamls/antrea-gke.yml
@@ -4190,7 +4190,7 @@ spec:
           name: api
           protocol: TCP
         readinessProbe:
-          failureThreshold: 5
+          failureThreshold: 8
           httpGet:
             host: localhost
             path: /readyz
diff --git a/build/yamls/antrea-ipsec.yml b/build/yamls/antrea-ipsec.yml
index 9a6ad8dcb..79a39d974 100644
--- a/build/yamls/antrea-ipsec.yml
+++ b/build/yamls/antrea-ipsec.yml
@@ -4239,7 +4239,7 @@ spec:
           name: api
           protocol: TCP
         readinessProbe:
-          failureThreshold: 5
+          failureThreshold: 8
           httpGet:
             host: localhost
             path: /readyz
diff --git a/build/yamls/antrea.yml b/build/yamls/antrea.yml
index 5f0c5b6a3..cf15a4cd2 100644
--- a/build/yamls/antrea.yml
+++ b/build/yamls/antrea.yml
@@ -4195,7 +4195,7 @@ spec:
           name: api
           protocol: TCP
         readinessProbe:
-          failureThreshold: 5
+          failureThreshold: 8
           httpGet:
             host: localhost
             path: /readyz
diff --git a/build/yamls/base/agent.yml b/build/yamls/base/agent.yml
index e8a8b32f5..a1a0ec5dc 100644
--- a/build/yamls/base/agent.yml
+++ b/build/yamls/base/agent.yml
@@ -110,7 +110,11 @@ spec:
             initialDelaySeconds: 5
             timeoutSeconds: 5
             periodSeconds: 10
-            failureThreshold: 5
+            # In large-scale clusters, it may take up to 40~50 seconds for the antrea-agent to reconnect to the antrea
+            # Service after the antrea-controller restarts. The antrea-agent shouldn't be reported as NotReady in this
+            # scenario, otherwise the DaemonSet controller would restart all agents at once, as opposed to performing a
+            # rolling update. Set failureThreshold to 8 so it can tolerate 70s of disconnection.
+            failureThreshold: 8
           securityContext:
             # antrea-agent needs to perform sysctl configuration.
             privileged: true
diff --git a/pkg/agent/nodeportlocal/portcache/port_table.go b/pkg/agent/nodeportlocal/portcache/port_table.go
index 644af03ce..19a776913 100644
--- a/pkg/agent/nodeportlocal/portcache/port_table.go
+++ b/pkg/agent/nodeportlocal/portcache/port_table.go
@@ -119,7 +119,7 @@ func (pt *PortTable) AddRule(podIP string, podPort int) (int, error) {
 	if err != nil {
 		return 0, err
 	}
-	if err := pt.PodPortRules.AddRule(nodePort, fmt.Sprintf("%s:%d", podIP, podPort)); err != nil {
+	if err := pt.PodPortRules.AddRule(nodePort, podIP, podPort); err != nil {
 		if err := socket.Close(); err != nil {
 			klog.ErrorS(err, "Unexpected error when closing socket")
 		}
@@ -138,7 +138,7 @@ func (pt *PortTable) DeleteRule(podIP string, podPort int) error {
 	pt.tableLock.Lock()
 	defer pt.tableLock.Unlock()
 	data := pt.getEntryByPodIPPort(podIP, podPort)
-	if err := pt.PodPortRules.DeleteRule(data.NodePort, fmt.Sprintf("%s:%d", podIP, podPort)); err != nil {
+	if err := pt.PodPortRules.DeleteRule(data.NodePort, podIP, podPort); err != nil {
 		return err
 	}
 	if err := data.socket.Close(); err != nil {
diff --git a/pkg/agent/nodeportlocal/rules/iptable_rule.go b/pkg/agent/nodeportlocal/rules/iptable_rule.go
index 7cdb0cf9e..af4bb91e9 100644
--- a/pkg/agent/nodeportlocal/rules/iptable_rule.go
+++ b/pkg/agent/nodeportlocal/rules/iptable_rule.go
@@ -56,16 +56,16 @@ func (ipt *iptablesRules) Init() error {
 // traffic) and OUTPUT chain (for locally-generated traffic). All NPL DNAT rules
 // will be added to this chain.
 func (ipt *iptablesRules) initRules() error {
-	if err := ipt.table.EnsureChain(iptables.NATTable, NodePortLocalChain); err != nil {
+	if err := ipt.table.EnsureChain(iptables.ProtocolIPv4, iptables.NATTable, NodePortLocalChain); err != nil {
 		return err
 	}
 	ruleSpec := []string{
 		"-p", "tcp", "-m", "addrtype", "--dst-type", "LOCAL", "-j", NodePortLocalChain,
 	}
-	if err := ipt.table.EnsureRule(iptables.NATTable, iptables.PreRoutingChain, ruleSpec); err != nil {
+	if err := ipt.table.AppendRule(iptables.ProtocolIPv4, iptables.NATTable, iptables.PreRoutingChain, ruleSpec); err != nil {
 		return err
 	}
-	if err := ipt.table.EnsureRule(iptables.NATTable, iptables.OutputChain, ruleSpec); err != nil {
+	if err := ipt.table.AppendRule(iptables.ProtocolIPv4, iptables.NATTable, iptables.OutputChain, ruleSpec); err != nil {
 		return err
 	}
 	return nil
@@ -78,13 +78,14 @@ func buildRuleForPod(port int, podIP string) []string {
 	}
 }
 
-// AddRule appends a DNAT rule in NodePortLocalChain chain of NAT table
-func (ipt *iptablesRules) AddRule(port int, podIP string) error {
-	rule := buildRuleForPod(port, podIP)
-	if err := ipt.table.EnsureRule(iptables.NATTable, NodePortLocalChain, rule); err != nil {
+// AddRule appends a DNAT rule in NodePortLocalChain chain of NAT table.
+func (ipt *iptablesRules) AddRule(nodePort int, podIP string, podPort int) error {
+	podAddr := fmt.Sprintf("%s:%d", podIP, podPort)
+	rule := buildRuleForPod(nodePort, podAddr)
+	if err := ipt.table.AppendRule(iptables.ProtocolIPv4, iptables.NATTable, NodePortLocalChain, rule); err != nil {
 		return err
 	}
-	klog.Infof("Successfully added DNAT rule for Pod IP %s, port %d", podIP, port)
+	klog.InfoS("Successfully added DNAT rule", "podAddr", podAddr, "nodePort", nodePort)
 	return nil
 }
 
@@ -107,18 +108,19 @@ func (ipt *iptablesRules) AddAllRules(nplList []PodNodePort) error {
 }
 
 // DeleteRule deletes a specific NPL rule from NodePortLocalChain chain
-func (ipt *iptablesRules) DeleteRule(port int, podIP string) error {
-	klog.Infof("Deleting DNAT rule for Pod IP %s, port %d", podIP, port)
-	rule := buildRuleForPod(port, podIP)
-	if err := ipt.table.DeleteRule(iptables.NATTable, NodePortLocalChain, rule); err != nil {
+func (ipt *iptablesRules) DeleteRule(nodePort int, podIP string, podPort int) error {
+	podAddr := fmt.Sprintf("%s:%d", podIP, podPort)
+	rule := buildRuleForPod(nodePort, podAddr)
+	if err := ipt.table.DeleteRule(iptables.ProtocolIPv4, iptables.NATTable, NodePortLocalChain, rule); err != nil {
 		return err
 	}
+	klog.InfoS("Successfully deleted DNAT rule", "podAddr", podAddr, "nodePort", nodePort)
 	return nil
 }
 
 // DeleteAllRules deletes all NPL rules programmed in the node
 func (ipt *iptablesRules) DeleteAllRules() error {
-	exists, err := ipt.table.ChainExists(iptables.NATTable, NodePortLocalChain)
+	exists, err := ipt.table.ChainExists(iptables.ProtocolIPv4, iptables.NATTable, NodePortLocalChain)
 	if err != nil {
 		return fmt.Errorf("failed to check if NodePortLocal chain exists in NAT table: %v", err)
 	}
@@ -128,13 +130,13 @@ func (ipt *iptablesRules) DeleteAllRules() error {
 	ruleSpec := []string{
 		"-p", "tcp", "-m", "addrtype", "--dst-type", "LOCAL", "-j", NodePortLocalChain,
 	}
-	if err := ipt.table.DeleteRule(iptables.NATTable, iptables.PreRoutingChain, ruleSpec); err != nil {
+	if err := ipt.table.DeleteRule(iptables.ProtocolIPv4, iptables.NATTable, iptables.PreRoutingChain, ruleSpec); err != nil {
 		return err
 	}
-	if err := ipt.table.DeleteRule(iptables.NATTable, iptables.OutputChain, ruleSpec); err != nil {
+	if err := ipt.table.DeleteRule(iptables.ProtocolIPv4, iptables.NATTable, iptables.OutputChain, ruleSpec); err != nil {
 		return err
 	}
-	if err := ipt.table.DeleteChain(iptables.NATTable, NodePortLocalChain); err != nil {
+	if err := ipt.table.DeleteChain(iptables.ProtocolIPv4, iptables.NATTable, NodePortLocalChain); err != nil {
 		return err
 	}
 	return nil
diff --git a/pkg/agent/nodeportlocal/rules/rules.go b/pkg/agent/nodeportlocal/rules/rules.go
index a4a70b6d8..7c9bca1ab 100644
--- a/pkg/agent/nodeportlocal/rules/rules.go
+++ b/pkg/agent/nodeportlocal/rules/rules.go
@@ -19,8 +19,8 @@ package rules
 // PodPortRules is an interface to abstract operations on rules for Pods
 type PodPortRules interface {
 	Init() error
-	AddRule(port int, podip string) error
-	DeleteRule(port int, podip string) error
+	AddRule(nodePort int, podIP string, podPort int) error
+	DeleteRule(nodePort int, podIP string, podPort int) error
 	DeleteAllRules() error
 	AddAllRules(nplList []PodNodePort) error
 }
diff --git a/pkg/agent/route/route_linux.go b/pkg/agent/route/route_linux.go
index 45acd8e9e..bd81e7334 100644
--- a/pkg/agent/route/route_linux.go
+++ b/pkg/agent/route/route_linux.go
@@ -258,6 +258,7 @@ func (c *Client) syncIPTables() error {
 	var err error
 	v4Enabled := config.IsIPv4Enabled(c.nodeConfig, c.networkConfig.TrafficEncapMode)
 	v6Enabled := config.IsIPv6Enabled(c.nodeConfig, c.networkConfig.TrafficEncapMode)
+
 	c.ipt, err = iptables.New(v4Enabled, v6Enabled)
 	if err != nil {
 		return fmt.Errorf("error creating IPTables instance: %v", err)
@@ -274,11 +275,11 @@ func (c *Client) syncIPTables() error {
 		{iptables.MangleTable, iptables.OutputChain, antreaOutputChain, "Antrea: jump to Antrea output rules"},
 	}
 	for _, rule := range jumpRules {
-		if err := c.ipt.EnsureChain(rule.table, rule.dstChain); err != nil {
+		if err := c.ipt.EnsureChain(iptables.ProtocolDual, rule.table, rule.dstChain); err != nil {
 			return err
 		}
 		ruleSpec := []string{"-j", rule.dstChain, "-m", "comment", "--comment", rule.comment}
-		if err := c.ipt.EnsureRule(rule.table, rule.srcChain, ruleSpec); err != nil {
+		if err := c.ipt.AppendRule(iptables.ProtocolDual, rule.table, rule.srcChain, ruleSpec); err != nil {
 			return err
 		}
 	}
@@ -771,5 +772,5 @@ func (c *Client) DeleteSNATRule(mark uint32) error {
 	}
 	c.markToSNATIP.Delete(mark)
 	snatIP := value.(net.IP)
-	return c.ipt.DeleteRule(iptables.NATTable, antreaPostRoutingChain, c.snatRuleSpec(snatIP, mark))
+	return c.ipt.DeleteRule(iptables.ProtocolDual, iptables.NATTable, antreaPostRoutingChain, c.snatRuleSpec(snatIP, mark))
 }
diff --git a/pkg/agent/util/iptables/iptables.go b/pkg/agent/util/iptables/iptables.go
index 2c6a3fb74..dfc14a090 100644
--- a/pkg/agent/util/iptables/iptables.go
+++ b/pkg/agent/util/iptables/iptables.go
@@ -52,6 +52,15 @@ const (
 
 type Protocol byte
 
+var protocolStrMap = map[Protocol]string{
+	ProtocolIPv4: "IPv4",
+	ProtocolIPv6: "IPv6",
+}
+
+func (p Protocol) String() string {
+	return protocolStrMap[p]
+}
+
 const (
 	ProtocolDual Protocol = iota
 	ProtocolIPv4
@@ -63,20 +72,20 @@ const (
 var restoreWaitSupportedMinVersion = semver.Version{Major: 1, Minor: 6, Patch: 2}
 
 type Client struct {
-	ipts []*iptables.IPTables
+	ipts map[Protocol]*iptables.IPTables
 	// restoreWaitSupported indicates whether iptables-restore (or ip6tables-restore) supports --wait flag.
 	restoreWaitSupported bool
 }
 
 func New(enableIPV4, enableIPV6 bool) (*Client, error) {
-	var ipts []*iptables.IPTables
+	ipts := make(map[Protocol]*iptables.IPTables)
 	var restoreWaitSupported bool
 	if enableIPV4 {
 		ipt, err := iptables.New()
 		if err != nil {
 			return nil, fmt.Errorf("error creating IPTables instance: %v", err)
 		}
-		ipts = append(ipts, ipt)
+		ipts[ProtocolIPv4] = ipt
 		restoreWaitSupported = isRestoreWaitSupported(ipt)
 	}
 	if enableIPV6 {
@@ -84,7 +93,7 @@ func New(enableIPV4, enableIPV6 bool) (*Client, error) {
 		if err != nil {
 			return nil, fmt.Errorf("error creating IPTables instance for IPv6: %v", err)
 		}
-		ipts = append(ipts, ip6t)
+		ipts[ProtocolIPv6] = ip6t
 		if !restoreWaitSupported {
 			restoreWaitSupported = isRestoreWaitSupported(ip6t)
 		}
@@ -99,9 +108,12 @@ func isRestoreWaitSupported(ipt *iptables.IPTables) bool {
 }
 
 // EnsureChain checks if target chain already exists, creates it if not.
-func (c *Client) EnsureChain(table string, chain string) error {
-	for idx := range c.ipts {
-		ipt := c.ipts[idx]
+func (c *Client) EnsureChain(protocol Protocol, table string, chain string) error {
+	for p := range c.ipts {
+		ipt := c.ipts[p]
+		if !matchProtocol(ipt, protocol) {
+			continue
+		}
 		exists, err := ipt.ChainExists(table, chain)
 		if err != nil {
 			return fmt.Errorf("error checking if chain %s exists in table %s: %v", chain, table, err)
@@ -112,48 +124,56 @@ func (c *Client) EnsureChain(table string, chain string) error {
 		if err := ipt.NewChain(table, chain); err != nil {
 			return fmt.Errorf("error creating chain %s in table %s: %v", chain, table, err)
 		}
-		klog.V(2).Infof("Created chain %s in table %s", chain, table)
+		klog.V(2).InfoS("Created a chain", "chain", chain, "table", table, "protocol", p)
 	}
 	return nil
 }
 
-// ChainExists checks if a chain already exists in a table
-func (c *Client) ChainExists(table string, chain string) (bool, error) {
-	for idx := range c.ipts {
-		exists, err := c.ipts[idx].ChainExists(table, chain)
+// ChainExists checks if target chain already exists in a table
+func (c *Client) ChainExists(protocol Protocol, table string, chain string) (bool, error) {
+	for p := range c.ipts {
+		ipt := c.ipts[p]
+		if !matchProtocol(ipt, protocol) {
+			continue
+		}
+		exists, err := ipt.ChainExists(table, chain)
 		if err != nil {
 			return false, fmt.Errorf("error checking if chain %s exists in table %s: %v", chain, table, err)
 		}
-		if exists {
-			return true, nil
+		if !exists {
+			return false, nil
 		}
+		klog.V(2).InfoS("A chain exists", "chain", chain, "table", table, "protocol", p)
 	}
-	return false, nil
+	return true, nil
 }
 
-// EnsureRule checks if target rule already exists, appends it if not.
-func (c *Client) EnsureRule(table string, chain string, ruleSpec []string) error {
-	for idx := range c.ipts {
-		ipt := c.ipts[idx]
+// AppendRule checks if target rule already exists with the protocol, appends it if not.
+func (c *Client) AppendRule(protocol Protocol, table string, chain string, ruleSpec []string) error {
+	for p := range c.ipts {
+		ipt := c.ipts[p]
+		if !matchProtocol(ipt, protocol) {
+			continue
+		}
 		exist, err := ipt.Exists(table, chain, ruleSpec...)
 		if err != nil {
 			return fmt.Errorf("error checking if rule %v exists in table %s chain %s: %v", ruleSpec, table, chain, err)
 		}
 		if exist {
-			return nil
+			continue
 		}
 		if err := ipt.Append(table, chain, ruleSpec...); err != nil {
 			return fmt.Errorf("error appending rule %v to table %s chain %s: %v", ruleSpec, table, chain, err)
 		}
+		klog.V(2).InfoS("Appended a rule", "rule", ruleSpec, "table", table, "chain", chain, "protocol", p)
 	}
-	klog.V(2).Infof("Appended rule %v to table %s chain %s", ruleSpec, table, chain)
 	return nil
 }
 
 // InsertRule checks if target rule already exists, inserts it if not.
 func (c *Client) InsertRule(protocol Protocol, table string, chain string, ruleSpec []string) error {
-	for idx := range c.ipts {
-		ipt := c.ipts[idx]
+	for p := range c.ipts {
+		ipt := c.ipts[p]
 		if !matchProtocol(ipt, protocol) {
 			continue
 		}
@@ -162,13 +182,13 @@ func (c *Client) InsertRule(protocol Protocol, table string, chain string, ruleS
 			return fmt.Errorf("error checking if rule %v exists in table %s chain %s: %v", ruleSpec, table, chain, err)
 		}
 		if exist {
-			return nil
+			continue
 		}
 		if err := ipt.Insert(table, chain, 1, ruleSpec...); err != nil {
 			return fmt.Errorf("error inserting rule %v to table %s chain %s: %v", ruleSpec, table, chain, err)
 		}
+		klog.V(2).InfoS("Inserted a rule", "rule", ruleSpec, "table", table, "chain", chain)
 	}
-	klog.V(2).Infof("Inserted rule %v to table %s chain %s", ruleSpec, table, chain)
 	return nil
 }
 
@@ -185,46 +205,59 @@ func matchProtocol(ipt *iptables.IPTables, protocol Protocol) bool {
 }
 
 // DeleteRule checks if target rule already exists, deletes the rule if found.
-func (c *Client) DeleteRule(table string, chain string, ruleSpec []string) error {
-	for idx := range c.ipts {
-		exist, err := c.ipts[idx].Exists(table, chain, ruleSpec...)
+func (c *Client) DeleteRule(protocol Protocol, table string, chain string, ruleSpec []string) error {
+	for p := range c.ipts {
+		ipt := c.ipts[p]
+		if !matchProtocol(ipt, protocol) {
+			continue
+		}
+		exist, err := ipt.Exists(table, chain, ruleSpec...)
 		if err != nil {
 			return fmt.Errorf("error checking if rule %v exists in table %s chain %s: %v", ruleSpec, table, chain, err)
 		}
 		if !exist {
-			return nil
+			continue
 		}
-		if err := c.ipts[idx].Delete(table, chain, ruleSpec...); err != nil {
+		if err := ipt.Delete(table, chain, ruleSpec...); err != nil {
 			return fmt.Errorf("error deleting rule %v from table %s chain %s: %v", ruleSpec, table, chain, err)
 		}
-		klog.V(2).Infof("Deleted rule %v from table %s chain %s", ruleSpec, table, chain)
+		klog.V(2).InfoS("Deleted a rule", "rule", ruleSpec, "table", table, "chain", chain, "protocol", p)
 	}
 	return nil
 }
 
-// DeleteChain deletes all rules from a chain in a table and then delete the chain
-func (c *Client) DeleteChain(table string, chain string) error {
-	for idx := range c.ipts {
-		err := c.ipts[idx].ClearChain(table, chain)
+// DeleteChain deletes all rules from a chain in a table and then delete the chain.
+func (c *Client) DeleteChain(protocol Protocol, table string, chain string) error {
+	for p := range c.ipts {
+		ipt := c.ipts[p]
+		if !matchProtocol(ipt, protocol) {
+			continue
+		}
+		exists, err := ipt.ChainExists(table, chain)
 		if err != nil {
+			return fmt.Errorf("error checking if chain %s exists in table %s: %v", chain, table, err)
+		}
+		if !exists {
+			continue
+		}
+		if err = ipt.ClearChain(table, chain); err != nil {
 			return fmt.Errorf("error clearing rules from table %s chain %s: %v", table, chain, err)
 		}
-		err = c.ipts[idx].DeleteChain(table, chain)
-		if err != nil {
+		if err = ipt.DeleteChain(table, chain); err != nil {
 			return fmt.Errorf("error deleting chain %s from table %s: %v", chain, table, err)
 		}
-		klog.V(2).Infof("Deleted chain %s from table %s", chain, table)
+		klog.V(2).InfoS("Deleted a chain", "chain", chain, "table", table, "protocol", p)
 	}
 	return nil
 }
 
-// ListRules lists all rules from a chain in a table
+// ListRules lists all rules from a chain in a table.
 func (c *Client) ListRules(table string, chain string) ([]string, error) {
 	var allRules []string
-	for idx := range c.ipts {
-		rules, err := c.ipts[idx].List(table, chain)
+	for p := range c.ipts {
+		rules, err := c.ipts[p].List(table, chain)
 		if err != nil {
-			return rules, fmt.Errorf("error getting rules from table %s chain %s: %v", table, chain, err)
+			return rules, fmt.Errorf("error getting rules from table %s chain %s protocol %s: %v", table, chain, p, err)
 		}
 		allRules = append(allRules, rules...)
 	}
@@ -265,7 +298,7 @@ func (c *Client) Restore(data []byte, flush bool, useIPv6 bool) error {
 		defer unlockFunc()
 	}
 	if err := cmd.Run(); err != nil {
-		klog.Errorf("Failed to execute %s: %v\nstdin:\n%s\nstderr:\n%s", iptablesCmd, err, data, stderr)
+		klog.ErrorS(err, "Failed to execute iptables command", "iptablesCmd", iptablesCmd, "stdin", data, "stderr", stderr)
 		return fmt.Errorf("error executing %s: %v", iptablesCmd, err)
 	}
 	return nil
@@ -274,9 +307,9 @@ func (c *Client) Restore(data []byte, flush bool, useIPv6 bool) error {
 // Save calls iptables-saves to dump chains and tables in iptables.
 func (c *Client) Save() ([]byte, error) {
 	var output []byte
-	for idx := range c.ipts {
+	for p := range c.ipts {
 		var cmd string
-		ipt := c.ipts[idx]
+		ipt := c.ipts[p]
 		switch ipt.Proto() {
 		case iptables.ProtocolIPv6:
 			cmd = "ip6tables-save"


Test patch:
diff --git a/pkg/agent/nodeportlocal/npl_agent_test.go b/pkg/agent/nodeportlocal/npl_agent_test.go
index ea9aae035..40c8c7abf 100644
--- a/pkg/agent/nodeportlocal/npl_agent_test.go
+++ b/pkg/agent/nodeportlocal/npl_agent_test.go
@@ -191,8 +191,8 @@ func setUp(t *testing.T, tc *testConfig, objects ...runtime.Object) *testData {
 	mockCtrl := gomock.NewController(t)
 
 	mockIPTables := rulestesting.NewMockPodPortRules(mockCtrl)
-	mockIPTables.EXPECT().AddRule(gomock.Any(), gomock.Any()).AnyTimes()
-	mockIPTables.EXPECT().DeleteRule(gomock.Any(), gomock.Any()).AnyTimes()
+	mockIPTables.EXPECT().AddRule(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
+	mockIPTables.EXPECT().DeleteRule(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
 	mockIPTables.EXPECT().AddAllRules(gomock.Any()).AnyTimes()
 
 	mockPortOpener := portcachetesting.NewMockLocalPortOpener(mockCtrl)
diff --git a/pkg/agent/nodeportlocal/rules/testing/mock_rules.go b/pkg/agent/nodeportlocal/rules/testing/mock_rules.go
index b0ce5f010..da1d7a03e 100644
--- a/pkg/agent/nodeportlocal/rules/testing/mock_rules.go
+++ b/pkg/agent/nodeportlocal/rules/testing/mock_rules.go
@@ -63,17 +63,17 @@ func (mr *MockPodPortRulesMockRecorder) AddAllRules(arg0 interface{}) *gomock.Ca
 }
 
 // AddRule mocks base method
-func (m *MockPodPortRules) AddRule(arg0 int, arg1 string) error {
+func (m *MockPodPortRules) AddRule(arg0 int, arg1 string, arg2 int) error {
 	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "AddRule", arg0, arg1)
+	ret := m.ctrl.Call(m, "AddRule", arg0, arg1, arg2)
 	ret0, _ := ret[0].(error)
 	return ret0
 }
 
 // AddRule indicates an expected call of AddRule
-func (mr *MockPodPortRulesMockRecorder) AddRule(arg0, arg1 interface{}) *gomock.Call {
+func (mr *MockPodPortRulesMockRecorder) AddRule(arg0, arg1, arg2 interface{}) *gomock.Call {
 	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddRule", reflect.TypeOf((*MockPodPortRules)(nil).AddRule), arg0, arg1)
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddRule", reflect.TypeOf((*MockPodPortRules)(nil).AddRule), arg0, arg1, arg2)
 }
 
 // DeleteAllRules mocks base method
@@ -91,17 +91,17 @@ func (mr *MockPodPortRulesMockRecorder) DeleteAllRules() *gomock.Call {
 }
 
 // DeleteRule mocks base method
-func (m *MockPodPortRules) DeleteRule(arg0 int, arg1 string) error {
+func (m *MockPodPortRules) DeleteRule(arg0 int, arg1 string, arg2 int) error {
 	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "DeleteRule", arg0, arg1)
+	ret := m.ctrl.Call(m, "DeleteRule", arg0, arg1, arg2)
 	ret0, _ := ret[0].(error)
 	return ret0
 }
 
 // DeleteRule indicates an expected call of DeleteRule
-func (mr *MockPodPortRulesMockRecorder) DeleteRule(arg0, arg1 interface{}) *gomock.Call {
+func (mr *MockPodPortRulesMockRecorder) DeleteRule(arg0, arg1, arg2 interface{}) *gomock.Call {
 	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteRule", reflect.TypeOf((*MockPodPortRules)(nil).DeleteRule), arg0, arg1)
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteRule", reflect.TypeOf((*MockPodPortRules)(nil).DeleteRule), arg0, arg1, arg2)
 }
 
 // Init mocks base method