From: Alexandre Derumier <aderumier@odiso.com>
Date: Mon, 5 Oct 2020 15:08:53 +0000 (+0200)
Subject: Fix vnet gateway for routed setup + /32 pointopoint subnet
X-Git-Url: https://git.proxmox.com/?p=pve-network.git;a=commitdiff_plain;h=e612faf6ba28c0ba52446236067b4cbeeae1a5fa

Fix vnet gateway for routed setup + /32 pointopoint subnet

add vnet to subnets && remove subnetlist from vnet

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---

diff --git a/PVE/API2/Network/SDN/Subnets.pm b/PVE/API2/Network/SDN/Subnets.pm
index 5899a1a..bc2b18f 100644
--- a/PVE/API2/Network/SDN/Subnets.pm
+++ b/PVE/API2/Network/SDN/Subnets.pm
@@ -144,17 +144,7 @@ __PACKAGE__->register_method ({
 		}
 
 		$cfg->{ids}->{$id} = $opts;
-		PVE::Network::SDN::SubnetPlugin->on_update_hook($id, $cfg);
-
-		my $ipam_cfg = PVE::Network::SDN::Ipams::config();
-		my $ipam = $cfg->{ids}->{$id}->{ipam};
-		if ($ipam) {
-		    raise_param_exc({ ipam => "$ipam not existing"}) if !$ipam_cfg->{ids}->{$ipam};
-		    my $plugin_config = $ipam_cfg->{ids}->{$ipam};
-		    my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
-		    $plugin->add_subnet($plugin_config, $id, $cfg->{ids}->{$id});
-		    $plugin->add_ip($plugin_config, $id, $opts->{gateway}, 1) if $opts->{gateway};
-		}
+		PVE::Network::SDN::SubnetPlugin->on_update_hook($id, $opts);
 
 		PVE::Network::SDN::Subnets::write_config($cfg);
 		PVE::Network::SDN::increase_version();
@@ -192,24 +182,7 @@ __PACKAGE__->register_method ({
 	    my $opts = PVE::Network::SDN::SubnetPlugin->check_config($id, $param, 0, 1);
 	    $cfg->{ids}->{$id} = $opts;
 
-	    PVE::Network::SDN::SubnetPlugin->on_update_hook($id, $cfg);
-
-            my $ipam_cfg = PVE::Network::SDN::Ipams::config();
-            my $ipam = $cfg->{ids}->{$id}->{ipam};
-	    if ($ipam) {
-		raise_param_exc({ ipam => "$ipam not existing"}) if !$ipam_cfg->{ids}->{$ipam};
-		my $plugin_config = $ipam_cfg->{ids}->{$ipam};
-		my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
-		$plugin->add_subnet($plugin_config, $id, $cfg->{ids}->{$id});
-
-		if($opts->{gateway} && $scfg->{gateway} && $opts->{gateway} ne $scfg->{gateway}) {
-		    $plugin->del_ip($plugin_config, $id, $scfg->{gateway});
-		}
-		if (!defined($opts->{gateway}) && $scfg->{gateway}) {
-		    $plugin->del_ip($plugin_config, $id, $scfg->{gateway});
-		} 
-		$plugin->add_ip($plugin_config, $id, $opts->{gateway}, 1) if $opts->{gateway};
-	    }
+	    PVE::Network::SDN::SubnetPlugin->on_update_hook($id, $opts, $scfg);
 
 	    PVE::Network::SDN::Subnets::write_config($cfg);
 	    PVE::Network::SDN::increase_version();
diff --git a/PVE/Network/SDN/SubnetPlugin.pm b/PVE/Network/SDN/SubnetPlugin.pm
index 44751e9..97d8cb8 100644
--- a/PVE/Network/SDN/SubnetPlugin.pm
+++ b/PVE/Network/SDN/SubnetPlugin.pm
@@ -8,6 +8,8 @@ use base qw(PVE::SectionConfig);
 use PVE::JSONSchema qw(get_standard_option);
 use PVE::Exception qw(raise raise_param_exc);
 use Net::Subnet qw(subnet_matcher);
+use PVE::Network::SDN::Vnets;
+use PVE::Network::SDN::Ipams;
 
 PVE::Cluster::cfs_register_file('sdn/subnets.cfg',
                                  sub { __PACKAGE__->parse_config(@_); },
@@ -52,6 +54,10 @@ sub private {
 
 sub properties {
     return {
+        vnet => {
+            type => 'string',
+            description => "associated vnet",
+        },
         gateway => {
             type => 'string', format => 'ip',
             description => "Subnet Gateway: Will be assign on vnet for layer3 zones",
@@ -108,21 +114,29 @@ sub options {
 }
 
 sub on_update_hook {
-    my ($class, $subnetid, $subnet_cfg) = @_;
+    my ($class, $subnetid, $subnet, $old_subnet) = @_;
 
     my $cidr = $subnetid =~ s/-/\//r;
     my $subnet_matcher = subnet_matcher($cidr);
 
-    my $subnet = $subnet_cfg->{ids}->{$subnetid};
-
+    my $vnetid = $subnet->{vnet};
     my $gateway = $subnet->{gateway};
+    my $ipam = $subnet->{ipam};
     my $dns = $subnet->{dns};
     my $dnszone = $subnet->{dnszone};
     my $reversedns = $subnet->{reversedns};
     my $reversednszone = $subnet->{reversednszone};
 
-    #to: for /32 pointotoping, allow gateway outside the subnet
-    raise_param_exc({ gateway => "$gateway is not in subnet $subnet"}) if $gateway && !$subnet_matcher->($gateway);
+    my $old_gateway = $old_subnet->{gateway} if $old_subnet;
+
+    if($vnetid) {
+	my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid);
+	raise_param_exc({ vnet => "$vnetid don't exist"}) if !$vnet;
+    }
+
+    my ($ip, $mask) = split(/\//, $cidr);
+    #for /32 pointopoint, we allow gateway outside the subnet
+    raise_param_exc({ gateway => "$gateway is not in subnet $subnetid"}) if $gateway && !$subnet_matcher->($gateway) && $mask != 32;
 
     raise_param_exc({ dns => "missing dns provider"}) if $dnszone && !$dns;
     raise_param_exc({ dnszone => "missing dns zone"}) if $dns && !$dnszone;
@@ -130,21 +144,37 @@ sub on_update_hook {
     raise_param_exc({ reversednszone => "missing dns zone"}) if $reversedns && !$reversednszone;
     raise_param_exc({ reversedns => "missing forward dns zone"}) if $reversednszone && !$dnszone;
 
+    if ($ipam) {
+	my $ipam_cfg = PVE::Network::SDN::Ipams::config();
+	my $plugin_config = $ipam_cfg->{ids}->{$ipam};
+	raise_param_exc({ ipam => "$ipam not existing"}) if !$plugin_config;
+	my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
+	$plugin->add_subnet($plugin_config, $subnetid, $subnet);
+
+	#delete on removal
+	if (!defined($gateway) && $old_gateway) {
+	    eval {
+		PVE::Network::SDN::Subnets::del_ip($subnetid, $old_subnet, $old_gateway);
+	    };
+	    warn if $@;
+	}
+        if(!$old_gateway || $gateway && $gateway ne $old_gateway) {
+	    PVE::Network::SDN::Subnets::add_ip($subnetid, $subnet, $gateway);
+	}
+
+	#delete old ip after update
+	if($gateway && $old_gateway && $gateway ne $old_gateway) {
+	    eval {
+		PVE::Network::SDN::Subnets::del_ip($subnetid, $old_subnet, $old_gateway);
+	    };
+	    warn if $@;
+	}
+    }
 }
 
 sub on_delete_hook {
     my ($class, $subnetid, $subnet_cfg, $vnet_cfg) = @_;
 
-    #verify if vnets have subnet
-    foreach my $vnetid (keys %{$vnet_cfg->{ids}}) {
-	my $vnet = $vnet_cfg->{ids}->{$vnetid};
-	my @subnets = PVE::Tools::split_list($vnet->{subnets}) if $vnet->{subnets};
-	foreach my $subnet (@subnets) {
-	    my $id = $subnet =~ s/\//-/r;
-	    raise_param_exc({ subnet => "$subnet is attached to vnet $vnetid"}) if $id eq $subnetid;
-	}
-    }
-
     return;
 }
 
diff --git a/PVE/Network/SDN/Subnets.pm b/PVE/Network/SDN/Subnets.pm
index 07ef688..626b71d 100644
--- a/PVE/Network/SDN/Subnets.pm
+++ b/PVE/Network/SDN/Subnets.pm
@@ -57,20 +57,18 @@ sub get_subnet {
 }
 
 sub find_ip_subnet {
-    my ($ip, $subnetslist) = @_;
-
-    my $subnets_cfg = PVE::Network::SDN::Subnets::config();
-    my @subnets = PVE::Tools::split_list($subnetslist) if $subnetslist;
+    my ($ip, $subnets) = @_;
 
     my $subnet = undef;
     my $subnetid = undef;
 
-    foreach my $s (@subnets) {
-        my $subnet_matcher = subnet_matcher($s);
-        next if !$subnet_matcher->($ip);
-        $subnetid = $s =~ s/\//-/r;
-        $subnet = $subnets_cfg->{ids}->{$subnetid};
-        last;
+    foreach my $id (sort keys %{$subnets}) {
+	my $cidr = $id =~ s/-/\//r;
+	my $subnet_matcher = subnet_matcher($cidr);
+	next if !$subnet_matcher->($ip);
+	$subnet = $subnets->{$id};
+	$subnetid = $id;
+	last;
     }
     die  "can't find any subnet for ip $ip" if !$subnet;
 
@@ -159,8 +157,11 @@ sub next_free_ip {
 	my $ipam_cfg = PVE::Network::SDN::Ipams::config();
 	my $plugin_config = $ipam_cfg->{ids}->{$ipamid};
 	my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
-	$cidr = $plugin->add_next_freeip($plugin_config, $subnetid, $subnet);
-	($ip, undef) = split(/\//, $cidr);
+	eval {
+	    $cidr = $plugin->add_next_freeip($plugin_config, $subnetid, $subnet);
+	    ($ip, undef) = split(/\//, $cidr);
+	};
+	die $@ if $@;
     }
 
     eval {
@@ -183,6 +184,8 @@ sub next_free_ip {
 sub add_ip {
     my ($subnetid, $subnet, $ip, $hostname) = @_;
 
+    return if !$subnet;
+
     my $ipamid = $subnet->{ipam};
     my $dns = $subnet->{dns};
     my $dnszone = $subnet->{dnszone};
@@ -198,7 +201,10 @@ sub add_ip {
 	my $ipam_cfg = PVE::Network::SDN::Ipams::config();
 	my $plugin_config = $ipam_cfg->{ids}->{$ipamid};
 	my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
-	$plugin->add_ip($plugin_config, $subnetid, $ip);
+	eval {
+	    $plugin->add_ip($plugin_config, $subnetid, $ip);
+	};
+	die $@ if $@;
     }
 
     eval {
@@ -220,6 +226,8 @@ sub add_ip {
 sub del_ip {
     my ($subnetid, $subnet, $ip, $hostname) = @_;
 
+    return if !$subnet;
+
     my $ipamid = $subnet->{ipam};
     my $dns = $subnet->{dns};
     my $dnszone = $subnet->{dnszone};
diff --git a/PVE/Network/SDN/VnetPlugin.pm b/PVE/Network/SDN/VnetPlugin.pm
index 6b2bcc8..47fd4d4 100644
--- a/PVE/Network/SDN/VnetPlugin.pm
+++ b/PVE/Network/SDN/VnetPlugin.pm
@@ -68,11 +68,6 @@ sub properties {
             description => "alias name of the vnet",
 	    optional => 1,
         },
-        subnets => {
-            type => 'string',
-            description => "Subnets list",
-	    optional => 1,
-        },
         mac => {
             type => 'string',
             description => "Anycast router mac address",
@@ -86,16 +81,21 @@ sub options {
         zone => { optional => 0},
         tag => { optional => 1},
         alias => { optional => 1 },
-        subnets => { optional => 1 },
         mac => { optional => 1 },
         vlanaware => { optional => 1 },
     };
 }
 
 sub on_delete_hook {
-    my ($class, $sdnid, $vnet_cfg) = @_;
+    my ($class, $vnetid, $vnet_cfg) = @_;
 
-    return;
+    #verify if subnets are associated
+    my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid);
+    my @subnetlist = ();
+    foreach my $subnetid (sort keys %{$subnets}) {
+	push @subnetlist, $subnetid;
+    }
+    raise_param_exc({ vnet => "Vnet is attached to following subnets:". join(',', @subnetlist)}) if @subnetlist > 0;
 }
 
 sub on_update_hook {
@@ -111,13 +111,6 @@ sub on_update_hook {
 	    }
 	}
     }
-
-    #verify subnet
-    my @subnets = PVE::Tools::split_list($vnet_cfg->{ids}->{$vnetid}->{subnets}) if $vnet_cfg->{ids}->{$vnetid}->{subnets};
-    foreach my $subnet (@subnets) {
-	my $id = $subnet =~ s/\//-/r;
-	raise_param_exc({ subnet => "$subnet not existing"}) if !$subnet_cfg->{ids}->{$id};
-    }
 }
 
 1;
diff --git a/PVE/Network/SDN/Vnets.pm b/PVE/Network/SDN/Vnets.pm
index c9916b1..7cec418 100644
--- a/PVE/Network/SDN/Vnets.pm
+++ b/PVE/Network/SDN/Vnets.pm
@@ -54,22 +54,35 @@ sub get_vnet {
     return $vnet;
 }
 
+sub get_subnets {
+    my ($vnetid) = @_;
+
+    my $subnets = {};
+    my $subnets_cfg = PVE::Network::SDN::Subnets::config();
+    foreach my $subnetid (sort keys %{$subnets_cfg->{ids}}) {
+	my $subnet = $subnets_cfg->{ids}->{$subnetid};
+	next if !$subnet->{vnet} || $subnet->{vnet} ne $vnetid;
+	$subnets->{$subnetid} = $subnet;
+    }
+    return $subnets;
+
+}
+
 sub get_next_free_ip {
-    my ($vnet, $hostname, $ipversion) = @_;
+    my ($vnetid, $hostname, $ipversion) = @_;
 
     $ipversion = 4 if !$ipversion;
-    my $subnets_cfg = PVE::Network::SDN::Subnets::config();
-    my @subnets = PVE::Tools::split_list($vnet->{subnets}) if $vnet->{subnets};
+    my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid);
     my $ip = undef;
-    my $subnet = undef;
     my $subnetcount = 0;
-    foreach my $s (@subnets) {
-	my $subnetid = $s =~ s/\//-/r;
+
+    foreach my $subnetid (sort keys %{$subnets}) {
+        my $subnet = $subnets->{$subnetid};
 	my ($network, $mask) = split(/-/, $subnetid);
+
 	next if $ipversion != Net::IP::ip_get_version($network);
 	$subnetcount++;
-	$subnet = $subnets_cfg->{ids}->{$subnetid};
-	if ($subnet && $subnet->{ipam}) {
+	if ($subnet->{ipam}) {
 	    eval {
 		$ip = PVE::Network::SDN::Subnets::next_free_ip($subnetid, $subnet, $hostname);
 	    };
@@ -83,21 +96,23 @@ sub get_next_free_ip {
 }
 
 sub add_ip {
-    my ($vnet, $cidr, $hostname) = @_;
+    my ($vnetid, $cidr, $hostname) = @_;
+
+    my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid);
 
     my ($ip, $mask) = split(/\//, $cidr);
-    my ($subnetid, $subnet) = PVE::Network::SDN::Subnets::find_ip_subnet($ip, $vnet->{subnets});
-    return if !$subnet->{ipam};
+    my ($subnetid, $subnet) = PVE::Network::SDN::Subnets::find_ip_subnet($ip, $subnets);
 
     PVE::Network::SDN::Subnets::add_ip($subnetid, $subnet, $ip, $hostname);
 }
 
 sub del_ip {
-    my ($vnet, $cidr, $hostname) = @_;
+    my ($vnetid, $cidr, $hostname) = @_;
+
+    my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid);
 
     my ($ip, $mask) = split(/\//, $cidr);
-    my ($subnetid, $subnet) = PVE::Network::SDN::Subnets::find_ip_subnet($ip, $vnet->{subnets});
-    return if !$subnet->{ipam};
+    my ($subnetid, $subnet) = PVE::Network::SDN::Subnets::find_ip_subnet($ip, $subnets);
 
     PVE::Network::SDN::Subnets::del_ip($subnetid, $subnet, $ip, $hostname);
 }
diff --git a/PVE/Network/SDN/Zones/EvpnPlugin.pm b/PVE/Network/SDN/Zones/EvpnPlugin.pm
index 0ebe13e..ff25f12 100644
--- a/PVE/Network/SDN/Zones/EvpnPlugin.pm
+++ b/PVE/Network/SDN/Zones/EvpnPlugin.pm
@@ -76,10 +76,16 @@ sub generate_sdn_config {
     #vnet bridge
     @iface_config = ();
 
-    my @subnets = PVE::Tools::split_list($vnet->{subnets}) if $vnet->{subnets};
-    foreach my $subnet (@subnets) {
-        next if !defined($subnet_cfg->{ids}->{$subnet});
-        push @iface_config, "address $subnet_cfg->{ids}->{$subnet}->{gateway}" if $subnet_cfg->{ids}->{$subnet}->{gateway};
+    my $address = {};
+    my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid);
+    foreach my $subnetid (sort keys %{$subnets}) {
+	my $subnet = $subnets->{$subnetid};
+	my $cidr = $subnetid =~ s/-/\//r;
+	my $gateway = $subnet->{gateway};
+	if ($gateway) {
+	    push @iface_config, "address $gateway" if !defined($address->{$gateway});
+	    $address->{$gateway} = 1;
+	}
     }
 
     push @iface_config, "hwaddress $mac" if $mac;
diff --git a/PVE/Network/SDN/Zones/SimplePlugin.pm b/PVE/Network/SDN/Zones/SimplePlugin.pm
index 7006b13..a4299dd 100644
--- a/PVE/Network/SDN/Zones/SimplePlugin.pm
+++ b/PVE/Network/SDN/Zones/SimplePlugin.pm
@@ -35,10 +35,19 @@ sub generate_sdn_config {
     # vnet bridge
     my @iface_config = ();
 
-    my @subnets = PVE::Tools::split_list($vnet->{subnets}) if $vnet->{subnets};
-    foreach my $subnet (@subnets) {
-	next if !defined($subnet_cfg->{ids}->{$subnet});
-	push @iface_config, "address $subnet_cfg->{ids}->{$subnet}->{gateway}" if $subnet_cfg->{ids}->{$subnet}->{gateway};
+    my $address = {};
+    my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid);
+    foreach my $subnetid (sort keys %{$subnets}) {
+	my $subnet = $subnets->{$subnetid};
+	my $cidr = $subnetid =~ s/-/\//r; 
+	my $gateway = $subnet->{gateway};
+	if ($gateway) {
+	    push @iface_config, "address $gateway" if !defined($address->{$gateway});
+	    $address->{$gateway} = 1;
+	}
+	#add route for /32 pointtopoint
+	my ($ip, $mask) = split(/\//, $cidr);
+	push @iface_config, "up ip route add $cidr dev $vnetid" if $mask == 32;
     }
 
     push @iface_config, "hwaddress $mac" if $mac;
diff --git a/test/generateconfig.pl b/test/generateconfig.pl
index 36880ba..92108ec 100644
--- a/test/generateconfig.pl
+++ b/test/generateconfig.pl
@@ -3,17 +3,18 @@ use warnings;
 use File::Copy;
 use PVE::Cluster qw(cfs_read_file);
 
+use PVE::Network::SDN;
 use PVE::Network::SDN::Zones;
 use PVE::Network::SDN::Controllers;
 use Data::Dumper;
 
 my $network_config = PVE::Network::SDN::Zones::generate_etc_network_config();
+
 PVE::Network::SDN::Zones::write_etc_network_config($network_config);
 print "/etc/network/interfaces.d/sdn\n";
 print $network_config;
 print "\n";
 
-
 my $controller_config = PVE::Network::SDN::Controllers::generate_controller_config();
 if ($controller_config) {
     print Dumper($controller_config);