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 34E2D1FF144 for ; Tue, 10 Mar 2026 10:10:05 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 28BA114A74; Tue, 10 Mar 2026 10:09:59 +0100 (CET) Date: Tue, 10 Mar 2026 10:09:55 +0100 From: Gabriel Goller To: Lukas Sichert Subject: Re: [PATCH network] fix #6255: SNAT: fix incorrect IP collection for sdn config Message-ID: Mail-Followup-To: Lukas Sichert , pve-devel@lists.proxmox.com References: <20260309091346.20049-1-l.sichert@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20260309091346.20049-1-l.sichert@proxmox.com> User-Agent: NeoMutt/20241002-35-39f9a6 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1773133762046 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.635 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 URIBL_BLACK 3 Contains an URL listed in the URIBL blacklist [plugin.pm] Message-ID-Hash: PKHHX33Q7IQUXUIB27VRF73GG7MEMNM2 X-Message-ID-Hash: PKHHX33Q7IQUXUIB27VRF73GG7MEMNM2 X-MailFrom: g.goller@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 CC: pve-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: A `make tidy` run would be nice, there are some formatting issues :) > [snip] > diff --git a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm > index 8e7ddfd..f69bc48 100644 > --- a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm > +++ b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm > @@ -253,7 +253,7 @@ sub generate_sdn_config { > > #find outgoing interface > my ($outip, $outiface) = > - PVE::Network::SDN::Zones::Plugin::get_local_route_ip($checkrouteip); > + 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..b021899 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,9 +280,9 @@ sub del_bridge_fdb { > #helper > > sub get_local_route_ip { > - my ($targetip) = @_; > + my ($targetip, $interfaces_config) = @_; > > - my $ip = undef; > + my $ip = undef; > my $interface = undef; > > run_command( > @@ -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) { > + if ($ip_address_in_config ne $ip) { > + 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" Typo: s/it ip/its ip/ Also maybe "... does not match with its actual ip address reported by ip route ..." -- IMO this is clearer. > + ); > + } > + return ($ip_address_in_config, $interface); > + } > + Move this check directly below the declaration -- this way we can simplify the above if-statement and exit early. > + 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"); } > return ($ip, $interface); > } > > @@ -323,7 +342,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); > } > } The rest looks good. Acked-by: Gabriel Goller