Re: [PATCH network] fix #6255: SNAT: fix incorrect IP collection for sdn config

[Gabriel Goller <g.goller@proxmox.com>]

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 <g.goller@proxmox.com>