From: Alexandre Derumier <aderumier@odiso.com>
Date: Mon, 5 Oct 2020 15:09:08 +0000 (+0200)
Subject: subnets/ipam: allow same subnet on different zones
X-Git-Url: https://git.proxmox.com/?p=pve-network.git;a=commitdiff_plain;h=e8736dac7b67cb70274fc54fc70110bbea541889

subnets/ipam: allow same subnet on different zones

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 1e65ed4..a3bc10b 100644
--- a/PVE/API2/Network/SDN/Subnets.pm
+++ b/PVE/API2/Network/SDN/Subnets.pm
@@ -28,7 +28,6 @@ my $api_sdn_subnets_config = sub {
 
     my $scfg = dclone(PVE::Network::SDN::Subnets::sdn_subnets_config($cfg, $id));
     $scfg->{subnet} = $id;
-    $scfg->{cidr} = $id =~ s/-/\//r;
     $scfg->{digest} = $cfg->{digest};
 
     return $scfg;
@@ -168,7 +167,6 @@ __PACKAGE__->register_method ({
 
 	my $type = extract_param($param, 'type');
 	my $cidr = extract_param($param, 'subnet');
-	my $id = $cidr =~ s/\//-/r;
 
 	# create /etc/pve/sdn directory
 	PVE::Cluster::check_cfs_quorum();
@@ -183,7 +181,9 @@ __PACKAGE__->register_method ({
 		my $vnet = $param->{vnet};
 		my $zoneid = $vnet_cfg->{ids}->{$vnet}->{zone};
 		my $zone = $zone_cfg->{ids}->{$zoneid};      
-
+		my $id = $cidr =~ s/\//-/r;
+		$id = "$zoneid-$id";
+		
 		my $opts = PVE::Network::SDN::SubnetPlugin->check_config($id, $param, 1, 1);
 
 		my $scfg = undef;
@@ -192,7 +192,9 @@ __PACKAGE__->register_method ({
 		}
 
 		$cfg->{ids}->{$id} = $opts;
-		PVE::Network::SDN::SubnetPlugin->on_update_hook($zone, $id, $opts);
+
+		my $subnet = PVE::Network::SDN::Subnets::sdn_subnets_config($cfg, $id);
+		PVE::Network::SDN::SubnetPlugin->on_update_hook($zone, $id, $subnet);
 
 		PVE::Network::SDN::Subnets::write_config($cfg);
 
@@ -237,7 +239,8 @@ __PACKAGE__->register_method ({
 
 	    raise_param_exc({ ipam => "you can't change ipam"}) if $opts->{ipam} && $scfg->{ipam} && $opts->{ipam} ne $scfg->{ipam};
 
-	    PVE::Network::SDN::SubnetPlugin->on_update_hook($zone, $id, $opts, $scfg);
+	    my $subnet = PVE::Network::SDN::Subnets::sdn_subnets_config($cfg, $id);
+	    PVE::Network::SDN::SubnetPlugin->on_update_hook($zone, $id, $subnet);
 
 	    PVE::Network::SDN::Subnets::write_config($cfg);
 
diff --git a/PVE/API2/Network/SDN/Vnets.pm b/PVE/API2/Network/SDN/Vnets.pm
index 5fe524b..3a995a8 100644
--- a/PVE/API2/Network/SDN/Vnets.pm
+++ b/PVE/API2/Network/SDN/Vnets.pm
@@ -17,6 +17,7 @@ use PVE::API2::Network::SDN::Subnets;
 use Storable qw(dclone);
 use PVE::JSONSchema qw(get_standard_option);
 use PVE::RPCEnvironment;
+use PVE::Exception qw(raise raise_param_exc);
 
 use PVE::RESTHandler;
 
@@ -193,9 +194,7 @@ __PACKAGE__->register_method ({
 	    my $plugin = PVE::Network::SDN::Zones::Plugin->lookup($plugin_config->{type});
             $plugin->vnet_update_hook($cfg->{ids}->{$id});
 
-	    my $subnet_cfg = PVE::Network::SDN::Subnets::config();
-
-	    PVE::Network::SDN::VnetPlugin->on_update_hook($id, $cfg, $subnet_cfg);
+	    PVE::Network::SDN::VnetPlugin->on_update_hook($id, $cfg);
 
 	    PVE::Network::SDN::Vnets::write_config($cfg);
 
@@ -226,7 +225,12 @@ __PACKAGE__->register_method ({
 
 	    PVE::SectionConfig::assert_if_modified($cfg, $digest);
 
+
 	    my $opts = PVE::Network::SDN::VnetPlugin->check_config($id, $param, 0, 1);
+	    raise_param_exc({ zone => "missing zone"}) if !$opts->{zone};
+	    my $subnets = PVE::Network::SDN::Vnets::get_subnets($id);
+	    raise_param_exc({ zone => "can't change zone if subnets exists"}) if($subnets && $opts->{zone} ne $cfg->{ids}->{$id}->{zone});
+
 	    $cfg->{ids}->{$id} = $opts;
 
 	    my $zone_cfg = PVE::Network::SDN::Zones::config();
@@ -235,9 +239,7 @@ __PACKAGE__->register_method ({
 	    my $plugin = PVE::Network::SDN::Zones::Plugin->lookup($plugin_config->{type});
 	    $plugin->vnet_update_hook($cfg->{ids}->{$id});
 
-	    my $subnet_cfg = PVE::Network::SDN::Subnets::config();
-
-	    PVE::Network::SDN::VnetPlugin->on_update_hook($id, $cfg, $subnet_cfg);
+	    PVE::Network::SDN::VnetPlugin->on_update_hook($id, $cfg);
 
 	    PVE::Network::SDN::Vnets::write_config($cfg);
 
diff --git a/PVE/API2/Network/SDN/Zones.pm b/PVE/API2/Network/SDN/Zones.pm
index 54f087d..5ae577b 100644
--- a/PVE/API2/Network/SDN/Zones.pm
+++ b/PVE/API2/Network/SDN/Zones.pm
@@ -9,6 +9,7 @@ use PVE::Cluster qw(cfs_read_file cfs_write_file);
 use PVE::Network::SDN;
 use PVE::Network::SDN::Vnets;
 use PVE::Network::SDN::Zones;
+use PVE::Network::SDN::Subnets;
 use PVE::Network::SDN::Dns;
 use PVE::Network::SDN::Zones::Plugin;
 use PVE::Network::SDN::Zones::VlanPlugin;
@@ -263,6 +264,16 @@ __PACKAGE__->register_method ({
 	    my $plugin = PVE::Network::SDN::Zones::Plugin->lookup($scfg->{type});
 	    my $opts = $plugin->check_config($id, $param, 0, 1);
 
+	    if($opts->{ipam} ne $scfg->{ipam}) {
+
+		#don't allow ipam change if subnet are defined
+		my $subnets_cfg = PVE::Network::SDN::Subnets::config();
+		foreach my $subnetid (sort keys %{$subnets_cfg->{ids}}) {
+		    my $subnet = PVE::Network::SDN::Subnets::sdn_subnets_config($subnets_cfg, $subnetid);
+		    raise_param_exc({ ipam => "can't change ipam if subnet if already defined for this zone"}) if $subnet->{zone} eq $id;
+		}
+	    }
+
 	    foreach my $k (%$opts) {
 		$scfg->{$k} = $opts->{$k};
 	    }
diff --git a/PVE/Network/SDN/Dns/PowerdnsPlugin.pm b/PVE/Network/SDN/Dns/PowerdnsPlugin.pm
index 5b98e87..b00432e 100644
--- a/PVE/Network/SDN/Dns/PowerdnsPlugin.pm
+++ b/PVE/Network/SDN/Dns/PowerdnsPlugin.pm
@@ -186,11 +186,11 @@ sub verify_zone {
 }
 
 sub get_reversedns_zone {
-    my ($class, $plugin_config, $subnetid, $ip) = @_;
+    my ($class, $plugin_config, $subnetid, $subnet, $ip) = @_;
 
-    my ($network, $mask) = split(/-/, $subnetid);
+    my $cidr = $subnet->{cidr};
+    my $mask = $subnet->{mask};
 
-    my $cidr = "$ip/$mask";
     my $zone = "";
 
     if (Net::IP::ip_is_ipv4($ip)) {
diff --git a/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/PVE/Network/SDN/Ipams/NetboxPlugin.pm
index 01f82f2..d696b08 100644
--- a/PVE/Network/SDN/Ipams/NetboxPlugin.pm
+++ b/PVE/Network/SDN/Ipams/NetboxPlugin.pm
@@ -30,7 +30,7 @@ sub options {
 sub add_subnet {
     my ($class, $plugin_config, $subnetid, $subnet) = @_;
 
-    my $cidr = $subnetid =~ s/-/\//r;
+    my $cidr = $subnet->{cidr};
     my $gateway = $subnet->{gateway};
     my $url = $plugin_config->{url};
     my $token = $plugin_config->{token};
@@ -40,13 +40,11 @@ sub add_subnet {
 
     #create subnet
     if (!$internalid) {
-	my ($network, $mask) = split(/-/, $subnetid);
 
 	my $params = { prefix => $cidr };
 
 	eval {
 		my $result = PVE::Network::SDN::Ipams::Plugin::api_request("POST", "$url/ipam/prefixes/", $headers, $params);
-		$subnet->{ipamid} = $result->{id} if defined($result->{id});
 	};
 	if ($@) {
 	    die "error add subnet to ipam: $@";
@@ -58,7 +56,7 @@ sub add_subnet {
 sub del_subnet {
     my ($class, $plugin_config, $subnetid, $subnet) = @_;
 
-    my $cidr = $subnetid =~ s/-/\//r;
+    my $cidr = $subnet->{cidr};
     my $url = $plugin_config->{url};
     my $token = $plugin_config->{token};
     my $gateway = $subnet->{gateway};
@@ -66,9 +64,8 @@ sub del_subnet {
 
     my $internalid = get_prefix_id($url, $cidr, $headers);
     return if !$internalid;
-    #fixme: check that prefix is empty exluding gateway, before delete
 
-    PVE::Network::SDN::Ipams::NetboxPlugin::del_ip($class, $plugin_config, $subnetid, $gateway) if $gateway;
+    return; #fixme: check that prefix is empty exluding gateway, before delete
 
     eval {
 	PVE::Network::SDN::Ipams::Plugin::api_request("DELETE", "$url/ipam/prefixes/$internalid/", $headers);
@@ -80,9 +77,9 @@ sub del_subnet {
 }
 
 sub add_ip {
-    my ($class, $plugin_config, $subnetid, $ip, $is_gateway) = @_;
+    my ($class, $plugin_config, $subnetid, $subnet, $ip, $is_gateway) = @_;
 
-    my ($network, $mask) = split(/-/, $subnetid);
+    my $mask = $subnet->{mask};
     my $url = $plugin_config->{url};
     my $token = $plugin_config->{token};
     my $section = $plugin_config->{section};
@@ -102,7 +99,8 @@ sub add_ip {
 sub add_next_freeip {
     my ($class, $plugin_config, $subnetid, $subnet) = @_;
 
-    my $cidr = $subnetid =~ s/-/\//r;
+    my $cidr = $subnet->{cidr};
+
     my $url = $plugin_config->{url};
     my $token = $plugin_config->{token};
     my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Authorization' => "token $token"];
@@ -125,7 +123,7 @@ sub add_next_freeip {
 }
 
 sub del_ip {
-    my ($class, $plugin_config, $subnetid, $ip) = @_;
+    my ($class, $plugin_config, $subnetid, $subnet, $ip) = @_;
 
     return if !$ip;
 
diff --git a/PVE/Network/SDN/Ipams/PVEPlugin.pm b/PVE/Network/SDN/Ipams/PVEPlugin.pm
index 3b61695..0779b53 100644
--- a/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -7,6 +7,7 @@ use PVE::Cluster qw(cfs_read_file cfs_write_file cfs_register_file cfs_lock_file
 use PVE::Tools;
 use JSON;
 use NetAddr::IP;
+use Net::IP;
 use Digest::SHA;
 
 use base('PVE::Network::SDN::Ipams::Plugin');
@@ -33,15 +34,22 @@ sub options {
 sub add_subnet {
     my ($class, $plugin_config, $subnetid, $subnet) = @_;
 
-    my $cidr = $subnetid =~ s/-/\//r;
+    my $cidr = $subnet->{cidr};
+    my $zone = $subnet->{zone};
     my $gateway = $subnet->{gateway};
 
+
     cfs_lock_file($ipamdb_file, undef, sub {
-	my $config = read_db();
-	#create subnet
-	if (!defined($config->{subnets}->{$cidr})) {
-	    $config->{subnets}->{$cidr}->{ips} = {};
-	    write_db($config);
+	my $db = {};
+	$db = read_db();
+
+	$db->{zones}->{$zone} = {} if !$db->{zones}->{$zone};
+	my $zonedb = $db->{zones}->{$zone};
+
+	if(!$zonedb->{subnets}->{$cidr}) {
+	    #create subnet
+	    $zonedb->{subnets}->{$cidr}->{ips} = {};
+	    write_db($db);
 	}
     });
     die "$@" if $@;
@@ -50,14 +58,22 @@ sub add_subnet {
 sub del_subnet {
     my ($class, $plugin_config, $subnetid, $subnet) = @_;
 
-    my $cidr = $subnetid =~ s/-/\//r;
+    my $cidr = $subnet->{cidr};
+    my $zone = $subnet->{zone};
 
     cfs_lock_file($ipamdb_file, undef, sub {
 
 	my $db = read_db();
-	my $ips = $db->{subnets}->{$cidr}->{ips};
-	die "cannot delete subnet '$cidr', not empty\n" if keys %{$ips} > 0;
-	delete $db->{subnets}->{$cidr};
+
+	my $dbzone = $db->{zones}->{$zone};
+	die "zone '$zone' doesn't exist in IPAM DB\n" if !$dbzone;
+	my $dbsubnet = $dbzone->{subnets}->{$cidr};
+	die "subnet '$cidr' doesn't exist in IPAM DB\n" if !$dbsubnet;
+
+	die "cannot delete subnet '$cidr', not empty\n" if keys %{$dbsubnet->{ips}} > 0;
+
+	delete $dbzone->{subnets}->{$cidr};
+
 	write_db($db);
     });
     die "$@" if $@;
@@ -65,19 +81,24 @@ sub del_subnet {
 }
 
 sub add_ip {
-    my ($class, $plugin_config, $subnetid, $ip, $is_gateway) = @_;
+    my ($class, $plugin_config, $subnetid, $subnet, $ip, $is_gateway) = @_;
 
-    my $cidr = $subnetid =~ s/-/\//r;
+    my $cidr = $subnet->{cidr};
+    my $zone = $subnet->{zone};
 
     cfs_lock_file($ipamdb_file, undef, sub {
 
 	my $db = read_db();
-	my $s = $db->{subnets}->{$cidr};
 
-	die "IP '$ip' already exist\n" if defined($s->{ips}->{$ip});
+	my $dbzone = $db->{zones}->{$zone};
+	die "zone '$zone' doesn't exist in IPAM DB\n" if !$dbzone;
+	my $dbsubnet = $dbzone->{subnets}->{$cidr};
+	die "subnet '$cidr' doesn't exist in IPAM DB\n" if !$dbsubnet;
+
+	die "IP '$ip' already exist\n" if defined($dbsubnet->{ips}->{$ip});
+
+	$dbsubnet->{ips}->{$ip} = 1;
 
-	#verify that ip is valid for this subnet
-	$s->{ips}->{$ip} = 1;
 	write_db($db);
     });
     die "$@" if $@;
@@ -86,49 +107,64 @@ sub add_ip {
 sub add_next_freeip {
     my ($class, $plugin_config, $subnetid, $subnet) = @_;
 
-    my $cidr = $subnetid =~ s/-/\//r;
+    my $cidr = $subnet->{cidr};
+    my $network = $subnet->{network};
+    my $zone = $subnet->{zone};
+    my $mask = $subnet->{mask};
     my $freeip = undef;
 
     cfs_lock_file($ipamdb_file, undef, sub {
 
 	my $db = read_db();
-	my $s = $db->{subnets}->{$cidr};
-	my $iplist = new NetAddr::IP($cidr);
-	my $broadcast = $iplist->broadcast();
-
-	while (1) {
-	    $iplist++;
-	    last if $iplist eq $broadcast;
-	    my $ip = $iplist->addr();
-	    next if defined($s->{ips}->{$ip});
-	    $freeip = $ip;
-	    last;
+	my $dbzone = $db->{zones}->{$zone};
+	die "zone '$zone' doesn't exist in IPAM DB\n" if !$dbzone;
+	my $dbsubnet = $dbzone->{subnets}->{$cidr};
+	die "subnet '$cidr' doesn't exist in IPAM DB" if !$dbsubnet;
+
+	if (Net::IP::ip_is_ipv4($network) && $mask == 32) {
+	    die "cannot find free IP in subnet '$cidr'\n" if defined($dbsubnet->{ips}->{$network});
+	    $freeip = $network;
+	} else {
+	    my $iplist = new NetAddr::IP($cidr);
+	    my $broadcast = $iplist->broadcast();
+
+	    while(1) {
+		$iplist++;
+		last if $iplist eq $broadcast;
+		my $ip = $iplist->addr();
+		next if defined($dbsubnet->{ips}->{$ip});
+		$freeip = $ip;
+		last;
+	    }
 	}
 
 	die "can't find free ip in subnet '$cidr'\n" if !$freeip;
 
-	$s->{ips}->{$freeip} = 1;
+	$dbsubnet->{ips}->{$freeip} = 1;
 	write_db($db);
     });
     die "$@" if $@;
 
-    my ($network, $mask) = split(/-/, $subnetid);
     return "$freeip/$mask";
 }
 
 sub del_ip {
-    my ($class, $plugin_config, $subnetid, $ip) = @_;
+    my ($class, $plugin_config, $subnetid, $subnet, $ip) = @_;
 
-    my $cidr = $subnetid =~ s/-/\//r;
+    my $cidr = $subnet->{cidr};
+    my $zone = $subnet->{zone};
 
     cfs_lock_file($ipamdb_file, undef, sub {
 
 	my $db = read_db();
-	my $s = $db->{subnets}->{$cidr};
-	return if !$ip;
+	die "zone $zone don't exist in ipam db" if !$db->{zones}->{$zone};
+	my $dbzone = $db->{zones}->{$zone};
+	die "subnet $cidr don't exist in ipam db" if !$dbzone->{subnets}->{$cidr};
+	my $dbsubnet = $dbzone->{subnets}->{$cidr};
+
+	die "IP '$ip' does not exist in IPAM DB\n" if !defined($dbsubnet->{ips}->{$ip});
+	delete $dbsubnet->{ips}->{$ip};
 
-	die "IP '$ip' does not exist in IPAM DB\n" if !defined($s->{ips}->{$ip});
-	delete $s->{ips}->{$ip};
 	write_db($db);
     });
     die "$@" if $@;
diff --git a/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm b/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
index 4a9e6c4..f89ef29 100644
--- a/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
+++ b/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
@@ -40,7 +40,10 @@ sub options {
 sub add_subnet {
     my ($class, $plugin_config, $subnetid, $subnet) = @_;
 
-    my $cidr = $subnetid =~ s/-/\//r;
+    my $cidr = $subnet->{cidr};
+    my $network = $subnet->{network};
+    my $mask = $subnet->{mask};
+
     my $gateway = $subnet->{gateway};
     my $url = $plugin_config->{url};
     my $token = $plugin_config->{token};
@@ -52,7 +55,6 @@ sub add_subnet {
 
     #create subnet
     if (!$internalid) {
-	my ($network, $mask) = split(/-/, $subnetid);
 
 	my $params = { subnet => $network,
 		   mask => $mask,
@@ -72,7 +74,7 @@ sub add_subnet {
 sub del_subnet {
     my ($class, $plugin_config, $subnetid, $subnet) = @_;
 
-    my $cidr = $subnetid =~ s/-/\//r;
+    my $cidr = $subnet->{cidr};
     my $url = $plugin_config->{url};
     my $token = $plugin_config->{token};
     my $section = $plugin_config->{section};
@@ -81,7 +83,7 @@ sub del_subnet {
     my $internalid = get_internalid($url, $cidr, $headers);
     return if !$internalid;
 
-    #fixme: check that prefix is empty exluding gateway, before delete
+    return; #fixme: check that prefix is empty exluding gateway, before delete
 
     eval {
 	PVE::Network::SDN::Ipams::Plugin::api_request("DELETE", "$url/subnets/$internalid", $headers);
@@ -93,9 +95,9 @@ sub del_subnet {
 }
 
 sub add_ip {
-    my ($class, $plugin_config, $subnetid, $ip, $is_gateway) = @_;
+    my ($class, $plugin_config, $subnetid, $subnet, $ip, $is_gateway) = @_;
 
-    my $cidr = $subnetid =~ s/-/\//r;
+    my $cidr = $subnet->{cidr};
     my $url = $plugin_config->{url};
     my $token = $plugin_config->{token};
     my $section = $plugin_config->{section};
@@ -120,7 +122,8 @@ sub add_ip {
 sub add_next_freeip {
     my ($class, $plugin_config, $subnetid, $subnet, $internalid, $hostname) = @_;
 
-    my $cidr = $subnetid =~ s/-/\//r;
+    my $cidr = $subnet->{cidr};  
+    my $mask = $subnet->{mask};  
     my $url = $plugin_config->{url};
     my $token = $plugin_config->{token};
     my $section = $plugin_config->{section};
@@ -140,12 +143,11 @@ sub add_next_freeip {
         die "can't find free ip in subnet $cidr: $@";
     }
 
-    my ($network, $mask) = split(/-/, $subnetid);
     return "$ip/$mask";
 }
 
 sub del_ip {
-    my ($class, $plugin_config, $subnetid, $ip) = @_;
+    my ($class, $plugin_config, $subnetid, $subnet, $ip) = @_;
 
     return if !$ip;
 
diff --git a/PVE/Network/SDN/Ipams/Plugin.pm b/PVE/Network/SDN/Ipams/Plugin.pm
index 9330ba9..4c68287 100644
--- a/PVE/Network/SDN/Ipams/Plugin.pm
+++ b/PVE/Network/SDN/Ipams/Plugin.pm
@@ -75,16 +75,16 @@ sub del_subnet {
 }
 
 sub add_ip {
-    my ($class, $plugin_config, $subnetid, $subnet, $internalid, $ip, $hostname, $is_gateway) = @_;
+    my ($class, $plugin_config, $subnetid, $subnet, $ip, $is_gateway) = @_;
 
 }
 
 sub add_next_freeip {
-    my ($class, $plugin_config) = @_;
+    my ($class, $plugin_config, $subnetid, $subnet) = @_;
 }
 
 sub del_ip {
-    my ($class, $plugin_config, $subnetid, $ip) = @_;
+    my ($class, $plugin_config, $subnetid, $subnet, $ip) = @_;
 }
 
 sub on_update_hook {
diff --git a/PVE/Network/SDN/SubnetPlugin.pm b/PVE/Network/SDN/SubnetPlugin.pm
index a5d03f6..1444262 100644
--- a/PVE/Network/SDN/SubnetPlugin.pm
+++ b/PVE/Network/SDN/SubnetPlugin.pm
@@ -10,6 +10,7 @@ use PVE::Exception qw(raise raise_param_exc);
 use Net::Subnet qw(subnet_matcher);
 use PVE::Network::SDN::Vnets;
 use PVE::Network::SDN::Ipams;
+use Net::IP;
 
 PVE::Cluster::cfs_register_file('sdn/subnets.cfg',
                                  sub { __PACKAGE__->parse_config(@_); },
@@ -25,7 +26,13 @@ PVE::JSONSchema::register_format('pve-sdn-subnet-id', \&parse_sdn_subnet_id);
 sub parse_sdn_subnet_id {
     my ($id, $noerr) = @_;
 
-    my $cidr = $id =~ s/-/\//r;
+    my $cidr = "";
+    if($id =~ /\//) {
+	$cidr = $id;
+    } else {
+	my ($zone, $ip, $mask) = split(/-/, $id);
+	$cidr = "$ip/$mask";
+    }
 
     if (!(PVE::JSONSchema::pve_verify_cidrv4($cidr, 1) ||
           PVE::JSONSchema::pve_verify_cidrv6($cidr, 1)))
@@ -91,7 +98,9 @@ sub options {
 sub on_update_hook {
     my ($class, $zone, $subnetid, $subnet, $old_subnet) = @_;
 
-    my $cidr = $subnetid =~ s/-/\//r;
+    my $cidr = $subnet->{cidr};
+    my $mask = $subnet->{mask};
+
     my $subnet_matcher = subnet_matcher($cidr);
 
     my $vnetid = $subnet->{vnet};
@@ -109,9 +118,11 @@ sub on_update_hook {
 	raise_param_exc({ vnet => "you can't add a subnet on a vlanaware vnet"}) if $vnet->{vlanaware};
     }
 
-    my ($ip, $mask) = split(/\//, $cidr);
+    my $pointopoint = 1 if Net::IP::ip_is_ipv4($gateway) && $mask == 32;
+
     #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({ gateway => "$gateway is not in subnet $cidr"}) if $gateway && !$subnet_matcher->($gateway) && !$pointopoint;
+
 
     if ($ipam) {
 	my $ipam_cfg = PVE::Network::SDN::Ipams::config();
@@ -119,7 +130,10 @@ sub on_update_hook {
 	my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
 	$plugin->add_subnet($plugin_config, $subnetid, $subnet);
 
-	#delete on removal
+	#don't register gateway for pointopoint
+	return if $pointopoint;
+
+	#delete gateway on removal
 	if (!defined($gateway) && $old_gateway) {
 	    eval {
 		PVE::Network::SDN::Subnets::del_ip($zone, $subnetid, $old_subnet, $old_gateway);
@@ -130,7 +144,7 @@ sub on_update_hook {
 	    PVE::Network::SDN::Subnets::add_ip($zone, $subnetid, $subnet, $gateway);
 	}
 
-	#delete old ip after update
+	#delete old gateway after update
 	if($gateway && $old_gateway && $gateway ne $old_gateway) {
 	    eval {
 		PVE::Network::SDN::Subnets::del_ip($zone, $subnetid, $old_subnet, $old_gateway);
diff --git a/PVE/Network/SDN/Subnets.pm b/PVE/Network/SDN/Subnets.pm
index 50130d5..bd1eb36 100644
--- a/PVE/Network/SDN/Subnets.pm
+++ b/PVE/Network/SDN/Subnets.pm
@@ -21,6 +21,14 @@ sub sdn_subnets_config {
     my $scfg = $cfg->{ids}->{$id};
     die "sdn subnet '$id' does not exist\n" if (!$noerr && !$scfg);
 
+    if($scfg) {
+	my ($zone, $network, $mask) = split(/-/, $id);
+	$scfg->{cidr} = "$network/$mask";
+	$scfg->{zone} = $zone;
+	$scfg->{network} = $network;
+	$scfg->{mask} = $mask;
+    }
+
     return $scfg;
 }
 
@@ -64,13 +72,15 @@ sub get_subnet {
 }
 
 sub find_ip_subnet {
-    my ($ip, $subnets) = @_;
+    my ($ip, $mask, $subnets) = @_;
 
     my $subnet = undef;
     my $subnetid = undef;
 
     foreach my $id (sort keys %{$subnets}) {
-	my $cidr = $id =~ s/-/\//r;
+
+	next if $mask ne $subnets->{$id}->{mask};
+	my $cidr = $subnets->{$id}->{cidr};
 	my $subnet_matcher = subnet_matcher($cidr);
 	next if !$subnet_matcher->($ip);
 	$subnet = $subnets->{$id};
@@ -94,14 +104,14 @@ my $verify_dns_zone = sub {
 };
 
 my $get_reversedns_zone = sub {
-    my ($subnetid, $dns, $ip) = @_;
+    my ($subnetid, $subnet, $dns, $ip) = @_;
 
     return if !$subnetid || !$dns || !$ip;
 
     my $dns_cfg = PVE::Network::SDN::Dns::config();
     my $plugin_config = $dns_cfg->{ids}->{$dns};
     my $plugin = PVE::Network::SDN::Dns::Plugin->lookup($plugin_config->{type});
-    $plugin->get_reversedns_zone($plugin_config, $subnetid, $ip);
+    $plugin->get_reversedns_zone($plugin_config, $subnetid, $subnet, $ip);
 };
 
 my $add_dns_record = sub {
@@ -181,7 +191,7 @@ sub next_free_ip {
     }
 
     eval {
-	my $reversednszone = &$get_reversedns_zone($subnetid, $reversedns, $ip);
+	my $reversednszone = &$get_reversedns_zone($subnetid, $subnet, $reversedns, $ip);
 
 	#add dns
 	&$add_dns_record($dnszone, $dns, $hostname, $dnszoneprefix, $ip);
@@ -208,7 +218,7 @@ sub add_ip {
     my $dns = $zone->{dns};
     my $dnszone = $zone->{dnszone};
     my $reversedns = $zone->{reversedns};
-    my $reversednszone = &$get_reversedns_zone($subnetid, $reversedns, $ip);
+    my $reversednszone = &$get_reversedns_zone($subnetid, $subnet, $reversedns, $ip);
     my $dnszoneprefix = $subnet->{dnszoneprefix};
 
     #verify dns zones before ipam
@@ -220,7 +230,7 @@ sub add_ip {
 	my $plugin_config = $ipam_cfg->{ids}->{$ipamid};
 	my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
 	eval {
-	    $plugin->add_ip($plugin_config, $subnetid, $ip);
+	    $plugin->add_ip($plugin_config, $subnetid, $subnet, $ip);
 	};
 	die $@ if $@;
     }
@@ -250,7 +260,7 @@ sub del_ip {
     my $dns = $zone->{dns};
     my $dnszone = $zone->{dnszone};
     my $reversedns = $zone->{reversedns};
-    my $reversednszone = &$get_reversedns_zone($subnetid, $reversedns, $ip);
+    my $reversednszone = &$get_reversedns_zone($subnetid, $subnet, $reversedns, $ip);
     my $dnszoneprefix = $subnet->{dnszoneprefix};
 
     &$verify_dns_zone($dnszone, $dns);
@@ -260,7 +270,7 @@ sub del_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->del_ip($plugin_config, $subnetid, $ip);
+	$plugin->del_ip($plugin_config, $subnetid, $subnet, $ip);
     }
 
     eval {
diff --git a/PVE/Network/SDN/VnetPlugin.pm b/PVE/Network/SDN/VnetPlugin.pm
index 8481f0d..518d2dd 100644
--- a/PVE/Network/SDN/VnetPlugin.pm
+++ b/PVE/Network/SDN/VnetPlugin.pm
@@ -91,29 +91,28 @@ sub on_delete_hook {
 
     #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;
+    raise_param_exc({ vnet => "Can't delete vnet if subnets exists"}) if $subnets;
 }
 
 sub on_update_hook {
-    my ($class, $vnetid, $vnet_cfg, $subnet_cfg) = @_;
+    my ($class, $vnetid, $vnet_cfg) = @_;
+
+    my $vnet = $vnet_cfg->{ids}->{$vnetid};
+    my $tag = $vnet->{tag};
+    my $vlanaware = $vnet->{vlanaware};
+
+    #don't allow vlanaware change if subnets are defined
+    if($vnet->{vlanaware}) {
+	my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid);
+	raise_param_exc({ vlanaware => "vlanaware vnet is not compatible with subnets"}) if $subnets;
+    }
 
-    #fixme : don't allow change zone if subnets are defined
-    #fixme : don't vlanaware change if subnets are defined
-#    my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid);
-   
     # verify that tag is not already defined in another vnet
-    if (defined($vnet_cfg->{ids}->{$vnetid}->{tag})) {
-	my $tag = $vnet_cfg->{ids}->{$vnetid}->{tag};
+    if (defined($tag)) {
 	foreach my $id (keys %{$vnet_cfg->{ids}}) {
 	    next if $id eq $vnetid;
-	    my $vnet = $vnet_cfg->{ids}->{$id};
-	    if ($vnet->{type} eq 'vnet' && defined($vnet->{tag})) {
-		raise_param_exc({ tag => "tag $tag already exist in vnet $id"}) if $tag eq $vnet->{tag};
-	    }
+	    my $othervnettag = $vnet_cfg->{ids}->{$id}->{tag};
+	    raise_param_exc({ tag => "tag $tag already exist in vnet $id"}) if $othervnettag && $tag eq $othervnettag;
 	}
     }
 }
diff --git a/PVE/Network/SDN/Vnets.pm b/PVE/Network/SDN/Vnets.pm
index d08db51..6d11003 100644
--- a/PVE/Network/SDN/Vnets.pm
+++ b/PVE/Network/SDN/Vnets.pm
@@ -66,10 +66,10 @@ sub get_vnet {
 sub get_subnets {
     my ($vnetid) = @_;
 
-    my $subnets = {};
+    my $subnets = undef;
     my $subnets_cfg = PVE::Network::SDN::Subnets::config();
     foreach my $subnetid (sort keys %{$subnets_cfg->{ids}}) {
-	my $subnet = $subnets_cfg->{ids}->{$subnetid};
+	my $subnet = PVE::Network::SDN::Subnets::sdn_subnets_config($subnets_cfg, $subnetid);
 	next if !$subnet->{vnet} || $subnet->{vnet} ne $vnetid;
 	$subnets->{$subnetid} = $subnet;
     }
@@ -77,7 +77,7 @@ sub get_subnets {
 
 }
 
-sub get_next_free_ip {
+sub get_next_free_cidr {
     my ($vnetid, $hostname, $ipversion) = @_;
 
     my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid);
@@ -91,7 +91,7 @@ sub get_next_free_ip {
 
     foreach my $subnetid (sort keys %{$subnets}) {
         my $subnet = $subnets->{$subnetid};
-	my ($network, $mask) = split(/-/, $subnetid);
+	my $network = $subnet->{network};
 
 	next if $ipversion != Net::IP::ip_get_version($network);
 	$subnetcount++;
@@ -108,7 +108,7 @@ sub get_next_free_ip {
     return $ip;
 }
 
-sub add_ip {
+sub add_cidr {
     my ($vnetid, $cidr, $hostname) = @_;
 
     my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid, 1);
@@ -117,12 +117,13 @@ sub add_ip {
     my $zone = PVE::Network::SDN::Zones::get_zone($zoneid);
 
     my ($ip, $mask) = split(/\//, $cidr);
-    my ($subnetid, $subnet) = PVE::Network::SDN::Subnets::find_ip_subnet($ip, $subnets);
+    die "ip address is not in cidr format" if !$mask;
+    my ($subnetid, $subnet) = PVE::Network::SDN::Subnets::find_ip_subnet($ip, $mask, $subnets);
 
     PVE::Network::SDN::Subnets::add_ip($zone, $subnetid, $subnet, $ip, $hostname);
 }
 
-sub del_ip {
+sub del_cidr {
     my ($vnetid, $cidr, $hostname) = @_;
 
     my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid, 1);
@@ -131,7 +132,8 @@ sub del_ip {
     my $zone = PVE::Network::SDN::Zones::get_zone($zoneid);
 
     my ($ip, $mask) = split(/\//, $cidr);
-    my ($subnetid, $subnet) = PVE::Network::SDN::Subnets::find_ip_subnet($ip, $subnets);
+    die "ip address is not in cidr format" if !$mask;
+    my ($subnetid, $subnet) = PVE::Network::SDN::Subnets::find_ip_subnet($ip, $mask, $subnets);
 
     PVE::Network::SDN::Subnets::del_ip($zone, $subnetid, $subnet, $ip, $hostname);
 }
diff --git a/PVE/Network/SDN/Zones/EvpnPlugin.pm b/PVE/Network/SDN/Zones/EvpnPlugin.pm
index 723b3cc..62ab817 100644
--- a/PVE/Network/SDN/Zones/EvpnPlugin.pm
+++ b/PVE/Network/SDN/Zones/EvpnPlugin.pm
@@ -86,7 +86,8 @@ sub generate_sdn_config {
     my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid, 1);
     foreach my $subnetid (sort keys %{$subnets}) {
 	my $subnet = $subnets->{$subnetid};
-	my $cidr = $subnetid =~ s/-/\//r;
+	my $cidr = $subnet->{cidr};
+
 	my $gateway = $subnet->{gateway};
 	if ($gateway) {
 	    push @iface_config, "address $gateway" if !defined($address->{$gateway});
diff --git a/PVE/Network/SDN/Zones/SimplePlugin.pm b/PVE/Network/SDN/Zones/SimplePlugin.pm
index fe0f20f..5294485 100644
--- a/PVE/Network/SDN/Zones/SimplePlugin.pm
+++ b/PVE/Network/SDN/Zones/SimplePlugin.pm
@@ -60,14 +60,15 @@ sub generate_sdn_config {
     my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid, 1);
     foreach my $subnetid (sort keys %{$subnets}) {
 	my $subnet = $subnets->{$subnetid};
-	my $cidr = $subnetid =~ s/-/\//r; 
+	my $cidr = $subnet->{cidr};
+	my $mask = $subnet->{mask};
+
 	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;
 	if ($subnet->{snat}) {
 	    #find outgoing interface