* [PATCH docs/manager/network/qemu-server v6 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet
@ 2026-11-05 9:35 Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 01/13] sdn: ipam: stop caching IPAM misses Hannes Laimer
` (12 more replies)
0 siblings, 13 replies; 14+ messages in thread
From: Hannes Laimer @ 2026-11-05 9:35 UTC (permalink / raw)
To: pve-devel
This series addresses some problems in the IPAM code, mostly around
caching. It also solves the problem that the MAC cache and the lookup
behind it held a single address per MAC. This is a problem since MACs
only have to be unique within a vnet and can repeat across vnets, and
it led to a guest getting the IP of a guest with the same MAC on
another vnet.
Patches 1-5 address general, mostly cache related problems. Patch 1
leaves entries without an address in the file and treats them as a
miss, patch 2 skips the fill when the file changed while the IPAM was
asked, and patch 3 leaves the cache keys as written, since nodes not
yet upgraded still use them. Patch 4 replaces a stale reservation
line instead of dropping it, and patch 5 lets the built-in IPAM
release an address it no longer holds, so its cache entry is cleared.
Patch 6 adds updating of the dnsmasq reservations when a mapping
changes, so dnsmasq picks the change up immediately and a guest gets
its new address without a restart. The edit runs on the guest's node
under its migration lock, so pve-network now depends on
pve-guest-common. The lock could move to a package below both if we
want that.
Patches 7-8 scope the lookup and the cache to the vnet, so guests
don't get addresses of another vnet. The cache file now contains a
`vnets` map next to `macs`. `macs` stays, since nodes not yet
upgraded read and write only it, and keeps holding one address per
MAC as before. Every write goes into both maps. A lookup answers
from `vnets` as long as `macs` still names the same address.
Otherwise it asks the IPAM within the vnet's subnets, since a write
for the MAC on another vnet or a node not yet upgraded changed or
released the address in between. An entry only `macs` has is one
from before, it answers for a vnet only with an address in the
vnet's subnets. A MAC with an address of the same family on two
vnets can match `macs` on one of them at most, so a start on the
other asks the IPAM. So an old entry answers only for the vnet it
was written for. The one exception is the same or an overlapping range
on two vnets, where an old entry answers for the other vnet too, as
it did before, until one of the two has an entry of its own or the
address is released.
Patch 9 adds separation by zone for the two external IPAM providers
we support, since two zones can use the same range. It is behind a
`zone-namespaces` option of the IPAM entry, off by default, to be
turned on once every node runs this version.
- NetBox: each zone gets its own `pve-<zone>` VRF, and the prefixes
of its subnets go in there
- phpIPAM: each zone gets a sub-section `pve-<zone>` under the
configured one, and its subnets go in there (the option is refused
if the configured section is a sub-section already, phpIPAM only
allows 1 deep nesting)
The names start with a `zone-namespace-prefix` of the IPAM entry,
`pve` by default, so clusters sharing an IPAM stay apart. Patch 9
also gives phpIPAM the lookup by MAC, which failed before.
Patch 10 is qemu-server's side. An edit and a migration of the guest
exclude each other since patch 6. An edit through a node not yet
upgraded still goes through during one, and it writes the IPAM and the
cache but no reservation. A resume now pushes the current mapping of
each NIC, and a migration resumes the target after the config moved,
so the target serves the current addresses. The patch needs the
pve-network of this series.
Patches 11-12 are pve-manager's side, the DHCP tree keeping the
subnets of two zones with one range apart, it merged them under the
first zone before, and the two options in the NetBox and phpIPAM
dialogs. Patch 12 needs the pve-network of this series.
Patch 13 just adds docs explaining this for the two external IPAMs.
This is not a breaking change, existing assignments stay as they are
and keep answering for their vnet. Cache entries from before keep
their addresses, since nodes not yet upgraded still use them, and
answer for a vnet with the addresses in its subnets, so a running
guest keeps its reservation. Records in an external IPAM stay where
they are, in NetBox's global table or in phpIPAM's configured section.
Like the cache, an IPAM lookup with the option on first checks the
zone's VRF or section for the subnet and falls back to the global
table or the configured section. Unlike with the cache, migration can
be done manually, by moving a prefix together with its addresses and
IP ranges into the zone's VRF in NetBox, or a subnet into the zone's
sub-section in phpIPAM, since the lookup checks those first. Migration
is not necessary though. With the option on, creating a subnet creates
the VRF or sub-section if it doesn't exist yet, nothing else does. A
subnet whose range is still in the global table or the configured
section is then refused, since a copy would hide the records in it.
The error says to move that prefix or subnet into the VRF or
sub-section of its zone, or to remove it. We don't move anything and
we don't have to. With the option off nothing changes in the external
IPAM, so a rolling upgrade is safe, and a node not yet upgraded never
meets a subnet in a VRF or sub-section.
v6:
- never write entries lc'd into the old `macs`, this trips up
not-yet-updated nodes
- only ask ipam that errored out once in a pass
- only have a shorter timeout for resume-pushes, the whole resume
fast-path could leave a guest without a reservation. a too slow ipam
now leads to an error in the migrations resume task, the guest runs
fine though
- with the zone-namespace flag off the external ipam lookups are as
before again, first hit in any VRF or section, so manually moved
entries are picked up without the flag
- a resume-push writes the ipam's answer if the cache has nothing for
the MAC
- a resume-push failing on an ipam error fails the task too, after the
guest continued
- a failed update adds the old address back only if it was released
- the pve-guest-common dependency is declared by the api package
v5:
- also check the zone-namespace flag on external ipam lookup, so
manually moved entries are not picked up without the flag set
- for a dhcp update push on migration resume lock non-blocking,
whatever write would be holding the lock writes the same line the
push would have written, and ask the ipam if the cache can't vouch
for the entry
- phpipam: dont allow setting the zone-namespace flag if the configured
section is a sub-section, phpipam only allows 1 deep nesting, and of
the configured section is already 1 deep, we can't create the zone
sections
- refresh vmlist under lock
- migrate resume asks ipam if cache has disagreeing entries
v4:
- add IPAM config flag to use zone scoped layout, tried avoiding this,
but that added much more problems than it solved
- a mapping edit and a migration of the guest exclude each other
- the resume of a guest pushes its reservations from the cache only
- the same address on two vnets of a MAC is checked with the IPAM at
every start
- a prefix option of the IPAM entry names the zone namespaces, pve
by default, so clusters sharing an IPAM stay apart
- pve-manager: the two options in the dialogs, and the DHCP tree
keeps the subnets of two zones with one range apart
v3:
- a start asks the IPAM when the MAC sits on two vnets and the cache
can't vouch for it
- ask the IPAM for the changed MAC before walking the reservations,
so its stale line is replaced
- phpIPAM: fail subnet creation on api error
- an API mapping edit fails when the reservation update does, the
records are written by then
v2:
- the cache and the lookup are kept per vnet, the old map stays
for nodes not yet upgraded
- entries from before are left as written, a fill is skipped when
the file changed meanwhile
- a release leaves a record that belongs to another MAC alone
- each zone gets its own VRF or section in the external IPAMs, and
phpIPAM gets the lookup by MAC
- an edit for a guest whose node is offline runs here, and the
reservation rewrite reads the cache once
- the target of a live migration catches up when the guest resumes
pve-network:
Hannes Laimer (9):
sdn: ipam: stop caching IPAM misses
sdn: ipam: cache a MAC lookup's answer under the cluster lock
sdn: ipam: read the MAC cache case-insensitively
sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it
sdn: ipam: release an address the built-in IPAM no longer holds
sdn: dhcp: update reservations when IP mappings change
sdn: ipam: look a MAC up within the vnet's subnets
sdn: ipam: keep the MAC cache per vnet
sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM
debian/control | 2 +
src/PVE/API2/Network/SDN/Ips.pm | 123 ++-
src/PVE/Network/SDN/Dhcp.pm | 52 +-
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 191 +++--
src/PVE/Network/SDN/Dhcp/Plugin.pm | 8 +-
src/PVE/Network/SDN/Ipams.pm | 188 ++++-
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 244 ++++--
src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 27 +-
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 250 +++++-
src/PVE/Network/SDN/Ipams/Plugin.pm | 24 +-
src/PVE/Network/SDN/Subnets.pm | 14 +-
src/PVE/Network/SDN/Vnets.pm | 21 +-
src/test/run_test_ipams.pl | 620 +++++++++++++++
src/test/run_test_vnets_blackbox.pl | 870 ++++++++++++++++++++-
14 files changed, 2405 insertions(+), 229 deletions(-)
qemu-server:
Hannes Laimer (1):
network: sdn: push the DHCP mappings of a guest when it resumes
src/PVE/QemuServer/Network.pm | 18 ++++++++++++++++++
src/PVE/QemuServer/RunState.pm | 8 ++++++++
2 files changed, 26 insertions(+)
pve-manager:
Hannes Laimer (2):
ui: sdn: dhcp tree: do not merge the same subnet in different zones
ui: sdn: ipam: add the zone namespace options
www/manager6/sdn/ipams/Base.js | 37 +++++++++++++++++++++++++++
www/manager6/sdn/ipams/NetboxEdit.js | 4 +++
www/manager6/sdn/ipams/PhpIpamEdit.js | 4 +++
www/manager6/tree/DhcpTree.js | 8 +++---
4 files changed, 50 insertions(+), 3 deletions(-)
pve-docs:
Hannes Laimer (1):
sdn: ipam: document zone namespaces and the mapping edit
pvesdn.adoc | 44 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
Summary over all repositories:
21 files changed, 2524 insertions(+), 233 deletions(-)
--
Generated by murpp 0.12.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH pve-network v6 01/13] sdn: ipam: stop caching IPAM misses
2026-11-05 9:35 [PATCH docs/manager/network/qemu-server v6 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
@ 2026-11-05 9:35 ` Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 02/13] sdn: ipam: cache a MAC lookup's answer under the cluster lock Hannes Laimer
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Hannes Laimer @ 2026-11-05 9:35 UTC (permalink / raw)
To: pve-devel
A lookup that missed the cache stored the IPAM's empty answer, and
any entry counted as a hit. So a record created later directly in the
IPAM stayed hidden, and a guest start allocated another address.
Entries without an address, which older versions leave in the cache,
no longer count as a hit.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams.pm | 16 ++++++--
src/test/run_test_vnets_blackbox.pl | 58 ++++++++++++++++++++++++++++-
2 files changed, 69 insertions(+), 5 deletions(-)
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 179bdf76defc..7e7bec27b5ed 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -136,16 +136,24 @@ sub get_ips_from_mac {
my ($mac, $zoneid, $zone) = @_;
my $macdb = read_macdb();
- return ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6}) if $macdb->{macs}->{$mac};
+ my $cached = $macdb->{macs}->{$mac};
+ # an entry without an address was left behind by a lookup that missed, and
+ # says nothing about its MAC
+ return ($cached->{ip4}, $cached->{ip6})
+ if $cached && (defined($cached->{ip4}) || defined($cached->{ip6}));
my $plugin_config = get_plugin_config($zone);
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
- ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6}) =
- $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid);
+ my ($ip4, $ip6) = $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid);
+ # an empty answer is not cached, since the record may simply not exist yet
+ return if !defined($ip4) && !defined($ip6);
+
+ $macdb->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4);
+ $macdb->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6);
write_macdb($macdb);
- return ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6});
+ return ($ip4, $ip6);
}
1;
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index d5a78c5b9a16..37eb5567eae6 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -5,6 +5,7 @@ use warnings;
use lib qw(..);
use File::Slurp;
+use JSON;
use List::Util qw(first all);
use NetAddr::IP qw(:lower);
@@ -194,7 +195,7 @@ $mocked_sdn_ipams->mock(
$test_state->{ipam_config} = $cfg;
},
read_macdb => sub {
- return $test_state->{macdb};
+ return PVE::Network::SDN::Ipams::json_reader(undef, encode_json($test_state->{macdb}));
},
write_macdb => sub {
my ($cfg) = @_;
@@ -335,6 +336,34 @@ sub create_subnet {
PVE::API2::Network::SDN::Subnets->create($params);
}
+# a DHCP zone with one vnet, each subnet with its first address as the
+# gateway and a range over its hosts 100 to 200
+sub create_dhcp_zone {
+ my ($zoneid, $vnetid, @cidrs) = @_;
+ create_zone({ type => "simple", dhcp => "dnsmasq", ipam => "pve", zone => $zoneid });
+ create_dhcp_vnet($zoneid, $vnetid, @cidrs);
+}
+
+sub create_dhcp_vnet {
+ my ($zoneid, $vnetid, @cidrs) = @_;
+ create_vnet({ type => "vnet", zone => $zoneid, vnet => $vnetid });
+ create_dhcp_subnet($vnetid, $_) for @cidrs;
+}
+
+sub create_dhcp_subnet {
+ my ($vnetid, $cidr) = @_;
+ my $net = NetAddr::IP->new($cidr);
+ my $addr = sub { my ($ip) = @_; return $ip->version() == 4 ? $ip->addr() : $ip->short(); };
+ create_subnet({
+ type => "subnet",
+ vnet => $vnetid,
+ subnet => $cidr,
+ gateway => $addr->($net->first()),
+ 'dhcp-range' =>
+ ["start-address=" . $addr->($net + 100) . ",end-address=" . $addr->($net + 200)],
+ });
+}
+
sub get_ipam_entries {
return PVE::API2::Network::SDN::Ipams->ipamindex({ ipam => "pve" });
}
@@ -435,6 +464,33 @@ sub test_without_subnet {
}
run_test(\&test_without_subnet);
+sub test_ipam_cache_lookup {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+ my $cache = decode_json(encode_json($test_state->{macdb}));
+ PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is_deeply($test_state->{macdb}, $cache, "$test_name: the miss is not cached");
+
+ # an entry holding no address, left behind by an earlier miss, does not
+ # hide a record created since
+ create_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.50" });
+ $test_state->{macdb}->{macs}->{$mac} = { ip4 => undef, ip6 => undef };
+ my @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is($ips[0], "10.0.0.50", "$test_name: an address-less entry counts as a miss");
+ is(
+ $test_state->{macdb}->{macs}->{$mac}->{ip4},
+ "10.0.0.50",
+ "$test_name: the answer is cached",
+ );
+}
+
+run_test(\&test_ipam_cache_lookup);
+
sub test_nic_join {
my ($test_name, $subnets) = @_;
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH pve-network v6 02/13] sdn: ipam: cache a MAC lookup's answer under the cluster lock
2026-11-05 9:35 [PATCH docs/manager/network/qemu-server v6 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 01/13] sdn: ipam: stop caching IPAM misses Hannes Laimer
@ 2026-11-05 9:35 ` Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 03/13] sdn: ipam: read the MAC cache case-insensitively Hannes Laimer
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Hannes Laimer @ 2026-11-05 9:35 UTC (permalink / raw)
To: pve-devel
On a cache miss the MAC lookup stored the IPAM's answer without the
cluster lock the other cache writers take. It wrote back the whole
cache as read before the request, so a change made in between was
lost.
The lookup now stores the answer under that lock, and only if the
cache has not changed in the meantime. Such a change may be newer
than the answer, so the answer is then returned without being stored.
The IPAM is still asked outside the lock, since holding the lock
during a request to an external IPAM would make every cache write in
the cluster wait for the IPAM's answer.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams.pm | 24 +++++++++++++++++++-----
src/test/run_test_vnets_blackbox.pl | 11 +++++++++++
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 7e7bec27b5ed..ae2c4337ef8d 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -135,8 +135,9 @@ sub complete_sdn_vnet {
sub get_ips_from_mac {
my ($mac, $zoneid, $zone) = @_;
- my $macdb = read_macdb();
- my $cached = $macdb->{macs}->{$mac};
+ PVE::Cluster::cfs_update(); # the lock refreshes, so the snapshot must too
+ my $version = PVE::Cluster::cfs_file_version($macdb_filename);
+ my $cached = read_macdb()->{macs}->{$mac};
# an entry without an address was left behind by a lookup that missed, and
# says nothing about its MAC
return ($cached->{ip4}, $cached->{ip6})
@@ -149,9 +150,22 @@ sub get_ips_from_mac {
# an empty answer is not cached, since the record may simply not exist yet
return if !defined($ip4) && !defined($ip6);
- $macdb->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4);
- $macdb->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6);
- write_macdb($macdb);
+ cfs_lock_file(
+ $macdb_filename,
+ undef,
+ sub {
+ # the IPAM was asked outside the lock, so a write since the miss may
+ # be newer than the answer. The fill is then left to the next lookup
+ return
+ if (PVE::Cluster::cfs_file_version($macdb_filename) // '') ne ($version // '');
+
+ my $db = read_macdb();
+ $db->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4);
+ $db->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6);
+ write_macdb($db);
+ },
+ );
+ warn "$@" if $@;
return ($ip4, $ip6);
}
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 37eb5567eae6..927ab16adba9 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -487,6 +487,17 @@ sub test_ipam_cache_lookup {
"10.0.0.50",
"$test_name: the answer is cached",
);
+
+ # a write to the cache between the miss and the fill, which the file's
+ # version shows, keeps the IPAM's answer out of the cache
+ delete $test_state->{macdb}->{macs}->{$mac};
+ my $version = 0;
+ $mocked_pve_cluster_obj->mock(cfs_file_version => sub { return $version++ });
+ @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ $mocked_pve_cluster_obj->unmock('cfs_file_version');
+ is($ips[0], "10.0.0.50", "$test_name: the lookup still answers");
+ ok(!exists $test_state->{macdb}->{macs}->{$mac},
+ "$test_name: but leaves a changed cache alone");
}
run_test(\&test_ipam_cache_lookup);
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH pve-network v6 03/13] sdn: ipam: read the MAC cache case-insensitively
2026-11-05 9:35 [PATCH docs/manager/network/qemu-server v6 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 01/13] sdn: ipam: stop caching IPAM misses Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 02/13] sdn: ipam: cache a MAC lookup's answer under the cluster lock Hannes Laimer
@ 2026-11-05 9:35 ` Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 04/13] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it Hannes Laimer
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Hannes Laimer @ 2026-11-05 9:35 UTC (permalink / raw)
To: pve-devel
Generated guest MACs are upper case, but the API accepts either
spelling. The cache stored each MAC as its writer spelled it. A
deletion in the other spelling missed the entry, so the cache kept
answering with the deleted address.
Lookups now match a MAC in any spelling, and a write or a deletion
reaches every spelling of it in the cache file. The file keeps its
keys as written, since nodes not yet upgraded look a MAC up by its
exact spelling. A MAC new to the file is written as the caller
spells it.
If two spellings disagree on an address, the cache cannot tell which
is stale. A lookup then ignores them, asks the IPAM and writes its
answer under both.
Adding a dnsmasq reservation checks every line of its file against
the cache and drops the ones that do not match. The check ignores case
as well, since an existing line keeps its writer's spelling.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 3 ++
src/PVE/Network/SDN/Ipams.pm | 72 +++++++++++++++++++++--------
src/test/run_test_vnets_blackbox.pl | 51 +++++++++++++++++++-
3 files changed, 107 insertions(+), 19 deletions(-)
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index 477b700f2429..dc70b9a99a2e 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -53,6 +53,7 @@ sub update_lease {
sub add_ip_mapping {
my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
+ $mac = lc($mac);
my $ethers_file = ethers_file($dhcpid);
my $ethers_tmp_file = "$ethers_file.tmp";
@@ -76,6 +77,8 @@ sub add_ip_mapping {
warn "failed to parse MAC from $dhcpid ethers file on line $line_no: '$line'\n";
next;
}
+ # the cache is read in lower case, a line from before keeps its writer's spelling
+ $parsed_mac = lc($parsed_mac);
my ($parsed_ip4, $parsed_ip6);
if ($parsed_ip2) {
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index ae2c4337ef8d..028c523a60c6 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 new MAC is written as the caller spells it
+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 : ($mac));
+}
+
sub add_cache_mac_ip {
my ($mac, $ip) = @_;
@@ -54,12 +94,8 @@ sub add_cache_mac_ip {
$macdb_filename,
undef,
sub {
- my $db = read_macdb();
- if (Net::IP::ip_is_ipv4($ip)) {
- $db->{macs}->{$mac}->{ip4} = $ip;
- } else {
- $db->{macs}->{$mac}->{ip6} = $ip;
- }
+ my $db = read_macdb_file();
+ set_cache_entry($db, $mac, Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6', $ip);
write_macdb($db);
},
);
@@ -73,14 +109,14 @@ sub del_cache_mac_ip {
$macdb_filename,
undef,
sub {
- my $db = read_macdb();
- if (Net::IP::ip_is_ipv4($ip)) {
- delete $db->{macs}->{$mac}->{ip4};
- } else {
- delete $db->{macs}->{$mac}->{ip6};
+ my $db = read_macdb_file();
+ my $family = Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6';
+ my $macs = $db->{macs} // {};
+ for my $key (grep { lc($_) eq lc($mac) } keys %$macs) {
+ delete $macs->{$key}->{$family};
+ delete $macs->{$key}
+ if !defined($macs->{$key}->{ip4}) && !defined($macs->{$key}->{ip6});
}
- delete $db->{macs}->{$mac}
- if !defined($db->{macs}->{$mac}->{ip4}) && !defined($db->{macs}->{$mac}->{ip6});
write_macdb($db);
},
);
@@ -137,7 +173,7 @@ sub get_ips_from_mac {
PVE::Cluster::cfs_update(); # the lock refreshes, so the snapshot must too
my $version = PVE::Cluster::cfs_file_version($macdb_filename);
- my $cached = read_macdb()->{macs}->{$mac};
+ my $cached = read_macdb()->{macs}->{ lc($mac) };
# an entry without an address was left behind by a lookup that missed, and
# says nothing about its MAC
return ($cached->{ip4}, $cached->{ip6})
@@ -159,9 +195,9 @@ sub get_ips_from_mac {
return
if (PVE::Cluster::cfs_file_version($macdb_filename) // '') ne ($version // '');
- my $db = read_macdb();
- $db->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4);
- $db->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6);
+ my $db = read_macdb_file();
+ set_cache_entry($db, $mac, 'ip4', $ip4) if defined($ip4);
+ set_cache_entry($db, $mac, 'ip6', $ip6) if defined($ip6);
write_macdb($db);
},
);
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 927ab16adba9..be4c581e414e 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -194,7 +194,7 @@ $mocked_sdn_ipams->mock(
my ($cfg) = @_;
$test_state->{ipam_config} = $cfg;
},
- read_macdb => sub {
+ read_macdb_file => sub {
return PVE::Network::SDN::Ipams::json_reader(undef, encode_json($test_state->{macdb}));
},
write_macdb => sub {
@@ -498,6 +498,55 @@ sub test_ipam_cache_lookup {
is($ips[0], "10.0.0.50", "$test_name: the lookup still answers");
ok(!exists $test_state->{macdb}->{macs}->{$mac},
"$test_name: but leaves a changed cache alone");
+
+ # an entry written under the config's spelling, as a node not yet upgraded
+ # writes it, is read and released like any other and keeps its key
+ my $other = "da:65:8f:18:9b:70";
+ $test_state->{macdb}->{macs} = {
+ uc($mac) => { ip4 => '10.0.0.150' },
+ uc($other) => { ip4 => '10.0.0.170' },
+ };
+ my ($ip4) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is($ip4, '10.0.0.150', "$test_name: an entry is found however it is spelled");
+ PVE::Network::SDN::Ipams::del_cache_mac_ip($mac, '10.0.0.150');
+ is_deeply(
+ $test_state->{macdb}->{macs},
+ { uc($other) => { ip4 => '10.0.0.170' } },
+ "$test_name: and released, the other entry left as written",
+ );
+
+ # a write in another spelling goes to the spelling the file holds
+ $test_state->{macdb}->{macs} = { uc($mac) => { ip4 => '10.0.0.150' } };
+ PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, 'fd00::150');
+ is_deeply(
+ $test_state->{macdb}->{macs},
+ { uc($mac) => { ip4 => '10.0.0.150', ip6 => 'fd00::150' } },
+ "$test_name: a write keeps the key as written",
+ );
+
+ # a lookup past an address-less entry asks the IPAM and fills the entry
+ # under its spelling
+ $test_state->{macdb}->{macs} = { uc($mac) => { ip4 => undef, ip6 => undef } };
+ @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is($ips[0], '10.0.0.50', "$test_name: a lookup past an address-less entry asks the IPAM");
+ is_deeply(
+ $test_state->{macdb}->{macs},
+ { uc($mac) => { ip4 => '10.0.0.50', ip6 => undef } },
+ "$test_name: and fills the entry under its spelling",
+ );
+
+ # two spellings that disagree are not trusted, the lookup asks the IPAM
+ $test_state->{macdb}->{macs} = {
+ uc($mac) => { ip4 => '10.0.0.150' },
+ $mac => { ip4 => '10.0.0.151' },
+ };
+ @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is($ips[0], '10.0.0.50', "$test_name: a clash of spellings counts as a miss");
+ is_deeply(
+ $test_state->{macdb}->{macs},
+ { uc($mac) => { ip4 => '10.0.0.50' }, $mac => { ip4 => '10.0.0.50' } },
+ "$test_name: and its write brings both spellings in line",
+ );
}
run_test(\&test_ipam_cache_lookup);
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH pve-network v6 04/13] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it
2026-11-05 9:35 [PATCH docs/manager/network/qemu-server v6 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (2 preceding siblings ...)
2026-11-05 9:35 ` [PATCH pve-network v6 03/13] sdn: ipam: read the MAC cache case-insensitively Hannes Laimer
@ 2026-11-05 9:35 ` Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 05/13] sdn: ipam: release an address the built-in IPAM no longer holds Hannes Laimer
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Hannes Laimer @ 2026-11-05 9:35 UTC (permalink / raw)
To: pve-devel
Adding a reservation rewrote the zone's dnsmasq reservation file and
dropped every line that no longer matched the MAC cache. Since dnsmasq
answers only MACs with a reservation, the guest behind such a line got
no DHCP answer until its next start. Such a line now gets the
addresses the cache holds for its MAC, and is dropped only when the
cache holds none.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
Reviewed-by: Stefan Hanreich <s.hanreich@proxmox.com>
Tested-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 11 ++++++++++-
src/test/run_test_vnets_blackbox.pl | 28 ++++++++++++++++++++++++++++
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index dc70b9a99a2e..c736d02d8f23 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -91,7 +91,9 @@ sub add_ip_mapping {
}
$parsed_ip6 = $1 if $parsed_ip6 && $parsed_ip6 =~ m/\[(\S+)\]/;
- #delete changed
+ # a line that no longer matches the cache is replaced by the cache's
+ # current one, a dropped one would leave that guest unknown to
+ # dnsmasq until its next start
if (
!defined($macdb->{macs}->{$parsed_mac})
|| ($parsed_ip4
@@ -102,6 +104,13 @@ sub add_ip_mapping {
&& $macdb->{macs}->{$parsed_mac}->{'ip6'} ne $parsed_ip6)
) {
$reload = 1;
+ # the MAC being added gets its line below, like a missing one
+ my $current = $macdb->{macs}->{$parsed_mac};
+ next if $parsed_mac eq $mac || !$current;
+ my $replacement = $parsed_mac;
+ $replacement .= ",$current->{ip4}" if $current->{ip4};
+ $replacement .= ",[$current->{ip6}]" if $current->{ip6};
+ print $out "$replacement\n" if $replacement ne $parsed_mac;
next;
}
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index be4c581e414e..90241268077c 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -1107,4 +1107,32 @@ sub test_update_vnet_without_zone {
}
run_test(\&test_update_vnet_without_zone);
+sub test_dnsmasq_stale_reservation_replaced {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+
+ my $stale = "da:65:8f:18:9b:01";
+ my $gone = "da:65:8f:18:9b:02";
+ my $added = "da:65:8f:18:9b:03";
+ $test_state->{macdb} = {
+ macs => {
+ $stale => { ip4 => '10.0.1.11' },
+ $added => { ip4 => '10.0.1.13' },
+ },
+ };
+ PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$stale,10.0.1.10\n$gone,10.0.1.12\n");
+
+ PVE::Network::SDN::Dhcp::Dnsmasq->add_ip_mapping(
+ 'TESTZONE', $test_state->{macdb}, $added, '10.0.1.13', undef,
+ );
+
+ my @lines = sort grep { /\S/ } split(/\n/, PVE::Tools::file_get_contents($TMP_ETHERS_FILE));
+ eq_or_diff(
+ \@lines,
+ ["$stale,10.0.1.11", "$added,10.0.1.13"],
+ "$test_name: the stale line is replaced, the unknown one dropped",
+ );
+}
+
+run_test(\&test_dnsmasq_stale_reservation_replaced);
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH pve-network v6 05/13] sdn: ipam: release an address the built-in IPAM no longer holds
2026-11-05 9:35 [PATCH docs/manager/network/qemu-server v6 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (3 preceding siblings ...)
2026-11-05 9:35 ` [PATCH pve-network v6 04/13] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it Hannes Laimer
@ 2026-11-05 9:35 ` Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 06/13] sdn: dhcp: update reservations when IP mappings change Hannes Laimer
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Hannes Laimer @ 2026-11-05 9:35 UTC (permalink / raw)
To: pve-devel
The built-in IPAM died on releasing an address it had no record of,
before the cache entry of the address was cleared. So the API could
not delete the mapping, and lookups of its MAC kept returning the
address. NetBox and phpIPAM skip the deletion when they find no record
of the address. The built-in IPAM now warns and does the same, so the
mapping is deleted along with its cache entry.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 7 ++++++-
src/test/run_test_vnets_blackbox.pl | 17 +++++++++++++++++
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index a26da57a877a..77a7b2a7c144 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -265,7 +265,12 @@ sub del_ip {
die "subnet $cidr don't exist in ipam db" if !$dbzone->{subnets}->{$cidr};
my $dbsubnet = $dbzone->{subnets}->{$cidr};
- die "IP '$ip' does not exist in IPAM DB\n" if !defined($dbsubnet->{ips}->{$ip});
+ # a record that is gone was released already, and the address still
+ # has to leave the cache, so this is no error
+ if (!defined($dbsubnet->{ips}->{$ip})) {
+ warn "IP '$ip' does not exist in IPAM DB\n";
+ return;
+ }
delete $dbsubnet->{ips}->{$ip};
write_db($db);
},
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 90241268077c..164ef4b4592e 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -547,6 +547,23 @@ sub test_ipam_cache_lookup {
{ uc($mac) => { ip4 => '10.0.0.50' }, $mac => { ip4 => '10.0.0.50' } },
"$test_name: and its write brings both spellings in line",
);
+
+ # a mapping whose record is gone is still deleted, and its cache entry
+ # with it
+ $test_state->{macdb} = {};
+ create_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.70" });
+ delete $test_state->{ipamdb}->{zones}->{$zoneid}->{subnets}->{"10.0.0.0/24"}->{ips}
+ ->{"10.0.0.70"};
+ eval {
+ PVE::API2::Network::SDN::Ips->ipdelete(
+ { zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.70" });
+ };
+ is($@, '', "$test_name: a mapping whose record is gone is deleted without an error");
+ is(
+ (PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac))[0],
+ '10.0.0.50',
+ "$test_name: and the cache no longer answers with its address",
+ );
}
run_test(\&test_ipam_cache_lookup);
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH pve-network v6 06/13] sdn: dhcp: update reservations when IP mappings change
2026-11-05 9:35 [PATCH docs/manager/network/qemu-server v6 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (4 preceding siblings ...)
2026-11-05 9:35 ` [PATCH pve-network v6 05/13] sdn: ipam: release an address the built-in IPAM no longer holds Hannes Laimer
@ 2026-11-05 9:35 ` Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 07/13] sdn: ipam: look a MAC up within the vnet's subnets Hannes Laimer
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Hannes Laimer @ 2026-11-05 9:35 UTC (permalink / raw)
To: pve-devel
Creating, updating or deleting an IP mapping through the API changed
the IPAM and the MAC cache, but never the zone's dnsmasq reservation
file. dnsmasq could keep serving the old reservation, or none for a
new mapping, until the guest's next start.
A guest start, an API edit and the deletion of a NIC's addresses now
all rewrite the MAC's reservation from the cache. The reservations are
a file on each node, and only the guest's node answers its DHCP
requests. So an API edit is proxied to the node of the guest with that
MAC on the vnet, unless that node is offline. The other nodes catch up
when the guest starts on one of them.
The edit holds the guest's migration lock, so a migration cannot move
the guest during the edit. An edit that meets a migration, or finds
the guest moved to another node, fails before writing anything. If the
reservation update fails, the API edit fails with an error saying the
IPAM is already updated. A guest start and the deletion of a NIC's
addresses only warn.
Each rewrite reads the current cache only once it holds the
reservation file's lock. A rewrite working from an older read would
undo the lines another one had just written. The MAC's line is written
in full from the cache, so a guest gaining an IPv6 address next to its
IPv4 one gets it reserved too.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
debian/control | 2 +
src/PVE/API2/Network/SDN/Ips.pm | 116 +++++++++++--
src/PVE/Network/SDN/Dhcp.pm | 51 +++---
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 107 +++++++-----
src/PVE/Network/SDN/Dhcp/Plugin.pm | 6 +-
src/PVE/Network/SDN/Vnets.pm | 12 +-
src/test/run_test_vnets_blackbox.pl | 248 +++++++++++++++++++++++++++-
7 files changed, 455 insertions(+), 87 deletions(-)
diff --git a/debian/control b/debian/control
index 268f40186b6b..bcdc8e50fe76 100644
--- a/debian/control
+++ b/debian/control
@@ -6,6 +6,7 @@ Build-Depends: debhelper-compat (= 13),
lintian,
libfile-slurp-perl,
libpve-common-perl (>= 9.2.4),
+ libpve-guest-common-perl,
libnet-subnet-perl,
libpve-rs-perl (>= 0.15.0),
libtest-mockmodule-perl,
@@ -42,6 +43,7 @@ Description: Proxmox VE's SDN (Software Defined Network) stack
Package: libpve-network-api-perl
Architecture: all
Depends: libpve-common-perl (>= 9.0.2),
+ libpve-guest-common-perl,
pve-firewall (>= 5.1.0~),
pve-cluster (>= 9.0.1),
libpve-network-perl (= ${binary:Version}),
diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
index 04741d8750a4..8a6106ba29a7 100644
--- a/src/PVE/API2/Network/SDN/Ips.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -3,9 +3,13 @@ package PVE::API2::Network::SDN::Ips;
use strict;
use warnings;
+use PVE::Cluster;
+use PVE::GuestHelpers;
+use PVE::INotify;
use PVE::Tools qw(extract_param);
use PVE::Network::SDN::Vnets;
+use PVE::Network::SDN::Zones;
use PVE::Network::SDN::Dhcp;
use PVE::JSONSchema qw(get_standard_option);
@@ -13,12 +17,73 @@ use PVE::RESTHandler;
use base qw(PVE::RESTHandler);
+# only the guest's node answers its DHCP requests, so the edit runs there.
+# With that node offline it runs here, and the node catches up at the
+# guest's next start
+my $proxy_to_guest_node = sub {
+ my ($rpcenv, $proxyto, $param) = @_;
+
+ my $vnet = PVE::Network::SDN::Vnets::get_vnet($param->{vnet}, 1);
+ my $zone = $vnet ? PVE::Network::SDN::Zones::get_zone($vnet->{zone}, 1) : undef;
+ return 'localhost' if !$zone || !$zone->{dhcp};
+
+ my ($node) = PVE::Network::SDN::Dhcp::guest_node_by_mac($param->{vnet}, $param->{mac});
+ my $members = PVE::Cluster::get_members();
+ return 'localhost' if !$node || ($members && !($members->{$node} // {})->{online});
+ return $node;
+};
+
+# the edit runs under the guest's migration lock on its node, so a migration
+# cannot move the guest between the IPAM write and the reservation update,
+# and an edit that finds the guest moved away fails before it writes anything
+my $with_guest_lock = sub {
+ my ($vnet, $mac, $code) = @_;
+
+ my (undef, $vmid) = PVE::Network::SDN::Dhcp::guest_node_by_mac($vnet, $mac);
+ return $code->() if !defined($vmid);
+
+ # an error of the edit itself is kept apart from the lock's own failure
+ my ($res, $edit_err);
+ eval {
+ PVE::GuestHelpers::guest_migration_lock(
+ $vmid,
+ 1,
+ sub {
+ $res = eval {
+ # a migration that ended while the lock was waited for moved
+ # the config, and the vmlist of the request's start has not
+ # seen it
+ PVE::Cluster::cfs_update();
+ my $target =
+ $proxy_to_guest_node->(undef, undef, { vnet => $vnet, mac => $mac });
+ die "guest $vmid moved to $target, try again\n"
+ if $target ne 'localhost' && $target ne PVE::INotify::nodename();
+ $code->();
+ };
+ $edit_err = $@;
+ },
+ );
+ };
+ die "guest $vmid is locked, maybe by a migration\n" if $@;
+ die $edit_err if $edit_err;
+ return $res;
+};
+
+my $push_mapping = sub {
+ my ($vnet, $mac) = @_;
+
+ eval { PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac) };
+ # the records are written by then and stay
+ die "IPAM is updated for $mac, but DHCP is not: $@" if $@;
+};
+
__PACKAGE__->register_method({
name => 'ipdelete',
path => '',
method => 'DELETE',
description => 'Delete IP Mappings in a VNet',
protected => 1,
+ proxyto_callback => $proxy_to_guest_node,
permissions => {
description => "Requires 'SDN.Allocate' on '/sdn/zones/<zone>/<vnet>'",
user => 'all',
@@ -55,8 +120,16 @@ __PACKAGE__->register_method({
my $mac = extract_param($param, 'mac');
my $ip = extract_param($param, 'ip');
- eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
- die "$@\n" if $@;
+ $with_guest_lock->(
+ $vnet,
+ $mac,
+ sub {
+ eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
+ die "$@\n" if $@;
+
+ $push_mapping->($vnet, $mac);
+ },
+ );
return undef;
},
@@ -68,6 +141,7 @@ __PACKAGE__->register_method({
method => 'POST',
description => 'Create IP Mapping in a VNet',
protected => 1,
+ proxyto_callback => $proxy_to_guest_node,
permissions => {
description => "Requires 'SDN.Allocate' on '/sdn/zones/<zone>/<vnet>'",
user => 'all',
@@ -104,7 +178,14 @@ __PACKAGE__->register_method({
my $mac = extract_param($param, 'mac');
my $ip = extract_param($param, 'ip');
- PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, undef);
+ $with_guest_lock->(
+ $vnet,
+ $mac,
+ sub {
+ PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, undef);
+ $push_mapping->($vnet, $mac);
+ },
+ );
return undef;
},
@@ -115,6 +196,7 @@ __PACKAGE__->register_method({
method => 'PUT',
description => 'Update IP Mapping in a VNet',
protected => 1,
+ proxyto_callback => $proxy_to_guest_node,
permissions => {
description => "Requires 'SDN.Allocate' on '/sdn/zones/<zone>/<vnet>'",
user => 'all',
@@ -155,19 +237,29 @@ __PACKAGE__->register_method({
my $vmid = extract_param($param, 'vmid');
my $ip = extract_param($param, 'ip');
- my ($old_ip4, $old_ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnet, $mac);
- my $old_ip = (Net::IP::ip_get_version($ip) == 4) ? $old_ip4 : $old_ip6;
+ $with_guest_lock->(
+ $vnet,
+ $mac,
+ sub {
+ my ($old_ip4, $old_ip6) =
+ PVE::Network::SDN::Vnets::get_ips_from_mac($vnet, $mac);
+ my $old_ip = (Net::IP::ip_get_version($ip) == 4) ? $old_ip4 : $old_ip6;
- PVE::Network::SDN::Vnets::del_ip($vnet, $old_ip, '', $mac);
+ PVE::Network::SDN::Vnets::del_ip($vnet, $old_ip, '', $mac);
- eval { PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, $vmid); };
- my $error = $@;
+ eval { PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, $vmid); };
+ my $error = $@;
- if ($error) {
- PVE::Network::SDN::Vnets::add_ip($vnet, $old_ip, '', $mac, $vmid);
- }
+ if ($error) {
+ PVE::Network::SDN::Vnets::add_ip($vnet, $old_ip, '', $mac, $vmid);
+ }
+
+ die "$error\n" if $error;
+
+ $push_mapping->($vnet, $mac);
+ },
+ );
- die "$error\n" if $error;
return undef;
},
});
diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
index f6ce842c8cc2..11bb6df8c23a 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,47 @@ 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) = @_;
+
+ # a record without a MAC has no guest NIC behind it, so no backend serves it
+ return if !$mac;
- my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid);
+ # 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;
my $zoneid = $vnet->{zone};
- my $zone = PVE::Network::SDN::Zones::get_zone($zoneid);
-
- return if !$zone->{ipam} || !$zone->{dhcp};
+ 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);
}
-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};
+ return if !$mac;
+
+ my $vmlist = PVE::Cluster::get_vmlist();
+ my $nets = PVE::Cluster::get_guest_config_properties([map { "net$_" } 0 .. 31]);
+ for my $vmid (sort keys %$nets) {
+ my $guest = $vmlist->{ids}->{$vmid} // next;
+ for my $net (values %{ $nets->{$vmid} }) {
+ my ($bridge) = $net =~ m/(?:^|,)bridge=([^,]+)/ or next;
+ next if $bridge ne $vnetid;
+ # a VM names the MAC as the value of its model, a container as hwaddr
+ my ($nic_mac) = $net =~ m/(?:^|,)[a-z0-9_-]+=([0-9a-f]{2}(?::[0-9a-f]{2}){5})(?:,|$)/i
+ or next;
+ return ($guest->{node}, $vmid) if lc($nic_mac) eq lc($mac);
+ }
+ }
- my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp});
- $dhcp_plugin->del_ip_mapping($zoneid, $mac);
+ return;
}
sub regenerate_config {
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index c736d02d8f23..5864ead87d29 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,101 @@ 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 @resume_answer;
- my $appendFn = sub {
+ my $rewriteFn = sub {
+ # the lines are decided from the cache, which this process may hold an
+ # old read of, and the local lock does not refresh it
+ PVE::Cluster::cfs_update();
+
+ # read under the ethers lock. A snapshot from before it would sweep a
+ # line another push wrote meanwhile, and an older push would put its
+ # addresses back
+
+ my $macs = PVE::Network::SDN::Ipams::read_macdb()->{macs};
+
+ my $entry = $macs->{$mac};
+ ($ip4, $ip6) = ($entry->{ip4}, $entry->{ip6}) if $entry;
+ if ($ip4 || $ip6) {
+ $reservation = $mac;
+ $reservation .= ",$ip4" if $ip4;
+ $reservation .= ",[$ip6]" if $ip6;
+ # an external IPAM spells addresses its own way, while the file holds one spelling
+ $reservation = lc($reservation);
+ }
+
+ file_set_contents($ethers_file, '') if !-e $ethers_file;
open(my $in, '<', $ethers_file) or die "Could not open file '$ethers_file' $!\n";
open(my $out, '>', $ethers_tmp_file)
or die "Could not open file '$ethers_tmp_file' $!\n";
- my $match = undef;
+ my $kept = undef;
- my $line_no = 0;
while (my $line = <$in>) {
- $line_no++;
chomp($line);
+ next if $line !~ m/\S/;
my ($parsed_mac, $parsed_ip1, $parsed_ip2) = split(/,/, $line);
+ # a line from before the file was written in lower case keeps its writer's spelling
+ $parsed_mac = lc($parsed_mac);
- if (!defined($parsed_mac)) {
- warn "failed to parse MAC from $dhcpid ethers file on line $line_no: '$line'\n";
+ # the MAC's own reservation is rewritten. An unchanged one stays in place
+ if ($parsed_mac eq $mac) {
+ if (defined($reservation) && !$kept && lc($line) eq $reservation) {
+ $kept = 1;
+ print $out "$line\n";
+ } else {
+ $changed = 1;
+ }
next;
}
- # the cache is read in lower case, a line from before keeps its writer's spelling
- $parsed_mac = lc($parsed_mac);
my ($parsed_ip4, $parsed_ip6);
if ($parsed_ip2) {
$parsed_ip4 = $parsed_ip1;
$parsed_ip6 = $parsed_ip2;
- } elsif (Net::IP::ip_is_ipv4($parsed_ip1)) {
+ } elsif (defined($parsed_ip1) && Net::IP::ip_is_ipv4($parsed_ip1)) {
$parsed_ip4 = $parsed_ip1;
- } else {
+ } elsif (defined($parsed_ip1)) {
$parsed_ip6 = $parsed_ip1;
}
$parsed_ip6 = $1 if $parsed_ip6 && $parsed_ip6 =~ m/\[(\S+)\]/;
+ # a line from before addresses were lower-cased may spell one in upper case
+ $parsed_ip6 = lc($parsed_ip6) if $parsed_ip6;
# a line that no longer matches the cache is replaced by the cache's
# current one, a dropped one would leave that guest unknown to
# dnsmasq until its next start
+ my $current = $macs->{$parsed_mac};
if (
- !defined($macdb->{macs}->{$parsed_mac})
- || ($parsed_ip4
- && $macdb->{macs}->{$parsed_mac}->{'ip4'}
- && $macdb->{macs}->{$parsed_mac}->{'ip4'} ne $parsed_ip4)
- || ($parsed_ip6
- && $macdb->{macs}->{$parsed_mac}->{'ip6'}
- && $macdb->{macs}->{$parsed_mac}->{'ip6'} ne $parsed_ip6)
+ !$current
+ || ($parsed_ip4 && $current->{ip4} && $current->{ip4} ne $parsed_ip4)
+ || ($parsed_ip6 && $current->{ip6} && lc($current->{ip6}) ne $parsed_ip6)
) {
- $reload = 1;
- # the MAC being added gets its line below, like a missing one
- my $current = $macdb->{macs}->{$parsed_mac};
- next if $parsed_mac eq $mac || !$current;
+ $changed = 1;
+ next if !$current;
my $replacement = $parsed_mac;
$replacement .= ",$current->{ip4}" if $current->{ip4};
$replacement .= ",[$current->{ip6}]" if $current->{ip6};
- print $out "$replacement\n" if $replacement ne $parsed_mac;
+ print $out lc($replacement) . "\n";
next;
}
- if ($parsed_mac eq $mac) {
- $match = 1 if $ip4 && $parsed_ip4 && $ip4;
- $match = 1 if $ip6 && $parsed_ip6 && $ip6;
- }
-
print $out "$line\n";
}
- if (!$match) {
- my $reservation = $mac;
- $reservation .= ",$ip4" if $ip4;
- $reservation .= ",[$ip6]" if $ip6;
+ if (defined($reservation) && !$kept) {
print $out "$reservation\n";
- $reload = 1;
+ $changed = 1;
}
close $in;
@@ -136,16 +156,17 @@ sub add_ip_mapping {
chmod 0644, $ethers_file;
};
- PVE::Tools::lock_file($ethers_file, 10, $appendFn);
+ # the rewrite replaces the reservation file, so a lock on the file itself
+ # would not outlive the rename
+ PVE::Tools::lock_file("$ethers_file.lock", 10, $rewriteFn);
+ die "Unable to update $mac in the dnsmasq configuration: $@" if $@;
- if ($@) {
- warn "Unable to add $mac to the dnsmasq configuration: $@\n";
- return;
- }
+ systemctl_service('reload', "dnsmasq\@$dhcpid") if $changed;
- my $service_name = "dnsmasq\@$dhcpid";
- systemctl_service('reload', $service_name) if $reload;
- update_lease($dhcpid, $ip4, $mac);
+ if ($ip4) {
+ eval { update_lease($dhcpid, $ip4, $mac) };
+ log_warn("could not update the dnsmasq lease of $mac: $@") if $@;
+ }
}
sub configure_subnet {
diff --git a/src/PVE/Network/SDN/Dhcp/Plugin.pm b/src/PVE/Network/SDN/Dhcp/Plugin.pm
index b5d32fa13c70..472ff1534743 100644
--- a/src/PVE/Network/SDN/Dhcp/Plugin.pm
+++ b/src/PVE/Network/SDN/Dhcp/Plugin.pm
@@ -22,8 +22,10 @@ sub private {
return $defaultData;
}
-sub add_ip_mapping {
- my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
+# the MAC's records changed, so make the backend serve the current ones. With no
+# record left its reservation goes. Dies if the backend could not be updated
+sub update_ip_mapping {
+ my ($class, $dhcpid, $mac) = @_;
die 'implement in sub class';
}
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index ea66aa2177e1..56516ed9de83 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -214,6 +214,12 @@ sub del_ips_from_mac {
PVE::Network::SDN::Vnets::del_ip($vnetid, $ip4, $hostname, $mac) if $ip4;
PVE::Network::SDN::Vnets::del_ip($vnetid, $ip6, $hostname, $mac) if $ip6;
+ if ($ip4 || $ip6) {
+ # a NIC going away does not fail on its DHCP backend
+ eval { PVE::Network::SDN::Dhcp::update_mapping($vnetid, $mac) };
+ warn $@ if $@;
+ }
+
return ($ip4, $ip6);
}
@@ -228,11 +234,15 @@ sub add_dhcp_mapping {
return if !$zone->{ipam} || !$zone->{dhcp};
my ($ip4, $ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ # one push at the end covers the allocated and the existing records alike
add_next_free_cidr($vnetid, $name, $mac, "$vmid", undef, 1, 4) if !$ip4;
add_next_free_cidr($vnetid, $name, $mac, "$vmid", undef, 1, 6) if !$ip6;
($ip4, $ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
- PVE::Network::SDN::Dhcp::add_mapping($vnetid, $mac, $ip4, $ip6) if $ip4 || $ip6;
+ return if !$ip4 && !$ip6;
+ # a guest start does not fail on its DHCP backend
+ eval { PVE::Network::SDN::Dhcp::update_mapping($vnetid, $mac) };
+ warn $@ if $@;
}
=head3 api_get_vnet($cfg, $id)
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 164ef4b4592e..f641b8087e9b 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -47,6 +47,7 @@ sub clear_test_state {
vnets_config => {},
macdb => {},
ipamdb => {},
+ dnsmasq_calls => [],
ipam_config => {
'ids' => {
'pve' => {
@@ -222,7 +223,9 @@ $mocked_sdn_dhcp_dnsmasq->mock(
assert_dnsmasq_installed => sub { return 1; },
before_configure => sub { },
ethers_file => sub { return "/tmp/ethers"; },
- systemctl_service => sub { },
+ systemctl_service => sub {
+ push $test_state->{dnsmasq_calls}->@*, $_[0];
+ },
update_lease => sub { },
);
@@ -254,10 +257,31 @@ $mocked_rpc_env_obj->mock(
check_any => sub { return 1; },
);
+my $mocked_pve_inotify = Test::MockModule->new('PVE::INotify');
+$mocked_pve_inotify->mock(
+ nodename => sub { return 'localnode'; },
+);
+
+my $mocked_guest_helpers = Test::MockModule->new('PVE::GuestHelpers');
+$mocked_guest_helpers->mock(
+ guest_migration_lock => sub {
+ my ($vmid, $timeout, $code) = @_;
+ return $code->();
+ },
+);
+
my $mocked_pve_cluster_obj = Test::MockModule->new('PVE::Cluster');
$mocked_pve_cluster_obj->mock(
check_cfs_quorum => sub { return 1; },
cfs_lock_domain => $mocked_cfs_lock_domain,
+ # a vmlist change a test parks here lands with the next refresh, as a
+ # change made meanwhile on the cluster does
+ cfs_update => sub {
+ $test_state->{vmlist} = delete $test_state->{vmlist_pending}
+ if $test_state->{vmlist_pending};
+ },
+ get_vmlist => sub { return $test_state->{vmlist} // { ids => {} }; },
+ get_guest_config_properties => sub { return $test_state->{guest_nets} // {}; },
);
# ------- TEST FUNCTIONS --------------
@@ -303,6 +327,11 @@ sub create_zone {
return $zone;
}
+sub update_zone {
+ my ($zoneid, $params) = @_;
+ PVE::API2::Network::SDN::Zones->update({ zone => $zoneid, %$params });
+}
+
sub get_vnet {
my ($id) = @_;
return eval { PVE::API2::Network::SDN::Vnets->read({ vnet => $id }); };
@@ -373,6 +402,16 @@ sub create_ip {
return PVE::API2::Network::SDN::Ips->ipcreate($param);
}
+sub update_ip {
+ my ($param) = @_;
+ return PVE::API2::Network::SDN::Ips->ipupdate($param);
+}
+
+sub delete_ip {
+ my ($param) = @_;
+ return PVE::API2::Network::SDN::Ips->ipdelete($param);
+}
+
sub run_test {
my $test = shift;
clear_test_state();
@@ -1138,18 +1177,211 @@ 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
+ # and no guest is looked for, with guests on the vnet or not
+ my @warnings;
+ {
+ local $SIG{__WARN__} = sub { push @warnings, @_; };
+ local $test_state->{guest_nets} =
+ { 100 => { net0 => "virtio=da:65:8f:18:9b:71,bridge=$vnetid" } };
+ local $test_state->{vmlist} = { ids => { 100 => { node => 'localnode' } } };
+ create_ip({ zone => $zoneid, vnet => $vnetid, ip => "10.0.0.160" });
+ delete_ip({ zone => $zoneid, vnet => $vnetid, ip => "10.0.0.160" });
+ }
+ eq_or_diff(\@warnings, [], "$test_name: a record without a MAC warns nothing");
+
+ # a reservation of a MAC the cache does not know is swept by the next
+ # rewrite, and both addresses of the guest go on one line
+ create_dhcp_subnet($vnetid, "8888::/64");
+ PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "aa:bb:cc:dd:ee:ff,10.0.0.77\n");
+ nic_start($vnetid, $mac, "999", "testhostname");
+ is(
+ $ethers->(),
+ "$mac,10.0.0.100,[8888::64]\n",
+ "$test_name: dual-stack reservation on one line, the unknown MAC swept",
+ );
+
+ # a zone confined to other nodes runs no dnsmasq here, so an edit is not
+ # pushed on this node
+ update_zone($zoneid, { nodes => 'other' });
+ create_ip({
+ zone => $zoneid,
+ vnet => $vnetid,
+ mac => $mac,
+ ip => "10.0.0.150",
+ });
+ is(
+ $ethers->(),
+ "$mac,10.0.0.100,[8888::64]\n",
+ "$test_name: a zone confined to other nodes gets no push here",
+ );
+}
+
+run_test(\&test_dnsmasq_mapping_push);
+
+sub test_failed_mapping_push {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+ $mocked_sdn_dhcp_dnsmasq->mock(update_ip_mapping => sub { die "rewrite failed\n"; });
+
+ my @warnings;
+ {
+ local $SIG{__WARN__} = sub { push @warnings, @_; };
+ eval { nic_start($vnetid, $mac, "999", "testhostname"); };
+ is($@, '', "$test_name: a guest start does not fail on its backend");
+ }
+ like(join('', @warnings), qr/rewrite failed/, "$test_name: the guest start warns");
+
+ eval { update_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.150" }); };
+ like(
+ $@,
+ qr/IPAM is updated for \Q$mac\E, but DHCP is not: rewrite failed/,
+ "$test_name: an API edit fails",
+ );
+ is(get_ip4(get_ips_from_mac($mac)), "10.0.0.150", "$test_name: the edit is saved");
+
+ $mocked_sdn_dhcp_dnsmasq->unmock('update_ip_mapping');
+}
+
+run_test(\&test_failed_mapping_push);
+
+sub test_mapping_edit_target {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+ # the guest behind a MAC is found by its NIC on the vnet, and the config
+ # writes the MAC in upper case
+ $test_state->{vmlist} = {
+ ids => {
+ 100 => { type => 'qemu', node => 'other' },
+ },
+ };
+ $test_state->{guest_nets} = {
+ 1 => { net0 => "virtio=$mac,bridge=$vnetid" },
+ 100 => { net0 => "virtio=" . uc($mac) . ",bridge=$vnetid" },
+ };
+ is_deeply(
+ [PVE::Network::SDN::Dhcp::guest_node_by_mac($vnetid, $mac)],
+ ['other', 100],
+ "$test_name: the MAC resolves to the node of its guest, a guest the list lacks is skipped",
+ );
+
+ # a mapping edit through the API runs on the guest's node
+ my $proxyto = PVE::API2::Network::SDN::Ips->map_method_by_name('ipupdate')->{proxyto_callback};
+ is(
+ $proxyto->(undef, undef, { zone => $zoneid, vnet => $vnetid, mac => $mac }),
+ 'other',
+ "$test_name: a mapping edit runs on the guest's node",
+ );
+ is(
+ $proxyto->(undef, undef, { zone => $zoneid, vnet => $vnetid, mac => '00:11:22:33:44:66' }),
+ 'localhost',
+ "$test_name: one for a MAC without a guest runs here",
+ );
+
+ # the edit holds the guest's migration lock, a migration holding it fails
+ # the edit, and a guest found on another node under the lock fails it too
+ $test_state->{vmlist}->{ids}->{100}->{node} = 'localnode';
+ create_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.50" });
+ $mocked_guest_helpers->mock(
+ guest_migration_lock => sub { die "can't lock file '/var/lock/x' - got timeout\n" });
+ eval { update_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.60" }) };
+ like(
+ $@,
+ qr/^guest 100 is locked, maybe by a migration$/,
+ "$test_name: an edit during a migration fails",
+ );
+ $mocked_guest_helpers->mock(
+ guest_migration_lock => sub { my ($vmid, $timeout, $code) = @_; return $code->(); });
+ $test_state->{vmlist}->{ids}->{100}->{node} = 'other';
+ eval { update_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.60" }) };
+ like($@, qr/guest 100 moved to other/, "$test_name: an edit for a guest found elsewhere fails");
+ is(get_ip4(get_ips_from_mac($mac)), "10.0.0.50", "$test_name: and wrote nothing");
+ # a migration that ends while the lock is waited for is seen under the lock
+ $test_state->{vmlist}->{ids}->{100}->{node} = 'localnode';
+ $test_state->{vmlist_pending} = { ids => { 100 => { node => 'other' } } };
+ eval { update_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.60" }) };
+ like($@, qr/guest 100 moved to other/,
+ "$test_name: a move not yet seen is seen under the lock");
+ is(get_ip4(get_ips_from_mac($mac)), "10.0.0.50", "$test_name: and wrote nothing either");
+ $test_state->{vmlist}->{ids}->{100}->{node} = 'localnode';
+ update_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.60" });
+ is(
+ get_ip4(get_ips_from_mac($mac)),
+ "10.0.0.60",
+ "$test_name: one for a guest here goes through",
+ );
+ delete $test_state->{vmlist};
+ delete $test_state->{guest_nets};
+}
+
+run_test(\&test_mapping_edit_target);
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH pve-network v6 07/13] sdn: ipam: look a MAC up within the vnet's subnets
2026-11-05 9:35 [PATCH docs/manager/network/qemu-server v6 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (5 preceding siblings ...)
2026-11-05 9:35 ` [PATCH pve-network v6 06/13] sdn: dhcp: update reservations when IP mappings change Hannes Laimer
@ 2026-11-05 9:35 ` Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 08/13] sdn: ipam: keep the MAC cache per vnet Hannes Laimer
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Hannes Laimer @ 2026-11-05 9:35 UTC (permalink / raw)
To: pve-devel
A MAC only has to be unique within its vnet, so two vnets can carry
the same one. The IPAM lookup by MAC searched every subnet of the
zone, and NetBox searched its whole inventory. So a lookup for one
vnet could return the MAC's address on another, and the guest got no
address on its own vnet.
The built-in IPAM and NetBox are now searched only within the subnets
of the vnet the lookup is for. The phpIPAM plugin does not implement
the lookup. The MAC cache is still read first and keeps one address
per MAC across all vnets.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams.pm | 4 +--
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 34 +++++++++++++---------
src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 7 ++---
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 2 +-
src/PVE/Network/SDN/Ipams/Plugin.pm | 3 +-
src/PVE/Network/SDN/Vnets.pm | 3 +-
src/test/run_test_vnets_blackbox.pl | 25 ++++++++++++++++
7 files changed, 55 insertions(+), 23 deletions(-)
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 028c523a60c6..596f43951b23 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -169,7 +169,7 @@ sub complete_sdn_vnet {
}
sub get_ips_from_mac {
- my ($mac, $zoneid, $zone) = @_;
+ my ($mac, $zoneid, $zone, $subnets) = @_;
PVE::Cluster::cfs_update(); # the lock refreshes, so the snapshot must too
my $version = PVE::Cluster::cfs_file_version($macdb_filename);
@@ -181,7 +181,7 @@ sub get_ips_from_mac {
my $plugin_config = get_plugin_config($zone);
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
- my ($ip4, $ip6) = $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid);
+ my ($ip4, $ip6) = $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid, $subnets);
# an empty answer is not cached, since the record may simply not exist yet
return if !defined($ip4) && !defined($ip6);
diff --git a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
index e118d03e1eb0..9e1c9537e6ec 100644
--- a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
@@ -379,26 +379,32 @@ sub del_ip {
}
sub get_ips_from_mac {
- my ($class, $plugin_config, $mac, $zoneid, $noerr) = @_;
+ my ($class, $plugin_config, $mac, $zoneid, $subnets, $noerr) = @_;
my $ip4 = undef;
my $ip6 = undef;
- my $data = eval {
- netbox_api_request($plugin_config, "GET", "/ipam/ip-addresses/?description__ic=$mac");
- };
- if ($@) {
- return if $noerr;
- die "could not query ip address entry for mac $mac: $@";
- }
-
- for my $ip (@{ $data->{results} }) {
- if ($ip->{family}->{value} == 4 && !$ip4) {
- ($ip4, undef) = split(/\//, $ip->{address});
+ for my $cidr (sort map { $_->{cidr} } values %{ $subnets // {} }) {
+ my $data = eval {
+ netbox_api_request(
+ $plugin_config,
+ "GET",
+ "/ipam/ip-addresses/?parent=$cidr&description__ic=$mac",
+ );
+ };
+ if ($@) {
+ return if $noerr;
+ die "could not query IP address entry for MAC $mac in $cidr: $@";
}
- if ($ip->{family}->{value} == 6 && !$ip6) {
- ($ip6, undef) = split(/\//, $ip->{address});
+ for my $ip (@{ $data->{results} }) {
+ if ($ip->{family}->{value} == 4 && !$ip4) {
+ ($ip4, undef) = split(/\//, $ip->{address});
+ }
+
+ if ($ip->{family}->{value} == 6 && !$ip6) {
+ ($ip6, undef) = split(/\//, $ip->{address});
+ }
}
}
diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index 77a7b2a7c144..43d0a3daaf9a 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -279,7 +279,7 @@ sub del_ip {
}
sub get_ips_from_mac {
- my ($class, $plugin_config, $mac, $zoneid) = @_;
+ my ($class, $plugin_config, $mac, $zoneid, $subnets) = @_;
#just in case, as this should already be cached in local macs.db
@@ -289,13 +289,12 @@ sub get_ips_from_mac {
my $db = read_db();
die "zone $zoneid don't exist in ipam db" if !$db->{zones}->{$zoneid};
my $dbzone = $db->{zones}->{$zoneid};
- my $subnets = $dbzone->{subnets};
- for my $subnet (keys %$subnets) {
+ for my $subnet (sort map { $_->{cidr} } values %{ $subnets // {} }) {
# subnet keys are CIDRs, Net::IP only recognizes bare IPs
my ($network) = split(m|/|, $subnet);
next if Net::IP::ip_is_ipv4($network) ? $ip4 : $ip6;
- my $ips = $subnets->{$subnet}->{ips};
+ my $ips = $dbzone->{subnets}->{$subnet}->{ips} // next;
for my $ip (keys %$ips) {
my $ipobject = $ips->{$ip};
if ($ipobject->{mac} && $ipobject->{mac} eq $mac) {
diff --git a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
index 63256119e0cd..12d470e802c4 100644
--- a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
@@ -262,7 +262,7 @@ sub del_ip {
}
sub get_ips_from_mac {
- my ($class, $plugin_config, $mac, $zoneid) = @_;
+ my ($class, $plugin_config, $mac, $zoneid, $subnets) = @_;
my $url = $plugin_config->{url};
my $token = $plugin_config->{token};
diff --git a/src/PVE/Network/SDN/Ipams/Plugin.pm b/src/PVE/Network/SDN/Ipams/Plugin.pm
index 26d093139703..14f8fe9e67fe 100644
--- a/src/PVE/Network/SDN/Ipams/Plugin.pm
+++ b/src/PVE/Network/SDN/Ipams/Plugin.pm
@@ -138,8 +138,9 @@ sub del_ip {
die "please implement inside plugin";
}
+# the addresses of a MAC within the given subnets of the zone, one per family
sub get_ips_from_mac {
- my ($class, $plugin_config, $mac, $zone) = @_;
+ my ($class, $plugin_config, $mac, $zoneid, $subnets) = @_;
die "please implement inside plugin";
}
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index 56516ed9de83..395aa32b6a84 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -204,7 +204,8 @@ sub get_ips_from_mac {
return if !$zone->{ipam} || !$zone->{dhcp};
- return PVE::Network::SDN::Ipams::get_ips_from_mac($mac, $zoneid, $zone);
+ my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid);
+ return PVE::Network::SDN::Ipams::get_ips_from_mac($mac, $zoneid, $zone, $subnets);
}
sub del_ips_from_mac {
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index f641b8087e9b..752e5b0d3c70 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -1384,4 +1384,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] 14+ messages in thread
* [PATCH pve-network v6 08/13] sdn: ipam: keep the MAC cache per vnet
2026-11-05 9:35 [PATCH docs/manager/network/qemu-server v6 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (6 preceding siblings ...)
2026-11-05 9:35 ` [PATCH pve-network v6 07/13] sdn: ipam: look a MAC up within the vnet's subnets Hannes Laimer
@ 2026-11-05 9:35 ` Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 09/13] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM Hannes Laimer
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Hannes Laimer @ 2026-11-05 9:35 UTC (permalink / raw)
To: pve-devel
The same MAC can sit on several vnets, but the cache of its addresses
was keyed by the MAC alone. A mapping created or edited on one vnet
changed the cached answer on every other vnet as well. A guest with
that MAC on another vnet then got a reservation for an address outside
its own vnet.
The cache now holds the addresses per vnet. A zone's dnsmasq
reservations hold a line for each vnet a MAC sits on. A push of a
MAC's reservation looks the MAC up and checks the other lines against
the cache alone. A push at a resume asks the IPAM before it takes
the reservation file's lock. Then it does not wait long for the lock
and writes its line from the cache, or from the answer when the cache
has nothing. The built-in IPAM matches a MAC in any spelling, since a
push looks the MAC up in lower case.
The map under the MAC alone stays, since nodes not yet upgraded read
and write only that one. Every address written to the cache goes there
as well, one address per family and MAC as before. An entry there
answers for a vnet without an entry of its own, but only with an
address in that vnet's subnets. So a guest started before the upgrade
keeps its reservation.
A release or an edit on a node not yet upgraded shows only in that
map. A vnet's cached address is therefore used only while that map
still names it. Otherwise the lookup asks the IPAM again. A line the
cache cannot vouch for gets the IPAM's answer at any push on the
zone, and a push at a resume asks for its own line only.
A node not yet upgraded may also have released a MAC's address and
given it to another guest. So a release now leaves an IPAM record held
by another MAC in place, with its DNS records, and fails when it
cannot check the holder. Deleting such a mapping through the API
clears the MAC's cache entry and fails.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/API2/Network/SDN/Ips.pm | 13 +-
src/PVE/Network/SDN/Dhcp.pm | 5 +-
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 176 ++++---
src/PVE/Network/SDN/Dhcp/Plugin.pm | 8 +-
src/PVE/Network/SDN/Ipams.pm | 196 +++++---
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 15 +-
src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 13 +-
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 20 +-
src/PVE/Network/SDN/Ipams/Plugin.pm | 4 +-
src/PVE/Network/SDN/Subnets.pm | 14 +-
src/PVE/Network/SDN/Vnets.pm | 10 +-
src/test/run_test_ipams.pl | 49 ++
src/test/run_test_vnets_blackbox.pl | 518 +++++++++++++++++++--
13 files changed, 865 insertions(+), 176 deletions(-)
diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
index 8a6106ba29a7..c4816a6cd4ca 100644
--- a/src/PVE/API2/Network/SDN/Ips.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -124,10 +124,13 @@ __PACKAGE__->register_method({
$vnet,
$mac,
sub {
- eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
+ my $released = eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
die "$@\n" if $@;
$push_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;
},
);
@@ -245,12 +248,16 @@ __PACKAGE__->register_method({
PVE::Network::SDN::Vnets::get_ips_from_mac($vnet, $mac);
my $old_ip = (Net::IP::ip_get_version($ip) == 4) ? $old_ip4 : $old_ip6;
- PVE::Network::SDN::Vnets::del_ip($vnet, $old_ip, '', $mac);
+ # a MAC without an address in that family has nothing to release.
+ # A release answers 0 when another MAC holds the address
+ my $released = defined($old_ip)
+ && (PVE::Network::SDN::Vnets::del_ip($vnet, $old_ip, '', $mac) // 1);
eval { PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, $vmid); };
my $error = $@;
- if ($error) {
+ # an address that was not released is not added back
+ if ($error && $released) {
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 11bb6df8c23a..a986408ecbfa 100644
--- a/src/PVE/Network/SDN/Dhcp.pm
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -22,7 +22,7 @@ sub plugin_types {
}
sub update_mapping {
- my ($vnetid, $mac) = @_;
+ my ($vnetid, $mac, $resume) = @_;
# a record without a MAC has no guest NIC behind it, so no backend serves it
return if !$mac;
@@ -39,7 +39,8 @@ sub update_mapping {
# a zone confined to other nodes has no vnet bridge on this node
return if defined($zone->{nodes}) && !$zone->{nodes}->{ PVE::INotify::nodename() };
- PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp})->update_ip_mapping($zoneid, $mac);
+ PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp})
+ ->update_ip_mapping($zoneid, $vnetid, $mac, $resume);
}
sub guest_node_by_mac {
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index 5864ead87d29..e99bfa75e506 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -12,8 +12,8 @@ use File::Copy;
use Net::DBus;
use PVE::Cluster;
-use PVE::RESTEnvironment qw(log_warn);
use PVE::Network::SDN::Ipams;
+use PVE::RESTEnvironment qw(log_warn);
my $DNSMASQ_CONFIG_ROOT = '/etc/dnsmasq.d';
my $DNSMASQ_DEFAULT_ROOT = '/etc/default';
@@ -54,36 +54,80 @@ sub update_lease {
}
sub update_ip_mapping {
- my ($class, $dhcpid, $mac) = @_;
+ my ($class, $dhcpid, $vnetid, $mac, $resume) = @_;
$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 @resume_answer;
- my $rewriteFn = sub {
- # the lines are decided from the cache, which this process may hold an
- # old read of, and the local lock does not refresh it
+ my $rewrite_fn = sub {
+ # the lines are decided from the cache and the applied config, which
+ # the local lock does not refresh
PVE::Cluster::cfs_update();
- # read under the ethers lock. A snapshot from before it would sweep a
- # line another push wrote meanwhile, and an older push would put its
- # addresses back
-
- my $macs = PVE::Network::SDN::Ipams::read_macdb()->{macs};
-
- my $entry = $macs->{$mac};
- ($ip4, $ip6) = ($entry->{ip4}, $entry->{ip6}) if $entry;
- if ($ip4 || $ip6) {
- $reservation = $mac;
- $reservation .= ",$ip4" if $ip4;
- $reservation .= ",[$ip6]" if $ip6;
+ # the file holds a line per vnet a MAC sits on, and a line belongs to
+ # the vnet of the applied subnet that holds its address
+ my $vnets = PVE::Network::SDN::Zones::get_vnets($dhcpid, 1) // {};
+ my $subnets = PVE::Network::SDN::Vnets::get_subnets(undef, 1) // {};
+ $subnets = {
+ map { $_ => $subnets->{$_} } grep { $vnets->{ $subnets->{$_}->{vnet} } }
+ keys %$subnets
+ };
+ my $vnet_of = sub {
+ my ($ip) = @_;
+ my (undef, $subnet) =
+ eval { PVE::Network::SDN::Subnets::find_ip_subnet($ip, $subnets) };
+ return $subnet ? $subnet->{vnet} : undef;
+ };
+ my %subnets_of;
+ $subnets_of{ $subnets->{$_}->{vnet} }->{$_} = $subnets->{$_} for keys %$subnets;
+
+ # the changed MAC is looked up first, under the ethers lock so that two
+ # edits of one MAC cannot land out of order, and its lines come from
+ # that answer. The other lines are checked against one read of the
+ # cache, taken after the lookup's fill, and only a line the cache
+ # cannot vouch for asks the IPAM as well. A push at a resume looked
+ # the MAC up before the lock and takes its line from the cache, which
+ # an edit that landed in between wrote as well
+ my $cache;
+ my $line_of = sub {
+ my ($line_mac, $line_ip4, $line_ip6) = @_;
+ return (undef, undef) if !$line_ip4 && !$line_ip6;
+ my $line = $line_mac;
+ $line .= ",$line_ip4" if $line_ip4;
+ $line .= ",[$line_ip6]" if $line_ip6;
# an external IPAM spells addresses its own way, while the file holds one spelling
- $reservation = lc($reservation);
+ return (lc($line), $line_ip4);
+ };
+ my $reservation = sub {
+ my ($vnet, $line_mac, $from_cache) = @_;
+ my @ips =
+ $from_cache
+ ? PVE::Network::SDN::Ipams::cached_ips_from_mac(
+ $cache,
+ $vnet,
+ $line_mac,
+ $subnets_of{$vnet} // {},
+ )
+ : PVE::Network::SDN::Vnets::get_ips_from_mac($vnet, $line_mac, 1);
+ return $line_of->($line_mac, @ips);
+ };
+ my $own;
+ if ($resume) {
+ # the lookup's answer is taken when the cache has no line to vouch
+ # for. A delete since the lookup empties the cache too, the next
+ # sweep drops that line
+ $cache = PVE::Network::SDN::Ipams::read_cache();
+ ($own, $ip4) = $reservation->($vnetid, $mac, 1)
+ if PVE::Network::SDN::Ipams::cache_witnessed($cache, $vnetid, $mac);
+ ($own, $ip4) = $line_of->($mac, @resume_answer) if !defined($own);
+ } else {
+ ($own, $ip4) = $reservation->($vnetid, $mac);
+ $cache = PVE::Network::SDN::Ipams::read_cache();
}
file_set_contents($ethers_file, '') if !-e $ethers_file;
@@ -91,62 +135,56 @@ sub update_ip_mapping {
open(my $out, '>', $ethers_tmp_file)
or die "Could not open file '$ethers_tmp_file' $!\n";
- my $kept = undef;
-
+ my %written;
+ my $ipam_failed;
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";
+ $parsed_ip = $1 if defined($parsed_ip) && $parsed_ip =~ m/\[(\S+)\]/;
+ my $vnet = defined($parsed_ip) ? $vnet_of->($parsed_ip) : undef;
+ my $key = defined($vnet) ? "$parsed_mac,$vnet" : undef;
+
+ # a line that no longer matches is replaced, a dropped one would
+ # leave the guest unknown to dnsmasq until its next start. A line
+ # whose cache entry is not vouched for gets the IPAM's answer, so
+ # the file is right after any push
+ my ($want) = eval {
+ if (!$key || $written{$key}) {
+ ();
+ } elsif ($key eq "$mac,$vnetid") {
+ ($own);
+ } elsif ($resume || $ipam_failed) {
+ # a push at a resume asks for its own line only, and a
+ # push whose lookup failed asks no further, so an IPAM
+ # that does not answer costs one request. The other lines
+ # are kept as they are
+ (lc($line));
+ } elsif (PVE::Network::SDN::Ipams::cache_witnessed($cache, $vnet, $parsed_mac)) {
+ $reservation->($vnet, $parsed_mac, 1);
} else {
- $changed = 1;
+ $reservation->($vnet, $parsed_mac);
}
- next;
- }
-
- my ($parsed_ip4, $parsed_ip6);
- if ($parsed_ip2) {
- $parsed_ip4 = $parsed_ip1;
- $parsed_ip6 = $parsed_ip2;
- } elsif (defined($parsed_ip1) && Net::IP::ip_is_ipv4($parsed_ip1)) {
- $parsed_ip4 = $parsed_ip1;
- } elsif (defined($parsed_ip1)) {
- $parsed_ip6 = $parsed_ip1;
- }
- $parsed_ip6 = $1 if $parsed_ip6 && $parsed_ip6 =~ m/\[(\S+)\]/;
- # a line from before addresses were lower-cased may spell one in upper case
- $parsed_ip6 = lc($parsed_ip6) if $parsed_ip6;
-
- # a line that no longer matches the cache is replaced by the cache's
- # current one, a dropped one would leave that guest unknown to
- # dnsmasq until its next start
- my $current = $macs->{$parsed_mac};
- if (
- !$current
- || ($parsed_ip4 && $current->{ip4} && $current->{ip4} ne $parsed_ip4)
- || ($parsed_ip6 && $current->{ip6} && lc($current->{ip6}) ne $parsed_ip6)
- ) {
+ };
+ if ($@) {
+ warn "could not read the reservation of $parsed_mac: $@";
+ $ipam_failed = 1;
+ print $out "$line\n";
+ } elsif (!defined($want)) {
$changed = 1;
- next if !$current;
- my $replacement = $parsed_mac;
- $replacement .= ",$current->{ip4}" if $current->{ip4};
- $replacement .= ",[$current->{ip6}]" if $current->{ip6};
- print $out lc($replacement) . "\n";
next;
+ } elsif (lc($line) ne $want) {
+ $changed = 1;
+ print $out "$want\n";
+ } else {
+ print $out "$line\n";
}
-
- print $out "$line\n";
+ $written{$key}++;
}
- if (defined($reservation) && !$kept) {
- print $out "$reservation\n";
+ if (defined($own) && !$written{"$mac,$vnetid"}) {
+ print $out "$own\n";
$changed = 1;
}
@@ -158,7 +196,13 @@ 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);
+ if ($resume) {
+ # a push at a resume holds up the guest, so it asks the IPAM before
+ # the lock and does not wait long for it
+ @resume_answer = eval { PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac, 1) };
+ die "Unable to update $mac in the dnsmasq configuration: $@" if $@;
+ }
+ PVE::Tools::lock_file("$ethers_file.lock", $resume ? 1 : 10, $rewrite_fn);
die "Unable to update $mac in the dnsmasq configuration: $@" if $@;
systemctl_service('reload', "dnsmasq\@$dhcpid") if $changed;
diff --git a/src/PVE/Network/SDN/Dhcp/Plugin.pm b/src/PVE/Network/SDN/Dhcp/Plugin.pm
index 472ff1534743..a3e5102b601a 100644
--- a/src/PVE/Network/SDN/Dhcp/Plugin.pm
+++ b/src/PVE/Network/SDN/Dhcp/Plugin.pm
@@ -22,10 +22,12 @@ sub private {
return $defaultData;
}
-# the MAC's records changed, so make the backend serve the current ones. With no
-# record left its reservation goes. Dies if the backend could not be updated
+# the MAC's records on the vnet changed, so make the backend serve the current
+# ones. With no record left its reservation on the vnet goes. Dies if the
+# backend could not be updated. For a resume the push does not wait long for
+# a busy backend
sub update_ip_mapping {
- my ($class, $dhcpid, $mac) = @_;
+ my ($class, $dhcpid, $vnetid, $mac, $resume) = @_;
die 'implement in sub class';
}
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 596f43951b23..8e7bd537f4ec 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -35,40 +35,10 @@ sub json_writer {
return encode_json($data);
}
-# Writers spell a MAC their own way, the generated ones in upper case and the
-# API in either, and nodes not yet upgraded look an entry up under the spelling
-# their caller hands them, so the file keeps its keys as written. Readers get a
-# view with every MAC in lower case. Two spellings that disagree on an address
-# are left out of it, since one of them is stale and nothing here says which,
-# and so is an entry without an address. The next lookup then asks the IPAM
-my sub fold_macdb {
- my ($db) = @_;
-
- my $macs = {};
- my %clash;
- for my $key (sort keys %{ $db->{macs} // {} }) {
- my $entry = $db->{macs}->{$key};
- for my $family (grep { defined($entry->{$_}) } qw(ip4 ip6)) {
- my $seen = $macs->{ lc($key) }->{$family};
- $clash{ lc($key) } = 1 if defined($seen) && lc($seen) ne lc($entry->{$family});
- $macs->{ lc($key) }->{$family} //= $entry->{$family};
- }
- }
- delete $macs->{$_} for keys %clash;
- return { %$db, macs => $macs };
-}
-
-# the file as written, which the writers change and write back
-sub read_macdb_file {
- my () = @_;
-
- return cfs_read_file($macdb_filename);
-}
-
sub read_macdb {
my () = @_;
- return fold_macdb(read_macdb_file());
+ return cfs_read_file($macdb_filename);
}
sub write_macdb {
@@ -77,9 +47,42 @@ 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 new MAC is written as the caller spells it
-my sub set_cache_entry {
+# the map under the MAC alone is the one nodes not yet upgraded read and
+# write, under the MAC as their caller spelled it, so one read indexes it by
+# the MAC in lower case for every lookup on that read. Spellings that
+# disagree on an address are both left out, since one of them is stale
+my sub old_index {
+ my ($db) = @_;
+
+ my (%index, %clash);
+ my $macs = $db->{macs} // {};
+ for my $key (sort keys %$macs) {
+ my $mac = lc($key);
+ next if $clash{$mac};
+ my $entry = $index{$mac} //= {};
+ for my $family (qw(ip4 ip6)) {
+ my $ip = $macs->{$key}->{$family} // next;
+ if (defined($entry->{$family}) && lc($entry->{$family}) ne lc($ip)) {
+ $clash{$mac} = 1;
+ last;
+ }
+ $entry->{$family} = $ip;
+ }
+ }
+ delete $index{$_} for keys %clash;
+ return \%index;
+}
+
+sub read_cache {
+ my $db = read_macdb();
+ return { db => $db, old => old_index($db) };
+}
+
+# a write lands in the map under the MAC alone as well, under the keys it
+# already has, with one address per MAC as a node not yet upgraded reads
+# and writes it. A new MAC is written as the caller spells it, nodes not yet
+# upgraded look it up like that
+my sub set_old_cache_entry {
my ($db, $mac, $family, $ip) = @_;
my $macs = $db->{macs} //= {};
@@ -87,15 +90,78 @@ my sub set_cache_entry {
$macs->{$_}->{$family} = $ip for (@keys ? @keys : ($mac));
}
+my sub in_subnets {
+ my ($ip, $subnets) = @_;
+ return !!eval { PVE::Network::SDN::Subnets::find_ip_subnet($ip, $subnets) };
+}
+
+# the addresses the per-vnet entries hold for the MAC. A write puts its
+# address into the map under the MAC alone as well, so a value found in both
+# is some vnet's and not an entry from before
+my sub claimed_addresses {
+ my ($db, $mac) = @_;
+
+ my %claimed;
+ for my $vnet_entries (values %{ $db->{vnets} // {} }) {
+ my $entry = $vnet_entries->{ lc($mac) } // next;
+ $claimed{ lc($_) } = 1 for grep { defined($_) } @{$entry}{qw(ip4 ip6)};
+ }
+ return \%claimed;
+}
+
+# the vnet's entry answers, and an entry from before the cache was kept per
+# vnet fills the families it lacks with addresses in the vnet's subnets
+my sub cache_entry {
+ my ($cache, $mac, $vnetid, $subnets) = @_;
+
+ my $entry = (($cache->{db}->{vnets} // {})->{$vnetid} // {})->{ lc($mac) } // {};
+ my $old = $cache->{old}->{ lc($mac) } // {};
+ my $claimed = claimed_addresses($cache->{db}, $mac);
+ my $cached = {};
+ for my $family (qw(ip4 ip6)) {
+ my $ip = $entry->{$family};
+ $ip = $old->{$family}
+ if !defined($ip)
+ && defined($old->{$family})
+ && !$claimed->{ lc($old->{$family}) }
+ && in_subnets($old->{$family}, $subnets);
+ $cached->{$family} = $ip if defined($ip);
+ }
+ return %$cached ? $cached : undef;
+}
+
+# a node not yet upgraded releases or moves an address in the map under the
+# MAC alone, so a vnet's address holds only while that map still names it. The
+# same address on another vnet of the MAC leaves that map unable to say which
+# vnet's it names, so neither is trusted
+sub cache_witnessed {
+ my ($cache, $vnetid, $mac) = @_;
+
+ my $vnets = $cache->{db}->{vnets} // {};
+ my $entry = ($vnets->{$vnetid} // {})->{ lc($mac) } // {};
+ my $old = $cache->{old}->{ lc($mac) } // {};
+ for my $family (grep { defined($entry->{$_}) } qw(ip4 ip6)) {
+ my $ip = lc($entry->{$family});
+ return 0 if lc($old->{$family} // '') ne $ip;
+ for my $other (grep { $_ ne $vnetid } keys %$vnets) {
+ my $other_entry = $vnets->{$other}->{ lc($mac) } // next;
+ return 0 if lc($other_entry->{$family} // '') eq $ip;
+ }
+ }
+ return 1;
+}
+
sub add_cache_mac_ip {
- my ($mac, $ip) = @_;
+ my ($vnetid, $mac, $ip) = @_;
+ 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}->{ lc($mac) }->{$family} = $ip;
+ set_old_cache_entry($db, $mac, $family, $ip);
write_macdb($db);
},
);
@@ -103,19 +169,29 @@ sub add_cache_mac_ip {
}
sub del_cache_mac_ip {
- my ($mac, $ip) = @_;
+ my ($vnetid, $mac, $ip) = @_;
+ $mac = lc($mac);
+ my $family = Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6';
cfs_lock_file(
$macdb_filename,
undef,
sub {
- my $db = read_macdb_file();
- my $family = Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6';
+ my $db = read_macdb();
+ my $vnet_entries = ($db->{vnets} // {})->{$vnetid};
+ if (my $entry = $vnet_entries ? $vnet_entries->{$mac} : undef) {
+ delete $entry->{$family};
+ delete $vnet_entries->{$mac} if !%$entry;
+ delete $db->{vnets}->{$vnetid} if !%$vnet_entries;
+ delete $db->{vnets} if !%{ $db->{vnets} };
+ }
+ # the map under the MAC alone must not hand a released address back
my $macs = $db->{macs} // {};
- for my $key (grep { lc($_) eq lc($mac) } keys %$macs) {
- delete $macs->{$key}->{$family};
- delete $macs->{$key}
- if !defined($macs->{$key}->{ip4}) && !defined($macs->{$key}->{ip6});
+ for my $key (grep { lc($_) eq $mac } keys %$macs) {
+ my $old = $macs->{$key};
+ next if lc($old->{$family} // '') ne lc($ip);
+ delete $old->{$family};
+ delete $macs->{$key} if !defined($old->{ip4}) && !defined($old->{ip6});
}
write_macdb($db);
},
@@ -168,16 +244,22 @@ sub complete_sdn_vnet {
return $cmdname eq 'add' ? [] : [PVE::Network::SDN::Vnets::sdn_ipams_ids($cfg)];
}
+# the addresses one read of the cache holds for a MAC on a vnet
+sub cached_ips_from_mac {
+ my ($cache, $vnetid, $mac, $subnets) = @_;
+
+ my $cached = cache_entry($cache, $mac, $vnetid, $subnets);
+ return $cached ? ($cached->{ip4}, $cached->{ip6}) : ();
+}
+
sub get_ips_from_mac {
- my ($mac, $zoneid, $zone, $subnets) = @_;
+ my ($vnetid, $mac, $zoneid, $zone, $subnets) = @_;
PVE::Cluster::cfs_update(); # the lock refreshes, so the snapshot must too
my $version = PVE::Cluster::cfs_file_version($macdb_filename);
- my $cached = read_macdb()->{macs}->{ lc($mac) };
- # an entry without an address was left behind by a lookup that missed, and
- # says nothing about its MAC
- return ($cached->{ip4}, $cached->{ip6})
- if $cached && (defined($cached->{ip4}) || defined($cached->{ip6}));
+ my $cache = read_cache();
+ my $cached = cache_entry($cache, $mac, $vnetid, $subnets);
+ return ($cached->{ip4}, $cached->{ip6}) if $cached && cache_witnessed($cache, $vnetid, $mac);
my $plugin_config = get_plugin_config($zone);
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
@@ -195,9 +277,17 @@ sub get_ips_from_mac {
return
if (PVE::Cluster::cfs_file_version($macdb_filename) // '') ne ($version // '');
- my $db = read_macdb_file();
- set_cache_entry($db, $mac, 'ip4', $ip4) if defined($ip4);
- set_cache_entry($db, $mac, 'ip6', $ip6) if defined($ip6);
+ my $db = read_macdb();
+ my $entry = $db->{vnets}->{$vnetid}->{ lc($mac) } //= {};
+ for my $family (qw(ip4 ip6)) {
+ my $ip = $family eq 'ip4' ? $ip4 : $ip6;
+ if (!defined($ip)) {
+ delete $entry->{$family};
+ next;
+ }
+ $entry->{$family} = $ip;
+ set_old_cache_entry($db, $mac, $family, $ip);
+ }
write_macdb($db);
},
);
diff --git a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
index 9e1c9537e6ec..84ce4382e0e7 100644
--- a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
@@ -360,7 +360,7 @@ sub add_range_next_freeip {
}
sub del_ip {
- my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr) = @_;
+ my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr, $mac) = @_;
return if !$ip;
@@ -369,6 +369,19 @@ sub del_ip {
warn "could not find id for ip $ip";
return;
}
+ if ($mac) {
+ my $record =
+ eval { netbox_api_request($plugin_config, "GET", "/ipam/ip-addresses/$ip_id/") };
+ if ($@) {
+ return 0 if $noerr;
+ die "could not check who holds IP $ip: $@";
+ }
+ my ($owner) = ($record->{description} // '') =~ m/^mac:(\S+)$/;
+ if ($owner && lc($owner) ne lc($mac)) {
+ warn "not releasing IP '$ip' for $mac, it is held by $owner\n";
+ return 0;
+ }
+ }
eval { netbox_api_request($plugin_config, "DELETE", "/ipam/ip-addresses/$ip_id/"); };
if ($@) {
diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index 43d0a3daaf9a..86e594a4c604 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -249,11 +249,12 @@ sub add_range_next_freeip {
}
sub del_ip {
- my ($class, $plugin_config, $subnetid, $subnet, $ip) = @_;
+ my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr, $mac) = @_;
my $cidr = $subnet->{cidr};
my $zone = $subnet->{zone};
+ my $released = 1;
cfs_lock_file(
$ipamdb_file,
undef,
@@ -271,11 +272,18 @@ sub del_ip {
warn "IP '$ip' does not exist in IPAM DB\n";
return;
}
+ my $owner = $dbsubnet->{ips}->{$ip}->{mac};
+ if ($mac && $owner && lc($owner) ne lc($mac)) {
+ warn "not releasing IP '$ip' for $mac, it is held by $owner\n";
+ $released = 0;
+ return;
+ }
delete $dbsubnet->{ips}->{$ip};
write_db($db);
},
);
die "$@" if $@;
+ return $released;
}
sub get_ips_from_mac {
@@ -297,7 +305,8 @@ sub get_ips_from_mac {
my $ips = $dbzone->{subnets}->{$subnet}->{ips} // next;
for my $ip (keys %$ips) {
my $ipobject = $ips->{$ip};
- if ($ipobject->{mac} && $ipobject->{mac} eq $mac) {
+ # a record keeps the MAC as its writer spelled it
+ if ($ipobject->{mac} && lc($ipobject->{mac}) eq lc($mac)) {
if (Net::IP::ip_is_ipv4($ip)) {
$ip4 = $ip;
} else {
diff --git a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
index 12d470e802c4..e969c3dcdf4f 100644
--- a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
@@ -239,7 +239,7 @@ sub add_range_next_freeip {
}
sub del_ip {
- my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr) = @_;
+ my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr, $mac) = @_;
return if !$ip;
@@ -250,6 +250,22 @@ sub del_ip {
my $ip_id = get_ip_id($url, $ip, $headers);
return if !$ip_id;
+ if ($mac) {
+ my $record = eval {
+ PVE::Network::SDN::api_request(
+ "GET", "$url/addresses/$ip_id/", $headers, undef, $fingerprint,
+ );
+ };
+ if ($@) {
+ return 0 if $noerr;
+ die "could not check who holds IP $ip: $@";
+ }
+ my $owner = ($record->{data} // {})->{mac};
+ if ($owner && lc($owner) ne lc($mac)) {
+ warn "not releasing IP '$ip' for $mac, it is held by $owner\n";
+ return 0;
+ }
+ }
eval {
PVE::Network::SDN::api_request(
@@ -259,6 +275,8 @@ sub del_ip {
if ($@) {
die "error delete ip $ip: $@" if !$noerr;
}
+
+ return 1;
}
sub get_ips_from_mac {
diff --git a/src/PVE/Network/SDN/Ipams/Plugin.pm b/src/PVE/Network/SDN/Ipams/Plugin.pm
index 14f8fe9e67fe..6e23d5434d4b 100644
--- a/src/PVE/Network/SDN/Ipams/Plugin.pm
+++ b/src/PVE/Network/SDN/Ipams/Plugin.pm
@@ -132,8 +132,10 @@ sub add_range_next_freeip {
die "please implement inside plugin";
}
+# a MAC names the guest the address is released for, a record holding the
+# address for another MAC is then left alone and 0 returned
sub del_ip {
- my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr) = @_;
+ my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr, $mac) = @_;
die "please implement inside plugin";
}
diff --git a/src/PVE/Network/SDN/Subnets.pm b/src/PVE/Network/SDN/Subnets.pm
index f8be98201476..a7fa7056e556 100644
--- a/src/PVE/Network/SDN/Subnets.pm
+++ b/src/PVE/Network/SDN/Subnets.pm
@@ -266,7 +266,7 @@ sub add_next_free_ip {
die $@ if $@;
- eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, $ip); };
+ eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($subnet->{vnet}, $mac, $ip); };
warn $@ if $@;
}
@@ -325,7 +325,7 @@ sub add_ip {
};
die $@ if $@;
- eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, $ip) if $mac; };
+ eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($subnet->{vnet}, $mac, $ip) if $mac; };
warn $@ if $@;
}
@@ -413,18 +413,22 @@ sub del_ip {
verify_dns_zone($reversednszone, $reversedns);
}
+ my $released;
if ($ipamid) {
my $ipam_cfg = PVE::Network::SDN::Ipams::config();
my $plugin_config = $ipam_cfg->{ids}->{$ipamid};
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
- $plugin->del_ip($plugin_config, $subnetid, $subnet, $ip);
+ $released = $plugin->del_ip($plugin_config, $subnetid, $subnet, $ip, undef, $mac);
if ($mac) {
- eval { PVE::Network::SDN::Ipams::del_cache_mac_ip($mac, $ip) };
+ eval { PVE::Network::SDN::Ipams::del_cache_mac_ip($subnet->{vnet}, $mac, $ip) };
warn $@ if $@;
}
}
+ # a record another MAC holds keeps its DNS entries as well
+ return 0 if defined($released) && !$released;
+
eval {
if (!$skipdns) {
del_dns_record($dnszone, $dns, $hostname, $ip);
@@ -434,6 +438,8 @@ sub del_ip {
if ($@) {
warn $@;
}
+
+ return $released;
}
=head3 api_get_subnet($cfg, $id)
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index 395aa32b6a84..58db0f803f8b 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -194,18 +194,18 @@ sub del_ip {
}
sub get_ips_from_mac {
- my ($vnetid, $mac) = @_;
+ my ($vnetid, $mac, $running) = @_;
- my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid);
+ my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid, $running);
return if !$vnet;
my $zoneid = $vnet->{zone};
- my $zone = PVE::Network::SDN::Zones::get_zone($zoneid);
+ my $zone = PVE::Network::SDN::Zones::get_zone($zoneid, $running);
return if !$zone->{ipam} || !$zone->{dhcp};
- my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid);
- return PVE::Network::SDN::Ipams::get_ips_from_mac($mac, $zoneid, $zone, $subnets);
+ my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid, $running);
+ return PVE::Network::SDN::Ipams::get_ips_from_mac($vnetid, $mac, $zoneid, $zone, $subnets);
}
sub del_ips_from_mac {
diff --git a/src/test/run_test_ipams.pl b/src/test/run_test_ipams.pl
index f9c44fe1a3b9..f07b4bb4b486 100755
--- a/src/test/run_test_ipams.pl
+++ b/src/test/run_test_ipams.pl
@@ -219,5 +219,54 @@ foreach my $path (@plugins) {
}
+# an address released for a MAC stays when the external IPAM records it
+# for another MAC
+{
+ my $subnet = { zone => 'z', cidr => '10.0.0.0/24' };
+ my $owner;
+ my $check_fails = 0;
+ my @deletes;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url) = @_;
+ die "Invalid response from server: 503 Service Unavailable\n"
+ if $check_fails && $method eq 'GET';
+ push @deletes, $url if $method eq 'DELETE';
+ return {
+ description => "mac:$owner",
+ data => { mac => $owner },
+ };
+ },
+ );
+ my $netbox = Test::MockModule->new('PVE::Network::SDN::Ipams::NetboxPlugin');
+ $netbox->mock(subnet_prefix => sub { return (1, 2); }, get_ip_id => sub { return 3; });
+ my $phpipam = Test::MockModule->new('PVE::Network::SDN::Ipams::PhpIpamPlugin');
+ $phpipam->mock(subnet_id => sub { return 7; }, get_ip_id => sub { return 3; });
+
+ for my $plugin (
+ 'PVE::Network::SDN::Ipams::NetboxPlugin',
+ 'PVE::Network::SDN::Ipams::PhpIpamPlugin',
+ ) {
+ my $config = { url => 'u', token => 't' };
+ @deletes = ();
+ $owner = 'da:65:8f:18:9b:70';
+ $plugin->del_ip($config, 'z-10.0.0.0-24', $subnet, '10.0.0.50', 1, 'DA:65:8F:18:9B:6F');
+ is(scalar(@deletes), 0, "$plugin keeps the record of another MAC");
+ $owner = 'da:65:8f:18:9b:6f';
+ $plugin->del_ip($config, 'z-10.0.0.0-24', $subnet, '10.0.0.50', 1, 'DA:65:8F:18:9B:6F');
+ is(scalar(@deletes), 1, "$plugin releases the MAC's own record");
+
+ # a failed check of the holder releases nothing either
+ @deletes = ();
+ $check_fails = 1;
+ my $released =
+ $plugin->del_ip($config, 'z-10.0.0.0-24', $subnet, '10.0.0.50', 1, 'DA:65:8F:18:9B:6F');
+ $check_fails = 0;
+ is($released, 0, "$plugin releases nothing when the check of the holder fails");
+ is(scalar(@deletes), 0, "$plugin sent no delete for it");
+ }
+}
+
done_testing();
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 752e5b0d3c70..b00be65bae48 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -4,8 +4,11 @@ use strict;
use warnings;
use lib qw(..);
+use Fcntl qw(LOCK_EX);
use File::Slurp;
+use IO::File;
use JSON;
+use POSIX qw(_exit);
use List::Util qw(first all);
use NetAddr::IP qw(:lower);
@@ -195,7 +198,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 {
@@ -518,74 +521,118 @@ sub test_ipam_cache_lookup {
# an entry holding no address, left behind by an earlier miss, does not
# hide a record created since
create_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.50" });
- $test_state->{macdb}->{macs}->{$mac} = { ip4 => undef, ip6 => undef };
+ $test_state->{macdb} = { macs => { $mac => { ip4 => undef, ip6 => undef } } };
my @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
is($ips[0], "10.0.0.50", "$test_name: an address-less entry counts as a miss");
- is(
- $test_state->{macdb}->{macs}->{$mac}->{ip4},
- "10.0.0.50",
- "$test_name: the answer is cached",
+ is_deeply(
+ $test_state->{macdb},
+ {
+ macs => { $mac => { ip4 => "10.0.0.50", ip6 => undef } },
+ vnets => { $vnetid => { $mac => { ip4 => "10.0.0.50" } } },
+ },
+ "$test_name: the answer is cached in both maps",
);
# a write to the cache between the miss and the fill, which the file's
# version shows, keeps the IPAM's answer out of the cache
- delete $test_state->{macdb}->{macs}->{$mac};
+ $test_state->{macdb} = {};
my $version = 0;
$mocked_pve_cluster_obj->mock(cfs_file_version => sub { return $version++ });
@ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
$mocked_pve_cluster_obj->unmock('cfs_file_version');
is($ips[0], "10.0.0.50", "$test_name: the lookup still answers");
- ok(!exists $test_state->{macdb}->{macs}->{$mac},
- "$test_name: but leaves a changed cache alone");
+ is_deeply($test_state->{macdb}, {}, "$test_name: but leaves a changed cache alone");
- # an entry written under the config's spelling, as a node not yet upgraded
- # writes it, is read and released like any other and keeps its key
+ # a MAC new to the map under the MAC alone is written as the caller
+ # spells it, a node not yet upgraded looks it up so
+ $test_state->{macdb} = {};
+ PVE::Network::SDN::Ipams::add_cache_mac_ip($vnetid, uc($mac), '10.0.0.160');
+ is_deeply(
+ [keys %{ $test_state->{macdb}->{macs} }],
+ [uc($mac)],
+ "$test_name: a new key carries the caller's spelling",
+ );
+ is(
+ $test_state->{macdb}->{vnets}->{$vnetid}->{$mac}->{ip4},
+ '10.0.0.160',
+ "$test_name: and the vnet's entry is keyed in lower case",
+ );
+ $test_state->{macdb} = {};
+
+ # an entry written under the config's spelling is read and released like
+ # any other
my $other = "da:65:8f:18:9b:70";
$test_state->{macdb}->{macs} = {
uc($mac) => { ip4 => '10.0.0.150' },
uc($other) => { ip4 => '10.0.0.170' },
};
my ($ip4) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
- is($ip4, '10.0.0.150', "$test_name: an entry is found however it is spelled");
- PVE::Network::SDN::Ipams::del_cache_mac_ip($mac, '10.0.0.150');
+ is($ip4, '10.0.0.150', "$test_name: an old entry is found however it is spelled");
+ PVE::Network::SDN::Ipams::del_cache_mac_ip($vnetid, $mac, '10.0.0.150');
is_deeply(
$test_state->{macdb}->{macs},
{ uc($other) => { ip4 => '10.0.0.170' } },
"$test_name: and released, the other entry left as written",
);
- # a write in another spelling goes to the spelling the file holds
+ # a write by this code updates an old entry under the spelling a node not
+ # yet upgraded wrote it in, so that node still finds it
$test_state->{macdb}->{macs} = { uc($mac) => { ip4 => '10.0.0.150' } };
- PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, 'fd00::150');
+ PVE::Network::SDN::Ipams::add_cache_mac_ip($vnetid, uc($mac), 'fd00::150');
is_deeply(
$test_state->{macdb}->{macs},
{ uc($mac) => { ip4 => '10.0.0.150', ip6 => 'fd00::150' } },
- "$test_name: a write keeps the key as written",
+ "$test_name: a write updates the old entry under its spelling",
+ );
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac)],
+ ['10.0.0.150', 'fd00::150'],
+ "$test_name: holding both addresses",
);
- # a lookup past an address-less entry asks the IPAM and fills the entry
- # under its spelling
- $test_state->{macdb}->{macs} = { uc($mac) => { ip4 => undef, ip6 => undef } };
+ # a vnet's entry and an old one that disagree are not trusted either, a
+ # node not yet upgraded may have moved the address since, and a family
+ # the old entry lacks was released by such a node
+ $test_state->{macdb} = {
+ macs => { $mac => { ip4 => '10.0.0.151' } },
+ vnets => { $vnetid => { $mac => { ip4 => '10.0.0.150', ip6 => 'fd00::150' } } },
+ };
@ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
- is($ips[0], '10.0.0.50', "$test_name: a lookup past an address-less entry asks the IPAM");
+ is(
+ $ips[0],
+ '10.0.0.50',
+ "$test_name: an old entry disagreeing with the vnet's counts as a miss",
+ );
is_deeply(
- $test_state->{macdb}->{macs},
- { uc($mac) => { ip4 => '10.0.0.50', ip6 => undef } },
- "$test_name: and fills the entry under its spelling",
+ $test_state->{macdb},
+ {
+ macs => { $mac => { ip4 => '10.0.0.50' } },
+ vnets => { $vnetid => { $mac => { ip4 => '10.0.0.50' } } },
+ },
+ "$test_name: the answer replaces the vnet's entry and the old one,"
+ . " the family without a record is gone",
);
# two spellings that disagree are not trusted, the lookup asks the IPAM
- $test_state->{macdb}->{macs} = {
- uc($mac) => { ip4 => '10.0.0.150' },
- $mac => { ip4 => '10.0.0.151' },
+ $test_state->{macdb} = {
+ macs => {
+ uc($mac) => { ip4 => '10.0.0.150' },
+ $mac => { ip4 => '10.0.0.151' },
+ },
};
@ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
is($ips[0], '10.0.0.50', "$test_name: a clash of spellings counts as a miss");
- is_deeply(
- $test_state->{macdb}->{macs},
- { uc($mac) => { ip4 => '10.0.0.50' }, $mac => { ip4 => '10.0.0.50' } },
- "$test_name: and its write brings both spellings in line",
- );
+
+ # an old entry's address outside the vnet's subnets is another vnet's
+ $test_state->{macdb} = { macs => { uc($mac) => { ip4 => '10.9.0.150' } } };
+ @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is($ips[0], '10.0.0.50', "$test_name: an old address of another vnet counts as a miss");
+
+ # the record keeps the MAC as its writer spelled it, a lookup in another
+ # spelling finds it
+ $test_state->{macdb} = {};
+ @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, uc($mac));
+ is($ips[0], '10.0.0.50', "$test_name: a lookup in another spelling finds the record");
# a mapping whose record is gone is still deleted, and its cache entry
# with it
@@ -1169,21 +1216,40 @@ sub test_dnsmasq_stale_reservation_replaced {
my $stale = "da:65:8f:18:9b:01";
my $gone = "da:65:8f:18:9b:02";
my $added = "da:65:8f:18:9b:03";
+ create_dhcp_zone("TESTZONE", "testvnet", "10.0.1.0/24");
$test_state->{macdb} = {
- macs => {
- $stale => { ip4 => '10.0.1.11' },
- $added => { ip4 => '10.0.1.13' },
+ macs => { $stale => { ip4 => '10.0.1.11' }, $added => { ip4 => '10.0.1.13' } },
+ vnets => {
+ testvnet => {
+ $stale => { ip4 => '10.0.1.11' },
+ $added => { ip4 => '10.0.1.13' },
+ },
},
};
PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$stale,10.0.1.10\n$gone,10.0.1.12\n");
- PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping('TESTZONE', $added);
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping('TESTZONE', 'testvnet', $added);
is(
PVE::Tools::file_get_contents($TMP_ETHERS_FILE),
"$stale,10.0.1.11\n$added,10.0.1.13\n",
"$test_name: the stale line is replaced, the unknown one dropped",
);
+
+ # the changed MAC's own line follows the IPAM's answer, not the cache,
+ # when its entry is no longer witnessed
+ create_ip({ zone => "TESTZONE", vnet => "testvnet", mac => $added, ip => "10.0.1.20" });
+ $test_state->{macdb} = {
+ macs => { $added => { ip4 => '10.0.1.22' } },
+ vnets => { testvnet => { $added => { ip4 => '10.0.1.21' } } },
+ };
+ PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$added,10.0.1.21\n");
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping('TESTZONE', 'testvnet', $added);
+ is(
+ PVE::Tools::file_get_contents($TMP_ETHERS_FILE),
+ "$added,10.0.1.20\n",
+ "$test_name: the changed MAC's stale line is replaced by the IPAM's answer",
+ );
}
run_test(\&test_dnsmasq_stale_reservation_replaced);
@@ -1405,8 +1471,390 @@ sub test_ipam_per_vnet {
"$test_name: vnet$n answers with its own address",
);
}
+
+ # a mapping edit on one vnet leaves the other's answer alone
+ PVE::API2::Network::SDN::Ips->ipupdate(
+ { zone => $zoneid, vnet => 'vnet1', mac => $mac, ip => '10.1.0.51' });
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+ ['10.2.0.50', undef],
+ "$test_name: an edit on the first vnet leaves the second's answer alone",
+ );
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet1', $mac)],
+ ['10.1.0.51', undef],
+ "$test_name: and changes the first's",
+ );
+
+ # an entry from before the cache was kept per vnet answers for a vnet
+ # without an entry of its own, until a write for the MAC on another vnet
+ # replaces it in the map under the MAC alone, which holds one address.
+ # The vnet then asks the IPAM
+ $test_state->{macdb} = { macs => { $mac => { ip4 => '10.2.0.60' } } };
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+ ['10.2.0.60', undef],
+ "$test_name: an old entry answers for a vnet without an entry of its own",
+ );
+ create_ip({ zone => $zoneid, vnet => 'vnet1', mac => $mac, ip => '10.1.0.52' });
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+ ['10.2.0.50', undef],
+ "$test_name: once a write for the MAC elsewhere replaced it, the IPAM answers",
+ );
+
+ # a vnet's address holds only while the map under the MAC alone names it,
+ # since that map is where a node not yet upgraded releases or moves an
+ # address. With the MAC on two vnets the map witnesses one of them
+ $test_state->{macdb} = {
+ macs => { $mac => { ip4 => '10.2.0.50' } },
+ vnets => {
+ vnet1 => { $mac => { ip4 => '10.1.0.50' } },
+ vnet2 => { $mac => { ip4 => '10.2.0.50' } },
+ },
+ };
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+ ['10.2.0.50', undef],
+ "$test_name: the vnet whose address the old map names answers from the cache",
+ );
+ like(
+ (PVE::Network::SDN::Vnets::get_ips_from_mac('vnet1', $mac))[0],
+ qr/^10\.1\.0\.5[12]$/,
+ "$test_name: the other one asks the IPAM",
+ );
+ like(
+ $test_state->{macdb}->{macs}->{$mac}->{ip4},
+ qr/^10\.1\.0\.5[12]$/,
+ "$test_name: and its answer takes the old map",
+ );
+ delete $test_state->{macdb}->{macs};
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+ ['10.2.0.50', undef],
+ "$test_name: without the family in the old map the IPAM is asked",
+ );
+
+ # the address of the MAC on a vnet of another zone with the same range
+ # sits in the old map as well, and is that vnet's, so a vnet without an
+ # entry asks the IPAM rather than taking it, and one with an entry the
+ # old map does not name asks as well
+ create_zone({ type => "simple", dhcp => "dnsmasq", ipam => "pve", zone => "ZONEB" });
+ create_dhcp_vnet("ZONEB", "vnet3", "10.1.0.0/24");
+ create_ip({ zone => "ZONEB", vnet => "vnet3", mac => $mac, ip => "10.1.0.70" });
+ $test_state->{macdb} = {
+ macs => { $mac => { ip4 => '10.1.0.52' } },
+ vnets => { vnet1 => { $mac => { ip4 => '10.1.0.52' } } },
+ };
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet3', $mac)],
+ ['10.1.0.70', undef],
+ "$test_name: another zone's address in the old map is not taken"
+ . " for a vnet without an entry",
+ );
+ $test_state->{macdb}->{macs}->{$mac}->{ip4} = '10.1.0.52';
+ $test_state->{macdb}->{vnets}->{vnet3}->{$mac} = { ip4 => '10.1.0.71' };
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet3', $mac)],
+ ['10.1.0.70', undef],
+ "$test_name: and an entry the old map does not name is checked with the IPAM",
+ );
+
+ # a release on an upgraded node clears the old map's slot, the MAC's
+ # other vnet asks the IPAM at its next start and takes it
+ $test_state->{macdb} = {
+ macs => { $mac => { ip4 => '10.2.0.50' } },
+ vnets => {
+ vnet1 => { $mac => { ip4 => '10.1.0.52' } },
+ vnet2 => { $mac => { ip4 => '10.2.0.50' } },
+ },
+ };
+ PVE::Network::SDN::Ipams::del_cache_mac_ip('vnet2', $mac, '10.2.0.50');
+ is_deeply(
+ $test_state->{macdb},
+ { macs => {}, vnets => { vnet1 => { $mac => { ip4 => '10.1.0.52' } } } },
+ "$test_name: a release clears the old map's slot",
+ );
}
run_test(\&test_ipam_per_vnet);
+sub test_ipam_edit_stale_entry {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+ my $other = "da:65:8f:18:9b:70";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+ # a node not yet upgraded released the MAC's address and handed it to
+ # another guest, while the vnet's cache entry still holds it. The entry
+ # is not trusted, so the edit releases nothing and records the new address
+ create_ip({ zone => $zoneid, vnet => $vnetid, mac => $other, ip => '10.0.0.50' });
+ $test_state->{macdb}->{vnets}->{$vnetid}->{$mac} = { ip4 => '10.0.0.50' };
+
+ PVE::API2::Network::SDN::Ips->ipupdate(
+ { zone => $zoneid, vnet => $vnetid, mac => $mac, ip => '10.0.0.60' });
+ is(
+ get_ip4(get_ips_from_mac($other)),
+ '10.0.0.50',
+ "$test_name: an edit leaves the other guest's record alone",
+ );
+ is(get_ip4(get_ips_from_mac($mac)), '10.0.0.60', "$test_name: and records the new address");
+ is(
+ $test_state->{macdb}->{vnets}->{$vnetid}->{$mac}->{ip4},
+ '10.0.0.60',
+ "$test_name: which the cache holds now",
+ );
+
+ # an edit that fails keeps its own error, the address the other guest
+ # holds was not released and is not added back
+ my $third = "da:65:8f:18:9b:71";
+ create_ip({ zone => $zoneid, vnet => $vnetid, mac => $third, ip => '10.0.0.70' });
+ $test_state->{macdb}->{vnets}->{$vnetid}->{$mac} = { ip4 => '10.0.0.50' };
+ $test_state->{macdb}->{macs}->{$mac} = { ip4 => '10.0.0.50' };
+ eval {
+ PVE::API2::Network::SDN::Ips->ipupdate(
+ { zone => $zoneid, vnet => $vnetid, mac => $mac, ip => '10.0.0.70' });
+ };
+ like($@, qr/10\.0\.0\.70/, "$test_name: a failed edit names the address it could not add");
+ is(
+ get_ip4(get_ips_from_mac($other)),
+ '10.0.0.50',
+ "$test_name: and leaves the other guest's record alone",
+ );
+
+ # a deletion of a mapping the IPAM does not hold is refused
+ $test_state->{macdb}->{vnets}->{$vnetid}->{$mac} = { ip4 => '10.0.0.50' };
+ eval {
+ PVE::API2::Network::SDN::Ips->ipdelete(
+ { zone => $zoneid, vnet => $vnetid, mac => $mac, ip => '10.0.0.50' });
+ };
+ like($@, qr/not mapped to/, "$test_name: a deletion for another guest's address is refused");
+}
+
+run_test(\&test_ipam_edit_stale_entry);
+
+sub test_dnsmasq_reservation_per_vnet {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ # the same MAC on two vnets of one zone has a reservation on each, and a
+ # start on one vnet changes that vnet's line only
+ create_zone({ type => "simple", dhcp => "dnsmasq", ipam => "pve", zone => $zoneid });
+ for my $n (1, 2) {
+ create_dhcp_vnet($zoneid, "vnet$n", "10.$n.0.0/24");
+ create_ip({ zone => $zoneid, vnet => "vnet$n", mac => $mac, ip => "10.$n.0.50" });
+ }
+
+ my $reservations = sub {
+ return [sort grep { $_ } split(/\n/, PVE::Tools::file_get_contents($TMP_ETHERS_FILE))];
+ };
+
+ nic_start("vnet$_", $mac, "999", "testhostname") for (1, 2, 1);
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.50", "$mac,10.2.0.50"],
+ "$test_name: each vnet keeps its reservation",
+ );
+
+ PVE::API2::Network::SDN::Ips->ipupdate(
+ { zone => $zoneid, vnet => 'vnet1', mac => $mac, ip => '10.1.0.51' });
+ nic_start('vnet1', $mac, "999", "testhostname");
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.51", "$mac,10.2.0.50"],
+ "$test_name: an edit replaces the reservation of its vnet only",
+ );
+
+ # the other lines are checked against the cache alone, so a line neither
+ # map holds is dropped without the IPAM being asked
+ delete $test_state->{macdb}->{vnets}->{vnet2};
+ delete $test_state->{macdb}->{macs};
+ nic_start('vnet1', $mac, "999", "testhostname");
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.51"],
+ "$test_name: a line the cache does not hold is dropped",
+ );
+
+ # the changed MAC's own line is answered by the IPAM when the cache does
+ # not hold it
+ delete $test_state->{macdb}->{vnets}->{vnet1};
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, 'vnet1', $mac);
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.51"],
+ "$test_name: the changed MAC's line is kept without a cache entry",
+ );
+
+ # a push at a resume asks the IPAM like any other when the cache cannot
+ # vouch for the MAC
+ $test_state->{macdb} = {};
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, 'vnet1', $mac, 1);
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.51"],
+ "$test_name: a push at a resume asks the IPAM for a MAC the cache does not hold",
+ );
+ is(
+ $test_state->{macdb}->{vnets}->{vnet1}->{$mac}->{ip4},
+ '10.1.0.51',
+ "$test_name: and fills the cache",
+ );
+
+ # a push at a resume waits for a push holding the file, but not long,
+ # so the guest is not kept paused for a push waiting on the IPAM
+ $mocked_pve_tools->unmock('lock_file');
+ $test_state->{macdb} = {
+ macs => { $mac => { ip4 => '10.1.0.52' } },
+ vnets => { vnet1 => { $mac => { ip4 => '10.1.0.52' } } },
+ };
+ # the holder says when it has the lock and leaves a file behind when it
+ # lets go, so the tests measure no time
+ my $let_go = "$TMP_ETHERS_FILE.let-go";
+ my $hold_lock = sub {
+ my ($seconds) = @_;
+ unlink($let_go);
+ pipe(my $has_lock, my $holder_end) or die "pipe failed: $!";
+ my $pid = fork() // die "fork failed: $!";
+ if (!$pid) {
+ my $lock = IO::File->new(">>$TMP_ETHERS_FILE.lock");
+ flock($lock, LOCK_EX);
+ close($holder_end);
+ select(undef, undef, undef, $seconds);
+ IO::File->new(">$let_go");
+ _exit(0);
+ }
+ close($holder_end);
+ readline($has_lock);
+ return $pid;
+ };
+ my $pid = $hold_lock->(0.3);
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, 'vnet1', $mac, 1);
+ ok(-e $let_go, "$test_name: a push at a resume waits for a push holding the file");
+ waitpid($pid, 0);
+ is_deeply(
+ $reservations->(), ["$mac,10.1.0.52"], "$test_name: and writes the line",
+ );
+
+ $test_state->{macdb}->{vnets}->{vnet1}->{$mac}->{ip4} = '10.1.0.53';
+ $test_state->{macdb}->{macs}->{$mac}->{ip4} = '10.1.0.53';
+ $pid = $hold_lock->(2.5);
+ eval { PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, 'vnet1', $mac, 1) };
+ my $err = $@;
+ ok(!-e $let_go, "$test_name: a push at a resume does not wait out a long hold");
+ waitpid($pid, 0);
+ like($err, qr/Unable to update/, "$test_name: and fails");
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.52"],
+ "$test_name: and left the line to the next push",
+ );
+ $mocked_pve_tools->mock(lock_file => $mocked_cfs_lock_file);
+
+ # the lookup's fill is skipped when the cache file changed meanwhile, the
+ # line then comes from the lookup's answer, the cache cannot vouch for it
+ $test_state->{macdb} = {
+ macs => { $mac => { ip4 => '10.1.0.61' } },
+ vnets => { vnet1 => { $mac => { ip4 => '10.1.0.60' } } },
+ };
+ PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$mac,10.1.0.60\n");
+ my $version = 0;
+ $mocked_pve_cluster_obj->mock(cfs_file_version => sub { return $version++ });
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, 'vnet1', $mac, 1);
+ $mocked_pve_cluster_obj->unmock('cfs_file_version');
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.51"],
+ "$test_name: a push at a resume writes the IPAM's answer when its fill was skipped",
+ );
+ is(
+ $test_state->{macdb}->{vnets}->{vnet1}->{$mac}->{ip4},
+ '10.1.0.60',
+ "$test_name: with the cache left as it was",
+ );
+
+ # the same with no entry for the MAC in either map
+ $test_state->{macdb} = {};
+ PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$mac,10.1.0.60\n");
+ $version = 0;
+ $mocked_pve_cluster_obj->mock(cfs_file_version => sub { return $version++ });
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, 'vnet1', $mac, 1);
+ $mocked_pve_cluster_obj->unmock('cfs_file_version');
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.51"],
+ "$test_name: a push at a resume writes the answer when the cache has nothing",
+ );
+ is_deeply($test_state->{macdb}, {}, "$test_name: and leaves the cache empty");
+
+ # a lookup that fails in the sweep ends the asking for that push, so an
+ # IPAM that does not answer costs one request, the lines are kept
+ my ($other1, $other2) = ('da:65:8f:18:9b:72', 'da:65:8f:18:9b:73');
+ $test_state->{macdb} = {
+ macs => {
+ $mac => { ip4 => '10.1.0.53' },
+ $other1 => { ip4 => '10.1.0.71' },
+ $other2 => { ip4 => '10.2.0.71' },
+ },
+ vnets => {
+ vnet1 => { $mac => { ip4 => '10.1.0.53' }, $other1 => { ip4 => '10.1.0.70' } },
+ vnet2 => { $other2 => { ip4 => '10.2.0.70' } },
+ },
+ };
+ PVE::Tools::file_set_contents(
+ $TMP_ETHERS_FILE,
+ "$mac,10.1.0.53\n$other1,10.1.0.70\n$other2,10.2.0.70\n",
+ );
+ my $asked = 0;
+ my $mocked_pve_plugin = Test::MockModule->new('PVE::Network::SDN::Ipams::PVEPlugin');
+ $mocked_pve_plugin->mock(get_ips_from_mac => sub { $asked++; die "IPAM down\n"; });
+ my @warned;
+ {
+ local $SIG{__WARN__} = sub { push @warned, @_; };
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, 'vnet1', $mac);
+ }
+ $mocked_pve_plugin->unmock('get_ips_from_mac');
+ is($asked, 1, "$test_name: a failed lookup in the sweep ends the asking");
+ is(scalar(@warned), 1, "$test_name: with one warning");
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.53", "$other1,10.1.0.70", "$other2,10.2.0.70"],
+ "$test_name: and the lines are kept",
+ );
+
+ # a line the cache cannot vouch for gets the IPAM's answer at any push,
+ # a push at a resume writes its own line only
+ $test_state->{macdb} = {
+ macs => { $mac => { ip4 => '10.1.0.52' } },
+ vnets => {
+ vnet1 => { $mac => { ip4 => '10.1.0.52' } },
+ vnet2 => { $mac => { ip4 => '10.2.0.60' } },
+ },
+ };
+ PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$mac,10.1.0.52\n$mac,10.2.0.61\n");
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, 'vnet1', $mac, 1);
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.52", "$mac,10.2.0.61"],
+ "$test_name: a push at a resume leaves the other lines alone",
+ );
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, 'vnet1', $mac);
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.52", "$mac,10.2.0.50"],
+ "$test_name: another push gives a line the cache cannot vouch for the IPAM's answer",
+ );
+ is(
+ $test_state->{macdb}->{vnets}->{vnet2}->{$mac}->{ip4},
+ '10.2.0.50',
+ "$test_name: and fills its cache entry",
+ );
+}
+
+run_test(\&test_dnsmasq_reservation_per_vnet);
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH pve-network v6 09/13] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM
2026-11-05 9:35 [PATCH docs/manager/network/qemu-server v6 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (7 preceding siblings ...)
2026-11-05 9:35 ` [PATCH pve-network v6 08/13] sdn: ipam: keep the MAC cache per vnet Hannes Laimer
@ 2026-11-05 9:35 ` Hannes Laimer
2026-11-05 9:35 ` [PATCH qemu-server v6 10/13] network: sdn: push the DHCP mappings of a guest when it resumes Hannes Laimer
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Hannes Laimer @ 2026-11-05 9:35 UTC (permalink / raw)
To: pve-devel
Two zones can use the same subnet, and an external IPAM asked by the
address alone cannot tell their records apart. With a new option of
the IPAM entry, the subnets added to a zone go into the zone's own
namespace, a VRF in NetBox and a sub-section of the configured
section in phpIPAM. The option is off by default, since a node not
yet upgraded does not know the namespaces and keeps looking records
up by the address alone. With the option off the lookups run as
before, the first prefix or subnet of that range in any VRF or section
counts. A namespace is named after its zone and created with its first
subnet. A second option sets the start of the names, pve by default,
so clusters sharing an IPAM can keep theirs apart.
With the namespaces on, an existing subnet is looked for in the
zone's namespace first and in the global table or the configured
section after it. So subnets from before are still found and nothing
has to be moved. Adding a subnet that the global table or the
configured section already holds fails then, since a copy in the
zone's namespace would hide its records. phpIPAM nests sections one
level deep, so the option is refused for a configured section that is
a sub-section itself.
The phpIPAM lookup by MAC, unimplemented until now, returns the
addresses recorded for the MAC in the vnet's subnets. In phpIPAM a
failed lookup stops the operation unless the answer is not found, so
a failure cannot send the zone's records to the configured section.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 199 +++++--
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 228 ++++++--
src/PVE/Network/SDN/Ipams/Plugin.pm | 17 +
src/test/run_test_ipams.pl | 571 +++++++++++++++++++++
4 files changed, 933 insertions(+), 82 deletions(-)
diff --git a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
index 84ce4382e0e7..55421025acfc 100644
--- a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
@@ -21,6 +21,8 @@ sub options {
url => { optional => 0 },
token => { optional => 0 },
fingerprint => { optional => 1 },
+ 'zone-namespaces' => { optional => 1 },
+ 'zone-namespace-prefix' => { optional => 1 },
};
}
@@ -40,18 +42,15 @@ sub netbox_api_request {
}
sub add_dhcp_range {
- my ($config, $dhcp_range, $noerr) = @_;
+ my ($config, $dhcp_range, $vrf_id, $noerr) = @_;
my $result = eval {
- netbox_api_request(
- $config,
- "POST",
- "/ipam/ip-ranges/",
- {
- start_address => $dhcp_range->{'start-address'},
- end_address => $dhcp_range->{'end-address'},
- },
- );
+ my $params = {
+ start_address => $dhcp_range->{'start-address'},
+ end_address => $dhcp_range->{'end-address'},
+ };
+ $params->{vrf} = $vrf_id if defined($vrf_id);
+ netbox_api_request($config, "POST", "/ipam/ip-ranges/", $params);
};
if ($@) {
return if $noerr;
@@ -72,26 +71,60 @@ sub del_dhcp_range {
# Plugin implementation
+my sub zone_vrf_name {
+ my ($plugin_config, $zoneid) = @_;
+
+ my $prefix = $plugin_config->{'zone-namespace-prefix'} // 'pve';
+ return "$prefix-$zoneid";
+}
+
sub add_subnet {
my ($class, $plugin_config, $subnetid, $subnet, $noerr) = @_;
my $cidr = $subnet->{cidr};
my $gateway = $subnet->{gateway};
- if (get_prefix_id($plugin_config, $cidr, $noerr)) {
+ my ($vrf_id, $taken_over);
+ if ($plugin_config->{'zone-namespaces'}) {
+ # a prefix already in the zone's VRF is used, one in the global table
+ # is not taken over, since a copy in the VRF would hide its records
+ $vrf_id = get_zone_namespace($plugin_config, $subnet->{zone}, $noerr);
+ $taken_over = defined($vrf_id)
+ && defined(get_prefix_id($plugin_config, $cidr, $vrf_id, $noerr));
+ if (defined(get_prefix_id($plugin_config, $cidr, undef, $noerr))) {
+ return if $noerr;
+ my $vrf = zone_vrf_name($plugin_config, $subnet->{zone});
+ die "prefix $cidr exists in the global table of NetBox, move it with its"
+ . " addresses and ranges into the VRF of its zone, $vrf for this one, or"
+ . " remove it first\n";
+ }
+
+ $vrf_id //= add_zone_namespace($plugin_config, $subnet->{zone}, $noerr);
+ return if !defined($vrf_id) && $noerr;
+ } elsif (defined(get_prefix_id($plugin_config, $cidr, undef, $noerr))) {
return if $noerr;
die "prefix $cidr already exists in netbox";
}
- eval { netbox_api_request($plugin_config, "POST", "/ipam/prefixes/", { prefix => $cidr }); };
- if ($@) {
- return if $noerr;
- die "error adding subnet to ipam: $@";
+ if (!$taken_over) {
+ eval {
+ my $params = { prefix => $cidr };
+ $params->{vrf} = $vrf_id if defined($vrf_id);
+ netbox_api_request($plugin_config, "POST", "/ipam/prefixes/", $params);
+ };
+ if ($@) {
+ return if $noerr;
+ die "error adding subnet to ipam: $@";
+ }
}
my $dhcp_ranges = PVE::Network::SDN::Subnets::get_dhcp_ranges($subnet);
for my $dhcp_range (@$dhcp_ranges) {
- add_dhcp_range($plugin_config, $dhcp_range, $noerr);
+ # a prefix moved into the VRF came with its ranges
+ next
+ if $taken_over
+ && defined(get_iprange_id($plugin_config, $dhcp_range, $vrf_id, $noerr));
+ add_dhcp_range($plugin_config, $dhcp_range, $vrf_id, $noerr);
}
}
@@ -122,6 +155,8 @@ sub update_subnet {
my $to_delete_ids = ();
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr, 1) or return;
+
# delete first so we don't get errors with overlapping ranges
for my $dhcp_range (@$old_dhcp_ranges) {
my $hash = $hash_range->($dhcp_range);
@@ -130,7 +165,7 @@ sub update_subnet {
next;
}
- my $internalid = get_iprange_id($plugin_config, $dhcp_range, $noerr);
+ my $internalid = get_iprange_id($plugin_config, $dhcp_range, $vrf_id, $noerr);
# definedness check, because ID could be 0
if (!defined($internalid)) {
@@ -145,7 +180,7 @@ sub update_subnet {
for my $dhcp_range (@$new_dhcp_ranges) {
my $hash = $hash_range->($dhcp_range);
- add_dhcp_range($plugin_config, $dhcp_range, $noerr)
+ add_dhcp_range($plugin_config, $dhcp_range, $vrf_id, $noerr)
if !exists($old_lookup->{$hash});
}
}
@@ -155,7 +190,7 @@ sub del_subnet {
my $cidr = $subnet->{cidr};
- my $internalid = get_prefix_id($plugin_config, $cidr, $noerr);
+ my ($internalid, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr);
# definedness check, because ID could be 0
if (!defined($internalid)) {
@@ -163,7 +198,7 @@ sub del_subnet {
return;
}
- if (!is_prefix_empty($plugin_config, $cidr, $noerr)) {
+ if (!is_prefix_empty($plugin_config, $cidr, $vrf_id, $noerr)) {
return if $noerr;
die "not deleting prefix $cidr because it still contains entries";
}
@@ -176,7 +211,7 @@ sub del_subnet {
my $dhcp_ranges = PVE::Network::SDN::Subnets::get_dhcp_ranges($subnet);
for my $dhcp_range (@$dhcp_ranges) {
- my $internalid = get_iprange_id($plugin_config, $dhcp_range, $noerr);
+ my $internalid = get_iprange_id($plugin_config, $dhcp_range, $vrf_id, $noerr);
# definedness check, because ID could be 0
if (!defined($internalid)) {
@@ -215,6 +250,8 @@ sub add_ip {
$description = "mac:$mac";
}
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr, 1) or return;
+
eval {
my $params = {
address => "$ip/$mask",
@@ -222,6 +259,7 @@ sub add_ip {
};
$params->{dns_name} = $hostname if $hostname;
+ $params->{vrf} = $vrf_id if defined($vrf_id);
netbox_api_request($plugin_config, "POST", "/ipam/ip-addresses/", $params);
};
@@ -229,7 +267,7 @@ sub add_ip {
if ($@) {
if ($is_gateway) {
die "error add subnet ip to ipam: ip $ip already exist: $@"
- if !is_ip_gateway($plugin_config, $ip, $noerr);
+ if !is_ip_gateway($plugin_config, $ip, $vrf_id, $noerr);
} elsif (!$noerr) {
die "error add subnet ip to ipam: ip already exist: $@";
}
@@ -259,7 +297,8 @@ sub update_ip {
$description = "mac:$mac";
}
- my $ip_id = get_ip_id($plugin_config, $ip, $noerr);
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr, 1) or return;
+ my $ip_id = get_ip_id($plugin_config, $ip, $vrf_id, $noerr);
# definedness check, because ID could be 0
if (!defined($ip_id)) {
@@ -274,6 +313,7 @@ sub update_ip {
};
$params->{dns_name} = $hostname if $hostname;
+ $params->{vrf} = $vrf_id if defined($vrf_id);
netbox_api_request($plugin_config, "PATCH", "/ipam/ip-addresses/$ip_id/", $params);
};
@@ -287,13 +327,7 @@ sub add_next_freeip {
my $cidr = $subnet->{cidr};
- my $internalid = get_prefix_id($plugin_config, $cidr, $noerr);
-
- # definedness check, because ID could be 0
- if (!defined($internalid)) {
- return if $noerr;
- die "could not find id for prefix $cidr";
- }
+ my ($internalid) = subnet_prefix($plugin_config, $subnet, $noerr, 1) or return;
my $description = undef;
$description = "mac:$mac" if $mac;
@@ -323,7 +357,8 @@ sub add_next_freeip {
sub add_range_next_freeip {
my ($class, $plugin_config, $subnet, $range, $data, $noerr) = @_;
- my $internalid = get_iprange_id($plugin_config, $range, $noerr);
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr, 1) or return;
+ my $internalid = get_iprange_id($plugin_config, $range, $vrf_id, $noerr);
# definedness check, because ID could be 0
if (!defined($internalid)) {
@@ -364,7 +399,8 @@ sub del_ip {
return if !$ip;
- my $ip_id = get_ip_id($plugin_config, $ip, $noerr);
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr, 1) or return;
+ my $ip_id = get_ip_id($plugin_config, $ip, $vrf_id, $noerr);
if (!defined($ip_id)) {
warn "could not find id for ip $ip";
return;
@@ -397,12 +433,16 @@ sub get_ips_from_mac {
my $ip4 = undef;
my $ip6 = undef;
- for my $cidr (sort map { $_->{cidr} } values %{ $subnets // {} }) {
+ for my $subnet (sort { $a->{cidr} cmp $b->{cidr} } values %{ $subnets // {} }) {
+ my $cidr = $subnet->{cidr};
+ my ($prefix_id, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr);
+ next if !defined($prefix_id);
+ my $vrf = vrf_filter($plugin_config, $vrf_id);
my $data = eval {
netbox_api_request(
$plugin_config,
"GET",
- "/ipam/ip-addresses/?parent=$cidr&description__ic=$mac",
+ "/ipam/ip-addresses/?parent=$cidr$vrf&description__ic=$mac",
);
};
if ($@) {
@@ -439,14 +479,72 @@ 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);
+# no VRF given means the global table with the zone namespaces on. With them
+# off the VRF is not looked at, as before
+sub vrf_filter {
+ my ($config, $vrf_id) = @_;
+
+ return "&vrf_id=$vrf_id" if defined($vrf_id);
+ return $config->{'zone-namespaces'} ? "&vrf_id=null" : "";
+}
+
+sub get_zone_namespace {
+ my ($config, $zoneid, $noerr) = @_;
+
+ my $name = zone_vrf_name($config, $zoneid);
+ my $result = eval { netbox_api_request($config, "GET", "/ipam/vrfs/?name=$name") };
+ if ($@) {
+ return if $noerr;
+ die "could not obtain ID for VRF $name: $@";
+ }
+
+ my $data = @{ $result->{results} }[0];
+ return $data->{id};
+}
+
+sub add_zone_namespace {
+ my ($config, $zoneid, $noerr) = @_;
+
+ my $name = zone_vrf_name($config, $zoneid);
+ my $result = eval { netbox_api_request($config, "POST", "/ipam/vrfs/", { name => $name }) };
+ if ($@) {
+ return if $noerr;
+ die "could not create VRF $name: $@";
+ }
+
+ return $result->{id};
+}
+
+# a prefix from before sits in the global table and keeps its addresses there,
+# so a subnet is looked for in the zone's VRF first and in the global table
+# after it. Without zone namespaces any VRF counts, as before. A subnet with
+# no prefix anywhere stops an operation that needs one, since the address and
+# range operations would act on the global table where another zone's records
+# may sit
+sub subnet_prefix {
+ my ($config, $subnet, $noerr, $needed) = @_;
+
+ my $vrf_id;
+ $vrf_id = get_zone_namespace($config, $subnet->{zone}, $noerr)
+ if $config->{'zone-namespaces'};
+ if (defined($vrf_id)) {
+ my $prefix_id = get_prefix_id($config, $subnet->{cidr}, $vrf_id, $noerr);
+ return ($prefix_id, $vrf_id) if defined($prefix_id);
+ }
+ my $prefix_id = get_prefix_id($config, $subnet->{cidr}, undef, $noerr);
+ if ($needed && !defined($prefix_id)) {
+ return if $noerr;
+ die "could not find id for prefix $subnet->{cidr}";
+ }
+ return ($prefix_id, undef);
+}
+
+sub get_prefix_id {
+ my ($config, $cidr, $vrf_id, $noerr) = @_;
- my $result = eval { netbox_api_request($config, "GET", "/ipam/prefixes/?q=$ip") };
+ my $vrf = vrf_filter($config, $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 +555,14 @@ sub get_prefix_id {
}
sub get_iprange_id {
- my ($config, $range, $noerr) = @_;
+ my ($config, $range, $vrf_id, $noerr) = @_;
+ my $vrf = vrf_filter($config, $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 +576,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($config, $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 +590,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($config, $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 +604,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($config, $vrf_id);
+ my $result =
+ eval { netbox_api_request($config, "GET", "/ipam/ip-addresses/?parent=$cidr$vrf") };
if ($@) {
return if $noerr;
die "could not query children for prefix $cidr: $@";
diff --git a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
index e969c3dcdf4f..1dc227a998c4 100644
--- a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
@@ -2,6 +2,9 @@ package PVE::Network::SDN::Ipams::PhpIpamPlugin;
use strict;
use warnings;
+use Math::BigInt;
+use Net::IP;
+
use PVE::INotify;
use PVE::Cluster;
use PVE::Tools;
@@ -35,11 +38,23 @@ sub options {
token => { optional => 0 },
section => { optional => 0 },
fingerprint => { optional => 1 },
+ 'zone-namespaces' => { optional => 1 },
+ 'zone-namespace-prefix' => { optional => 1 },
};
}
# Plugin implementation
+# a zone's section is a sub-section of the configured one, so SDN writes only
+# under the section it was given. Section names are global in phpIPAM, so
+# clusters sharing one set a prefix of their own
+my sub zone_section_name {
+ my ($plugin_config, $zoneid) = @_;
+
+ my $prefix = $plugin_config->{'zone-namespace-prefix'} // 'pve';
+ return "$prefix-$zoneid";
+}
+
sub add_subnet {
my ($class, $plugin_config, $subnetid, $subnet, $noerr) = @_;
@@ -49,15 +64,41 @@ 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];
- #search subnet
- my $internalid = eval { get_prefix_id($url, $cidr, $headers) };
+ # with zone namespaces a subnet being added is new to its zone, so it goes
+ # into the zone's section, and a subnet from before in the configured
+ # section is not taken over, since a copy in the zone's section would hide
+ # its records. Without them any section counts, as before
+ my $section = $plugin_config->{section};
+ my $internalid;
+ eval {
+ if ($plugin_config->{'zone-namespaces'}) {
+ $section = get_zone_namespace($plugin_config, $subnet->{zone}, $headers);
+ $internalid = get_prefix_id($url, $cidr, $headers, $section) if defined($section);
+ if (
+ !$internalid
+ && defined(get_prefix_id($url, $cidr, $headers, $plugin_config->{section}))
+ ) {
+ my $name = zone_section_name($plugin_config, $subnet->{zone});
+ die "subnet $cidr exists in section $plugin_config->{section} of phpIPAM,"
+ . " move it into the sub-section of its zone under that section, $name"
+ . " for this one, or remove it first\n";
+ }
+ } else {
+ $internalid = get_prefix_id($url, $cidr, $headers);
+ }
+ };
+ if (my $err = $@) {
+ return if $noerr;
+ die $err;
+ }
#create subnet
if (!$internalid) {
+ $section //= add_zone_namespace($plugin_config, $subnet->{zone}, $headers, $noerr);
+ return if !defined($section);
my $params = {
subnet => $network,
mask => $mask,
@@ -87,7 +128,7 @@ sub del_subnet {
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
- my $internalid = get_prefix_id($url, $cidr, $headers);
+ my $internalid = subnet_id($plugin_config, $subnet, $headers);
return if !$internalid;
return; #fixme: check that prefix is empty exluding gateway, before delete
@@ -120,7 +161,7 @@ sub add_ip {
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
- my $internalid = get_prefix_id($url, $cidr, $headers);
+ my $internalid = subnet_id($plugin_config, $subnet, $headers);
my $params = {
ip => $ip,
@@ -140,7 +181,7 @@ sub add_ip {
if ($@) {
if ($is_gateway) {
die "error add subnet ip to ipam: ip $ip already exist: $@"
- if !is_ip_gateway($url, $ip, $headers) && !$noerr;
+ if !is_ip_gateway($url, $ip, $headers, $internalid) && !$noerr;
} else {
die "error add subnet ip to ipam: ip $ip already exist: $@" if !$noerr;
}
@@ -167,7 +208,7 @@ sub update_ip {
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
- my $ip_id = get_ip_id($url, $ip, $headers);
+ my $ip_id = get_ip_id($url, $ip, $headers, subnet_id($plugin_config, $subnet, $headers));
die "can't find ip address in ipam\n" if !$ip_id;
my $params = {
@@ -198,7 +239,7 @@ sub add_next_freeip {
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
- my $internalid = get_prefix_id($url, $cidr, $headers);
+ my $internalid = subnet_id($plugin_config, $subnet, $headers);
my $params = {
hostname => $hostname,
@@ -248,7 +289,7 @@ sub del_ip {
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
- my $ip_id = get_ip_id($url, $ip, $headers);
+ my $ip_id = get_ip_id($url, $ip, $headers, subnet_id($plugin_config, $subnet, $headers));
return if !$ip_id;
if ($mac) {
my $record = eval {
@@ -279,6 +320,24 @@ sub del_ip {
return 1;
}
+# phpIPAM answers a lookup that finds nothing with a 404
+my sub not_found {
+ my ($err) = @_;
+ return $err =~ m/^Invalid response from server: 404 /;
+}
+
+# an address comes back in its text form or as the decimal phpIPAM stores
+my sub address_of_hit {
+ my ($hit, $family) = @_;
+
+ my $ip = $hit->{ip} // $hit->{ip_addr};
+ return undef if !defined($ip);
+ return $ip if $ip !~ m/^\d+$/;
+ my $bin = Net::IP::ip_inttobin(Math::BigInt->new($ip), $family) // return undef;
+ $ip = Net::IP::ip_bintoip($bin, $family);
+ return $family == 6 ? Net::IP::ip_compress_address($ip, 6) : $ip;
+}
+
sub get_ips_from_mac {
my ($class, $plugin_config, $mac, $zoneid, $subnets) = @_;
@@ -290,7 +349,15 @@ sub get_ips_from_mac {
my $ip4 = undef;
my $ip6 = undef;
- my $ips = eval {
+ # a hit names its subnet, and only the vnet's subnets count
+ my %wanted;
+ for my $subnet (values %{ $subnets // {} }) {
+ my $id = subnet_id($plugin_config, $subnet, $headers) // next;
+ $wanted{$id} = Net::IP::ip_is_ipv4($subnet->{network}) ? 4 : 6;
+ }
+ return if !%wanted;
+
+ my $result = eval {
PVE::Network::SDN::api_request(
"GET",
"$url/addresses/search_mac/$mac",
@@ -299,19 +366,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);
@@ -326,7 +393,7 @@ sub verify_api {
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
- eval {
+ my $result = eval {
PVE::Network::SDN::api_request(
"GET", "$url/sections/$sectionid", $headers, undef, $fingerprint,
);
@@ -334,37 +401,130 @@ sub verify_api {
if ($@) {
die "Can't connect to phpipam api: $@";
}
+ return $result->{data};
}
sub on_update_hook {
my ($class, $plugin_config) = @_;
- PVE::Network::SDN::Ipams::PhpIpamPlugin::verify_api($class, $plugin_config);
+ my $section = PVE::Network::SDN::Ipams::PhpIpamPlugin::verify_api($class, $plugin_config);
+ # phpIPAM nests sections one level deep, so the zone namespaces need a
+ # top-level section to go under
+ die "section $plugin_config->{section} of phpIPAM is a sub-section, the zone"
+ . " namespaces need a top-level one, configure section $section->{masterSection}"
+ . " and move the subnets into it or into their zones sub-sections, or turn"
+ . " them off\n"
+ if $plugin_config->{'zone-namespaces'} && ($section->{masterSection} // 0);
}
#helpers
+sub get_zone_namespace {
+ my ($plugin_config, $zoneid, $headers) = @_;
+
+ my $name = zone_section_name($plugin_config, $zoneid);
+ my $result = eval {
+ PVE::Network::SDN::api_request(
+ "GET",
+ "$plugin_config->{url}/sections/$name/",
+ $headers,
+ undef,
+ $plugin_config->{fingerprint},
+ );
+ };
+ if (my $err = $@) {
+ # only a missing section means the zone has none yet, any other
+ # failure would send the zone's records to the configured section
+ return if not_found($err);
+ die "could not look up section $name: $err";
+ }
+ # section names are global in phpIPAM, so one of the zone's name under
+ # another parent is another cluster's, and the zone has none of its own
+ my $section = $result->{data};
+ return if ($section->{masterSection} // 0) != $plugin_config->{section};
+ return $section->{id};
+}
+
+sub add_zone_namespace {
+ my ($plugin_config, $zoneid, $headers, $noerr) = @_;
+
+ my $name = zone_section_name($plugin_config, $zoneid);
+ my $result = eval {
+ PVE::Network::SDN::api_request(
+ "POST",
+ "$plugin_config->{url}/sections/",
+ $headers,
+ { name => $name, masterSection => $plugin_config->{section} },
+ $plugin_config->{fingerprint},
+ );
+ };
+ if (my $err = $@) {
+ return if $noerr;
+ my $taken = eval {
+ PVE::Network::SDN::api_request(
+ "GET",
+ "$plugin_config->{url}/sections/$name/",
+ $headers,
+ undef,
+ $plugin_config->{fingerprint},
+ );
+ };
+ die "section $name exists under another section in phpIPAM, move it under"
+ . " section $plugin_config->{section} or remove it\n"
+ if $taken;
+ die "could not create section $name: $err";
+ }
+ return $result->{id};
+}
+
+# a subnet from before sits in the configured section, so with zone namespaces
+# a subnet is looked for in the zone's section first and in the configured one
+# after it. Without them any section counts, as before
+sub subnet_id {
+ my ($plugin_config, $subnet, $headers) = @_;
+
+ my $url = $plugin_config->{url};
+ return get_prefix_id($url, $subnet->{cidr}, $headers)
+ if !$plugin_config->{'zone-namespaces'};
+ my $section = get_zone_namespace($plugin_config, $subnet->{zone}, $headers);
+ my @sections = grep { defined } $section, $plugin_config->{section};
+ return get_prefix_id($url, $subnet->{cidr}, $headers, @sections);
+}
+
+# the subnet of that range in the first of the sections that holds it, or in
+# any section when none is given
sub get_prefix_id {
- my ($url, $cidr, $headers) = @_;
+ my ($url, $cidr, $headers, @sections) = @_;
- 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 $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 @subnets = @{ $result->{data} // [] };
+ my ($data) = @subnets;
+ for my $section (@sections) {
+ ($data) = grep { ($_->{sectionId} // 0) == $section } @subnets;
+ last if $data;
+ }
+ return $data->{id};
}
sub get_ip_id {
- my ($url, $ip, $headers) = @_;
+ my ($url, $ip, $headers, $subnet_id) = @_;
my $result = PVE::Network::SDN::api_request("GET", "$url/addresses/search/$ip", $headers);
- my $data = @{ $result->{data} }[0];
+ my ($data) =
+ grep { defined($subnet_id) && $_->{subnetId} == $subnet_id } @{ $result->{data} // [] };
my $ip_id = $data->{id};
return $ip_id;
}
sub is_ip_gateway {
- my ($url, $ip, $headers) = @_;
+ my ($url, $ip, $headers, $subnet_id) = @_;
my $result = PVE::Network::SDN::api_request("GET", "$url/addresses/search/$ip", $headers);
- my $data = @{ $result->{data} }[0];
+ my ($data) =
+ grep { defined($subnet_id) && $_->{subnetId} == $subnet_id } @{ $result->{data} // [] };
my $is_gateway = $data->{is_gateway};
return $is_gateway;
}
diff --git a/src/PVE/Network/SDN/Ipams/Plugin.pm b/src/PVE/Network/SDN/Ipams/Plugin.pm
index 6e23d5434d4b..4998957f1485 100644
--- a/src/PVE/Network/SDN/Ipams/Plugin.pm
+++ b/src/PVE/Network/SDN/Ipams/Plugin.pm
@@ -45,6 +45,23 @@ my $defaultData = {
},
),
fingerprint => get_standard_option('fingerprint-sha256', { optional => 1 }),
+ 'zone-namespaces' => {
+ type => 'boolean',
+ description => "Keep the subnets of each zone in a namespace of its own, a VRF"
+ . " in NetBox and a sub-section in phpIPAM, so two zones can use the same"
+ . " range. Turn it on once every node runs a version that knows them.",
+ optional => 1,
+ default => 0,
+ },
+ 'zone-namespace-prefix' => {
+ type => 'string',
+ description => "The names of the zone namespaces start with this, so clusters"
+ . " sharing an IPAM set one of their own.",
+ pattern => '[a-zA-Z0-9][a-zA-Z0-9_-]*',
+ maxLength => 32,
+ optional => 1,
+ default => 'pve',
+ },
},
};
diff --git a/src/test/run_test_ipams.pl b/src/test/run_test_ipams.pl
index f07b4bb4b486..3b318ceb7df3 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,226 @@ foreach my $path (@plugins) {
}
+# the phpipam MAC lookup, with the addresses as the decimals phpipam hands out
+{
+ my $plugin = 'PVE::Network::SDN::Ipams::PhpIpamPlugin';
+ my $mocked = Test::MockModule->new($plugin);
+ $mocked->mock(subnet_id => sub { my (undef, $subnet) = @_; return $subnet->{id}; });
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ return {
+ data => [
+ { ip => 171048980, subnetId => 7 },
+ { ip => '10.60.0.20', subnetId => 8 },
+ { ip => '55832768119825001489588427028103692289', subnetId => 9 },
+ ],
+ };
+ },
+ );
+ my $subnets = {
+ 7 => { id => 7, cidr => '10.50.0.0/24', network => '10.50.0.0' },
+ 9 => { id => 9, cidr => '2a01::/64', network => '2a01::' },
+ };
+ my @ips = $plugin->get_ips_from_mac(
+ { url => 'u', token => 't' }, 'da:65:8f:18:9b:6f', 'z', $subnets,
+ );
+ is_deeply(
+ \@ips,
+ ['10.50.0.20', '2a01::1'],
+ 'phpipam lookup converts decimal addresses by the subnet family',
+ );
+
+ # a MAC without addresses is a 404, any other failure fails the lookup
+ $sdn_module->mock(api_request => sub { die "Invalid response from server: 404 Not Found\n" });
+ is_deeply(
+ [
+ $plugin->get_ips_from_mac(
+ { url => 'u', token => 't' },
+ 'da:65:8f:18:9b:6f',
+ 'z',
+ $subnets,
+ ),
+ ],
+ [],
+ 'phpipam lookup answers nothing for a MAC without addresses',
+ );
+ $sdn_module->mock(
+ api_request => sub { die "Invalid response from server: 503 Service Unavailable\n" });
+ eval {
+ $plugin->get_ips_from_mac(
+ { url => 'u', token => 't' },
+ 'da:65:8f:18:9b:6f',
+ 'z',
+ $subnets,
+ );
+ };
+ like($@, qr/could not look up the addresses/, 'and dies on any other failure');
+}
+
+# a range the zone's namespace does not hold is refused while a subnet from
+# before holds it in the global table or the configured section, since a copy
+# would hide its records, and the error names the way out
+{
+ my $subnet = {
+ zone => 'z',
+ cidr => '10.0.0.0/24',
+ network => '10.0.0.0',
+ mask => '24',
+ };
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return {};
+ },
+ );
+
+ my $netbox = Test::MockModule->new('PVE::Network::SDN::Ipams::NetboxPlugin');
+ $netbox->mock(
+ get_zone_namespace => sub { return 2; },
+ get_prefix_id =>
+ sub { my (undef, undef, $vrf_id) = @_; return defined($vrf_id) ? undef : 5; },
+ );
+ eval {
+ PVE::Network::SDN::Ipams::NetboxPlugin->add_subnet(
+ { url => 'u', token => 't', 'zone-namespaces' => 1 },
+ 'z-10.0.0.0-24',
+ $subnet,
+ );
+ };
+ like(
+ $@,
+ qr/move it .* into the VRF of its zone, pve-z for this one/,
+ 'netbox refuses a range the global table holds',
+ );
+ is(scalar(@requests), 0, 'and creates nothing');
+
+ @requests = ();
+ my $phpipam = Test::MockModule->new('PVE::Network::SDN::Ipams::PhpIpamPlugin');
+ $phpipam->mock(
+ get_zone_namespace => sub { return 2; },
+ get_prefix_id => sub {
+ my (undef, undef, undef, $section) = @_;
+ return ($section // '') eq '2' ? undef : 5;
+ },
+ );
+ eval {
+ PVE::Network::SDN::Ipams::PhpIpamPlugin->add_subnet(
+ { url => 'u', token => 't', section => 1, 'zone-namespaces' => 1 },
+ 'z-10.0.0.0-24',
+ $subnet,
+ );
+ };
+ like(
+ $@,
+ qr/move it into the sub-section of its zone .* pve-z for this one/,
+ 'phpipam refuses a range the configured section holds',
+ );
+ is(scalar(@requests), 0, 'and creates nothing');
+}
+
+# a prefix already in the zone's VRF, moved there as the refusal advises, is
+# used by the subnet added next, and only the ranges it came without are created
+{
+ my $subnet = {
+ zone => 'z',
+ cidr => '10.0.0.0/24',
+ network => '10.0.0.0',
+ mask => '24',
+ 'dhcp-range' => [
+ 'start-address=10.0.0.10,end-address=10.0.0.20',
+ 'start-address=10.0.0.30,end-address=10.0.0.40',
+ ],
+ };
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return { results => [{ id => 7 }] }
+ if $url =~ m|/ipam/ip-ranges/\?start_address=10\.0\.0\.10&|;
+ return { results => [] };
+ },
+ );
+ my $netbox = Test::MockModule->new('PVE::Network::SDN::Ipams::NetboxPlugin');
+ $netbox->mock(
+ get_zone_namespace => sub { return 2; },
+ get_prefix_id =>
+ sub { my (undef, undef, $vrf_id) = @_; return defined($vrf_id) ? 5 : undef; },
+ );
+ eval {
+ PVE::Network::SDN::Ipams::NetboxPlugin->add_subnet(
+ { url => 'u', token => 't', 'zone-namespaces' => 1 },
+ 'z-10.0.0.0-24',
+ $subnet,
+ );
+ };
+ is($@, '', 'netbox uses a prefix already in the zone\'s VRF');
+ is_deeply(
+ [grep { $_->[0] eq 'POST' } @requests],
+ [[
+ 'POST',
+ 'u/ipam/ip-ranges/',
+ { start_address => '10.0.0.30', end_address => '10.0.0.40', vrf => 2 },
+ ]],
+ 'creates only the range the VRF does not hold and no prefix',
+ );
+}
+
+# the phpipam gateway check answers for the subnet it is asked about, also
+# when the address sits in another zone's section as well
+{
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ return {
+ data => [
+ { ip => '10.0.0.1', subnetId => 7, is_gateway => 1 },
+ { ip => '10.0.0.1', subnetId => 8, is_gateway => 0 },
+ ],
+ };
+ },
+ );
+ my $is_gateway = \&PVE::Network::SDN::Ipams::PhpIpamPlugin::is_ip_gateway;
+ ok($is_gateway->('u', '10.0.0.1', [], 7), 'phpipam sees the gateway in its subnet');
+ ok(!$is_gateway->('u', '10.0.0.1', [], 8), 'and not in another subnet holding the address');
+ ok(!$is_gateway->('u', '10.0.0.1', [], undef), 'and not without a subnet to ask about');
+}
+
+# an address goes into the VRF its subnet's prefix sits in, and into the
+# global table without the zone namespaces
+{
+ my $subnet = { zone => 'z', cidr => '10.0.0.0/24' };
+ my @posts;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @posts, $data if $method eq 'POST';
+ return {};
+ },
+ );
+ my $netbox = Test::MockModule->new('PVE::Network::SDN::Ipams::NetboxPlugin');
+ $netbox->mock(
+ get_zone_namespace => sub { return 2; },
+ get_prefix_id => sub { return 1; },
+ );
+ my $config = { url => 'u', token => 't', 'zone-namespaces' => 1 };
+ PVE::Network::SDN::Ipams::NetboxPlugin->add_ip(
+ $config, 'z-10.0.0.0-24', $subnet, '10.0.0.50', 'h', 'da:65:8f:18:9b:6f', 'd', 0,
+ );
+ is($posts[-1]->{vrf}, 2, 'netbox records an address in the VRF of its prefix');
+ delete $config->{'zone-namespaces'};
+ PVE::Network::SDN::Ipams::NetboxPlugin->add_ip(
+ $config, 'z-10.0.0.0-24', $subnet, '10.0.0.50', 'h', 'da:65:8f:18:9b:6f', 'd', 0,
+ );
+ ok(!exists($posts[-1]->{vrf}), 'and in the global table without the zone namespaces');
+}
+
# an address released for a MAC stays when the external IPAM records it
# for another MAC
{
@@ -268,5 +491,353 @@ foreach my $path (@plugins) {
}
}
+# an address operation on a subnet whose prefix is in neither the zone's VRF
+# nor the global table stops instead of acting on the global table
+{
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url) = @_;
+ push @requests, [$method, $url];
+ return { results => [] };
+ },
+ );
+ my $netbox = Test::MockModule->new('PVE::Network::SDN::Ipams::NetboxPlugin');
+ $netbox->mock(
+ get_zone_namespace => sub { return undef; },
+ get_prefix_id => sub { return undef; },
+ );
+ my $config = { url => 'u', token => 't' };
+ my $subnet = { zone => 'z', cidr => '10.0.0.0/24' };
+ my $plugin = 'PVE::Network::SDN::Ipams::NetboxPlugin';
+ my %ops = (
+ add_ip => sub {
+ $plugin->add_ip(
+ $config,
+ 'z-10.0.0.0-24',
+ $subnet,
+ '10.0.0.5',
+ 'h',
+ 'da:65:8f:18:9b:6f',
+ 'd',
+ 0,
+ );
+ },
+ update_ip => sub {
+ $plugin->update_ip(
+ $config,
+ 'z-10.0.0.0-24',
+ $subnet,
+ '10.0.0.5',
+ 'h',
+ 'da:65:8f:18:9b:6f',
+ 'd',
+ 0,
+ );
+ },
+ del_ip => sub { $plugin->del_ip($config, 'z-10.0.0.0-24', $subnet, '10.0.0.5') },
+ );
+ for my $op (sort keys %ops) {
+ @requests = ();
+ eval { $ops{$op}->() };
+ like($@, qr/could not find id for prefix 10.0.0.0\/24/,
+ "netbox $op stops without a prefix");
+ is(scalar(@requests), 0, "and sends nothing");
+ }
+}
+
+# without the zone namespaces a subnet goes where it went before, the global
+# table or the configured section, and no namespace is looked for or created
+{
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return $method eq 'GET' ? { results => [], data => [] } : { id => 5 };
+ },
+ );
+ my $subnet = { zone => 'z', cidr => '10.0.0.0/24', network => '10.0.0.0', mask => '24' };
+ PVE::Network::SDN::Ipams::NetboxPlugin->add_subnet(
+ { url => 'u', token => 't' },
+ 'z-10.0.0.0-24',
+ $subnet,
+ );
+ is_deeply(
+ [grep { $_->[0] eq 'POST' } @requests],
+ [['POST', 'u/ipam/prefixes/', { prefix => '10.0.0.0/24' }]],
+ 'netbox puts a subnet into the global table without the zone namespaces',
+ );
+ ok(!grep({ $_->[1] =~ m|/vrfs/| } @requests), 'and never asks for a VRF');
+
+ @requests = ();
+ PVE::Network::SDN::Ipams::PhpIpamPlugin->add_subnet(
+ { url => 'u', token => 't', section => 1 },
+ 'z-10.0.0.0-24',
+ $subnet,
+ );
+ is_deeply(
+ [grep { $_->[0] eq 'POST' } @requests],
+ [['POST', 'u/subnets/', { subnet => '10.0.0.0', mask => '24', sectionId => 1 }]],
+ 'phpipam puts a subnet into the configured section without the zone namespaces',
+ );
+ ok(!grep({ $_->[1] =~ m|/sections/| } @requests), 'and never asks for a section');
+}
+
+# a zone's VRF is found and created under its prefixed name
+{
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return {
+ results => [{ id => 4 }],
+ id => 4,
+ data => { id => 4, masterSection => 1 },
+ };
+ },
+ );
+ my $config = { url => 'u', token => 't' };
+ PVE::Network::SDN::Ipams::NetboxPlugin::get_zone_namespace($config, 'z');
+ PVE::Network::SDN::Ipams::NetboxPlugin::add_zone_namespace($config, 'z');
+ is_deeply(
+ \@requests,
+ [
+ ['GET', 'u/ipam/vrfs/?name=pve-z', undef],
+ ['POST', 'u/ipam/vrfs/', { name => 'pve-z' }],
+ ],
+ 'netbox finds and creates the zone\'s VRF under the name pve-<zone>',
+ );
+
+ # a prefix set on the entry keeps clusters sharing the IPAM apart
+ @requests = ();
+ PVE::Network::SDN::Ipams::NetboxPlugin::get_zone_namespace(
+ { %$config, 'zone-namespace-prefix' => 'lab' }, 'z',
+ );
+ is($requests[0]->[1], 'u/ipam/vrfs/?name=lab-z', 'and under the configured prefix');
+ is(
+ PVE::Network::SDN::Ipams::PhpIpamPlugin::get_zone_namespace(
+ { url => 'u', token => 't', section => 1, 'zone-namespace-prefix' => 'lab' },
+ 'z',
+ [],
+ ),
+ 4,
+ 'phpipam looks the zone\'s section up under the configured prefix as well',
+ );
+ like($requests[-1]->[1], qr{/sections/lab-z/$}, 'by its name');
+}
+
+# a subnet from before is found in the global table while the zone's VRF holds
+# no prefix for its range, and in the VRF once it was moved there. Without the
+# zone namespaces any VRF counts
+{
+ 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 = $2 // 'any';
+ return { results => [{ id => 5 }] } if $vrf eq '2' && $moved;
+ return { results => [{ id => 5 }] } if $vrf eq 'null' && !$moved;
+ return { results => [{ id => 5 }] } if $vrf eq 'any';
+ return { results => [] };
+ }
+ die "unexpected request $method $url\n";
+ },
+ );
+ my $config = { url => 'u', token => 't', 'zone-namespaces' => 1 };
+ 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',
+ );
+ delete $config->{'zone-namespaces'};
+ is_deeply(
+ [PVE::Network::SDN::Ipams::NetboxPlugin::subnet_prefix($config, $subnet)],
+ [5, undef],
+ 'and in any VRF without the zone namespaces',
+ );
+}
+
+# the same for phpipam, which is asked once for both sections
+{
+ my ($moved, $both);
+ 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 => 1 }, { id => 6, sectionId => 2 }] }
+ if $both;
+ return { data => [{ id => 5, sectionId => $moved ? 2 : 1 }] };
+ }
+ die "unexpected request $method $url\n";
+ },
+ );
+ my $config = { url => 'u', token => 't', section => 1, 'zone-namespaces' => 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), 1, 'with one request');
+ $moved = 1;
+ is(
+ PVE::Network::SDN::Ipams::PhpIpamPlugin::subnet_id($config, $subnet, []),
+ 5,
+ 'and in the zone\'s section once the subnet was moved there',
+ );
+ ($moved, $both) = (0, 1);
+ is(
+ PVE::Network::SDN::Ipams::PhpIpamPlugin::subnet_id($config, $subnet, []),
+ 6,
+ 'and the zone\'s section wins when both hold the range',
+ );
+ $both = 0;
+ $moved = 1;
+ delete $config->{'zone-namespaces'};
+ is(
+ PVE::Network::SDN::Ipams::PhpIpamPlugin::subnet_id($config, $subnet, []),
+ 5,
+ 'and in any section without the zone namespaces',
+ );
+}
+
+# a zone without a section is told apart from a section lookup that failed
+{
+ my $error;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(api_request => sub { die $error; });
+ my $lookup = \&PVE::Network::SDN::Ipams::PhpIpamPlugin::get_zone_namespace;
+ my $config = { url => 'u', token => 't', section => 1 };
+
+ $error = "Invalid response from server: 404 Not Found\n";
+ is(scalar($lookup->($config, 'z', [])), undef, 'phpipam reports a missing section as none');
+ $error = "Invalid response from server: 503 Service Unavailable\n";
+ eval { $lookup->($config, 'z', []) };
+ like($@, qr/could not look up section pve-z/, 'and dies on any other failure');
+
+ # a section of the zone's name elsewhere in phpIPAM is another cluster's
+ $sdn_module->mock(api_request => sub { return { data => { id => 7, masterSection => 2 } }; });
+ is(
+ scalar($lookup->({ url => 'u', token => 't', section => 1 }, 'z', [])),
+ undef,
+ 'a section outside the configured one is not the zone\'s',
+ );
+ # and creating the zone's own then fails with the reason
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method) = @_;
+ die "Invalid response from server: 500 Internal Server Error\n"
+ if $method eq 'POST';
+ return { data => { id => 7, masterSection => 2 } };
+ },
+ );
+ eval {
+ PVE::Network::SDN::Ipams::PhpIpamPlugin::add_zone_namespace(
+ { url => 'u', token => 't', section => 1 },
+ 'z',
+ [],
+ );
+ };
+ like(
+ $@,
+ qr/exists under another section/,
+ 'creating a section another cluster has fails with the reason',
+ );
+ $sdn_module->mock(api_request => sub { return { data => { id => 7, masterSection => 1 } }; });
+ is(
+ $lookup->({ url => 'u', token => 't', section => 1 }, 'z', []),
+ 7,
+ 'and one under it is the zone\'s',
+ );
+}
+
+# a zone's section is created under the configured section
+{
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return { id => 5 };
+ },
+ );
+ my $phpipam = Test::MockModule->new('PVE::Network::SDN::Ipams::PhpIpamPlugin');
+ $phpipam->mock(
+ get_zone_namespace => sub { return undef; },
+ get_prefix_id => sub { return undef; },
+ );
+ PVE::Network::SDN::Ipams::PhpIpamPlugin->add_subnet(
+ { url => 'u', token => 't', section => 1, 'zone-namespaces' => 1 },
+ 'z-10.0.0.0/24',
+ { zone => 'z', cidr => '10.0.0.0/24', network => '10.0.0.0', mask => '24' },
+ 1,
+ );
+ is_deeply(
+ [map { [$_->[0], $_->[1], $_->[2]] } @requests],
+ [
+ ['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',
+ );
+
+ # the zone namespaces need a top-level section, so the option is refused
+ # on a configured section that is a sub-section itself
+ $sdn_module->mock(api_request => sub { return { data => { id => 1, masterSection => 3 } }; });
+ my $config = { url => 'u', token => 't', section => 1, 'zone-namespaces' => 1 };
+ eval { PVE::Network::SDN::Ipams::PhpIpamPlugin->on_update_hook($config) };
+ like($@, qr/is a sub-section/, 'phpipam refuses the zone namespaces on a sub-section');
+ delete $config->{'zone-namespaces'};
+ eval { PVE::Network::SDN::Ipams::PhpIpamPlugin->on_update_hook($config) };
+ is($@, '', 'and takes the section without them');
+ $sdn_module->mock(api_request => sub { return { data => { id => 1, masterSection => 0 } }; });
+ $config->{'zone-namespaces'} = 1;
+ eval { PVE::Network::SDN::Ipams::PhpIpamPlugin->on_update_hook($config) };
+ is($@, '', 'and a top-level section with them');
+
+ # a lookup that fails stops the creation, a copy would be made otherwise
+ @requests = ();
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ die "Invalid response from server: 503 Service Unavailable\n"
+ if $url =~ m|/subnets/cidr/|;
+ return { data => { id => 1, masterSection => 3 } };
+ },
+ );
+ $phpipam->unmock('get_prefix_id');
+ eval {
+ PVE::Network::SDN::Ipams::PhpIpamPlugin->add_subnet(
+ { url => 'u', token => 't', section => 1 },
+ 'z-10.0.0.0/24',
+ { zone => 'z', cidr => '10.0.0.0/24', network => '10.0.0.0', mask => '24' },
+ );
+ };
+ like($@, qr/could not look up subnet/, 'a failed subnet lookup stops the creation');
+ is(scalar(grep { $_->[0] eq 'POST' } @requests), 0, 'and nothing is created');
+}
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH qemu-server v6 10/13] network: sdn: push the DHCP mappings of a guest when it resumes
2026-11-05 9:35 [PATCH docs/manager/network/qemu-server v6 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (8 preceding siblings ...)
2026-11-05 9:35 ` [PATCH pve-network v6 09/13] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM Hannes Laimer
@ 2026-11-05 9:35 ` Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-manager v6 11/13] ui: sdn: dhcp tree: do not merge the same subnet in different zones Hannes Laimer
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Hannes Laimer @ 2026-11-05 9:35 UTC (permalink / raw)
To: pve-devel
A mapping edit and a migration of the guest exclude each other, but an
edit through a node not yet upgraded takes no lock and writes no
reservation, so it can land while the guest is migrated. Every resume
now pushes the mapping of each NIC of the guest on the local node
before the guest continues, and the last step of a live migration
resumes the target after the config moved, so the target serves what
the cache vouches for. The push asks the IPAM only when the cache
cannot vouch for the address. The push does not wait long for a push
holding the reservation file. A failed request fails the task after
the guest continued, naming the IPAM's error. An IPAM that does not
answer holds the resume up to the request's timeout first, and a slow
one for every request the lookup makes. That beats serving the guest a
stale address.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/QemuServer/Network.pm | 18 ++++++++++++++++++
src/PVE/QemuServer/RunState.pm | 8 ++++++++
2 files changed, 26 insertions(+)
diff --git a/src/PVE/QemuServer/Network.pm b/src/PVE/QemuServer/Network.pm
index b4893c015646..7ae448ea1369 100644
--- a/src/PVE/QemuServer/Network.pm
+++ b/src/PVE/QemuServer/Network.pm
@@ -272,6 +272,24 @@ sub add_random_macs {
}
}
+# a resume pushes the current mapping of each NIC. The target of a live
+# migration resumes after the config moved and serves the current addresses
+# from then on. The push does not wait long for a push holding the
+# reservation file
+sub add_nets_dhcp_mapping {
+ my ($conf) = @_;
+
+ my @errors;
+ 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};
+ eval { PVE::Network::SDN::Dhcp::update_mapping($net->{bridge}, $net->{macaddr}, 1) };
+ push @errors, $@ if $@;
+ }
+ die join('', @errors) if @errors;
+}
+
sub add_nets_bridge_fdb {
my ($conf, $vmid) = @_;
diff --git a/src/PVE/QemuServer/RunState.pm b/src/PVE/QemuServer/RunState.pm
index 889ddd28287b..cf1e1873c39d 100644
--- a/src/PVE/QemuServer/RunState.pm
+++ b/src/PVE/QemuServer/RunState.pm
@@ -181,8 +181,16 @@ sub vm_resume($vmid, $skiplock = undef, $nocheck = undef) {
PVE::QemuServer::Network::add_nets_bridge_fdb($conf, $vmid)
if $resume_cmd eq 'cont';
+ # a failed push is raised after the guest continued, so the task
+ # says what went wrong and the guest does not stay paused for it
+ my $dhcp_err;
+ if ($resume_cmd eq 'cont') {
+ eval { PVE::QemuServer::Network::add_nets_dhcp_mapping($conf) };
+ $dhcp_err = $@;
+ }
mon_cmd($vmid, $resume_cmd);
+ die $dhcp_err if $dhcp_err;
},
);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH pve-manager v6 11/13] ui: sdn: dhcp tree: do not merge the same subnet in different zones
2026-11-05 9:35 [PATCH docs/manager/network/qemu-server v6 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (9 preceding siblings ...)
2026-11-05 9:35 ` [PATCH qemu-server v6 10/13] network: sdn: push the DHCP mappings of a guest when it resumes Hannes Laimer
@ 2026-11-05 9:35 ` Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-manager v6 12/13] ui: sdn: ipam: add the zone namespace options Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-docs v6 13/13] sdn: ipam: document zone namespaces and the mapping edit Hannes Laimer
12 siblings, 0 replies; 14+ messages in thread
From: Hannes Laimer @ 2026-11-05 9:35 UTC (permalink / raw)
To: pve-devel
Two zones can each have a subnet with the same CIDR. The tree keyed
its subnet nodes by the CIDR alone, so it listed the mappings of both
zones under the subnet of one of them. The vnet of the other zone did
not show the subnet.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/manager6/tree/DhcpTree.js | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/www/manager6/tree/DhcpTree.js b/www/manager6/tree/DhcpTree.js
index 40eb40bf9408..23f91fe51996 100644
--- a/www/manager6/tree/DhcpTree.js
+++ b/www/manager6/tree/DhcpTree.js
@@ -60,7 +60,9 @@ Ext.define('PVE.sdn.DhcpTree', {
zones[element.zone].children.push(vnet);
}
- if (!(element.subnet in subnets)) {
+ // the same subnet can exist in several zones
+ let subnetKey = `${element.zone}/${element.subnet}`;
+ if (!(subnetKey in subnets)) {
let subnet = {
name: element.subnet,
zone: element.zone,
@@ -71,14 +73,14 @@ Ext.define('PVE.sdn.DhcpTree', {
children: [],
};
- subnets[element.subnet] = subnet;
+ subnets[subnetKey] = subnet;
vnets[element.vnet].children.push(subnet);
}
element.name = element.vmid; // for sorting
element.type = 'mapping';
element.iconCls = 'x-tree-icon-none';
- subnets[element.subnet].children.push(element);
+ subnets[subnetKey].children.push(element);
});
me.getView().setRootNode(root);
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH pve-manager v6 12/13] ui: sdn: ipam: add the zone namespace options
2026-11-05 9:35 [PATCH docs/manager/network/qemu-server v6 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (10 preceding siblings ...)
2026-11-05 9:35 ` [PATCH pve-manager v6 11/13] ui: sdn: dhcp tree: do not merge the same subnet in different zones Hannes Laimer
@ 2026-11-05 9:35 ` Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-docs v6 13/13] sdn: ipam: document zone namespaces and the mapping edit Hannes Laimer
12 siblings, 0 replies; 14+ messages in thread
From: Hannes Laimer @ 2026-11-05 9:35 UTC (permalink / raw)
To: pve-devel
The NetBox and phpIPAM dialogs show both options in an advanced
section. The namespace per zone is off by default, and the namespace
prefix is pve when left empty.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
www/manager6/sdn/ipams/Base.js | 37 +++++++++++++++++++++++++++
www/manager6/sdn/ipams/NetboxEdit.js | 4 +++
www/manager6/sdn/ipams/PhpIpamEdit.js | 4 +++
3 files changed, 45 insertions(+)
diff --git a/www/manager6/sdn/ipams/Base.js b/www/manager6/sdn/ipams/Base.js
index 1f6eab296c20..4d37f93d504c 100644
--- a/www/manager6/sdn/ipams/Base.js
+++ b/www/manager6/sdn/ipams/Base.js
@@ -3,6 +3,12 @@ Ext.define('PVE.panel.SDNIpamBase', {
type: '',
+ viewModel: {
+ data: {
+ zoneNamespaces: false,
+ },
+ },
+
onGetValues: function (values) {
var me = this;
@@ -15,6 +21,37 @@ Ext.define('PVE.panel.SDNIpamBase', {
return values;
},
+ // the external IPAMs keep each zone's subnets in a namespace of their
+ // own once the option is on, named with the prefix
+ zoneNamespaceFields: function () {
+ let me = this;
+
+ return [
+ {
+ xtype: 'proxmoxcheckbox',
+ name: 'zone-namespaces',
+ fieldLabel: gettext('Namespace per Zone'),
+ uncheckedValue: null,
+ checked: false,
+ deleteEmpty: !me.isCreate,
+ bind: {
+ value: '{zoneNamespaces}',
+ },
+ },
+ {
+ xtype: 'proxmoxtextfield',
+ name: 'zone-namespace-prefix',
+ fieldLabel: gettext('Namespace Prefix'),
+ emptyText: 'pve',
+ skipEmptyText: true,
+ deleteEmpty: !me.isCreate,
+ bind: {
+ disabled: '{!zoneNamespaces}',
+ },
+ },
+ ];
+ },
+
initComponent: function () {
var me = this;
diff --git a/www/manager6/sdn/ipams/NetboxEdit.js b/www/manager6/sdn/ipams/NetboxEdit.js
index 17971026c93e..51e7b834f7a7 100644
--- a/www/manager6/sdn/ipams/NetboxEdit.js
+++ b/www/manager6/sdn/ipams/NetboxEdit.js
@@ -53,6 +53,10 @@ Ext.define('PVE.sdn.ipams.NetboxInputPanel', {
},
];
+ let [namespaces, prefix] = me.zoneNamespaceFields();
+ me.advancedColumn1 = [namespaces];
+ me.advancedColumn2 = [prefix];
+
me.callParent();
},
});
diff --git a/www/manager6/sdn/ipams/PhpIpamEdit.js b/www/manager6/sdn/ipams/PhpIpamEdit.js
index b28232ab67b7..3824fd734487 100644
--- a/www/manager6/sdn/ipams/PhpIpamEdit.js
+++ b/www/manager6/sdn/ipams/PhpIpamEdit.js
@@ -58,6 +58,10 @@ Ext.define('PVE.sdn.ipams.PhpIpamInputPanel', {
},
];
+ let [namespaces, prefix] = me.zoneNamespaceFields();
+ me.advancedColumn1 = [namespaces];
+ me.advancedColumn2 = [prefix];
+
me.callParent();
},
});
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH pve-docs v6 13/13] sdn: ipam: document zone namespaces and the mapping edit
2026-11-05 9:35 [PATCH docs/manager/network/qemu-server v6 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (11 preceding siblings ...)
2026-11-05 9:35 ` [PATCH pve-manager v6 12/13] ui: sdn: ipam: add the zone namespace options Hannes Laimer
@ 2026-11-05 9:35 ` Hannes Laimer
12 siblings, 0 replies; 14+ messages in thread
From: Hannes Laimer @ 2026-11-05 9:35 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pvesdn.adoc | 44 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/pvesdn.adoc b/pvesdn.adoc
index c85d86a48112..407986901ff2 100644
--- a/pvesdn.adoc
+++ b/pvesdn.adoc
@@ -1265,7 +1265,9 @@ If you are using DHCP, you can use the IPAM panel to create or edit leases for
specific VMs, which enables you to change the IPs allocated via DHCP. When
editing an IP of a VM that is using DHCP you must make sure to force the guest
to acquire a new DHCP leases. This can usually be done by reloading the network
-stack of the guest or rebooting it.
+stack of the guest or rebooting it. The edit updates the DHCP reservation on the
+node of the guest right away. It is refused while the guest is being migrated,
+repeat it once the migration is done.
[[pvesdn_ipam_plugin_netbox]]
NetBox IPAM Plugin
@@ -1287,6 +1289,25 @@ Fingerprint:: The SHA-256 fingerprint of the NetBox API. Can be retrieved with
`openssl x509 -in /etc/ssl/certs/netbox.crt -noout -fingerprint -sha256` when
using the default certificate.
+With the `zone-namespaces` option of the IPAM entry turned on, {pve} creates a
+VRF named `pve-<zone>` for every zone, together with the first subnet of the
+zone, and keeps the prefixes of the zone's subnets in it. This way two zones can
+use the same subnet. The option is off by default and meant to be turned on once
+every node runs a version that knows the VRFs, since a node that does not puts
+an explicit mapping into the global table. With it off the prefixes are looked
+up without regard to their VRF, as before, so the option stays on once two zones
+share a range. Clusters sharing one NetBox set a `zone-namespace-prefix` of
+their own, the names start with `pve` by default. The IP addresses allocated in
+a prefix carry the MAC address of the guest in their description. Prefixes
+created in the global table, by earlier versions or with the option off, are
+still found there and can be moved into the VRF of their zone, together with
+their IP addresses and IP ranges. A zone gets its VRF only with the next subnet
+added to it, so to move prefixes into it earlier, create the VRF
+`<zone-namespace-prefix>-<zone>` by hand. With the option on, creating a subnet
+whose range the global table still holds is refused, since a prefix in the VRF
+would hide the addresses recorded in the old one, so move or remove that prefix
+first.
+
[[pvesdn_ipam_plugin_phpipam]]
phpIPAM Plugin
@@ -1304,6 +1325,27 @@ Token:: An API access token
Section:: An integer ID. Sections are a group of subnets in phpIPAM. Default
installations use `sectionid=1` for customers.
+With the `zone-namespaces` option of the IPAM entry turned on, {pve} creates a
+sub-section named `pve-<zone>` under the configured section for every zone,
+together with the first subnet of the zone, and keeps the zone's subnets in it.
+This way two zones can use the same subnet. The option is off by default and
+meant to be turned on once every node runs a version that knows the
+sub-sections. With it off the subnets are looked up in any section, as before,
+so the option stays on once two zones share a range. phpIPAM nests sections one
+level deep, so the option is refused for a configured section that is itself a
+sub-section. Configure its parent instead. The subnets in the old section have
+to move as well, into the parent or into the sub-sections of their zones, the
+lookup searches only these. Section names are global in phpIPAM, so clusters
+sharing one set a `zone-namespace-prefix` of their own, the names start with
+`pve` by default. Subnets created in the configured section itself, by earlier
+versions or with the option off, are still found there and can be moved into the
+sub-section of their zone. A zone gets its sub-section only with the next subnet
+added to it, so to move subnets into it earlier, create the sub-section
+`<zone-namespace-prefix>-<zone>` under the configured section by hand. With the
+option on, creating a subnet whose range the configured section still holds is
+refused, since a copy in the sub-section would hide the addresses recorded in
+the old one, so move or remove that subnet first.
+
[[pvesdn_config_dns]]
DNS
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2026-11-05 9:37 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-05 9:35 [PATCH docs/manager/network/qemu-server v6 00/13] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 01/13] sdn: ipam: stop caching IPAM misses Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 02/13] sdn: ipam: cache a MAC lookup's answer under the cluster lock Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 03/13] sdn: ipam: read the MAC cache case-insensitively Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 04/13] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 05/13] sdn: ipam: release an address the built-in IPAM no longer holds Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 06/13] sdn: dhcp: update reservations when IP mappings change Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 07/13] sdn: ipam: look a MAC up within the vnet's subnets Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 08/13] sdn: ipam: keep the MAC cache per vnet Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-network v6 09/13] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM Hannes Laimer
2026-11-05 9:35 ` [PATCH qemu-server v6 10/13] network: sdn: push the DHCP mappings of a guest when it resumes Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-manager v6 11/13] ui: sdn: dhcp tree: do not merge the same subnet in different zones Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-manager v6 12/13] ui: sdn: ipam: add the zone namespace options Hannes Laimer
2026-11-05 9:35 ` [PATCH pve-docs v6 13/13] sdn: ipam: document zone namespaces and the mapping edit Hannes Laimer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.