[PATCH pve-network v4 07/13] sdn: ipam: look a MAC up within the vnet's subnets

[Hannes Laimer <h.laimer@proxmox.com>]

A MAC only has to be unique within its vnet, so two vnets can carry
the same one. The IPAM lookup by MAC searched every subnet of the
zone, and NetBox searched its whole inventory. So a lookup for one
vnet could return the MAC's address on another, and the guest got no
address on its own vnet.

The built-in IPAM and NetBox are now searched only within the subnets
of the vnet the lookup is for. The phpIPAM plugin does not implement
the lookup. The MAC cache is still read first and keeps one address
per MAC across all vnets.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/PVE/Network/SDN/Ipams.pm               |  4 +--
 src/PVE/Network/SDN/Ipams/NetboxPlugin.pm  | 34 +++++++++++++---------
 src/PVE/Network/SDN/Ipams/PVEPlugin.pm     |  7 ++---
 src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm |  2 +-
 src/PVE/Network/SDN/Ipams/Plugin.pm        |  3 +-
 src/PVE/Network/SDN/Vnets.pm               |  3 +-
 src/test/run_test_vnets_blackbox.pl        | 25 ++++++++++++++++
 7 files changed, 55 insertions(+), 23 deletions(-)

diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 1476c4236be0..9a31eaf39f58 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -169,7 +169,7 @@ sub complete_sdn_vnet {
 }
 
 sub get_ips_from_mac {
-    my ($mac, $zoneid, $zone) = @_;
+    my ($mac, $zoneid, $zone, $subnets) = @_;
 
     PVE::Cluster::cfs_update(); # the lock refreshes, so the snapshot must too
     my $version = PVE::Cluster::cfs_file_version($macdb_filename);
@@ -181,7 +181,7 @@ sub get_ips_from_mac {
 
     my $plugin_config = get_plugin_config($zone);
     my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
-    my ($ip4, $ip6) = $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid);
+    my ($ip4, $ip6) = $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid, $subnets);
 
     # an empty answer is not cached, since the record may simply not exist yet
     return if !defined($ip4) && !defined($ip6);
diff --git a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
index e118d03e1eb0..9e1c9537e6ec 100644
--- a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
@@ -379,26 +379,32 @@ sub del_ip {
 }
 
 sub get_ips_from_mac {
-    my ($class, $plugin_config, $mac, $zoneid, $noerr) = @_;
+    my ($class, $plugin_config, $mac, $zoneid, $subnets, $noerr) = @_;
 
     my $ip4 = undef;
     my $ip6 = undef;
 
-    my $data = eval {
-        netbox_api_request($plugin_config, "GET", "/ipam/ip-addresses/?description__ic=$mac");
-    };
-    if ($@) {
-        return if $noerr;
-        die "could not query ip address entry for mac $mac: $@";
-    }
-
-    for my $ip (@{ $data->{results} }) {
-        if ($ip->{family}->{value} == 4 && !$ip4) {
-            ($ip4, undef) = split(/\//, $ip->{address});
+    for my $cidr (sort map { $_->{cidr} } values %{ $subnets // {} }) {
+        my $data = eval {
+            netbox_api_request(
+                $plugin_config,
+                "GET",
+                "/ipam/ip-addresses/?parent=$cidr&description__ic=$mac",
+            );
+        };
+        if ($@) {
+            return if $noerr;
+            die "could not query IP address entry for MAC $mac in $cidr: $@";
         }
 
-        if ($ip->{family}->{value} == 6 && !$ip6) {
-            ($ip6, undef) = split(/\//, $ip->{address});
+        for my $ip (@{ $data->{results} }) {
+            if ($ip->{family}->{value} == 4 && !$ip4) {
+                ($ip4, undef) = split(/\//, $ip->{address});
+            }
+
+            if ($ip->{family}->{value} == 6 && !$ip6) {
+                ($ip6, undef) = split(/\//, $ip->{address});
+            }
         }
     }
 
diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index 77a7b2a7c144..43d0a3daaf9a 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -279,7 +279,7 @@ sub del_ip {
 }
 
 sub get_ips_from_mac {
-    my ($class, $plugin_config, $mac, $zoneid) = @_;
+    my ($class, $plugin_config, $mac, $zoneid, $subnets) = @_;
 
     #just in case, as this should already be cached in local macs.db
 
@@ -289,13 +289,12 @@ sub get_ips_from_mac {
     my $db = read_db();
     die "zone $zoneid don't exist in ipam db" if !$db->{zones}->{$zoneid};
     my $dbzone = $db->{zones}->{$zoneid};
-    my $subnets = $dbzone->{subnets};
 
-    for my $subnet (keys %$subnets) {
+    for my $subnet (sort map { $_->{cidr} } values %{ $subnets // {} }) {
         # subnet keys are CIDRs, Net::IP only recognizes bare IPs
         my ($network) = split(m|/|, $subnet);
         next if Net::IP::ip_is_ipv4($network) ? $ip4 : $ip6;
-        my $ips = $subnets->{$subnet}->{ips};
+        my $ips = $dbzone->{subnets}->{$subnet}->{ips} // next;
         for my $ip (keys %$ips) {
             my $ipobject = $ips->{$ip};
             if ($ipobject->{mac} && $ipobject->{mac} eq $mac) {
diff --git a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
index 63256119e0cd..12d470e802c4 100644
--- a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
@@ -262,7 +262,7 @@ sub del_ip {
 }
 
 sub get_ips_from_mac {
-    my ($class, $plugin_config, $mac, $zoneid) = @_;
+    my ($class, $plugin_config, $mac, $zoneid, $subnets) = @_;
 
     my $url = $plugin_config->{url};
     my $token = $plugin_config->{token};
diff --git a/src/PVE/Network/SDN/Ipams/Plugin.pm b/src/PVE/Network/SDN/Ipams/Plugin.pm
index 26d093139703..14f8fe9e67fe 100644
--- a/src/PVE/Network/SDN/Ipams/Plugin.pm
+++ b/src/PVE/Network/SDN/Ipams/Plugin.pm
@@ -138,8 +138,9 @@ sub del_ip {
     die "please implement inside plugin";
 }
 
+# the addresses of a MAC within the given subnets of the zone, one per family
 sub get_ips_from_mac {
-    my ($class, $plugin_config, $mac, $zone) = @_;
+    my ($class, $plugin_config, $mac, $zoneid, $subnets) = @_;
 
     die "please implement inside plugin";
 }
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index 56516ed9de83..395aa32b6a84 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -204,7 +204,8 @@ sub get_ips_from_mac {
 
     return if !$zone->{ipam} || !$zone->{dhcp};
 
-    return PVE::Network::SDN::Ipams::get_ips_from_mac($mac, $zoneid, $zone);
+    my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid);
+    return PVE::Network::SDN::Ipams::get_ips_from_mac($mac, $zoneid, $zone, $subnets);
 }
 
 sub del_ips_from_mac {
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 04776ea2f8af..bc418f691fa6 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -1368,4 +1368,29 @@ sub test_mapping_edit_target {
 
 run_test(\&test_mapping_edit_target);
 
+sub test_ipam_per_vnet {
+    my $test_name = (split(/::/, (caller(0))[3]))[-1];
+    my $zoneid = "TESTZONE";
+    my $mac = "da:65:8f:18:9b:6f";
+
+    # the same MAC on two vnets of one zone has an address on each, and a
+    # lookup for a vnet answers with that vnet's address only
+    create_zone({ type => "simple", dhcp => "dnsmasq", ipam => "pve", zone => $zoneid });
+    for my $n (1, 2) {
+        create_dhcp_vnet($zoneid, "vnet$n", "10.$n.0.0/24");
+        create_ip({ zone => $zoneid, vnet => "vnet$n", mac => $mac, ip => "10.$n.0.50" });
+    }
+
+    for my $n (1, 2) {
+        $test_state->{macdb} = {};
+        is_deeply(
+            [PVE::Network::SDN::Vnets::get_ips_from_mac("vnet$n", $mac)],
+            ["10.$n.0.50", undef],
+            "$test_name: vnet$n answers with its own address",
+        );
+    }
+}
+
+run_test(\&test_ipam_per_vnet);
+
 done_testing();
-- 
2.47.3