public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH network] sdn: zones: simple, evpn: do not stack duplicate snat rules
@ 2026-10-19 11:50 Dzidula Gati
  2026-10-20 16:02 ` Stefan Hanreich
  0 siblings, 1 reply; 2+ messages in thread
From: Dzidula Gati @ 2026-10-19 11:50 UTC (permalink / raw)
  To: pve-devel

The SNAT and conntrack zone rules are installed with plain
`iptables -A`/`-I` in post-up. ifupdown2 re-runs post-up when an
existing interface is reconfigured by `ifreload -a` (e.g. on every
SDN apply), without running the old post-down first. Each apply
therefore adds another copy of the same rules.

Since post-down only removes a single exact match, the extra copies
are never cleaned up. If the SNAT source address changed in the
meantime (e.g. the node's IP was changed and SDN re-applied), the
stale rules also stay in front of the new one, keep matching all
traffic first and break outbound connectivity of the guests until
they are removed by hand or the node is rebooted.

Check whether the rule already exists before adding it, so that
re-running post-up is idempotent. This does not remove rules that
were left behind with an outdated source address, but it stops new
duplicates from being created.

Signed-off-by: Dzidula Gati <dzidulajubilee@gmail.com>
---
Tested on PVE 9.2 / ifupdown2 3.3.0: the full test suite passes
(zones/ipams/dns/subnets/vnets_blackbox, 235/235). Verified in a network
namespace that running the generated post-up commands repeatedly leaves
exactly one rule, and a single post-down removes it again.

 src/PVE/Network/SDN/Zones/EvpnPlugin.pm           | 15 +++++++++------
 src/PVE/Network/SDN/Zones/SimplePlugin.pm         | 15 +++++++++------
 .../evpn/exitnode_snat/expected_sdn_interfaces    |  8 ++++----
 .../zones/simple/ipv4snat/expected_sdn_interfaces |  4 ++--
 .../zones/simple/ipv6snat/expected_sdn_interfaces |  4 ++--
 5 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
index 0e79707..5b4efbc 100644
--- a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
+++ b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
@@ -282,15 +282,18 @@ sub generate_sdn_config {
             }
             if ($outip && $outiface && $is_evpn_gateway) {
                 #use snat, faster than masquerade
+                #check before adding, so that re-running post-up on ifreload does not
+                #stack duplicate rules
+                my $snat_rule =
+                    "POSTROUTING -s '$cidr' -o $outiface -j SNAT --to-source $outip";
                 push @iface_config,
-                    "post-up $iptables -t nat -A POSTROUTING -s '$cidr' -o $outiface -j SNAT --to-source $outip";
-                push @iface_config,
-                    "post-down $iptables -t nat -D POSTROUTING -s '$cidr' -o $outiface -j SNAT --to-source $outip";
+                    "post-up $iptables -t nat -C $snat_rule 2>/dev/null || $iptables -t nat -A $snat_rule";
+                push @iface_config, "post-down $iptables -t nat -D $snat_rule";
                 #add conntrack zone once on outgoing interface
+                my $ct_rule = "PREROUTING -i fwbr+ -j CT --zone 1";
                 push @iface_config,
-                    "post-up $iptables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1";
-                push @iface_config,
-                    "post-down $iptables -t raw -D PREROUTING -i fwbr+ -j CT --zone 1";
+                    "post-up $iptables -t raw -C $ct_rule 2>/dev/null || $iptables -t raw -I $ct_rule";
+                push @iface_config, "post-down $iptables -t raw -D $ct_rule";
             }
         }
     }
diff --git a/src/PVE/Network/SDN/Zones/SimplePlugin.pm b/src/PVE/Network/SDN/Zones/SimplePlugin.pm
index 347eee9..dac1b55 100644
--- a/src/PVE/Network/SDN/Zones/SimplePlugin.pm
+++ b/src/PVE/Network/SDN/Zones/SimplePlugin.pm
@@ -125,15 +125,18 @@ sub generate_sdn_config {
             }
             if ($outip && $outiface) {
                 #use snat, faster than masquerade
+                #check before adding, so that re-running post-up on ifreload does not
+                #stack duplicate rules
+                my $snat_rule =
+                    "POSTROUTING -s '$cidr' -o $outiface -j SNAT --to-source $outip";
                 push @iface_config,
-                    "post-up $iptables -t nat -A POSTROUTING -s '$cidr' -o $outiface -j SNAT --to-source $outip";
-                push @iface_config,
-                    "post-down $iptables -t nat -D POSTROUTING -s '$cidr' -o $outiface -j SNAT --to-source $outip";
+                    "post-up $iptables -t nat -C $snat_rule 2>/dev/null || $iptables -t nat -A $snat_rule";
+                push @iface_config, "post-down $iptables -t nat -D $snat_rule";
                 #add conntrack zone once on outgoing interface
+                my $ct_rule = "PREROUTING -i fwbr+ -j CT --zone 1";
                 push @iface_config,
-                    "post-up $iptables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1";
-                push @iface_config,
-                    "post-down $iptables -t raw -D PREROUTING -i fwbr+ -j CT --zone 1";
+                    "post-up $iptables -t raw -C $ct_rule 2>/dev/null || $iptables -t raw -I $ct_rule";
+                push @iface_config, "post-down $iptables -t raw -D $ct_rule";
             }
         }
     }
diff --git a/src/test/zones/evpn/exitnode_snat/expected_sdn_interfaces b/src/test/zones/evpn/exitnode_snat/expected_sdn_interfaces
index 0d7d174..8c69da4 100644
--- a/src/test/zones/evpn/exitnode_snat/expected_sdn_interfaces
+++ b/src/test/zones/evpn/exitnode_snat/expected_sdn_interfaces
@@ -3,9 +3,9 @@
 auto myvnet
 iface myvnet
 	address 10.0.0.1/24
-	post-up iptables -t nat -A POSTROUTING -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1
+	post-up iptables -t nat -C POSTROUTING -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1 2>/dev/null || iptables -t nat -A POSTROUTING -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1
 	post-down iptables -t nat -D POSTROUTING -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1
-	post-up iptables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1
+	post-up iptables -t raw -C PREROUTING -i fwbr+ -j CT --zone 1 2>/dev/null || iptables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1
 	post-down iptables -t raw -D PREROUTING -i fwbr+ -j CT --zone 1
 	bridge_ports vxlan_myvnet
 	bridge_stp off
@@ -18,9 +18,9 @@ iface myvnet
 auto myvnet2
 iface myvnet2
 	address 2a08:2142:302:3::1/64
-	post-up ip6tables -t nat -A POSTROUTING -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2
+	post-up ip6tables -t nat -C POSTROUTING -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2 2>/dev/null || ip6tables -t nat -A POSTROUTING -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2
 	post-down ip6tables -t nat -D POSTROUTING -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2
-	post-up ip6tables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1
+	post-up ip6tables -t raw -C PREROUTING -i fwbr+ -j CT --zone 1 2>/dev/null || ip6tables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1
 	post-down ip6tables -t raw -D PREROUTING -i fwbr+ -j CT --zone 1
 	bridge_ports vxlan_myvnet2
 	bridge_stp off
diff --git a/src/test/zones/simple/ipv4snat/expected_sdn_interfaces b/src/test/zones/simple/ipv4snat/expected_sdn_interfaces
index 69d7986..c527d41 100644
--- a/src/test/zones/simple/ipv4snat/expected_sdn_interfaces
+++ b/src/test/zones/simple/ipv4snat/expected_sdn_interfaces
@@ -3,9 +3,9 @@
 auto myvnet
 iface myvnet
 	address 10.0.0.1/24
-	post-up iptables -t nat -A POSTROUTING -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1
+	post-up iptables -t nat -C POSTROUTING -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1 2>/dev/null || iptables -t nat -A POSTROUTING -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1
 	post-down iptables -t nat -D POSTROUTING -s '10.0.0.0/24' -o vmbr0 -j SNAT --to-source 192.168.0.1
-	post-up iptables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1
+	post-up iptables -t raw -C PREROUTING -i fwbr+ -j CT --zone 1 2>/dev/null || iptables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1
 	post-down iptables -t raw -D PREROUTING -i fwbr+ -j CT --zone 1
 	bridge_ports none
 	bridge_stp off
diff --git a/src/test/zones/simple/ipv6snat/expected_sdn_interfaces b/src/test/zones/simple/ipv6snat/expected_sdn_interfaces
index 5f6d40b..7075c51 100644
--- a/src/test/zones/simple/ipv6snat/expected_sdn_interfaces
+++ b/src/test/zones/simple/ipv6snat/expected_sdn_interfaces
@@ -3,9 +3,9 @@
 auto myvnet
 iface myvnet
 	address 2a08:2142:302:3::1/64
-	post-up ip6tables -t nat -A POSTROUTING -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2
+	post-up ip6tables -t nat -C POSTROUTING -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2 2>/dev/null || ip6tables -t nat -A POSTROUTING -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2
 	post-down ip6tables -t nat -D POSTROUTING -s '2a08:2142:302:3::/64' -o vmbr0 -j SNAT --to-source 2001:db8::2
-	post-up ip6tables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1
+	post-up ip6tables -t raw -C PREROUTING -i fwbr+ -j CT --zone 1 2>/dev/null || ip6tables -t raw -I PREROUTING -i fwbr+ -j CT --zone 1
 	post-down ip6tables -t raw -D PREROUTING -i fwbr+ -j CT --zone 1
 	bridge_ports none
 	bridge_stp off
-- 
2.43.0




^ permalink raw reply related	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-20 16:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-19 11:50 [PATCH network] sdn: zones: simple, evpn: do not stack duplicate snat rules Dzidula Gati
2026-10-20 16:02 ` Stefan Hanreich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal