From 2a88f86b285fd65ae6d597fadab95e2a139c1bf2 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Wed, 20 May 2026 16:04:38 +0530 Subject: [PATCH 1/3] Fix Static NAT/Port Forwarding when VM NIC is not the default --- systemvm/debian/opt/cloud/bin/configure.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/systemvm/debian/opt/cloud/bin/configure.py b/systemvm/debian/opt/cloud/bin/configure.py index bf48be66694..22671b95a07 100755 --- a/systemvm/debian/opt/cloud/bin/configure.py +++ b/systemvm/debian/opt/cloud/bin/configure.py @@ -1448,7 +1448,7 @@ class CsForwardingRules(CsDataBag): ) fw4 = "-j SNAT --to-source %s -A POSTROUTING -s %s -d %s/32 -o %s -p %s -m %s --dport %s" % \ ( - self.getGuestIp(), + self.getGuestIpByIp(rule['internal_ip']), self.getNetworkByIp(rule['internal_ip']), rule['internal_ip'], internal_fwinterface, @@ -1567,6 +1567,13 @@ class CsForwardingRules(CsDataBag): self.fw.append(["nat", "front", "-A POSTROUTING -s %s -d %s -j SNAT -o %s --to-source %s" % (self.getNetworkByIp(rule['internal_ip']), rule["internal_ip"], self.getDeviceByIp(rule["internal_ip"]), self.getGuestIpByIp(rule["internal_ip"]))]) + internal_device = self.getDeviceByIp(rule["internal_ip"]) + internal_vr_ip = self.getGuestIpByIp(rule["internal_ip"]) + if internal_device and internal_vr_ip and internal_device != device: + self.fw.append(["nat", "front", + "-A POSTROUTING -o %s -d %s/32 -j SNAT --to-source %s" % + (internal_device, rule["internal_ip"], internal_vr_ip)]) + class IpTablesExecutor: From 4a9d0f43bba187a66681bea561a1e46fe8180561 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Thu, 21 May 2026 17:24:38 +0530 Subject: [PATCH 2/3] rewrite source only for non-default nics --- .../com/cloud/agent/api/to/StaticNatRuleTO.java | 9 +++++++++ .../facade/SetStaticNatRulesConfigItem.java | 3 ++- .../virtualnetwork/model/StaticNatRule.java | 15 +++++++++++++++ .../cloud/network/router/CommandSetupHelper.java | 11 +++++++++++ systemvm/debian/opt/cloud/bin/configure.py | 13 +++++++------ .../debian/opt/cloud/bin/cs_forwardingrules.py | 2 ++ 6 files changed, 46 insertions(+), 7 deletions(-) diff --git a/api/src/main/java/com/cloud/agent/api/to/StaticNatRuleTO.java b/api/src/main/java/com/cloud/agent/api/to/StaticNatRuleTO.java index 8064c301991..4cac660ace1 100644 --- a/api/src/main/java/com/cloud/agent/api/to/StaticNatRuleTO.java +++ b/api/src/main/java/com/cloud/agent/api/to/StaticNatRuleTO.java @@ -28,6 +28,7 @@ import com.cloud.network.rules.StaticNatRule; public class StaticNatRuleTO extends FirewallRuleTO { String dstIp; + boolean destinationIpOnDefaultNic = true; protected StaticNatRuleTO() { } @@ -79,4 +80,12 @@ public class StaticNatRuleTO extends FirewallRuleTO { return dstIp; } + public boolean isDestinationIpOnDefaultNic() { + return destinationIpOnDefaultNic; + } + + public void setDestinationIpOnDefaultNic(boolean destinationIpOnDefaultNic) { + this.destinationIpOnDefaultNic = destinationIpOnDefaultNic; + } + } diff --git a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetStaticNatRulesConfigItem.java b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetStaticNatRulesConfigItem.java index 0439b168a9d..1b60cc6ca7b 100644 --- a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetStaticNatRulesConfigItem.java +++ b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetStaticNatRulesConfigItem.java @@ -39,7 +39,8 @@ public class SetStaticNatRulesConfigItem extends AbstractConfigItemFacade { final LinkedList rules = new LinkedList<>(); for (final StaticNatRuleTO rule : command.getRules()) { - final StaticNatRule staticNatRule = new StaticNatRule(rule.revoked(), rule.getProtocol(), rule.getSrcIp(), rule.getStringSrcPortRange(), rule.getDstIp()); + final StaticNatRule staticNatRule = new StaticNatRule(rule.revoked(), rule.getProtocol(), rule.getSrcIp(), + rule.getStringSrcPortRange(), rule.getDstIp(), rule.isDestinationIpOnDefaultNic()); rules.add(staticNatRule); } final StaticNatRules staticNatRules = new StaticNatRules(rules); diff --git a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/StaticNatRule.java b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/StaticNatRule.java index a375a913b28..417c5edc11d 100644 --- a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/StaticNatRule.java +++ b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/StaticNatRule.java @@ -25,18 +25,25 @@ public class StaticNatRule { private String sourceIpAddress; private String sourcePortRange; private String destinationIpAddress; + private boolean destinationIpOnDefaultNic = true; public StaticNatRule() { // Empty constructor for (de)serialization } public StaticNatRule(boolean revoke, String protocol, String sourceIpAddress, String sourcePortRange, String destinationIpAddress) { + this(revoke, protocol, sourceIpAddress, sourcePortRange, destinationIpAddress, true); + } + + public StaticNatRule(boolean revoke, String protocol, String sourceIpAddress, String sourcePortRange, + String destinationIpAddress, boolean destinationIpOnDefaultNic) { super(); this.revoke = revoke; this.protocol = protocol; this.sourceIpAddress = sourceIpAddress; this.sourcePortRange = sourcePortRange; this.destinationIpAddress = destinationIpAddress; + this.destinationIpOnDefaultNic = destinationIpOnDefaultNic; } public boolean isRevoke() { @@ -79,4 +86,12 @@ public class StaticNatRule { this.destinationIpAddress = destinationIpAddress; } + public boolean isDestinationIpOnDefaultNic() { + return destinationIpOnDefaultNic; + } + + public void setDestinationIpOnDefaultNic(boolean destinationIpOnDefaultNic) { + this.destinationIpOnDefaultNic = destinationIpOnDefaultNic; + } + } diff --git a/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java b/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java index c6296682bb0..1214e8e3d26 100644 --- a/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java +++ b/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java @@ -446,6 +446,7 @@ public class CommandSetupHelper { for (final StaticNatRule rule : rules) { final IpAddress sourceIp = _networkModel.getIp(rule.getSourceIpAddressId()); final StaticNatRuleTO ruleTO = new StaticNatRuleTO(rule, null, sourceIp.getAddress().addr(), rule.getDestIpAddress()); + ruleTO.setDestinationIpOnDefaultNic(isDestinationIpOnDefaultNic(guestNetworkId, rule.getDestIpAddress())); rulesTO.add(ruleTO); } } @@ -459,6 +460,15 @@ public class CommandSetupHelper { cmds.addCommand(cmd); } + private boolean isDestinationIpOnDefaultNic(final long networkId, final String destinationIp) { + final NicVO destinationNic = _nicDao.findByIp4AddressAndNetworkId(destinationIp, networkId); + if (destinationNic == null) { + logger.debug("Unable to find destination NIC for ip [{}] in network [{}], assuming default NIC.", destinationIp, networkId); + return true; + } + return destinationNic.isDefaultNic(); + } + public void createApplyFirewallRulesCommands(final List rules, final VirtualRouter router, final Commands cmds, final long guestNetworkId) { final List rulesTO = new ArrayList<>(); String systemRule = null; @@ -697,6 +707,7 @@ public class CommandSetupHelper { final IpAddress sourceIp = _networkModel.getIp(rule.getSourceIpAddressId()); final StaticNatRuleTO ruleTO = new StaticNatRuleTO(0, sourceIp.getAddress().addr(), null, null, rule.getDestIpAddress(), null, null, null, rule.isForRevoke(), false); + ruleTO.setDestinationIpOnDefaultNic(isDestinationIpOnDefaultNic(guestNetworkId, rule.getDestIpAddress())); rulesTO.add(ruleTO); } } diff --git a/systemvm/debian/opt/cloud/bin/configure.py b/systemvm/debian/opt/cloud/bin/configure.py index 22671b95a07..787a6c97ac5 100755 --- a/systemvm/debian/opt/cloud/bin/configure.py +++ b/systemvm/debian/opt/cloud/bin/configure.py @@ -1567,12 +1567,13 @@ class CsForwardingRules(CsDataBag): self.fw.append(["nat", "front", "-A POSTROUTING -s %s -d %s -j SNAT -o %s --to-source %s" % (self.getNetworkByIp(rule['internal_ip']), rule["internal_ip"], self.getDeviceByIp(rule["internal_ip"]), self.getGuestIpByIp(rule["internal_ip"]))]) - internal_device = self.getDeviceByIp(rule["internal_ip"]) - internal_vr_ip = self.getGuestIpByIp(rule["internal_ip"]) - if internal_device and internal_vr_ip and internal_device != device: - self.fw.append(["nat", "front", - "-A POSTROUTING -o %s -d %s/32 -j SNAT --to-source %s" % - (internal_device, rule["internal_ip"], internal_vr_ip)]) + destination_ip_on_default_nic = rule.get("destination_ip_on_default_nic", True) + if not destination_ip_on_default_nic: + internal_device = self.getDeviceByIp(rule["internal_ip"]) + internal_vr_ip = self.getGuestIpByIp(rule["internal_ip"]) + if internal_device and internal_vr_ip and internal_device != device: + self.fw.append(["nat", "front", + "-A POSTROUTING -o %s -d %s/32 -j SNAT --to-source %s" % (internal_device, rule["internal_ip"], internal_vr_ip)]) class IpTablesExecutor: diff --git a/systemvm/debian/opt/cloud/bin/cs_forwardingrules.py b/systemvm/debian/opt/cloud/bin/cs_forwardingrules.py index 199d4e77a98..f106d6b7a2d 100755 --- a/systemvm/debian/opt/cloud/bin/cs_forwardingrules.py +++ b/systemvm/debian/opt/cloud/bin/cs_forwardingrules.py @@ -27,6 +27,8 @@ def merge(dbag, rules): newrule = dict() newrule["public_ip"] = source_ip newrule["internal_ip"] = destination_ip + if "destination_ip_on_default_nic" in rule: + newrule["destination_ip_on_default_nic"] = rule["destination_ip_on_default_nic"] if rules["type"] == "staticnatrules": newrule["type"] = "staticnat" From 6d8746f15accbccbf53702c4da74688a993773e4 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Fri, 22 May 2026 15:13:50 +0530 Subject: [PATCH 3/3] add config network.nic.snat.enabled to setup nic aware snat rule --- .../cloud/agent/api/to/StaticNatRuleTO.java | 10 +++++----- .../facade/SetStaticNatRulesConfigItem.java | 2 +- .../virtualnetwork/model/StaticNatRule.java | 16 +++++++-------- .../network/router/CommandSetupHelper.java | 20 ++++++++++++++----- .../VirtualNetworkApplianceManager.java | 12 +++++++++++ .../VirtualNetworkApplianceManagerImpl.java | 3 ++- systemvm/debian/opt/cloud/bin/configure.py | 13 ++++++------ .../opt/cloud/bin/cs_forwardingrules.py | 4 ++-- 8 files changed, 52 insertions(+), 28 deletions(-) diff --git a/api/src/main/java/com/cloud/agent/api/to/StaticNatRuleTO.java b/api/src/main/java/com/cloud/agent/api/to/StaticNatRuleTO.java index 4cac660ace1..9c817f2debb 100644 --- a/api/src/main/java/com/cloud/agent/api/to/StaticNatRuleTO.java +++ b/api/src/main/java/com/cloud/agent/api/to/StaticNatRuleTO.java @@ -28,7 +28,7 @@ import com.cloud.network.rules.StaticNatRule; public class StaticNatRuleTO extends FirewallRuleTO { String dstIp; - boolean destinationIpOnDefaultNic = true; + boolean shouldApplyCrossNetworkSnat = false; protected StaticNatRuleTO() { } @@ -80,12 +80,12 @@ public class StaticNatRuleTO extends FirewallRuleTO { return dstIp; } - public boolean isDestinationIpOnDefaultNic() { - return destinationIpOnDefaultNic; + public boolean isShouldApplyCrossNetworkSnat() { + return shouldApplyCrossNetworkSnat; } - public void setDestinationIpOnDefaultNic(boolean destinationIpOnDefaultNic) { - this.destinationIpOnDefaultNic = destinationIpOnDefaultNic; + public void setShouldApplyCrossNetworkSnat(boolean shouldApplyCrossNetworkSnat) { + this.shouldApplyCrossNetworkSnat = shouldApplyCrossNetworkSnat; } } diff --git a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetStaticNatRulesConfigItem.java b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetStaticNatRulesConfigItem.java index 1b60cc6ca7b..af0332b70f6 100644 --- a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetStaticNatRulesConfigItem.java +++ b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/SetStaticNatRulesConfigItem.java @@ -40,7 +40,7 @@ public class SetStaticNatRulesConfigItem extends AbstractConfigItemFacade { final LinkedList rules = new LinkedList<>(); for (final StaticNatRuleTO rule : command.getRules()) { final StaticNatRule staticNatRule = new StaticNatRule(rule.revoked(), rule.getProtocol(), rule.getSrcIp(), - rule.getStringSrcPortRange(), rule.getDstIp(), rule.isDestinationIpOnDefaultNic()); + rule.getStringSrcPortRange(), rule.getDstIp(), rule.isShouldApplyCrossNetworkSnat()); rules.add(staticNatRule); } final StaticNatRules staticNatRules = new StaticNatRules(rules); diff --git a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/StaticNatRule.java b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/StaticNatRule.java index 417c5edc11d..09f8daab0f7 100644 --- a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/StaticNatRule.java +++ b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/model/StaticNatRule.java @@ -25,25 +25,25 @@ public class StaticNatRule { private String sourceIpAddress; private String sourcePortRange; private String destinationIpAddress; - private boolean destinationIpOnDefaultNic = true; + private boolean shouldApplyCrossNetworkSnat = false; public StaticNatRule() { // Empty constructor for (de)serialization } public StaticNatRule(boolean revoke, String protocol, String sourceIpAddress, String sourcePortRange, String destinationIpAddress) { - this(revoke, protocol, sourceIpAddress, sourcePortRange, destinationIpAddress, true); + this(revoke, protocol, sourceIpAddress, sourcePortRange, destinationIpAddress, false); } public StaticNatRule(boolean revoke, String protocol, String sourceIpAddress, String sourcePortRange, - String destinationIpAddress, boolean destinationIpOnDefaultNic) { + String destinationIpAddress, boolean shouldApplyCrossNetworkSnat) { super(); this.revoke = revoke; this.protocol = protocol; this.sourceIpAddress = sourceIpAddress; this.sourcePortRange = sourcePortRange; this.destinationIpAddress = destinationIpAddress; - this.destinationIpOnDefaultNic = destinationIpOnDefaultNic; + this.shouldApplyCrossNetworkSnat = shouldApplyCrossNetworkSnat; } public boolean isRevoke() { @@ -86,12 +86,12 @@ public class StaticNatRule { this.destinationIpAddress = destinationIpAddress; } - public boolean isDestinationIpOnDefaultNic() { - return destinationIpOnDefaultNic; + public boolean isShouldApplyCrossNetworkSnat() { + return shouldApplyCrossNetworkSnat; } - public void setDestinationIpOnDefaultNic(boolean destinationIpOnDefaultNic) { - this.destinationIpOnDefaultNic = destinationIpOnDefaultNic; + public void setShouldApplyCrossNetworkSnat(boolean shouldApplyCrossNetworkSnat) { + this.shouldApplyCrossNetworkSnat = shouldApplyCrossNetworkSnat; } } diff --git a/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java b/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java index 1214e8e3d26..b01083e6b07 100644 --- a/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java +++ b/server/src/main/java/com/cloud/network/router/CommandSetupHelper.java @@ -446,7 +446,7 @@ public class CommandSetupHelper { for (final StaticNatRule rule : rules) { final IpAddress sourceIp = _networkModel.getIp(rule.getSourceIpAddressId()); final StaticNatRuleTO ruleTO = new StaticNatRuleTO(rule, null, sourceIp.getAddress().addr(), rule.getDestIpAddress()); - ruleTO.setDestinationIpOnDefaultNic(isDestinationIpOnDefaultNic(guestNetworkId, rule.getDestIpAddress())); + ruleTO.setShouldApplyCrossNetworkSnat(requiresReturnPathSnat(guestNetworkId, rule.getDestIpAddress())); rulesTO.add(ruleTO); } } @@ -460,13 +460,23 @@ public class CommandSetupHelper { cmds.addCommand(cmd); } - private boolean isDestinationIpOnDefaultNic(final long networkId, final String destinationIp) { + /** + * This method determines whether to use network-wide SNAT or NIC aware SNAT + * @param networkId + * @param destinationIp + * @return + */ + private boolean requiresReturnPathSnat(final long networkId, final String destinationIp) { + if (!VirtualNetworkApplianceManager.NicSnatEnabled.value()) { + return false; + } + final NicVO destinationNic = _nicDao.findByIp4AddressAndNetworkId(destinationIp, networkId); if (destinationNic == null) { logger.debug("Unable to find destination NIC for ip [{}] in network [{}], assuming default NIC.", destinationIp, networkId); - return true; + return false; } - return destinationNic.isDefaultNic(); + return !destinationNic.isDefaultNic(); } public void createApplyFirewallRulesCommands(final List rules, final VirtualRouter router, final Commands cmds, final long guestNetworkId) { @@ -707,7 +717,7 @@ public class CommandSetupHelper { final IpAddress sourceIp = _networkModel.getIp(rule.getSourceIpAddressId()); final StaticNatRuleTO ruleTO = new StaticNatRuleTO(0, sourceIp.getAddress().addr(), null, null, rule.getDestIpAddress(), null, null, null, rule.isForRevoke(), false); - ruleTO.setDestinationIpOnDefaultNic(isDestinationIpOnDefaultNic(guestNetworkId, rule.getDestIpAddress())); + ruleTO.setShouldApplyCrossNetworkSnat(requiresReturnPathSnat(guestNetworkId, rule.getDestIpAddress())); rulesTO.add(ruleTO); } } diff --git a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManager.java b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManager.java index 8ef77d3fb32..58607b4a106 100644 --- a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManager.java +++ b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManager.java @@ -50,6 +50,7 @@ public interface VirtualNetworkApplianceManager extends Manager, VirtualNetworkA String RouterHealthChecksResultFetchIntervalCK = "router.health.checks.results.fetch.interval"; String RouterHealthChecksFailuresToRecreateVrCK = "router.health.checks.failures.to.recreate.vr"; String RemoveControlIpOnStopCK = "systemvm.release.control.ip.on.stop"; + String UseNicAwareSnat = "network.nic.snat.enabled"; ConfigKey RouterTemplateXen = new ConfigKey<>(String.class, RouterTemplateXenCK, "Advanced", "SystemVM Template (XenServer)", "Name of the default router template on Xenserver.", true, ConfigKey.Scope.Zone, null); @@ -131,6 +132,17 @@ public interface VirtualNetworkApplianceManager extends Manager, VirtualNetworkA ConfigKey RemoveControlIpOnStop = new ConfigKey<>(Boolean.class, RemoveControlIpOnStopCK, "Advanced", "true", "on stopping routers and system VMs the IP will be released to preserve IPv4 space.", true, ConfigKey.Scope.Zone, null); + ConfigKey NicSnatEnabled = new ConfigKey<>(Boolean.class, + UseNicAwareSnat, + "Advanced", + "false", + "When enabled, Virtual Router overwrites the SNAT source IP in POSTROUTING for traffic exiting " + + "non-default NICs. The SNAT source is selected based on the egress interface to preserve return path " + + "consistency in multi-NIC configurations.", + true, + ConfigKey.Scope.Global, + null); + int DEFAULT_ROUTER_VM_RAMSIZE = 256; // 256M int DEFAULT_ROUTER_CPU_MHZ = 500; // 500 MHz boolean USE_POD_VLAN = false; diff --git a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java index dd65719ad03..f2207a34299 100644 --- a/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java +++ b/server/src/main/java/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java @@ -3382,7 +3382,8 @@ Configurable, StateListener