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 B407E1FF153 for ; Mon, 22 Jun 2026 14:54:27 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 61C3FB9F3; Mon, 22 Jun 2026 14:54:26 +0200 (CEST) Message-ID: <87cf364d-2253-4d47-b6f0-29120f12fceb@proxmox.com> Date: Mon, 22 Jun 2026 14:53:50 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH networking v2] fix #6255: SNAT: fix incorrect IP collection for sdn config To: pve-devel@lists.proxmox.com References: <20260327153959.87687-1-l.sichert@proxmox.com> Content-Language: en-US From: Stefan Hanreich In-Reply-To: <20260327153959.87687-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.445 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 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: HBCIVG5NT6H3T3GOKFZSRZTVQI4KL6TJ X-Message-ID-Hash: HBCIVG5NT6H3T3GOKFZSRZTVQI4KL6TJ 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: functionality-wise looks fine, some annotations to the code inline On 3/27/26 4:39 PM, Lukas Sichert wrote: > On a host with a SimpleZone or EVPN and SNAT configured, when changing > the ip address of the bridge that handles the SNAT, the changes are not > reflected in /etc/network/interfaces.d/sdn. This happens because the ip > address is resolved via 'ip route get 8.8.8.8'. By the time this command > is executed, no ifreload has been executed yet, so ip route still > contains the old ip address. > > Determine the IP version (IPv4 or IPv6) of the address used with 'ip > route'. Then parse the incoming /etc/network/interfaces file and look up > the interface currently used by 'ip route'. If the interface is still > present and has a gateway and ip address with the correct version, but > the ip address has changed, update the ip address in the SDN > configuration accordingly and log the change to the journal. > > Signed-off-by: Lukas Sichert > --- > > Notes: > changes from v1 to v2: (thanks @Gabriel, @Stefan) > -add handling for IPV6 addresses > -remove warnings for missing ip/gateway, as they might trigger > in some legitimate scenarios > -add canonical formatting for IPv4 and IPV6 > -execute make tidy > -small restructuring of if clauses for earlier exit > -fix typos > > concerning the ignoring of live routing tables: > > The function only determines the currently used interface (via 'ip > route') and its ip address and returns these two. The gateway configured > in /etc/network/interfaces is only used to verify that the interface is > still a valid candidate after an 'ifreload -a'. As it cannot switch > interfaces relative what to 'ip route' reports it should not be able > influence gateway selection. > > If I am missing a more obscure way or an edge case where gateway > selection could still be affected, please let me know. > > Also I have thought about adding "... does not match with its actual ip > address reported by ip route ...", but after this would be displayed > 'ifreload -a' gets executed and 'ip route' changes. > > src/PVE/Network/SDN/Zones/EvpnPlugin.pm | 5 ++-- > src/PVE/Network/SDN/Zones/Plugin.pm | 30 +++++++++++++++++++++-- > src/PVE/Network/SDN/Zones/SimplePlugin.pm | 5 ++-- > 3 files changed, 34 insertions(+), 6 deletions(-) > > diff --git a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm > index 8e7ddfd..91b8a7e 100644 > --- a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm > +++ b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm > @@ -252,8 +252,9 @@ sub generate_sdn_config { > if ($subnet->{snat}) { > > #find outgoing interface > - my ($outip, $outiface) = > - PVE::Network::SDN::Zones::Plugin::get_local_route_ip($checkrouteip); > + my ($outip, $outiface) = PVE::Network::SDN::Zones::Plugin::get_local_route_ip( > + $checkrouteip, $interfaces_config, > + ); > if ($outip && $outiface && $is_evpn_gateway) { > #use snat, faster than masquerade > push @iface_config, > diff --git a/src/PVE/Network/SDN/Zones/Plugin.pm b/src/PVE/Network/SDN/Zones/Plugin.pm > index 826ebdf..1d18acc 100644 > --- a/src/PVE/Network/SDN/Zones/Plugin.pm > +++ b/src/PVE/Network/SDN/Zones/Plugin.pm > @@ -8,6 +8,7 @@ use PVE::IPRoute2; > use PVE::JSONSchema; > use PVE::Cluster; > use PVE::Network; > +use PVE::SafeSyslog; > > use PVE::JSONSchema qw(get_standard_option); > use base qw(PVE::SectionConfig); > @@ -279,7 +280,7 @@ sub del_bridge_fdb { > #helper > > sub get_local_route_ip { > - my ($targetip) = @_; > + my ($targetip, $interfaces_config) = @_; > > my $ip = undef; > my $interface = undef; > @@ -296,6 +297,31 @@ sub get_local_route_ip { > > }, > ); > + my $ipversion = Net::IP::ip_is_ipv6($targetip) ? 6 : 4; > + my $interface_in_config = $interfaces_config->{ifaces}->{$interface}; > + my $ip_address_in_config = undef; > + my $gateway_in_config = undef; > + > + if ($ipversion == 6) { > + $ip_address_in_config = PVE::Network::canonical_ip($interface_in_config->{address6}); > + $gateway_in_config = PVE::Network::canonical_ip($interface_in_config->{gateway6}); > + } else { > + $ip_address_in_config = PVE::Network::canonical_ip($interface_in_config->{address}); > + $gateway_in_config = PVE::Network::canonical_ip($interface_in_config->{gateway}); > + } nit: potentially the following is a bit nicer? my $address_key = ($ipversion == 6) ? 'address6' : 'address'; my $ip_address_in_config = PVE::Network::canonical_ip($interface_in_config->{$address_key}); > + # if the device currently used for routing still has a valid description in /network/interfaces/, use it > + if ($interface_in_config && $gateway_in_config) { > + if ($ip_address_in_config eq $ip) { > + return ($ip_address_in_config, $interface); > + } else { > + syslog( > + "warning", > + "ip address $ip_address_in_config of interface $interface in /etc/network/interfaces does not match with its ip address reported by ip route: $ip, switching to $ip_address_in_config for SNAT", > + ); > + return ($ip_address_in_config, $interface); > + } > + } > return ($ip, $interface); nit: could be shortened if ( $interface_in_config && $gateway_in_config && $ip_address_in_config ne $ip ) { syslog(..) return ($ip_address_in_config, $interface); } return ($ip, $interface); > } > > @@ -323,7 +349,7 @@ sub find_local_ip_interface_peers { > > #if peer is remote, find source with ip route > foreach my $address (@{$peers}) { > - my ($ip, $interface) = get_local_route_ip($address); > + my ($ip, $interface) = get_local_route_ip($address, $network_config); > return ($ip, $interface); > } > } > diff --git a/src/PVE/Network/SDN/Zones/SimplePlugin.pm b/src/PVE/Network/SDN/Zones/SimplePlugin.pm > index f5cd18e..ac772e7 100644 > --- a/src/PVE/Network/SDN/Zones/SimplePlugin.pm > +++ b/src/PVE/Network/SDN/Zones/SimplePlugin.pm > @@ -112,8 +112,9 @@ sub generate_sdn_config { > push @iface_config, "up ip route add $cidr dev $vnetid" if $mask == 32 && $ipversion == 4; > if ($subnet->{snat}) { > #find outgoing interface > - my ($outip, $outiface) = > - PVE::Network::SDN::Zones::Plugin::get_local_route_ip($checkrouteip); > + my ($outip, $outiface) = PVE::Network::SDN::Zones::Plugin::get_local_route_ip( > + $checkrouteip, $interfaces_config, > + ); > if ($outip && $outiface) { > #use snat, faster than masquerade > push @iface_config,