From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id DE3471FF13F for ; Thu, 12 Mar 2026 14:21:04 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B730B156D0; Thu, 12 Mar 2026 14:20:59 +0100 (CET) Message-ID: <23ea11b7-0903-4ffe-99f3-66e2cbdfd9f0@proxmox.com> Date: Thu, 12 Mar 2026 14:20:54 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH network] fix #6255: SNAT: fix incorrect IP collection for sdn config To: pve-devel@lists.proxmox.com References: <20260309091346.20049-1-l.sichert@proxmox.com> Content-Language: en-US From: Stefan Hanreich In-Reply-To: <20260309091346.20049-1-l.sichert@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.493 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes RCVD_IN_MSPIKE_H2 0.001 Average reputation (+2) RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.408 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.819 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.903 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: MTHK6KRYWXFDRJZB6N2HHITQTTGIYWOY X-Message-ID-Hash: MTHK6KRYWXFDRJZB6N2HHITQTTGIYWOY X-MailFrom: s.hanreich@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Thanks for looking at this! Some comments inline On 3/9/26 10:14 AM, Lukas Sichert wrote: [snip] > @@ -296,6 +297,24 @@ sub get_local_route_ip { > > }, > ); > + my $interface_in_config = $interfaces_config->{ifaces}->{$interface}; > + my $ip_address_in_config = $interface_in_config->{address}; > + my $gateway_in_config = $interface_in_config->{gateway}; > + > +# if the device currently used for routing still has a valid description in /network/interfaces/, use it > + if ($interface_in_config && $gateway_in_config) { This only checks for the IPv4 case. But $targetip could be an IPv6 address as well, in which case we would have to check for address6 / gateway6 respectively. E.g. the following configuration on a test host: auto vmbr0 iface vmbr0 inet static address 192.0.2.10/24 gateway 192.0.2.1 bridge-ports nic0 bridge-stp off bridge-fd 0 iface vmbr0 inet6 static address 2001:db8::1234/64 with the following SDN config: subnet: simplez-fc80::-60 vnet simplnet gateway fc80::1 snat 1 Results in an error, because the plugin tries to create an IPv6 NAT rule with an IPv4 source: simplnet : warning: simplnet: post-up cmd 'ip6tables -t nat -A POSTROUTING -s 'fc80::/60' -o vmbr0 -j SNAT --to-source 192.0.2.10' failed: returned 2 (ip6tables v1.8.11 (legacy): Bad IP address "192.0.2.10" ------------------------------- There's also the case of users announcing a default GW via a dynamic routing protocol. Usually it is advisable to configure a default gateway in this case as a fallback, but with this change we'd always utilize the fallback gateway instead of the *actual* gateway. I can see this happening in some setups where the gateways announce routes for 0.0.0.0/1 and 128.0.0.0/1 - effectively making them the default routes. > + if ($ip_address_in_config ne $ip) { whilst probably a bit niche - for IPv6, if we want to be super correct, we cannot do a simple comparison, since it is possible to express the same IP address in a multitude of ways. This is actually an issue in find_local_ip_interface_peers as well. Technically this holds true for IPv4 as well, so if we want to be really correct we'd always need to parse the address and use the canonicalized form there as well. > + syslog( "warning", > +"ip address $ip_address_in_config of interface $interface in /etc/network/interfaces does not match with it ip address reported by ip route: $ip, switching to $ip_address_in_config for SNAT" > + ); > + } > + return ($ip_address_in_config, $interface); > + } > + > + if (!$interface_in_config) { > + syslog( "warning", "current SNAT networking interface $interface is not listed in /etc/network/interfaces anymore"); > + } elsif (!$gateway_in_config && $ip_address_in_config) { > + syslog( "warning", "currently used networking interface $interface does not have a gateway configured in /etc/network/interfaces"); } I know we talked about this already, but I'm not so sure anymore that printing a warning here is such a good idea, because it can trigger in quite a few scenarios that are completely legitimate and might confuse users more than they help: * Changing a network interface name (which would be only once at least) * Configuring a gateway via post-up commands in the interfaces config * Moving the default gateway to another interface [snip]