* [PATCH pve-network v4 01/13] sdn: ipam: stop caching IPAM misses
2026-11-03 18:55 [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
@ 2026-11-03 18:55 ` Hannes Laimer
2026-11-03 18:55 ` [PATCH pve-network v4 02/13] sdn: ipam: cache a MAC lookup's answer under the cluster lock Hannes Laimer
` (12 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 18:55 UTC (permalink / raw)
To: pve-devel
A lookup that missed the cache stored the IPAM's empty answer, and
any entry counted as a hit. So a record created later directly in the
IPAM stayed hidden, and a guest start allocated another address.
Entries without an address, which older versions leave in the cache,
no longer count as a hit.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams.pm | 16 ++++++--
src/test/run_test_vnets_blackbox.pl | 58 ++++++++++++++++++++++++++++-
2 files changed, 69 insertions(+), 5 deletions(-)
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 179bdf76defc..7e7bec27b5ed 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -136,16 +136,24 @@ sub get_ips_from_mac {
my ($mac, $zoneid, $zone) = @_;
my $macdb = read_macdb();
- return ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6}) if $macdb->{macs}->{$mac};
+ my $cached = $macdb->{macs}->{$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 $plugin_config = get_plugin_config($zone);
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
- ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6}) =
- $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid);
+ my ($ip4, $ip6) = $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid);
+ # an empty answer is not cached, since the record may simply not exist yet
+ return if !defined($ip4) && !defined($ip6);
+
+ $macdb->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4);
+ $macdb->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6);
write_macdb($macdb);
- return ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6});
+ return ($ip4, $ip6);
}
1;
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index d5a78c5b9a16..37eb5567eae6 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -5,6 +5,7 @@ use warnings;
use lib qw(..);
use File::Slurp;
+use JSON;
use List::Util qw(first all);
use NetAddr::IP qw(:lower);
@@ -194,7 +195,7 @@ $mocked_sdn_ipams->mock(
$test_state->{ipam_config} = $cfg;
},
read_macdb => sub {
- return $test_state->{macdb};
+ return PVE::Network::SDN::Ipams::json_reader(undef, encode_json($test_state->{macdb}));
},
write_macdb => sub {
my ($cfg) = @_;
@@ -335,6 +336,34 @@ sub create_subnet {
PVE::API2::Network::SDN::Subnets->create($params);
}
+# a DHCP zone with one vnet, each subnet with its first address as the
+# gateway and a range over its hosts 100 to 200
+sub create_dhcp_zone {
+ my ($zoneid, $vnetid, @cidrs) = @_;
+ create_zone({ type => "simple", dhcp => "dnsmasq", ipam => "pve", zone => $zoneid });
+ create_dhcp_vnet($zoneid, $vnetid, @cidrs);
+}
+
+sub create_dhcp_vnet {
+ my ($zoneid, $vnetid, @cidrs) = @_;
+ create_vnet({ type => "vnet", zone => $zoneid, vnet => $vnetid });
+ create_dhcp_subnet($vnetid, $_) for @cidrs;
+}
+
+sub create_dhcp_subnet {
+ my ($vnetid, $cidr) = @_;
+ my $net = NetAddr::IP->new($cidr);
+ my $addr = sub { my ($ip) = @_; return $ip->version() == 4 ? $ip->addr() : $ip->short(); };
+ create_subnet({
+ type => "subnet",
+ vnet => $vnetid,
+ subnet => $cidr,
+ gateway => $addr->($net->first()),
+ 'dhcp-range' =>
+ ["start-address=" . $addr->($net + 100) . ",end-address=" . $addr->($net + 200)],
+ });
+}
+
sub get_ipam_entries {
return PVE::API2::Network::SDN::Ipams->ipamindex({ ipam => "pve" });
}
@@ -435,6 +464,33 @@ sub test_without_subnet {
}
run_test(\&test_without_subnet);
+sub test_ipam_cache_lookup {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+ my $cache = decode_json(encode_json($test_state->{macdb}));
+ PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is_deeply($test_state->{macdb}, $cache, "$test_name: the miss is not cached");
+
+ # 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 };
+ 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",
+ );
+}
+
+run_test(\&test_ipam_cache_lookup);
+
sub test_nic_join {
my ($test_name, $subnets) = @_;
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH pve-network v4 02/13] sdn: ipam: cache a MAC lookup's answer under the cluster lock
2026-11-03 18:55 [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-03 18:55 ` [PATCH pve-network v4 01/13] sdn: ipam: stop caching IPAM misses Hannes Laimer
@ 2026-11-03 18:55 ` Hannes Laimer
2026-11-03 18:55 ` [PATCH pve-network v4 03/13] sdn: ipam: read the MAC cache case-insensitively Hannes Laimer
` (11 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 18:55 UTC (permalink / raw)
To: pve-devel
On a cache miss the MAC lookup stored the IPAM's answer without the
cluster lock the other cache writers take. It wrote back the whole
cache as read before the request, so a change made in between was
lost.
The lookup now stores the answer under that lock, and only if the
cache has not changed in the meantime. Such a change may be newer
than the answer, so the answer is then returned without being stored.
The IPAM is still asked outside the lock, since holding the lock
during a request to an external IPAM would make every cache write in
the cluster wait for the IPAM's answer.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams.pm | 24 +++++++++++++++++++-----
src/test/run_test_vnets_blackbox.pl | 11 +++++++++++
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 7e7bec27b5ed..ae2c4337ef8d 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -135,8 +135,9 @@ sub complete_sdn_vnet {
sub get_ips_from_mac {
my ($mac, $zoneid, $zone) = @_;
- my $macdb = read_macdb();
- my $cached = $macdb->{macs}->{$mac};
+ 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}->{$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})
@@ -149,9 +150,22 @@ sub get_ips_from_mac {
# an empty answer is not cached, since the record may simply not exist yet
return if !defined($ip4) && !defined($ip6);
- $macdb->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4);
- $macdb->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6);
- write_macdb($macdb);
+ cfs_lock_file(
+ $macdb_filename,
+ undef,
+ sub {
+ # the IPAM was asked outside the lock, so a write since the miss may
+ # be newer than the answer. The fill is then left to the next lookup
+ return
+ if (PVE::Cluster::cfs_file_version($macdb_filename) // '') ne ($version // '');
+
+ my $db = read_macdb();
+ $db->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4);
+ $db->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6);
+ write_macdb($db);
+ },
+ );
+ warn "$@" if $@;
return ($ip4, $ip6);
}
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 37eb5567eae6..927ab16adba9 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -487,6 +487,17 @@ sub test_ipam_cache_lookup {
"10.0.0.50",
"$test_name: the answer is cached",
);
+
+ # 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};
+ 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");
}
run_test(\&test_ipam_cache_lookup);
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH pve-network v4 03/13] sdn: ipam: read the MAC cache case-insensitively
2026-11-03 18:55 [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-03 18:55 ` [PATCH pve-network v4 01/13] sdn: ipam: stop caching IPAM misses Hannes Laimer
2026-11-03 18:55 ` [PATCH pve-network v4 02/13] sdn: ipam: cache a MAC lookup's answer under the cluster lock Hannes Laimer
@ 2026-11-03 18:55 ` Hannes Laimer
2026-11-03 18:55 ` [PATCH pve-network v4 04/13] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it Hannes Laimer
` (10 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 18:55 UTC (permalink / raw)
To: pve-devel
Generated guest MACs are upper case, but the API accepts either
spelling. The cache stored each MAC as its writer spelled it. A
deletion in the other spelling missed the entry, so the cache kept
answering with the deleted address.
Lookups now match a MAC in any spelling, and a write or a deletion
reaches every spelling of it in the cache file. The file keeps its
keys as written, since nodes not yet upgraded look a MAC up by its
exact spelling. A MAC new to the file is written in lower case.
If two spellings disagree on an address, the cache cannot tell which
is stale. A lookup then ignores them, asks the IPAM and writes its
answer under both.
Adding a dnsmasq reservation checks every line of its file against
the cache and drops the ones that do not match. The check ignores case
as well, since an existing line keeps its writer's spelling.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 3 ++
src/PVE/Network/SDN/Ipams.pm | 72 +++++++++++++++++++++--------
src/test/run_test_vnets_blackbox.pl | 51 +++++++++++++++++++-
3 files changed, 107 insertions(+), 19 deletions(-)
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index 477b700f2429..dc70b9a99a2e 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -53,6 +53,7 @@ sub update_lease {
sub add_ip_mapping {
my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
+ $mac = lc($mac);
my $ethers_file = ethers_file($dhcpid);
my $ethers_tmp_file = "$ethers_file.tmp";
@@ -76,6 +77,8 @@ sub add_ip_mapping {
warn "failed to parse MAC from $dhcpid ethers file on line $line_no: '$line'\n";
next;
}
+ # the cache is read in lower case, a line from before keeps its writer's spelling
+ $parsed_mac = lc($parsed_mac);
my ($parsed_ip4, $parsed_ip6);
if ($parsed_ip2) {
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index ae2c4337ef8d..1476c4236be0 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -35,18 +35,58 @@ sub json_writer {
return encode_json($data);
}
-sub read_macdb {
+# 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());
+}
+
sub write_macdb {
my ($data) = @_;
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 {
+ my ($db, $mac, $family, $ip) = @_;
+
+ my $macs = $db->{macs} //= {};
+ my @keys = grep { lc($_) eq lc($mac) } keys %$macs;
+ $macs->{$_}->{$family} = $ip for (@keys ? @keys : (lc($mac)));
+}
+
sub add_cache_mac_ip {
my ($mac, $ip) = @_;
@@ -54,12 +94,8 @@ sub add_cache_mac_ip {
$macdb_filename,
undef,
sub {
- my $db = read_macdb();
- if (Net::IP::ip_is_ipv4($ip)) {
- $db->{macs}->{$mac}->{ip4} = $ip;
- } else {
- $db->{macs}->{$mac}->{ip6} = $ip;
- }
+ my $db = read_macdb_file();
+ set_cache_entry($db, $mac, Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6', $ip);
write_macdb($db);
},
);
@@ -73,14 +109,14 @@ sub del_cache_mac_ip {
$macdb_filename,
undef,
sub {
- my $db = read_macdb();
- if (Net::IP::ip_is_ipv4($ip)) {
- delete $db->{macs}->{$mac}->{ip4};
- } else {
- delete $db->{macs}->{$mac}->{ip6};
+ my $db = read_macdb_file();
+ my $family = Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6';
+ 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});
}
- delete $db->{macs}->{$mac}
- if !defined($db->{macs}->{$mac}->{ip4}) && !defined($db->{macs}->{$mac}->{ip6});
write_macdb($db);
},
);
@@ -137,7 +173,7 @@ sub get_ips_from_mac {
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}->{$mac};
+ 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})
@@ -159,9 +195,9 @@ sub get_ips_from_mac {
return
if (PVE::Cluster::cfs_file_version($macdb_filename) // '') ne ($version // '');
- my $db = read_macdb();
- $db->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4);
- $db->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6);
+ 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);
write_macdb($db);
},
);
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 927ab16adba9..be4c581e414e 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -194,7 +194,7 @@ $mocked_sdn_ipams->mock(
my ($cfg) = @_;
$test_state->{ipam_config} = $cfg;
},
- read_macdb => sub {
+ read_macdb_file => sub {
return PVE::Network::SDN::Ipams::json_reader(undef, encode_json($test_state->{macdb}));
},
write_macdb => sub {
@@ -498,6 +498,55 @@ sub test_ipam_cache_lookup {
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");
+
+ # 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
+ 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_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
+ $test_state->{macdb}->{macs} = { uc($mac) => { ip4 => '10.0.0.150' } };
+ PVE::Network::SDN::Ipams::add_cache_mac_ip($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",
+ );
+
+ # 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 } };
+ @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_deeply(
+ $test_state->{macdb}->{macs},
+ { uc($mac) => { ip4 => '10.0.0.50', ip6 => undef } },
+ "$test_name: and fills the entry under its spelling",
+ );
+
+ # 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' },
+ };
+ @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",
+ );
}
run_test(\&test_ipam_cache_lookup);
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH pve-network v4 04/13] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it
2026-11-03 18:55 [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (2 preceding siblings ...)
2026-11-03 18:55 ` [PATCH pve-network v4 03/13] sdn: ipam: read the MAC cache case-insensitively Hannes Laimer
@ 2026-11-03 18:55 ` Hannes Laimer
2026-11-03 18:55 ` [PATCH pve-network v4 05/13] sdn: ipam: release an address the built-in IPAM no longer holds Hannes Laimer
` (9 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 18:55 UTC (permalink / raw)
To: pve-devel
Adding a reservation rewrote the zone's dnsmasq reservation file and
dropped every line that no longer matched the MAC cache. Since dnsmasq
answers only MACs with a reservation, the guest behind such a line got
no DHCP answer until its next start. Such a line now gets the
addresses the cache holds for its MAC, and is dropped only when the
cache holds none.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
Reviewed-by: Stefan Hanreich <s.hanreich@proxmox.com>
Tested-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 11 ++++++++++-
src/test/run_test_vnets_blackbox.pl | 28 ++++++++++++++++++++++++++++
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index dc70b9a99a2e..c736d02d8f23 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -91,7 +91,9 @@ sub add_ip_mapping {
}
$parsed_ip6 = $1 if $parsed_ip6 && $parsed_ip6 =~ m/\[(\S+)\]/;
- #delete changed
+ # a line that no longer matches the cache is replaced by the cache's
+ # current one, a dropped one would leave that guest unknown to
+ # dnsmasq until its next start
if (
!defined($macdb->{macs}->{$parsed_mac})
|| ($parsed_ip4
@@ -102,6 +104,13 @@ sub add_ip_mapping {
&& $macdb->{macs}->{$parsed_mac}->{'ip6'} ne $parsed_ip6)
) {
$reload = 1;
+ # the MAC being added gets its line below, like a missing one
+ my $current = $macdb->{macs}->{$parsed_mac};
+ next if $parsed_mac eq $mac || !$current;
+ my $replacement = $parsed_mac;
+ $replacement .= ",$current->{ip4}" if $current->{ip4};
+ $replacement .= ",[$current->{ip6}]" if $current->{ip6};
+ print $out "$replacement\n" if $replacement ne $parsed_mac;
next;
}
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index be4c581e414e..90241268077c 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -1107,4 +1107,32 @@ sub test_update_vnet_without_zone {
}
run_test(\&test_update_vnet_without_zone);
+sub test_dnsmasq_stale_reservation_replaced {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+
+ my $stale = "da:65:8f:18:9b:01";
+ my $gone = "da:65:8f:18:9b:02";
+ my $added = "da:65:8f:18:9b:03";
+ $test_state->{macdb} = {
+ macs => {
+ $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->add_ip_mapping(
+ 'TESTZONE', $test_state->{macdb}, $added, '10.0.1.13', undef,
+ );
+
+ my @lines = sort grep { /\S/ } split(/\n/, PVE::Tools::file_get_contents($TMP_ETHERS_FILE));
+ eq_or_diff(
+ \@lines,
+ ["$stale,10.0.1.11", "$added,10.0.1.13"],
+ "$test_name: the stale line is replaced, the unknown one dropped",
+ );
+}
+
+run_test(\&test_dnsmasq_stale_reservation_replaced);
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH pve-network v4 05/13] sdn: ipam: release an address the built-in IPAM no longer holds
2026-11-03 18:55 [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (3 preceding siblings ...)
2026-11-03 18:55 ` [PATCH pve-network v4 04/13] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it Hannes Laimer
@ 2026-11-03 18:55 ` Hannes Laimer
2026-11-03 18:55 ` [PATCH pve-network v4 06/13] sdn: dhcp: update reservations when IP mappings change Hannes Laimer
` (8 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 18:55 UTC (permalink / raw)
To: pve-devel
The built-in IPAM died on releasing an address it had no record of,
before the cache entry of the address was cleared. So the API could
not delete the mapping, and lookups of its MAC kept returning the
address. NetBox and phpIPAM skip the deletion when they find no record
of the address. The built-in IPAM now warns and does the same, so the
mapping is deleted along with its cache entry.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 7 ++++++-
src/test/run_test_vnets_blackbox.pl | 17 +++++++++++++++++
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index a26da57a877a..77a7b2a7c144 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -265,7 +265,12 @@ sub del_ip {
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});
+ # a record that is gone was released already, and the address still
+ # has to leave the cache, so this is no error
+ if (!defined($dbsubnet->{ips}->{$ip})) {
+ warn "IP '$ip' does not exist in IPAM DB\n";
+ return;
+ }
delete $dbsubnet->{ips}->{$ip};
write_db($db);
},
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 90241268077c..164ef4b4592e 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -547,6 +547,23 @@ sub test_ipam_cache_lookup {
{ uc($mac) => { ip4 => '10.0.0.50' }, $mac => { ip4 => '10.0.0.50' } },
"$test_name: and its write brings both spellings in line",
);
+
+ # a mapping whose record is gone is still deleted, and its cache entry
+ # with it
+ $test_state->{macdb} = {};
+ create_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.70" });
+ delete $test_state->{ipamdb}->{zones}->{$zoneid}->{subnets}->{"10.0.0.0/24"}->{ips}
+ ->{"10.0.0.70"};
+ eval {
+ PVE::API2::Network::SDN::Ips->ipdelete(
+ { zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.70" });
+ };
+ is($@, '', "$test_name: a mapping whose record is gone is deleted without an error");
+ is(
+ (PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac))[0],
+ '10.0.0.50',
+ "$test_name: and the cache no longer answers with its address",
+ );
}
run_test(\&test_ipam_cache_lookup);
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH pve-network v4 06/13] sdn: dhcp: update reservations when IP mappings change
2026-11-03 18:55 [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (4 preceding siblings ...)
2026-11-03 18:55 ` [PATCH pve-network v4 05/13] sdn: ipam: release an address the built-in IPAM no longer holds Hannes Laimer
@ 2026-11-03 18:55 ` Hannes Laimer
2026-11-03 18:55 ` [PATCH pve-network v4 07/13] sdn: ipam: look a MAC up within the vnet's subnets Hannes Laimer
` (7 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 18:55 UTC (permalink / raw)
To: pve-devel
Creating, updating or deleting an IP mapping through the API changed
the IPAM and the MAC cache, but never the zone's dnsmasq reservation
file. dnsmasq could keep serving the old reservation, or none for a
new mapping, until the guest's next start.
A guest start, an API edit and the deletion of a NIC's addresses now
all rewrite the MAC's reservation from the cache. The reservations are
a file on each node, and only the guest's node answers its DHCP
requests. So an API edit is proxied to the node of the guest with that
MAC on the vnet, unless that node is offline. The other nodes catch up
when the guest starts on one of them.
The edit holds the guest's migration lock, so a migration cannot move
the guest during the edit. An edit that meets a migration, or finds
the guest moved to another node, fails before writing anything. If the
reservation update fails, the API edit fails with an error saying the
IPAM is already updated. A guest start and the deletion of a NIC's
addresses only warn.
Each rewrite reads the current cache only once it holds the
reservation file's lock. A rewrite working from an older read would
undo the lines another one had just written. The MAC's line is written
in full from the cache, so a guest gaining an IPv6 address next to its
IPv4 one gets it reserved too.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
debian/control | 2 +
src/PVE/API2/Network/SDN/Ips.pm | 104 +++++++++++--
src/PVE/Network/SDN/Dhcp.pm | 56 ++++---
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 106 +++++++------
src/PVE/Network/SDN/Dhcp/Plugin.pm | 6 +-
src/PVE/Network/SDN/Vnets.pm | 12 +-
src/test/run_test_vnets_blackbox.pl | 232 +++++++++++++++++++++++++++-
7 files changed, 430 insertions(+), 88 deletions(-)
diff --git a/debian/control b/debian/control
index 268f40186b6b..79b8fa1a9264 100644
--- a/debian/control
+++ b/debian/control
@@ -6,6 +6,7 @@ Build-Depends: debhelper-compat (= 13),
lintian,
libfile-slurp-perl,
libpve-common-perl (>= 9.2.4),
+ libpve-guest-common-perl,
libnet-subnet-perl,
libpve-rs-perl (>= 0.15.0),
libtest-mockmodule-perl,
@@ -21,6 +22,7 @@ Homepage: https://www.proxmox.com
Package: libpve-network-perl
Architecture: all
Depends: libpve-common-perl (>= 9.2.4),
+ libpve-guest-common-perl,
pve-cluster (>= 8.0.10),
libio-socket-ssl-perl,
ifupdown2,
diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
index 04741d8750a4..5d39472addf0 100644
--- a/src/PVE/API2/Network/SDN/Ips.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -3,9 +3,13 @@ package PVE::API2::Network::SDN::Ips;
use strict;
use warnings;
+use PVE::Cluster;
+use PVE::GuestHelpers;
+use PVE::INotify;
use PVE::Tools qw(extract_param);
use PVE::Network::SDN::Vnets;
+use PVE::Network::SDN::Zones;
use PVE::Network::SDN::Dhcp;
use PVE::JSONSchema qw(get_standard_option);
@@ -13,12 +17,61 @@ use PVE::RESTHandler;
use base qw(PVE::RESTHandler);
+# only the guest's node answers its DHCP requests, so the edit runs there.
+# With that node offline it runs here, and the node catches up at the
+# guest's next start
+my $proxy_to_guest_node = sub {
+ my ($rpcenv, $proxyto, $param) = @_;
+
+ my $vnet = PVE::Network::SDN::Vnets::get_vnet($param->{vnet}, 1);
+ my $zone = $vnet ? PVE::Network::SDN::Zones::get_zone($vnet->{zone}, 1) : undef;
+ return 'localhost' if !$zone || !$zone->{dhcp};
+
+ my ($node) = PVE::Network::SDN::Dhcp::guest_node_by_mac($param->{vnet}, $param->{mac});
+ my $members = PVE::Cluster::get_members();
+ return 'localhost' if !$node || ($members && !($members->{$node} // {})->{online});
+ return $node;
+};
+
+# the edit runs under the guest's migration lock on its node, so a migration
+# cannot move the guest between the IPAM write and the reservation update,
+# and an edit that finds the guest moved away fails before it writes anything
+my $with_guest_lock = sub {
+ my ($vnet, $mac, $code) = @_;
+
+ my (undef, $vmid) = PVE::Network::SDN::Dhcp::guest_node_by_mac($vnet, $mac);
+ return $code->() if !defined($vmid);
+
+ # an error of the edit itself is kept apart from the lock's own failure
+ my ($res, $edit_err);
+ eval {
+ PVE::GuestHelpers::guest_migration_lock(
+ $vmid,
+ 1,
+ sub {
+ $res = eval {
+ my $target =
+ $proxy_to_guest_node->(undef, undef, { vnet => $vnet, mac => $mac });
+ die "guest $vmid moved to $target, try again\n"
+ if $target ne 'localhost' && $target ne PVE::INotify::nodename();
+ $code->();
+ };
+ $edit_err = $@;
+ },
+ );
+ };
+ die "guest $vmid is locked, maybe by a migration\n" if $@;
+ die $edit_err if $edit_err;
+ return $res;
+};
+
__PACKAGE__->register_method({
name => 'ipdelete',
path => '',
method => 'DELETE',
description => 'Delete IP Mappings in a VNet',
protected => 1,
+ proxyto_callback => $proxy_to_guest_node,
permissions => {
description => "Requires 'SDN.Allocate' on '/sdn/zones/<zone>/<vnet>'",
user => 'all',
@@ -55,8 +108,16 @@ __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); };
- die "$@\n" if $@;
+ $with_guest_lock->(
+ $vnet,
+ $mac,
+ sub {
+ eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
+ die "$@\n" if $@;
+
+ PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+ },
+ );
return undef;
},
@@ -68,6 +129,7 @@ __PACKAGE__->register_method({
method => 'POST',
description => 'Create IP Mapping in a VNet',
protected => 1,
+ proxyto_callback => $proxy_to_guest_node,
permissions => {
description => "Requires 'SDN.Allocate' on '/sdn/zones/<zone>/<vnet>'",
user => 'all',
@@ -104,7 +166,14 @@ __PACKAGE__->register_method({
my $mac = extract_param($param, 'mac');
my $ip = extract_param($param, 'ip');
- PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, undef);
+ $with_guest_lock->(
+ $vnet,
+ $mac,
+ sub {
+ PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, undef);
+ PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+ },
+ );
return undef;
},
@@ -115,6 +184,7 @@ __PACKAGE__->register_method({
method => 'PUT',
description => 'Update IP Mapping in a VNet',
protected => 1,
+ proxyto_callback => $proxy_to_guest_node,
permissions => {
description => "Requires 'SDN.Allocate' on '/sdn/zones/<zone>/<vnet>'",
user => 'all',
@@ -155,19 +225,29 @@ __PACKAGE__->register_method({
my $vmid = extract_param($param, 'vmid');
my $ip = extract_param($param, 'ip');
- 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;
+ $with_guest_lock->(
+ $vnet,
+ $mac,
+ sub {
+ 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);
- PVE::Network::SDN::Vnets::del_ip($vnet, $old_ip, '', $mac);
+ eval { PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, $vmid); };
+ my $error = $@;
- eval { PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, $vmid); };
- my $error = $@;
+ if ($error) {
+ PVE::Network::SDN::Vnets::add_ip($vnet, $old_ip, '', $mac, $vmid);
+ }
- if ($error) {
- PVE::Network::SDN::Vnets::add_ip($vnet, $old_ip, '', $mac, $vmid);
- }
+ die "$error\n" if $error;
+
+ PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+ },
+ );
- die "$error\n" if $error;
return undef;
},
});
diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
index f6ce842c8cc2..8778b62c66e8 100644
--- a/src/PVE/Network/SDN/Dhcp.pm
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -6,7 +6,6 @@ use warnings;
use PVE::Cluster;
use PVE::Network::SDN;
-use PVE::Network::SDN::Ipams;
use PVE::Network::SDN::Subnets;
use PVE::Network::SDN::Dhcp::Plugin;
use PVE::Network::SDN::Dhcp::Dnsmasq;
@@ -22,37 +21,50 @@ sub plugin_types {
return PVE::Network::SDN::Dhcp::Plugin->lookup_types();
}
-sub add_mapping {
- my ($vnetid, $mac, $ip4, $ip6) = @_;
+sub update_mapping {
+ my ($vnetid, $mac) = @_;
- my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid);
- return if !$vnet;
+ # a record without a MAC has no guest NIC behind it, so no backend serves it
+ return if !$mac;
- my $zoneid = $vnet->{zone};
- my $zone = PVE::Network::SDN::Zones::get_zone($zoneid);
+ eval {
+ # the backend to push to is the applied one, since a zone edited but
+ # not applied has nothing running yet
+ my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid, 1);
+ return if !$vnet;
- return if !$zone->{ipam} || !$zone->{dhcp};
+ my $zoneid = $vnet->{zone};
+ my $zone = PVE::Network::SDN::Zones::get_zone($zoneid, 1);
+ return if !$zone || !$zone->{ipam} || !$zone->{dhcp};
- my $dhcptype = $zone->{dhcp};
+ # a zone confined to other nodes has no vnet bridge on this node
+ return if defined($zone->{nodes}) && !$zone->{nodes}->{ PVE::INotify::nodename() };
- my $macdb = PVE::Network::SDN::Ipams::read_macdb();
- my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($dhcptype);
- $dhcp_plugin->add_ip_mapping($zoneid, $macdb, $mac, $ip4, $ip6);
+ PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp})
+ ->update_ip_mapping($zoneid, $mac);
+ };
+ # the records are written by then and stay
+ die "IPAM is updated for $mac, but DHCP is not: $@" if $@;
}
-sub remove_mapping {
+sub guest_node_by_mac {
my ($vnetid, $mac) = @_;
- my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid);
- return if !$vnet;
-
- my $zoneid = $vnet->{zone};
- my $zone = PVE::Network::SDN::Zones::get_zone($zoneid);
-
- return if !$zone->{ipam} || !$zone->{dhcp};
+ my $vmlist = PVE::Cluster::get_vmlist();
+ my $nets = PVE::Cluster::get_guest_config_properties([map { "net$_" } 0 .. 31]);
+ for my $vmid (sort keys %$nets) {
+ my $guest = $vmlist->{ids}->{$vmid} // next;
+ for my $net (values %{ $nets->{$vmid} }) {
+ my ($bridge) = $net =~ m/(?:^|,)bridge=([^,]+)/ or next;
+ next if $bridge ne $vnetid;
+ # a VM names the MAC as the value of its model, a container as hwaddr
+ my ($nic_mac) = $net =~ m/(?:^|,)[a-z0-9_-]+=([0-9a-f]{2}(?::[0-9a-f]{2}){5})(?:,|$)/i
+ or next;
+ return ($guest->{node}, $vmid) if lc($nic_mac) eq lc($mac);
+ }
+ }
- my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp});
- $dhcp_plugin->del_ip_mapping($zoneid, $mac);
+ return;
}
sub regenerate_config {
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index c736d02d8f23..dbc136190586 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -11,7 +11,9 @@ use PVE::Tools qw(file_set_contents run_command lock_file);
use File::Copy;
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';
@@ -51,83 +53,100 @@ sub update_lease {
$manager->AddDhcpLease($ip4, $mac, \@hostname, undef, 0, 0, 0) if $ip4;
}
-sub add_ip_mapping {
- my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
+sub update_ip_mapping {
+ my ($class, $dhcpid, $mac) = @_;
$mac = lc($mac);
my $ethers_file = ethers_file($dhcpid);
my $ethers_tmp_file = "$ethers_file.tmp";
- my $reload = undef;
+ my ($ip4, $ip6);
+ my $reservation = undef;
+ my $changed = undef;
- my $appendFn = sub {
+ 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
+ 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;
+ # an external IPAM spells addresses its own way, while the file holds one spelling
+ $reservation = lc($reservation);
+ }
+
+ 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 $match = undef;
+ my $kept = undef;
- my $line_no = 0;
while (my $line = <$in>) {
- $line_no++;
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
+ $parsed_mac = lc($parsed_mac);
- if (!defined($parsed_mac)) {
- warn "failed to parse MAC from $dhcpid ethers file on line $line_no: '$line'\n";
+ # 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;
}
- # the cache is read in lower case, a line from before keeps its writer's spelling
- $parsed_mac = lc($parsed_mac);
my ($parsed_ip4, $parsed_ip6);
if ($parsed_ip2) {
$parsed_ip4 = $parsed_ip1;
$parsed_ip6 = $parsed_ip2;
- } elsif (Net::IP::ip_is_ipv4($parsed_ip1)) {
+ } elsif (defined($parsed_ip1) && Net::IP::ip_is_ipv4($parsed_ip1)) {
$parsed_ip4 = $parsed_ip1;
- } else {
+ } 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, a dropped one would leave that guest unknown to
# dnsmasq until its next start
+ my $current = $macs->{$parsed_mac};
if (
- !defined($macdb->{macs}->{$parsed_mac})
- || ($parsed_ip4
- && $macdb->{macs}->{$parsed_mac}->{'ip4'}
- && $macdb->{macs}->{$parsed_mac}->{'ip4'} ne $parsed_ip4)
- || ($parsed_ip6
- && $macdb->{macs}->{$parsed_mac}->{'ip6'}
- && $macdb->{macs}->{$parsed_mac}->{'ip6'} ne $parsed_ip6)
+ !$current
+ || ($parsed_ip4 && $current->{ip4} && $current->{ip4} ne $parsed_ip4)
+ || ($parsed_ip6 && $current->{ip6} && lc($current->{ip6}) ne $parsed_ip6)
) {
- $reload = 1;
- # the MAC being added gets its line below, like a missing one
- my $current = $macdb->{macs}->{$parsed_mac};
- next if $parsed_mac eq $mac || !$current;
+ $changed = 1;
+ next if !$current;
my $replacement = $parsed_mac;
$replacement .= ",$current->{ip4}" if $current->{ip4};
$replacement .= ",[$current->{ip6}]" if $current->{ip6};
- print $out "$replacement\n" if $replacement ne $parsed_mac;
+ print $out lc($replacement) . "\n";
next;
}
- if ($parsed_mac eq $mac) {
- $match = 1 if $ip4 && $parsed_ip4 && $ip4;
- $match = 1 if $ip6 && $parsed_ip6 && $ip6;
- }
-
print $out "$line\n";
}
- if (!$match) {
- my $reservation = $mac;
- $reservation .= ",$ip4" if $ip4;
- $reservation .= ",[$ip6]" if $ip6;
+ if (defined($reservation) && !$kept) {
print $out "$reservation\n";
- $reload = 1;
+ $changed = 1;
}
close $in;
@@ -136,16 +155,17 @@ sub add_ip_mapping {
chmod 0644, $ethers_file;
};
- PVE::Tools::lock_file($ethers_file, 10, $appendFn);
+ # 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);
+ die "Unable to update $mac in the dnsmasq configuration: $@" if $@;
- if ($@) {
- warn "Unable to add $mac to the dnsmasq configuration: $@\n";
- return;
- }
+ systemctl_service('reload', "dnsmasq\@$dhcpid") if $changed;
- my $service_name = "dnsmasq\@$dhcpid";
- systemctl_service('reload', $service_name) if $reload;
- update_lease($dhcpid, $ip4, $mac);
+ if ($ip4) {
+ eval { update_lease($dhcpid, $ip4, $mac) };
+ log_warn("could not update the dnsmasq lease of $mac: $@") if $@;
+ }
}
sub configure_subnet {
diff --git a/src/PVE/Network/SDN/Dhcp/Plugin.pm b/src/PVE/Network/SDN/Dhcp/Plugin.pm
index b5d32fa13c70..472ff1534743 100644
--- a/src/PVE/Network/SDN/Dhcp/Plugin.pm
+++ b/src/PVE/Network/SDN/Dhcp/Plugin.pm
@@ -22,8 +22,10 @@ sub private {
return $defaultData;
}
-sub add_ip_mapping {
- my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
+# 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
+sub update_ip_mapping {
+ my ($class, $dhcpid, $mac) = @_;
die 'implement in sub class';
}
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index ea66aa2177e1..56516ed9de83 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -214,6 +214,12 @@ sub del_ips_from_mac {
PVE::Network::SDN::Vnets::del_ip($vnetid, $ip4, $hostname, $mac) if $ip4;
PVE::Network::SDN::Vnets::del_ip($vnetid, $ip6, $hostname, $mac) if $ip6;
+ if ($ip4 || $ip6) {
+ # a NIC going away does not fail on its DHCP backend
+ eval { PVE::Network::SDN::Dhcp::update_mapping($vnetid, $mac) };
+ warn $@ if $@;
+ }
+
return ($ip4, $ip6);
}
@@ -228,11 +234,15 @@ sub add_dhcp_mapping {
return if !$zone->{ipam} || !$zone->{dhcp};
my ($ip4, $ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ # one push at the end covers the allocated and the existing records alike
add_next_free_cidr($vnetid, $name, $mac, "$vmid", undef, 1, 4) if !$ip4;
add_next_free_cidr($vnetid, $name, $mac, "$vmid", undef, 1, 6) if !$ip6;
($ip4, $ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
- PVE::Network::SDN::Dhcp::add_mapping($vnetid, $mac, $ip4, $ip6) if $ip4 || $ip6;
+ return if !$ip4 && !$ip6;
+ # a guest start does not fail on its DHCP backend
+ eval { PVE::Network::SDN::Dhcp::update_mapping($vnetid, $mac) };
+ warn $@ if $@;
}
=head3 api_get_vnet($cfg, $id)
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 164ef4b4592e..04776ea2f8af 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -47,6 +47,7 @@ sub clear_test_state {
vnets_config => {},
macdb => {},
ipamdb => {},
+ dnsmasq_calls => [],
ipam_config => {
'ids' => {
'pve' => {
@@ -222,7 +223,9 @@ $mocked_sdn_dhcp_dnsmasq->mock(
assert_dnsmasq_installed => sub { return 1; },
before_configure => sub { },
ethers_file => sub { return "/tmp/ethers"; },
- systemctl_service => sub { },
+ systemctl_service => sub {
+ push $test_state->{dnsmasq_calls}->@*, $_[0];
+ },
update_lease => sub { },
);
@@ -254,10 +257,26 @@ $mocked_rpc_env_obj->mock(
check_any => sub { return 1; },
);
+my $mocked_pve_inotify = Test::MockModule->new('PVE::INotify');
+$mocked_pve_inotify->mock(
+ nodename => sub { return 'localnode'; },
+);
+
+my $mocked_guest_helpers = Test::MockModule->new('PVE::GuestHelpers');
+$mocked_guest_helpers->mock(
+ guest_migration_lock => sub {
+ my ($vmid, $timeout, $code) = @_;
+ return $code->();
+ },
+);
+
my $mocked_pve_cluster_obj = Test::MockModule->new('PVE::Cluster');
$mocked_pve_cluster_obj->mock(
check_cfs_quorum => sub { return 1; },
cfs_lock_domain => $mocked_cfs_lock_domain,
+ cfs_update => sub { },
+ get_vmlist => sub { return $test_state->{vmlist} // { ids => {} }; },
+ get_guest_config_properties => sub { return $test_state->{guest_nets} // {}; },
);
# ------- TEST FUNCTIONS --------------
@@ -303,6 +322,11 @@ sub create_zone {
return $zone;
}
+sub update_zone {
+ my ($zoneid, $params) = @_;
+ PVE::API2::Network::SDN::Zones->update({ zone => $zoneid, %$params });
+}
+
sub get_vnet {
my ($id) = @_;
return eval { PVE::API2::Network::SDN::Vnets->read({ vnet => $id }); };
@@ -373,6 +397,16 @@ sub create_ip {
return PVE::API2::Network::SDN::Ips->ipcreate($param);
}
+sub update_ip {
+ my ($param) = @_;
+ return PVE::API2::Network::SDN::Ips->ipupdate($param);
+}
+
+sub delete_ip {
+ my ($param) = @_;
+ return PVE::API2::Network::SDN::Ips->ipdelete($param);
+}
+
sub run_test {
my $test = shift;
clear_test_state();
@@ -1138,18 +1172,200 @@ sub test_dnsmasq_stale_reservation_replaced {
};
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->add_ip_mapping(
- 'TESTZONE', $test_state->{macdb}, $added, '10.0.1.13', undef,
- );
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping('TESTZONE', $added);
- my @lines = sort grep { /\S/ } split(/\n/, PVE::Tools::file_get_contents($TMP_ETHERS_FILE));
- eq_or_diff(
- \@lines,
- ["$stale,10.0.1.11", "$added,10.0.1.13"],
+ 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",
);
}
run_test(\&test_dnsmasq_stale_reservation_replaced);
+# -------------- dnsmasq mapping pushes
+
+sub test_dnsmasq_mapping_push {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+ # the first push of a zone finds no ethers file yet
+ unlink($TMP_ETHERS_FILE);
+
+ my $ethers = sub { PVE::Tools::file_get_contents($TMP_ETHERS_FILE) };
+
+ eval { nic_start($vnetid, $mac, "999", "testhostname"); };
+ if ($@) {
+ fail("$test_name: nic_start: $@");
+ return;
+ }
+ is($ethers->(), "$mac,10.0.0.100\n", "$test_name: guest start reserves the allocation");
+
+ # the records exist already, so the restart pushes them without a change
+ $test_state->{dnsmasq_calls} = [];
+ nic_start($vnetid, $mac, "999", "testhostname");
+ is($ethers->(), "$mac,10.0.0.100\n", "$test_name: restart keeps the reservation");
+ is_deeply(
+ $test_state->{dnsmasq_calls},
+ [],
+ "$test_name: restart without a change does not reload",
+ );
+
+ update_ip({
+ zone => $zoneid,
+ vnet => $vnetid,
+ mac => $mac,
+ ip => "10.0.0.150",
+ });
+ is($ethers->(), "$mac,10.0.0.150\n", "$test_name: mapping edit replaces the reservation");
+
+ delete_ip({
+ zone => $zoneid,
+ vnet => $vnetid,
+ mac => $mac,
+ ip => "10.0.0.150",
+ });
+ is($ethers->(), "", "$test_name: mapping delete drops the reservation");
+
+ # a record without a MAC has no NIC to serve, so nothing is pushed for it
+ my @warnings;
+ {
+ local $SIG{__WARN__} = sub { push @warnings, @_; };
+ create_ip({ zone => $zoneid, vnet => $vnetid, ip => "10.0.0.160" });
+ delete_ip({ zone => $zoneid, vnet => $vnetid, ip => "10.0.0.160" });
+ }
+ eq_or_diff(\@warnings, [], "$test_name: a record without a MAC warns nothing");
+
+ # a reservation of a MAC the cache does not know is swept by the next
+ # rewrite, and both addresses of the guest go on one line
+ create_dhcp_subnet($vnetid, "8888::/64");
+ PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "aa:bb:cc:dd:ee:ff,10.0.0.77\n");
+ nic_start($vnetid, $mac, "999", "testhostname");
+ is(
+ $ethers->(),
+ "$mac,10.0.0.100,[8888::64]\n",
+ "$test_name: dual-stack reservation on one line, the unknown MAC swept",
+ );
+
+ # a zone confined to other nodes runs no dnsmasq here, so an edit is not
+ # pushed on this node
+ update_zone($zoneid, { nodes => 'other' });
+ create_ip({
+ zone => $zoneid,
+ vnet => $vnetid,
+ mac => $mac,
+ ip => "10.0.0.150",
+ });
+ is(
+ $ethers->(),
+ "$mac,10.0.0.100,[8888::64]\n",
+ "$test_name: a zone confined to other nodes gets no push here",
+ );
+}
+
+run_test(\&test_dnsmasq_mapping_push);
+
+sub test_failed_mapping_push {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+ $mocked_sdn_dhcp_dnsmasq->mock(update_ip_mapping => sub { die "rewrite failed\n"; });
+
+ my @warnings;
+ {
+ local $SIG{__WARN__} = sub { push @warnings, @_; };
+ eval { nic_start($vnetid, $mac, "999", "testhostname"); };
+ is($@, '', "$test_name: a guest start does not fail on its backend");
+ }
+ like(join('', @warnings), qr/rewrite failed/, "$test_name: the guest start warns");
+
+ eval { update_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.150" }); };
+ like(
+ $@,
+ qr/IPAM is updated for \Q$mac\E, but DHCP is not: rewrite failed/,
+ "$test_name: an API edit fails",
+ );
+ is(get_ip4(get_ips_from_mac($mac)), "10.0.0.150", "$test_name: the edit is saved");
+
+ $mocked_sdn_dhcp_dnsmasq->unmock('update_ip_mapping');
+}
+
+run_test(\&test_failed_mapping_push);
+
+sub test_mapping_edit_target {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+ # the guest behind a MAC is found by its NIC on the vnet, and the config
+ # writes the MAC in upper case
+ $test_state->{vmlist} = {
+ ids => {
+ 100 => { type => 'qemu', node => 'other' },
+ },
+ };
+ $test_state->{guest_nets} = {
+ 1 => { net0 => "virtio=$mac,bridge=$vnetid" },
+ 100 => { net0 => "virtio=" . uc($mac) . ",bridge=$vnetid" },
+ };
+ is_deeply(
+ [PVE::Network::SDN::Dhcp::guest_node_by_mac($vnetid, $mac)],
+ ['other', 100],
+ "$test_name: the MAC resolves to the node of its guest, a guest the list lacks is skipped",
+ );
+
+ # a mapping edit through the API runs on the guest's node
+ my $proxyto = PVE::API2::Network::SDN::Ips->map_method_by_name('ipupdate')->{proxyto_callback};
+ is(
+ $proxyto->(undef, undef, { zone => $zoneid, vnet => $vnetid, mac => $mac }),
+ 'other',
+ "$test_name: a mapping edit runs on the guest's node",
+ );
+ is(
+ $proxyto->(undef, undef, { zone => $zoneid, vnet => $vnetid, mac => '00:11:22:33:44:66' }),
+ 'localhost',
+ "$test_name: one for a MAC without a guest runs here",
+ );
+
+ # the edit holds the guest's migration lock, a migration holding it fails
+ # the edit, and a guest found on another node under the lock fails it too
+ $test_state->{vmlist}->{ids}->{100}->{node} = 'localnode';
+ create_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.50" });
+ $mocked_guest_helpers->mock(
+ guest_migration_lock => sub { die "can't lock file '/var/lock/x' - got timeout\n" });
+ eval { update_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.60" }) };
+ like(
+ $@,
+ qr/^guest 100 is locked, maybe by a migration$/,
+ "$test_name: an edit during a migration fails",
+ );
+ $mocked_guest_helpers->mock(
+ guest_migration_lock => sub { my ($vmid, $timeout, $code) = @_; return $code->(); });
+ $test_state->{vmlist}->{ids}->{100}->{node} = 'other';
+ eval { update_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.60" }) };
+ like($@, qr/guest 100 moved to other/, "$test_name: an edit for a guest found elsewhere fails");
+ is(get_ip4(get_ips_from_mac($mac)), "10.0.0.50", "$test_name: and wrote nothing");
+ $test_state->{vmlist}->{ids}->{100}->{node} = 'localnode';
+ update_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.60" });
+ is(
+ get_ip4(get_ips_from_mac($mac)),
+ "10.0.0.60",
+ "$test_name: one for a guest here goes through",
+ );
+ delete $test_state->{vmlist};
+ delete $test_state->{guest_nets};
+}
+
+run_test(\&test_mapping_edit_target);
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH pve-network v4 07/13] sdn: ipam: look a MAC up within the vnet's subnets
2026-11-03 18:55 [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (5 preceding siblings ...)
2026-11-03 18:55 ` [PATCH pve-network v4 06/13] sdn: dhcp: update reservations when IP mappings change Hannes Laimer
@ 2026-11-03 18:55 ` Hannes Laimer
2026-11-03 18:55 ` [PATCH pve-network v4 08/13] sdn: ipam: keep the MAC cache per vnet Hannes Laimer
` (6 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 18:55 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH pve-network v4 08/13] sdn: ipam: keep the MAC cache per vnet
2026-11-03 18:55 [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (6 preceding siblings ...)
2026-11-03 18:55 ` [PATCH pve-network v4 07/13] sdn: ipam: look a MAC up within the vnet's subnets Hannes Laimer
@ 2026-11-03 18:55 ` Hannes Laimer
2026-11-03 18:55 ` [PATCH pve-network v4 09/13] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM Hannes Laimer
` (5 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 18:55 UTC (permalink / raw)
To: pve-devel
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. A zone's dnsmasq
reservations hold a line for each vnet a MAC sits on. A push of a
MAC's reservation looks the MAC up and checks the other lines against
the cache alone. A push can also be limited to the cache, for a caller
that cannot wait on the IPAM. The built-in IPAM matches a MAC in any
spelling, since a push looks the MAC up in lower case.
The map under the MAC alone stays, since nodes not yet upgraded read
and write only that one. Every address written to the cache goes there
as well, one address per family and MAC as before. An entry there
answers for a vnet without an entry of its own, but only with an
address in that vnet's subnets. So a guest started before the upgrade
keeps its reservation.
A release or an edit on a node not yet upgraded shows only in that
map. A vnet's cached address is therefore used only while that map
still names it. Otherwise the lookup asks the IPAM again. A line
checked against the cache stays unchanged when that map no longer
names its cached address.
A node not yet upgraded may also have released a MAC's address and
given it to another guest. So a release now leaves an IPAM record held
by another MAC in place, with its DNS records, and fails when it
cannot check the holder. Deleting such a mapping through the API
clears the MAC's cache entry and fails.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/API2/Network/SDN/Ips.pm | 10 +-
src/PVE/Network/SDN/Dhcp.pm | 4 +-
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 153 +++++-----
src/PVE/Network/SDN/Dhcp/Plugin.pm | 8 +-
src/PVE/Network/SDN/Ipams.pm | 196 ++++++++----
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 | 328 ++++++++++++++++++---
13 files changed, 646 insertions(+), 178 deletions(-)
diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
index 5d39472addf0..0a41b65ea964 100644
--- a/src/PVE/API2/Network/SDN/Ips.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -112,10 +112,13 @@ __PACKAGE__->register_method({
$vnet,
$mac,
sub {
- 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;
},
);
@@ -233,12 +236,13 @@ __PACKAGE__->register_method({
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 8778b62c66e8..cc9cf0030dc8 100644
--- a/src/PVE/Network/SDN/Dhcp.pm
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -22,7 +22,7 @@ sub plugin_types {
}
sub update_mapping {
- my ($vnetid, $mac) = @_;
+ my ($vnetid, $mac, $cached_only) = @_;
# a record without a MAC has no guest NIC behind it, so no backend serves it
return if !$mac;
@@ -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, $cached_only);
};
# 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 dbc136190586..d67a1ad55819 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,35 +53,71 @@ sub update_lease {
}
sub update_ip_mapping {
- my ($class, $dhcpid, $mac) = @_;
+ my ($class, $dhcpid, $vnetid, $mac, $cached_only) = @_;
$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_unknown);
+ if ($cached_only) {
+ $cache = PVE::Network::SDN::Ipams::read_cache();
+ ($own, $ip4) = $reservation->($vnetid, $mac, 1)
+ if PVE::Network::SDN::Ipams::cache_witnessed($cache, $vnetid, $mac);
+ # a cache holding nothing for the MAC says nothing about its line
+ $own_unknown = !defined($own);
+ } else {
+ ($own, $ip4) = $reservation->($vnetid, $mac);
+ $cache = PVE::Network::SDN::Ipams::read_cache();
}
file_set_contents($ethers_file, '') if !-e $ethers_file;
@@ -90,62 +125,44 @@ sub update_ip_mapping {
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, a dropped one would leave 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, a dropped one would
+ # leave the guest unknown to dnsmasq until its next start. A line
+ # whose cache entry is not vouched for stays as it is, the IPAM's
+ # answer replaces it at that MAC's next push
+ my ($want) = eval {
+ !$key || $written{$key} ? ()
+ : $key eq "$mac,$vnetid" ? ($own_unknown ? (lc($line)) : ($own))
+ : !PVE::Network::SDN::Ipams::cache_witnessed($cache, $vnet, $parsed_mac)
+ ? (lc($line))
+ : $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 +174,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..38264d6fd2b3 100644
--- a/src/PVE/Network/SDN/Dhcp/Plugin.pm
+++ b/src/PVE/Network/SDN/Dhcp/Plugin.pm
@@ -22,10 +22,12 @@ 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. With cached_only the backend writes what the
+# cache holds and asks the IPAM for nothing
sub update_ip_mapping {
- my ($class, $dhcpid, $mac) = @_;
+ my ($class, $dhcpid, $vnetid, $mac, $cached_only) = @_;
die 'implement in sub class';
}
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 9a31eaf39f58..d8d07ee66f01 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,79 @@ 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. The
+# same address on another vnet of the MAC leaves that map unable to say which
+# vnet's it names, so neither is trusted
+sub cache_witnessed {
+ my ($cache, $vnetid, $mac) = @_;
+
+ my $vnets = $cache->{db}->{vnets} // {};
+ my $entry = ($vnets->{$vnetid} // {})->{ lc($mac) } // {};
+ my $old = $cache->{old}->{ lc($mac) } // {};
+ for my $family (grep { defined($entry->{$_}) } qw(ip4 ip6)) {
+ my $ip = lc($entry->{$family});
+ return 0 if lc($old->{$family} // '') ne $ip;
+ for my $other (grep { $_ ne $vnetid } keys %$vnets) {
+ my $other_entry = $vnets->{$other}->{ lc($mac) } // next;
+ return 0 if lc($other_entry->{$family} // '') eq $ip;
+ }
+ }
+ 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 +169,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 +244,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 && cache_witnessed($cache, $vnetid, $mac);
my $plugin_config = get_plugin_config($zone);
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
@@ -195,9 +277,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 bc418f691fa6..f4eed3346670 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 {
@@ -513,74 +513,102 @@ 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
@@ -1164,21 +1192,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);
@@ -1389,8 +1436,219 @@ 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",
+ );
+
+ # a push from the cache alone asks the IPAM for nothing, so a cache
+ # holding nothing for the MAC leaves its line as it is
+ $test_state->{macdb} = {};
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, 'vnet1', $mac, 1);
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.51"],
+ "$test_name: a push from the cache alone keeps the line of a MAC it does not hold",
+ );
+}
+
+run_test(\&test_dnsmasq_reservation_per_vnet);
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH pve-network v4 09/13] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM
2026-11-03 18:55 [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (7 preceding siblings ...)
2026-11-03 18:55 ` [PATCH pve-network v4 08/13] sdn: ipam: keep the MAC cache per vnet Hannes Laimer
@ 2026-11-03 18:55 ` Hannes Laimer
2026-11-03 18:55 ` [PATCH qemu-server v4 10/13] network: sdn: push the DHCP mappings of a guest when it resumes Hannes Laimer
` (4 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 18:55 UTC (permalink / raw)
To: pve-devel
Two zones can use the same subnet, and an external IPAM asked by the
address alone cannot tell their records apart. With a new option of
the IPAM entry, the subnets added to a zone go into the zone's own
namespace, a VRF in NetBox and a sub-section of the configured
section in phpIPAM. The option is off by default, since a node not
yet upgraded does not know the namespaces and keeps looking records
up by the address alone. A namespace is named after its zone and
created with its first subnet. A second option sets the start of the
names, pve by default, so clusters sharing an IPAM can keep theirs
apart.
An existing subnet is looked for in the zone's namespace first and in
the global table or the configured section after it. So subnets from
before are still found and nothing has to be moved. With the
namespaces on, adding a subnet that the global table or the
configured section already holds fails, since a copy in the zone's
namespace would hide its records. phpIPAM nests sections one level
deep, so a configured section that is a sub-section itself is shared
by every zone as before.
The phpIPAM lookup by MAC, unimplemented until now, returns the
addresses recorded for the MAC in the vnet's subnets. In phpIPAM a
failed lookup stops the operation unless the answer is not found, so
a failure cannot send the zone's records to the configured section.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 194 +++++--
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 223 ++++++--
src/PVE/Network/SDN/Ipams/Plugin.pm | 17 +
src/test/ipams/netbox/expected.add_ip | 2 +-
.../ipams/netbox/expected.add_ip_notgateway | 2 +-
src/test/ipams/netbox/expected.update_ip | 2 +-
src/test/run_test_ipams.pl | 533 ++++++++++++++++++
7 files changed, 890 insertions(+), 83 deletions(-)
diff --git a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
index 84ce4382e0e7..30905aaef9ff 100644
--- a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
@@ -21,6 +21,8 @@ sub options {
url => { optional => 0 },
token => { optional => 0 },
fingerprint => { optional => 1 },
+ 'zone-namespaces' => { optional => 1 },
+ 'zone-namespace-prefix' => { optional => 1 },
};
}
@@ -40,18 +42,15 @@ sub netbox_api_request {
}
sub add_dhcp_range {
- my ($config, $dhcp_range, $noerr) = @_;
+ my ($config, $dhcp_range, $vrf_id, $noerr) = @_;
my $result = eval {
- netbox_api_request(
- $config,
- "POST",
- "/ipam/ip-ranges/",
- {
- start_address => $dhcp_range->{'start-address'},
- end_address => $dhcp_range->{'end-address'},
- },
- );
+ my $params = {
+ start_address => $dhcp_range->{'start-address'},
+ end_address => $dhcp_range->{'end-address'},
+ };
+ $params->{vrf} = $vrf_id if defined($vrf_id);
+ netbox_api_request($config, "POST", "/ipam/ip-ranges/", $params);
};
if ($@) {
return if $noerr;
@@ -72,26 +71,60 @@ sub del_dhcp_range {
# Plugin implementation
+my sub zone_vrf_name {
+ my ($plugin_config, $zoneid) = @_;
+
+ my $prefix = $plugin_config->{'zone-namespace-prefix'} // 'pve';
+ return "$prefix-$zoneid";
+}
+
sub add_subnet {
my ($class, $plugin_config, $subnetid, $subnet, $noerr) = @_;
my $cidr = $subnet->{cidr};
my $gateway = $subnet->{gateway};
- if (get_prefix_id($plugin_config, $cidr, $noerr)) {
+ my ($vrf_id, $taken_over);
+ if ($plugin_config->{'zone-namespaces'}) {
+ # a prefix already in the zone's VRF is used, one in the global table
+ # is not taken over, since a copy in the VRF would hide its records
+ $vrf_id = get_zone_namespace($plugin_config, $subnet->{zone}, $noerr);
+ $taken_over = defined($vrf_id)
+ && defined(get_prefix_id($plugin_config, $cidr, $vrf_id, $noerr));
+ if (defined(get_prefix_id($plugin_config, $cidr, undef, $noerr))) {
+ return if $noerr;
+ my $vrf = zone_vrf_name($plugin_config, $subnet->{zone});
+ die "prefix $cidr exists in the global table of NetBox, move it with its"
+ . " addresses and ranges into the VRF of its zone, $vrf for this one, or"
+ . " remove it first\n";
+ }
+
+ $vrf_id //= add_zone_namespace($plugin_config, $subnet->{zone}, $noerr);
+ return if !defined($vrf_id) && $noerr;
+ } elsif (defined(get_prefix_id($plugin_config, $cidr, undef, $noerr))) {
return if $noerr;
die "prefix $cidr already exists in netbox";
}
- eval { netbox_api_request($plugin_config, "POST", "/ipam/prefixes/", { prefix => $cidr }); };
- if ($@) {
- return if $noerr;
- die "error adding subnet to ipam: $@";
+ if (!$taken_over) {
+ eval {
+ my $params = { prefix => $cidr };
+ $params->{vrf} = $vrf_id if defined($vrf_id);
+ netbox_api_request($plugin_config, "POST", "/ipam/prefixes/", $params);
+ };
+ if ($@) {
+ return if $noerr;
+ die "error adding subnet to ipam: $@";
+ }
}
my $dhcp_ranges = PVE::Network::SDN::Subnets::get_dhcp_ranges($subnet);
for my $dhcp_range (@$dhcp_ranges) {
- add_dhcp_range($plugin_config, $dhcp_range, $noerr);
+ # a prefix moved into the VRF came with its ranges
+ next
+ if $taken_over
+ && defined(get_iprange_id($plugin_config, $dhcp_range, $vrf_id, $noerr));
+ add_dhcp_range($plugin_config, $dhcp_range, $vrf_id, $noerr);
}
}
@@ -122,6 +155,8 @@ sub update_subnet {
my $to_delete_ids = ();
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr, 1) or return;
+
# delete first so we don't get errors with overlapping ranges
for my $dhcp_range (@$old_dhcp_ranges) {
my $hash = $hash_range->($dhcp_range);
@@ -130,7 +165,7 @@ sub update_subnet {
next;
}
- my $internalid = get_iprange_id($plugin_config, $dhcp_range, $noerr);
+ my $internalid = get_iprange_id($plugin_config, $dhcp_range, $vrf_id, $noerr);
# definedness check, because ID could be 0
if (!defined($internalid)) {
@@ -145,7 +180,7 @@ sub update_subnet {
for my $dhcp_range (@$new_dhcp_ranges) {
my $hash = $hash_range->($dhcp_range);
- add_dhcp_range($plugin_config, $dhcp_range, $noerr)
+ add_dhcp_range($plugin_config, $dhcp_range, $vrf_id, $noerr)
if !exists($old_lookup->{$hash});
}
}
@@ -155,7 +190,7 @@ sub del_subnet {
my $cidr = $subnet->{cidr};
- my $internalid = get_prefix_id($plugin_config, $cidr, $noerr);
+ my ($internalid, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr);
# definedness check, because ID could be 0
if (!defined($internalid)) {
@@ -163,7 +198,7 @@ sub del_subnet {
return;
}
- if (!is_prefix_empty($plugin_config, $cidr, $noerr)) {
+ if (!is_prefix_empty($plugin_config, $cidr, $vrf_id, $noerr)) {
return if $noerr;
die "not deleting prefix $cidr because it still contains entries";
}
@@ -176,7 +211,7 @@ sub del_subnet {
my $dhcp_ranges = PVE::Network::SDN::Subnets::get_dhcp_ranges($subnet);
for my $dhcp_range (@$dhcp_ranges) {
- my $internalid = get_iprange_id($plugin_config, $dhcp_range, $noerr);
+ my $internalid = get_iprange_id($plugin_config, $dhcp_range, $vrf_id, $noerr);
# definedness check, because ID could be 0
if (!defined($internalid)) {
@@ -215,6 +250,8 @@ sub add_ip {
$description = "mac:$mac";
}
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr, 1) or return;
+
eval {
my $params = {
address => "$ip/$mask",
@@ -222,6 +259,7 @@ sub add_ip {
};
$params->{dns_name} = $hostname if $hostname;
+ $params->{vrf} = $vrf_id if defined($vrf_id);
netbox_api_request($plugin_config, "POST", "/ipam/ip-addresses/", $params);
};
@@ -229,7 +267,7 @@ sub add_ip {
if ($@) {
if ($is_gateway) {
die "error add subnet ip to ipam: ip $ip already exist: $@"
- if !is_ip_gateway($plugin_config, $ip, $noerr);
+ if !is_ip_gateway($plugin_config, $ip, $vrf_id, $noerr);
} elsif (!$noerr) {
die "error add subnet ip to ipam: ip already exist: $@";
}
@@ -259,7 +297,8 @@ sub update_ip {
$description = "mac:$mac";
}
- my $ip_id = get_ip_id($plugin_config, $ip, $noerr);
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr, 1) or return;
+ my $ip_id = get_ip_id($plugin_config, $ip, $vrf_id, $noerr);
# definedness check, because ID could be 0
if (!defined($ip_id)) {
@@ -274,6 +313,7 @@ sub update_ip {
};
$params->{dns_name} = $hostname if $hostname;
+ $params->{vrf} = $vrf_id if defined($vrf_id);
netbox_api_request($plugin_config, "PATCH", "/ipam/ip-addresses/$ip_id/", $params);
};
@@ -287,13 +327,7 @@ sub add_next_freeip {
my $cidr = $subnet->{cidr};
- my $internalid = get_prefix_id($plugin_config, $cidr, $noerr);
-
- # definedness check, because ID could be 0
- if (!defined($internalid)) {
- return if $noerr;
- die "could not find id for prefix $cidr";
- }
+ my ($internalid) = subnet_prefix($plugin_config, $subnet, $noerr, 1) or return;
my $description = undef;
$description = "mac:$mac" if $mac;
@@ -323,7 +357,8 @@ sub add_next_freeip {
sub add_range_next_freeip {
my ($class, $plugin_config, $subnet, $range, $data, $noerr) = @_;
- my $internalid = get_iprange_id($plugin_config, $range, $noerr);
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr, 1) or return;
+ my $internalid = get_iprange_id($plugin_config, $range, $vrf_id, $noerr);
# definedness check, because ID could be 0
if (!defined($internalid)) {
@@ -364,7 +399,8 @@ sub del_ip {
return if !$ip;
- my $ip_id = get_ip_id($plugin_config, $ip, $noerr);
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr, 1) or return;
+ my $ip_id = get_ip_id($plugin_config, $ip, $vrf_id, $noerr);
if (!defined($ip_id)) {
warn "could not find id for ip $ip";
return;
@@ -397,12 +433,16 @@ sub get_ips_from_mac {
my $ip4 = undef;
my $ip6 = undef;
- for my $cidr (sort map { $_->{cidr} } values %{ $subnets // {} }) {
+ for my $subnet (sort { $a->{cidr} cmp $b->{cidr} } values %{ $subnets // {} }) {
+ my $cidr = $subnet->{cidr};
+ my ($prefix_id, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr);
+ next if !defined($prefix_id);
+ my $vrf = vrf_filter($vrf_id);
my $data = eval {
netbox_api_request(
$plugin_config,
"GET",
- "/ipam/ip-addresses/?parent=$cidr&description__ic=$mac",
+ "/ipam/ip-addresses/?parent=$cidr&$vrf&description__ic=$mac",
);
};
if ($@) {
@@ -439,14 +479,67 @@ sub on_update_hook {
}
# helpers
-sub get_prefix_id {
- my ($config, $cidr, $noerr) = @_;
- # we need to supply any IP inside the prefix, without supplying the mask, so
- # just take the one from the cidr
- my ($ip, undef) = split(/\//, $cidr);
+sub vrf_filter {
+ my ($vrf_id) = @_;
+
+ return defined($vrf_id) ? "vrf_id=$vrf_id" : "vrf_id=null";
+}
+
+sub get_zone_namespace {
+ my ($config, $zoneid, $noerr) = @_;
+
+ my $name = zone_vrf_name($config, $zoneid);
+ my $result = eval { netbox_api_request($config, "GET", "/ipam/vrfs/?name=$name") };
+ if ($@) {
+ return if $noerr;
+ die "could not obtain ID for VRF $name: $@";
+ }
+
+ my $data = @{ $result->{results} }[0];
+ return $data->{id};
+}
+
+sub add_zone_namespace {
+ my ($config, $zoneid, $noerr) = @_;
+
+ my $name = zone_vrf_name($config, $zoneid);
+ my $result = eval { netbox_api_request($config, "POST", "/ipam/vrfs/", { name => $name }) };
+ if ($@) {
+ return if $noerr;
+ die "could not create VRF $name: $@";
+ }
+
+ return $result->{id};
+}
+
+# a prefix from before sits in the global table and keeps its addresses there,
+# so a subnet is looked for in the zone's VRF first and in the global table
+# after it, with none as the VRF of the global table. A subnet with no prefix
+# anywhere stops an operation that needs one, since the address and range
+# operations would act on the global table where another zone's records may
+# sit
+sub subnet_prefix {
+ my ($config, $subnet, $noerr, $needed) = @_;
+
+ my $vrf_id = get_zone_namespace($config, $subnet->{zone}, $noerr);
+ if (defined($vrf_id)) {
+ my $prefix_id = get_prefix_id($config, $subnet->{cidr}, $vrf_id, $noerr);
+ return ($prefix_id, $vrf_id) if defined($prefix_id);
+ }
+ my $prefix_id = get_prefix_id($config, $subnet->{cidr}, undef, $noerr);
+ if ($needed && !defined($prefix_id)) {
+ return if $noerr;
+ die "could not find id for prefix $subnet->{cidr}";
+ }
+ return ($prefix_id, undef);
+}
+
+sub get_prefix_id {
+ my ($config, $cidr, $vrf_id, $noerr) = @_;
- my $result = eval { netbox_api_request($config, "GET", "/ipam/prefixes/?q=$ip") };
+ my $vrf = vrf_filter($vrf_id);
+ my $result = eval { netbox_api_request($config, "GET", "/ipam/prefixes/?prefix=$cidr&$vrf") };
if ($@) {
return if $noerr;
die "could not obtain ID for prefix $cidr: $@";
@@ -457,13 +550,14 @@ sub get_prefix_id {
}
sub get_iprange_id {
- my ($config, $range, $noerr) = @_;
+ my ($config, $range, $vrf_id, $noerr) = @_;
+ my $vrf = vrf_filter($vrf_id);
my $result = eval {
netbox_api_request(
$config,
"GET",
- "/ipam/ip-ranges/?start_address=$range->{'start-address'}&end_address=$range->{'end-address'}",
+ "/ipam/ip-ranges/?start_address=$range->{'start-address'}&end_address=$range->{'end-address'}&$vrf",
);
};
if ($@) {
@@ -477,9 +571,10 @@ sub get_iprange_id {
}
sub get_ip_id {
- my ($config, $ip, $noerr) = @_;
+ my ($config, $ip, $vrf_id, $noerr) = @_;
- my $result = eval { netbox_api_request($config, "GET", "/ipam/ip-addresses/?q=$ip") };
+ my $vrf = vrf_filter($vrf_id);
+ my $result = eval { netbox_api_request($config, "GET", "/ipam/ip-addresses/?q=$ip&$vrf") };
if ($@) {
return if $noerr;
die "could not obtain ID for IP $ip: $@";
@@ -490,9 +585,10 @@ sub get_ip_id {
}
sub is_ip_gateway {
- my ($config, $ip, $noerr) = @_;
+ my ($config, $ip, $vrf_id, $noerr) = @_;
- my $result = eval { netbox_api_request($config, "GET", "/ipam/ip-addresses/?q=$ip") };
+ my $vrf = vrf_filter($vrf_id);
+ my $result = eval { netbox_api_request($config, "GET", "/ipam/ip-addresses/?q=$ip&$vrf") };
if ($@) {
return if $noerr;
die "could not obtain ipam entry for address $ip: $@";
@@ -503,9 +599,11 @@ sub is_ip_gateway {
}
sub is_prefix_empty {
- my ($config, $cidr, $noerr) = @_;
+ my ($config, $cidr, $vrf_id, $noerr) = @_;
- my $result = eval { netbox_api_request($config, "GET", "/ipam/ip-addresses/?parent=$cidr") };
+ my $vrf = vrf_filter($vrf_id);
+ my $result =
+ eval { netbox_api_request($config, "GET", "/ipam/ip-addresses/?parent=$cidr&$vrf") };
if ($@) {
return if $noerr;
die "could not query children for prefix $cidr: $@";
diff --git a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
index e969c3dcdf4f..63f5f2a1ffd5 100644
--- a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
@@ -2,6 +2,9 @@ package PVE::Network::SDN::Ipams::PhpIpamPlugin;
use strict;
use warnings;
+use Math::BigInt;
+use Net::IP;
+
use PVE::INotify;
use PVE::Cluster;
use PVE::Tools;
@@ -35,11 +38,23 @@ sub options {
token => { optional => 0 },
section => { optional => 0 },
fingerprint => { optional => 1 },
+ 'zone-namespaces' => { optional => 1 },
+ 'zone-namespace-prefix' => { optional => 1 },
};
}
# Plugin implementation
+# a zone's section is a sub-section of the configured one, so SDN writes only
+# under the section it was given. Section names are global in phpIPAM, so
+# clusters sharing one set a prefix of their own
+my sub zone_section_name {
+ my ($plugin_config, $zoneid) = @_;
+
+ my $prefix = $plugin_config->{'zone-namespace-prefix'} // 'pve';
+ return "$prefix-$zoneid";
+}
+
sub add_subnet {
my ($class, $plugin_config, $subnetid, $subnet, $noerr) = @_;
@@ -49,15 +64,42 @@ sub add_subnet {
my $url = $plugin_config->{url};
my $token = $plugin_config->{token};
- my $section = $plugin_config->{section};
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
+ # with zone namespaces a subnet being added is new to its zone, so it goes
+ # into the zone's section, and a subnet from before in the configured
+ # section is not taken over, since a copy in the zone's section would hide
+ # its records
+ my $section = $plugin_config->{section};
+ if ($plugin_config->{'zone-namespaces'}) {
+ $section = get_zone_namespace($plugin_config, $subnet->{zone}, $headers);
+ $section //= shared_section($plugin_config, $headers);
+ }
+
#search subnet
- my $internalid = eval { get_prefix_id($url, $cidr, $headers) };
+ my $internalid = eval { get_prefix_id($url, $cidr, $headers, $section) };
+ if (my $err = $@) {
+ return if $noerr;
+ die $err;
+ }
+ my $in_shared = defined($section) && $section == $plugin_config->{section};
+ if (
+ !$internalid
+ && !$in_shared
+ && defined(get_prefix_id($url, $cidr, $headers, $plugin_config->{section}))
+ ) {
+ return if $noerr;
+ my $name = zone_section_name($plugin_config, $subnet->{zone});
+ die "subnet $cidr exists in section $plugin_config->{section} of phpIPAM, move it"
+ . " into the sub-section of its zone under that section, $name for this one,"
+ . " or remove it first\n";
+ }
#create subnet
if (!$internalid) {
+ $section //= add_zone_namespace($plugin_config, $subnet->{zone}, $headers, $noerr);
+ return if !defined($section);
my $params = {
subnet => $network,
mask => $mask,
@@ -87,7 +129,7 @@ sub del_subnet {
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
- my $internalid = get_prefix_id($url, $cidr, $headers);
+ my $internalid = subnet_id($plugin_config, $subnet, $headers);
return if !$internalid;
return; #fixme: check that prefix is empty exluding gateway, before delete
@@ -120,7 +162,7 @@ sub add_ip {
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
- my $internalid = get_prefix_id($url, $cidr, $headers);
+ my $internalid = subnet_id($plugin_config, $subnet, $headers);
my $params = {
ip => $ip,
@@ -140,7 +182,7 @@ sub add_ip {
if ($@) {
if ($is_gateway) {
die "error add subnet ip to ipam: ip $ip already exist: $@"
- if !is_ip_gateway($url, $ip, $headers) && !$noerr;
+ if !is_ip_gateway($url, $ip, $headers, $internalid) && !$noerr;
} else {
die "error add subnet ip to ipam: ip $ip already exist: $@" if !$noerr;
}
@@ -167,7 +209,7 @@ sub update_ip {
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
- my $ip_id = get_ip_id($url, $ip, $headers);
+ my $ip_id = get_ip_id($url, $ip, $headers, subnet_id($plugin_config, $subnet, $headers));
die "can't find ip address in ipam\n" if !$ip_id;
my $params = {
@@ -198,7 +240,7 @@ sub add_next_freeip {
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
- my $internalid = get_prefix_id($url, $cidr, $headers);
+ my $internalid = subnet_id($plugin_config, $subnet, $headers);
my $params = {
hostname => $hostname,
@@ -248,7 +290,7 @@ sub del_ip {
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
- my $ip_id = get_ip_id($url, $ip, $headers);
+ my $ip_id = get_ip_id($url, $ip, $headers, subnet_id($plugin_config, $subnet, $headers));
return if !$ip_id;
if ($mac) {
my $record = eval {
@@ -279,6 +321,24 @@ sub del_ip {
return 1;
}
+# phpIPAM answers a lookup that finds nothing with a 404
+my sub not_found {
+ my ($err) = @_;
+ return $err =~ m/^Invalid response from server: 404 /;
+}
+
+# an address comes back in its text form or as the decimal phpIPAM stores
+my sub address_of_hit {
+ my ($hit, $family) = @_;
+
+ my $ip = $hit->{ip} // $hit->{ip_addr};
+ return undef if !defined($ip);
+ return $ip if $ip !~ m/^\d+$/;
+ my $bin = Net::IP::ip_inttobin(Math::BigInt->new($ip), $family) // return undef;
+ $ip = Net::IP::ip_bintoip($bin, $family);
+ return $family == 6 ? Net::IP::ip_compress_address($ip, 6) : $ip;
+}
+
sub get_ips_from_mac {
my ($class, $plugin_config, $mac, $zoneid, $subnets) = @_;
@@ -290,7 +350,15 @@ sub get_ips_from_mac {
my $ip4 = undef;
my $ip6 = undef;
- my $ips = eval {
+ # a hit names its subnet, and only the vnet's subnets count
+ my %wanted;
+ for my $subnet (values %{ $subnets // {} }) {
+ my $id = subnet_id($plugin_config, $subnet, $headers) // next;
+ $wanted{$id} = Net::IP::ip_is_ipv4($subnet->{network}) ? 4 : 6;
+ }
+ return if !%wanted;
+
+ my $result = eval {
PVE::Network::SDN::api_request(
"GET",
"$url/addresses/search_mac/$mac",
@@ -299,19 +367,19 @@ sub get_ips_from_mac {
$fingerprint,
);
};
- return if $@;
-
- #fixme
- die "parsing of result not yet implemented";
-
- for my $ip (@$ips) {
- # if ($ip->{family}->{value} == 4 && !$ip4) {
- # ($ip4, undef) = split(/\//, $ip->{address});
- # }
- #
- # if ($ip->{family}->{value} == 6 && !$ip6) {
- # ($ip6, undef) = split(/\//, $ip->{address});
- # }
+ if (my $err = $@) {
+ return if not_found($err);
+ die "could not look up the addresses of $mac: $err";
+ }
+
+ for my $hit (@{ $result->{data} // [] }) {
+ my $family = $wanted{ $hit->{subnetId} // '' } // next;
+ my $ip = address_of_hit($hit, $family) // next;
+ if ($family == 4) {
+ $ip4 //= $ip;
+ } else {
+ $ip6 //= $ip;
+ }
}
return ($ip4, $ip6);
@@ -344,27 +412,118 @@ sub on_update_hook {
#helpers
-sub get_prefix_id {
- my ($url, $cidr, $headers) = @_;
+sub get_zone_namespace {
+ my ($plugin_config, $zoneid, $headers) = @_;
- my $result = PVE::Network::SDN::api_request("GET", "$url/subnets/cidr/$cidr", $headers);
- my $data = @{ $result->{data} }[0];
- my $internalid = $data->{id};
- return $internalid;
+ my $name = zone_section_name($plugin_config, $zoneid);
+ my $result = eval {
+ PVE::Network::SDN::api_request(
+ "GET",
+ "$plugin_config->{url}/sections/$name/",
+ $headers,
+ undef,
+ $plugin_config->{fingerprint},
+ );
+ };
+ if (my $err = $@) {
+ # only a missing section means the zone has none yet, any other
+ # failure would send the zone's records to the configured section
+ return if not_found($err);
+ die "could not look up section $name: $err";
+ }
+ # section names are global in phpIPAM, so one of the zone's name under
+ # another parent is another cluster's, and the zone has none of its own
+ my $section = $result->{data};
+ return if ($section->{masterSection} // 0) != $plugin_config->{section};
+ return $section->{id};
+}
+
+sub add_zone_namespace {
+ my ($plugin_config, $zoneid, $headers, $noerr) = @_;
+
+ my $name = zone_section_name($plugin_config, $zoneid);
+ my $result = eval {
+ PVE::Network::SDN::api_request(
+ "POST",
+ "$plugin_config->{url}/sections/",
+ $headers,
+ { name => $name, masterSection => $plugin_config->{section} },
+ $plugin_config->{fingerprint},
+ );
+ };
+ if (my $err = $@) {
+ return if $noerr;
+ my $taken = eval {
+ PVE::Network::SDN::api_request(
+ "GET",
+ "$plugin_config->{url}/sections/$name/",
+ $headers,
+ undef,
+ $plugin_config->{fingerprint},
+ );
+ };
+ die "section $name exists under another section in phpIPAM, move it under"
+ . " section $plugin_config->{section} or remove it\n"
+ if $taken;
+ die "could not create section $name: $err";
+ }
+ return $result->{id};
+}
+
+# phpIPAM nests sections one level deep, so a configured section that is a
+# sub-section itself gets no zone sections and every zone's subnets stay in it
+sub shared_section {
+ my ($plugin_config, $headers) = @_;
+
+ my $result = PVE::Network::SDN::api_request(
+ "GET",
+ "$plugin_config->{url}/sections/$plugin_config->{section}/",
+ $headers,
+ undef,
+ $plugin_config->{fingerprint},
+ );
+ return ($result->{data}->{masterSection} // 0) ? $plugin_config->{section} : undef;
+}
+
+# a subnet from before sits in the configured section, so a subnet is looked
+# for in the zone's section first and in the configured one after it
+sub subnet_id {
+ my ($plugin_config, $subnet, $headers) = @_;
+
+ my $section = get_zone_namespace($plugin_config, $subnet->{zone}, $headers);
+ my $url = $plugin_config->{url};
+ return get_prefix_id($url, $subnet->{cidr}, $headers, $section)
+ // get_prefix_id($url, $subnet->{cidr}, $headers, $plugin_config->{section});
+}
+
+sub get_prefix_id {
+ my ($url, $cidr, $headers, $section) = @_;
+
+ return undef if !defined($section);
+ my $result =
+ eval { PVE::Network::SDN::api_request("GET", "$url/subnets/cidr/$cidr", $headers) };
+ if (my $err = $@) {
+ return undef if not_found($err);
+ die "could not look up subnet $cidr: $err";
+ }
+ my ($data) = grep { $_->{sectionId} == $section } @{ $result->{data} // [] };
+ return $data->{id};
}
sub get_ip_id {
- my ($url, $ip, $headers) = @_;
+ my ($url, $ip, $headers, $subnet_id) = @_;
my $result = PVE::Network::SDN::api_request("GET", "$url/addresses/search/$ip", $headers);
- my $data = @{ $result->{data} }[0];
+ my ($data) =
+ grep { defined($subnet_id) && $_->{subnetId} == $subnet_id } @{ $result->{data} // [] };
my $ip_id = $data->{id};
return $ip_id;
}
sub is_ip_gateway {
- my ($url, $ip, $headers) = @_;
+ my ($url, $ip, $headers, $subnet_id) = @_;
my $result = PVE::Network::SDN::api_request("GET", "$url/addresses/search/$ip", $headers);
- my $data = @{ $result->{data} }[0];
+ my ($data) =
+ grep { defined($subnet_id) && $_->{subnetId} == $subnet_id } @{ $result->{data} // [] };
my $is_gateway = $data->{is_gateway};
return $is_gateway;
}
diff --git a/src/PVE/Network/SDN/Ipams/Plugin.pm b/src/PVE/Network/SDN/Ipams/Plugin.pm
index 6e23d5434d4b..4998957f1485 100644
--- a/src/PVE/Network/SDN/Ipams/Plugin.pm
+++ b/src/PVE/Network/SDN/Ipams/Plugin.pm
@@ -45,6 +45,23 @@ my $defaultData = {
},
),
fingerprint => get_standard_option('fingerprint-sha256', { optional => 1 }),
+ 'zone-namespaces' => {
+ type => 'boolean',
+ description => "Keep the subnets of each zone in a namespace of its own, a VRF"
+ . " in NetBox and a sub-section in phpIPAM, so two zones can use the same"
+ . " range. Turn it on once every node runs a version that knows them.",
+ optional => 1,
+ default => 0,
+ },
+ 'zone-namespace-prefix' => {
+ type => 'string',
+ description => "The names of the zone namespaces start with this, so clusters"
+ . " sharing an IPAM set one of their own.",
+ pattern => '[a-zA-Z0-9][a-zA-Z0-9_-]*',
+ maxLength => 32,
+ optional => 1,
+ default => 'pve',
+ },
},
};
diff --git a/src/test/ipams/netbox/expected.add_ip b/src/test/ipams/netbox/expected.add_ip
index cd46c53f1566..f54f8d05b16e 100644
--- a/src/test/ipams/netbox/expected.add_ip
+++ b/src/test/ipams/netbox/expected.add_ip
@@ -1,5 +1,5 @@
bless( {
- '_content' => '{"address":"10.0.0.1/24","description":"gateway","dns_name":"myhostname"}',
+ '_content' => '{"address":"10.0.0.1/24","description":"gateway","dns_name":"myhostname","vrf":1}',
'_headers' => bless( {
'authorization' => 'token 0123456789abcdef0123456789abcdef01234567',
'content-type' => 'application/json; charset=UTF-8'
diff --git a/src/test/ipams/netbox/expected.add_ip_notgateway b/src/test/ipams/netbox/expected.add_ip_notgateway
index 5ea43cd65296..327efbab8c6b 100644
--- a/src/test/ipams/netbox/expected.add_ip_notgateway
+++ b/src/test/ipams/netbox/expected.add_ip_notgateway
@@ -1,5 +1,5 @@
bless( {
- '_content' => '{"address":"10.0.0.1/24","description":"mac:da:65:8f:18:9b:6f","dns_name":"myhostname"}',
+ '_content' => '{"address":"10.0.0.1/24","description":"mac:da:65:8f:18:9b:6f","dns_name":"myhostname","vrf":1}',
'_headers' => bless( {
'authorization' => 'token 0123456789abcdef0123456789abcdef01234567',
'content-type' => 'application/json; charset=UTF-8'
diff --git a/src/test/ipams/netbox/expected.update_ip b/src/test/ipams/netbox/expected.update_ip
index d3bc1addbdea..8a6e0a8cbbad 100644
--- a/src/test/ipams/netbox/expected.update_ip
+++ b/src/test/ipams/netbox/expected.update_ip
@@ -1,5 +1,5 @@
bless( {
- '_content' => '{"address":"10.0.0.1/24","description":"gateway","dns_name":"myhostname"}',
+ '_content' => '{"address":"10.0.0.1/24","description":"gateway","dns_name":"myhostname","vrf":1}',
'_headers' => bless( {
'authorization' => 'token 0123456789abcdef0123456789abcdef01234567',
'content-type' => 'application/json; charset=UTF-8'
diff --git a/src/test/run_test_ipams.pl b/src/test/run_test_ipams.pl
index f07b4bb4b486..75a700d7d604 100755
--- a/src/test/run_test_ipams.pl
+++ b/src/test/run_test_ipams.pl
@@ -92,6 +92,9 @@ foreach my $path (@plugins) {
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
my $sdn_ipam_plugin = Test::MockModule->new($plugin);
$sdn_ipam_plugin->mock(
+ get_zone_namespace => sub {
+ return 1;
+ },
get_prefix_id => sub {
return 1;
},
@@ -219,6 +222,196 @@ foreach my $path (@plugins) {
}
+# the phpipam MAC lookup, with the addresses as the decimals phpipam hands out
+{
+ my $plugin = 'PVE::Network::SDN::Ipams::PhpIpamPlugin';
+ my $mocked = Test::MockModule->new($plugin);
+ $mocked->mock(subnet_id => sub { my (undef, $subnet) = @_; return $subnet->{id}; });
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ return {
+ data => [
+ { ip => 171048980, subnetId => 7 },
+ { ip => '10.60.0.20', subnetId => 8 },
+ { ip => '55832768119825001489588427028103692289', subnetId => 9 },
+ ],
+ };
+ },
+ );
+ my $subnets = {
+ 7 => { id => 7, cidr => '10.50.0.0/24', network => '10.50.0.0' },
+ 9 => { id => 9, cidr => '2a01::/64', network => '2a01::' },
+ };
+ my @ips = $plugin->get_ips_from_mac(
+ { url => 'u', token => 't' }, 'da:65:8f:18:9b:6f', 'z', $subnets,
+ );
+ is_deeply(
+ \@ips,
+ ['10.50.0.20', '2a01::1'],
+ 'phpipam lookup converts decimal addresses by the subnet family',
+ );
+
+ # a MAC without addresses is a 404, any other failure fails the lookup
+ $sdn_module->mock(api_request => sub { die "Invalid response from server: 404 Not Found\n" });
+ is_deeply(
+ [
+ $plugin->get_ips_from_mac(
+ { url => 'u', token => 't' },
+ 'da:65:8f:18:9b:6f',
+ 'z',
+ $subnets,
+ ),
+ ],
+ [],
+ 'phpipam lookup answers nothing for a MAC without addresses',
+ );
+ $sdn_module->mock(
+ api_request => sub { die "Invalid response from server: 503 Service Unavailable\n" });
+ eval {
+ $plugin->get_ips_from_mac(
+ { url => 'u', token => 't' },
+ 'da:65:8f:18:9b:6f',
+ 'z',
+ $subnets,
+ );
+ };
+ like($@, qr/could not look up the addresses/, 'and dies on any other failure');
+}
+
+# a range the zone's namespace does not hold is refused while a subnet from
+# before holds it in the global table or the configured section, since a copy
+# would hide its records, and the error names the way out
+{
+ my $subnet = {
+ zone => 'z',
+ cidr => '10.0.0.0/24',
+ network => '10.0.0.0',
+ mask => '24',
+ };
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return {};
+ },
+ );
+
+ my $netbox = Test::MockModule->new('PVE::Network::SDN::Ipams::NetboxPlugin');
+ $netbox->mock(
+ get_zone_namespace => sub { return 2; },
+ get_prefix_id =>
+ sub { my (undef, undef, $vrf_id) = @_; return defined($vrf_id) ? undef : 5; },
+ );
+ eval {
+ PVE::Network::SDN::Ipams::NetboxPlugin->add_subnet(
+ { url => 'u', token => 't', 'zone-namespaces' => 1 },
+ 'z-10.0.0.0-24',
+ $subnet,
+ );
+ };
+ like(
+ $@,
+ qr/move it .* into the VRF of its zone, pve-z for this one/,
+ 'netbox refuses a range the global table holds',
+ );
+ is(scalar(@requests), 0, 'and creates nothing');
+
+ @requests = ();
+ my $phpipam = Test::MockModule->new('PVE::Network::SDN::Ipams::PhpIpamPlugin');
+ $phpipam->mock(
+ get_zone_namespace => sub { return 2; },
+ get_prefix_id => sub {
+ my (undef, undef, undef, $section) = @_;
+ return ($section // '') eq '2' ? undef : 5;
+ },
+ );
+ eval {
+ PVE::Network::SDN::Ipams::PhpIpamPlugin->add_subnet(
+ { url => 'u', token => 't', section => 1, 'zone-namespaces' => 1 },
+ 'z-10.0.0.0-24',
+ $subnet,
+ );
+ };
+ like(
+ $@,
+ qr/move it into the sub-section of its zone .* pve-z for this one/,
+ 'phpipam refuses a range the configured section holds',
+ );
+ is(scalar(@requests), 0, 'and creates nothing');
+}
+
+# a prefix already in the zone's VRF, moved there as the refusal advises, is
+# used by the subnet added next, and only the ranges it came without are created
+{
+ my $subnet = {
+ zone => 'z',
+ cidr => '10.0.0.0/24',
+ network => '10.0.0.0',
+ mask => '24',
+ 'dhcp-range' => [
+ 'start-address=10.0.0.10,end-address=10.0.0.20',
+ 'start-address=10.0.0.30,end-address=10.0.0.40',
+ ],
+ };
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return { results => [{ id => 7 }] }
+ if $url =~ m|/ipam/ip-ranges/\?start_address=10\.0\.0\.10&|;
+ return { results => [] };
+ },
+ );
+ my $netbox = Test::MockModule->new('PVE::Network::SDN::Ipams::NetboxPlugin');
+ $netbox->mock(
+ get_zone_namespace => sub { return 2; },
+ get_prefix_id =>
+ sub { my (undef, undef, $vrf_id) = @_; return defined($vrf_id) ? 5 : undef; },
+ );
+ eval {
+ PVE::Network::SDN::Ipams::NetboxPlugin->add_subnet(
+ { url => 'u', token => 't', 'zone-namespaces' => 1 },
+ 'z-10.0.0.0-24',
+ $subnet,
+ );
+ };
+ is($@, '', 'netbox uses a prefix already in the zone\'s VRF');
+ is_deeply(
+ [grep { $_->[0] eq 'POST' } @requests],
+ [[
+ 'POST',
+ 'u/ipam/ip-ranges/',
+ { start_address => '10.0.0.30', end_address => '10.0.0.40', vrf => 2 },
+ ]],
+ 'creates only the range the VRF does not hold and no prefix',
+ );
+}
+
+# the phpipam gateway check answers for the subnet it is asked about, also
+# when the address sits in another zone's section as well
+{
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ return {
+ data => [
+ { ip => '10.0.0.1', subnetId => 7, is_gateway => 1 },
+ { ip => '10.0.0.1', subnetId => 8, is_gateway => 0 },
+ ],
+ };
+ },
+ );
+ my $is_gateway = \&PVE::Network::SDN::Ipams::PhpIpamPlugin::is_ip_gateway;
+ ok($is_gateway->('u', '10.0.0.1', [], 7), 'phpipam sees the gateway in its subnet');
+ ok(!$is_gateway->('u', '10.0.0.1', [], 8), 'and not in another subnet holding the address');
+ ok(!$is_gateway->('u', '10.0.0.1', [], undef), 'and not without a subnet to ask about');
+}
+
# an address released for a MAC stays when the external IPAM records it
# for another MAC
{
@@ -268,5 +461,345 @@ foreach my $path (@plugins) {
}
}
+# an address operation on a subnet whose prefix is in neither the zone's VRF
+# nor the global table stops instead of acting on the global table
+{
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url) = @_;
+ push @requests, [$method, $url];
+ return { results => [] };
+ },
+ );
+ my $netbox = Test::MockModule->new('PVE::Network::SDN::Ipams::NetboxPlugin');
+ $netbox->mock(
+ get_zone_namespace => sub { return undef; },
+ get_prefix_id => sub { return undef; },
+ );
+ my $config = { url => 'u', token => 't' };
+ my $subnet = { zone => 'z', cidr => '10.0.0.0/24' };
+ my $plugin = 'PVE::Network::SDN::Ipams::NetboxPlugin';
+ my %ops = (
+ add_ip => sub {
+ $plugin->add_ip(
+ $config,
+ 'z-10.0.0.0-24',
+ $subnet,
+ '10.0.0.5',
+ 'h',
+ 'da:65:8f:18:9b:6f',
+ 'd',
+ 0,
+ );
+ },
+ update_ip => sub {
+ $plugin->update_ip(
+ $config,
+ 'z-10.0.0.0-24',
+ $subnet,
+ '10.0.0.5',
+ 'h',
+ 'da:65:8f:18:9b:6f',
+ 'd',
+ 0,
+ );
+ },
+ del_ip => sub { $plugin->del_ip($config, 'z-10.0.0.0-24', $subnet, '10.0.0.5') },
+ );
+ for my $op (sort keys %ops) {
+ @requests = ();
+ eval { $ops{$op}->() };
+ like($@, qr/could not find id for prefix 10.0.0.0\/24/,
+ "netbox $op stops without a prefix");
+ is(scalar(@requests), 0, "and sends nothing");
+ }
+}
+
+# without the zone namespaces a subnet goes where it went before, the global
+# table or the configured section, and no namespace is looked for or created
+{
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return $method eq 'GET' ? { results => [], data => [] } : { id => 5 };
+ },
+ );
+ my $subnet = { zone => 'z', cidr => '10.0.0.0/24', network => '10.0.0.0', mask => '24' };
+ PVE::Network::SDN::Ipams::NetboxPlugin->add_subnet(
+ { url => 'u', token => 't' },
+ 'z-10.0.0.0-24',
+ $subnet,
+ );
+ is_deeply(
+ [grep { $_->[0] eq 'POST' } @requests],
+ [['POST', 'u/ipam/prefixes/', { prefix => '10.0.0.0/24' }]],
+ 'netbox puts a subnet into the global table without the zone namespaces',
+ );
+ ok(!grep({ $_->[1] =~ m|/vrfs/| } @requests), 'and never asks for a VRF');
+
+ @requests = ();
+ PVE::Network::SDN::Ipams::PhpIpamPlugin->add_subnet(
+ { url => 'u', token => 't', section => 1 },
+ 'z-10.0.0.0-24',
+ $subnet,
+ );
+ is_deeply(
+ [grep { $_->[0] eq 'POST' } @requests],
+ [['POST', 'u/subnets/', { subnet => '10.0.0.0', mask => '24', sectionId => 1 }]],
+ 'phpipam puts a subnet into the configured section without the zone namespaces',
+ );
+ ok(!grep({ $_->[1] =~ m|/sections/| } @requests), 'and never asks for a section');
+}
+
+# a zone's VRF is found and created under its prefixed name
+{
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return {
+ results => [{ id => 4 }],
+ id => 4,
+ data => { id => 4, masterSection => 1 },
+ };
+ },
+ );
+ my $config = { url => 'u', token => 't' };
+ PVE::Network::SDN::Ipams::NetboxPlugin::get_zone_namespace($config, 'z');
+ PVE::Network::SDN::Ipams::NetboxPlugin::add_zone_namespace($config, 'z');
+ is_deeply(
+ \@requests,
+ [
+ ['GET', 'u/ipam/vrfs/?name=pve-z', undef],
+ ['POST', 'u/ipam/vrfs/', { name => 'pve-z' }],
+ ],
+ 'netbox finds and creates the zone\'s VRF under the name pve-<zone>',
+ );
+
+ # a prefix set on the entry keeps clusters sharing the IPAM apart
+ @requests = ();
+ PVE::Network::SDN::Ipams::NetboxPlugin::get_zone_namespace(
+ { %$config, 'zone-namespace-prefix' => 'lab' }, 'z',
+ );
+ is($requests[0]->[1], 'u/ipam/vrfs/?name=lab-z', 'and under the configured prefix');
+ is(
+ PVE::Network::SDN::Ipams::PhpIpamPlugin::get_zone_namespace(
+ { url => 'u', token => 't', section => 1, 'zone-namespace-prefix' => 'lab' },
+ 'z',
+ [],
+ ),
+ 4,
+ 'phpipam looks the zone\'s section up under the configured prefix as well',
+ );
+ like($requests[-1]->[1], qr{/sections/lab-z/$}, 'by its name');
+}
+
+# a subnet from before is found in the global table while the zone's VRF holds
+# no prefix for its range, and in the VRF once it was moved there
+{
+ my $moved;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url) = @_;
+ return { results => [{ id => 2 }] } if $url =~ m|/ipam/vrfs/\?name=pve-z$|;
+ if ($url =~ m|/ipam/prefixes/\?prefix=10\.0\.0\.0/24&vrf_id=(\S+)$|) {
+ my $vrf = $1;
+ return { results => [{ id => 5 }] } if $vrf eq '2' && $moved;
+ return { results => [{ id => 5 }] } if $vrf eq 'null' && !$moved;
+ return { results => [] };
+ }
+ die "unexpected request $method $url\n";
+ },
+ );
+ my $config = { url => 'u', token => 't' };
+ my $subnet = { zone => 'z', cidr => '10.0.0.0/24' };
+ is_deeply(
+ [PVE::Network::SDN::Ipams::NetboxPlugin::subnet_prefix($config, $subnet)],
+ [5, undef],
+ 'netbox finds a prefix from before in the global table next to the zone\'s VRF',
+ );
+ $moved = 1;
+ is_deeply(
+ [PVE::Network::SDN::Ipams::NetboxPlugin::subnet_prefix($config, $subnet)],
+ [5, 2],
+ 'and in the VRF once the prefix was moved there',
+ );
+}
+
+# the same for phpipam, where the subnet keeps its id across the move, so the
+# requests show which section answered
+{
+ my $moved;
+ my @cidr_requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url) = @_;
+ return { data => { id => 2, masterSection => 1 } }
+ if $url =~ m|/sections/pve-z/$|;
+ if ($url =~ m|/subnets/cidr/10\.0\.0\.0/24$|) {
+ push @cidr_requests, $url;
+ return { data => [{ id => 5, sectionId => $moved ? 2 : 1 }] };
+ }
+ die "unexpected request $method $url\n";
+ },
+ );
+ my $config = { url => 'u', token => 't', section => 1 };
+ my $subnet = { zone => 'z', cidr => '10.0.0.0/24' };
+ is(
+ PVE::Network::SDN::Ipams::PhpIpamPlugin::subnet_id($config, $subnet, []),
+ 5,
+ 'phpipam finds a subnet from before in the configured section',
+ );
+ is(scalar(@cidr_requests), 2, 'after looking in the zone\'s section first');
+ $moved = 1;
+ @cidr_requests = ();
+ is(
+ PVE::Network::SDN::Ipams::PhpIpamPlugin::subnet_id($config, $subnet, []),
+ 5,
+ 'and in the zone\'s section once the subnet was moved there',
+ );
+ is(scalar(@cidr_requests), 1, 'without a second look');
+}
+
+# a zone without a section is told apart from a section lookup that failed
+{
+ my $error;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(api_request => sub { die $error; });
+ my $lookup = \&PVE::Network::SDN::Ipams::PhpIpamPlugin::get_zone_namespace;
+ my $config = { url => 'u', token => 't', section => 1 };
+
+ $error = "Invalid response from server: 404 Not Found\n";
+ is(scalar($lookup->($config, 'z', [])), undef, 'phpipam reports a missing section as none');
+ $error = "Invalid response from server: 503 Service Unavailable\n";
+ eval { $lookup->($config, 'z', []) };
+ like($@, qr/could not look up section pve-z/, 'and dies on any other failure');
+
+ # a section of the zone's name elsewhere in phpIPAM is another cluster's
+ $sdn_module->mock(api_request => sub { return { data => { id => 7, masterSection => 2 } }; });
+ is(
+ scalar($lookup->({ url => 'u', token => 't', section => 1 }, 'z', [])),
+ undef,
+ 'a section outside the configured one is not the zone\'s',
+ );
+ # and creating the zone's own then fails with the reason
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method) = @_;
+ die "Invalid response from server: 500 Internal Server Error\n"
+ if $method eq 'POST';
+ return { data => { id => 7, masterSection => 2 } };
+ },
+ );
+ eval {
+ PVE::Network::SDN::Ipams::PhpIpamPlugin::add_zone_namespace(
+ { url => 'u', token => 't', section => 1 },
+ 'z',
+ [],
+ );
+ };
+ like(
+ $@,
+ qr/exists under another section/,
+ 'creating a section another cluster has fails with the reason',
+ );
+ $sdn_module->mock(api_request => sub { return { data => { id => 7, masterSection => 1 } }; });
+ is(
+ $lookup->({ url => 'u', token => 't', section => 1 }, 'z', []),
+ 7,
+ 'and one under it is the zone\'s',
+ );
+}
+
+# a zone's section is created under the configured section
+{
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return { id => 5 };
+ },
+ );
+ my $phpipam = Test::MockModule->new('PVE::Network::SDN::Ipams::PhpIpamPlugin');
+ $phpipam->mock(
+ get_zone_namespace => sub { return undef; },
+ get_prefix_id => sub { return undef; },
+ );
+ PVE::Network::SDN::Ipams::PhpIpamPlugin->add_subnet(
+ { url => 'u', token => 't', section => 1, 'zone-namespaces' => 1 },
+ 'z-10.0.0.0/24',
+ { zone => 'z', cidr => '10.0.0.0/24', network => '10.0.0.0', mask => '24' },
+ 1,
+ );
+ is_deeply(
+ [map { [$_->[0], $_->[1], $_->[2]] } @requests],
+ [
+ ['GET', 'u/sections/1/', undef],
+ ['POST', 'u/sections/', { name => 'pve-z', masterSection => 1 }],
+ ['POST', 'u/subnets/', { subnet => '10.0.0.0', mask => '24', sectionId => 5 }],
+ ],
+ 'phpipam creates the zone\'s section under the configured one',
+ );
+
+ # a configured section that is a sub-section itself gets no zone sections
+ @requests = ();
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return $url eq 'u/sections/1/'
+ ? { data => { id => 1, masterSection => 3 } }
+ : { id => 5 };
+ },
+ );
+ PVE::Network::SDN::Ipams::PhpIpamPlugin->add_subnet(
+ { url => 'u', token => 't', section => 1, 'zone-namespaces' => 1 },
+ 'z-10.0.0.0/24',
+ { zone => 'z', cidr => '10.0.0.0/24', network => '10.0.0.0', mask => '24' },
+ 1,
+ );
+ is_deeply(
+ [map { [$_->[0], $_->[1], $_->[2]] } @requests],
+ [
+ ['GET', 'u/sections/1/', undef],
+ ['POST', 'u/subnets/', { subnet => '10.0.0.0', mask => '24', sectionId => 1 }],
+ ],
+ 'and keeps the subnets in a configured section that is a sub-section itself',
+ );
+
+ # a lookup that fails stops the creation, a copy would be made otherwise
+ @requests = ();
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ die "Invalid response from server: 503 Service Unavailable\n"
+ if $url =~ m|/subnets/cidr/|;
+ return { data => { id => 1, masterSection => 3 } };
+ },
+ );
+ $phpipam->unmock('get_prefix_id');
+ eval {
+ PVE::Network::SDN::Ipams::PhpIpamPlugin->add_subnet(
+ { url => 'u', token => 't', section => 1 },
+ 'z-10.0.0.0/24',
+ { zone => 'z', cidr => '10.0.0.0/24', network => '10.0.0.0', mask => '24' },
+ );
+ };
+ like($@, qr/could not look up subnet/, 'a failed subnet lookup stops the creation');
+ is(scalar(grep { $_->[0] eq 'POST' } @requests), 0, 'and nothing is created');
+}
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH qemu-server v4 10/13] network: sdn: push the DHCP mappings of a guest when it resumes
2026-11-03 18:55 [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (8 preceding siblings ...)
2026-11-03 18:55 ` [PATCH pve-network v4 09/13] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM Hannes Laimer
@ 2026-11-03 18:55 ` Hannes Laimer
2026-11-03 18:55 ` [PATCH pve-manager v4 11/13] ui: sdn: dhcp tree: do not merge the same subnet in different zones Hannes Laimer
` (3 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 18:55 UTC (permalink / raw)
To: pve-devel
A mapping edit and a migration of the guest exclude each other, but
an edit through a node not yet upgraded takes no lock and writes no
reservation, so it can land while the guest is migrated. Every resume
now pushes the mapping of each NIC of the guest on the local node
before the guest continues, and the last step of a live migration
resumes the target after the config moved, so the target serves what
the cache vouches for. The push reads the cache only and asks the
IPAM for nothing, so the paused guest waits on no request.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/QemuServer/Network.pm | 17 +++++++++++++++++
src/PVE/QemuServer/RunState.pm | 2 ++
2 files changed, 19 insertions(+)
diff --git a/src/PVE/QemuServer/Network.pm b/src/PVE/QemuServer/Network.pm
index b4893c015646..b063f5b75b36 100644
--- a/src/PVE/QemuServer/Network.pm
+++ b/src/PVE/QemuServer/Network.pm
@@ -272,6 +272,23 @@ sub add_random_macs {
}
}
+# every resume pushes the current mapping of each NIC, so the target of a
+# live migration, resumed after the config moved, holds what the cache
+# vouches for. The push takes it from the cache and asks the IPAM for
+# nothing, so the paused guest waits on no request
+sub add_nets_dhcp_mapping {
+ my ($conf) = @_;
+
+ for my $opt (keys %$conf) {
+ next if $opt !~ m/^net\d+$/;
+ my $net = parse_net($conf->{$opt}, 1) or next;
+ next if !$net->{bridge} || !$net->{macaddr};
+ # a resume does not fail on its DHCP backend
+ eval { PVE::Network::SDN::Dhcp::update_mapping($net->{bridge}, $net->{macaddr}, 1) };
+ warn $@ if $@;
+ }
+}
+
sub add_nets_bridge_fdb {
my ($conf, $vmid) = @_;
diff --git a/src/PVE/QemuServer/RunState.pm b/src/PVE/QemuServer/RunState.pm
index 889ddd28287b..828d79159e90 100644
--- a/src/PVE/QemuServer/RunState.pm
+++ b/src/PVE/QemuServer/RunState.pm
@@ -181,6 +181,8 @@ sub vm_resume($vmid, $skiplock = undef, $nocheck = undef) {
PVE::QemuServer::Network::add_nets_bridge_fdb($conf, $vmid)
if $resume_cmd eq 'cont';
+ PVE::QemuServer::Network::add_nets_dhcp_mapping($conf)
+ if $resume_cmd eq 'cont';
mon_cmd($vmid, $resume_cmd);
},
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH pve-manager v4 11/13] ui: sdn: dhcp tree: do not merge the same subnet in different zones
2026-11-03 18:55 [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (9 preceding siblings ...)
2026-11-03 18:55 ` [PATCH qemu-server v4 10/13] network: sdn: push the DHCP mappings of a guest when it resumes Hannes Laimer
@ 2026-11-03 18:55 ` Hannes Laimer
2026-11-03 18:55 ` [PATCH pve-manager v4 12/13] ui: sdn: ipam: add the zone namespace options Hannes Laimer
` (2 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 18:55 UTC (permalink / raw)
To: pve-devel
Two zones can each have a subnet with the same CIDR. The tree keyed
its subnet nodes by the CIDR alone, so it listed the mappings of both
zones under the subnet of one of them. The vnet of the other zone did
not show the subnet.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/manager6/tree/DhcpTree.js | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/www/manager6/tree/DhcpTree.js b/www/manager6/tree/DhcpTree.js
index 40eb40bf9408..23f91fe51996 100644
--- a/www/manager6/tree/DhcpTree.js
+++ b/www/manager6/tree/DhcpTree.js
@@ -60,7 +60,9 @@ Ext.define('PVE.sdn.DhcpTree', {
zones[element.zone].children.push(vnet);
}
- if (!(element.subnet in subnets)) {
+ // the same subnet can exist in several zones
+ let subnetKey = `${element.zone}/${element.subnet}`;
+ if (!(subnetKey in subnets)) {
let subnet = {
name: element.subnet,
zone: element.zone,
@@ -71,14 +73,14 @@ Ext.define('PVE.sdn.DhcpTree', {
children: [],
};
- subnets[element.subnet] = subnet;
+ subnets[subnetKey] = subnet;
vnets[element.vnet].children.push(subnet);
}
element.name = element.vmid; // for sorting
element.type = 'mapping';
element.iconCls = 'x-tree-icon-none';
- subnets[element.subnet].children.push(element);
+ subnets[subnetKey].children.push(element);
});
me.getView().setRootNode(root);
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH pve-manager v4 12/13] ui: sdn: ipam: add the zone namespace options
2026-11-03 18:55 [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (10 preceding siblings ...)
2026-11-03 18:55 ` [PATCH pve-manager v4 11/13] ui: sdn: dhcp tree: do not merge the same subnet in different zones Hannes Laimer
@ 2026-11-03 18:55 ` Hannes Laimer
2026-11-03 18:55 ` [PATCH pve-docs v4 13/13] sdn: ipam: document zone namespaces in NetBox and phpIPAM Hannes Laimer
2026-11-04 9:56 ` superseded: [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
13 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 18:55 UTC (permalink / raw)
To: pve-devel
The NetBox and phpIPAM dialogs show both options in an advanced
section. The namespace per zone is off by default, and the namespace
prefix is pve when left empty.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/manager6/sdn/ipams/Base.js | 25 +++++++++++++++++++++++++
www/manager6/sdn/ipams/NetboxEdit.js | 2 ++
www/manager6/sdn/ipams/PhpIpamEdit.js | 2 ++
3 files changed, 29 insertions(+)
diff --git a/www/manager6/sdn/ipams/Base.js b/www/manager6/sdn/ipams/Base.js
index 1f6eab296c20..e7b607497e25 100644
--- a/www/manager6/sdn/ipams/Base.js
+++ b/www/manager6/sdn/ipams/Base.js
@@ -15,6 +15,31 @@ Ext.define('PVE.panel.SDNIpamBase', {
return values;
},
+ // the external IPAMs keep each zone's subnets in a namespace of their
+ // own once the option is on, named with the prefix
+ zoneNamespaceFields: function () {
+ let me = this;
+
+ return [
+ {
+ xtype: 'proxmoxcheckbox',
+ name: 'zone-namespaces',
+ fieldLabel: gettext('Namespace per Zone'),
+ uncheckedValue: null,
+ checked: false,
+ deleteEmpty: !me.isCreate,
+ },
+ {
+ xtype: 'proxmoxtextfield',
+ name: 'zone-namespace-prefix',
+ fieldLabel: gettext('Namespace Prefix'),
+ emptyText: 'pve',
+ skipEmptyText: true,
+ deleteEmpty: !me.isCreate,
+ },
+ ];
+ },
+
initComponent: function () {
var me = this;
diff --git a/www/manager6/sdn/ipams/NetboxEdit.js b/www/manager6/sdn/ipams/NetboxEdit.js
index 17971026c93e..8183aa5a5618 100644
--- a/www/manager6/sdn/ipams/NetboxEdit.js
+++ b/www/manager6/sdn/ipams/NetboxEdit.js
@@ -53,6 +53,8 @@ Ext.define('PVE.sdn.ipams.NetboxInputPanel', {
},
];
+ me.advancedColumnB = me.zoneNamespaceFields();
+
me.callParent();
},
});
diff --git a/www/manager6/sdn/ipams/PhpIpamEdit.js b/www/manager6/sdn/ipams/PhpIpamEdit.js
index b28232ab67b7..b74da37596a2 100644
--- a/www/manager6/sdn/ipams/PhpIpamEdit.js
+++ b/www/manager6/sdn/ipams/PhpIpamEdit.js
@@ -58,6 +58,8 @@ Ext.define('PVE.sdn.ipams.PhpIpamInputPanel', {
},
];
+ me.advancedColumnB = me.zoneNamespaceFields();
+
me.callParent();
},
});
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH pve-docs v4 13/13] sdn: ipam: document zone namespaces in NetBox and phpIPAM
2026-11-03 18:55 [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (11 preceding siblings ...)
2026-11-03 18:55 ` [PATCH pve-manager v4 12/13] ui: sdn: ipam: add the zone namespace options Hannes Laimer
@ 2026-11-03 18:55 ` Hannes Laimer
2026-11-04 9:56 ` superseded: [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
13 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 18:55 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pvesdn.adoc | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/pvesdn.adoc b/pvesdn.adoc
index c85d86a48112..0cd1b938528d 100644
--- a/pvesdn.adoc
+++ b/pvesdn.adoc
@@ -1287,6 +1287,22 @@ Fingerprint:: The SHA-256 fingerprint of the NetBox API. Can be retrieved with
`openssl x509 -in /etc/ssl/certs/netbox.crt -noout -fingerprint -sha256` when
using the default certificate.
+With the `zone-namespaces` option of the IPAM entry turned on, {pve} creates a
+VRF named `pve-<zone>` for every zone, together with the first subnet of the
+zone, and keeps the prefixes of the zone's subnets in it. This way two zones can
+use the same subnet. The option is off by default and meant to be turned on once
+every node runs a version that knows the VRFs, since a node that does not puts
+an explicit mapping into the global table. Clusters sharing one NetBox set a
+`zone-namespace-prefix` of their own, the names start with `pve` by default. The
+IP addresses allocated in a prefix carry the MAC address of the guest in their
+description. Prefixes created in the global table, by earlier versions or with
+the option off, are still found there and can be moved into the VRF of their
+zone, together with their IP addresses and IP ranges. A zone gets its VRF only
+with the next subnet added to it, so to move prefixes into it earlier, create
+the VRF `pve-<zone>` by hand. With the option on, creating a subnet whose range
+the global table still holds is refused, since a prefix in the VRF would hide
+the addresses recorded in the old one, so move or remove that prefix first.
+
[[pvesdn_ipam_plugin_phpipam]]
phpIPAM Plugin
@@ -1304,6 +1320,23 @@ Token:: An API access token
Section:: An integer ID. Sections are a group of subnets in phpIPAM. Default
installations use `sectionid=1` for customers.
+With the `zone-namespaces` option of the IPAM entry turned on, {pve} creates a
+sub-section named `pve-<zone>` under the configured section for every zone,
+together with the first subnet of the zone, and keeps the zone's subnets in it.
+This way two zones can use the same subnet. The option is off by default and
+meant to be turned on once every node runs a version that knows the
+sub-sections. phpIPAM nests sections one level deep, so a configured section
+that is itself a sub-section gets no sub-sections and the subnets of all zones
+stay in it. Section names are global in phpIPAM, so clusters sharing one set a
+`zone-namespace-prefix` of their own, the names start with `pve` by default.
+Subnets created in the configured section itself, by earlier versions or with
+the option off, are still found there and can be moved into the sub-section of
+their zone. A zone gets its sub-section only with the next subnet added to it,
+so to move subnets into it earlier, create the sub-section under the configured
+section by hand. With the option on, creating a subnet whose range the
+configured section still holds is refused, since a copy in the sub-section would
+hide the addresses recorded in the old one, so move or remove that subnet first.
+
[[pvesdn_config_dns]]
DNS
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* superseded: [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet
2026-11-03 18:55 [PATCH docs/manager/network/qemu-server v4 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (12 preceding siblings ...)
2026-11-03 18:55 ` [PATCH pve-docs v4 13/13] sdn: ipam: document zone namespaces in NetBox and phpIPAM Hannes Laimer
@ 2026-11-04 9:56 ` Hannes Laimer
13 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-04 9:56 UTC (permalink / raw)
To: pve-devel
superseded-by:
https://lore.proxmox.com/pve-devel/20261104095520.398544-1-h.laimer@proxmox.com/
On 2026-11-03 19:56, Hannes Laimer wrote:
> This series addresses some problems in the IPAM code, mostly around
> caching. It also solves the problem that the MAC cache and the lookup
> behind it held a single address per MAC. This is a problem since MACs
> only have to be unique within a vnet and can repeat across vnets, and
> it led to a guest getting the IP of a guest with the same MAC on
> another vnet.
>
> Patches 1-5 address general, mostly cache related problems. Patch 1
> leaves entries without an address in the file and treats them as a
> miss, patch 2 skips the fill when the file changed while the IPAM was
> asked, and patch 3 leaves the cache keys as written, since nodes not
> yet upgraded still use them. Patch 4 replaces a stale reservation
> line instead of dropping it, and patch 5 lets the built-in IPAM
> release an address it no longer holds, so its cache entry is cleared.
>
> Patch 6 adds updating of the dnsmasq reservations when a mapping
> changes, so dnsmasq picks the change up immediately and a guest gets
> its new address without a restart. The edit runs on the guest's node
> under its migration lock, so pve-network now depends on
> pve-guest-common.
>
> Patches 7-8 scope the lookup and the cache to the vnet, so guests
> don't get addresses of another vnet. The cache file now contains a
> `vnets` map next to `macs`. `macs` stays, since nodes not yet
> upgraded read and write only it, and keeps holding one address per
> MAC as before. Every write goes into both maps. A lookup answers
> from `vnets` as long as `macs` still names the same address.
> Otherwise it asks the IPAM within the vnet's subnets, since a write
> for the MAC on another vnet or a node not yet upgraded changed or
> released the address in between. An entry only `macs` has is one
> from before, it answers for a vnet only with an address in the
> vnet's subnets. A MAC with an address of the same family on two
> vnets can match `macs` on one of them at most, so a start on the
> other asks the IPAM. So an old entry answers only for the vnet it
> was written for. The one exception is the same or an overlapping range
> on two vnets, where an old entry answers for the other vnet too, as
> it did before, until one of the two has an entry of its own or the
> address is released.
>
> Patch 9 adds separation by zone for the two external IPAM providers
> we support, since two zones can use the same range. It is behind a
> `zone-namespaces` option of the IPAM entry, off by default, to be
> turned on once every node runs this version.
> - NetBox: each zone gets its own `pve-<zone>` VRF, and the prefixes
> of its subnets go in there
> - phpIPAM: each zone gets a sub-section `pve-<zone>` under the
> configured one, and its subnets go in there(only if the configured
> section is not a sub-section already, phpIPAM only allows 1 deep
> nesting)
> The names start with a `zone-namespace-prefix` of the IPAM entry,
> `pve` by default, so clusters sharing an IPAM stay apart. Patch 9
> also gives phpIPAM the lookup by MAC, which failed before.
>
> Patch 10 is qemu-server's side. An edit and a migration of the guest
> exclude each other since patch 6. An edit through a node not yet
> upgraded still goes through during one, and it writes the IPAM and
> the cache but no reservation. A resume now pushes the current mapping
> of each NIC from the cache, and a migration resumes the target after
> the config moved, so the target serves what the cache vouches for.
> The patch needs the pve-network of this series.
>
> Patches 11-12 are pve-manager's side, the DHCP tree keeping the
> subnets of two zones with one range apart, it merged them under the
> first zone before, and the two options in the NetBox and phpIPAM
> dialogs. Patch 12 needs the pve-network of this series.
>
> Patch 13 just adds docs explaining this for the two external IPAMs.
>
> This is not a breaking change, existing assignments stay as they are
> and keep answering for their vnet. Cache entries from before keep
> their addresses, since nodes not yet upgraded still use them, and
> answer for a vnet with the addresses in its subnets, so a running
> guest keeps its reservation. Records in an external IPAM stay where
> they are, in NetBox's global table or in phpIPAM's configured section.
> Like the cache, an IPAM lookup first checks the zone's VRF or section
> for the subnet and falls back to the global table or the configured
> section. Unlike with the cache, migration can be done manually, by
> moving a prefix together with its addresses and IP ranges into the
> zone's VRF in NetBox, or a subnet into the zone's sub-section in
> phpIPAM, since the lookup checks those first. Migration is not
> necessary though. With the option on, creating a subnet creates the
> VRF or sub-section if it doesn't exist yet, nothing else does. A
> subnet whose range is still in the global table or the configured
> section is then refused, since a copy would hide the records in it.
> The error says to move that prefix or subnet into the VRF or
> sub-section of its zone, or to remove it. We don't move anything and
> we don't have to. With the option off nothing changes in the external
> IPAM, so a rolling upgrade is safe, and a node not yet upgraded never
> meets a subnet in a VRF or sub-section.
>
> v4:
> - add IPAM config flag to use zone scoped layout, tried avoiding this,
> but that added much more problems than it solved
> - a mapping edit and a migration of the guest exclude each other
> - the resume of a guest pushes its reservations from the cache only
> - the same address on two vnets of a MAC is checked with the IPAM at
> every start
> - a prefix option of the IPAM entry names the zone namespaces, pve
> by default, so clusters sharing an IPAM stay apart
> - pve-manager: the two options in the dialogs, and the DHCP tree
> keeps the subnets of two zones with one range apart
>
> v3:
> - a start asks the IPAM when the MAC sits on two vnets and the cache
> can't vouch for it
> - ask the IPAM for the changed MAC before walking the reservations,
> so its stale line is replaced
> - phpIPAM: fail subnet creation on api error
> - an API mapping edit fails when the reservation update does, the
> records are written by then
>
> v2:
> - the cache and the lookup are kept per vnet, the old map stays
> for nodes not yet upgraded
> - entries from before are left as written, a fill is skipped when
> the file changed meanwhile
> - a release leaves a record that belongs to another MAC alone
> - each zone gets its own VRF or section in the external IPAMs, and
> phpIPAM gets the lookup by MAC
> - an edit for a guest whose node is offline runs here, and the
> reservation rewrite reads the cache once
> - the target of a live migration catches up when the guest resumes
>
>
> pve-network:
>
> Hannes Laimer (9):
> sdn: ipam: stop caching IPAM misses
> sdn: ipam: cache a MAC lookup's answer under the cluster lock
> sdn: ipam: read the MAC cache case-insensitively
> sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it
> sdn: ipam: release an address the built-in IPAM no longer holds
> sdn: dhcp: update reservations when IP mappings change
> sdn: ipam: look a MAC up within the vnet's subnets
> sdn: ipam: keep the MAC cache per vnet
> sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM
>
> debian/control | 2 +
> src/PVE/API2/Network/SDN/Ips.pm | 108 ++-
> src/PVE/Network/SDN/Dhcp.pm | 56 +-
> src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 163 +++--
> src/PVE/Network/SDN/Dhcp/Plugin.pm | 8 +-
> src/PVE/Network/SDN/Ipams.pm | 188 ++++-
> src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 239 +++++--
> src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 27 +-
> src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 245 ++++++-
> src/PVE/Network/SDN/Ipams/Plugin.pm | 24 +-
> src/PVE/Network/SDN/Subnets.pm | 14 +-
> src/PVE/Network/SDN/Vnets.pm | 21 +-
> src/test/ipams/netbox/expected.add_ip | 2 +-
> .../ipams/netbox/expected.add_ip_notgateway | 2 +-
> src/test/ipams/netbox/expected.update_ip | 2 +-
> src/test/run_test_ipams.pl | 582 +++++++++++++++
> src/test/run_test_vnets_blackbox.pl | 664 +++++++++++++++++-
> 17 files changed, 2116 insertions(+), 231 deletions(-)
>
>
> qemu-server:
>
> Hannes Laimer (1):
> network: sdn: push the DHCP mappings of a guest when it resumes
>
> src/PVE/QemuServer/Network.pm | 17 +++++++++++++++++
> src/PVE/QemuServer/RunState.pm | 2 ++
> 2 files changed, 19 insertions(+)
>
>
> pve-manager:
>
> Hannes Laimer (2):
> ui: sdn: dhcp tree: do not merge the same subnet in different zones
> ui: sdn: ipam: add the zone namespace options
>
> www/manager6/sdn/ipams/Base.js | 25 +++++++++++++++++++++++++
> www/manager6/sdn/ipams/NetboxEdit.js | 2 ++
> www/manager6/sdn/ipams/PhpIpamEdit.js | 2 ++
> www/manager6/tree/DhcpTree.js | 8 +++++---
> 4 files changed, 34 insertions(+), 3 deletions(-)
>
>
> pve-docs:
>
> Hannes Laimer (1):
> sdn: ipam: document zone namespaces in NetBox and phpIPAM
>
> pvesdn.adoc | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
>
> Summary over all repositories:
> 24 files changed, 2202 insertions(+), 234 deletions(-)
>
^ permalink raw reply [flat|nested] 15+ messages in thread