* [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet
@ 2026-11-02 14:56 Hannes Laimer
0 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-02 14:56 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. Patches
1-4 were already included in [1], 1-3 changed since then. 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 5 is new.
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.
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 and is still written, since
nodes not yet upgraded read and write only it. Its entries are never
replaced, a write only adds an address where the MAC has none of that
family yet, and they serve as fallback for a vnet only with addresses
inside that vnet's subnets. A miss in the cache asks the IPAM within
the vnet's subnets, and that answer is put into `vnets`. A `vnets`
address is trusted only while `macs` still holds the MAC's family and
names no address inside the vnet's subnets that none of the MAC's
entries holds, so a release or an edit on a node not yet upgraded is
noticed and the IPAM is asked again. 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 that vnet 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.
- NetBox: each zone gets its own `pve-<zone>` VRF, and the prefixes
of its subnets go in there
- phpIPAM: each zone gets a sub-section `pve-<zone>` under the
configured one, and its subnets go in there(only if the configured
section is not a sub-section already, phpIPAM only allows 1 deep
nesting)
Patch 10 is qemu-server's side. The vmlist names the source of a live
migration until the config moves at its end, so an edit in that
window updates the source. Every resume now pushes the current
mapping of each NIC, and the migration's last step resumes the target
after the move, so it catches up there. The patch needs the
pve-network of this series. Taken from [1] as well.
Patch 11 just adds docs explaining this for the two external IPAMs.
This is not a breaking change, existing assignments are returned
exactly like they are already. Cache entries from before keep their
addresses, since nodes not yet upgraded still use them, and answer for
a vnet with the addresses in its subnets, so a running guest keeps its
reservation. Records in an external IPAM stay where they are, in
NetBox's global table or in phpIPAM's configured section. Like the
cache, an IPAM lookup first checks the zone's VRF or section for the
subnet and falls back to the global table or the configured section.
Unlike with the cache, migration can be done manually, by moving a
prefix together with its addresses and IP ranges into the zone's VRF
in NetBox, or a subnet into the zone's sub-section in phpIPAM, since
the lookup checks those first. Migration is not necessary though.
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 refused, since a copy would hide
the records in it. The error says to move that prefix or subnet into
the zone's VRF or sub-section, or to remove it. We don't move
anything and we don't have to.
If users are bothered by guests getting IPs not assigned to them, they
can move the global entries into the `pve-<zone>` VRF or sub-section,
where they'll be picked up correctly.
note: I'll rebase [1] onto this, it'll lose some patches that moved
here. With [1] things addressed here become more apparent/visible
eventhough they are pre-existing, that is why [1] will be based on this.
[1] https://lore.proxmox.com/pve-devel/20261021084445.462195-1-h.laimer@proxmox.com/
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: write the lookup's cache entry 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
src/PVE/API2/Network/SDN/Ips.pm | 38 +-
src/PVE/Network/SDN/Dhcp.pm | 56 +-
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 148 +++--
src/PVE/Network/SDN/Dhcp/Plugin.pm | 6 +-
src/PVE/Network/SDN/Ipams.pm | 223 ++++++-
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 209 +++++--
src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 27 +-
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 220 +++++--
src/PVE/Network/SDN/Ipams/Plugin.pm | 7 +-
src/PVE/Network/SDN/Subnets.pm | 14 +-
src/PVE/Network/SDN/Vnets.pm | 14 +-
src/test/ipams/netbox/expected.add_ip | 2 +-
.../ipams/netbox/expected.add_ip_notgateway | 2 +-
src/test/ipams/netbox/expected.add_subnet | 2 +-
src/test/ipams/netbox/expected.update_ip | 2 +-
src/test/run_test_ipams.pl | 366 +++++++++++
src/test/run_test_vnets_blackbox.pl | 569 +++++++++++++++++-
17 files changed, 1692 insertions(+), 213 deletions(-)
qemu-server:
Hannes Laimer (1):
network: sdn: push the DHCP mappings of a guest when it resumes
src/PVE/QemuServer/Network.pm | 14 ++++++++++++++
src/PVE/QemuServer/RunState.pm | 2 ++
2 files changed, 16 insertions(+)
pve-docs:
Hannes Laimer (1):
sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM
pvesdn.adoc | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
Summary over all repositories:
20 files changed, 1733 insertions(+), 213 deletions(-)
--
Generated by murpp 0.12.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet
@ 2026-11-03 15:15 Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 01/11] sdn: ipam: stop caching IPAM misses Hannes Laimer
` (12 more replies)
0 siblings, 13 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 15:15 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. Patches
1-4 were already included in [1], 1-3 changed since then. 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 5 is new.
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.
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 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 on two vnets can
match `macs` on one of them only, so the other one asks the IPAM at
every start. 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 that vnet 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.
- NetBox: each zone gets its own `pve-<zone>` VRF, and the prefixes
of its subnets go in there
- phpIPAM: each zone gets a sub-section `pve-<zone>` under the
configured one, and its subnets go in there(only if the configured
section is not a sub-section already, phpIPAM only allows 1 deep
nesting). Section names are global in phpIPAM, so zone ids must
be unique across the clusters sharing one
Patch 10 is qemu-server's side. The vmlist names the source of a live
migration until the config moves at its end, so an edit in that
window updates the source. Every resume now pushes the current
mapping of each NIC, and the migration's last step resumes the target
after the move, so it catches up there. The patch needs the
pve-network of this series. Taken from [1] as well.
Patch 11 just adds docs explaining this for the two external IPAMs.
This is not a breaking change, existing assignments are returned
exactly like they are already. Cache entries from before keep their
addresses, since nodes not yet upgraded still use them, and answer for
a vnet with the addresses in its subnets, so a running guest keeps its
reservation. Records in an external IPAM stay where they are, in
NetBox's global table or in phpIPAM's configured section. Like the
cache, an IPAM lookup first checks the zone's VRF or section for the
subnet and falls back to the global table or the configured section.
Unlike with the cache, migration can be done manually, by moving a
prefix together with its addresses and IP ranges into the zone's VRF
in NetBox, or a subnet into the zone's sub-section in phpIPAM, since
the lookup checks those first. Migration is not necessary though.
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 refused, since a copy would hide
the records in it. The error says to move that prefix or subnet into
the zone's VRF or sub-section, or to remove it. We don't move
anything and we don't have to.
If users are bothered by guests getting IPs not assigned to them, they
can move the global entries into the `pve-<zone>` VRF or sub-section,
where they'll be picked up correctly. While a rolling upgrade is
under way, a node not yet upgraded puts an explicit mapping into
NetBox's global table, also for a subnet created on an upgraded
node, such addresses get moved like the ones from before.
note: I'll rebase [1] onto this, it'll lose some patches that moved
here. With [1] things addressed here become more apparent/visible
eventhough they are pre-existing, that is why [1] will be based on this.
[1] https://lore.proxmox.com/pve-devel/20261021084445.462195-1-h.laimer@proxmox.com/
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: write the lookup's cache entry 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
src/PVE/API2/Network/SDN/Ips.pm | 38 +-
src/PVE/Network/SDN/Dhcp.pm | 56 +-
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 153 +++--
src/PVE/Network/SDN/Dhcp/Plugin.pm | 7 +-
src/PVE/Network/SDN/Ipams.pm | 180 ++++-
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 223 +++++--
src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 27 +-
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 237 ++++++-
src/PVE/Network/SDN/Ipams/Plugin.pm | 7 +-
src/PVE/Network/SDN/Subnets.pm | 14 +-
src/PVE/Network/SDN/Vnets.pm | 21 +-
src/test/ipams/netbox/expected.add_ip | 2 +-
.../ipams/netbox/expected.add_ip_notgateway | 2 +-
src/test/ipams/netbox/expected.add_subnet | 2 +-
src/test/ipams/netbox/expected.update_ip | 2 +-
src/test/run_test_ipams.pl | 468 +++++++++++++
src/test/run_test_vnets_blackbox.pl | 618 +++++++++++++++++-
17 files changed, 1836 insertions(+), 221 deletions(-)
qemu-server:
Hannes Laimer (1):
network: sdn: push the DHCP mappings of a guest when it resumes
src/PVE/QemuServer/Network.pm | 19 +++++++++++++++++++
src/PVE/QemuServer/RunState.pm | 2 ++
2 files changed, 21 insertions(+)
pve-docs:
Hannes Laimer (1):
sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM
pvesdn.adoc | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
Summary over all repositories:
20 files changed, 1888 insertions(+), 221 deletions(-)
--
Generated by murpp 0.12.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH pve-network v2 01/11] sdn: ipam: stop caching IPAM misses
2026-11-03 15:15 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
@ 2026-11-03 15:15 ` Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 02/11] sdn: ipam: write the lookup's cache entry under the cluster lock Hannes Laimer
` (11 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 15:15 UTC (permalink / raw)
To: pve-devel
A lookup that missed the cache stored the IPAM's empty answer, and
any entry counted as a hit. So a record created later directly in the
IPAM stayed hidden, and a guest start allocated another address. An
empty answer is no longer stored, and an entry without an address, as
earlier misses left them, no longer counts as a hit.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams.pm | 16 ++++++--
src/test/run_test_vnets_blackbox.pl | 58 ++++++++++++++++++++++++++++-
2 files changed, 69 insertions(+), 5 deletions(-)
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 179bdf76defc..7e7bec27b5ed 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -136,16 +136,24 @@ sub get_ips_from_mac {
my ($mac, $zoneid, $zone) = @_;
my $macdb = read_macdb();
- return ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6}) if $macdb->{macs}->{$mac};
+ my $cached = $macdb->{macs}->{$mac};
+ # an entry without an address was left behind by a lookup that missed, and
+ # says nothing about its MAC
+ return ($cached->{ip4}, $cached->{ip6})
+ if $cached && (defined($cached->{ip4}) || defined($cached->{ip6}));
my $plugin_config = get_plugin_config($zone);
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
- ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6}) =
- $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid);
+ my ($ip4, $ip6) = $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid);
+ # an empty answer is not cached, since the record may simply not exist yet
+ return if !defined($ip4) && !defined($ip6);
+
+ $macdb->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4);
+ $macdb->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6);
write_macdb($macdb);
- return ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6});
+ return ($ip4, $ip6);
}
1;
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index d5a78c5b9a16..37eb5567eae6 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -5,6 +5,7 @@ use warnings;
use lib qw(..);
use File::Slurp;
+use JSON;
use List::Util qw(first all);
use NetAddr::IP qw(:lower);
@@ -194,7 +195,7 @@ $mocked_sdn_ipams->mock(
$test_state->{ipam_config} = $cfg;
},
read_macdb => sub {
- return $test_state->{macdb};
+ return PVE::Network::SDN::Ipams::json_reader(undef, encode_json($test_state->{macdb}));
},
write_macdb => sub {
my ($cfg) = @_;
@@ -335,6 +336,34 @@ sub create_subnet {
PVE::API2::Network::SDN::Subnets->create($params);
}
+# a DHCP zone with one vnet, each subnet with its first address as the
+# gateway and a range over its hosts 100 to 200
+sub create_dhcp_zone {
+ my ($zoneid, $vnetid, @cidrs) = @_;
+ create_zone({ type => "simple", dhcp => "dnsmasq", ipam => "pve", zone => $zoneid });
+ create_dhcp_vnet($zoneid, $vnetid, @cidrs);
+}
+
+sub create_dhcp_vnet {
+ my ($zoneid, $vnetid, @cidrs) = @_;
+ create_vnet({ type => "vnet", zone => $zoneid, vnet => $vnetid });
+ create_dhcp_subnet($vnetid, $_) for @cidrs;
+}
+
+sub create_dhcp_subnet {
+ my ($vnetid, $cidr) = @_;
+ my $net = NetAddr::IP->new($cidr);
+ my $addr = sub { my ($ip) = @_; return $ip->version() == 4 ? $ip->addr() : $ip->short(); };
+ create_subnet({
+ type => "subnet",
+ vnet => $vnetid,
+ subnet => $cidr,
+ gateway => $addr->($net->first()),
+ 'dhcp-range' =>
+ ["start-address=" . $addr->($net + 100) . ",end-address=" . $addr->($net + 200)],
+ });
+}
+
sub get_ipam_entries {
return PVE::API2::Network::SDN::Ipams->ipamindex({ ipam => "pve" });
}
@@ -435,6 +464,33 @@ sub test_without_subnet {
}
run_test(\&test_without_subnet);
+sub test_ipam_cache_lookup {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+ my $cache = decode_json(encode_json($test_state->{macdb}));
+ PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is_deeply($test_state->{macdb}, $cache, "$test_name: the miss is not cached");
+
+ # an entry holding no address, left behind by an earlier miss, does not
+ # hide a record created since
+ create_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.50" });
+ $test_state->{macdb}->{macs}->{$mac} = { ip4 => undef, ip6 => undef };
+ my @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is($ips[0], "10.0.0.50", "$test_name: an address-less entry counts as a miss");
+ is(
+ $test_state->{macdb}->{macs}->{$mac}->{ip4},
+ "10.0.0.50",
+ "$test_name: the answer is cached",
+ );
+}
+
+run_test(\&test_ipam_cache_lookup);
+
sub test_nic_join {
my ($test_name, $subnets) = @_;
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH pve-network v2 02/11] sdn: ipam: write the lookup's cache entry under the cluster lock
2026-11-03 15:15 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 01/11] sdn: ipam: stop caching IPAM misses Hannes Laimer
@ 2026-11-03 15:15 ` Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 03/11] sdn: ipam: read the MAC cache case-insensitively Hannes Laimer
` (10 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 15:15 UTC (permalink / raw)
To: pve-devel
The lookup that stores the IPAM's answer wrote the whole cache back
without the cluster lock the other writers take. A change made
between its read and its write was lost. The lookup now takes the
lock and reads the cache again before it stores its answer. The IPAM
is still asked outside the lock, since the lock is cluster-wide and a
request to an external IPAM does not belong under it. A cache write
that lands while the IPAM is asked may be newer than the answer, so
the answer is then returned without being stored, and the next lookup
stores it.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams.pm | 24 +++++++++++++++++++-----
src/test/run_test_vnets_blackbox.pl | 11 +++++++++++
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 7e7bec27b5ed..ae2c4337ef8d 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -135,8 +135,9 @@ sub complete_sdn_vnet {
sub get_ips_from_mac {
my ($mac, $zoneid, $zone) = @_;
- my $macdb = read_macdb();
- my $cached = $macdb->{macs}->{$mac};
+ PVE::Cluster::cfs_update(); # the lock refreshes, so the snapshot must too
+ my $version = PVE::Cluster::cfs_file_version($macdb_filename);
+ my $cached = read_macdb()->{macs}->{$mac};
# an entry without an address was left behind by a lookup that missed, and
# says nothing about its MAC
return ($cached->{ip4}, $cached->{ip6})
@@ -149,9 +150,22 @@ sub get_ips_from_mac {
# an empty answer is not cached, since the record may simply not exist yet
return if !defined($ip4) && !defined($ip6);
- $macdb->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4);
- $macdb->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6);
- write_macdb($macdb);
+ cfs_lock_file(
+ $macdb_filename,
+ undef,
+ sub {
+ # the IPAM was asked outside the lock, so a write since the miss may
+ # be newer than the answer. The fill is then left to the next lookup
+ return
+ if (PVE::Cluster::cfs_file_version($macdb_filename) // '') ne ($version // '');
+
+ my $db = read_macdb();
+ $db->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4);
+ $db->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6);
+ write_macdb($db);
+ },
+ );
+ warn "$@" if $@;
return ($ip4, $ip6);
}
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 37eb5567eae6..927ab16adba9 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -487,6 +487,17 @@ sub test_ipam_cache_lookup {
"10.0.0.50",
"$test_name: the answer is cached",
);
+
+ # a write to the cache between the miss and the fill, which the file's
+ # version shows, keeps the IPAM's answer out of the cache
+ delete $test_state->{macdb}->{macs}->{$mac};
+ my $version = 0;
+ $mocked_pve_cluster_obj->mock(cfs_file_version => sub { return $version++ });
+ @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ $mocked_pve_cluster_obj->unmock('cfs_file_version');
+ is($ips[0], "10.0.0.50", "$test_name: the lookup still answers");
+ ok(!exists $test_state->{macdb}->{macs}->{$mac},
+ "$test_name: but leaves a changed cache alone");
}
run_test(\&test_ipam_cache_lookup);
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH pve-network v2 03/11] sdn: ipam: read the MAC cache case-insensitively
2026-11-03 15:15 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 01/11] sdn: ipam: stop caching IPAM misses Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 02/11] sdn: ipam: write the lookup's cache entry under the cluster lock Hannes Laimer
@ 2026-11-03 15:15 ` Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 04/11] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it Hannes Laimer
` (9 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 15:15 UTC (permalink / raw)
To: pve-devel
Generated guest MACs are upper case, but the API accepts either
spelling, and the cache was keyed by the MAC as its writer spelled it.
A deletion in the other spelling missed the entry, so the cache kept
answering with the deleted address.
Lookups now match a MAC in any spelling, and a write or a deletion
reaches its entry under every spelling the file holds. The keys stay
as written, since nodes not yet upgraded look an entry up under the
spelling their caller hands them, and a MAC new to the file is
written in lower case. Two spellings that disagree on an address are
both left out of the lookup, since one of them is stale and the cache
cannot tell which, so the next lookup asks the IPAM. The dnsmasq
reservation file is written and read with its MACs in lower case as
well, since its lines from before keep their writer's spelling.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 3 ++
src/PVE/Network/SDN/Ipams.pm | 72 +++++++++++++++++++++--------
src/test/run_test_vnets_blackbox.pl | 51 +++++++++++++++++++-
3 files changed, 107 insertions(+), 19 deletions(-)
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index 477b700f2429..d3b78fe9a894 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -53,6 +53,7 @@ sub update_lease {
sub add_ip_mapping {
my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
+ $mac = lc($mac);
my $ethers_file = ethers_file($dhcpid);
my $ethers_tmp_file = "$ethers_file.tmp";
@@ -76,6 +77,8 @@ sub add_ip_mapping {
warn "failed to parse MAC from $dhcpid ethers file on line $line_no: '$line'\n";
next;
}
+ # the cache is keyed in lower case, a line from before keeps its writer's spelling
+ $parsed_mac = lc($parsed_mac);
my ($parsed_ip4, $parsed_ip6);
if ($parsed_ip2) {
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index ae2c4337ef8d..1476c4236be0 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -35,18 +35,58 @@ sub json_writer {
return encode_json($data);
}
-sub read_macdb {
+# Writers spell a MAC their own way, the generated ones in upper case and the
+# API in either, and nodes not yet upgraded look an entry up under the spelling
+# their caller hands them, so the file keeps its keys as written. Readers get a
+# view with every MAC in lower case. Two spellings that disagree on an address
+# are left out of it, since one of them is stale and nothing here says which,
+# and so is an entry without an address. The next lookup then asks the IPAM
+my sub fold_macdb {
+ my ($db) = @_;
+
+ my $macs = {};
+ my %clash;
+ for my $key (sort keys %{ $db->{macs} // {} }) {
+ my $entry = $db->{macs}->{$key};
+ for my $family (grep { defined($entry->{$_}) } qw(ip4 ip6)) {
+ my $seen = $macs->{ lc($key) }->{$family};
+ $clash{ lc($key) } = 1 if defined($seen) && lc($seen) ne lc($entry->{$family});
+ $macs->{ lc($key) }->{$family} //= $entry->{$family};
+ }
+ }
+ delete $macs->{$_} for keys %clash;
+ return { %$db, macs => $macs };
+}
+
+# the file as written, which the writers change and write back
+sub read_macdb_file {
my () = @_;
return cfs_read_file($macdb_filename);
}
+sub read_macdb {
+ my () = @_;
+
+ return fold_macdb(read_macdb_file());
+}
+
sub write_macdb {
my ($data) = @_;
cfs_write_file($macdb_filename, $data);
}
+# a write goes to every spelling the file holds for the MAC, so each reader
+# finds it under its own, and a MAC new to the file is written in lower case
+my sub set_cache_entry {
+ my ($db, $mac, $family, $ip) = @_;
+
+ my $macs = $db->{macs} //= {};
+ my @keys = grep { lc($_) eq lc($mac) } keys %$macs;
+ $macs->{$_}->{$family} = $ip for (@keys ? @keys : (lc($mac)));
+}
+
sub add_cache_mac_ip {
my ($mac, $ip) = @_;
@@ -54,12 +94,8 @@ sub add_cache_mac_ip {
$macdb_filename,
undef,
sub {
- my $db = read_macdb();
- if (Net::IP::ip_is_ipv4($ip)) {
- $db->{macs}->{$mac}->{ip4} = $ip;
- } else {
- $db->{macs}->{$mac}->{ip6} = $ip;
- }
+ my $db = read_macdb_file();
+ set_cache_entry($db, $mac, Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6', $ip);
write_macdb($db);
},
);
@@ -73,14 +109,14 @@ sub del_cache_mac_ip {
$macdb_filename,
undef,
sub {
- my $db = read_macdb();
- if (Net::IP::ip_is_ipv4($ip)) {
- delete $db->{macs}->{$mac}->{ip4};
- } else {
- delete $db->{macs}->{$mac}->{ip6};
+ my $db = read_macdb_file();
+ my $family = Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6';
+ my $macs = $db->{macs} // {};
+ for my $key (grep { lc($_) eq lc($mac) } keys %$macs) {
+ delete $macs->{$key}->{$family};
+ delete $macs->{$key}
+ if !defined($macs->{$key}->{ip4}) && !defined($macs->{$key}->{ip6});
}
- delete $db->{macs}->{$mac}
- if !defined($db->{macs}->{$mac}->{ip4}) && !defined($db->{macs}->{$mac}->{ip6});
write_macdb($db);
},
);
@@ -137,7 +173,7 @@ sub get_ips_from_mac {
PVE::Cluster::cfs_update(); # the lock refreshes, so the snapshot must too
my $version = PVE::Cluster::cfs_file_version($macdb_filename);
- my $cached = read_macdb()->{macs}->{$mac};
+ my $cached = read_macdb()->{macs}->{ lc($mac) };
# an entry without an address was left behind by a lookup that missed, and
# says nothing about its MAC
return ($cached->{ip4}, $cached->{ip6})
@@ -159,9 +195,9 @@ sub get_ips_from_mac {
return
if (PVE::Cluster::cfs_file_version($macdb_filename) // '') ne ($version // '');
- my $db = read_macdb();
- $db->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4);
- $db->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6);
+ my $db = read_macdb_file();
+ set_cache_entry($db, $mac, 'ip4', $ip4) if defined($ip4);
+ set_cache_entry($db, $mac, 'ip6', $ip6) if defined($ip6);
write_macdb($db);
},
);
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 927ab16adba9..be4c581e414e 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -194,7 +194,7 @@ $mocked_sdn_ipams->mock(
my ($cfg) = @_;
$test_state->{ipam_config} = $cfg;
},
- read_macdb => sub {
+ read_macdb_file => sub {
return PVE::Network::SDN::Ipams::json_reader(undef, encode_json($test_state->{macdb}));
},
write_macdb => sub {
@@ -498,6 +498,55 @@ sub test_ipam_cache_lookup {
is($ips[0], "10.0.0.50", "$test_name: the lookup still answers");
ok(!exists $test_state->{macdb}->{macs}->{$mac},
"$test_name: but leaves a changed cache alone");
+
+ # an entry written under the config's spelling, as a node not yet upgraded
+ # writes it, is read and released like any other and keeps its key
+ my $other = "da:65:8f:18:9b:70";
+ $test_state->{macdb}->{macs} = {
+ uc($mac) => { ip4 => '10.0.0.150' },
+ uc($other) => { ip4 => '10.0.0.170' },
+ };
+ my ($ip4) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is($ip4, '10.0.0.150', "$test_name: an entry is found however it is spelled");
+ PVE::Network::SDN::Ipams::del_cache_mac_ip($mac, '10.0.0.150');
+ is_deeply(
+ $test_state->{macdb}->{macs},
+ { uc($other) => { ip4 => '10.0.0.170' } },
+ "$test_name: and released, the other entry left as written",
+ );
+
+ # a write in another spelling goes to the spelling the file holds
+ $test_state->{macdb}->{macs} = { uc($mac) => { ip4 => '10.0.0.150' } };
+ PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, 'fd00::150');
+ is_deeply(
+ $test_state->{macdb}->{macs},
+ { uc($mac) => { ip4 => '10.0.0.150', ip6 => 'fd00::150' } },
+ "$test_name: a write keeps the key as written",
+ );
+
+ # a lookup past an address-less entry asks the IPAM and fills the entry
+ # under its spelling
+ $test_state->{macdb}->{macs} = { uc($mac) => { ip4 => undef, ip6 => undef } };
+ @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is($ips[0], '10.0.0.50', "$test_name: a lookup past an address-less entry asks the IPAM");
+ is_deeply(
+ $test_state->{macdb}->{macs},
+ { uc($mac) => { ip4 => '10.0.0.50', ip6 => undef } },
+ "$test_name: and fills the entry under its spelling",
+ );
+
+ # two spellings that disagree are not trusted, the lookup asks the IPAM
+ $test_state->{macdb}->{macs} = {
+ uc($mac) => { ip4 => '10.0.0.150' },
+ $mac => { ip4 => '10.0.0.151' },
+ };
+ @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is($ips[0], '10.0.0.50', "$test_name: a clash of spellings counts as a miss");
+ is_deeply(
+ $test_state->{macdb}->{macs},
+ { uc($mac) => { ip4 => '10.0.0.50' }, $mac => { ip4 => '10.0.0.50' } },
+ "$test_name: and its write brings both spellings in line",
+ );
}
run_test(\&test_ipam_cache_lookup);
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH pve-network v2 04/11] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it
2026-11-03 15:15 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (2 preceding siblings ...)
2026-11-03 15:15 ` [PATCH pve-network v2 03/11] sdn: ipam: read the MAC cache case-insensitively Hannes Laimer
@ 2026-11-03 15:15 ` Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 05/11] sdn: ipam: release an address the built-in IPAM no longer holds Hannes Laimer
` (8 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 15:15 UTC (permalink / raw)
To: pve-devel
A reservation rewrite dropped every line that no longer matched the
MAC cache and wrote nothing in its place. The guest behind such a
line was unknown to dnsmasq until its next start, so its requests
were ignored. The line is now replaced by the cache's current one.
Only a MAC the cache holds no address for is dropped.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
Reviewed-by: Stefan Hanreich <s.hanreich@proxmox.com>
Tested-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 11 ++++++++++-
src/test/run_test_vnets_blackbox.pl | 28 ++++++++++++++++++++++++++++
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index d3b78fe9a894..a1e4c70f9ba4 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -91,7 +91,9 @@ sub add_ip_mapping {
}
$parsed_ip6 = $1 if $parsed_ip6 && $parsed_ip6 =~ m/\[(\S+)\]/;
- #delete changed
+ # a line that no longer matches the cache is replaced by the cache's
+ # current one. Dropping it left that guest unknown to dnsmasq until
+ # its next start
if (
!defined($macdb->{macs}->{$parsed_mac})
|| ($parsed_ip4
@@ -102,6 +104,13 @@ sub add_ip_mapping {
&& $macdb->{macs}->{$parsed_mac}->{'ip6'} ne $parsed_ip6)
) {
$reload = 1;
+ # the MAC being added gets its line below, like a missing one
+ my $current = $macdb->{macs}->{$parsed_mac};
+ next if $parsed_mac eq $mac || !$current;
+ my $replacement = $parsed_mac;
+ $replacement .= ",$current->{ip4}" if $current->{ip4};
+ $replacement .= ",[$current->{ip6}]" if $current->{ip6};
+ print $out "$replacement\n" if $replacement ne $parsed_mac;
next;
}
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index be4c581e414e..90241268077c 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -1107,4 +1107,32 @@ sub test_update_vnet_without_zone {
}
run_test(\&test_update_vnet_without_zone);
+sub test_dnsmasq_stale_reservation_replaced {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+
+ my $stale = "da:65:8f:18:9b:01";
+ my $gone = "da:65:8f:18:9b:02";
+ my $added = "da:65:8f:18:9b:03";
+ $test_state->{macdb} = {
+ macs => {
+ $stale => { ip4 => '10.0.1.11' },
+ $added => { ip4 => '10.0.1.13' },
+ },
+ };
+ PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$stale,10.0.1.10\n$gone,10.0.1.12\n");
+
+ PVE::Network::SDN::Dhcp::Dnsmasq->add_ip_mapping(
+ 'TESTZONE', $test_state->{macdb}, $added, '10.0.1.13', undef,
+ );
+
+ my @lines = sort grep { /\S/ } split(/\n/, PVE::Tools::file_get_contents($TMP_ETHERS_FILE));
+ eq_or_diff(
+ \@lines,
+ ["$stale,10.0.1.11", "$added,10.0.1.13"],
+ "$test_name: the stale line is replaced, the unknown one dropped",
+ );
+}
+
+run_test(\&test_dnsmasq_stale_reservation_replaced);
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH pve-network v2 05/11] sdn: ipam: release an address the built-in IPAM no longer holds
2026-11-03 15:15 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (3 preceding siblings ...)
2026-11-03 15:15 ` [PATCH pve-network v2 04/11] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it Hannes Laimer
@ 2026-11-03 15:15 ` Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 06/11] sdn: dhcp: update reservations when ip mappings change Hannes Laimer
` (7 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 15:15 UTC (permalink / raw)
To: pve-devel
The built-in IPAM died on releasing an address it had no record of,
before the cache entry of the address was cleared. The entry kept
answering with the address, and the API could not delete the mapping
either. NetBox and phpIPAM warn and release nothing in that case. The
built-in IPAM now does the same, so the cache entry is cleared and the
API deletion succeeds.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 7 ++++++-
src/test/run_test_vnets_blackbox.pl | 17 +++++++++++++++++
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index a26da57a877a..77a7b2a7c144 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -265,7 +265,12 @@ sub del_ip {
die "subnet $cidr don't exist in ipam db" if !$dbzone->{subnets}->{$cidr};
my $dbsubnet = $dbzone->{subnets}->{$cidr};
- die "IP '$ip' does not exist in IPAM DB\n" if !defined($dbsubnet->{ips}->{$ip});
+ # a record that is gone was released already, and the address still
+ # has to leave the cache, so this is no error
+ if (!defined($dbsubnet->{ips}->{$ip})) {
+ warn "IP '$ip' does not exist in IPAM DB\n";
+ return;
+ }
delete $dbsubnet->{ips}->{$ip};
write_db($db);
},
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 90241268077c..164ef4b4592e 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -547,6 +547,23 @@ sub test_ipam_cache_lookup {
{ uc($mac) => { ip4 => '10.0.0.50' }, $mac => { ip4 => '10.0.0.50' } },
"$test_name: and its write brings both spellings in line",
);
+
+ # a mapping whose record is gone is still deleted, and its cache entry
+ # with it
+ $test_state->{macdb} = {};
+ create_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.70" });
+ delete $test_state->{ipamdb}->{zones}->{$zoneid}->{subnets}->{"10.0.0.0/24"}->{ips}
+ ->{"10.0.0.70"};
+ eval {
+ PVE::API2::Network::SDN::Ips->ipdelete(
+ { zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.70" });
+ };
+ is($@, '', "$test_name: a mapping whose record is gone is deleted without an error");
+ is(
+ (PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac))[0],
+ '10.0.0.50',
+ "$test_name: and the cache no longer answers with its address",
+ );
}
run_test(\&test_ipam_cache_lookup);
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH pve-network v2 06/11] sdn: dhcp: update reservations when ip mappings change
2026-11-03 15:15 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (4 preceding siblings ...)
2026-11-03 15:15 ` [PATCH pve-network v2 05/11] sdn: ipam: release an address the built-in IPAM no longer holds Hannes Laimer
@ 2026-11-03 15:15 ` Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 07/11] sdn: ipam: look a MAC up within the vnet's subnets Hannes Laimer
` (6 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 15:15 UTC (permalink / raw)
To: pve-devel
Creating, updating or deleting an IP mapping through the API changed
the IPAM and the MAC cache, but never the zone's dnsmasq reservation
file. dnsmasq kept serving the old reservation, or none for a new
mapping, until the guest's next start.
A guest start, an API edit and the deletion of a NIC's addresses now
all rewrite the MAC's reservation from the cache. The reservations
are a file on each node, and only the guest's node answers its DHCP
requests, so an API edit is proxied to the node of the guest whose
NIC carries the MAC on that vnet. The other nodes catch up when the
guest starts on one of them. When the update fails, the API edit
fails with the records already written, and its error says so. A
guest start and the removal of a NIC only warn.
The rewrite refreshes the cluster state and reads the cache only
once it holds the file's lock, so a rewrite that ran in between is
not undone. The MAC's line is written in full from the cache, so a
guest gaining an IPv6 address next to its IPv4 one gets it reserved
too.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/API2/Network/SDN/Ips.pm | 28 ++++
src/PVE/Network/SDN/Dhcp.pm | 56 ++++----
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 106 +++++++++------
src/PVE/Network/SDN/Dhcp/Plugin.pm | 6 +-
src/PVE/Network/SDN/Vnets.pm | 12 +-
src/test/run_test_vnets_blackbox.pl | 198 ++++++++++++++++++++++++++--
6 files changed, 330 insertions(+), 76 deletions(-)
diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
index 04741d8750a4..7653a126eb64 100644
--- a/src/PVE/API2/Network/SDN/Ips.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -3,9 +3,11 @@ package PVE::API2::Network::SDN::Ips;
use strict;
use warnings;
+use PVE::Cluster;
use PVE::Tools qw(extract_param);
use PVE::Network::SDN::Vnets;
+use PVE::Network::SDN::Zones;
use PVE::Network::SDN::Dhcp;
use PVE::JSONSchema qw(get_standard_option);
@@ -13,12 +15,29 @@ use PVE::RESTHandler;
use base qw(PVE::RESTHandler);
+# only the guest's node answers its DHCP requests, so the edit runs there.
+# With that node offline it runs here, and the node catches up at the
+# guest's next start
+my $proxy_to_guest_node = sub {
+ my ($rpcenv, $proxyto, $param) = @_;
+
+ my $vnet = PVE::Network::SDN::Vnets::get_vnet($param->{vnet}, 1);
+ my $zone = $vnet ? PVE::Network::SDN::Zones::get_zone($vnet->{zone}, 1) : undef;
+ return 'localhost' if !$zone || !$zone->{dhcp};
+
+ my $node = PVE::Network::SDN::Dhcp::guest_node_by_mac($param->{vnet}, $param->{mac});
+ my $members = PVE::Cluster::get_members();
+ return 'localhost' if !$node || ($members && !($members->{$node} // {})->{online});
+ return $node;
+};
+
__PACKAGE__->register_method({
name => 'ipdelete',
path => '',
method => 'DELETE',
description => 'Delete IP Mappings in a VNet',
protected => 1,
+ proxyto_callback => $proxy_to_guest_node,
permissions => {
description => "Requires 'SDN.Allocate' on '/sdn/zones/<zone>/<vnet>'",
user => 'all',
@@ -58,6 +77,8 @@ __PACKAGE__->register_method({
eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
die "$@\n" if $@;
+ PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+
return undef;
},
});
@@ -68,6 +89,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',
@@ -106,6 +128,8 @@ __PACKAGE__->register_method({
PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, undef);
+ PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+
return undef;
},
});
@@ -115,6 +139,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',
@@ -168,6 +193,9 @@ __PACKAGE__->register_method({
}
die "$error\n" if $error;
+
+ PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+
return undef;
},
});
diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
index f6ce842c8cc2..964e3fd2f1d3 100644
--- a/src/PVE/Network/SDN/Dhcp.pm
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -6,7 +6,6 @@ use warnings;
use PVE::Cluster;
use PVE::Network::SDN;
-use PVE::Network::SDN::Ipams;
use PVE::Network::SDN::Subnets;
use PVE::Network::SDN::Dhcp::Plugin;
use PVE::Network::SDN::Dhcp::Dnsmasq;
@@ -22,37 +21,50 @@ sub plugin_types {
return PVE::Network::SDN::Dhcp::Plugin->lookup_types();
}
-sub add_mapping {
- my ($vnetid, $mac, $ip4, $ip6) = @_;
+sub update_mapping {
+ my ($vnetid, $mac) = @_;
- my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid);
- return if !$vnet;
+ # a record without a MAC has no guest NIC behind it, so no backend serves it
+ return if !$mac;
- my $zoneid = $vnet->{zone};
- my $zone = PVE::Network::SDN::Zones::get_zone($zoneid);
+ eval {
+ # the backend to push to is the applied one, since a zone edited but
+ # not applied has nothing running yet
+ my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid, 1);
+ return if !$vnet;
- return if !$zone->{ipam} || !$zone->{dhcp};
+ my $zoneid = $vnet->{zone};
+ my $zone = PVE::Network::SDN::Zones::get_zone($zoneid, 1);
+ return if !$zone || !$zone->{ipam} || !$zone->{dhcp};
- my $dhcptype = $zone->{dhcp};
+ # a zone confined to other nodes has no vnet bridge on this node
+ return if defined($zone->{nodes}) && !$zone->{nodes}->{ PVE::INotify::nodename() };
- my $macdb = PVE::Network::SDN::Ipams::read_macdb();
- my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($dhcptype);
- $dhcp_plugin->add_ip_mapping($zoneid, $macdb, $mac, $ip4, $ip6);
+ PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp})
+ ->update_ip_mapping($zoneid, $mac);
+ };
+ # the records are written by then and stay
+ die "IPAM is updated for $mac, but DHCP is not: $@" if $@;
}
-sub remove_mapping {
+sub guest_node_by_mac {
my ($vnetid, $mac) = @_;
- my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid);
- return if !$vnet;
-
- my $zoneid = $vnet->{zone};
- my $zone = PVE::Network::SDN::Zones::get_zone($zoneid);
-
- return if !$zone->{ipam} || !$zone->{dhcp};
+ my $vmlist = PVE::Cluster::get_vmlist();
+ my $nets = PVE::Cluster::get_guest_config_properties([map { "net$_" } 0 .. 31]);
+ for my $vmid (sort keys %$nets) {
+ my $guest = $vmlist->{ids}->{$vmid} // next;
+ for my $net (values %{ $nets->{$vmid} }) {
+ my ($bridge) = $net =~ m/(?:^|,)bridge=([^,]+)/ or next;
+ next if $bridge ne $vnetid;
+ # a VM names the MAC as the value of its model, a container as hwaddr
+ my ($nic_mac) = $net =~ m/(?:^|,)[a-z0-9_-]+=([0-9a-f]{2}(?::[0-9a-f]{2}){5})(?:,|$)/i
+ or next;
+ return $guest->{node} if lc($nic_mac) eq lc($mac);
+ }
+ }
- my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp});
- $dhcp_plugin->del_ip_mapping($zoneid, $mac);
+ return undef;
}
sub regenerate_config {
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index a1e4c70f9ba4..9cd16020887e 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -11,7 +11,9 @@ use PVE::Tools qw(file_set_contents run_command lock_file);
use File::Copy;
use Net::DBus;
+use PVE::Cluster;
use PVE::RESTEnvironment qw(log_warn);
+use PVE::Network::SDN::Ipams;
my $DNSMASQ_CONFIG_ROOT = '/etc/dnsmasq.d';
my $DNSMASQ_DEFAULT_ROOT = '/etc/default';
@@ -51,83 +53,100 @@ sub update_lease {
$manager->AddDhcpLease($ip4, $mac, \@hostname, undef, 0, 0, 0) if $ip4;
}
-sub add_ip_mapping {
- my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
+sub update_ip_mapping {
+ my ($class, $dhcpid, $mac) = @_;
$mac = lc($mac);
my $ethers_file = ethers_file($dhcpid);
my $ethers_tmp_file = "$ethers_file.tmp";
- my $reload = undef;
+ my ($ip4, $ip6);
+ my $reservation = undef;
+ my $changed = undef;
- my $appendFn = sub {
+ my $rewriteFn = sub {
+ # the lines are decided from the cache, which this process may hold an
+ # old read of, and the local lock does not refresh it
+ PVE::Cluster::cfs_update();
+
+ # read under the ethers lock. A snapshot from before it would sweep a
+ # line another push wrote meanwhile, and an older push would put its
+ # addresses back
+
+ my $macs = PVE::Network::SDN::Ipams::read_macdb()->{macs};
+
+ my $entry = $macs->{$mac};
+ ($ip4, $ip6) = ($entry->{ip4}, $entry->{ip6}) if $entry;
+ if ($ip4 || $ip6) {
+ $reservation = $mac;
+ $reservation .= ",$ip4" if $ip4;
+ $reservation .= ",[$ip6]" if $ip6;
+ # an external IPAM spells addresses its own way, while the file holds one spelling
+ $reservation = lc($reservation);
+ }
+
+ file_set_contents($ethers_file, '') if !-e $ethers_file;
open(my $in, '<', $ethers_file) or die "Could not open file '$ethers_file' $!\n";
open(my $out, '>', $ethers_tmp_file)
or die "Could not open file '$ethers_tmp_file' $!\n";
- my $match = undef;
+ my $kept = undef;
- my $line_no = 0;
while (my $line = <$in>) {
- $line_no++;
chomp($line);
+ next if $line !~ m/\S/;
my ($parsed_mac, $parsed_ip1, $parsed_ip2) = split(/,/, $line);
+ # a line from before the file was written in lower case keeps its writer's spelling
+ $parsed_mac = lc($parsed_mac);
- if (!defined($parsed_mac)) {
- warn "failed to parse MAC from $dhcpid ethers file on line $line_no: '$line'\n";
+ # the MAC's own reservation is rewritten. An unchanged one stays in place
+ if ($parsed_mac eq $mac) {
+ if (defined($reservation) && !$kept && lc($line) eq $reservation) {
+ $kept = 1;
+ print $out "$line\n";
+ } else {
+ $changed = 1;
+ }
next;
}
- # the cache is keyed in lower case, a line from before keeps its writer's spelling
- $parsed_mac = lc($parsed_mac);
my ($parsed_ip4, $parsed_ip6);
if ($parsed_ip2) {
$parsed_ip4 = $parsed_ip1;
$parsed_ip6 = $parsed_ip2;
- } elsif (Net::IP::ip_is_ipv4($parsed_ip1)) {
+ } elsif (defined($parsed_ip1) && Net::IP::ip_is_ipv4($parsed_ip1)) {
$parsed_ip4 = $parsed_ip1;
- } else {
+ } elsif (defined($parsed_ip1)) {
$parsed_ip6 = $parsed_ip1;
}
$parsed_ip6 = $1 if $parsed_ip6 && $parsed_ip6 =~ m/\[(\S+)\]/;
+ # a line from before addresses were lower-cased may spell one in upper case
+ $parsed_ip6 = lc($parsed_ip6) if $parsed_ip6;
# a line that no longer matches the cache is replaced by the cache's
# current one. Dropping it left that guest unknown to dnsmasq until
# its next start
+ my $current = $macs->{$parsed_mac};
if (
- !defined($macdb->{macs}->{$parsed_mac})
- || ($parsed_ip4
- && $macdb->{macs}->{$parsed_mac}->{'ip4'}
- && $macdb->{macs}->{$parsed_mac}->{'ip4'} ne $parsed_ip4)
- || ($parsed_ip6
- && $macdb->{macs}->{$parsed_mac}->{'ip6'}
- && $macdb->{macs}->{$parsed_mac}->{'ip6'} ne $parsed_ip6)
+ !$current
+ || ($parsed_ip4 && $current->{ip4} && $current->{ip4} ne $parsed_ip4)
+ || ($parsed_ip6 && $current->{ip6} && lc($current->{ip6}) ne $parsed_ip6)
) {
- $reload = 1;
- # the MAC being added gets its line below, like a missing one
- my $current = $macdb->{macs}->{$parsed_mac};
- next if $parsed_mac eq $mac || !$current;
+ $changed = 1;
+ next if !$current;
my $replacement = $parsed_mac;
$replacement .= ",$current->{ip4}" if $current->{ip4};
$replacement .= ",[$current->{ip6}]" if $current->{ip6};
- print $out "$replacement\n" if $replacement ne $parsed_mac;
+ print $out lc($replacement) . "\n";
next;
}
- if ($parsed_mac eq $mac) {
- $match = 1 if $ip4 && $parsed_ip4 && $ip4;
- $match = 1 if $ip6 && $parsed_ip6 && $ip6;
- }
-
print $out "$line\n";
}
- if (!$match) {
- my $reservation = $mac;
- $reservation .= ",$ip4" if $ip4;
- $reservation .= ",[$ip6]" if $ip6;
+ if (defined($reservation) && !$kept) {
print $out "$reservation\n";
- $reload = 1;
+ $changed = 1;
}
close $in;
@@ -136,16 +155,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..0b84a045d878 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,18 @@ $mocked_rpc_env_obj->mock(
check_any => sub { return 1; },
);
+my $mocked_pve_inotify = Test::MockModule->new('PVE::INotify');
+$mocked_pve_inotify->mock(
+ nodename => sub { return 'localnode'; },
+);
+
my $mocked_pve_cluster_obj = Test::MockModule->new('PVE::Cluster');
$mocked_pve_cluster_obj->mock(
check_cfs_quorum => sub { return 1; },
cfs_lock_domain => $mocked_cfs_lock_domain,
+ cfs_update => sub { },
+ get_vmlist => sub { return $test_state->{vmlist} // { ids => {} }; },
+ get_guest_config_properties => sub { return $test_state->{guest_nets} // {}; },
);
# ------- TEST FUNCTIONS --------------
@@ -303,6 +314,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 +389,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 +1164,174 @@ sub test_dnsmasq_stale_reservation_replaced {
};
PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$stale,10.0.1.10\n$gone,10.0.1.12\n");
- PVE::Network::SDN::Dhcp::Dnsmasq->add_ip_mapping(
- 'TESTZONE', $test_state->{macdb}, $added, '10.0.1.13', undef,
- );
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping('TESTZONE', $added);
- my @lines = sort grep { /\S/ } split(/\n/, PVE::Tools::file_get_contents($TMP_ETHERS_FILE));
- eq_or_diff(
- \@lines,
- ["$stale,10.0.1.11", "$added,10.0.1.13"],
+ is(
+ PVE::Tools::file_get_contents($TMP_ETHERS_FILE),
+ "$stale,10.0.1.11\n$added,10.0.1.13\n",
"$test_name: the stale line is replaced, the unknown one dropped",
);
}
run_test(\&test_dnsmasq_stale_reservation_replaced);
+# -------------- dnsmasq mapping pushes
+
+sub test_dnsmasq_mapping_push {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+ # the first push of a zone finds no ethers file yet
+ unlink($TMP_ETHERS_FILE);
+
+ my $ethers = sub { PVE::Tools::file_get_contents($TMP_ETHERS_FILE) };
+
+ eval { nic_start($vnetid, $mac, "999", "testhostname"); };
+ if ($@) {
+ fail("$test_name: nic_start: $@");
+ return;
+ }
+ is($ethers->(), "$mac,10.0.0.100\n", "$test_name: guest start reserves the allocation");
+
+ # the records exist already, so the restart pushes them without a change
+ $test_state->{dnsmasq_calls} = [];
+ nic_start($vnetid, $mac, "999", "testhostname");
+ is($ethers->(), "$mac,10.0.0.100\n", "$test_name: restart keeps the reservation");
+ is_deeply(
+ $test_state->{dnsmasq_calls},
+ [],
+ "$test_name: restart without a change does not reload",
+ );
+
+ update_ip({
+ zone => $zoneid,
+ vnet => $vnetid,
+ mac => $mac,
+ ip => "10.0.0.150",
+ });
+ is($ethers->(), "$mac,10.0.0.150\n", "$test_name: mapping edit replaces the reservation");
+
+ delete_ip({
+ zone => $zoneid,
+ vnet => $vnetid,
+ mac => $mac,
+ ip => "10.0.0.150",
+ });
+ is($ethers->(), "", "$test_name: mapping delete drops the reservation");
+
+ # a record without a MAC has no NIC to serve, so nothing is pushed for it
+ my @warnings;
+ {
+ local $SIG{__WARN__} = sub { push @warnings, @_; };
+ create_ip({ zone => $zoneid, vnet => $vnetid, ip => "10.0.0.160" });
+ delete_ip({ zone => $zoneid, vnet => $vnetid, ip => "10.0.0.160" });
+ }
+ eq_or_diff(\@warnings, [], "$test_name: a record without a MAC warns nothing");
+
+ # a reservation of a MAC the cache does not know is swept by the next
+ # rewrite, and both addresses of the guest go on one line
+ create_dhcp_subnet($vnetid, "8888::/64");
+ PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "aa:bb:cc:dd:ee:ff,10.0.0.77\n");
+ nic_start($vnetid, $mac, "999", "testhostname");
+ is(
+ $ethers->(),
+ "$mac,10.0.0.100,[8888::64]\n",
+ "$test_name: dual-stack reservation on one line, the unknown MAC swept",
+ );
+
+ # a zone confined to other nodes runs no dnsmasq here, so an edit is not
+ # pushed on this node
+ update_zone($zoneid, { nodes => 'other' });
+ create_ip({
+ zone => $zoneid,
+ vnet => $vnetid,
+ mac => $mac,
+ ip => "10.0.0.150",
+ });
+ is(
+ $ethers->(),
+ "$mac,10.0.0.100,[8888::64]\n",
+ "$test_name: a zone confined to other nodes gets no push here",
+ );
+}
+
+run_test(\&test_dnsmasq_mapping_push);
+
+sub test_failed_mapping_push {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+ $mocked_sdn_dhcp_dnsmasq->mock(update_ip_mapping => sub { die "rewrite failed\n"; });
+
+ my @warnings;
+ {
+ local $SIG{__WARN__} = sub { push @warnings, @_; };
+ eval { nic_start($vnetid, $mac, "999", "testhostname"); };
+ is($@, '', "$test_name: a guest start does not fail on its backend");
+ }
+ like(join('', @warnings), qr/rewrite failed/, "$test_name: the guest start warns");
+
+ eval { update_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.150" }); };
+ like(
+ $@,
+ qr/IPAM is updated for \Q$mac\E, but DHCP is not: rewrite failed/,
+ "$test_name: an API edit fails",
+ );
+ is(get_ip4(get_ips_from_mac($mac)), "10.0.0.150", "$test_name: the edit is saved");
+
+ $mocked_sdn_dhcp_dnsmasq->unmock('update_ip_mapping');
+}
+
+run_test(\&test_failed_mapping_push);
+
+sub test_mapping_edit_target {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+ # the guest behind a MAC is found by its NIC on the vnet, and the config
+ # writes the MAC in upper case
+ $test_state->{vmlist} = {
+ ids => {
+ 100 => { type => 'qemu', node => 'other' },
+ },
+ };
+ $test_state->{guest_nets} = {
+ 1 => { net0 => "virtio=$mac,bridge=$vnetid" },
+ 100 => { net0 => "virtio=" . uc($mac) . ",bridge=$vnetid" },
+ };
+ is(
+ PVE::Network::SDN::Dhcp::guest_node_by_mac($vnetid, $mac),
+ 'other',
+ "$test_name: the MAC resolves to the node of its guest, a guest the list lacks is skipped",
+ );
+
+ # a mapping edit through the API runs on the guest's node
+ my $proxyto = PVE::API2::Network::SDN::Ips->map_method_by_name('ipupdate')->{proxyto_callback};
+ is(
+ $proxyto->(undef, undef, { zone => $zoneid, vnet => $vnetid, mac => $mac }),
+ 'other',
+ "$test_name: a mapping edit runs on the guest's node",
+ );
+ is(
+ $proxyto->(undef, undef, { zone => $zoneid, vnet => $vnetid, mac => '00:11:22:33:44:66' }),
+ 'localhost',
+ "$test_name: one for a MAC without a guest runs here",
+ );
+ delete $test_state->{vmlist};
+ delete $test_state->{guest_nets};
+}
+
+run_test(\&test_mapping_edit_target);
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH pve-network v2 07/11] sdn: ipam: look a MAC up within the vnet's subnets
2026-11-03 15:15 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (5 preceding siblings ...)
2026-11-03 15:15 ` [PATCH pve-network v2 06/11] sdn: dhcp: update reservations when ip mappings change Hannes Laimer
@ 2026-11-03 15:15 ` Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 08/11] sdn: ipam: keep the MAC cache per vnet Hannes Laimer
` (5 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 15:15 UTC (permalink / raw)
To: pve-devel
A MAC only has to be unique within its vnet, so two vnets can carry
the same one. The IPAM lookup by MAC searched every subnet of the
zone, with NetBox the whole inventory, so a lookup for one vnet could
return the address the MAC has on another, and the guest then got no
address allocated on its own vnet. The built-in IPAM and NetBox are
now searched only within the subnets of the vnet the lookup is for.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams.pm | 4 +--
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 34 +++++++++++++---------
src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 7 ++---
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 2 +-
src/PVE/Network/SDN/Ipams/Plugin.pm | 3 +-
src/PVE/Network/SDN/Vnets.pm | 3 +-
src/test/run_test_vnets_blackbox.pl | 25 ++++++++++++++++
7 files changed, 55 insertions(+), 23 deletions(-)
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 1476c4236be0..9a31eaf39f58 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -169,7 +169,7 @@ sub complete_sdn_vnet {
}
sub get_ips_from_mac {
- my ($mac, $zoneid, $zone) = @_;
+ my ($mac, $zoneid, $zone, $subnets) = @_;
PVE::Cluster::cfs_update(); # the lock refreshes, so the snapshot must too
my $version = PVE::Cluster::cfs_file_version($macdb_filename);
@@ -181,7 +181,7 @@ sub get_ips_from_mac {
my $plugin_config = get_plugin_config($zone);
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
- my ($ip4, $ip6) = $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid);
+ my ($ip4, $ip6) = $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid, $subnets);
# an empty answer is not cached, since the record may simply not exist yet
return if !defined($ip4) && !defined($ip6);
diff --git a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
index e118d03e1eb0..9e1c9537e6ec 100644
--- a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
@@ -379,26 +379,32 @@ sub del_ip {
}
sub get_ips_from_mac {
- my ($class, $plugin_config, $mac, $zoneid, $noerr) = @_;
+ my ($class, $plugin_config, $mac, $zoneid, $subnets, $noerr) = @_;
my $ip4 = undef;
my $ip6 = undef;
- my $data = eval {
- netbox_api_request($plugin_config, "GET", "/ipam/ip-addresses/?description__ic=$mac");
- };
- if ($@) {
- return if $noerr;
- die "could not query ip address entry for mac $mac: $@";
- }
-
- for my $ip (@{ $data->{results} }) {
- if ($ip->{family}->{value} == 4 && !$ip4) {
- ($ip4, undef) = split(/\//, $ip->{address});
+ for my $cidr (sort map { $_->{cidr} } values %{ $subnets // {} }) {
+ my $data = eval {
+ netbox_api_request(
+ $plugin_config,
+ "GET",
+ "/ipam/ip-addresses/?parent=$cidr&description__ic=$mac",
+ );
+ };
+ if ($@) {
+ return if $noerr;
+ die "could not query IP address entry for MAC $mac in $cidr: $@";
}
- if ($ip->{family}->{value} == 6 && !$ip6) {
- ($ip6, undef) = split(/\//, $ip->{address});
+ for my $ip (@{ $data->{results} }) {
+ if ($ip->{family}->{value} == 4 && !$ip4) {
+ ($ip4, undef) = split(/\//, $ip->{address});
+ }
+
+ if ($ip->{family}->{value} == 6 && !$ip6) {
+ ($ip6, undef) = split(/\//, $ip->{address});
+ }
}
}
diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index 77a7b2a7c144..43d0a3daaf9a 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -279,7 +279,7 @@ sub del_ip {
}
sub get_ips_from_mac {
- my ($class, $plugin_config, $mac, $zoneid) = @_;
+ my ($class, $plugin_config, $mac, $zoneid, $subnets) = @_;
#just in case, as this should already be cached in local macs.db
@@ -289,13 +289,12 @@ sub get_ips_from_mac {
my $db = read_db();
die "zone $zoneid don't exist in ipam db" if !$db->{zones}->{$zoneid};
my $dbzone = $db->{zones}->{$zoneid};
- my $subnets = $dbzone->{subnets};
- for my $subnet (keys %$subnets) {
+ for my $subnet (sort map { $_->{cidr} } values %{ $subnets // {} }) {
# subnet keys are CIDRs, Net::IP only recognizes bare IPs
my ($network) = split(m|/|, $subnet);
next if Net::IP::ip_is_ipv4($network) ? $ip4 : $ip6;
- my $ips = $subnets->{$subnet}->{ips};
+ my $ips = $dbzone->{subnets}->{$subnet}->{ips} // next;
for my $ip (keys %$ips) {
my $ipobject = $ips->{$ip};
if ($ipobject->{mac} && $ipobject->{mac} eq $mac) {
diff --git a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
index 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 0b84a045d878..507bae29a84e 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -1334,4 +1334,29 @@ sub test_mapping_edit_target {
run_test(\&test_mapping_edit_target);
+sub test_ipam_per_vnet {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ # the same MAC on two vnets of one zone has an address on each, and a
+ # lookup for a vnet answers with that vnet's address only
+ create_zone({ type => "simple", dhcp => "dnsmasq", ipam => "pve", zone => $zoneid });
+ for my $n (1, 2) {
+ create_dhcp_vnet($zoneid, "vnet$n", "10.$n.0.0/24");
+ create_ip({ zone => $zoneid, vnet => "vnet$n", mac => $mac, ip => "10.$n.0.50" });
+ }
+
+ for my $n (1, 2) {
+ $test_state->{macdb} = {};
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac("vnet$n", $mac)],
+ ["10.$n.0.50", undef],
+ "$test_name: vnet$n answers with its own address",
+ );
+ }
+}
+
+run_test(\&test_ipam_per_vnet);
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH pve-network v2 08/11] sdn: ipam: keep the MAC cache per vnet
2026-11-03 15:15 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (6 preceding siblings ...)
2026-11-03 15:15 ` [PATCH pve-network v2 07/11] sdn: ipam: look a MAC up within the vnet's subnets Hannes Laimer
@ 2026-11-03 15:15 ` Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 09/11] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM Hannes Laimer
` (4 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 15:15 UTC (permalink / raw)
To: pve-devel
The same MAC can sit on several vnets, but the cache of its addresses
was keyed by the MAC alone. A mapping created or edited on one vnet
changed the cached answer on every other vnet as well. A guest with
that MAC on another vnet then got a reservation for an address
outside its own vnet.
The cache now holds the addresses per vnet, and a zone's dnsmasq
reservations hold a line for each vnet a MAC sits on, each written
from the cache of that vnet. A MAC matches in any spelling.
The map under the MAC alone stays, since nodes not yet upgraded read
and write only it. Every write lands there as well, one address per
MAC as such a node expects, and its entries answer for a vnet only
with addresses in its subnets, so a guest from before keeps its
reservation across the upgrade. A release or an edit on such a node
shows only in that map, so a vnet's address holds only while the map
still names it, otherwise the IPAM is asked again. A release checks
that the record still belongs to its MAC, leaves it and its DNS
records alone otherwise, and fails when it cannot check. The API
refuses to delete such a mapping.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/API2/Network/SDN/Ips.pm | 10 +-
src/PVE/Network/SDN/Dhcp.pm | 2 +-
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 143 +++++-----
src/PVE/Network/SDN/Dhcp/Plugin.pm | 7 +-
src/PVE/Network/SDN/Ipams.pm | 188 ++++++++----
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 15 +-
src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 13 +-
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 20 +-
src/PVE/Network/SDN/Ipams/Plugin.pm | 4 +-
src/PVE/Network/SDN/Subnets.pm | 14 +-
src/PVE/Network/SDN/Vnets.pm | 10 +-
src/test/run_test_ipams.pl | 49 ++++
src/test/run_test_vnets_blackbox.pl | 316 ++++++++++++++++++---
13 files changed, 613 insertions(+), 178 deletions(-)
diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
index 7653a126eb64..72871856362e 100644
--- a/src/PVE/API2/Network/SDN/Ips.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -74,11 +74,14 @@ __PACKAGE__->register_method({
my $mac = extract_param($param, 'mac');
my $ip = extract_param($param, 'ip');
- eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
+ my $released = eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
die "$@\n" if $@;
PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+ # the cache entry is cleared, the IPAM record belongs to another MAC
+ die "IP '$ip' is not mapped to $mac\n" if defined($released) && !$released;
+
return undef;
},
});
@@ -183,12 +186,13 @@ __PACKAGE__->register_method({
my ($old_ip4, $old_ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnet, $mac);
my $old_ip = (Net::IP::ip_get_version($ip) == 4) ? $old_ip4 : $old_ip6;
- PVE::Network::SDN::Vnets::del_ip($vnet, $old_ip, '', $mac);
+ # a MAC without an address in that family has nothing to release
+ PVE::Network::SDN::Vnets::del_ip($vnet, $old_ip, '', $mac) if defined($old_ip);
eval { PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, $vmid); };
my $error = $@;
- if ($error) {
+ if ($error && defined($old_ip)) {
PVE::Network::SDN::Vnets::add_ip($vnet, $old_ip, '', $mac, $vmid);
}
diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
index 964e3fd2f1d3..2e9dd31dcb6f 100644
--- a/src/PVE/Network/SDN/Dhcp.pm
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -41,7 +41,7 @@ sub update_mapping {
return if defined($zone->{nodes}) && !$zone->{nodes}->{ PVE::INotify::nodename() };
PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp})
- ->update_ip_mapping($zoneid, $mac);
+ ->update_ip_mapping($zoneid, $vnetid, $mac);
};
# the records are written by then and stay
die "IPAM is updated for $mac, but DHCP is not: $@" if $@;
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index 9cd16020887e..f3b8dab3dcf4 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -13,7 +13,6 @@ use Net::DBus;
use PVE::Cluster;
use PVE::RESTEnvironment qw(log_warn);
-use PVE::Network::SDN::Ipams;
my $DNSMASQ_CONFIG_ROOT = '/etc/dnsmasq.d';
my $DNSMASQ_DEFAULT_ROOT = '/etc/default';
@@ -54,98 +53,104 @@ sub update_lease {
}
sub update_ip_mapping {
- my ($class, $dhcpid, $mac) = @_;
+ my ($class, $dhcpid, $vnetid, $mac) = @_;
$mac = lc($mac);
my $ethers_file = ethers_file($dhcpid);
my $ethers_tmp_file = "$ethers_file.tmp";
- my ($ip4, $ip6);
- my $reservation = undef;
+ my $ip4;
my $changed = undef;
- my $rewriteFn = sub {
- # the lines are decided from the cache, which this process may hold an
- # old read of, and the local lock does not refresh it
+ my $rewrite_fn = sub {
+ # the lines are decided from the cache and the applied config, which
+ # the local lock does not refresh
PVE::Cluster::cfs_update();
- # read under the ethers lock. A snapshot from before it would sweep a
- # line another push wrote meanwhile, and an older push would put its
- # addresses back
-
- my $macs = PVE::Network::SDN::Ipams::read_macdb()->{macs};
-
- my $entry = $macs->{$mac};
- ($ip4, $ip6) = ($entry->{ip4}, $entry->{ip6}) if $entry;
- if ($ip4 || $ip6) {
- $reservation = $mac;
- $reservation .= ",$ip4" if $ip4;
- $reservation .= ",[$ip6]" if $ip6;
+ # the file holds a line per vnet a MAC sits on, and a line belongs to
+ # the vnet of the applied subnet that holds its address
+ my $vnets = PVE::Network::SDN::Zones::get_vnets($dhcpid, 1) // {};
+ my $subnets = PVE::Network::SDN::Vnets::get_subnets(undef, 1) // {};
+ $subnets = {
+ map { $_ => $subnets->{$_} } grep { $vnets->{ $subnets->{$_}->{vnet} } }
+ keys %$subnets
+ };
+ my $vnet_of = sub {
+ my ($ip) = @_;
+ my (undef, $subnet) =
+ eval { PVE::Network::SDN::Subnets::find_ip_subnet($ip, $subnets) };
+ return $subnet ? $subnet->{vnet} : undef;
+ };
+ my %subnets_of;
+ $subnets_of{ $subnets->{$_}->{vnet} }->{$_} = $subnets->{$_} for keys %$subnets;
+
+ # the changed MAC is looked up first, under the ethers lock so that two
+ # edits of one MAC cannot land out of order, and its lines come from
+ # that answer. The other lines are checked against one read of the
+ # cache, taken after the lookup's fill, so only the changed MAC asks
+ # the IPAM
+ my $cache;
+ my $reservation = sub {
+ my ($vnet, $line_mac, $cached_only) = @_;
+ my ($line_ip4, $line_ip6) =
+ $cached_only
+ ? PVE::Network::SDN::Ipams::cached_ips_from_mac(
+ $cache,
+ $vnet,
+ $line_mac,
+ $subnets_of{$vnet} // {},
+ )
+ : PVE::Network::SDN::Vnets::get_ips_from_mac($vnet, $line_mac, 1);
+ return (undef, undef) if !$line_ip4 && !$line_ip6;
+ my $line = $line_mac;
+ $line .= ",$line_ip4" if $line_ip4;
+ $line .= ",[$line_ip6]" if $line_ip6;
# an external IPAM spells addresses its own way, while the file holds one spelling
- $reservation = lc($reservation);
- }
+ return (lc($line), $line_ip4);
+ };
+ my $own;
+ ($own, $ip4) = $reservation->($vnetid, $mac);
+ $cache = PVE::Network::SDN::Ipams::read_cache();
file_set_contents($ethers_file, '') if !-e $ethers_file;
open(my $in, '<', $ethers_file) or die "Could not open file '$ethers_file' $!\n";
open(my $out, '>', $ethers_tmp_file)
or die "Could not open file '$ethers_tmp_file' $!\n";
- my $kept = undef;
-
+ my %written;
while (my $line = <$in>) {
chomp($line);
next if $line !~ m/\S/;
- my ($parsed_mac, $parsed_ip1, $parsed_ip2) = split(/,/, $line);
- # a line from before the file was written in lower case keeps its writer's spelling
+ my ($parsed_mac, $parsed_ip) = split(/,/, $line);
$parsed_mac = lc($parsed_mac);
-
- # the MAC's own reservation is rewritten. An unchanged one stays in place
- if ($parsed_mac eq $mac) {
- if (defined($reservation) && !$kept && lc($line) eq $reservation) {
- $kept = 1;
- print $out "$line\n";
- } else {
- $changed = 1;
- }
- next;
- }
-
- my ($parsed_ip4, $parsed_ip6);
- if ($parsed_ip2) {
- $parsed_ip4 = $parsed_ip1;
- $parsed_ip6 = $parsed_ip2;
- } elsif (defined($parsed_ip1) && Net::IP::ip_is_ipv4($parsed_ip1)) {
- $parsed_ip4 = $parsed_ip1;
- } elsif (defined($parsed_ip1)) {
- $parsed_ip6 = $parsed_ip1;
- }
- $parsed_ip6 = $1 if $parsed_ip6 && $parsed_ip6 =~ m/\[(\S+)\]/;
- # a line from before addresses were lower-cased may spell one in upper case
- $parsed_ip6 = lc($parsed_ip6) if $parsed_ip6;
-
- # a line that no longer matches the cache is replaced by the cache's
- # current one. Dropping it left that guest unknown to dnsmasq until
- # its next start
- my $current = $macs->{$parsed_mac};
- if (
- !$current
- || ($parsed_ip4 && $current->{ip4} && $current->{ip4} ne $parsed_ip4)
- || ($parsed_ip6 && $current->{ip6} && lc($current->{ip6}) ne $parsed_ip6)
- ) {
+ $parsed_ip = $1 if defined($parsed_ip) && $parsed_ip =~ m/\[(\S+)\]/;
+ my $vnet = defined($parsed_ip) ? $vnet_of->($parsed_ip) : undef;
+ my $key = defined($vnet) ? "$parsed_mac,$vnet" : undef;
+
+ # a line that no longer matches is replaced, since dropping it left
+ # that guest unknown to dnsmasq until its next start
+ my ($want) = eval {
+ !$key || $written{$key} ? ()
+ : $key eq "$mac,$vnetid" ? ($own)
+ : $reservation->($vnet, $parsed_mac, 1);
+ };
+ if ($@) {
+ warn "could not read the reservation of $parsed_mac: $@";
+ print $out "$line\n";
+ } elsif (!defined($want)) {
$changed = 1;
- next if !$current;
- my $replacement = $parsed_mac;
- $replacement .= ",$current->{ip4}" if $current->{ip4};
- $replacement .= ",[$current->{ip6}]" if $current->{ip6};
- print $out lc($replacement) . "\n";
next;
+ } elsif (lc($line) ne $want) {
+ $changed = 1;
+ print $out "$want\n";
+ } else {
+ print $out "$line\n";
}
-
- print $out "$line\n";
+ $written{$key}++;
}
- if (defined($reservation) && !$kept) {
- print $out "$reservation\n";
+ if (defined($own) && !$written{"$mac,$vnetid"}) {
+ print $out "$own\n";
$changed = 1;
}
@@ -157,7 +162,7 @@ sub update_ip_mapping {
# the rewrite replaces the reservation file, so a lock on the file itself
# would not outlive the rename
- PVE::Tools::lock_file("$ethers_file.lock", 10, $rewriteFn);
+ PVE::Tools::lock_file("$ethers_file.lock", 10, $rewrite_fn);
die "Unable to update $mac in the dnsmasq configuration: $@" if $@;
systemctl_service('reload', "dnsmasq\@$dhcpid") if $changed;
diff --git a/src/PVE/Network/SDN/Dhcp/Plugin.pm b/src/PVE/Network/SDN/Dhcp/Plugin.pm
index 472ff1534743..9ee9a442cafd 100644
--- a/src/PVE/Network/SDN/Dhcp/Plugin.pm
+++ b/src/PVE/Network/SDN/Dhcp/Plugin.pm
@@ -22,10 +22,11 @@ sub private {
return $defaultData;
}
-# the MAC's records changed, so make the backend serve the current ones. With no
-# record left its reservation goes. Dies if the backend could not be updated
+# the MAC's records on the vnet changed, so make the backend serve the current
+# ones. With no record left its reservation on the vnet goes. Dies if the
+# backend could not be updated
sub update_ip_mapping {
- my ($class, $dhcpid, $mac) = @_;
+ my ($class, $dhcpid, $vnetid, $mac) = @_;
die 'implement in sub class';
}
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 9a31eaf39f58..a6082438d148 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -35,40 +35,10 @@ sub json_writer {
return encode_json($data);
}
-# Writers spell a MAC their own way, the generated ones in upper case and the
-# API in either, and nodes not yet upgraded look an entry up under the spelling
-# their caller hands them, so the file keeps its keys as written. Readers get a
-# view with every MAC in lower case. Two spellings that disagree on an address
-# are left out of it, since one of them is stale and nothing here says which,
-# and so is an entry without an address. The next lookup then asks the IPAM
-my sub fold_macdb {
- my ($db) = @_;
-
- my $macs = {};
- my %clash;
- for my $key (sort keys %{ $db->{macs} // {} }) {
- my $entry = $db->{macs}->{$key};
- for my $family (grep { defined($entry->{$_}) } qw(ip4 ip6)) {
- my $seen = $macs->{ lc($key) }->{$family};
- $clash{ lc($key) } = 1 if defined($seen) && lc($seen) ne lc($entry->{$family});
- $macs->{ lc($key) }->{$family} //= $entry->{$family};
- }
- }
- delete $macs->{$_} for keys %clash;
- return { %$db, macs => $macs };
-}
-
-# the file as written, which the writers change and write back
-sub read_macdb_file {
- my () = @_;
-
- return cfs_read_file($macdb_filename);
-}
-
sub read_macdb {
my () = @_;
- return fold_macdb(read_macdb_file());
+ return cfs_read_file($macdb_filename);
}
sub write_macdb {
@@ -77,9 +47,41 @@ sub write_macdb {
cfs_write_file($macdb_filename, $data);
}
-# a write goes to every spelling the file holds for the MAC, so each reader
-# finds it under its own, and a MAC new to the file is written in lower case
-my sub set_cache_entry {
+# the map under the MAC alone is the one nodes not yet upgraded read and
+# write, under the MAC as their caller spelled it, so one read indexes it by
+# the MAC in lower case for every lookup on that read. Spellings that
+# disagree on an address are both left out, since one of them is stale
+my sub old_index {
+ my ($db) = @_;
+
+ my (%index, %clash);
+ my $macs = $db->{macs} // {};
+ for my $key (sort keys %$macs) {
+ my $mac = lc($key);
+ next if $clash{$mac};
+ my $entry = $index{$mac} //= {};
+ for my $family (qw(ip4 ip6)) {
+ my $ip = $macs->{$key}->{$family} // next;
+ if (defined($entry->{$family}) && lc($entry->{$family}) ne lc($ip)) {
+ $clash{$mac} = 1;
+ last;
+ }
+ $entry->{$family} = $ip;
+ }
+ }
+ delete $index{$_} for keys %clash;
+ return \%index;
+}
+
+sub read_cache {
+ my $db = read_macdb();
+ return { db => $db, old => old_index($db) };
+}
+
+# a write lands in the map under the MAC alone as well, under the keys it
+# already has, with one address per MAC as a node not yet upgraded reads
+# and writes it
+my sub set_old_cache_entry {
my ($db, $mac, $family, $ip) = @_;
my $macs = $db->{macs} //= {};
@@ -87,15 +89,71 @@ my sub set_cache_entry {
$macs->{$_}->{$family} = $ip for (@keys ? @keys : (lc($mac)));
}
+my sub in_subnets {
+ my ($ip, $subnets) = @_;
+ return !!eval { PVE::Network::SDN::Subnets::find_ip_subnet($ip, $subnets) };
+}
+
+# the addresses the per-vnet entries hold for the MAC. A write puts its
+# address into the map under the MAC alone as well, so a value found in both
+# is some vnet's and not an entry from before
+my sub claimed_addresses {
+ my ($db, $mac) = @_;
+
+ my %claimed;
+ for my $vnet_entries (values %{ $db->{vnets} // {} }) {
+ my $entry = $vnet_entries->{ lc($mac) } // next;
+ $claimed{ lc($_) } = 1 for grep { defined($_) } @{$entry}{qw(ip4 ip6)};
+ }
+ return \%claimed;
+}
+
+# the vnet's entry answers, and an entry from before the cache was kept per
+# vnet fills the families it lacks with addresses in the vnet's subnets
+my sub cache_entry {
+ my ($cache, $mac, $vnetid, $subnets) = @_;
+
+ my $entry = (($cache->{db}->{vnets} // {})->{$vnetid} // {})->{ lc($mac) } // {};
+ my $old = $cache->{old}->{ lc($mac) } // {};
+ my $claimed = claimed_addresses($cache->{db}, $mac);
+ my $cached = {};
+ for my $family (qw(ip4 ip6)) {
+ my $ip = $entry->{$family};
+ $ip = $old->{$family}
+ if !defined($ip)
+ && defined($old->{$family})
+ && !$claimed->{ lc($old->{$family}) }
+ && in_subnets($old->{$family}, $subnets);
+ $cached->{$family} = $ip if defined($ip);
+ }
+ return %$cached ? $cached : undef;
+}
+
+# a node not yet upgraded releases or moves an address in the map under the
+# MAC alone, so a vnet's address holds only while that map still names it
+my sub witnessed {
+ my ($cache, $mac, $vnetid) = @_;
+
+ my $entry = (($cache->{db}->{vnets} // {})->{$vnetid} // {})->{ lc($mac) } // {};
+ my $old = $cache->{old}->{ lc($mac) } // {};
+ for my $family (grep { defined($entry->{$_}) } qw(ip4 ip6)) {
+ return 0 if lc($old->{$family} // '') ne lc($entry->{$family});
+ }
+ return 1;
+}
+
sub add_cache_mac_ip {
- my ($mac, $ip) = @_;
+ my ($vnetid, $mac, $ip) = @_;
+ $mac = lc($mac);
+ my $family = Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6';
cfs_lock_file(
$macdb_filename,
undef,
sub {
- my $db = read_macdb_file();
- set_cache_entry($db, $mac, Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6', $ip);
+ my $db = read_macdb();
+ $db->{vnets}->{$vnetid}->{$mac}->{$family} = $ip;
+ set_old_cache_entry($db, $mac, $family, $ip);
write_macdb($db);
},
);
@@ -103,19 +161,29 @@ sub add_cache_mac_ip {
}
sub del_cache_mac_ip {
- my ($mac, $ip) = @_;
+ my ($vnetid, $mac, $ip) = @_;
+ $mac = lc($mac);
+ my $family = Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6';
cfs_lock_file(
$macdb_filename,
undef,
sub {
- my $db = read_macdb_file();
- my $family = Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6';
+ my $db = read_macdb();
+ my $vnet_entries = ($db->{vnets} // {})->{$vnetid};
+ if (my $entry = $vnet_entries ? $vnet_entries->{$mac} : undef) {
+ delete $entry->{$family};
+ delete $vnet_entries->{$mac} if !%$entry;
+ delete $db->{vnets}->{$vnetid} if !%$vnet_entries;
+ delete $db->{vnets} if !%{ $db->{vnets} };
+ }
+ # the map under the MAC alone must not hand a released address back
my $macs = $db->{macs} // {};
- for my $key (grep { lc($_) eq lc($mac) } keys %$macs) {
- delete $macs->{$key}->{$family};
- delete $macs->{$key}
- if !defined($macs->{$key}->{ip4}) && !defined($macs->{$key}->{ip6});
+ for my $key (grep { lc($_) eq $mac } keys %$macs) {
+ my $old = $macs->{$key};
+ next if lc($old->{$family} // '') ne lc($ip);
+ delete $old->{$family};
+ delete $macs->{$key} if !defined($old->{ip4}) && !defined($old->{ip6});
}
write_macdb($db);
},
@@ -168,16 +236,22 @@ sub complete_sdn_vnet {
return $cmdname eq 'add' ? [] : [PVE::Network::SDN::Vnets::sdn_ipams_ids($cfg)];
}
+# the addresses one read of the cache holds for a MAC on a vnet
+sub cached_ips_from_mac {
+ my ($cache, $vnetid, $mac, $subnets) = @_;
+
+ my $cached = cache_entry($cache, $mac, $vnetid, $subnets);
+ return $cached ? ($cached->{ip4}, $cached->{ip6}) : ();
+}
+
sub get_ips_from_mac {
- my ($mac, $zoneid, $zone, $subnets) = @_;
+ my ($vnetid, $mac, $zoneid, $zone, $subnets) = @_;
PVE::Cluster::cfs_update(); # the lock refreshes, so the snapshot must too
my $version = PVE::Cluster::cfs_file_version($macdb_filename);
- my $cached = read_macdb()->{macs}->{ lc($mac) };
- # an entry without an address was left behind by a lookup that missed, and
- # says nothing about its MAC
- return ($cached->{ip4}, $cached->{ip6})
- if $cached && (defined($cached->{ip4}) || defined($cached->{ip6}));
+ my $cache = read_cache();
+ my $cached = cache_entry($cache, $mac, $vnetid, $subnets);
+ return ($cached->{ip4}, $cached->{ip6}) if $cached && witnessed($cache, $mac, $vnetid);
my $plugin_config = get_plugin_config($zone);
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
@@ -195,9 +269,17 @@ sub get_ips_from_mac {
return
if (PVE::Cluster::cfs_file_version($macdb_filename) // '') ne ($version // '');
- my $db = read_macdb_file();
- set_cache_entry($db, $mac, 'ip4', $ip4) if defined($ip4);
- set_cache_entry($db, $mac, 'ip6', $ip6) if defined($ip6);
+ my $db = read_macdb();
+ my $entry = $db->{vnets}->{$vnetid}->{ lc($mac) } //= {};
+ for my $family (qw(ip4 ip6)) {
+ my $ip = $family eq 'ip4' ? $ip4 : $ip6;
+ if (!defined($ip)) {
+ delete $entry->{$family};
+ next;
+ }
+ $entry->{$family} = $ip;
+ set_old_cache_entry($db, $mac, $family, $ip);
+ }
write_macdb($db);
},
);
diff --git a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
index 9e1c9537e6ec..84ce4382e0e7 100644
--- a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
@@ -360,7 +360,7 @@ sub add_range_next_freeip {
}
sub del_ip {
- my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr) = @_;
+ my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr, $mac) = @_;
return if !$ip;
@@ -369,6 +369,19 @@ sub del_ip {
warn "could not find id for ip $ip";
return;
}
+ if ($mac) {
+ my $record =
+ eval { netbox_api_request($plugin_config, "GET", "/ipam/ip-addresses/$ip_id/") };
+ if ($@) {
+ return 0 if $noerr;
+ die "could not check who holds IP $ip: $@";
+ }
+ my ($owner) = ($record->{description} // '') =~ m/^mac:(\S+)$/;
+ if ($owner && lc($owner) ne lc($mac)) {
+ warn "not releasing IP '$ip' for $mac, it is held by $owner\n";
+ return 0;
+ }
+ }
eval { netbox_api_request($plugin_config, "DELETE", "/ipam/ip-addresses/$ip_id/"); };
if ($@) {
diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index 43d0a3daaf9a..86e594a4c604 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -249,11 +249,12 @@ sub add_range_next_freeip {
}
sub del_ip {
- my ($class, $plugin_config, $subnetid, $subnet, $ip) = @_;
+ my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr, $mac) = @_;
my $cidr = $subnet->{cidr};
my $zone = $subnet->{zone};
+ my $released = 1;
cfs_lock_file(
$ipamdb_file,
undef,
@@ -271,11 +272,18 @@ sub del_ip {
warn "IP '$ip' does not exist in IPAM DB\n";
return;
}
+ my $owner = $dbsubnet->{ips}->{$ip}->{mac};
+ if ($mac && $owner && lc($owner) ne lc($mac)) {
+ warn "not releasing IP '$ip' for $mac, it is held by $owner\n";
+ $released = 0;
+ return;
+ }
delete $dbsubnet->{ips}->{$ip};
write_db($db);
},
);
die "$@" if $@;
+ return $released;
}
sub get_ips_from_mac {
@@ -297,7 +305,8 @@ sub get_ips_from_mac {
my $ips = $dbzone->{subnets}->{$subnet}->{ips} // next;
for my $ip (keys %$ips) {
my $ipobject = $ips->{$ip};
- if ($ipobject->{mac} && $ipobject->{mac} eq $mac) {
+ # a record keeps the MAC as its writer spelled it
+ if ($ipobject->{mac} && lc($ipobject->{mac}) eq lc($mac)) {
if (Net::IP::ip_is_ipv4($ip)) {
$ip4 = $ip;
} else {
diff --git a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
index 12d470e802c4..e969c3dcdf4f 100644
--- a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
@@ -239,7 +239,7 @@ sub add_range_next_freeip {
}
sub del_ip {
- my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr) = @_;
+ my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr, $mac) = @_;
return if !$ip;
@@ -250,6 +250,22 @@ sub del_ip {
my $ip_id = get_ip_id($url, $ip, $headers);
return if !$ip_id;
+ if ($mac) {
+ my $record = eval {
+ PVE::Network::SDN::api_request(
+ "GET", "$url/addresses/$ip_id/", $headers, undef, $fingerprint,
+ );
+ };
+ if ($@) {
+ return 0 if $noerr;
+ die "could not check who holds IP $ip: $@";
+ }
+ my $owner = ($record->{data} // {})->{mac};
+ if ($owner && lc($owner) ne lc($mac)) {
+ warn "not releasing IP '$ip' for $mac, it is held by $owner\n";
+ return 0;
+ }
+ }
eval {
PVE::Network::SDN::api_request(
@@ -259,6 +275,8 @@ sub del_ip {
if ($@) {
die "error delete ip $ip: $@" if !$noerr;
}
+
+ return 1;
}
sub get_ips_from_mac {
diff --git a/src/PVE/Network/SDN/Ipams/Plugin.pm b/src/PVE/Network/SDN/Ipams/Plugin.pm
index 14f8fe9e67fe..6e23d5434d4b 100644
--- a/src/PVE/Network/SDN/Ipams/Plugin.pm
+++ b/src/PVE/Network/SDN/Ipams/Plugin.pm
@@ -132,8 +132,10 @@ sub add_range_next_freeip {
die "please implement inside plugin";
}
+# a MAC names the guest the address is released for, a record holding the
+# address for another MAC is then left alone and 0 returned
sub del_ip {
- my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr) = @_;
+ my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr, $mac) = @_;
die "please implement inside plugin";
}
diff --git a/src/PVE/Network/SDN/Subnets.pm b/src/PVE/Network/SDN/Subnets.pm
index f8be98201476..a7fa7056e556 100644
--- a/src/PVE/Network/SDN/Subnets.pm
+++ b/src/PVE/Network/SDN/Subnets.pm
@@ -266,7 +266,7 @@ sub add_next_free_ip {
die $@ if $@;
- eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, $ip); };
+ eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($subnet->{vnet}, $mac, $ip); };
warn $@ if $@;
}
@@ -325,7 +325,7 @@ sub add_ip {
};
die $@ if $@;
- eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, $ip) if $mac; };
+ eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($subnet->{vnet}, $mac, $ip) if $mac; };
warn $@ if $@;
}
@@ -413,18 +413,22 @@ sub del_ip {
verify_dns_zone($reversednszone, $reversedns);
}
+ my $released;
if ($ipamid) {
my $ipam_cfg = PVE::Network::SDN::Ipams::config();
my $plugin_config = $ipam_cfg->{ids}->{$ipamid};
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
- $plugin->del_ip($plugin_config, $subnetid, $subnet, $ip);
+ $released = $plugin->del_ip($plugin_config, $subnetid, $subnet, $ip, undef, $mac);
if ($mac) {
- eval { PVE::Network::SDN::Ipams::del_cache_mac_ip($mac, $ip) };
+ eval { PVE::Network::SDN::Ipams::del_cache_mac_ip($subnet->{vnet}, $mac, $ip) };
warn $@ if $@;
}
}
+ # a record another MAC holds keeps its DNS entries as well
+ return 0 if defined($released) && !$released;
+
eval {
if (!$skipdns) {
del_dns_record($dnszone, $dns, $hostname, $ip);
@@ -434,6 +438,8 @@ sub del_ip {
if ($@) {
warn $@;
}
+
+ return $released;
}
=head3 api_get_subnet($cfg, $id)
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index 395aa32b6a84..58db0f803f8b 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -194,18 +194,18 @@ sub del_ip {
}
sub get_ips_from_mac {
- my ($vnetid, $mac) = @_;
+ my ($vnetid, $mac, $running) = @_;
- my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid);
+ my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid, $running);
return if !$vnet;
my $zoneid = $vnet->{zone};
- my $zone = PVE::Network::SDN::Zones::get_zone($zoneid);
+ my $zone = PVE::Network::SDN::Zones::get_zone($zoneid, $running);
return if !$zone->{ipam} || !$zone->{dhcp};
- my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid);
- return PVE::Network::SDN::Ipams::get_ips_from_mac($mac, $zoneid, $zone, $subnets);
+ my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid, $running);
+ return PVE::Network::SDN::Ipams::get_ips_from_mac($vnetid, $mac, $zoneid, $zone, $subnets);
}
sub del_ips_from_mac {
diff --git a/src/test/run_test_ipams.pl b/src/test/run_test_ipams.pl
index f9c44fe1a3b9..f07b4bb4b486 100755
--- a/src/test/run_test_ipams.pl
+++ b/src/test/run_test_ipams.pl
@@ -219,5 +219,54 @@ foreach my $path (@plugins) {
}
+# an address released for a MAC stays when the external IPAM records it
+# for another MAC
+{
+ my $subnet = { zone => 'z', cidr => '10.0.0.0/24' };
+ my $owner;
+ my $check_fails = 0;
+ my @deletes;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url) = @_;
+ die "Invalid response from server: 503 Service Unavailable\n"
+ if $check_fails && $method eq 'GET';
+ push @deletes, $url if $method eq 'DELETE';
+ return {
+ description => "mac:$owner",
+ data => { mac => $owner },
+ };
+ },
+ );
+ my $netbox = Test::MockModule->new('PVE::Network::SDN::Ipams::NetboxPlugin');
+ $netbox->mock(subnet_prefix => sub { return (1, 2); }, get_ip_id => sub { return 3; });
+ my $phpipam = Test::MockModule->new('PVE::Network::SDN::Ipams::PhpIpamPlugin');
+ $phpipam->mock(subnet_id => sub { return 7; }, get_ip_id => sub { return 3; });
+
+ for my $plugin (
+ 'PVE::Network::SDN::Ipams::NetboxPlugin',
+ 'PVE::Network::SDN::Ipams::PhpIpamPlugin',
+ ) {
+ my $config = { url => 'u', token => 't' };
+ @deletes = ();
+ $owner = 'da:65:8f:18:9b:70';
+ $plugin->del_ip($config, 'z-10.0.0.0-24', $subnet, '10.0.0.50', 1, 'DA:65:8F:18:9B:6F');
+ is(scalar(@deletes), 0, "$plugin keeps the record of another MAC");
+ $owner = 'da:65:8f:18:9b:6f';
+ $plugin->del_ip($config, 'z-10.0.0.0-24', $subnet, '10.0.0.50', 1, 'DA:65:8F:18:9B:6F');
+ is(scalar(@deletes), 1, "$plugin releases the MAC's own record");
+
+ # a failed check of the holder releases nothing either
+ @deletes = ();
+ $check_fails = 1;
+ my $released =
+ $plugin->del_ip($config, 'z-10.0.0.0-24', $subnet, '10.0.0.50', 1, 'DA:65:8F:18:9B:6F');
+ $check_fails = 0;
+ is($released, 0, "$plugin releases nothing when the check of the holder fails");
+ is(scalar(@deletes), 0, "$plugin sent no delete for it");
+ }
+}
+
done_testing();
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 507bae29a84e..28bf7d30e670 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -195,7 +195,7 @@ $mocked_sdn_ipams->mock(
my ($cfg) = @_;
$test_state->{ipam_config} = $cfg;
},
- read_macdb_file => sub {
+ read_macdb => sub {
return PVE::Network::SDN::Ipams::json_reader(undef, encode_json($test_state->{macdb}));
},
write_macdb => sub {
@@ -505,74 +505,101 @@ sub test_ipam_cache_lookup {
# an entry holding no address, left behind by an earlier miss, does not
# hide a record created since
create_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.50" });
- $test_state->{macdb}->{macs}->{$mac} = { ip4 => undef, ip6 => undef };
+ $test_state->{macdb} = { macs => { $mac => { ip4 => undef, ip6 => undef } } };
my @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
is($ips[0], "10.0.0.50", "$test_name: an address-less entry counts as a miss");
- is(
- $test_state->{macdb}->{macs}->{$mac}->{ip4},
- "10.0.0.50",
- "$test_name: the answer is cached",
+ is_deeply(
+ $test_state->{macdb},
+ {
+ macs => { $mac => { ip4 => "10.0.0.50", ip6 => undef } },
+ vnets => { $vnetid => { $mac => { ip4 => "10.0.0.50" } } },
+ },
+ "$test_name: the answer is cached in both maps",
);
# a write to the cache between the miss and the fill, which the file's
# version shows, keeps the IPAM's answer out of the cache
- delete $test_state->{macdb}->{macs}->{$mac};
+ $test_state->{macdb} = {};
my $version = 0;
$mocked_pve_cluster_obj->mock(cfs_file_version => sub { return $version++ });
@ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
$mocked_pve_cluster_obj->unmock('cfs_file_version');
is($ips[0], "10.0.0.50", "$test_name: the lookup still answers");
- ok(!exists $test_state->{macdb}->{macs}->{$mac},
- "$test_name: but leaves a changed cache alone");
+ is_deeply($test_state->{macdb}, {}, "$test_name: but leaves a changed cache alone");
- # an entry written under the config's spelling, as a node not yet upgraded
- # writes it, is read and released like any other and keeps its key
+ # an entry written under the config's spelling before the cache was keyed in
+ # lower case is read and released like any other
my $other = "da:65:8f:18:9b:70";
$test_state->{macdb}->{macs} = {
uc($mac) => { ip4 => '10.0.0.150' },
uc($other) => { ip4 => '10.0.0.170' },
};
my ($ip4) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
- is($ip4, '10.0.0.150', "$test_name: an entry is found however it is spelled");
- PVE::Network::SDN::Ipams::del_cache_mac_ip($mac, '10.0.0.150');
+ is($ip4, '10.0.0.150', "$test_name: an old entry is found however it is spelled");
+ PVE::Network::SDN::Ipams::del_cache_mac_ip($vnetid, $mac, '10.0.0.150');
is_deeply(
$test_state->{macdb}->{macs},
{ uc($other) => { ip4 => '10.0.0.170' } },
"$test_name: and released, the other entry left as written",
);
- # a write in another spelling goes to the spelling the file holds
+ # a write by this code updates an old entry under the spelling a node not
+ # yet upgraded wrote it in, so that node still finds it
$test_state->{macdb}->{macs} = { uc($mac) => { ip4 => '10.0.0.150' } };
- PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, 'fd00::150');
+ PVE::Network::SDN::Ipams::add_cache_mac_ip($vnetid, uc($mac), 'fd00::150');
is_deeply(
$test_state->{macdb}->{macs},
{ uc($mac) => { ip4 => '10.0.0.150', ip6 => 'fd00::150' } },
- "$test_name: a write keeps the key as written",
+ "$test_name: a write updates the old entry under its spelling",
+ );
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac)],
+ ['10.0.0.150', 'fd00::150'],
+ "$test_name: holding both addresses",
);
- # a lookup past an address-less entry asks the IPAM and fills the entry
- # under its spelling
- $test_state->{macdb}->{macs} = { uc($mac) => { ip4 => undef, ip6 => undef } };
+ # a vnet's entry and an old one that disagree are not trusted either, a
+ # node not yet upgraded may have moved the address since, and a family
+ # the old entry lacks was released by such a node
+ $test_state->{macdb} = {
+ macs => { $mac => { ip4 => '10.0.0.151' } },
+ vnets => { $vnetid => { $mac => { ip4 => '10.0.0.150', ip6 => 'fd00::150' } } },
+ };
@ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
- is($ips[0], '10.0.0.50', "$test_name: a lookup past an address-less entry asks the IPAM");
+ is(
+ $ips[0],
+ '10.0.0.50',
+ "$test_name: an old entry disagreeing with the vnet's counts as a miss",
+ );
is_deeply(
- $test_state->{macdb}->{macs},
- { uc($mac) => { ip4 => '10.0.0.50', ip6 => undef } },
- "$test_name: and fills the entry under its spelling",
+ $test_state->{macdb},
+ {
+ macs => { $mac => { ip4 => '10.0.0.50' } },
+ vnets => { $vnetid => { $mac => { ip4 => '10.0.0.50' } } },
+ },
+ "$test_name: the answer replaces the vnet's entry and the old one, the family without a record is gone",
);
# two spellings that disagree are not trusted, the lookup asks the IPAM
- $test_state->{macdb}->{macs} = {
- uc($mac) => { ip4 => '10.0.0.150' },
- $mac => { ip4 => '10.0.0.151' },
+ $test_state->{macdb} = {
+ macs => {
+ uc($mac) => { ip4 => '10.0.0.150' },
+ $mac => { ip4 => '10.0.0.151' },
+ },
};
@ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
is($ips[0], '10.0.0.50', "$test_name: a clash of spellings counts as a miss");
- is_deeply(
- $test_state->{macdb}->{macs},
- { uc($mac) => { ip4 => '10.0.0.50' }, $mac => { ip4 => '10.0.0.50' } },
- "$test_name: and its write brings both spellings in line",
- );
+
+ # an old entry's address outside the vnet's subnets is another vnet's
+ $test_state->{macdb} = { macs => { uc($mac) => { ip4 => '10.9.0.150' } } };
+ @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is($ips[0], '10.0.0.50', "$test_name: an old address of another vnet counts as a miss");
+
+ # the record keeps the MAC as its writer spelled it, a lookup in another
+ # spelling finds it
+ $test_state->{macdb} = {};
+ @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, uc($mac));
+ is($ips[0], '10.0.0.50', "$test_name: a lookup in another spelling finds the record");
# a mapping whose record is gone is still deleted, and its cache entry
# with it
@@ -1156,21 +1183,40 @@ sub test_dnsmasq_stale_reservation_replaced {
my $stale = "da:65:8f:18:9b:01";
my $gone = "da:65:8f:18:9b:02";
my $added = "da:65:8f:18:9b:03";
+ create_dhcp_zone("TESTZONE", "testvnet", "10.0.1.0/24");
$test_state->{macdb} = {
- macs => {
- $stale => { ip4 => '10.0.1.11' },
- $added => { ip4 => '10.0.1.13' },
+ macs => { $stale => { ip4 => '10.0.1.11' }, $added => { ip4 => '10.0.1.13' } },
+ vnets => {
+ testvnet => {
+ $stale => { ip4 => '10.0.1.11' },
+ $added => { ip4 => '10.0.1.13' },
+ },
},
};
PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$stale,10.0.1.10\n$gone,10.0.1.12\n");
- PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping('TESTZONE', $added);
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping('TESTZONE', 'testvnet', $added);
is(
PVE::Tools::file_get_contents($TMP_ETHERS_FILE),
"$stale,10.0.1.11\n$added,10.0.1.13\n",
"$test_name: the stale line is replaced, the unknown one dropped",
);
+
+ # the changed MAC's own line follows the IPAM's answer, not the cache,
+ # when its entry is no longer witnessed
+ create_ip({ zone => "TESTZONE", vnet => "testvnet", mac => $added, ip => "10.0.1.20" });
+ $test_state->{macdb} = {
+ macs => { $added => { ip4 => '10.0.1.22' } },
+ vnets => { testvnet => { $added => { ip4 => '10.0.1.21' } } },
+ };
+ PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$added,10.0.1.21\n");
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping('TESTZONE', 'testvnet', $added);
+ is(
+ PVE::Tools::file_get_contents($TMP_ETHERS_FILE),
+ "$added,10.0.1.20\n",
+ "$test_name: the changed MAC's stale line is replaced by the IPAM's answer",
+ );
}
run_test(\&test_dnsmasq_stale_reservation_replaced);
@@ -1355,8 +1401,208 @@ sub test_ipam_per_vnet {
"$test_name: vnet$n answers with its own address",
);
}
+
+ # a mapping edit on one vnet leaves the other's answer alone
+ PVE::API2::Network::SDN::Ips->ipupdate(
+ { zone => $zoneid, vnet => 'vnet1', mac => $mac, ip => '10.1.0.51' });
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+ ['10.2.0.50', undef],
+ "$test_name: an edit on the first vnet leaves the second's answer alone",
+ );
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet1', $mac)],
+ ['10.1.0.51', undef],
+ "$test_name: and changes the first's",
+ );
+
+ # an entry from before the cache was kept per vnet answers for a vnet
+ # without an entry of its own, until a write for the MAC on another vnet
+ # replaces it in the map under the MAC alone, which holds one address.
+ # The vnet then asks the IPAM
+ $test_state->{macdb} = { macs => { $mac => { ip4 => '10.2.0.60' } } };
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+ ['10.2.0.60', undef],
+ "$test_name: an old entry answers for a vnet without an entry of its own",
+ );
+ create_ip({ zone => $zoneid, vnet => 'vnet1', mac => $mac, ip => '10.1.0.52' });
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+ ['10.2.0.50', undef],
+ "$test_name: once a write for the MAC elsewhere replaced it, the IPAM answers",
+ );
+
+ # a vnet's address holds only while the map under the MAC alone names it,
+ # since that map is where a node not yet upgraded releases or moves an
+ # address. With the MAC on two vnets the map witnesses one of them
+ $test_state->{macdb} = {
+ macs => { $mac => { ip4 => '10.2.0.50' } },
+ vnets => {
+ vnet1 => { $mac => { ip4 => '10.1.0.50' } },
+ vnet2 => { $mac => { ip4 => '10.2.0.50' } },
+ },
+ };
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+ ['10.2.0.50', undef],
+ "$test_name: the vnet whose address the old map names answers from the cache",
+ );
+ like(
+ (PVE::Network::SDN::Vnets::get_ips_from_mac('vnet1', $mac))[0],
+ qr/^10\.1\.0\.5[12]$/,
+ "$test_name: the other one asks the IPAM",
+ );
+ like(
+ $test_state->{macdb}->{macs}->{$mac}->{ip4},
+ qr/^10\.1\.0\.5[12]$/,
+ "$test_name: and its answer takes the old map",
+ );
+ delete $test_state->{macdb}->{macs};
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+ ['10.2.0.50', undef],
+ "$test_name: without the family in the old map the IPAM is asked",
+ );
+
+ # the address of the MAC on a vnet of another zone with the same range
+ # sits in the old map as well, and is that vnet's, so a vnet without an
+ # entry asks the IPAM rather than taking it, and one with an entry the
+ # old map does not name asks as well
+ create_zone({ type => "simple", dhcp => "dnsmasq", ipam => "pve", zone => "ZONEB" });
+ create_dhcp_vnet("ZONEB", "vnet3", "10.1.0.0/24");
+ create_ip({ zone => "ZONEB", vnet => "vnet3", mac => $mac, ip => "10.1.0.70" });
+ $test_state->{macdb} = {
+ macs => { $mac => { ip4 => '10.1.0.52' } },
+ vnets => { vnet1 => { $mac => { ip4 => '10.1.0.52' } } },
+ };
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet3', $mac)],
+ ['10.1.0.70', undef],
+ "$test_name: another zone's address in the old map is not taken for a vnet without an entry",
+ );
+ $test_state->{macdb}->{macs}->{$mac}->{ip4} = '10.1.0.52';
+ $test_state->{macdb}->{vnets}->{vnet3}->{$mac} = { ip4 => '10.1.0.71' };
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet3', $mac)],
+ ['10.1.0.70', undef],
+ "$test_name: and an entry the old map does not name is checked with the IPAM",
+ );
+
+ # a release on an upgraded node clears the old map's slot, the MAC's
+ # other vnet asks the IPAM at its next start and takes it
+ $test_state->{macdb} = {
+ macs => { $mac => { ip4 => '10.2.0.50' } },
+ vnets => {
+ vnet1 => { $mac => { ip4 => '10.1.0.52' } },
+ vnet2 => { $mac => { ip4 => '10.2.0.50' } },
+ },
+ };
+ PVE::Network::SDN::Ipams::del_cache_mac_ip('vnet2', $mac, '10.2.0.50');
+ is_deeply(
+ $test_state->{macdb},
+ { macs => {}, vnets => { vnet1 => { $mac => { ip4 => '10.1.0.52' } } } },
+ "$test_name: a release clears the old map's slot",
+ );
}
run_test(\&test_ipam_per_vnet);
+sub test_ipam_edit_stale_entry {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+ my $other = "da:65:8f:18:9b:70";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+ # a node not yet upgraded released the MAC's address and handed it to
+ # another guest, while the vnet's cache entry still holds it. The entry
+ # is not trusted, so the edit releases nothing and records the new address
+ create_ip({ zone => $zoneid, vnet => $vnetid, mac => $other, ip => '10.0.0.50' });
+ $test_state->{macdb}->{vnets}->{$vnetid}->{$mac} = { ip4 => '10.0.0.50' };
+
+ PVE::API2::Network::SDN::Ips->ipupdate(
+ { zone => $zoneid, vnet => $vnetid, mac => $mac, ip => '10.0.0.60' });
+ is(
+ get_ip4(get_ips_from_mac($other)),
+ '10.0.0.50',
+ "$test_name: an edit leaves the other guest's record alone",
+ );
+ is(get_ip4(get_ips_from_mac($mac)), '10.0.0.60', "$test_name: and records the new address");
+ is(
+ $test_state->{macdb}->{vnets}->{$vnetid}->{$mac}->{ip4},
+ '10.0.0.60',
+ "$test_name: which the cache holds now",
+ );
+
+ # a deletion of a mapping the IPAM does not hold is refused
+ $test_state->{macdb}->{vnets}->{$vnetid}->{$mac} = { ip4 => '10.0.0.50' };
+ eval {
+ PVE::API2::Network::SDN::Ips->ipdelete(
+ { zone => $zoneid, vnet => $vnetid, mac => $mac, ip => '10.0.0.50' });
+ };
+ like($@, qr/not mapped to/, "$test_name: a deletion for another guest's address is refused");
+}
+
+run_test(\&test_ipam_edit_stale_entry);
+
+sub test_dnsmasq_reservation_per_vnet {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ # the same MAC on two vnets of one zone has a reservation on each, and a
+ # start on one vnet changes that vnet's line only
+ create_zone({ type => "simple", dhcp => "dnsmasq", ipam => "pve", zone => $zoneid });
+ for my $n (1, 2) {
+ create_dhcp_vnet($zoneid, "vnet$n", "10.$n.0.0/24");
+ create_ip({ zone => $zoneid, vnet => "vnet$n", mac => $mac, ip => "10.$n.0.50" });
+ }
+
+ my $reservations = sub {
+ return [sort grep { $_ } split(/\n/, PVE::Tools::file_get_contents($TMP_ETHERS_FILE))];
+ };
+
+ nic_start("vnet$_", $mac, "999", "testhostname") for (1, 2, 1);
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.50", "$mac,10.2.0.50"],
+ "$test_name: each vnet keeps its reservation",
+ );
+
+ PVE::API2::Network::SDN::Ips->ipupdate(
+ { zone => $zoneid, vnet => 'vnet1', mac => $mac, ip => '10.1.0.51' });
+ nic_start('vnet1', $mac, "999", "testhostname");
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.51", "$mac,10.2.0.50"],
+ "$test_name: an edit replaces the reservation of its vnet only",
+ );
+
+ # the other lines are checked against the cache alone, so a line neither
+ # map holds is dropped without the IPAM being asked
+ delete $test_state->{macdb}->{vnets}->{vnet2};
+ delete $test_state->{macdb}->{macs};
+ nic_start('vnet1', $mac, "999", "testhostname");
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.51"],
+ "$test_name: a line the cache does not hold is dropped",
+ );
+
+ # the changed MAC's own line is answered by the IPAM when the cache does
+ # not hold it
+ delete $test_state->{macdb}->{vnets}->{vnet1};
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping($zoneid, 'vnet1', $mac);
+ is_deeply(
+ $reservations->(),
+ ["$mac,10.1.0.51"],
+ "$test_name: the changed MAC's line is kept without a cache entry",
+ );
+}
+
+run_test(\&test_dnsmasq_reservation_per_vnet);
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH pve-network v2 09/11] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM
2026-11-03 15:15 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (7 preceding siblings ...)
2026-11-03 15:15 ` [PATCH pve-network v2 08/11] sdn: ipam: keep the MAC cache per vnet Hannes Laimer
@ 2026-11-03 15:15 ` Hannes Laimer
2026-11-03 15:15 ` [PATCH qemu-server v2 10/11] network: sdn: push the DHCP mappings of a guest when it resumes Hannes Laimer
` (3 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 15:15 UTC (permalink / raw)
To: pve-devel
Two zones can use the same subnet, and an external IPAM asked by the
address alone cannot tell their records apart. A zone's subnets now go
into a VRF in NetBox and into a section under the configured section
in phpIPAM, both named pve-<zone> and created with the first subnet
added to the zone. A subnet being added goes into the zone's VRF or
section, also when another zone holds the same range in its own. A
range the global table or the configured section still holds is
refused, since a copy would hide the records in it. The error says to
move that prefix or subnet into the zone's VRF or section, or to
remove it. phpIPAM nests sections one level deep, so under a
configured section that is a sub-section itself the zones share the
configured section as before.
Subnets from before stay in the global table or the configured
section and are still found there, so nothing has to be moved. The
phpIPAM lookup by MAC, unimplemented until now, returns the addresses
recorded for the MAC in the vnet's subnets, and its gateway check
answers for the subnet it is asked about and for no other. A failed
lookup stops the operation, only a not found answer means there is
no such section, subnet or record.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 178 ++++++--
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 215 +++++++--
src/test/ipams/netbox/expected.add_ip | 2 +-
.../ipams/netbox/expected.add_ip_notgateway | 2 +-
src/test/ipams/netbox/expected.add_subnet | 2 +-
src/test/ipams/netbox/expected.update_ip | 2 +-
src/test/run_test_ipams.pl | 419 ++++++++++++++++++
7 files changed, 739 insertions(+), 81 deletions(-)
diff --git a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
index 84ce4382e0e7..7e8dad1881d0 100644
--- a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
@@ -40,18 +40,15 @@ sub netbox_api_request {
}
sub add_dhcp_range {
- my ($config, $dhcp_range, $noerr) = @_;
+ my ($config, $dhcp_range, $vrf_id, $noerr) = @_;
my $result = eval {
- netbox_api_request(
- $config,
- "POST",
- "/ipam/ip-ranges/",
- {
- start_address => $dhcp_range->{'start-address'},
- end_address => $dhcp_range->{'end-address'},
- },
- );
+ my $params = {
+ start_address => $dhcp_range->{'start-address'},
+ end_address => $dhcp_range->{'end-address'},
+ };
+ $params->{vrf} = $vrf_id if defined($vrf_id);
+ netbox_api_request($config, "POST", "/ipam/ip-ranges/", $params);
};
if ($@) {
return if $noerr;
@@ -72,18 +69,44 @@ sub del_dhcp_range {
# Plugin implementation
+my sub zone_vrf_name {
+ my ($zoneid) = @_;
+
+ return "pve-$zoneid";
+}
+
sub add_subnet {
my ($class, $plugin_config, $subnetid, $subnet, $noerr) = @_;
my $cidr = $subnet->{cidr};
my $gateway = $subnet->{gateway};
- if (get_prefix_id($plugin_config, $cidr, $noerr)) {
+ # a subnet being added is new to its zone, so it goes into the zone's VRF,
+ # and a prefix from before in the global table is not taken over, since
+ # a copy in the VRF would hide its records
+ my $vrf_id = get_zone_namespace($plugin_config, $subnet->{zone}, $noerr);
+ if (defined($vrf_id) && defined(get_prefix_id($plugin_config, $cidr, $vrf_id, $noerr))) {
return if $noerr;
die "prefix $cidr already exists in netbox";
}
+ if (defined(get_prefix_id($plugin_config, $cidr, undef, $noerr))) {
+ return if $noerr;
+ my $vrf = zone_vrf_name($subnet->{zone});
+ die "prefix $cidr exists in the global table of NetBox, move it with its"
+ . " addresses and ranges into VRF $vrf or remove it first\n";
+ }
+
+ $vrf_id //= add_zone_namespace($plugin_config, $subnet->{zone}, $noerr);
+ return if !defined($vrf_id) && $noerr;
- eval { netbox_api_request($plugin_config, "POST", "/ipam/prefixes/", { prefix => $cidr }); };
+ eval {
+ netbox_api_request(
+ $plugin_config,
+ "POST",
+ "/ipam/prefixes/",
+ { prefix => $cidr, vrf => $vrf_id },
+ );
+ };
if ($@) {
return if $noerr;
die "error adding subnet to ipam: $@";
@@ -91,7 +114,7 @@ sub add_subnet {
my $dhcp_ranges = PVE::Network::SDN::Subnets::get_dhcp_ranges($subnet);
for my $dhcp_range (@$dhcp_ranges) {
- add_dhcp_range($plugin_config, $dhcp_range, $noerr);
+ add_dhcp_range($plugin_config, $dhcp_range, $vrf_id, $noerr);
}
}
@@ -122,6 +145,8 @@ sub update_subnet {
my $to_delete_ids = ();
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr, 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 +155,7 @@ sub update_subnet {
next;
}
- my $internalid = get_iprange_id($plugin_config, $dhcp_range, $noerr);
+ my $internalid = get_iprange_id($plugin_config, $dhcp_range, $vrf_id, $noerr);
# definedness check, because ID could be 0
if (!defined($internalid)) {
@@ -145,7 +170,7 @@ sub update_subnet {
for my $dhcp_range (@$new_dhcp_ranges) {
my $hash = $hash_range->($dhcp_range);
- add_dhcp_range($plugin_config, $dhcp_range, $noerr)
+ add_dhcp_range($plugin_config, $dhcp_range, $vrf_id, $noerr)
if !exists($old_lookup->{$hash});
}
}
@@ -155,7 +180,7 @@ sub del_subnet {
my $cidr = $subnet->{cidr};
- my $internalid = get_prefix_id($plugin_config, $cidr, $noerr);
+ my ($internalid, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr);
# definedness check, because ID could be 0
if (!defined($internalid)) {
@@ -163,7 +188,7 @@ sub del_subnet {
return;
}
- if (!is_prefix_empty($plugin_config, $cidr, $noerr)) {
+ if (!is_prefix_empty($plugin_config, $cidr, $vrf_id, $noerr)) {
return if $noerr;
die "not deleting prefix $cidr because it still contains entries";
}
@@ -176,7 +201,7 @@ sub del_subnet {
my $dhcp_ranges = PVE::Network::SDN::Subnets::get_dhcp_ranges($subnet);
for my $dhcp_range (@$dhcp_ranges) {
- my $internalid = get_iprange_id($plugin_config, $dhcp_range, $noerr);
+ my $internalid = get_iprange_id($plugin_config, $dhcp_range, $vrf_id, $noerr);
# definedness check, because ID could be 0
if (!defined($internalid)) {
@@ -215,6 +240,8 @@ sub add_ip {
$description = "mac:$mac";
}
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr, 1) or return;
+
eval {
my $params = {
address => "$ip/$mask",
@@ -222,6 +249,7 @@ sub add_ip {
};
$params->{dns_name} = $hostname if $hostname;
+ $params->{vrf} = $vrf_id if defined($vrf_id);
netbox_api_request($plugin_config, "POST", "/ipam/ip-addresses/", $params);
};
@@ -229,7 +257,7 @@ sub add_ip {
if ($@) {
if ($is_gateway) {
die "error add subnet ip to ipam: ip $ip already exist: $@"
- if !is_ip_gateway($plugin_config, $ip, $noerr);
+ if !is_ip_gateway($plugin_config, $ip, $vrf_id, $noerr);
} elsif (!$noerr) {
die "error add subnet ip to ipam: ip already exist: $@";
}
@@ -259,7 +287,8 @@ sub update_ip {
$description = "mac:$mac";
}
- my $ip_id = get_ip_id($plugin_config, $ip, $noerr);
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr, 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 +303,7 @@ sub update_ip {
};
$params->{dns_name} = $hostname if $hostname;
+ $params->{vrf} = $vrf_id if defined($vrf_id);
netbox_api_request($plugin_config, "PATCH", "/ipam/ip-addresses/$ip_id/", $params);
};
@@ -287,13 +317,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 +347,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 +389,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 +423,16 @@ sub get_ips_from_mac {
my $ip4 = undef;
my $ip6 = undef;
- for my $cidr (sort map { $_->{cidr} } values %{ $subnets // {} }) {
+ for my $subnet (sort { $a->{cidr} cmp $b->{cidr} } values %{ $subnets // {} }) {
+ my $cidr = $subnet->{cidr};
+ my ($prefix_id, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr);
+ next if !defined($prefix_id);
+ my $vrf = vrf_filter($vrf_id);
my $data = eval {
netbox_api_request(
$plugin_config,
"GET",
- "/ipam/ip-addresses/?parent=$cidr&description__ic=$mac",
+ "/ipam/ip-addresses/?parent=$cidr&$vrf&description__ic=$mac",
);
};
if ($@) {
@@ -439,14 +469,67 @@ sub on_update_hook {
}
# helpers
-sub get_prefix_id {
- my ($config, $cidr, $noerr) = @_;
- # we need to supply any IP inside the prefix, without supplying the mask, so
- # just take the one from the cidr
- my ($ip, undef) = split(/\//, $cidr);
+sub vrf_filter {
+ my ($vrf_id) = @_;
+
+ return defined($vrf_id) ? "vrf_id=$vrf_id" : "vrf_id=null";
+}
+
+sub get_zone_namespace {
+ my ($config, $zoneid, $noerr) = @_;
+
+ my $name = zone_vrf_name($zoneid);
+ my $result = eval { netbox_api_request($config, "GET", "/ipam/vrfs/?name=$name") };
+ if ($@) {
+ return if $noerr;
+ die "could not obtain ID for VRF $name: $@";
+ }
+
+ my $data = @{ $result->{results} }[0];
+ return $data->{id};
+}
+
+sub add_zone_namespace {
+ my ($config, $zoneid, $noerr) = @_;
+
+ my $name = zone_vrf_name($zoneid);
+ my $result = eval { netbox_api_request($config, "POST", "/ipam/vrfs/", { name => $name }) };
+ if ($@) {
+ return if $noerr;
+ die "could not create VRF $name: $@";
+ }
+
+ return $result->{id};
+}
+
+# a prefix from before sits in the global table and keeps its addresses there,
+# so a subnet is looked for in the zone's VRF first and in the global table
+# after it, with none as the VRF of the global table. A subnet with no prefix
+# anywhere stops an operation that needs one, since the address and range
+# operations would act on the global table where another zone's records may
+# sit
+sub subnet_prefix {
+ my ($config, $subnet, $noerr, $needed) = @_;
+
+ my $vrf_id = get_zone_namespace($config, $subnet->{zone}, $noerr);
+ if (defined($vrf_id)) {
+ my $prefix_id = get_prefix_id($config, $subnet->{cidr}, $vrf_id, $noerr);
+ return ($prefix_id, $vrf_id) if defined($prefix_id);
+ }
+ my $prefix_id = get_prefix_id($config, $subnet->{cidr}, undef, $noerr);
+ if ($needed && !defined($prefix_id)) {
+ return if $noerr;
+ die "could not find id for prefix $subnet->{cidr}";
+ }
+ return ($prefix_id, undef);
+}
+
+sub get_prefix_id {
+ my ($config, $cidr, $vrf_id, $noerr) = @_;
- my $result = eval { netbox_api_request($config, "GET", "/ipam/prefixes/?q=$ip") };
+ my $vrf = vrf_filter($vrf_id);
+ my $result = eval { netbox_api_request($config, "GET", "/ipam/prefixes/?prefix=$cidr&$vrf") };
if ($@) {
return if $noerr;
die "could not obtain ID for prefix $cidr: $@";
@@ -457,13 +540,14 @@ sub get_prefix_id {
}
sub get_iprange_id {
- my ($config, $range, $noerr) = @_;
+ my ($config, $range, $vrf_id, $noerr) = @_;
+ my $vrf = vrf_filter($vrf_id);
my $result = eval {
netbox_api_request(
$config,
"GET",
- "/ipam/ip-ranges/?start_address=$range->{'start-address'}&end_address=$range->{'end-address'}",
+ "/ipam/ip-ranges/?start_address=$range->{'start-address'}&end_address=$range->{'end-address'}&$vrf",
);
};
if ($@) {
@@ -477,9 +561,10 @@ sub get_iprange_id {
}
sub get_ip_id {
- my ($config, $ip, $noerr) = @_;
+ my ($config, $ip, $vrf_id, $noerr) = @_;
- my $result = eval { netbox_api_request($config, "GET", "/ipam/ip-addresses/?q=$ip") };
+ my $vrf = vrf_filter($vrf_id);
+ my $result = eval { netbox_api_request($config, "GET", "/ipam/ip-addresses/?q=$ip&$vrf") };
if ($@) {
return if $noerr;
die "could not obtain ID for IP $ip: $@";
@@ -490,9 +575,10 @@ sub get_ip_id {
}
sub is_ip_gateway {
- my ($config, $ip, $noerr) = @_;
+ my ($config, $ip, $vrf_id, $noerr) = @_;
- my $result = eval { netbox_api_request($config, "GET", "/ipam/ip-addresses/?q=$ip") };
+ my $vrf = vrf_filter($vrf_id);
+ my $result = eval { netbox_api_request($config, "GET", "/ipam/ip-addresses/?q=$ip&$vrf") };
if ($@) {
return if $noerr;
die "could not obtain ipam entry for address $ip: $@";
@@ -503,9 +589,11 @@ sub is_ip_gateway {
}
sub is_prefix_empty {
- my ($config, $cidr, $noerr) = @_;
+ my ($config, $cidr, $vrf_id, $noerr) = @_;
- my $result = eval { netbox_api_request($config, "GET", "/ipam/ip-addresses/?parent=$cidr") };
+ my $vrf = vrf_filter($vrf_id);
+ my $result =
+ eval { netbox_api_request($config, "GET", "/ipam/ip-addresses/?parent=$cidr&$vrf") };
if ($@) {
return if $noerr;
die "could not query children for prefix $cidr: $@";
diff --git a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
index e969c3dcdf4f..f88ccc632415 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;
@@ -40,6 +43,15 @@ sub options {
# Plugin implementation
+# a zone's section is a sub-section of the configured one, so SDN writes only
+# under the section it was given. phpIPAM wants at least three characters in a
+# section name, a zone id may have two
+my sub zone_section_name {
+ my ($zoneid) = @_;
+
+ return "pve-$zoneid";
+}
+
sub add_subnet {
my ($class, $plugin_config, $subnetid, $subnet, $noerr) = @_;
@@ -49,15 +61,37 @@ sub add_subnet {
my $url = $plugin_config->{url};
my $token = $plugin_config->{token};
- my $section = $plugin_config->{section};
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
+ # a subnet being added is new to its zone, so it goes into the zone's
+ # section, and a subnet from before in the configured section is not taken
+ # over, since a copy in the zone's section would hide its records
+ my $section = get_zone_namespace($plugin_config, $subnet->{zone}, $headers);
+ $section //= shared_section($plugin_config, $headers);
+
#search subnet
- my $internalid = eval { get_prefix_id($url, $cidr, $headers) };
+ my $internalid = eval { get_prefix_id($url, $cidr, $headers, $section) };
+ if (my $err = $@) {
+ return if $noerr;
+ die $err;
+ }
+ my $in_shared = defined($section) && $section == $plugin_config->{section};
+ if (
+ !$internalid
+ && !$in_shared
+ && defined(get_prefix_id($url, $cidr, $headers, $plugin_config->{section}))
+ ) {
+ return if $noerr;
+ my $name = zone_section_name($subnet->{zone});
+ die "subnet $cidr exists in section $plugin_config->{section} of phpIPAM, move it"
+ . " into sub-section $name of that section or remove it first\n";
+ }
#create subnet
if (!$internalid) {
+ $section //= add_zone_namespace($plugin_config, $subnet->{zone}, $headers, $noerr);
+ return if !defined($section);
my $params = {
subnet => $network,
mask => $mask,
@@ -87,7 +121,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 +154,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 +174,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 +201,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 +232,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 +282,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 +313,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 +342,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 +359,19 @@ sub get_ips_from_mac {
$fingerprint,
);
};
- return if $@;
-
- #fixme
- die "parsing of result not yet implemented";
-
- for my $ip (@$ips) {
- # if ($ip->{family}->{value} == 4 && !$ip4) {
- # ($ip4, undef) = split(/\//, $ip->{address});
- # }
- #
- # if ($ip->{family}->{value} == 6 && !$ip6) {
- # ($ip6, undef) = split(/\//, $ip->{address});
- # }
+ if (my $err = $@) {
+ return if not_found($err);
+ die "could not look up the addresses of $mac: $err";
+ }
+
+ for my $hit (@{ $result->{data} // [] }) {
+ my $family = $wanted{ $hit->{subnetId} // '' } // next;
+ my $ip = address_of_hit($hit, $family) // next;
+ if ($family == 4) {
+ $ip4 //= $ip;
+ } else {
+ $ip6 //= $ip;
+ }
}
return ($ip4, $ip6);
@@ -344,27 +404,118 @@ sub on_update_hook {
#helpers
-sub get_prefix_id {
- my ($url, $cidr, $headers) = @_;
+sub get_zone_namespace {
+ my ($plugin_config, $zoneid, $headers) = @_;
+
+ my $name = zone_section_name($zoneid);
+ my $result = eval {
+ PVE::Network::SDN::api_request(
+ "GET",
+ "$plugin_config->{url}/sections/$name/",
+ $headers,
+ undef,
+ $plugin_config->{fingerprint},
+ );
+ };
+ if (my $err = $@) {
+ # only a missing section means the zone has none yet, any other
+ # failure would send the zone's records to the configured section
+ return if not_found($err);
+ die "could not look up section $name: $err";
+ }
+ # 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($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, zone ids must be"
+ . " unique across the clusters sharing it\n"
+ if $taken;
+ die "could not create section $name: $err";
+ }
+ return $result->{id};
+}
+
+# phpIPAM nests sections one level deep, so a configured section that is a
+# sub-section itself gets no zone sections and every zone's subnets stay in it
+sub shared_section {
+ my ($plugin_config, $headers) = @_;
+
+ my $result = PVE::Network::SDN::api_request(
+ "GET",
+ "$plugin_config->{url}/sections/$plugin_config->{section}/",
+ $headers,
+ undef,
+ $plugin_config->{fingerprint},
+ );
+ return ($result->{data}->{masterSection} // 0) ? $plugin_config->{section} : undef;
+}
- my $result = PVE::Network::SDN::api_request("GET", "$url/subnets/cidr/$cidr", $headers);
- my $data = @{ $result->{data} }[0];
- my $internalid = $data->{id};
- return $internalid;
+# a subnet from before sits in the configured section, so a subnet is looked
+# for in the zone's section first and in the configured one after it
+sub subnet_id {
+ my ($plugin_config, $subnet, $headers) = @_;
+
+ my $section = get_zone_namespace($plugin_config, $subnet->{zone}, $headers);
+ my $url = $plugin_config->{url};
+ return get_prefix_id($url, $subnet->{cidr}, $headers, $section)
+ // get_prefix_id($url, $subnet->{cidr}, $headers, $plugin_config->{section});
+}
+
+sub get_prefix_id {
+ my ($url, $cidr, $headers, $section) = @_;
+
+ return undef if !defined($section);
+ my $result =
+ eval { PVE::Network::SDN::api_request("GET", "$url/subnets/cidr/$cidr", $headers) };
+ if (my $err = $@) {
+ return undef if not_found($err);
+ die "could not look up subnet $cidr: $err";
+ }
+ my ($data) = grep { $_->{sectionId} == $section } @{ $result->{data} // [] };
+ return $data->{id};
}
sub get_ip_id {
- my ($url, $ip, $headers) = @_;
+ my ($url, $ip, $headers, $subnet_id) = @_;
my $result = PVE::Network::SDN::api_request("GET", "$url/addresses/search/$ip", $headers);
- my $data = @{ $result->{data} }[0];
+ my ($data) =
+ grep { defined($subnet_id) && $_->{subnetId} == $subnet_id } @{ $result->{data} // [] };
my $ip_id = $data->{id};
return $ip_id;
}
sub is_ip_gateway {
- my ($url, $ip, $headers) = @_;
+ my ($url, $ip, $headers, $subnet_id) = @_;
my $result = PVE::Network::SDN::api_request("GET", "$url/addresses/search/$ip", $headers);
- my $data = @{ $result->{data} }[0];
+ my ($data) =
+ grep { defined($subnet_id) && $_->{subnetId} == $subnet_id } @{ $result->{data} // [] };
my $is_gateway = $data->{is_gateway};
return $is_gateway;
}
diff --git a/src/test/ipams/netbox/expected.add_ip b/src/test/ipams/netbox/expected.add_ip
index cd46c53f1566..f54f8d05b16e 100644
--- a/src/test/ipams/netbox/expected.add_ip
+++ b/src/test/ipams/netbox/expected.add_ip
@@ -1,5 +1,5 @@
bless( {
- '_content' => '{"address":"10.0.0.1/24","description":"gateway","dns_name":"myhostname"}',
+ '_content' => '{"address":"10.0.0.1/24","description":"gateway","dns_name":"myhostname","vrf":1}',
'_headers' => bless( {
'authorization' => 'token 0123456789abcdef0123456789abcdef01234567',
'content-type' => 'application/json; charset=UTF-8'
diff --git a/src/test/ipams/netbox/expected.add_ip_notgateway b/src/test/ipams/netbox/expected.add_ip_notgateway
index 5ea43cd65296..327efbab8c6b 100644
--- a/src/test/ipams/netbox/expected.add_ip_notgateway
+++ b/src/test/ipams/netbox/expected.add_ip_notgateway
@@ -1,5 +1,5 @@
bless( {
- '_content' => '{"address":"10.0.0.1/24","description":"mac:da:65:8f:18:9b:6f","dns_name":"myhostname"}',
+ '_content' => '{"address":"10.0.0.1/24","description":"mac:da:65:8f:18:9b:6f","dns_name":"myhostname","vrf":1}',
'_headers' => bless( {
'authorization' => 'token 0123456789abcdef0123456789abcdef01234567',
'content-type' => 'application/json; charset=UTF-8'
diff --git a/src/test/ipams/netbox/expected.add_subnet b/src/test/ipams/netbox/expected.add_subnet
index 6f21265a2a18..7c2785adf9cd 100644
--- a/src/test/ipams/netbox/expected.add_subnet
+++ b/src/test/ipams/netbox/expected.add_subnet
@@ -1,5 +1,5 @@
bless( {
- '_content' => '{"prefix":"10.0.0.0/24"}',
+ '_content' => '{"prefix":"10.0.0.0/24","vrf":1}',
'_headers' => bless( {
'authorization' => 'token 0123456789abcdef0123456789abcdef01234567',
'content-type' => 'application/json; charset=UTF-8'
diff --git a/src/test/ipams/netbox/expected.update_ip b/src/test/ipams/netbox/expected.update_ip
index d3bc1addbdea..8a6e0a8cbbad 100644
--- a/src/test/ipams/netbox/expected.update_ip
+++ b/src/test/ipams/netbox/expected.update_ip
@@ -1,5 +1,5 @@
bless( {
- '_content' => '{"address":"10.0.0.1/24","description":"gateway","dns_name":"myhostname"}',
+ '_content' => '{"address":"10.0.0.1/24","description":"gateway","dns_name":"myhostname","vrf":1}',
'_headers' => bless( {
'authorization' => 'token 0123456789abcdef0123456789abcdef01234567',
'content-type' => 'application/json; charset=UTF-8'
diff --git a/src/test/run_test_ipams.pl b/src/test/run_test_ipams.pl
index f07b4bb4b486..db02ff2a3b37 100755
--- a/src/test/run_test_ipams.pl
+++ b/src/test/run_test_ipams.pl
@@ -92,6 +92,9 @@ foreach my $path (@plugins) {
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
my $sdn_ipam_plugin = Test::MockModule->new($plugin);
$sdn_ipam_plugin->mock(
+ get_zone_namespace => sub {
+ return 1;
+ },
get_prefix_id => sub {
return 1;
},
@@ -219,6 +222,143 @@ foreach my $path (@plugins) {
}
+# the phpipam MAC lookup, with the addresses as the decimals phpipam hands out
+{
+ my $plugin = 'PVE::Network::SDN::Ipams::PhpIpamPlugin';
+ my $mocked = Test::MockModule->new($plugin);
+ $mocked->mock(subnet_id => sub { my (undef, $subnet) = @_; return $subnet->{id}; });
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ return {
+ data => [
+ { ip => 171048980, subnetId => 7 },
+ { ip => '10.60.0.20', subnetId => 8 },
+ { ip => '55832768119825001489588427028103692289', subnetId => 9 },
+ ],
+ };
+ },
+ );
+ my $subnets = {
+ 7 => { id => 7, cidr => '10.50.0.0/24', network => '10.50.0.0' },
+ 9 => { id => 9, cidr => '2a01::/64', network => '2a01::' },
+ };
+ my @ips = $plugin->get_ips_from_mac(
+ { url => 'u', token => 't' }, 'da:65:8f:18:9b:6f', 'z', $subnets,
+ );
+ is_deeply(
+ \@ips,
+ ['10.50.0.20', '2a01::1'],
+ 'phpipam lookup converts decimal addresses by the subnet family',
+ );
+
+ # a MAC without addresses is a 404, any other failure fails the lookup
+ $sdn_module->mock(api_request => sub { die "Invalid response from server: 404 Not Found\n" });
+ is_deeply(
+ [
+ $plugin->get_ips_from_mac(
+ { url => 'u', token => 't' },
+ 'da:65:8f:18:9b:6f',
+ 'z',
+ $subnets,
+ ),
+ ],
+ [],
+ 'phpipam lookup answers nothing for a MAC without addresses',
+ );
+ $sdn_module->mock(
+ api_request => sub { die "Invalid response from server: 503 Service Unavailable\n" });
+ eval {
+ $plugin->get_ips_from_mac(
+ { url => 'u', token => 't' },
+ 'da:65:8f:18:9b:6f',
+ 'z',
+ $subnets,
+ );
+ };
+ like($@, qr/could not look up the addresses/, 'and dies on any other failure');
+}
+
+# a range the zone's namespace does not hold is refused while a subnet from
+# before holds it in the global table or the configured section, since a copy
+# would hide its records, and the error names the way out
+{
+ my $subnet = {
+ zone => 'z',
+ cidr => '10.0.0.0/24',
+ network => '10.0.0.0',
+ mask => '24',
+ };
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return {};
+ },
+ );
+
+ my $netbox = Test::MockModule->new('PVE::Network::SDN::Ipams::NetboxPlugin');
+ $netbox->mock(
+ get_zone_namespace => sub { return 2; },
+ get_prefix_id =>
+ sub { my (undef, undef, $vrf_id) = @_; return defined($vrf_id) ? undef : 5; },
+ );
+ eval {
+ PVE::Network::SDN::Ipams::NetboxPlugin->add_subnet(
+ { url => 'u', token => 't' },
+ 'z-10.0.0.0-24',
+ $subnet,
+ );
+ };
+ like($@, qr/move it .* into VRF pve-z/, 'netbox refuses a range the global table holds');
+ is(scalar(@requests), 0, 'and creates nothing');
+
+ @requests = ();
+ my $phpipam = Test::MockModule->new('PVE::Network::SDN::Ipams::PhpIpamPlugin');
+ $phpipam->mock(
+ get_zone_namespace => sub { return 2; },
+ get_prefix_id => sub {
+ my (undef, undef, undef, $section) = @_;
+ return ($section // '') eq '2' ? undef : 5;
+ },
+ );
+ eval {
+ PVE::Network::SDN::Ipams::PhpIpamPlugin->add_subnet(
+ { url => 'u', token => 't', section => 1 },
+ 'z-10.0.0.0-24',
+ $subnet,
+ );
+ };
+ like(
+ $@,
+ qr/move it into sub-section pve-z/,
+ 'phpipam refuses a range the configured section holds',
+ );
+ is(scalar(@requests), 0, 'and creates nothing');
+}
+
+# the phpipam gateway check answers for the subnet it is asked about, also
+# when the address sits in another zone's section as well
+{
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ return {
+ data => [
+ { ip => '10.0.0.1', subnetId => 7, is_gateway => 1 },
+ { ip => '10.0.0.1', subnetId => 8, is_gateway => 0 },
+ ],
+ };
+ },
+ );
+ my $is_gateway = \&PVE::Network::SDN::Ipams::PhpIpamPlugin::is_ip_gateway;
+ ok($is_gateway->('u', '10.0.0.1', [], 7), 'phpipam sees the gateway in its subnet');
+ ok(!$is_gateway->('u', '10.0.0.1', [], 8), 'and not in another subnet holding the address');
+ ok(!$is_gateway->('u', '10.0.0.1', [], undef), 'and not without a subnet to ask about');
+}
+
# an address released for a MAC stays when the external IPAM records it
# for another MAC
{
@@ -268,5 +408,284 @@ 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");
+ }
+}
+
+# a zone's VRF is found and created under its prefixed name
+{
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return { results => [{ id => 4 }], id => 4 };
+ },
+ );
+ my $config = { url => 'u', token => 't' };
+ PVE::Network::SDN::Ipams::NetboxPlugin::get_zone_namespace($config, 'z');
+ PVE::Network::SDN::Ipams::NetboxPlugin::add_zone_namespace($config, 'z');
+ is_deeply(
+ \@requests,
+ [
+ ['GET', 'u/ipam/vrfs/?name=pve-z', undef],
+ ['POST', 'u/ipam/vrfs/', { name => 'pve-z' }],
+ ],
+ 'netbox finds and creates the zone\'s VRF under the name pve-<zone>',
+ );
+}
+
+# a subnet from before is found in the global table while the zone's VRF holds
+# no prefix for its range, and in the VRF once it was moved there
+{
+ my $moved;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url) = @_;
+ return { results => [{ id => 2 }] } if $url =~ m|/ipam/vrfs/\?name=pve-z$|;
+ if ($url =~ m|/ipam/prefixes/\?prefix=10\.0\.0\.0/24&vrf_id=(\S+)$|) {
+ my $vrf = $1;
+ return { results => [{ id => 5 }] } if $vrf eq '2' && $moved;
+ return { results => [{ id => 5 }] } if $vrf eq 'null' && !$moved;
+ return { results => [] };
+ }
+ die "unexpected request $method $url\n";
+ },
+ );
+ my $config = { url => 'u', token => 't' };
+ my $subnet = { zone => 'z', cidr => '10.0.0.0/24' };
+ is_deeply(
+ [PVE::Network::SDN::Ipams::NetboxPlugin::subnet_prefix($config, $subnet)],
+ [5, undef],
+ 'netbox finds a prefix from before in the global table next to the zone\'s VRF',
+ );
+ $moved = 1;
+ is_deeply(
+ [PVE::Network::SDN::Ipams::NetboxPlugin::subnet_prefix($config, $subnet)],
+ [5, 2],
+ 'and in the VRF once the prefix was moved there',
+ );
+}
+
+# the same for phpipam, where the subnet keeps its id across the move, so the
+# requests show which section answered
+{
+ my $moved;
+ my @cidr_requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url) = @_;
+ return { data => { id => 2, masterSection => 1 } } if $url =~ m|/sections/pve-z/$|;
+ if ($url =~ m|/subnets/cidr/10\.0\.0\.0/24$|) {
+ push @cidr_requests, $url;
+ return { data => [{ id => 5, sectionId => $moved ? 2 : 1 }] };
+ }
+ die "unexpected request $method $url\n";
+ },
+ );
+ my $config = { url => 'u', token => 't', section => 1 };
+ my $subnet = { zone => 'z', cidr => '10.0.0.0/24' };
+ is(
+ PVE::Network::SDN::Ipams::PhpIpamPlugin::subnet_id($config, $subnet, []),
+ 5,
+ 'phpipam finds a subnet from before in the configured section',
+ );
+ is(scalar(@cidr_requests), 2, 'after looking in the zone\'s section first');
+ $moved = 1;
+ @cidr_requests = ();
+ is(
+ PVE::Network::SDN::Ipams::PhpIpamPlugin::subnet_id($config, $subnet, []),
+ 5,
+ 'and in the zone\'s section once the subnet was moved there',
+ );
+ is(scalar(@cidr_requests), 1, 'without a second look');
+}
+
+# a zone without a section is told apart from a section lookup that failed
+{
+ my $error;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(api_request => sub { die $error; });
+ my $lookup = \&PVE::Network::SDN::Ipams::PhpIpamPlugin::get_zone_namespace;
+ my $config = { url => 'u', token => 't' };
+
+ $error = "Invalid response from server: 404 Not Found\n";
+ is(scalar($lookup->($config, 'z', [])), undef, 'phpipam reports a missing section as none');
+ $error = "Invalid response from server: 503 Service Unavailable\n";
+ eval { $lookup->($config, 'z', []) };
+ like($@, qr/could not look up section pve-z/, 'and dies on any other failure');
+
+ # a section of the zone's name elsewhere in phpIPAM is 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/zone ids must be unique/,
+ '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 },
+ 'z-10.0.0.0/24',
+ { zone => 'z', cidr => '10.0.0.0/24', network => '10.0.0.0', mask => '24' },
+ 1,
+ );
+ is_deeply(
+ [map { [$_->[0], $_->[1], $_->[2]] } @requests],
+ [
+ ['GET', 'u/sections/1/', undef],
+ ['POST', 'u/sections/', { name => 'pve-z', masterSection => 1 }],
+ ['POST', 'u/subnets/', { subnet => '10.0.0.0', mask => '24', sectionId => 5 }],
+ ],
+ 'phpipam creates the zone\'s section under the configured one',
+ );
+
+ # a configured section that is a sub-section itself gets no zone sections
+ @requests = ();
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return $url eq 'u/sections/1/'
+ ? { data => { id => 1, masterSection => 3 } }
+ : { id => 5 };
+ },
+ );
+ PVE::Network::SDN::Ipams::PhpIpamPlugin->add_subnet(
+ { url => 'u', token => 't', section => 1 },
+ 'z-10.0.0.0/24',
+ { zone => 'z', cidr => '10.0.0.0/24', network => '10.0.0.0', mask => '24' },
+ 1,
+ );
+ is_deeply(
+ [map { [$_->[0], $_->[1], $_->[2]] } @requests],
+ [
+ ['GET', 'u/sections/1/', undef],
+ ['POST', 'u/subnets/', { subnet => '10.0.0.0', mask => '24', sectionId => 1 }],
+ ],
+ 'and keeps the subnets in a configured section that is a sub-section itself',
+ );
+
+ # a lookup that fails stops the creation, a copy would be made otherwise
+ @requests = ();
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ die "Invalid response from server: 503 Service Unavailable\n"
+ if $url =~ m|/subnets/cidr/|;
+ return { data => { id => 1, masterSection => 3 } };
+ },
+ );
+ $phpipam->unmock('get_prefix_id');
+ eval {
+ PVE::Network::SDN::Ipams::PhpIpamPlugin->add_subnet(
+ { url => 'u', token => 't', section => 1 },
+ 'z-10.0.0.0/24',
+ { zone => 'z', cidr => '10.0.0.0/24', network => '10.0.0.0', mask => '24' },
+ );
+ };
+ like($@, qr/could not look up subnet/, 'a failed subnet lookup stops the creation');
+ is(scalar(grep { $_->[0] eq 'POST' } @requests), 0, 'and nothing is created');
+}
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH qemu-server v2 10/11] network: sdn: push the DHCP mappings of a guest when it resumes
2026-11-03 15:15 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (8 preceding siblings ...)
2026-11-03 15:15 ` [PATCH pve-network v2 09/11] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM Hannes Laimer
@ 2026-11-03 15:15 ` Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-docs v2 11/11] sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM Hannes Laimer
` (2 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 15:15 UTC (permalink / raw)
To: pve-devel
A mapping edit updates the reservation on the guest's node, taken
from the vmlist, which names the source of a live migration until
the config moves at its end. An edit in that window went to the
source, while the target kept the record it started with.
Every resume now pushes the current mapping of each NIC first. The
last step of a migration resumes the target after the config moved,
so the target catches up there, or at the later resume of a guest
the migration left paused.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/QemuServer/Network.pm | 19 +++++++++++++++++++
src/PVE/QemuServer/RunState.pm | 2 ++
2 files changed, 21 insertions(+)
diff --git a/src/PVE/QemuServer/Network.pm b/src/PVE/QemuServer/Network.pm
index b4893c015646..9cdbfe6baad9 100644
--- a/src/PVE/QemuServer/Network.pm
+++ b/src/PVE/QemuServer/Network.pm
@@ -272,6 +272,25 @@ sub add_random_macs {
}
}
+# an edit made during the migration went to the source node, every resume
+# pushes the current mapping so the target catches up. An edit that started
+# before the config moved (so got proxied to the source node) and ended after
+# the resume on the target node (wrote the cache after the resume) is not
+# covered. The next guest start or reservation update of that zone on the
+# target catches up.
+sub add_nets_dhcp_mapping {
+ my ($conf) = @_;
+
+ for my $opt (keys %$conf) {
+ next if $opt !~ m/^net\d+$/;
+ my $net = parse_net($conf->{$opt}, 1) or next;
+ next if !$net->{bridge} || !$net->{macaddr};
+ # a resume does not fail on its DHCP backend
+ eval { PVE::Network::SDN::Dhcp::update_mapping($net->{bridge}, $net->{macaddr}) };
+ warn $@ if $@;
+ }
+}
+
sub add_nets_bridge_fdb {
my ($conf, $vmid) = @_;
diff --git a/src/PVE/QemuServer/RunState.pm b/src/PVE/QemuServer/RunState.pm
index 889ddd28287b..828d79159e90 100644
--- a/src/PVE/QemuServer/RunState.pm
+++ b/src/PVE/QemuServer/RunState.pm
@@ -181,6 +181,8 @@ sub vm_resume($vmid, $skiplock = undef, $nocheck = undef) {
PVE::QemuServer::Network::add_nets_bridge_fdb($conf, $vmid)
if $resume_cmd eq 'cont';
+ PVE::QemuServer::Network::add_nets_dhcp_mapping($conf)
+ if $resume_cmd eq 'cont';
mon_cmd($vmid, $resume_cmd);
},
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH pve-docs v2 11/11] sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM
2026-11-03 15:15 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (9 preceding siblings ...)
2026-11-03 15:15 ` [PATCH qemu-server v2 10/11] network: sdn: push the DHCP mappings of a guest when it resumes Hannes Laimer
@ 2026-11-03 15:15 ` Hannes Laimer
2026-11-03 15:18 ` [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-03 18:57 ` superseded: " Hannes Laimer
12 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 15:15 UTC (permalink / raw)
To: pve-devel
Every zone's subnets now go into a VRF or a sub-section named after
the zone, which is what an admin sees in the IPAM, and a prefix or
subnet from before stays where it is.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
pvesdn.adoc | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/pvesdn.adoc b/pvesdn.adoc
index c85d86a48112..194ab62f7dcb 100644
--- a/pvesdn.adoc
+++ b/pvesdn.adoc
@@ -1287,6 +1287,21 @@ Fingerprint:: The SHA-256 fingerprint of the NetBox API. Can be retrieved with
`openssl x509 -in /etc/ssl/certs/netbox.crt -noout -fingerprint -sha256` when
using the default certificate.
+{pve} creates a VRF named `pve-<zone>` for every zone, together with the first
+subnet of the zone, and keeps the prefixes of the zone's subnets in it. This way
+two zones can use the same subnet. The IP addresses allocated in a prefix carry
+the MAC address of the guest in their description. Prefixes that earlier
+versions created in the global table are still found there and can be moved
+into the VRF of their zone, together with their IP addresses and IP ranges. A
+zone from before gets its VRF only with the next subnet added to it, so to move
+prefixes into it earlier, create the VRF `pve-<zone>` by hand. Creating a
+subnet whose range the global table still holds is refused, since a prefix in
+the VRF would hide the addresses recorded in the old one, so move or remove
+that prefix first. While a rolling upgrade is under way, a node not yet upgraded
+puts an explicit mapping into the global table, also for a subnet created on an
+upgraded node. Move such addresses into the zone's VRF like the ones from
+before.
+
[[pvesdn_ipam_plugin_phpipam]]
phpIPAM Plugin
@@ -1304,6 +1319,22 @@ Token:: An API access token
Section:: An integer ID. Sections are a group of subnets in phpIPAM. Default
installations use `sectionid=1` for customers.
+{pve} creates a sub-section named `pve-<zone>` under the configured section for
+every zone, together with the first subnet of the zone, and keeps the zone's
+subnets in it. This way two zones can use the same subnet. phpIPAM nests
+sections one level deep, so a configured section that is itself a sub-section
+gets no sub-sections and the subnets of all zones stay in it. Subnets that
+earlier versions created in the configured section itself are still found there
+and can be moved into the sub-section of their zone. A zone from before gets its
+sub-section only with the next subnet added to it, so to move subnets into it
+earlier, create the sub-section `pve-<zone>` under the configured section by
+hand. Creating a subnet whose range the configured section still holds is
+refused, since a copy in the sub-section would hide the addresses recorded in
+the old one, so move or remove that subnet first. Section names are global in
+phpIPAM, so zone IDs must be unique across the clusters sharing one. A zone
+whose name another cluster's section carries keeps its subnets in the
+configured section and cannot get a sub-section of its own.
+
[[pvesdn_config_dns]]
DNS
--
2.47.3
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet
2026-11-03 15:15 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (10 preceding siblings ...)
2026-11-03 15:15 ` [PATCH pve-docs v2 11/11] sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM Hannes Laimer
@ 2026-11-03 15:18 ` Hannes Laimer
2026-11-03 18:57 ` superseded: " Hannes Laimer
12 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 15:18 UTC (permalink / raw)
To: pve-devel
should be v3 in the mail prefix, not v2, sorry
On 2026-11-03 16:16, Hannes Laimer wrote:
> This series addresses some problems in the IPAM code, mostly around
> caching. It also solves the problem that the MAC cache and the lookup
> behind it held a single address per MAC. This is a problem since MACs
> only have to be unique within a vnet and can repeat across vnets, and
> it led to a guest getting the IP of a guest with the same MAC on
> another vnet.
>
> Patches 1-5 address general, mostly cache related problems. Patches
> 1-4 were already included in [1], 1-3 changed since then. 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 5 is new.
>
> 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.
>
> 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 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 on two vnets can
> match `macs` on one of them only, so the other one asks the IPAM at
> every start. 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 that vnet 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.
> - NetBox: each zone gets its own `pve-<zone>` VRF, and the prefixes
> of its subnets go in there
> - phpIPAM: each zone gets a sub-section `pve-<zone>` under the
> configured one, and its subnets go in there(only if the configured
> section is not a sub-section already, phpIPAM only allows 1 deep
> nesting). Section names are global in phpIPAM, so zone ids must
> be unique across the clusters sharing one
>
> Patch 10 is qemu-server's side. The vmlist names the source of a live
> migration until the config moves at its end, so an edit in that
> window updates the source. Every resume now pushes the current
> mapping of each NIC, and the migration's last step resumes the target
> after the move, so it catches up there. The patch needs the
> pve-network of this series. Taken from [1] as well.
>
> Patch 11 just adds docs explaining this for the two external IPAMs.
>
> This is not a breaking change, existing assignments are returned
> exactly like they are already. Cache entries from before keep their
> addresses, since nodes not yet upgraded still use them, and answer for
> a vnet with the addresses in its subnets, so a running guest keeps its
> reservation. Records in an external IPAM stay where they are, in
> NetBox's global table or in phpIPAM's configured section. Like the
> cache, an IPAM lookup first checks the zone's VRF or section for the
> subnet and falls back to the global table or the configured section.
> Unlike with the cache, migration can be done manually, by moving a
> prefix together with its addresses and IP ranges into the zone's VRF
> in NetBox, or a subnet into the zone's sub-section in phpIPAM, since
> the lookup checks those first. Migration is not necessary though.
> 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 refused, since a copy would hide
> the records in it. The error says to move that prefix or subnet into
> the zone's VRF or sub-section, or to remove it. We don't move
> anything and we don't have to.
> If users are bothered by guests getting IPs not assigned to them, they
> can move the global entries into the `pve-<zone>` VRF or sub-section,
> where they'll be picked up correctly. While a rolling upgrade is
> under way, a node not yet upgraded puts an explicit mapping into
> NetBox's global table, also for a subnet created on an upgraded
> node, such addresses get moved like the ones from before.
>
> note: I'll rebase [1] onto this, it'll lose some patches that moved
> here. With [1] things addressed here become more apparent/visible
> eventhough they are pre-existing, that is why [1] will be based on this.
>
> [1] https://lore.proxmox.com/pve-devel/20261021084445.462195-1-h.laimer@proxmox.com/
>
> 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: write the lookup's cache entry 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
>
> src/PVE/API2/Network/SDN/Ips.pm | 38 +-
> src/PVE/Network/SDN/Dhcp.pm | 56 +-
> src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 153 +++--
> src/PVE/Network/SDN/Dhcp/Plugin.pm | 7 +-
> src/PVE/Network/SDN/Ipams.pm | 180 ++++-
> src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 223 +++++--
> src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 27 +-
> src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 237 ++++++-
> src/PVE/Network/SDN/Ipams/Plugin.pm | 7 +-
> src/PVE/Network/SDN/Subnets.pm | 14 +-
> src/PVE/Network/SDN/Vnets.pm | 21 +-
> src/test/ipams/netbox/expected.add_ip | 2 +-
> .../ipams/netbox/expected.add_ip_notgateway | 2 +-
> src/test/ipams/netbox/expected.add_subnet | 2 +-
> src/test/ipams/netbox/expected.update_ip | 2 +-
> src/test/run_test_ipams.pl | 468 +++++++++++++
> src/test/run_test_vnets_blackbox.pl | 618 +++++++++++++++++-
> 17 files changed, 1836 insertions(+), 221 deletions(-)
>
>
> qemu-server:
>
> Hannes Laimer (1):
> network: sdn: push the DHCP mappings of a guest when it resumes
>
> src/PVE/QemuServer/Network.pm | 19 +++++++++++++++++++
> src/PVE/QemuServer/RunState.pm | 2 ++
> 2 files changed, 21 insertions(+)
>
>
> pve-docs:
>
> Hannes Laimer (1):
> sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM
>
> pvesdn.adoc | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
>
> Summary over all repositories:
> 20 files changed, 1888 insertions(+), 221 deletions(-)
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* superseded: [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet
2026-11-03 15:15 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (11 preceding siblings ...)
2026-11-03 15:18 ` [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
@ 2026-11-03 18:57 ` Hannes Laimer
12 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2026-11-03 18:57 UTC (permalink / raw)
To: pve-devel
superseded-by:
https://lore.proxmox.com/pve-devel/20261103185559.420277-1-h.laimer@proxmox.com
On 2026-11-03 16:16, Hannes Laimer wrote:
> This series addresses some problems in the IPAM code, mostly around
> caching. It also solves the problem that the MAC cache and the lookup
> behind it held a single address per MAC. This is a problem since MACs
> only have to be unique within a vnet and can repeat across vnets, and
> it led to a guest getting the IP of a guest with the same MAC on
> another vnet.
>
> Patches 1-5 address general, mostly cache related problems. Patches
> 1-4 were already included in [1], 1-3 changed since then. 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 5 is new.
>
> 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.
>
> 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 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 on two vnets can
> match `macs` on one of them only, so the other one asks the IPAM at
> every start. 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 that vnet 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.
> - NetBox: each zone gets its own `pve-<zone>` VRF, and the prefixes
> of its subnets go in there
> - phpIPAM: each zone gets a sub-section `pve-<zone>` under the
> configured one, and its subnets go in there(only if the configured
> section is not a sub-section already, phpIPAM only allows 1 deep
> nesting). Section names are global in phpIPAM, so zone ids must
> be unique across the clusters sharing one
>
> Patch 10 is qemu-server's side. The vmlist names the source of a live
> migration until the config moves at its end, so an edit in that
> window updates the source. Every resume now pushes the current
> mapping of each NIC, and the migration's last step resumes the target
> after the move, so it catches up there. The patch needs the
> pve-network of this series. Taken from [1] as well.
>
> Patch 11 just adds docs explaining this for the two external IPAMs.
>
> This is not a breaking change, existing assignments are returned
> exactly like they are already. Cache entries from before keep their
> addresses, since nodes not yet upgraded still use them, and answer for
> a vnet with the addresses in its subnets, so a running guest keeps its
> reservation. Records in an external IPAM stay where they are, in
> NetBox's global table or in phpIPAM's configured section. Like the
> cache, an IPAM lookup first checks the zone's VRF or section for the
> subnet and falls back to the global table or the configured section.
> Unlike with the cache, migration can be done manually, by moving a
> prefix together with its addresses and IP ranges into the zone's VRF
> in NetBox, or a subnet into the zone's sub-section in phpIPAM, since
> the lookup checks those first. Migration is not necessary though.
> 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 refused, since a copy would hide
> the records in it. The error says to move that prefix or subnet into
> the zone's VRF or sub-section, or to remove it. We don't move
> anything and we don't have to.
> If users are bothered by guests getting IPs not assigned to them, they
> can move the global entries into the `pve-<zone>` VRF or sub-section,
> where they'll be picked up correctly. While a rolling upgrade is
> under way, a node not yet upgraded puts an explicit mapping into
> NetBox's global table, also for a subnet created on an upgraded
> node, such addresses get moved like the ones from before.
>
> note: I'll rebase [1] onto this, it'll lose some patches that moved
> here. With [1] things addressed here become more apparent/visible
> eventhough they are pre-existing, that is why [1] will be based on this.
>
> [1] https://lore.proxmox.com/pve-devel/20261021084445.462195-1-h.laimer@proxmox.com/
>
> 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: write the lookup's cache entry 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
>
> src/PVE/API2/Network/SDN/Ips.pm | 38 +-
> src/PVE/Network/SDN/Dhcp.pm | 56 +-
> src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 153 +++--
> src/PVE/Network/SDN/Dhcp/Plugin.pm | 7 +-
> src/PVE/Network/SDN/Ipams.pm | 180 ++++-
> src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 223 +++++--
> src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 27 +-
> src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 237 ++++++-
> src/PVE/Network/SDN/Ipams/Plugin.pm | 7 +-
> src/PVE/Network/SDN/Subnets.pm | 14 +-
> src/PVE/Network/SDN/Vnets.pm | 21 +-
> src/test/ipams/netbox/expected.add_ip | 2 +-
> .../ipams/netbox/expected.add_ip_notgateway | 2 +-
> src/test/ipams/netbox/expected.add_subnet | 2 +-
> src/test/ipams/netbox/expected.update_ip | 2 +-
> src/test/run_test_ipams.pl | 468 +++++++++++++
> src/test/run_test_vnets_blackbox.pl | 618 +++++++++++++++++-
> 17 files changed, 1836 insertions(+), 221 deletions(-)
>
>
> qemu-server:
>
> Hannes Laimer (1):
> network: sdn: push the DHCP mappings of a guest when it resumes
>
> src/PVE/QemuServer/Network.pm | 19 +++++++++++++++++++
> src/PVE/QemuServer/RunState.pm | 2 ++
> 2 files changed, 21 insertions(+)
>
>
> pve-docs:
>
> Hannes Laimer (1):
> sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM
>
> pvesdn.adoc | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
>
> Summary over all repositories:
> 20 files changed, 1888 insertions(+), 221 deletions(-)
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-11-03 18:58 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03 15:15 [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 01/11] sdn: ipam: stop caching IPAM misses Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 02/11] sdn: ipam: write the lookup's cache entry under the cluster lock Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 03/11] sdn: ipam: read the MAC cache case-insensitively Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 04/11] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 05/11] sdn: ipam: release an address the built-in IPAM no longer holds Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 06/11] sdn: dhcp: update reservations when ip mappings change Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 07/11] sdn: ipam: look a MAC up within the vnet's subnets Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 08/11] sdn: ipam: keep the MAC cache per vnet Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-network v2 09/11] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM Hannes Laimer
2026-11-03 15:15 ` [PATCH qemu-server v2 10/11] network: sdn: push the DHCP mappings of a guest when it resumes Hannes Laimer
2026-11-03 15:15 ` [PATCH pve-docs v2 11/11] sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM Hannes Laimer
2026-11-03 15:18 ` [PATCH docs/network/qemu-server v2 00/11] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-03 18:57 ` superseded: " Hannes Laimer
-- strict thread matches above, loose matches on Subject: below --
2026-11-02 14:56 Hannes Laimer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox