* [PATCH pve-network v2 01/11] sdn: ipam: stop caching IPAM misses
2026-11-02 14:56 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
@ 2026-11-02 14:56 ` Hannes Laimer
2026-11-02 14:56 ` [PATCH pve-network v2 02/11] sdn: ipam: write the lookup's cache entry under the cluster lock Hannes Laimer
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Hannes Laimer @ 2026-11-02 14:56 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. An
empty answer is no longer stored, and an entry without an address, as
earlier misses left them, no longer counts 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 9f4c424f3d2f..fd3cfa949108 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);
@@ -193,7 +194,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) = @_;
@@ -321,6 +322,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" });
}
@@ -421,6 +450,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] 12+ messages in thread* [PATCH pve-network v2 02/11] sdn: ipam: write the lookup's cache entry under the cluster lock
2026-11-02 14:56 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-02 14:56 ` [PATCH pve-network v2 01/11] sdn: ipam: stop caching IPAM misses Hannes Laimer
@ 2026-11-02 14:56 ` Hannes Laimer
2026-11-02 14:56 ` [PATCH pve-network v2 03/11] sdn: ipam: read the MAC cache case-insensitively Hannes Laimer
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Hannes Laimer @ 2026-11-02 14:56 UTC (permalink / raw)
To: pve-devel
The lookup that stores the IPAM's answer wrote the whole cache back
without the cluster lock the other writers take. A change made
between its read and its write was lost. The lookup now takes the
lock and reads the cache again before it stores its answer. The IPAM
is still asked outside the lock, since the lock is cluster-wide and a
request to an external IPAM does not belong under it. A cache write
that lands while the IPAM is asked may be newer than the answer, so
the answer is then returned without being stored, and the next lookup
stores it.
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 fd3cfa949108..0362b7ca2f8d 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -473,6 +473,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] 12+ messages in thread* [PATCH pve-network v2 03/11] sdn: ipam: read the MAC cache case-insensitively
2026-11-02 14:56 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-02 14:56 ` [PATCH pve-network v2 01/11] sdn: ipam: stop caching IPAM misses Hannes Laimer
2026-11-02 14:56 ` [PATCH pve-network v2 02/11] sdn: ipam: write the lookup's cache entry under the cluster lock Hannes Laimer
@ 2026-11-02 14:56 ` Hannes Laimer
2026-11-02 14:56 ` [PATCH pve-network v2 04/11] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it Hannes Laimer
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Hannes Laimer @ 2026-11-02 14:56 UTC (permalink / raw)
To: pve-devel
Generated guest MACs are upper case, but the API accepts either
spelling, and the cache was keyed by the 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 its entry under every spelling the file holds. The keys stay
as written, since nodes not yet upgraded look an entry up under the
spelling their caller hands them, and a MAC new to the file is
written in lower case. Two spellings that disagree on an address are
both left out of the lookup, since one of them is stale and the cache
cannot tell which, so the next lookup asks the IPAM. The dnsmasq
reservation file is written and read with its MACs in lower case as
well, since its lines from before keep their 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..d3b78fe9a894 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 keyed 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 0362b7ca2f8d..e9c9bbb6be32 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -193,7 +193,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 {
@@ -484,6 +484,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] 12+ messages in thread* [PATCH pve-network v2 04/11] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it
2026-11-02 14:56 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (2 preceding siblings ...)
2026-11-02 14:56 ` [PATCH pve-network v2 03/11] sdn: ipam: read the MAC cache case-insensitively Hannes Laimer
@ 2026-11-02 14:56 ` Hannes Laimer
2026-11-02 14:56 ` [PATCH pve-network v2 05/11] sdn: ipam: release an address the built-in IPAM no longer holds Hannes Laimer
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Hannes Laimer @ 2026-11-02 14:56 UTC (permalink / raw)
To: pve-devel
A reservation rewrite dropped every line that no longer matched the
MAC cache and wrote nothing in its place. The guest behind such a
line was unknown to dnsmasq until its next start, so its requests
were ignored. The line is now replaced by the cache's current one.
Only a MAC the cache holds no address for is dropped.
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 d3b78fe9a894..a1e4c70f9ba4 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. Dropping it left 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 e9c9bbb6be32..368ed5565554 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -1079,4 +1079,32 @@ run_test(
2,
);
+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] 12+ messages in thread* [PATCH pve-network v2 05/11] sdn: ipam: release an address the built-in IPAM no longer holds
2026-11-02 14:56 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (3 preceding siblings ...)
2026-11-02 14:56 ` [PATCH pve-network v2 04/11] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it Hannes Laimer
@ 2026-11-02 14:56 ` Hannes Laimer
2026-11-02 14:56 ` [PATCH pve-network v2 06/11] sdn: dhcp: update reservations when ip mappings change Hannes Laimer
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Hannes Laimer @ 2026-11-02 14:56 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. The entry kept
answering with the address, and the API could not delete the mapping
either. NetBox and phpIPAM warn and release nothing in that case. The
built-in IPAM now does the same, so the cache entry is cleared and the
API deletion succeeds.
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 368ed5565554..1fa3b55cae18 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -533,6 +533,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] 12+ messages in thread* [PATCH pve-network v2 06/11] sdn: dhcp: update reservations when ip mappings change
2026-11-02 14:56 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (4 preceding siblings ...)
2026-11-02 14:56 ` [PATCH pve-network v2 05/11] sdn: ipam: release an address the built-in IPAM no longer holds Hannes Laimer
@ 2026-11-02 14:56 ` Hannes Laimer
2026-11-02 14:56 ` [PATCH pve-network v2 07/11] sdn: ipam: look a MAC up within the vnet's subnets Hannes Laimer
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Hannes Laimer @ 2026-11-02 14:56 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 kept 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 whose
NIC carries the MAC on that vnet. The other nodes catch up when the
guest starts on one of them. A failed update only warns, since the
IPAM records are already written.
The rewrite refreshes the cluster state and reads the cache only
once it holds the file's lock, so a rewrite that ran in between is
not undone. 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>
---
src/PVE/API2/Network/SDN/Ips.pm | 28 +++++
src/PVE/Network/SDN/Dhcp.pm | 56 ++++++----
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 104 ++++++++++-------
src/PVE/Network/SDN/Dhcp/Plugin.pm | 6 +-
src/PVE/Network/SDN/Vnets.pm | 5 +-
src/test/run_test_vnets_blackbox.pl | 168 ++++++++++++++++++++++++++--
6 files changed, 294 insertions(+), 73 deletions(-)
diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
index 5ff05e75e30a..b1faee71c6ba 100644
--- a/src/PVE/API2/Network/SDN/Ips.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -3,9 +3,11 @@ package PVE::API2::Network::SDN::Ips;
use strict;
use warnings;
+use PVE::Cluster;
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 +15,29 @@ 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;
+};
+
__PACKAGE__->register_method({
name => 'ipdelete',
path => '',
method => 'DELETE',
description => 'Delete IP Mappings in a VNet',
protected => 1,
+ proxyto_callback => $proxy_to_guest_node,
permissions => {
check => ['perm', '/sdn/zones/{zone}/{vnet}', ['SDN.Allocate']],
},
@@ -46,6 +65,8 @@ __PACKAGE__->register_method({
eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
die "$@\n" if $@;
+ PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+
return undef;
},
});
@@ -56,6 +77,7 @@ __PACKAGE__->register_method({
method => 'POST',
description => 'Create IP Mapping in a VNet',
protected => 1,
+ proxyto_callback => $proxy_to_guest_node,
permissions => {
check => ['perm', '/sdn/zones/{zone}/{vnet}', ['SDN.Allocate']],
},
@@ -82,6 +104,8 @@ __PACKAGE__->register_method({
PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, undef);
+ PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+
return undef;
},
});
@@ -91,6 +115,7 @@ __PACKAGE__->register_method({
method => 'PUT',
description => 'Update IP Mapping in a VNet',
protected => 1,
+ proxyto_callback => $proxy_to_guest_node,
permissions => {
check => ['perm', '/sdn/zones/{zone}/{vnet}', ['SDN.Allocate']],
},
@@ -132,6 +157,9 @@ __PACKAGE__->register_method({
}
die "$error\n" if $error;
+
+ PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+
return undef;
},
});
diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
index 65e40d40465a..4650270df09f 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, so a failed update only warns
+ warn "could not update dhcp mapping for $mac: $@" 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} 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 undef;
}
sub regenerate_config {
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index a1e4c70f9ba4..4767fb5162ad 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 keyed 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. Dropping it left 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,21 @@ 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);
if ($@) {
- warn "Unable to add $mac to the dnsmasq configuration: $@\n";
+ warn "Unable to update $mac in the dnsmasq configuration: $@\n";
return;
}
- my $service_name = "dnsmasq\@$dhcpid";
- systemctl_service('reload', $service_name) if $reload;
- update_lease($dhcpid, $ip4, $mac);
+ systemctl_service('reload', "dnsmasq\@$dhcpid") if $changed;
+
+ 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..a3a5ae24403f 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
+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 c327a4beba81..8453a2781ae9 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -211,6 +211,8 @@ 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;
+ PVE::Network::SDN::Dhcp::update_mapping($vnetid, $mac) if $ip4 || $ip6;
+
return ($ip4, $ip6);
}
@@ -225,11 +227,12 @@ 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;
+ PVE::Network::SDN::Dhcp::update_mapping($vnetid, $mac) if $ip4 || $ip6;
}
1;
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 1fa3b55cae18..a8886ada04ed 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' => {
@@ -221,7 +222,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 { },
);
@@ -240,10 +243,18 @@ $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_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 --------------
@@ -289,6 +300,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 }); };
@@ -359,6 +375,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();
@@ -1110,18 +1136,144 @@ 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_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(
+ PVE::Network::SDN::Dhcp::guest_node_by_mac($vnetid, $mac),
+ 'other',
+ "$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",
+ );
+ 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] 12+ messages in thread* [PATCH pve-network v2 07/11] sdn: ipam: look a MAC up within the vnet's subnets
2026-11-02 14:56 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (5 preceding siblings ...)
2026-11-02 14:56 ` [PATCH pve-network v2 06/11] sdn: dhcp: update reservations when ip mappings change Hannes Laimer
@ 2026-11-02 14:56 ` Hannes Laimer
2026-11-02 14:56 ` [PATCH pve-network v2 08/11] sdn: ipam: keep the MAC cache per vnet Hannes Laimer
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Hannes Laimer @ 2026-11-02 14:56 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, with NetBox the whole inventory, so a lookup for one vnet could
return the address the MAC has on another, and the guest then got no
address allocated on its own vnet. The built-in IPAM and NetBox are
now searched only within the subnets of the vnet the lookup is for.
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 395d72c9494c..c3f014d45585 100644
--- a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
@@ -260,7 +260,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 8453a2781ae9..b1a2192ac767 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -201,7 +201,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 a8886ada04ed..cb1b14b8e960 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -1276,4 +1276,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] 12+ messages in thread* [PATCH pve-network v2 08/11] sdn: ipam: keep the MAC cache per vnet
2026-11-02 14:56 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (6 preceding siblings ...)
2026-11-02 14:56 ` [PATCH pve-network v2 07/11] sdn: ipam: look a MAC up within the vnet's subnets Hannes Laimer
@ 2026-11-02 14:56 ` Hannes Laimer
2026-11-02 14:56 ` [PATCH pve-network v2 09/11] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM Hannes Laimer
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Hannes Laimer @ 2026-11-02 14:56 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, and a zone's dnsmasq
reservations hold a line for each vnet a MAC sits on, each written
from the cache of that vnet. A MAC matches in any spelling.
The map under the MAC alone stays, since nodes not yet upgraded read
and write only it. Every write lands there as well where the MAC has
no address of that family yet, so such a node finds one, and its
entries answer for a vnet only with addresses in its subnets, so a
guest from before keeps its reservation across the upgrade. A
release or an edit on such a node shows only in that map, so a
vnet's address holds only while the map still has the family and
names no other address in the vnet's subnets. Otherwise the IPAM is
asked again. A release checks that the record still belongs to its
MAC, leaves it and its DNS records alone otherwise, and fails when
it cannot check. The API refuses to delete such a mapping.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/API2/Network/SDN/Ips.pm | 10 +-
src/PVE/Network/SDN/Dhcp.pm | 4 +-
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 140 +++++-----
src/PVE/Network/SDN/Dhcp/Plugin.pm | 6 +-
src/PVE/Network/SDN/Ipams.pm | 229 ++++++++++++----
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 | 297 ++++++++++++++++++---
13 files changed, 632 insertions(+), 179 deletions(-)
diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
index b1faee71c6ba..c816dd9941b4 100644
--- a/src/PVE/API2/Network/SDN/Ips.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -62,11 +62,14 @@ __PACKAGE__->register_method({
my $mac = extract_param($param, 'mac');
my $ip = extract_param($param, 'ip');
- eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
+ my $released = eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
die "$@\n" if $@;
PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+ # the cache entry is cleared, the IPAM record belongs to another MAC
+ die "IP '$ip' is not mapped to $mac\n" if defined($released) && !$released;
+
return undef;
},
});
@@ -147,12 +150,13 @@ __PACKAGE__->register_method({
my ($old_ip4, $old_ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnet, $mac);
my $old_ip = (Net::IP::ip_get_version($ip) == 4) ? $old_ip4 : $old_ip6;
- PVE::Network::SDN::Vnets::del_ip($vnet, $old_ip, '', $mac);
+ # a MAC without an address in that family has nothing to release
+ PVE::Network::SDN::Vnets::del_ip($vnet, $old_ip, '', $mac) if defined($old_ip);
eval { PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, $vmid); };
my $error = $@;
- if ($error) {
+ if ($error && defined($old_ip)) {
PVE::Network::SDN::Vnets::add_ip($vnet, $old_ip, '', $mac, $vmid);
}
diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
index 4650270df09f..86bf98249bdb 100644
--- a/src/PVE/Network/SDN/Dhcp.pm
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -41,10 +41,10 @@ sub update_mapping {
return if defined($zone->{nodes}) && !$zone->{nodes}->{ PVE::INotify::nodename() };
PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp})
- ->update_ip_mapping($zoneid, $mac);
+ ->update_ip_mapping($zoneid, $vnetid, $mac);
};
# the records are written, so a failed update only warns
- warn "could not update dhcp mapping for $mac: $@" if $@;
+ warn "could not update DHCP mapping for $mac: $@" if $@;
}
sub guest_node_by_mac {
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index 4767fb5162ad..752588b98352 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -13,7 +13,6 @@ use Net::DBus;
use PVE::Cluster;
use PVE::RESTEnvironment qw(log_warn);
-use PVE::Network::SDN::Ipams;
my $DNSMASQ_CONFIG_ROOT = '/etc/dnsmasq.d';
my $DNSMASQ_DEFAULT_ROOT = '/etc/default';
@@ -54,98 +53,101 @@ sub update_lease {
}
sub update_ip_mapping {
- my ($class, $dhcpid, $mac) = @_;
+ my ($class, $dhcpid, $vnetid, $mac) = @_;
$mac = lc($mac);
my $ethers_file = ethers_file($dhcpid);
my $ethers_tmp_file = "$ethers_file.tmp";
- my ($ip4, $ip6);
- my $reservation = undef;
+ my $ip4;
my $changed = undef;
- my $rewriteFn = sub {
- # the lines are decided from the cache, which this process may hold an
- # old read of, and the local lock does not refresh it
+ my $rewrite_fn = sub {
+ # the lines are decided from the cache and the applied config, which
+ # the local lock does not refresh
PVE::Cluster::cfs_update();
- # read under the ethers lock. A snapshot from before it would sweep a
- # line another push wrote meanwhile, and an older push would put its
- # addresses back
-
- my $macs = PVE::Network::SDN::Ipams::read_macdb()->{macs};
-
- my $entry = $macs->{$mac};
- ($ip4, $ip6) = ($entry->{ip4}, $entry->{ip6}) if $entry;
- if ($ip4 || $ip6) {
- $reservation = $mac;
- $reservation .= ",$ip4" if $ip4;
- $reservation .= ",[$ip6]" if $ip6;
+ # the file holds a line per vnet a MAC sits on, and a line belongs to
+ # the vnet of the applied subnet that holds its address
+ my $vnets = PVE::Network::SDN::Zones::get_vnets($dhcpid, 1) // {};
+ my $subnets = PVE::Network::SDN::Vnets::get_subnets(undef, 1) // {};
+ $subnets = {
+ map { $_ => $subnets->{$_} } grep { $vnets->{ $subnets->{$_}->{vnet} } }
+ keys %$subnets
+ };
+ my $vnet_of = sub {
+ my ($ip) = @_;
+ my (undef, $subnet) =
+ eval { PVE::Network::SDN::Subnets::find_ip_subnet($ip, $subnets) };
+ return $subnet ? $subnet->{vnet} : undef;
+ };
+ my %subnets_of;
+ $subnets_of{ $subnets->{$_}->{vnet} }->{$_} = $subnets->{$_} for keys %$subnets;
+
+ # read under the ethers lock, a read from before it would undo a push
+ # that ran meanwhile. The lines of other MACs are checked against one
+ # read of the cache, so under the lock only the changed MAC may ask the
+ # IPAM
+ my $db = PVE::Network::SDN::Ipams::read_macdb();
+ my $reservation = sub {
+ my ($vnet, $line_mac, $cached_only) = @_;
+ my ($line_ip4, $line_ip6) =
+ $cached_only
+ ? PVE::Network::SDN::Ipams::cached_ips_from_mac(
+ $db,
+ $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);
+ };
file_set_contents($ethers_file, '') if !-e $ethers_file;
open(my $in, '<', $ethers_file) or die "Could not open file '$ethers_file' $!\n";
open(my $out, '>', $ethers_tmp_file)
or die "Could not open file '$ethers_tmp_file' $!\n";
- my $kept = undef;
-
+ my %written;
while (my $line = <$in>) {
chomp($line);
next if $line !~ m/\S/;
- my ($parsed_mac, $parsed_ip1, $parsed_ip2) = split(/,/, $line);
- # a line from before the file was written in lower case keeps its writer's spelling
+ my ($parsed_mac, $parsed_ip) = split(/,/, $line);
$parsed_mac = lc($parsed_mac);
-
- # the MAC's own reservation is rewritten. An unchanged one stays in place
- if ($parsed_mac eq $mac) {
- if (defined($reservation) && !$kept && lc($line) eq $reservation) {
- $kept = 1;
- print $out "$line\n";
- } else {
- $changed = 1;
- }
- next;
- }
-
- my ($parsed_ip4, $parsed_ip6);
- if ($parsed_ip2) {
- $parsed_ip4 = $parsed_ip1;
- $parsed_ip6 = $parsed_ip2;
- } elsif (defined($parsed_ip1) && Net::IP::ip_is_ipv4($parsed_ip1)) {
- $parsed_ip4 = $parsed_ip1;
- } elsif (defined($parsed_ip1)) {
- $parsed_ip6 = $parsed_ip1;
- }
- $parsed_ip6 = $1 if $parsed_ip6 && $parsed_ip6 =~ m/\[(\S+)\]/;
- # a line from before addresses were lower-cased may spell one in upper case
- $parsed_ip6 = lc($parsed_ip6) if $parsed_ip6;
-
- # a line that no longer matches the cache is replaced by the cache's
- # current one. Dropping it left that guest unknown to dnsmasq until
- # its next start
- my $current = $macs->{$parsed_mac};
- if (
- !$current
- || ($parsed_ip4 && $current->{ip4} && $current->{ip4} ne $parsed_ip4)
- || ($parsed_ip6 && $current->{ip6} && lc($current->{ip6}) ne $parsed_ip6)
- ) {
+ $parsed_ip = $1 if defined($parsed_ip) && $parsed_ip =~ m/\[(\S+)\]/;
+ my $vnet = defined($parsed_ip) ? $vnet_of->($parsed_ip) : undef;
+ my $key = defined($vnet) ? "$parsed_mac,$vnet" : undef;
+
+ # a line that no longer matches is replaced, since dropping it left
+ # that guest unknown to dnsmasq until its next start
+ my ($want) =
+ eval { $key && !$written{$key} ? $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";
+ # the changed MAC's own line may have been dropped above, since only
+ # the cache was asked for it, so it is written from the IPAM's answer
+ my $want;
+ ($want, $ip4) = $reservation->($vnetid, $mac);
+ if (defined($want) && !$written{"$mac,$vnetid"}) {
+ print $out "$want\n";
$changed = 1;
}
@@ -157,7 +159,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);
if ($@) {
warn "Unable to update $mac in the dnsmasq configuration: $@\n";
diff --git a/src/PVE/Network/SDN/Dhcp/Plugin.pm b/src/PVE/Network/SDN/Dhcp/Plugin.pm
index a3a5ae24403f..3484df19b6dc 100644
--- a/src/PVE/Network/SDN/Dhcp/Plugin.pm
+++ b/src/PVE/Network/SDN/Dhcp/Plugin.pm
@@ -22,10 +22,10 @@ sub private {
return $defaultData;
}
-# the MAC's records changed, so make the backend serve the current ones. With no
-# record left its reservation goes
+# 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
sub update_ip_mapping {
- my ($class, $dhcpid, $mac) = @_;
+ my ($class, $dhcpid, $vnetid, $mac) = @_;
die 'implement in sub class';
}
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 9a31eaf39f58..30963a61288a 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,25 +47,137 @@ 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. Spellings that disagree on
+# an address are both left out, since one of them is stale
+my sub old_cache_entry {
+ my ($db, $mac) = @_;
+
+ my $entry = {};
+ my $macs = $db->{macs} // {};
+ for my $key (grep { lc($_) eq lc($mac) } sort keys %$macs) {
+ for my $family (qw(ip4 ip6)) {
+ my $ip = $macs->{$key}->{$family} // next;
+ return undef if defined($entry->{$family}) && lc($entry->{$family}) ne lc($ip);
+ $entry->{$family} = $ip;
+ }
+ }
+ return %$entry ? $entry : undef;
+}
+
+# a write lands in the map under the MAC alone as well, under the keys it
+# already has, where the MAC has no address of that family yet. The one
+# address per MAC a node not yet upgraded reads may be the MAC's on another
+# vnet, or the one a guest from before still relies on
+my sub set_old_cache_entry {
my ($db, $mac, $family, $ip) = @_;
my $macs = $db->{macs} //= {};
my @keys = grep { lc($_) eq lc($mac) } keys %$macs;
+ return if grep { defined($macs->{$_}->{$family}) } @keys;
$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 ($db, $mac, $vnetid, $subnets) = @_;
+
+ my $entry = (($db->{vnets} // {})->{$vnetid} // {})->{ lc($mac) } // {};
+ my $old = old_cache_entry($db, $mac) // {};
+ my $claimed = claimed_addresses($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 only, so a vnet's address holds only while that map still has
+# the family under some spelling and names no address in the vnet's subnets
+# that is neither the vnet's own nor another vnet's
+my sub stale_families {
+ my ($db, $mac, $vnetid, $subnets) = @_;
+
+ my $entry = (($db->{vnets} // {})->{$vnetid} // {})->{ lc($mac) } // {};
+ my $macs = $db->{macs} // {};
+ my @keys = grep { lc($_) eq lc($mac) } keys %$macs;
+ my $claimed = claimed_addresses($db, $mac);
+ my (@released, @moved);
+ for my $family (grep { defined($entry->{$_}) } qw(ip4 ip6)) {
+ my @old_ips = grep { defined($_) } map { $macs->{$_}->{$family} } @keys;
+ if (!@old_ips) {
+ push @released, $family;
+ } elsif (grep { !$claimed->{ lc($_) } && in_subnets($_, $subnets) } @old_ips) {
+ push @moved, $family;
+ }
+ }
+ return (\@released, \@moved);
+}
+
+# a family a node not yet upgraded released is dropped from every vnet's
+# entry of the MAC, so each vnet asks the IPAM again
+my sub drop_cache_families {
+ my ($mac, $families) = @_;
+ $mac = lc($mac);
+
+ cfs_lock_file(
+ $macdb_filename,
+ undef,
+ sub {
+ my $db = read_macdb();
+ my $vnets = $db->{vnets} // {};
+ for my $vnetid (keys %$vnets) {
+ my $entry = $vnets->{$vnetid}->{$mac} // next;
+ delete $entry->{$_} for @$families;
+ delete $vnets->{$vnetid}->{$mac} if !%$entry;
+ delete $vnets->{$vnetid} if !%{ $vnets->{$vnetid} };
+ }
+ delete $db->{vnets} if !%$vnets;
+ write_macdb($db);
+ },
+ );
+ die "could not drop the released addresses of $mac from the cache: $@" if $@;
+}
+
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 +185,37 @@ 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,
+ # and takes the address the MAC still has on another vnet
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});
+ my $cleared;
+ for my $key (grep { lc($_) eq $mac } keys %$macs) {
+ my $old = $macs->{$key};
+ next if lc($old->{$family} // '') ne lc($ip);
+ delete $old->{$family};
+ $cleared = 1;
+ delete $macs->{$key} if !defined($old->{ip4}) && !defined($old->{ip6});
+ }
+ if ($cleared) {
+ my ($other) = grep { defined($_) }
+ map { ($_->{$mac} // {})->{$family} } values %{ $db->{vnets} // {} };
+ set_old_cache_entry($db, $mac, $family, $other) if defined($other);
}
write_macdb($db);
},
@@ -168,16 +268,32 @@ 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 ($db, $vnetid, $mac, $subnets) = @_;
+
+ my $cached = cache_entry($db, $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 $db = read_macdb();
+ my ($released, $moved) = stale_families($db, $mac, $vnetid, $subnets);
+ my $stale_kept;
+ if (@$released) {
+ eval { drop_cache_families($mac, $released) };
+ if ($@) {
+ warn "$@";
+ $stale_kept = 1;
+ }
+ PVE::Cluster::cfs_update();
+ }
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 $cached = cache_entry($db, $mac, $vnetid, $subnets);
+ return ($cached->{ip4}, $cached->{ip6}) if $cached && !@$released && !@$moved;
my $plugin_config = get_plugin_config($zone);
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
@@ -185,6 +301,8 @@ 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);
+ # a fill would witness the entries that could not be dropped
+ return ($ip4, $ip6) if $stale_kept;
cfs_lock_file(
$macdb_filename,
@@ -195,9 +313,14 @@ 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;
+ next if !defined($ip);
+ $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 c3f014d45585..af33868f32f6 100644
--- a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
@@ -237,7 +237,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;
@@ -248,6 +248,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(
@@ -257,6 +273,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 1f27fac45db1..1e0cfa619e79 100644
--- a/src/PVE/Network/SDN/Subnets.pm
+++ b/src/PVE/Network/SDN/Subnets.pm
@@ -263,7 +263,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 $@;
}
@@ -322,7 +322,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 $@;
}
@@ -410,18 +410,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);
@@ -431,6 +435,8 @@ sub del_ip {
if ($@) {
warn $@;
}
+
+ return $released;
}
1;
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index b1a2192ac767..fd6ba4491116 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -191,18 +191,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 cb1b14b8e960..7a16468c0eaa 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_file => sub {
+ read_macdb => sub {
return PVE::Network::SDN::Ipams::json_reader(undef, encode_json($test_state->{macdb}));
},
write_macdb => sub {
@@ -491,74 +491,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};
+ delete $test_state->{macdb}->{vnets};
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");
+ ok(!exists $test_state->{macdb}->{vnets}, "$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. The old entry is left
+ # as it is, the IPAM is asked again as long as it disagrees
+ $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.151' } },
+ vnets => { $vnetid => { $mac => { ip4 => '10.0.0.50' } } },
+ },
+ "$test_name: the answer replaces the vnet's entry and the released family 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
@@ -1128,15 +1156,19 @@ 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),
@@ -1297,8 +1329,203 @@ 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, and a write for the MAC on another vnet
+ # leaves it in place, a guest from before may still rely on it
+ $test_state->{macdb} = { macs => { $mac => { ip4 => '10.2.0.60' } } };
+ 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.60', undef],
+ "$test_name: an old entry still answers for the other vnet",
+ );
+ is_deeply(
+ $test_state->{macdb}->{macs},
+ { $mac => { ip4 => '10.2.0.60' } },
+ "$test_name: and the write for the MAC elsewhere left it in place",
+ );
+
+ # the map under the MAC alone is what a node not yet upgraded releases
+ # in, so a vnet's entry holds only while that map still has the family,
+ # whichever vnet's address it names
+ $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('vnet1', $mac)],
+ ['10.1.0.50', undef],
+ "$test_name: the other vnet's address in the old map keeps an entry trusted",
+ );
+ delete $test_state->{macdb}->{macs};
+ like(
+ (PVE::Network::SDN::Vnets::get_ips_from_mac('vnet1', $mac))[0],
+ qr/^10\.1\.0\.5[12]$/,
+ "$test_name: without the family in the old map the IPAM is asked",
+ );
+ ok(
+ !exists $test_state->{macdb}->{vnets}->{vnet2},
+ "$test_name: and the MAC's entries on the other vnets are dropped",
+ );
+
+ # 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 keeps
+ # it without asking
+ 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}->{vnets}->{vnet3}->{$mac} = { ip4 => '10.1.0.71' };
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet3', $mac)],
+ ['10.1.0.71', undef],
+ "$test_name: and does not make a vnet's own entry a miss",
+ );
+
+ # a release on an upgraded node hands the old map the address the MAC
+ # still has on another vnet
+ $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 => { $mac => { ip4 => '10.1.0.52' } },
+ vnets => { vnet1 => { $mac => { ip4 => '10.1.0.52' } } },
+ },
+ "$test_name: a release leaves the other vnet's address in the old map",
+ );
}
run_test(\&test_ipam_per_vnet);
+sub test_ipam_edit_stale_entry {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+ my $other = "da:65:8f:18:9b:70";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+ # a node not yet upgraded released the MAC's address and handed it to
+ # another guest, while the vnet's cache entry still holds it. The entry
+ # is not trusted, so the edit releases nothing and records the new address
+ create_ip({ zone => $zoneid, vnet => $vnetid, mac => $other, ip => '10.0.0.50' });
+ $test_state->{macdb}->{vnets}->{$vnetid}->{$mac} = { ip4 => '10.0.0.50' };
+
+ PVE::API2::Network::SDN::Ips->ipupdate(
+ { zone => $zoneid, vnet => $vnetid, mac => $mac, ip => '10.0.0.60' });
+ is(
+ get_ip4(get_ips_from_mac($other)),
+ '10.0.0.50',
+ "$test_name: an edit leaves the other guest's record alone",
+ );
+ is(get_ip4(get_ips_from_mac($mac)), '10.0.0.60', "$test_name: and records the new address");
+ is(
+ $test_state->{macdb}->{vnets}->{$vnetid}->{$mac}->{ip4},
+ '10.0.0.60',
+ "$test_name: which the cache holds now",
+ );
+
+ # a deletion of a mapping the IPAM does not hold is refused
+ $test_state->{macdb}->{vnets}->{$vnetid}->{$mac} = { ip4 => '10.0.0.50' };
+ eval {
+ PVE::API2::Network::SDN::Ips->ipdelete(
+ { zone => $zoneid, vnet => $vnetid, mac => $mac, ip => '10.0.0.50' });
+ };
+ like($@, qr/not mapped to/, "$test_name: a deletion for another guest's address is refused");
+}
+
+run_test(\&test_ipam_edit_stale_entry);
+
+sub test_dnsmasq_reservation_per_vnet {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ # the same MAC on two vnets of one zone has a reservation on each, and a
+ # start on one vnet changes that vnet's line only
+ create_zone({ type => "simple", dhcp => "dnsmasq", ipam => "pve", zone => $zoneid });
+ for my $n (1, 2) {
+ create_dhcp_vnet($zoneid, "vnet$n", "10.$n.0.0/24");
+ create_ip({ zone => $zoneid, vnet => "vnet$n", mac => $mac, ip => "10.$n.0.50" });
+ }
+
+ my $reservations = sub {
+ return [sort grep { $_ } split(/\n/, PVE::Tools::file_get_contents($TMP_ETHERS_FILE))];
+ };
+
+ nic_start("vnet$_", $mac, "999", "testhostname") for (1, 2, 1);
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.50", "$mac,10.2.0.50"],
+ "$test_name: each vnet keeps its reservation",
+ );
+
+ PVE::API2::Network::SDN::Ips->ipupdate(
+ { zone => $zoneid, vnet => 'vnet1', mac => $mac, ip => '10.1.0.51' });
+ nic_start('vnet1', $mac, "999", "testhostname");
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.51", "$mac,10.2.0.50"],
+ "$test_name: an edit replaces the reservation of its vnet only",
+ );
+
+ # the other lines are checked against the cache alone, so a line neither
+ # map holds is dropped without the IPAM being asked
+ delete $test_state->{macdb}->{vnets}->{vnet2};
+ delete $test_state->{macdb}->{macs};
+ nic_start('vnet1', $mac, "999", "testhostname");
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.51"],
+ "$test_name: a line the cache does not hold is dropped",
+ );
+
+ # the changed MAC's own line is answered by the IPAM when the cache does
+ # not hold it
+ delete $test_state->{macdb}->{vnets}->{vnet1};
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, 'vnet1', $mac);
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.51"],
+ "$test_name: the changed MAC's line is kept without a cache entry",
+ );
+}
+
+run_test(\&test_dnsmasq_reservation_per_vnet);
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH pve-network v2 09/11] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM
2026-11-02 14:56 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (7 preceding siblings ...)
2026-11-02 14:56 ` [PATCH pve-network v2 08/11] sdn: ipam: keep the MAC cache per vnet Hannes Laimer
@ 2026-11-02 14:56 ` Hannes Laimer
2026-11-02 14:56 ` [PATCH qemu-server v2 10/11] network: sdn: push the DHCP mappings of a guest when it resumes Hannes Laimer
2026-11-02 14:56 ` [PATCH pve-docs v2 11/11] sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM Hannes Laimer
10 siblings, 0 replies; 12+ messages in thread
From: Hannes Laimer @ 2026-11-02 14:56 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. A zone's subnets now go
into a VRF in NetBox and into a section under the configured section
in phpIPAM, both named pve-<zone> and created with the first subnet
added to the zone. A subnet being added goes into the zone's VRF or
section, also when another zone holds the same range in its own. A
range the global table or the configured section still holds is
refused, since a copy would hide the records in it. The error says to
move that prefix or subnet into the zone's VRF or section, or to
remove it. phpIPAM nests sections one level deep, so under a
configured section that is a sub-section itself the zones share the
configured section as before.
Subnets from before stay in the global table or the configured
section and are still found there, so nothing has to be moved. The
phpIPAM lookup by MAC, unimplemented until now, returns the addresses
recorded for the MAC in the vnet's subnets, and its gateway check
answers for the subnet it is asked about and for no other. A failed
lookup stops the operation, only a not found answer means there is
no such section, subnet or record.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 164 ++++++---
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 198 +++++++++--
src/test/ipams/netbox/expected.add_ip | 2 +-
.../ipams/netbox/expected.add_ip_notgateway | 2 +-
src/test/ipams/netbox/expected.add_subnet | 2 +-
src/test/ipams/netbox/expected.update_ip | 2 +-
src/test/run_test_ipams.pl | 317 ++++++++++++++++++
7 files changed, 612 insertions(+), 75 deletions(-)
diff --git a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
index 84ce4382e0e7..f2d1af55d154 100644
--- a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
@@ -40,18 +40,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,18 +69,44 @@ sub del_dhcp_range {
# Plugin implementation
+my sub zone_vrf_name {
+ my ($zoneid) = @_;
+
+ return "pve-$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)) {
+ # a subnet being added is new to its zone, so it goes into the zone's VRF,
+ # and a prefix from before in the global table is not taken over, since
+ # a copy in the VRF would hide its records
+ my $vrf_id = get_zone_namespace($plugin_config, $subnet->{zone}, $noerr);
+ if (defined($vrf_id) && defined(get_prefix_id($plugin_config, $cidr, $vrf_id, $noerr))) {
return if $noerr;
die "prefix $cidr already exists in netbox";
}
+ if (defined(get_prefix_id($plugin_config, $cidr, undef, $noerr))) {
+ return if $noerr;
+ my $vrf = zone_vrf_name($subnet->{zone});
+ die "prefix $cidr exists in the global table of NetBox, move it with its"
+ . " addresses and ranges into VRF $vrf or remove it first\n";
+ }
+
+ $vrf_id //= add_zone_namespace($plugin_config, $subnet->{zone}, $noerr);
+ return if !defined($vrf_id) && $noerr;
- eval { netbox_api_request($plugin_config, "POST", "/ipam/prefixes/", { prefix => $cidr }); };
+ eval {
+ netbox_api_request(
+ $plugin_config,
+ "POST",
+ "/ipam/prefixes/",
+ { prefix => $cidr, vrf => $vrf_id },
+ );
+ };
if ($@) {
return if $noerr;
die "error adding subnet to ipam: $@";
@@ -91,7 +114,7 @@ sub add_subnet {
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);
+ add_dhcp_range($plugin_config, $dhcp_range, $vrf_id, $noerr);
}
}
@@ -122,6 +145,8 @@ sub update_subnet {
my $to_delete_ids = ();
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr);
+
# 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 +155,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 +170,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 +180,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 +188,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 +201,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 +240,8 @@ sub add_ip {
$description = "mac:$mac";
}
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr);
+
eval {
my $params = {
address => "$ip/$mask",
@@ -222,6 +249,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 +257,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 +287,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);
+ 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 +303,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,7 +317,7 @@ sub add_next_freeip {
my $cidr = $subnet->{cidr};
- my $internalid = get_prefix_id($plugin_config, $cidr, $noerr);
+ my ($internalid) = subnet_prefix($plugin_config, $subnet, $noerr);
# definedness check, because ID could be 0
if (!defined($internalid)) {
@@ -323,7 +353,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);
+ my $internalid = get_iprange_id($plugin_config, $range, $vrf_id, $noerr);
# definedness check, because ID could be 0
if (!defined($internalid)) {
@@ -364,7 +395,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);
+ 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 +429,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 +475,59 @@ 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($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($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
+sub subnet_prefix {
+ my ($config, $subnet, $noerr) = @_;
+
+ 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);
+ }
+ return (get_prefix_id($config, $subnet->{cidr}, undef, $noerr), 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 +538,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 +559,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 +573,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 +587,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 af33868f32f6..32d961e9efb6 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;
@@ -38,6 +41,15 @@ sub options {
# Plugin implementation
+# a zone's section is a sub-section of the configured one, so SDN writes only
+# under the section it was given. phpIPAM wants at least three characters in a
+# section name, a zone id may have two
+my sub zone_section_name {
+ my ($zoneid) = @_;
+
+ return "pve-$zoneid";
+}
+
sub add_subnet {
my ($class, $plugin_config, $subnetid, $subnet, $noerr) = @_;
@@ -47,15 +59,33 @@ 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];
+ # 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 = 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) };
+ 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($subnet->{zone});
+ die "subnet $cidr exists in section $plugin_config->{section} of phpIPAM, move it"
+ . " into sub-section $name of that section 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,
@@ -85,7 +115,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
@@ -118,7 +148,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,
@@ -138,7 +168,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;
}
@@ -165,7 +195,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 = {
@@ -196,7 +226,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,
@@ -246,7 +276,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 {
@@ -277,6 +307,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) = @_;
@@ -288,7 +336,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",
@@ -297,19 +353,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);
@@ -342,27 +398,105 @@ 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($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";
+ }
+ my $section = $result->{data};
+ die "section $name is not under section $plugin_config->{section}\n"
+ 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($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 ($@) {
+ return if $noerr;
+ die "could not create section $name: $@";
+ }
+ 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/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.add_subnet b/src/test/ipams/netbox/expected.add_subnet
index 6f21265a2a18..7c2785adf9cd 100644
--- a/src/test/ipams/netbox/expected.add_subnet
+++ b/src/test/ipams/netbox/expected.add_subnet
@@ -1,5 +1,5 @@
bless( {
- '_content' => '{"prefix":"10.0.0.0/24"}',
+ '_content' => '{"prefix":"10.0.0.0/24","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..997f48669a85 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,143 @@ 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' },
+ 'z-10.0.0.0-24',
+ $subnet,
+ );
+ };
+ like($@, qr/move it .* into VRF pve-z/, '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 },
+ 'z-10.0.0.0-24',
+ $subnet,
+ );
+ };
+ like(
+ $@,
+ qr/move it into sub-section pve-z/,
+ 'phpipam refuses a range the configured section holds',
+ );
+ is(scalar(@requests), 0, 'and creates nothing');
+}
+
+# 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 +408,182 @@ foreach my $path (@plugins) {
}
}
+# 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 };
+ },
+ );
+ 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 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' };
+
+ $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 not the zone's
+ $sdn_module->mock(api_request => sub { return { data => { id => 7, masterSection => 2 } }; });
+ eval { $lookup->({ url => 'u', token => 't', section => 1 }, 'z', []) };
+ like($@, qr/not under section 1/, 'a section outside the configured one is refused');
+ $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 },
+ '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 },
+ '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',
+ );
+}
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH qemu-server v2 10/11] network: sdn: push the DHCP mappings of a guest when it resumes
2026-11-02 14:56 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (8 preceding siblings ...)
2026-11-02 14:56 ` [PATCH pve-network v2 09/11] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM Hannes Laimer
@ 2026-11-02 14:56 ` Hannes Laimer
2026-11-02 14:56 ` [PATCH pve-docs v2 11/11] sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM Hannes Laimer
10 siblings, 0 replies; 12+ messages in thread
From: Hannes Laimer @ 2026-11-02 14:56 UTC (permalink / raw)
To: pve-devel
A mapping edit updates the reservation on the guest's node, taken
from the vmlist, which names the source of a live migration until
the config moves at its end. An edit in that window went to the
source, while the target kept the record it started with.
Every resume now pushes the current mapping of each NIC first. The
last step of a migration resumes the target after the config moved,
so the target catches up there, or at the later resume of a guest
the migration left paused.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/QemuServer/Network.pm | 14 ++++++++++++++
src/PVE/QemuServer/RunState.pm | 2 ++
2 files changed, 16 insertions(+)
diff --git a/src/PVE/QemuServer/Network.pm b/src/PVE/QemuServer/Network.pm
index b4893c015646..88a2290672e6 100644
--- a/src/PVE/QemuServer/Network.pm
+++ b/src/PVE/QemuServer/Network.pm
@@ -272,6 +272,20 @@ sub add_random_macs {
}
}
+# an edit made while the guest migrated went to the source, which the vmlist
+# still named. Every resume pushes the current mapping of each NIC, so the
+# target catches up
+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};
+ PVE::Network::SDN::Dhcp::update_mapping($net->{bridge}, $net->{macaddr});
+ }
+}
+
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] 12+ messages in thread* [PATCH pve-docs v2 11/11] sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM
2026-11-02 14:56 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (9 preceding siblings ...)
2026-11-02 14:56 ` [PATCH qemu-server v2 10/11] network: sdn: push the DHCP mappings of a guest when it resumes Hannes Laimer
@ 2026-11-02 14:56 ` Hannes Laimer
10 siblings, 0 replies; 12+ messages in thread
From: Hannes Laimer @ 2026-11-02 14:56 UTC (permalink / raw)
To: pve-devel
Every zone's subnets now go into a VRF or a sub-section named after
the zone, which is what an admin sees in the IPAM, and a prefix or
subnet from before stays where it is.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pvesdn.adoc | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/pvesdn.adoc b/pvesdn.adoc
index 3fd3533ad07f..75ac781e8c92 100644
--- a/pvesdn.adoc
+++ b/pvesdn.adoc
@@ -1287,6 +1287,18 @@ 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.
+{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 IP addresses allocated in a prefix carry
+the MAC address of the guest in their description. Prefixes that earlier
+versions created in the global table are still found there and can be moved
+into the VRF of their zone, together with their IP addresses and IP ranges. A
+zone from before 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. 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 +1316,19 @@ Token:: An API access token
Section:: An integer ID. Sections are a group of subnets in phpIPAM. Default
installations use `sectionid=1` for customers.
+{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. 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. Subnets that
+earlier versions created in the configured section itself are still found there
+and can be moved into the sub-section of their zone. A zone from before gets its
+sub-section only with the next subnet added to it, so to move subnets into it
+earlier, create the sub-section `pve-<zone>` under the configured section by
+hand. 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] 12+ messages in thread