public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Hannes Laimer <h.laimer@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH pve-network v2 08/11] sdn: ipam: keep the MAC cache per vnet
Date: Tue,  3 Nov 2026 16:15:40 +0100	[thread overview]
Message-ID: <20261103151543.11917-9-h.laimer@proxmox.com> (raw)
In-Reply-To: <20261103151543.11917-1-h.laimer@proxmox.com>

The same MAC can sit on several vnets, but the cache of its addresses
was keyed by the MAC alone. A mapping created or edited on one vnet
changed the cached answer on every other vnet as well. A guest with
that MAC on another vnet then got a reservation for an address
outside its own vnet.

The cache now holds the addresses per vnet, and a zone's dnsmasq
reservations hold a line for each vnet a MAC sits on, each written
from the cache of that vnet. A MAC matches in any spelling.

The map under the MAC alone stays, since nodes not yet upgraded read
and write only it. Every write lands there as well, one address per
MAC as such a node expects, and its entries answer for a vnet only
with addresses in its subnets, so a guest from before keeps its
reservation across the upgrade. A release or an edit on such a node
shows only in that map, so a vnet's address holds only while the map
still names it, otherwise the IPAM is asked again. A release checks
that the record still belongs to its MAC, leaves it and its DNS
records alone otherwise, and fails when it cannot check. The API
refuses to delete such a mapping.

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/PVE/API2/Network/SDN/Ips.pm            |  10 +-
 src/PVE/Network/SDN/Dhcp.pm                |   2 +-
 src/PVE/Network/SDN/Dhcp/Dnsmasq.pm        | 143 +++++-----
 src/PVE/Network/SDN/Dhcp/Plugin.pm         |   7 +-
 src/PVE/Network/SDN/Ipams.pm               | 188 ++++++++----
 src/PVE/Network/SDN/Ipams/NetboxPlugin.pm  |  15 +-
 src/PVE/Network/SDN/Ipams/PVEPlugin.pm     |  13 +-
 src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm |  20 +-
 src/PVE/Network/SDN/Ipams/Plugin.pm        |   4 +-
 src/PVE/Network/SDN/Subnets.pm             |  14 +-
 src/PVE/Network/SDN/Vnets.pm               |  10 +-
 src/test/run_test_ipams.pl                 |  49 ++++
 src/test/run_test_vnets_blackbox.pl        | 316 ++++++++++++++++++---
 13 files changed, 613 insertions(+), 178 deletions(-)

diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
index 7653a126eb64..72871856362e 100644
--- a/src/PVE/API2/Network/SDN/Ips.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -74,11 +74,14 @@ __PACKAGE__->register_method({
         my $mac = extract_param($param, 'mac');
         my $ip = extract_param($param, 'ip');
 
-        eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
+        my $released = eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
         die "$@\n" if $@;
 
         PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
 
+        # the cache entry is cleared, the IPAM record belongs to another MAC
+        die "IP '$ip' is not mapped to $mac\n" if defined($released) && !$released;
+
         return undef;
     },
 });
@@ -183,12 +186,13 @@ __PACKAGE__->register_method({
         my ($old_ip4, $old_ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnet, $mac);
         my $old_ip = (Net::IP::ip_get_version($ip) == 4) ? $old_ip4 : $old_ip6;
 
-        PVE::Network::SDN::Vnets::del_ip($vnet, $old_ip, '', $mac);
+        # a MAC without an address in that family has nothing to release
+        PVE::Network::SDN::Vnets::del_ip($vnet, $old_ip, '', $mac) if defined($old_ip);
 
         eval { PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, $vmid); };
         my $error = $@;
 
-        if ($error) {
+        if ($error && defined($old_ip)) {
             PVE::Network::SDN::Vnets::add_ip($vnet, $old_ip, '', $mac, $vmid);
         }
 
diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
index 964e3fd2f1d3..2e9dd31dcb6f 100644
--- a/src/PVE/Network/SDN/Dhcp.pm
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -41,7 +41,7 @@ sub update_mapping {
         return if defined($zone->{nodes}) && !$zone->{nodes}->{ PVE::INotify::nodename() };
 
         PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp})
-            ->update_ip_mapping($zoneid, $mac);
+            ->update_ip_mapping($zoneid, $vnetid, $mac);
     };
     # the records are written by then and stay
     die "IPAM is updated for $mac, but DHCP is not: $@" if $@;
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index 9cd16020887e..f3b8dab3dcf4 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -13,7 +13,6 @@ use Net::DBus;
 
 use PVE::Cluster;
 use PVE::RESTEnvironment qw(log_warn);
-use PVE::Network::SDN::Ipams;
 
 my $DNSMASQ_CONFIG_ROOT = '/etc/dnsmasq.d';
 my $DNSMASQ_DEFAULT_ROOT = '/etc/default';
@@ -54,98 +53,104 @@ sub update_lease {
 }
 
 sub update_ip_mapping {
-    my ($class, $dhcpid, $mac) = @_;
+    my ($class, $dhcpid, $vnetid, $mac) = @_;
     $mac = lc($mac);
 
     my $ethers_file = ethers_file($dhcpid);
     my $ethers_tmp_file = "$ethers_file.tmp";
 
-    my ($ip4, $ip6);
-    my $reservation = undef;
+    my $ip4;
     my $changed = undef;
 
-    my $rewriteFn = sub {
-        # the lines are decided from the cache, which this process may hold an
-        # old read of, and the local lock does not refresh it
+    my $rewrite_fn = sub {
+        # the lines are decided from the cache and the applied config, which
+        # the local lock does not refresh
         PVE::Cluster::cfs_update();
 
-        # read under the ethers lock. A snapshot from before it would sweep a
-        # line another push wrote meanwhile, and an older push would put its
-        # addresses back
-
-        my $macs = PVE::Network::SDN::Ipams::read_macdb()->{macs};
-
-        my $entry = $macs->{$mac};
-        ($ip4, $ip6) = ($entry->{ip4}, $entry->{ip6}) if $entry;
-        if ($ip4 || $ip6) {
-            $reservation = $mac;
-            $reservation .= ",$ip4" if $ip4;
-            $reservation .= ",[$ip6]" if $ip6;
+        # the file holds a line per vnet a MAC sits on, and a line belongs to
+        # the vnet of the applied subnet that holds its address
+        my $vnets = PVE::Network::SDN::Zones::get_vnets($dhcpid, 1) // {};
+        my $subnets = PVE::Network::SDN::Vnets::get_subnets(undef, 1) // {};
+        $subnets = {
+            map { $_ => $subnets->{$_} } grep { $vnets->{ $subnets->{$_}->{vnet} } }
+                keys %$subnets
+        };
+        my $vnet_of = sub {
+            my ($ip) = @_;
+            my (undef, $subnet) =
+                eval { PVE::Network::SDN::Subnets::find_ip_subnet($ip, $subnets) };
+            return $subnet ? $subnet->{vnet} : undef;
+        };
+        my %subnets_of;
+        $subnets_of{ $subnets->{$_}->{vnet} }->{$_} = $subnets->{$_} for keys %$subnets;
+
+        # the changed MAC is looked up first, under the ethers lock so that two
+        # edits of one MAC cannot land out of order, and its lines come from
+        # that answer. The other lines are checked against one read of the
+        # cache, taken after the lookup's fill, so only the changed MAC asks
+        # the IPAM
+        my $cache;
+        my $reservation = sub {
+            my ($vnet, $line_mac, $cached_only) = @_;
+            my ($line_ip4, $line_ip6) =
+                $cached_only
+                ? PVE::Network::SDN::Ipams::cached_ips_from_mac(
+                    $cache,
+                    $vnet,
+                    $line_mac,
+                    $subnets_of{$vnet} // {},
+                )
+                : PVE::Network::SDN::Vnets::get_ips_from_mac($vnet, $line_mac, 1);
+            return (undef, undef) if !$line_ip4 && !$line_ip6;
+            my $line = $line_mac;
+            $line .= ",$line_ip4" if $line_ip4;
+            $line .= ",[$line_ip6]" if $line_ip6;
             # an external IPAM spells addresses its own way, while the file holds one spelling
-            $reservation = lc($reservation);
-        }
+            return (lc($line), $line_ip4);
+        };
+        my $own;
+        ($own, $ip4) = $reservation->($vnetid, $mac);
+        $cache = PVE::Network::SDN::Ipams::read_cache();
 
         file_set_contents($ethers_file, '') if !-e $ethers_file;
         open(my $in, '<', $ethers_file) or die "Could not open file '$ethers_file' $!\n";
         open(my $out, '>', $ethers_tmp_file)
             or die "Could not open file '$ethers_tmp_file' $!\n";
 
-        my $kept = undef;
-
+        my %written;
         while (my $line = <$in>) {
             chomp($line);
             next if $line !~ m/\S/;
-            my ($parsed_mac, $parsed_ip1, $parsed_ip2) = split(/,/, $line);
-            # a line from before the file was written in lower case keeps its writer's spelling
+            my ($parsed_mac, $parsed_ip) = split(/,/, $line);
             $parsed_mac = lc($parsed_mac);
-
-            # the MAC's own reservation is rewritten. An unchanged one stays in place
-            if ($parsed_mac eq $mac) {
-                if (defined($reservation) && !$kept && lc($line) eq $reservation) {
-                    $kept = 1;
-                    print $out "$line\n";
-                } else {
-                    $changed = 1;
-                }
-                next;
-            }
-
-            my ($parsed_ip4, $parsed_ip6);
-            if ($parsed_ip2) {
-                $parsed_ip4 = $parsed_ip1;
-                $parsed_ip6 = $parsed_ip2;
-            } elsif (defined($parsed_ip1) && Net::IP::ip_is_ipv4($parsed_ip1)) {
-                $parsed_ip4 = $parsed_ip1;
-            } elsif (defined($parsed_ip1)) {
-                $parsed_ip6 = $parsed_ip1;
-            }
-            $parsed_ip6 = $1 if $parsed_ip6 && $parsed_ip6 =~ m/\[(\S+)\]/;
-            # a line from before addresses were lower-cased may spell one in upper case
-            $parsed_ip6 = lc($parsed_ip6) if $parsed_ip6;
-
-            # a line that no longer matches the cache is replaced by the cache's
-            # current one. Dropping it left that guest unknown to dnsmasq until
-            # its next start
-            my $current = $macs->{$parsed_mac};
-            if (
-                !$current
-                || ($parsed_ip4 && $current->{ip4} && $current->{ip4} ne $parsed_ip4)
-                || ($parsed_ip6 && $current->{ip6} && lc($current->{ip6}) ne $parsed_ip6)
-            ) {
+            $parsed_ip = $1 if defined($parsed_ip) && $parsed_ip =~ m/\[(\S+)\]/;
+            my $vnet = defined($parsed_ip) ? $vnet_of->($parsed_ip) : undef;
+            my $key = defined($vnet) ? "$parsed_mac,$vnet" : undef;
+
+            # a line that no longer matches is replaced, since dropping it left
+            # that guest unknown to dnsmasq until its next start
+            my ($want) = eval {
+                !$key || $written{$key} ? ()
+                    : $key eq "$mac,$vnetid" ? ($own)
+                    : $reservation->($vnet, $parsed_mac, 1);
+            };
+            if ($@) {
+                warn "could not read the reservation of $parsed_mac: $@";
+                print $out "$line\n";
+            } elsif (!defined($want)) {
                 $changed = 1;
-                next if !$current;
-                my $replacement = $parsed_mac;
-                $replacement .= ",$current->{ip4}" if $current->{ip4};
-                $replacement .= ",[$current->{ip6}]" if $current->{ip6};
-                print $out lc($replacement) . "\n";
                 next;
+            } elsif (lc($line) ne $want) {
+                $changed = 1;
+                print $out "$want\n";
+            } else {
+                print $out "$line\n";
             }
-
-            print $out "$line\n";
+            $written{$key}++;
         }
 
-        if (defined($reservation) && !$kept) {
-            print $out "$reservation\n";
+        if (defined($own) && !$written{"$mac,$vnetid"}) {
+            print $out "$own\n";
             $changed = 1;
         }
 
@@ -157,7 +162,7 @@ sub update_ip_mapping {
 
     # the rewrite replaces the reservation file, so a lock on the file itself
     # would not outlive the rename
-    PVE::Tools::lock_file("$ethers_file.lock", 10, $rewriteFn);
+    PVE::Tools::lock_file("$ethers_file.lock", 10, $rewrite_fn);
     die "Unable to update $mac in the dnsmasq configuration: $@" if $@;
 
     systemctl_service('reload', "dnsmasq\@$dhcpid") if $changed;
diff --git a/src/PVE/Network/SDN/Dhcp/Plugin.pm b/src/PVE/Network/SDN/Dhcp/Plugin.pm
index 472ff1534743..9ee9a442cafd 100644
--- a/src/PVE/Network/SDN/Dhcp/Plugin.pm
+++ b/src/PVE/Network/SDN/Dhcp/Plugin.pm
@@ -22,10 +22,11 @@ sub private {
     return $defaultData;
 }
 
-# the MAC's records changed, so make the backend serve the current ones. With no
-# record left its reservation goes. Dies if the backend could not be updated
+# the MAC's records on the vnet changed, so make the backend serve the current
+# ones. With no record left its reservation on the vnet goes. Dies if the
+# backend could not be updated
 sub update_ip_mapping {
-    my ($class, $dhcpid, $mac) = @_;
+    my ($class, $dhcpid, $vnetid, $mac) = @_;
     die 'implement in sub class';
 }
 
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 9a31eaf39f58..a6082438d148 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -35,40 +35,10 @@ sub json_writer {
     return encode_json($data);
 }
 
-# Writers spell a MAC their own way, the generated ones in upper case and the
-# API in either, and nodes not yet upgraded look an entry up under the spelling
-# their caller hands them, so the file keeps its keys as written. Readers get a
-# view with every MAC in lower case. Two spellings that disagree on an address
-# are left out of it, since one of them is stale and nothing here says which,
-# and so is an entry without an address. The next lookup then asks the IPAM
-my sub fold_macdb {
-    my ($db) = @_;
-
-    my $macs = {};
-    my %clash;
-    for my $key (sort keys %{ $db->{macs} // {} }) {
-        my $entry = $db->{macs}->{$key};
-        for my $family (grep { defined($entry->{$_}) } qw(ip4 ip6)) {
-            my $seen = $macs->{ lc($key) }->{$family};
-            $clash{ lc($key) } = 1 if defined($seen) && lc($seen) ne lc($entry->{$family});
-            $macs->{ lc($key) }->{$family} //= $entry->{$family};
-        }
-    }
-    delete $macs->{$_} for keys %clash;
-    return { %$db, macs => $macs };
-}
-
-# the file as written, which the writers change and write back
-sub read_macdb_file {
-    my () = @_;
-
-    return cfs_read_file($macdb_filename);
-}
-
 sub read_macdb {
     my () = @_;
 
-    return fold_macdb(read_macdb_file());
+    return cfs_read_file($macdb_filename);
 }
 
 sub write_macdb {
@@ -77,9 +47,41 @@ sub write_macdb {
     cfs_write_file($macdb_filename, $data);
 }
 
-# a write goes to every spelling the file holds for the MAC, so each reader
-# finds it under its own, and a MAC new to the file is written in lower case
-my sub set_cache_entry {
+# the map under the MAC alone is the one nodes not yet upgraded read and
+# write, under the MAC as their caller spelled it, so one read indexes it by
+# the MAC in lower case for every lookup on that read. Spellings that
+# disagree on an address are both left out, since one of them is stale
+my sub old_index {
+    my ($db) = @_;
+
+    my (%index, %clash);
+    my $macs = $db->{macs} // {};
+    for my $key (sort keys %$macs) {
+        my $mac = lc($key);
+        next if $clash{$mac};
+        my $entry = $index{$mac} //= {};
+        for my $family (qw(ip4 ip6)) {
+            my $ip = $macs->{$key}->{$family} // next;
+            if (defined($entry->{$family}) && lc($entry->{$family}) ne lc($ip)) {
+                $clash{$mac} = 1;
+                last;
+            }
+            $entry->{$family} = $ip;
+        }
+    }
+    delete $index{$_} for keys %clash;
+    return \%index;
+}
+
+sub read_cache {
+    my $db = read_macdb();
+    return { db => $db, old => old_index($db) };
+}
+
+# a write lands in the map under the MAC alone as well, under the keys it
+# already has, with one address per MAC as a node not yet upgraded reads
+# and writes it
+my sub set_old_cache_entry {
     my ($db, $mac, $family, $ip) = @_;
 
     my $macs = $db->{macs} //= {};
@@ -87,15 +89,71 @@ my sub set_cache_entry {
     $macs->{$_}->{$family} = $ip for (@keys ? @keys : (lc($mac)));
 }
 
+my sub in_subnets {
+    my ($ip, $subnets) = @_;
+    return !!eval { PVE::Network::SDN::Subnets::find_ip_subnet($ip, $subnets) };
+}
+
+# the addresses the per-vnet entries hold for the MAC. A write puts its
+# address into the map under the MAC alone as well, so a value found in both
+# is some vnet's and not an entry from before
+my sub claimed_addresses {
+    my ($db, $mac) = @_;
+
+    my %claimed;
+    for my $vnet_entries (values %{ $db->{vnets} // {} }) {
+        my $entry = $vnet_entries->{ lc($mac) } // next;
+        $claimed{ lc($_) } = 1 for grep { defined($_) } @{$entry}{qw(ip4 ip6)};
+    }
+    return \%claimed;
+}
+
+# the vnet's entry answers, and an entry from before the cache was kept per
+# vnet fills the families it lacks with addresses in the vnet's subnets
+my sub cache_entry {
+    my ($cache, $mac, $vnetid, $subnets) = @_;
+
+    my $entry = (($cache->{db}->{vnets} // {})->{$vnetid} // {})->{ lc($mac) } // {};
+    my $old = $cache->{old}->{ lc($mac) } // {};
+    my $claimed = claimed_addresses($cache->{db}, $mac);
+    my $cached = {};
+    for my $family (qw(ip4 ip6)) {
+        my $ip = $entry->{$family};
+        $ip = $old->{$family}
+            if !defined($ip)
+            && defined($old->{$family})
+            && !$claimed->{ lc($old->{$family}) }
+            && in_subnets($old->{$family}, $subnets);
+        $cached->{$family} = $ip if defined($ip);
+    }
+    return %$cached ? $cached : undef;
+}
+
+# a node not yet upgraded releases or moves an address in the map under the
+# MAC alone, so a vnet's address holds only while that map still names it
+my sub witnessed {
+    my ($cache, $mac, $vnetid) = @_;
+
+    my $entry = (($cache->{db}->{vnets} // {})->{$vnetid} // {})->{ lc($mac) } // {};
+    my $old = $cache->{old}->{ lc($mac) } // {};
+    for my $family (grep { defined($entry->{$_}) } qw(ip4 ip6)) {
+        return 0 if lc($old->{$family} // '') ne lc($entry->{$family});
+    }
+    return 1;
+}
+
 sub add_cache_mac_ip {
-    my ($mac, $ip) = @_;
+    my ($vnetid, $mac, $ip) = @_;
+    $mac = lc($mac);
+    my $family = Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6';
 
     cfs_lock_file(
         $macdb_filename,
         undef,
         sub {
-            my $db = read_macdb_file();
-            set_cache_entry($db, $mac, Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6', $ip);
+            my $db = read_macdb();
+            $db->{vnets}->{$vnetid}->{$mac}->{$family} = $ip;
+            set_old_cache_entry($db, $mac, $family, $ip);
             write_macdb($db);
         },
     );
@@ -103,19 +161,29 @@ sub add_cache_mac_ip {
 }
 
 sub del_cache_mac_ip {
-    my ($mac, $ip) = @_;
+    my ($vnetid, $mac, $ip) = @_;
+    $mac = lc($mac);
+    my $family = Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6';
 
     cfs_lock_file(
         $macdb_filename,
         undef,
         sub {
-            my $db = read_macdb_file();
-            my $family = Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6';
+            my $db = read_macdb();
+            my $vnet_entries = ($db->{vnets} // {})->{$vnetid};
+            if (my $entry = $vnet_entries ? $vnet_entries->{$mac} : undef) {
+                delete $entry->{$family};
+                delete $vnet_entries->{$mac} if !%$entry;
+                delete $db->{vnets}->{$vnetid} if !%$vnet_entries;
+                delete $db->{vnets} if !%{ $db->{vnets} };
+            }
+            # the map under the MAC alone must not hand a released address back
             my $macs = $db->{macs} // {};
-            for my $key (grep { lc($_) eq lc($mac) } keys %$macs) {
-                delete $macs->{$key}->{$family};
-                delete $macs->{$key}
-                    if !defined($macs->{$key}->{ip4}) && !defined($macs->{$key}->{ip6});
+            for my $key (grep { lc($_) eq $mac } keys %$macs) {
+                my $old = $macs->{$key};
+                next if lc($old->{$family} // '') ne lc($ip);
+                delete $old->{$family};
+                delete $macs->{$key} if !defined($old->{ip4}) && !defined($old->{ip6});
             }
             write_macdb($db);
         },
@@ -168,16 +236,22 @@ sub complete_sdn_vnet {
     return $cmdname eq 'add' ? [] : [PVE::Network::SDN::Vnets::sdn_ipams_ids($cfg)];
 }
 
+# the addresses one read of the cache holds for a MAC on a vnet
+sub cached_ips_from_mac {
+    my ($cache, $vnetid, $mac, $subnets) = @_;
+
+    my $cached = cache_entry($cache, $mac, $vnetid, $subnets);
+    return $cached ? ($cached->{ip4}, $cached->{ip6}) : ();
+}
+
 sub get_ips_from_mac {
-    my ($mac, $zoneid, $zone, $subnets) = @_;
+    my ($vnetid, $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);
-    my $cached = read_macdb()->{macs}->{ lc($mac) };
-    # an entry without an address was left behind by a lookup that missed, and
-    # says nothing about its MAC
-    return ($cached->{ip4}, $cached->{ip6})
-        if $cached && (defined($cached->{ip4}) || defined($cached->{ip6}));
+    my $cache = read_cache();
+    my $cached = cache_entry($cache, $mac, $vnetid, $subnets);
+    return ($cached->{ip4}, $cached->{ip6}) if $cached && witnessed($cache, $mac, $vnetid);
 
     my $plugin_config = get_plugin_config($zone);
     my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
@@ -195,9 +269,17 @@ sub get_ips_from_mac {
             return
                 if (PVE::Cluster::cfs_file_version($macdb_filename) // '') ne ($version // '');
 
-            my $db = read_macdb_file();
-            set_cache_entry($db, $mac, 'ip4', $ip4) if defined($ip4);
-            set_cache_entry($db, $mac, 'ip6', $ip6) if defined($ip6);
+            my $db = read_macdb();
+            my $entry = $db->{vnets}->{$vnetid}->{ lc($mac) } //= {};
+            for my $family (qw(ip4 ip6)) {
+                my $ip = $family eq 'ip4' ? $ip4 : $ip6;
+                if (!defined($ip)) {
+                    delete $entry->{$family};
+                    next;
+                }
+                $entry->{$family} = $ip;
+                set_old_cache_entry($db, $mac, $family, $ip);
+            }
             write_macdb($db);
         },
     );
diff --git a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
index 9e1c9537e6ec..84ce4382e0e7 100644
--- a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
@@ -360,7 +360,7 @@ sub add_range_next_freeip {
 }
 
 sub del_ip {
-    my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr) = @_;
+    my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr, $mac) = @_;
 
     return if !$ip;
 
@@ -369,6 +369,19 @@ sub del_ip {
         warn "could not find id for ip $ip";
         return;
     }
+    if ($mac) {
+        my $record =
+            eval { netbox_api_request($plugin_config, "GET", "/ipam/ip-addresses/$ip_id/") };
+        if ($@) {
+            return 0 if $noerr;
+            die "could not check who holds IP $ip: $@";
+        }
+        my ($owner) = ($record->{description} // '') =~ m/^mac:(\S+)$/;
+        if ($owner && lc($owner) ne lc($mac)) {
+            warn "not releasing IP '$ip' for $mac, it is held by $owner\n";
+            return 0;
+        }
+    }
 
     eval { netbox_api_request($plugin_config, "DELETE", "/ipam/ip-addresses/$ip_id/"); };
     if ($@) {
diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index 43d0a3daaf9a..86e594a4c604 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -249,11 +249,12 @@ sub add_range_next_freeip {
 }
 
 sub del_ip {
-    my ($class, $plugin_config, $subnetid, $subnet, $ip) = @_;
+    my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr, $mac) = @_;
 
     my $cidr = $subnet->{cidr};
     my $zone = $subnet->{zone};
 
+    my $released = 1;
     cfs_lock_file(
         $ipamdb_file,
         undef,
@@ -271,11 +272,18 @@ sub del_ip {
                 warn "IP '$ip' does not exist in IPAM DB\n";
                 return;
             }
+            my $owner = $dbsubnet->{ips}->{$ip}->{mac};
+            if ($mac && $owner && lc($owner) ne lc($mac)) {
+                warn "not releasing IP '$ip' for $mac, it is held by $owner\n";
+                $released = 0;
+                return;
+            }
             delete $dbsubnet->{ips}->{$ip};
             write_db($db);
         },
     );
     die "$@" if $@;
+    return $released;
 }
 
 sub get_ips_from_mac {
@@ -297,7 +305,8 @@ sub get_ips_from_mac {
         my $ips = $dbzone->{subnets}->{$subnet}->{ips} // next;
         for my $ip (keys %$ips) {
             my $ipobject = $ips->{$ip};
-            if ($ipobject->{mac} && $ipobject->{mac} eq $mac) {
+            # a record keeps the MAC as its writer spelled it
+            if ($ipobject->{mac} && lc($ipobject->{mac}) eq lc($mac)) {
                 if (Net::IP::ip_is_ipv4($ip)) {
                     $ip4 = $ip;
                 } else {
diff --git a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
index 12d470e802c4..e969c3dcdf4f 100644
--- a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
@@ -239,7 +239,7 @@ sub add_range_next_freeip {
 }
 
 sub del_ip {
-    my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr) = @_;
+    my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr, $mac) = @_;
 
     return if !$ip;
 
@@ -250,6 +250,22 @@ sub del_ip {
 
     my $ip_id = get_ip_id($url, $ip, $headers);
     return if !$ip_id;
+    if ($mac) {
+        my $record = eval {
+            PVE::Network::SDN::api_request(
+                "GET", "$url/addresses/$ip_id/", $headers, undef, $fingerprint,
+            );
+        };
+        if ($@) {
+            return 0 if $noerr;
+            die "could not check who holds IP $ip: $@";
+        }
+        my $owner = ($record->{data} // {})->{mac};
+        if ($owner && lc($owner) ne lc($mac)) {
+            warn "not releasing IP '$ip' for $mac, it is held by $owner\n";
+            return 0;
+        }
+    }
 
     eval {
         PVE::Network::SDN::api_request(
@@ -259,6 +275,8 @@ sub del_ip {
     if ($@) {
         die "error delete ip $ip: $@" if !$noerr;
     }
+
+    return 1;
 }
 
 sub get_ips_from_mac {
diff --git a/src/PVE/Network/SDN/Ipams/Plugin.pm b/src/PVE/Network/SDN/Ipams/Plugin.pm
index 14f8fe9e67fe..6e23d5434d4b 100644
--- a/src/PVE/Network/SDN/Ipams/Plugin.pm
+++ b/src/PVE/Network/SDN/Ipams/Plugin.pm
@@ -132,8 +132,10 @@ sub add_range_next_freeip {
     die "please implement inside plugin";
 }
 
+# a MAC names the guest the address is released for, a record holding the
+# address for another MAC is then left alone and 0 returned
 sub del_ip {
-    my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr) = @_;
+    my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr, $mac) = @_;
 
     die "please implement inside plugin";
 }
diff --git a/src/PVE/Network/SDN/Subnets.pm b/src/PVE/Network/SDN/Subnets.pm
index f8be98201476..a7fa7056e556 100644
--- a/src/PVE/Network/SDN/Subnets.pm
+++ b/src/PVE/Network/SDN/Subnets.pm
@@ -266,7 +266,7 @@ sub add_next_free_ip {
 
         die $@ if $@;
 
-        eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, $ip); };
+        eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($subnet->{vnet}, $mac, $ip); };
         warn $@ if $@;
     }
 
@@ -325,7 +325,7 @@ sub add_ip {
         };
         die $@ if $@;
 
-        eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, $ip) if $mac; };
+        eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($subnet->{vnet}, $mac, $ip) if $mac; };
         warn $@ if $@;
     }
 
@@ -413,18 +413,22 @@ sub del_ip {
         verify_dns_zone($reversednszone, $reversedns);
     }
 
+    my $released;
     if ($ipamid) {
         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, $subnet, $ip);
+        $released = $plugin->del_ip($plugin_config, $subnetid, $subnet, $ip, undef, $mac);
 
         if ($mac) {
-            eval { PVE::Network::SDN::Ipams::del_cache_mac_ip($mac, $ip) };
+            eval { PVE::Network::SDN::Ipams::del_cache_mac_ip($subnet->{vnet}, $mac, $ip) };
             warn $@ if $@;
         }
     }
 
+    # a record another MAC holds keeps its DNS entries as well
+    return 0 if defined($released) && !$released;
+
     eval {
         if (!$skipdns) {
             del_dns_record($dnszone, $dns, $hostname, $ip);
@@ -434,6 +438,8 @@ sub del_ip {
     if ($@) {
         warn $@;
     }
+
+    return $released;
 }
 
 =head3 api_get_subnet($cfg, $id)
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index 395aa32b6a84..58db0f803f8b 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -194,18 +194,18 @@ sub del_ip {
 }
 
 sub get_ips_from_mac {
-    my ($vnetid, $mac) = @_;
+    my ($vnetid, $mac, $running) = @_;
 
-    my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid);
+    my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid, $running);
     return if !$vnet;
 
     my $zoneid = $vnet->{zone};
-    my $zone = PVE::Network::SDN::Zones::get_zone($zoneid);
+    my $zone = PVE::Network::SDN::Zones::get_zone($zoneid, $running);
 
     return if !$zone->{ipam} || !$zone->{dhcp};
 
-    my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid);
-    return PVE::Network::SDN::Ipams::get_ips_from_mac($mac, $zoneid, $zone, $subnets);
+    my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid, $running);
+    return PVE::Network::SDN::Ipams::get_ips_from_mac($vnetid, $mac, $zoneid, $zone, $subnets);
 }
 
 sub del_ips_from_mac {
diff --git a/src/test/run_test_ipams.pl b/src/test/run_test_ipams.pl
index f9c44fe1a3b9..f07b4bb4b486 100755
--- a/src/test/run_test_ipams.pl
+++ b/src/test/run_test_ipams.pl
@@ -219,5 +219,54 @@ foreach my $path (@plugins) {
 
 }
 
+# an address released for a MAC stays when the external IPAM records it
+# for another MAC
+{
+    my $subnet = { zone => 'z', cidr => '10.0.0.0/24' };
+    my $owner;
+    my $check_fails = 0;
+    my @deletes;
+    my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+    $sdn_module->mock(
+        api_request => sub {
+            my ($method, $url) = @_;
+            die "Invalid response from server: 503 Service Unavailable\n"
+                if $check_fails && $method eq 'GET';
+            push @deletes, $url if $method eq 'DELETE';
+            return {
+                description => "mac:$owner",
+                data => { mac => $owner },
+            };
+        },
+    );
+    my $netbox = Test::MockModule->new('PVE::Network::SDN::Ipams::NetboxPlugin');
+    $netbox->mock(subnet_prefix => sub { return (1, 2); }, get_ip_id => sub { return 3; });
+    my $phpipam = Test::MockModule->new('PVE::Network::SDN::Ipams::PhpIpamPlugin');
+    $phpipam->mock(subnet_id => sub { return 7; }, get_ip_id => sub { return 3; });
+
+    for my $plugin (
+        'PVE::Network::SDN::Ipams::NetboxPlugin',
+        'PVE::Network::SDN::Ipams::PhpIpamPlugin',
+    ) {
+        my $config = { url => 'u', token => 't' };
+        @deletes = ();
+        $owner = 'da:65:8f:18:9b:70';
+        $plugin->del_ip($config, 'z-10.0.0.0-24', $subnet, '10.0.0.50', 1, 'DA:65:8F:18:9B:6F');
+        is(scalar(@deletes), 0, "$plugin keeps the record of another MAC");
+        $owner = 'da:65:8f:18:9b:6f';
+        $plugin->del_ip($config, 'z-10.0.0.0-24', $subnet, '10.0.0.50', 1, 'DA:65:8F:18:9B:6F');
+        is(scalar(@deletes), 1, "$plugin releases the MAC's own record");
+
+        # a failed check of the holder releases nothing either
+        @deletes = ();
+        $check_fails = 1;
+        my $released =
+            $plugin->del_ip($config, 'z-10.0.0.0-24', $subnet, '10.0.0.50', 1, 'DA:65:8F:18:9B:6F');
+        $check_fails = 0;
+        is($released, 0, "$plugin releases nothing when the check of the holder fails");
+        is(scalar(@deletes), 0, "$plugin sent no delete for it");
+    }
+}
+
 done_testing();
 
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 507bae29a84e..28bf7d30e670 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -195,7 +195,7 @@ $mocked_sdn_ipams->mock(
         my ($cfg) = @_;
         $test_state->{ipam_config} = $cfg;
     },
-    read_macdb_file => sub {
+    read_macdb => sub {
         return PVE::Network::SDN::Ipams::json_reader(undef, encode_json($test_state->{macdb}));
     },
     write_macdb => sub {
@@ -505,74 +505,101 @@ sub test_ipam_cache_lookup {
     # an entry holding no address, left behind by an earlier miss, does not
     # hide a record created since
     create_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.50" });
-    $test_state->{macdb}->{macs}->{$mac} = { ip4 => undef, ip6 => undef };
+    $test_state->{macdb} = { macs => { $mac => { ip4 => undef, ip6 => undef } } };
     my @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
     is($ips[0], "10.0.0.50", "$test_name: an address-less entry counts as a miss");
-    is(
-        $test_state->{macdb}->{macs}->{$mac}->{ip4},
-        "10.0.0.50",
-        "$test_name: the answer is cached",
+    is_deeply(
+        $test_state->{macdb},
+        {
+            macs => { $mac => { ip4 => "10.0.0.50", ip6 => undef } },
+            vnets => { $vnetid => { $mac => { ip4 => "10.0.0.50" } } },
+        },
+        "$test_name: the answer is cached in both maps",
     );
 
     # a write to the cache between the miss and the fill, which the file's
     # version shows, keeps the IPAM's answer out of the cache
-    delete $test_state->{macdb}->{macs}->{$mac};
+    $test_state->{macdb} = {};
     my $version = 0;
     $mocked_pve_cluster_obj->mock(cfs_file_version => sub { return $version++ });
     @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
     $mocked_pve_cluster_obj->unmock('cfs_file_version');
     is($ips[0], "10.0.0.50", "$test_name: the lookup still answers");
-    ok(!exists $test_state->{macdb}->{macs}->{$mac},
-        "$test_name: but leaves a changed cache alone");
+    is_deeply($test_state->{macdb}, {}, "$test_name: but leaves a changed cache alone");
 
-    # an entry written under the config's spelling, as a node not yet upgraded
-    # writes it, is read and released like any other and keeps its key
+    # an entry written under the config's spelling before the cache was keyed in
+    # lower case is read and released like any other
     my $other = "da:65:8f:18:9b:70";
     $test_state->{macdb}->{macs} = {
         uc($mac) => { ip4 => '10.0.0.150' },
         uc($other) => { ip4 => '10.0.0.170' },
     };
     my ($ip4) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
-    is($ip4, '10.0.0.150', "$test_name: an entry is found however it is spelled");
-    PVE::Network::SDN::Ipams::del_cache_mac_ip($mac, '10.0.0.150');
+    is($ip4, '10.0.0.150', "$test_name: an old entry is found however it is spelled");
+    PVE::Network::SDN::Ipams::del_cache_mac_ip($vnetid, $mac, '10.0.0.150');
     is_deeply(
         $test_state->{macdb}->{macs},
         { uc($other) => { ip4 => '10.0.0.170' } },
         "$test_name: and released, the other entry left as written",
     );
 
-    # a write in another spelling goes to the spelling the file holds
+    # a write by this code updates an old entry under the spelling a node not
+    # yet upgraded wrote it in, so that node still finds it
     $test_state->{macdb}->{macs} = { uc($mac) => { ip4 => '10.0.0.150' } };
-    PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, 'fd00::150');
+    PVE::Network::SDN::Ipams::add_cache_mac_ip($vnetid, uc($mac), 'fd00::150');
     is_deeply(
         $test_state->{macdb}->{macs},
         { uc($mac) => { ip4 => '10.0.0.150', ip6 => 'fd00::150' } },
-        "$test_name: a write keeps the key as written",
+        "$test_name: a write updates the old entry under its spelling",
+    );
+    is_deeply(
+        [PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac)],
+        ['10.0.0.150', 'fd00::150'],
+        "$test_name: holding both addresses",
     );
 
-    # a lookup past an address-less entry asks the IPAM and fills the entry
-    # under its spelling
-    $test_state->{macdb}->{macs} = { uc($mac) => { ip4 => undef, ip6 => undef } };
+    # a vnet's entry and an old one that disagree are not trusted either, a
+    # node not yet upgraded may have moved the address since, and a family
+    # the old entry lacks was released by such a node
+    $test_state->{macdb} = {
+        macs => { $mac => { ip4 => '10.0.0.151' } },
+        vnets => { $vnetid => { $mac => { ip4 => '10.0.0.150', ip6 => 'fd00::150' } } },
+    };
     @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
-    is($ips[0], '10.0.0.50', "$test_name: a lookup past an address-less entry asks the IPAM");
+    is(
+        $ips[0],
+        '10.0.0.50',
+        "$test_name: an old entry disagreeing with the vnet's counts as a miss",
+    );
     is_deeply(
-        $test_state->{macdb}->{macs},
-        { uc($mac) => { ip4 => '10.0.0.50', ip6 => undef } },
-        "$test_name: and fills the entry under its spelling",
+        $test_state->{macdb},
+        {
+            macs => { $mac => { ip4 => '10.0.0.50' } },
+            vnets => { $vnetid => { $mac => { ip4 => '10.0.0.50' } } },
+        },
+        "$test_name: the answer replaces the vnet's entry and the old one, the family without a record is gone",
     );
 
     # two spellings that disagree are not trusted, the lookup asks the IPAM
-    $test_state->{macdb}->{macs} = {
-        uc($mac) => { ip4 => '10.0.0.150' },
-        $mac => { ip4 => '10.0.0.151' },
+    $test_state->{macdb} = {
+        macs => {
+            uc($mac) => { ip4 => '10.0.0.150' },
+            $mac => { ip4 => '10.0.0.151' },
+        },
     };
     @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
     is($ips[0], '10.0.0.50', "$test_name: a clash of spellings counts as a miss");
-    is_deeply(
-        $test_state->{macdb}->{macs},
-        { uc($mac) => { ip4 => '10.0.0.50' }, $mac => { ip4 => '10.0.0.50' } },
-        "$test_name: and its write brings both spellings in line",
-    );
+
+    # an old entry's address outside the vnet's subnets is another vnet's
+    $test_state->{macdb} = { macs => { uc($mac) => { ip4 => '10.9.0.150' } } };
+    @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+    is($ips[0], '10.0.0.50', "$test_name: an old address of another vnet counts as a miss");
+
+    # the record keeps the MAC as its writer spelled it, a lookup in another
+    # spelling finds it
+    $test_state->{macdb} = {};
+    @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, uc($mac));
+    is($ips[0], '10.0.0.50', "$test_name: a lookup in another spelling finds the record");
 
     # a mapping whose record is gone is still deleted, and its cache entry
     # with it
@@ -1156,21 +1183,40 @@ sub test_dnsmasq_stale_reservation_replaced {
     my $stale = "da:65:8f:18:9b:01";
     my $gone = "da:65:8f:18:9b:02";
     my $added = "da:65:8f:18:9b:03";
+    create_dhcp_zone("TESTZONE", "testvnet", "10.0.1.0/24");
     $test_state->{macdb} = {
-        macs => {
-            $stale => { ip4 => '10.0.1.11' },
-            $added => { ip4 => '10.0.1.13' },
+        macs => { $stale => { ip4 => '10.0.1.11' }, $added => { ip4 => '10.0.1.13' } },
+        vnets => {
+            testvnet => {
+                $stale => { ip4 => '10.0.1.11' },
+                $added => { ip4 => '10.0.1.13' },
+            },
         },
     };
     PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$stale,10.0.1.10\n$gone,10.0.1.12\n");
 
-    PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping('TESTZONE', $added);
+    PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping('TESTZONE', 'testvnet', $added);
 
     is(
         PVE::Tools::file_get_contents($TMP_ETHERS_FILE),
         "$stale,10.0.1.11\n$added,10.0.1.13\n",
         "$test_name: the stale line is replaced, the unknown one dropped",
     );
+
+    # the changed MAC's own line follows the IPAM's answer, not the cache,
+    # when its entry is no longer witnessed
+    create_ip({ zone => "TESTZONE", vnet => "testvnet", mac => $added, ip => "10.0.1.20" });
+    $test_state->{macdb} = {
+        macs => { $added => { ip4 => '10.0.1.22' } },
+        vnets => { testvnet => { $added => { ip4 => '10.0.1.21' } } },
+    };
+    PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$added,10.0.1.21\n");
+    PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping('TESTZONE', 'testvnet', $added);
+    is(
+        PVE::Tools::file_get_contents($TMP_ETHERS_FILE),
+        "$added,10.0.1.20\n",
+        "$test_name: the changed MAC's stale line is replaced by the IPAM's answer",
+    );
 }
 
 run_test(\&test_dnsmasq_stale_reservation_replaced);
@@ -1355,8 +1401,208 @@ sub test_ipam_per_vnet {
             "$test_name: vnet$n answers with its own address",
         );
     }
+
+    # a mapping edit on one vnet leaves the other's answer alone
+    PVE::API2::Network::SDN::Ips->ipupdate(
+        { zone => $zoneid, vnet => 'vnet1', mac => $mac, ip => '10.1.0.51' });
+    is_deeply(
+        [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+        ['10.2.0.50', undef],
+        "$test_name: an edit on the first vnet leaves the second's answer alone",
+    );
+    is_deeply(
+        [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet1', $mac)],
+        ['10.1.0.51', undef],
+        "$test_name: and changes the first's",
+    );
+
+    # an entry from before the cache was kept per vnet answers for a vnet
+    # without an entry of its own, until a write for the MAC on another vnet
+    # replaces it in the map under the MAC alone, which holds one address.
+    # The vnet then asks the IPAM
+    $test_state->{macdb} = { macs => { $mac => { ip4 => '10.2.0.60' } } };
+    is_deeply(
+        [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+        ['10.2.0.60', undef],
+        "$test_name: an old entry answers for a vnet without an entry of its own",
+    );
+    create_ip({ zone => $zoneid, vnet => 'vnet1', mac => $mac, ip => '10.1.0.52' });
+    is_deeply(
+        [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+        ['10.2.0.50', undef],
+        "$test_name: once a write for the MAC elsewhere replaced it, the IPAM answers",
+    );
+
+    # a vnet's address holds only while the map under the MAC alone names it,
+    # since that map is where a node not yet upgraded releases or moves an
+    # address. With the MAC on two vnets the map witnesses one of them
+    $test_state->{macdb} = {
+        macs => { $mac => { ip4 => '10.2.0.50' } },
+        vnets => {
+            vnet1 => { $mac => { ip4 => '10.1.0.50' } },
+            vnet2 => { $mac => { ip4 => '10.2.0.50' } },
+        },
+    };
+    is_deeply(
+        [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+        ['10.2.0.50', undef],
+        "$test_name: the vnet whose address the old map names answers from the cache",
+    );
+    like(
+        (PVE::Network::SDN::Vnets::get_ips_from_mac('vnet1', $mac))[0],
+        qr/^10\.1\.0\.5[12]$/,
+        "$test_name: the other one asks the IPAM",
+    );
+    like(
+        $test_state->{macdb}->{macs}->{$mac}->{ip4},
+        qr/^10\.1\.0\.5[12]$/,
+        "$test_name: and its answer takes the old map",
+    );
+    delete $test_state->{macdb}->{macs};
+    is_deeply(
+        [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+        ['10.2.0.50', undef],
+        "$test_name: without the family in the old map the IPAM is asked",
+    );
+
+    # the address of the MAC on a vnet of another zone with the same range
+    # sits in the old map as well, and is that vnet's, so a vnet without an
+    # entry asks the IPAM rather than taking it, and one with an entry the
+    # old map does not name asks as well
+    create_zone({ type => "simple", dhcp => "dnsmasq", ipam => "pve", zone => "ZONEB" });
+    create_dhcp_vnet("ZONEB", "vnet3", "10.1.0.0/24");
+    create_ip({ zone => "ZONEB", vnet => "vnet3", mac => $mac, ip => "10.1.0.70" });
+    $test_state->{macdb} = {
+        macs => { $mac => { ip4 => '10.1.0.52' } },
+        vnets => { vnet1 => { $mac => { ip4 => '10.1.0.52' } } },
+    };
+    is_deeply(
+        [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet3', $mac)],
+        ['10.1.0.70', undef],
+        "$test_name: another zone's address in the old map is not taken for a vnet without an entry",
+    );
+    $test_state->{macdb}->{macs}->{$mac}->{ip4} = '10.1.0.52';
+    $test_state->{macdb}->{vnets}->{vnet3}->{$mac} = { ip4 => '10.1.0.71' };
+    is_deeply(
+        [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet3', $mac)],
+        ['10.1.0.70', undef],
+        "$test_name: and an entry the old map does not name is checked with the IPAM",
+    );
+
+    # a release on an upgraded node clears the old map's slot, the MAC's
+    # other vnet asks the IPAM at its next start and takes it
+    $test_state->{macdb} = {
+        macs => { $mac => { ip4 => '10.2.0.50' } },
+        vnets => {
+            vnet1 => { $mac => { ip4 => '10.1.0.52' } },
+            vnet2 => { $mac => { ip4 => '10.2.0.50' } },
+        },
+    };
+    PVE::Network::SDN::Ipams::del_cache_mac_ip('vnet2', $mac, '10.2.0.50');
+    is_deeply(
+        $test_state->{macdb},
+        { macs => {}, vnets => { vnet1 => { $mac => { ip4 => '10.1.0.52' } } } },
+        "$test_name: a release clears the old map's slot",
+    );
 }
 
 run_test(\&test_ipam_per_vnet);
 
+sub test_ipam_edit_stale_entry {
+    my $test_name = (split(/::/, (caller(0))[3]))[-1];
+    my $zoneid = "TESTZONE";
+    my $vnetid = "testvnet";
+    my $mac = "da:65:8f:18:9b:6f";
+    my $other = "da:65:8f:18:9b:70";
+
+    create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+    # a node not yet upgraded released the MAC's address and handed it to
+    # another guest, while the vnet's cache entry still holds it. The entry
+    # is not trusted, so the edit releases nothing and records the new address
+    create_ip({ zone => $zoneid, vnet => $vnetid, mac => $other, ip => '10.0.0.50' });
+    $test_state->{macdb}->{vnets}->{$vnetid}->{$mac} = { ip4 => '10.0.0.50' };
+
+    PVE::API2::Network::SDN::Ips->ipupdate(
+        { zone => $zoneid, vnet => $vnetid, mac => $mac, ip => '10.0.0.60' });
+    is(
+        get_ip4(get_ips_from_mac($other)),
+        '10.0.0.50',
+        "$test_name: an edit leaves the other guest's record alone",
+    );
+    is(get_ip4(get_ips_from_mac($mac)), '10.0.0.60', "$test_name: and records the new address");
+    is(
+        $test_state->{macdb}->{vnets}->{$vnetid}->{$mac}->{ip4},
+        '10.0.0.60',
+        "$test_name: which the cache holds now",
+    );
+
+    # a deletion of a mapping the IPAM does not hold is refused
+    $test_state->{macdb}->{vnets}->{$vnetid}->{$mac} = { ip4 => '10.0.0.50' };
+    eval {
+        PVE::API2::Network::SDN::Ips->ipdelete(
+            { zone => $zoneid, vnet => $vnetid, mac => $mac, ip => '10.0.0.50' });
+    };
+    like($@, qr/not mapped to/, "$test_name: a deletion for another guest's address is refused");
+}
+
+run_test(\&test_ipam_edit_stale_entry);
+
+sub test_dnsmasq_reservation_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 a reservation on each, and a
+    # start on one vnet changes that vnet's line 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" });
+    }
+
+    my $reservations = sub {
+        return [sort grep { $_ } split(/\n/, PVE::Tools::file_get_contents($TMP_ETHERS_FILE))];
+    };
+
+    nic_start("vnet$_", $mac, "999", "testhostname") for (1, 2, 1);
+    is_deeply(
+        $reservations->(),
+        ["$mac,10.1.0.50", "$mac,10.2.0.50"],
+        "$test_name: each vnet keeps its reservation",
+    );
+
+    PVE::API2::Network::SDN::Ips->ipupdate(
+        { zone => $zoneid, vnet => 'vnet1', mac => $mac, ip => '10.1.0.51' });
+    nic_start('vnet1', $mac, "999", "testhostname");
+    is_deeply(
+        $reservations->(),
+        ["$mac,10.1.0.51", "$mac,10.2.0.50"],
+        "$test_name: an edit replaces the reservation of its vnet only",
+    );
+
+    # the other lines are checked against the cache alone, so a line neither
+    # map holds is dropped without the IPAM being asked
+    delete $test_state->{macdb}->{vnets}->{vnet2};
+    delete $test_state->{macdb}->{macs};
+    nic_start('vnet1', $mac, "999", "testhostname");
+    is_deeply(
+        $reservations->(),
+        ["$mac,10.1.0.51"],
+        "$test_name: a line the cache does not hold is dropped",
+    );
+
+    # the changed MAC's own line is answered by the IPAM when the cache does
+    # not hold it
+    delete $test_state->{macdb}->{vnets}->{vnet1};
+    PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, 'vnet1', $mac);
+    is_deeply(
+        $reservations->(),
+        ["$mac,10.1.0.51"],
+        "$test_name: the changed MAC's line is kept without a cache entry",
+    );
+}
+
+run_test(\&test_dnsmasq_reservation_per_vnet);
+
 done_testing();
-- 
2.47.3





  parent reply	other threads:[~2026-11-03 15:17 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-03 15:15 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 01/11] sdn: ipam: stop caching IPAM misses Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 02/11] sdn: ipam: write the lookup's cache entry under the cluster lock Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 03/11] sdn: ipam: read the MAC cache case-insensitively Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 04/11] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 05/11] sdn: ipam: release an address the built-in IPAM no longer holds Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 06/11] sdn: dhcp: update reservations when ip mappings change Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 07/11] sdn: ipam: look a MAC up within the vnet's subnets Hannes Laimer
2026-11-03 15:15 ` Hannes Laimer [this message]
2026-11-03 15:15 ` [PATCH pve-network v2 09/11] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM Hannes Laimer
2026-11-03 15:15 ` [PATCH qemu-server v2 10/11] network: sdn: push the DHCP mappings of a guest when it resumes Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-docs v2 11/11] sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM Hannes Laimer
2026-11-03 15:18 ` [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-03 18:57 ` superseded: " Hannes Laimer
  -- strict thread matches above, loose matches on Subject: below --
2026-11-02 14:56 Hannes Laimer
2026-11-02 14:56 ` [PATCH pve-network v2 08/11] sdn: ipam: keep the MAC cache per vnet Hannes Laimer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261103151543.11917-9-h.laimer@proxmox.com \
    --to=h.laimer@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal