[pve-devel] applied: [PATCH pve-network] use PVE::Tools::split_list for ip lists

[Fabian Grünbichler]

with the following applied on-top, since we don't want declarations 
combined with post-if:

my $foo = 'bla' if $bar;

is undefined behaviour[1].

-----8<-----
diff --git a/PVE/Network/SDN/Controllers/EvpnPlugin.pm b/PVE/Network/SDN/Controllers/EvpnPlugin.pm
index 9321af1..d82de2a 100644
--- a/PVE/Network/SDN/Controllers/EvpnPlugin.pm
+++ b/PVE/Network/SDN/Controllers/EvpnPlugin.pm
@@ -47,11 +47,13 @@ sub options {
 sub generate_controller_config {
     my ($class, $plugin_config, $controller, $id, $uplinks, $config) = @_;
 
-    my @peers = PVE::Tools::split_list($plugin_config->{'peers'}) if $plugin_config->{'peers'};
+    my @peers;
+    @peers = PVE::Tools::split_list($plugin_config->{'peers'}) if $plugin_config->{'peers'};
 
     my $asn = $plugin_config->{asn};
     my $gatewaynodes = $plugin_config->{'gateway-nodes'};
-    my @gatewaypeers = PVE::Tools::split_list($plugin_config->{'gateway-external-peers'}) if $plugin_config->{'gateway-external-peers'};
+    my @gatewaypeers;
+    @gatewaypeers = PVE::Tools::split_list($plugin_config->{'gateway-external-peers'}) if $plugin_config->{'gateway-external-peers'};
 
     return if !$asn;
 
diff --git a/PVE/Network/SDN/Zones/VxlanPlugin.pm b/PVE/Network/SDN/Zones/VxlanPlugin.pm
index 5f17e15..e8cf1bd 100644
--- a/PVE/Network/SDN/Zones/VxlanPlugin.pm
+++ b/PVE/Network/SDN/Zones/VxlanPlugin.pm
@@ -50,7 +50,8 @@ sub generate_sdn_config {
     my $ipv6 = $vnet->{ipv6};
     my $mac = $vnet->{mac};
     my $multicastaddress = $plugin_config->{'multicast-address'};
-    my @peers = PVE::Tools::split_list($plugin_config->{'peers'}) if $plugin_config->{'peers'};
+    my @peers;
+    @peers = PVE::Tools::split_list($plugin_config->{'peers'}) if $plugin_config->{'peers'};
     my $vxlan_iface = "vxlan_$vnetid";
 
     die "missing vxlan tag" if !$tag;
----->8-----

I don't know enough about the code paths here to quickly evaluate this 
myself, but I have a feeling that some checks for undef are missing in 
these files, e.g.:

PVE::Network::SDN::Controllers::EvpnPlugin::generate_controler_config()

@peers and @gatewaypeers might be undef (or if not, then the post-if is 
unnecessary)
@peers is passed to PVE::Network::SDN::Zones::Plugin::find_local_ip_interface_peers
if @peers is undef, this returns undef as well ($ifaceip, $interface)
$ifaceip is put into a string inside @controller_config without checks

something else that confused me at first glance in that method:

@controller_config is re-used multiple times. it would probably be a 
good idea to find a better name for the three instances, and make that 
three variables ;)

1: https://git.proxmox.com/?p=pve-container.git;a=commitdiff;h=9de0505c772f7c382c82d9bfb170b3e0664af9ed

On June 30, 2020 2:25 pm, Alexandre Derumier wrote:
> Signed-off-by: Alexandre Derumier <aderumier at odiso.com>
> ---
>  PVE/Network/SDN/Controllers/EvpnPlugin.pm | 4 ++--
>  PVE/Network/SDN/Zones/EvpnPlugin.pm       | 2 +-
>  PVE/Network/SDN/Zones/VxlanPlugin.pm      | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/PVE/Network/SDN/Controllers/EvpnPlugin.pm b/PVE/Network/SDN/Controllers/EvpnPlugin.pm
> index 79ecaeb..9321af1 100644
> --- a/PVE/Network/SDN/Controllers/EvpnPlugin.pm
> +++ b/PVE/Network/SDN/Controllers/EvpnPlugin.pm
> @@ -47,11 +47,11 @@ sub options {
>  sub generate_controller_config {
>      my ($class, $plugin_config, $controller, $id, $uplinks, $config) = @_;
>  
> -    my @peers = split(',', $plugin_config->{'peers'}) if $plugin_config->{'peers'};
> +    my @peers = PVE::Tools::split_list($plugin_config->{'peers'}) if $plugin_config->{'peers'};
>  
>      my $asn = $plugin_config->{asn};
>      my $gatewaynodes = $plugin_config->{'gateway-nodes'};
> -    my @gatewaypeers = split(',', $plugin_config->{'gateway-external-peers'}) if $plugin_config->{'gateway-external-peers'};
> +    my @gatewaypeers = PVE::Tools::split_list($plugin_config->{'gateway-external-peers'}) if $plugin_config->{'gateway-external-peers'};
>  
>      return if !$asn;
>  
> diff --git a/PVE/Network/SDN/Zones/EvpnPlugin.pm b/PVE/Network/SDN/Zones/EvpnPlugin.pm
> index 95fbb64..b2f57ee 100644
> --- a/PVE/Network/SDN/Zones/EvpnPlugin.pm
> +++ b/PVE/Network/SDN/Zones/EvpnPlugin.pm
> @@ -52,7 +52,7 @@ sub generate_sdn_config {
>      die "missing vxlan tag" if !$tag;
>      warn "vlan-aware vnet can't be enabled with evpn plugin" if $vnet->{vlanaware};
>  
> -    my @peers = split(',', $controller->{'peers'});
> +    my @peers = PVE::Tools::split_list($controller->{'peers'});
>      my ($ifaceip, $iface) = PVE::Network::SDN::Zones::Plugin::find_local_ip_interface_peers(\@peers);
>  
>      my $mtu = 1450;
> diff --git a/PVE/Network/SDN/Zones/VxlanPlugin.pm b/PVE/Network/SDN/Zones/VxlanPlugin.pm
> index bc585c6..5f17e15 100644
> --- a/PVE/Network/SDN/Zones/VxlanPlugin.pm
> +++ b/PVE/Network/SDN/Zones/VxlanPlugin.pm
> @@ -50,7 +50,7 @@ sub generate_sdn_config {
>      my $ipv6 = $vnet->{ipv6};
>      my $mac = $vnet->{mac};
>      my $multicastaddress = $plugin_config->{'multicast-address'};
> -    my @peers = split(',', $plugin_config->{'peers'}) if $plugin_config->{'peers'};
> +    my @peers = PVE::Tools::split_list($plugin_config->{'peers'}) if $plugin_config->{'peers'};
>      my $vxlan_iface = "vxlan_$vnetid";
>  
>      die "missing vxlan tag" if !$tag;
> -- 
> 2.20.1
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>