Re: [PATCH network v3] fix #6255: snat: update changed bridge IPs in SDN config

[Stefan Hanreich <s.hanreich@proxmox.com>]

gave this another quick spin with an IPv{4,6} simple vnet, lgtm:

Tested-by: Stefan Hanreich <s.hanreich@proxmox.com>
Reviewed-by: Stefan Hanreich <s.hanreich@proxmox.com>


On 6/25/26 2:25 PM, Lukas Sichert wrote:
> When changing the IP address of an SNAT bridge in a Simple or EVPN zone,
> the generated SDN configuration retains the old address. The address is
> currently obtained through 'ip route get', which still reports the
> previous state before 'ifreload' runs.
> 
> Parse the incoming '/etc/network/interfaces' and look up the interface
> used by 'ip route'. If it still has a gateway and a changed IP address,
> update the SNAT address in the generated SDN configuration and log the
> change to the journal.
> 
> Signed-off-by: Lukas Sichert <l.sichert@proxmox.com>
> Link: https://bugzilla.proxmox.com/show_bug.cgi?id=6255
> ---
> 
> Notes:
>     changes from v2 to v3: (thanks @Stefan):
>     - simplified the address lookup by selecting the address and gateway key
>       based on the IP version
>     - add address and gateway check before canonicalzing as canonical_ip
>       creates default ip from undef input
>     - simplified the fallback logic for changed SNAT bridge addresses
>     
>     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
> 
>  src/PVE/Network/SDN/Zones/EvpnPlugin.pm   |  5 ++--
>  src/PVE/Network/SDN/Zones/Plugin.pm       | 32 +++++++++++++++++++++--
>  src/PVE/Network/SDN/Zones/SimplePlugin.pm |  5 ++--
>  3 files changed, 36 insertions(+), 6 deletions(-)
> 
> diff --git a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
> index dfbd7e9..37bc9f5 100644
> --- a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
> +++ b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
> @@ -269,8 +269,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 74a3384..5493ec1 100644
> --- a/src/PVE/Network/SDN/Zones/Plugin.pm
> +++ b/src/PVE/Network/SDN/Zones/Plugin.pm
> @@ -9,6 +9,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);
> @@ -306,7 +307,7 @@ sub get_iface_addresses {
>  }
>  
>  sub get_local_route_ip {
> -    my ($targetip) = @_;
> +    my ($targetip, $interfaces_config) = @_;
>  
>      my $ip = undef;
>      my $interface = undef;
> @@ -323,6 +324,33 @@ sub get_local_route_ip {
>  
>          },
>      );
> +    my $ipversion = Net::IP::ip_is_ipv6($targetip) ? 6 : 4;
> +    my $interface_in_config = $interfaces_config->{ifaces}->{$interface};
> +
> +    my $address_key = ($ipversion == 6) ? 'address6' : 'address';
> +    my $gateway_key = ($ipversion == 6) ? 'gateway6' : 'gateway';
> +
> +    # check if ip address and gateway exist in config before normalizing, because
> +    # canonical_ip will return 0.0.0.0, if called with undef
> +    my $ip_address_in_config;
> +    if ($ip_address_in_config = $interface_in_config->{$address_key}) {
> +        $ip_address_in_config = PVE::Network::canonical_ip($ip_address_in_config);
> +    }
> +
> +    my $gateway_in_config;
> +    if ($gateway_in_config = $interface_in_config->{$gateway_key}) {
> +        $gateway_in_config = PVE::Network::canonical_ip($gateway_in_config);
> +    }
> +
> +    # if the device currently used for routing still has a valid description in /network/interfaces/, use it
> +    if ($interface_in_config && $gateway_in_config && $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 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);
>  }
>  
> @@ -368,7 +396,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,