* [PATCH docs/network 00/10] sdn: improve MAC cache, DHCP reservations and scope both to the vnet
@ 2026-11-02 11:05 Hannes Laimer
2026-11-02 11:05 ` [PATCH pve-network 01/10] sdn: ipam: stop caching IPAM misses Hannes Laimer
` (9 more replies)
0 siblings, 10 replies; 11+ messages in thread
From: Hannes Laimer @ 2026-11-02 11:05 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` is not converted, its entries stay
as written until their addresses are released, since nodes not yet
upgraded still use them. They serve as fallback for a vnet, but 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 the
new `vnets` map. 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 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 stay as
written, 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. 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.
[1] https://lore.proxmox.com/pve-devel/20261021084445.462195-1-h.laimer@proxmox.com/
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 | 33 +-
src/PVE/Network/SDN/Dhcp.pm | 56 +-
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 137 +++--
src/PVE/Network/SDN/Dhcp/Plugin.pm | 6 +-
src/PVE/Network/SDN/Ipams.pm | 108 +++-
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 198 +++++--
src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 27 +-
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 181 +++++--
src/PVE/Network/SDN/Ipams/Plugin.pm | 7 +-
src/PVE/Network/SDN/Subnets.pm | 11 +-
src/PVE/Network/SDN/Vnets.pm | 16 +-
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 | 258 ++++++++++
src/test/run_test_vnets_blackbox.pl | 487 +++++++++++++++++-
17 files changed, 1327 insertions(+), 206 deletions(-)
pve-docs:
Hannes Laimer (1):
sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM
pvesdn.adoc | 15 +++++++++++++++
1 file changed, 15 insertions(+)
Summary over all repositories:
18 files changed, 1342 insertions(+), 206 deletions(-)
--
Generated by murpp 0.12.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH pve-network 01/10] sdn: ipam: stop caching IPAM misses
2026-11-02 11:05 [PATCH docs/network 00/10] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
@ 2026-11-02 11:05 ` Hannes Laimer
2026-11-02 11:05 ` [PATCH pve-network 02/10] sdn: ipam: write the lookup's cache entry under the cluster lock Hannes Laimer
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Hannes Laimer @ 2026-11-02 11:05 UTC (permalink / raw)
To: pve-devel
A lookup that missed the cache stored the IPAM's empty answer, and
any entry counted as a hit. So a record created later directly in the
IPAM stayed hidden, and a guest start allocated another address. An
empty answer is no longer stored, and an entry without an address, as
earlier misses left them, no longer counts as a hit.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams.pm | 16 ++++++--
src/test/run_test_vnets_blackbox.pl | 58 ++++++++++++++++++++++++++++-
2 files changed, 69 insertions(+), 5 deletions(-)
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 179bdf76defc..7e7bec27b5ed 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -136,16 +136,24 @@ sub get_ips_from_mac {
my ($mac, $zoneid, $zone) = @_;
my $macdb = read_macdb();
- return ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6}) if $macdb->{macs}->{$mac};
+ my $cached = $macdb->{macs}->{$mac};
+ # an entry without an address was left behind by a lookup that missed, and
+ # says nothing about its MAC
+ return ($cached->{ip4}, $cached->{ip6})
+ if $cached && (defined($cached->{ip4}) || defined($cached->{ip6}));
my $plugin_config = get_plugin_config($zone);
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
- ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6}) =
- $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid);
+ my ($ip4, $ip6) = $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid);
+ # an empty answer is not cached, since the record may simply not exist yet
+ return if !defined($ip4) && !defined($ip6);
+
+ $macdb->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4);
+ $macdb->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6);
write_macdb($macdb);
- return ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6});
+ return ($ip4, $ip6);
}
1;
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 9f4c424f3d2f..9724f1a8e4ef 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -8,6 +8,7 @@ use File::Slurp;
use List::Util qw(first all);
use NetAddr::IP qw(:lower);
+use JSON;
use Test::More;
use Test::MockModule;
use Test::Differences;
@@ -193,7 +194,7 @@ $mocked_sdn_ipams->mock(
$test_state->{ipam_config} = $cfg;
},
read_macdb => sub {
- return $test_state->{macdb};
+ return PVE::Network::SDN::Ipams::json_reader(undef, encode_json($test_state->{macdb}));
},
write_macdb => sub {
my ($cfg) = @_;
@@ -321,6 +322,34 @@ sub create_subnet {
PVE::API2::Network::SDN::Subnets->create($params);
}
+# a DHCP zone with one vnet, each subnet with its first address as the
+# gateway and a range over its hosts 100 to 200
+sub create_dhcp_zone {
+ my ($zoneid, $vnetid, @cidrs) = @_;
+ create_zone({ type => "simple", dhcp => "dnsmasq", ipam => "pve", zone => $zoneid });
+ create_dhcp_vnet($zoneid, $vnetid, @cidrs);
+}
+
+sub create_dhcp_vnet {
+ my ($zoneid, $vnetid, @cidrs) = @_;
+ create_vnet({ type => "vnet", zone => $zoneid, vnet => $vnetid });
+ create_dhcp_subnet($vnetid, $_) for @cidrs;
+}
+
+sub create_dhcp_subnet {
+ my ($vnetid, $cidr) = @_;
+ my $net = NetAddr::IP->new($cidr);
+ my $addr = sub { my ($ip) = @_; return $ip->version() == 4 ? $ip->addr() : $ip->short(); };
+ create_subnet({
+ type => "subnet",
+ vnet => $vnetid,
+ subnet => $cidr,
+ gateway => $addr->($net->first()),
+ 'dhcp-range' =>
+ ["start-address=" . $addr->($net + 100) . ",end-address=" . $addr->($net + 200)],
+ });
+}
+
sub get_ipam_entries {
return PVE::API2::Network::SDN::Ipams->ipamindex({ ipam => "pve" });
}
@@ -421,6 +450,33 @@ sub test_without_subnet {
}
run_test(\&test_without_subnet);
+sub test_ipam_cache_lookup {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+ my $cache = decode_json(encode_json($test_state->{macdb}));
+ PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is_deeply($test_state->{macdb}, $cache, "$test_name: the miss is not cached");
+
+ # an entry holding no address, left behind by an earlier miss, does not
+ # hide a record created since
+ create_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.50" });
+ $test_state->{macdb}->{macs}->{$mac} = { ip4 => undef, ip6 => undef };
+ my @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is($ips[0], "10.0.0.50", "$test_name: an address-less entry counts as a miss");
+ is(
+ $test_state->{macdb}->{macs}->{$mac}->{ip4},
+ "10.0.0.50",
+ "$test_name: the answer is cached",
+ );
+}
+
+run_test(\&test_ipam_cache_lookup);
+
sub test_nic_join {
my ($test_name, $subnets) = @_;
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH pve-network 02/10] sdn: ipam: write the lookup's cache entry under the cluster lock
2026-11-02 11:05 [PATCH docs/network 00/10] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-02 11:05 ` [PATCH pve-network 01/10] sdn: ipam: stop caching IPAM misses Hannes Laimer
@ 2026-11-02 11:05 ` Hannes Laimer
2026-11-02 11:05 ` [PATCH pve-network 03/10] sdn: ipam: read the MAC cache case-insensitively Hannes Laimer
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Hannes Laimer @ 2026-11-02 11:05 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 9724f1a8e4ef..fff5858f9c27 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -473,6 +473,17 @@ sub test_ipam_cache_lookup {
"10.0.0.50",
"$test_name: the answer is cached",
);
+
+ # a write to the cache between the miss and the fill, which the file's
+ # version shows, keeps the IPAM's answer out of the cache
+ delete $test_state->{macdb}->{macs}->{$mac};
+ my $version = 0;
+ $mocked_pve_cluster_obj->mock(cfs_file_version => sub { return $version++ });
+ @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ $mocked_pve_cluster_obj->unmock('cfs_file_version');
+ is($ips[0], "10.0.0.50", "$test_name: the lookup still answers");
+ ok(!exists $test_state->{macdb}->{macs}->{$mac},
+ "$test_name: but leaves a changed cache alone");
}
run_test(\&test_ipam_cache_lookup);
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH pve-network 03/10] sdn: ipam: read the MAC cache case-insensitively
2026-11-02 11:05 [PATCH docs/network 00/10] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-02 11:05 ` [PATCH pve-network 01/10] sdn: ipam: stop caching IPAM misses Hannes Laimer
2026-11-02 11:05 ` [PATCH pve-network 02/10] sdn: ipam: write the lookup's cache entry under the cluster lock Hannes Laimer
@ 2026-11-02 11:05 ` Hannes Laimer
2026-11-02 11:05 ` [PATCH pve-network 04/10] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it Hannes Laimer
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Hannes Laimer @ 2026-11-02 11:05 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 fff5858f9c27..612d2eb7b71f 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -193,7 +193,7 @@ $mocked_sdn_ipams->mock(
my ($cfg) = @_;
$test_state->{ipam_config} = $cfg;
},
- read_macdb => sub {
+ read_macdb_file => sub {
return PVE::Network::SDN::Ipams::json_reader(undef, encode_json($test_state->{macdb}));
},
write_macdb => sub {
@@ -484,6 +484,55 @@ sub test_ipam_cache_lookup {
is($ips[0], "10.0.0.50", "$test_name: the lookup still answers");
ok(!exists $test_state->{macdb}->{macs}->{$mac},
"$test_name: but leaves a changed cache alone");
+
+ # an entry written under the config's spelling, as a node not yet upgraded
+ # writes it, is read and released like any other and keeps its key
+ my $other = "da:65:8f:18:9b:70";
+ $test_state->{macdb}->{macs} = {
+ uc($mac) => { ip4 => '10.0.0.150' },
+ uc($other) => { ip4 => '10.0.0.170' },
+ };
+ my ($ip4) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is($ip4, '10.0.0.150', "$test_name: an entry is found however it is spelled");
+ PVE::Network::SDN::Ipams::del_cache_mac_ip($mac, '10.0.0.150');
+ is_deeply(
+ $test_state->{macdb}->{macs},
+ { uc($other) => { ip4 => '10.0.0.170' } },
+ "$test_name: and released, the other entry left as written",
+ );
+
+ # a write in another spelling goes to the spelling the file holds
+ $test_state->{macdb}->{macs} = { uc($mac) => { ip4 => '10.0.0.150' } };
+ PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, 'fd00::150');
+ is_deeply(
+ $test_state->{macdb}->{macs},
+ { uc($mac) => { ip4 => '10.0.0.150', ip6 => 'fd00::150' } },
+ "$test_name: a write keeps the key as written",
+ );
+
+ # a lookup past an address-less entry asks the IPAM and fills the entry
+ # under its spelling
+ $test_state->{macdb}->{macs} = { uc($mac) => { ip4 => undef, ip6 => undef } };
+ @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is($ips[0], '10.0.0.50', "$test_name: a lookup past an address-less entry asks the IPAM");
+ is_deeply(
+ $test_state->{macdb}->{macs},
+ { uc($mac) => { ip4 => '10.0.0.50', ip6 => undef } },
+ "$test_name: and fills the entry under its spelling",
+ );
+
+ # two spellings that disagree are not trusted, the lookup asks the IPAM
+ $test_state->{macdb}->{macs} = {
+ uc($mac) => { ip4 => '10.0.0.150' },
+ $mac => { ip4 => '10.0.0.151' },
+ };
+ @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is($ips[0], '10.0.0.50', "$test_name: a clash of spellings counts as a miss");
+ is_deeply(
+ $test_state->{macdb}->{macs},
+ { uc($mac) => { ip4 => '10.0.0.50' }, $mac => { ip4 => '10.0.0.50' } },
+ "$test_name: and its write brings both spellings in line",
+ );
}
run_test(\&test_ipam_cache_lookup);
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH pve-network 04/10] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it
2026-11-02 11:05 [PATCH docs/network 00/10] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (2 preceding siblings ...)
2026-11-02 11:05 ` [PATCH pve-network 03/10] sdn: ipam: read the MAC cache case-insensitively Hannes Laimer
@ 2026-11-02 11:05 ` Hannes Laimer
2026-11-02 11:05 ` [PATCH pve-network 05/10] sdn: ipam: release an address the built-in IPAM no longer holds Hannes Laimer
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Hannes Laimer @ 2026-11-02 11:05 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 612d2eb7b71f..0f1b181d8a33 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -1079,4 +1079,32 @@ run_test(
2,
);
+sub test_dnsmasq_stale_reservation_replaced {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+
+ my $stale = "da:65:8f:18:9b:01";
+ my $gone = "da:65:8f:18:9b:02";
+ my $added = "da:65:8f:18:9b:03";
+ $test_state->{macdb} = {
+ macs => {
+ $stale => { ip4 => '10.0.1.11' },
+ $added => { ip4 => '10.0.1.13' },
+ },
+ };
+ PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$stale,10.0.1.10\n$gone,10.0.1.12\n");
+
+ PVE::Network::SDN::Dhcp::Dnsmasq->add_ip_mapping(
+ 'TESTZONE', $test_state->{macdb}, $added, '10.0.1.13', undef,
+ );
+
+ my @lines = sort grep { /\S/ } split(/\n/, PVE::Tools::file_get_contents($TMP_ETHERS_FILE));
+ eq_or_diff(
+ \@lines,
+ ["$stale,10.0.1.11", "$added,10.0.1.13"],
+ "$test_name: the stale line is replaced, the unknown one dropped",
+ );
+}
+
+run_test(\&test_dnsmasq_stale_reservation_replaced);
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH pve-network 05/10] sdn: ipam: release an address the built-in IPAM no longer holds
2026-11-02 11:05 [PATCH docs/network 00/10] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (3 preceding siblings ...)
2026-11-02 11:05 ` [PATCH pve-network 04/10] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it Hannes Laimer
@ 2026-11-02 11:05 ` Hannes Laimer
2026-11-02 11:06 ` [PATCH pve-network 06/10] sdn: dhcp: update reservations when ip mappings change Hannes Laimer
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Hannes Laimer @ 2026-11-02 11:05 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 0f1b181d8a33..fc301a54de5f 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -533,6 +533,23 @@ sub test_ipam_cache_lookup {
{ uc($mac) => { ip4 => '10.0.0.50' }, $mac => { ip4 => '10.0.0.50' } },
"$test_name: and its write brings both spellings in line",
);
+
+ # a mapping whose record is gone is still deleted, and its cache entry
+ # with it
+ $test_state->{macdb} = {};
+ create_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.70" });
+ delete $test_state->{ipamdb}->{zones}->{$zoneid}->{subnets}->{"10.0.0.0/24"}->{ips}
+ ->{"10.0.0.70"};
+ eval {
+ PVE::API2::Network::SDN::Ips->ipdelete(
+ { zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.70" });
+ };
+ is($@, '', "$test_name: a mapping whose record is gone is deleted without an error");
+ is(
+ (PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac))[0],
+ '10.0.0.50',
+ "$test_name: and the cache no longer answers with its address",
+ );
}
run_test(\&test_ipam_cache_lookup);
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH pve-network 06/10] sdn: dhcp: update reservations when ip mappings change
2026-11-02 11:05 [PATCH docs/network 00/10] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (4 preceding siblings ...)
2026-11-02 11:05 ` [PATCH pve-network 05/10] sdn: ipam: release an address the built-in IPAM no longer holds Hannes Laimer
@ 2026-11-02 11:06 ` Hannes Laimer
2026-11-02 11:06 ` [PATCH pve-network 07/10] sdn: ipam: look a MAC up within the vnet's subnets Hannes Laimer
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Hannes Laimer @ 2026-11-02 11:06 UTC (permalink / raw)
To: pve-devel
Creating, updating or deleting an IP mapping through the API changed
the IPAM and the MAC cache, but never the zone's dnsmasq reservation
file. dnsmasq kept serving the old reservation, or none for a new
mapping, until the guest's next start.
A guest start, an API edit and the deletion of a NIC's addresses now
all rewrite the MAC's reservation from the cache. The reservations
are a file on each node, and only the guest's node answers its DHCP
requests, so an API edit is proxied to the node of the guest whose
NIC carries the MAC on that vnet. The other nodes catch up when the
guest starts on one of them. A failed update only warns, since the
IPAM records are already written.
The rewrite refreshes the cluster state and reads the cache only
once it holds the file's lock, so a rewrite that ran in between is
not undone. The MAC's line is written in full from the cache, so a
guest gaining an IPv6 address next to its IPv4 one gets it reserved
too.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/API2/Network/SDN/Ips.pm | 28 +++++
src/PVE/Network/SDN/Dhcp.pm | 56 ++++++----
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 104 ++++++++++-------
src/PVE/Network/SDN/Dhcp/Plugin.pm | 6 +-
src/PVE/Network/SDN/Vnets.pm | 5 +-
src/test/run_test_vnets_blackbox.pl | 168 ++++++++++++++++++++++++++--
6 files changed, 294 insertions(+), 73 deletions(-)
diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
index 5ff05e75e30a..b1faee71c6ba 100644
--- a/src/PVE/API2/Network/SDN/Ips.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -3,9 +3,11 @@ package PVE::API2::Network::SDN::Ips;
use strict;
use warnings;
+use PVE::Cluster;
use PVE::Tools qw(extract_param);
use PVE::Network::SDN::Vnets;
+use PVE::Network::SDN::Zones;
use PVE::Network::SDN::Dhcp;
use PVE::JSONSchema qw(get_standard_option);
@@ -13,12 +15,29 @@ use PVE::RESTHandler;
use base qw(PVE::RESTHandler);
+# only the guest's node answers its DHCP requests, so the edit runs there.
+# With that node offline it runs here, and the node catches up at the
+# guest's next start
+my $proxy_to_guest_node = sub {
+ my ($rpcenv, $proxyto, $param) = @_;
+
+ my $vnet = PVE::Network::SDN::Vnets::get_vnet($param->{vnet}, 1);
+ my $zone = $vnet ? PVE::Network::SDN::Zones::get_zone($vnet->{zone}, 1) : undef;
+ return 'localhost' if !$zone || !$zone->{dhcp};
+
+ my $node = PVE::Network::SDN::Dhcp::guest_node_by_mac($param->{vnet}, $param->{mac});
+ my $members = PVE::Cluster::get_members();
+ return 'localhost' if !$node || ($members && !($members->{$node} // {})->{online});
+ return $node;
+};
+
__PACKAGE__->register_method({
name => 'ipdelete',
path => '',
method => 'DELETE',
description => 'Delete IP Mappings in a VNet',
protected => 1,
+ proxyto_callback => $proxy_to_guest_node,
permissions => {
check => ['perm', '/sdn/zones/{zone}/{vnet}', ['SDN.Allocate']],
},
@@ -46,6 +65,8 @@ __PACKAGE__->register_method({
eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
die "$@\n" if $@;
+ PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+
return undef;
},
});
@@ -56,6 +77,7 @@ __PACKAGE__->register_method({
method => 'POST',
description => 'Create IP Mapping in a VNet',
protected => 1,
+ proxyto_callback => $proxy_to_guest_node,
permissions => {
check => ['perm', '/sdn/zones/{zone}/{vnet}', ['SDN.Allocate']],
},
@@ -82,6 +104,8 @@ __PACKAGE__->register_method({
PVE::Network::SDN::Vnets::add_ip($vnet, $ip, '', $mac, undef);
+ PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+
return undef;
},
});
@@ -91,6 +115,7 @@ __PACKAGE__->register_method({
method => 'PUT',
description => 'Update IP Mapping in a VNet',
protected => 1,
+ proxyto_callback => $proxy_to_guest_node,
permissions => {
check => ['perm', '/sdn/zones/{zone}/{vnet}', ['SDN.Allocate']],
},
@@ -132,6 +157,9 @@ __PACKAGE__->register_method({
}
die "$error\n" if $error;
+
+ PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+
return undef;
},
});
diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
index 65e40d40465a..4650270df09f 100644
--- a/src/PVE/Network/SDN/Dhcp.pm
+++ b/src/PVE/Network/SDN/Dhcp.pm
@@ -6,7 +6,6 @@ use warnings;
use PVE::Cluster;
use PVE::Network::SDN;
-use PVE::Network::SDN::Ipams;
use PVE::Network::SDN::Subnets;
use PVE::Network::SDN::Dhcp::Plugin;
use PVE::Network::SDN::Dhcp::Dnsmasq;
@@ -22,37 +21,50 @@ sub plugin_types {
return PVE::Network::SDN::Dhcp::Plugin->lookup_types();
}
-sub add_mapping {
- my ($vnetid, $mac, $ip4, $ip6) = @_;
+sub update_mapping {
+ my ($vnetid, $mac) = @_;
- my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid);
- return if !$vnet;
+ # a record without a MAC has no guest NIC behind it, so no backend serves it
+ return if !$mac;
- my $zoneid = $vnet->{zone};
- my $zone = PVE::Network::SDN::Zones::get_zone($zoneid);
+ eval {
+ # the backend to push to is the applied one, since a zone edited but
+ # not applied has nothing running yet
+ my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid, 1);
+ return if !$vnet;
- return if !$zone->{ipam} || !$zone->{dhcp};
+ my $zoneid = $vnet->{zone};
+ my $zone = PVE::Network::SDN::Zones::get_zone($zoneid, 1);
+ return if !$zone || !$zone->{ipam} || !$zone->{dhcp};
- my $dhcptype = $zone->{dhcp};
+ # a zone confined to other nodes has no vnet bridge on this node
+ return if defined($zone->{nodes}) && !$zone->{nodes}->{ PVE::INotify::nodename() };
- my $macdb = PVE::Network::SDN::Ipams::read_macdb();
- my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($dhcptype);
- $dhcp_plugin->add_ip_mapping($zoneid, $macdb, $mac, $ip4, $ip6);
+ PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp})
+ ->update_ip_mapping($zoneid, $mac);
+ };
+ # the records are written, so a failed update only warns
+ warn "could not update dhcp mapping for $mac: $@" if $@;
}
-sub remove_mapping {
+sub guest_node_by_mac {
my ($vnetid, $mac) = @_;
- my $vnet = PVE::Network::SDN::Vnets::get_vnet($vnetid);
- return if !$vnet;
-
- my $zoneid = $vnet->{zone};
- my $zone = PVE::Network::SDN::Zones::get_zone($zoneid);
-
- return if !$zone->{ipam} || !$zone->{dhcp};
+ my $vmlist = PVE::Cluster::get_vmlist();
+ my $nets = PVE::Cluster::get_guest_config_properties([map { "net$_" } 0 .. 31]);
+ for my $vmid (sort keys %$nets) {
+ my $guest = $vmlist->{ids}->{$vmid} // next;
+ for my $net (values %{ $nets->{$vmid} }) {
+ my ($bridge) = $net =~ m/(?:^|,)bridge=([^,]+)/ or next;
+ next if $bridge ne $vnetid;
+ # a VM names the MAC as the value of its model, a container as hwaddr
+ my ($nic_mac) = $net =~ m/(?:^|,)[a-z0-9_-]+=([0-9a-f]{2}(?::[0-9a-f]{2}){5})(?:,|$)/i
+ or next;
+ return $guest->{node} if lc($nic_mac) eq lc($mac);
+ }
+ }
- my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($zone->{dhcp});
- $dhcp_plugin->del_ip_mapping($zoneid, $mac);
+ return undef;
}
sub regenerate_config {
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index a1e4c70f9ba4..4767fb5162ad 100644
--- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
+++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
@@ -11,7 +11,9 @@ use PVE::Tools qw(file_set_contents run_command lock_file);
use File::Copy;
use Net::DBus;
+use PVE::Cluster;
use PVE::RESTEnvironment qw(log_warn);
+use PVE::Network::SDN::Ipams;
my $DNSMASQ_CONFIG_ROOT = '/etc/dnsmasq.d';
my $DNSMASQ_DEFAULT_ROOT = '/etc/default';
@@ -51,83 +53,100 @@ sub update_lease {
$manager->AddDhcpLease($ip4, $mac, \@hostname, undef, 0, 0, 0) if $ip4;
}
-sub add_ip_mapping {
- my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
+sub update_ip_mapping {
+ my ($class, $dhcpid, $mac) = @_;
$mac = lc($mac);
my $ethers_file = ethers_file($dhcpid);
my $ethers_tmp_file = "$ethers_file.tmp";
- my $reload = undef;
+ my ($ip4, $ip6);
+ my $reservation = undef;
+ my $changed = undef;
- my $appendFn = sub {
+ my $rewriteFn = sub {
+ # the lines are decided from the cache, which this process may hold an
+ # old read of, and the local lock does not refresh it
+ PVE::Cluster::cfs_update();
+
+ # read under the ethers lock. A snapshot from before it would sweep a
+ # line another push wrote meanwhile, and an older push would put its
+ # addresses back
+
+ my $macs = PVE::Network::SDN::Ipams::read_macdb()->{macs};
+
+ my $entry = $macs->{$mac};
+ ($ip4, $ip6) = ($entry->{ip4}, $entry->{ip6}) if $entry;
+ if ($ip4 || $ip6) {
+ $reservation = $mac;
+ $reservation .= ",$ip4" if $ip4;
+ $reservation .= ",[$ip6]" if $ip6;
+ # an external IPAM spells addresses its own way, while the file holds one spelling
+ $reservation = lc($reservation);
+ }
+
+ file_set_contents($ethers_file, '') if !-e $ethers_file;
open(my $in, '<', $ethers_file) or die "Could not open file '$ethers_file' $!\n";
open(my $out, '>', $ethers_tmp_file)
or die "Could not open file '$ethers_tmp_file' $!\n";
- my $match = undef;
+ my $kept = undef;
- my $line_no = 0;
while (my $line = <$in>) {
- $line_no++;
chomp($line);
+ next if $line !~ m/\S/;
my ($parsed_mac, $parsed_ip1, $parsed_ip2) = split(/,/, $line);
+ # a line from before the file was written in lower case keeps its writer's spelling
+ $parsed_mac = lc($parsed_mac);
- if (!defined($parsed_mac)) {
- warn "failed to parse MAC from $dhcpid ethers file on line $line_no: '$line'\n";
+ # the MAC's own reservation is rewritten. An unchanged one stays in place
+ if ($parsed_mac eq $mac) {
+ if (defined($reservation) && !$kept && lc($line) eq $reservation) {
+ $kept = 1;
+ print $out "$line\n";
+ } else {
+ $changed = 1;
+ }
next;
}
- # the cache is keyed in lower case, a line from before keeps its writer's spelling
- $parsed_mac = lc($parsed_mac);
my ($parsed_ip4, $parsed_ip6);
if ($parsed_ip2) {
$parsed_ip4 = $parsed_ip1;
$parsed_ip6 = $parsed_ip2;
- } elsif (Net::IP::ip_is_ipv4($parsed_ip1)) {
+ } elsif (defined($parsed_ip1) && Net::IP::ip_is_ipv4($parsed_ip1)) {
$parsed_ip4 = $parsed_ip1;
- } else {
+ } elsif (defined($parsed_ip1)) {
$parsed_ip6 = $parsed_ip1;
}
$parsed_ip6 = $1 if $parsed_ip6 && $parsed_ip6 =~ m/\[(\S+)\]/;
+ # a line from before addresses were lower-cased may spell one in upper case
+ $parsed_ip6 = lc($parsed_ip6) if $parsed_ip6;
# a line that no longer matches the cache is replaced by the cache's
# current one. Dropping it left that guest unknown to dnsmasq until
# its next start
+ my $current = $macs->{$parsed_mac};
if (
- !defined($macdb->{macs}->{$parsed_mac})
- || ($parsed_ip4
- && $macdb->{macs}->{$parsed_mac}->{'ip4'}
- && $macdb->{macs}->{$parsed_mac}->{'ip4'} ne $parsed_ip4)
- || ($parsed_ip6
- && $macdb->{macs}->{$parsed_mac}->{'ip6'}
- && $macdb->{macs}->{$parsed_mac}->{'ip6'} ne $parsed_ip6)
+ !$current
+ || ($parsed_ip4 && $current->{ip4} && $current->{ip4} ne $parsed_ip4)
+ || ($parsed_ip6 && $current->{ip6} && lc($current->{ip6}) ne $parsed_ip6)
) {
- $reload = 1;
- # the MAC being added gets its line below, like a missing one
- my $current = $macdb->{macs}->{$parsed_mac};
- next if $parsed_mac eq $mac || !$current;
+ $changed = 1;
+ next if !$current;
my $replacement = $parsed_mac;
$replacement .= ",$current->{ip4}" if $current->{ip4};
$replacement .= ",[$current->{ip6}]" if $current->{ip6};
- print $out "$replacement\n" if $replacement ne $parsed_mac;
+ print $out lc($replacement) . "\n";
next;
}
- if ($parsed_mac eq $mac) {
- $match = 1 if $ip4 && $parsed_ip4 && $ip4;
- $match = 1 if $ip6 && $parsed_ip6 && $ip6;
- }
-
print $out "$line\n";
}
- if (!$match) {
- my $reservation = $mac;
- $reservation .= ",$ip4" if $ip4;
- $reservation .= ",[$ip6]" if $ip6;
+ if (defined($reservation) && !$kept) {
print $out "$reservation\n";
- $reload = 1;
+ $changed = 1;
}
close $in;
@@ -136,16 +155,21 @@ sub add_ip_mapping {
chmod 0644, $ethers_file;
};
- PVE::Tools::lock_file($ethers_file, 10, $appendFn);
+ # the rewrite replaces the reservation file, so a lock on the file itself
+ # would not outlive the rename
+ PVE::Tools::lock_file("$ethers_file.lock", 10, $rewriteFn);
if ($@) {
- warn "Unable to add $mac to the dnsmasq configuration: $@\n";
+ warn "Unable to update $mac in the dnsmasq configuration: $@\n";
return;
}
- my $service_name = "dnsmasq\@$dhcpid";
- systemctl_service('reload', $service_name) if $reload;
- update_lease($dhcpid, $ip4, $mac);
+ systemctl_service('reload', "dnsmasq\@$dhcpid") if $changed;
+
+ if ($ip4) {
+ eval { update_lease($dhcpid, $ip4, $mac) };
+ log_warn("could not update the dnsmasq lease of $mac: $@") if $@;
+ }
}
sub configure_subnet {
diff --git a/src/PVE/Network/SDN/Dhcp/Plugin.pm b/src/PVE/Network/SDN/Dhcp/Plugin.pm
index b5d32fa13c70..a3a5ae24403f 100644
--- a/src/PVE/Network/SDN/Dhcp/Plugin.pm
+++ b/src/PVE/Network/SDN/Dhcp/Plugin.pm
@@ -22,8 +22,10 @@ sub private {
return $defaultData;
}
-sub add_ip_mapping {
- my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_;
+# the MAC's records changed, so make the backend serve the current ones. With no
+# record left its reservation goes
+sub update_ip_mapping {
+ my ($class, $dhcpid, $mac) = @_;
die 'implement in sub class';
}
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index c327a4beba81..8453a2781ae9 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -211,6 +211,8 @@ sub del_ips_from_mac {
PVE::Network::SDN::Vnets::del_ip($vnetid, $ip4, $hostname, $mac) if $ip4;
PVE::Network::SDN::Vnets::del_ip($vnetid, $ip6, $hostname, $mac) if $ip6;
+ PVE::Network::SDN::Dhcp::update_mapping($vnetid, $mac) if $ip4 || $ip6;
+
return ($ip4, $ip6);
}
@@ -225,11 +227,12 @@ sub add_dhcp_mapping {
return if !$zone->{ipam} || !$zone->{dhcp};
my ($ip4, $ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ # one push at the end covers the allocated and the existing records alike
add_next_free_cidr($vnetid, $name, $mac, "$vmid", undef, 1, 4) if !$ip4;
add_next_free_cidr($vnetid, $name, $mac, "$vmid", undef, 1, 6) if !$ip6;
($ip4, $ip6) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
- PVE::Network::SDN::Dhcp::add_mapping($vnetid, $mac, $ip4, $ip6) if $ip4 || $ip6;
+ PVE::Network::SDN::Dhcp::update_mapping($vnetid, $mac) if $ip4 || $ip6;
}
1;
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index fc301a54de5f..bde61f2d3401 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -47,6 +47,7 @@ sub clear_test_state {
vnets_config => {},
macdb => {},
ipamdb => {},
+ dnsmasq_calls => [],
ipam_config => {
'ids' => {
'pve' => {
@@ -221,7 +222,9 @@ $mocked_sdn_dhcp_dnsmasq->mock(
assert_dnsmasq_installed => sub { return 1; },
before_configure => sub { },
ethers_file => sub { return "/tmp/ethers"; },
- systemctl_service => sub { },
+ systemctl_service => sub {
+ push $test_state->{dnsmasq_calls}->@*, $_[0];
+ },
update_lease => sub { },
);
@@ -240,10 +243,18 @@ $mocked_rpc_env_obj->mock(
check_any => sub { return 1; },
);
+my $mocked_pve_inotify = Test::MockModule->new('PVE::INotify');
+$mocked_pve_inotify->mock(
+ nodename => sub { return 'localnode'; },
+);
+
my $mocked_pve_cluster_obj = Test::MockModule->new('PVE::Cluster');
$mocked_pve_cluster_obj->mock(
check_cfs_quorum => sub { return 1; },
cfs_lock_domain => $mocked_cfs_lock_domain,
+ cfs_update => sub { },
+ get_vmlist => sub { return $test_state->{vmlist} // { ids => {} }; },
+ get_guest_config_properties => sub { return $test_state->{guest_nets} // {}; },
);
# ------- TEST FUNCTIONS --------------
@@ -289,6 +300,11 @@ sub create_zone {
return $zone;
}
+sub update_zone {
+ my ($zoneid, $params) = @_;
+ PVE::API2::Network::SDN::Zones->update({ zone => $zoneid, %$params });
+}
+
sub get_vnet {
my ($id) = @_;
return eval { PVE::API2::Network::SDN::Vnets->read({ vnet => $id }); };
@@ -359,6 +375,16 @@ sub create_ip {
return PVE::API2::Network::SDN::Ips->ipcreate($param);
}
+sub update_ip {
+ my ($param) = @_;
+ return PVE::API2::Network::SDN::Ips->ipupdate($param);
+}
+
+sub delete_ip {
+ my ($param) = @_;
+ return PVE::API2::Network::SDN::Ips->ipdelete($param);
+}
+
sub run_test {
my $test = shift;
clear_test_state();
@@ -1110,18 +1136,144 @@ sub test_dnsmasq_stale_reservation_replaced {
};
PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$stale,10.0.1.10\n$gone,10.0.1.12\n");
- PVE::Network::SDN::Dhcp::Dnsmasq->add_ip_mapping(
- 'TESTZONE', $test_state->{macdb}, $added, '10.0.1.13', undef,
- );
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping('TESTZONE', $added);
- my @lines = sort grep { /\S/ } split(/\n/, PVE::Tools::file_get_contents($TMP_ETHERS_FILE));
- eq_or_diff(
- \@lines,
- ["$stale,10.0.1.11", "$added,10.0.1.13"],
+ is(
+ PVE::Tools::file_get_contents($TMP_ETHERS_FILE),
+ "$stale,10.0.1.11\n$added,10.0.1.13\n",
"$test_name: the stale line is replaced, the unknown one dropped",
);
}
run_test(\&test_dnsmasq_stale_reservation_replaced);
+# -------------- dnsmasq mapping pushes
+
+sub test_dnsmasq_mapping_push {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+ # the first push of a zone finds no ethers file yet
+ unlink($TMP_ETHERS_FILE);
+
+ my $ethers = sub { PVE::Tools::file_get_contents($TMP_ETHERS_FILE) };
+
+ eval { nic_start($vnetid, $mac, "999", "testhostname"); };
+ if ($@) {
+ fail("$test_name: nic_start: $@");
+ return;
+ }
+ is($ethers->(), "$mac,10.0.0.100\n", "$test_name: guest start reserves the allocation");
+
+ # the records exist already, so the restart pushes them without a change
+ $test_state->{dnsmasq_calls} = [];
+ nic_start($vnetid, $mac, "999", "testhostname");
+ is($ethers->(), "$mac,10.0.0.100\n", "$test_name: restart keeps the reservation");
+ is_deeply(
+ $test_state->{dnsmasq_calls},
+ [],
+ "$test_name: restart without a change does not reload",
+ );
+
+ update_ip({
+ zone => $zoneid,
+ vnet => $vnetid,
+ mac => $mac,
+ ip => "10.0.0.150",
+ });
+ is($ethers->(), "$mac,10.0.0.150\n", "$test_name: mapping edit replaces the reservation");
+
+ delete_ip({
+ zone => $zoneid,
+ vnet => $vnetid,
+ mac => $mac,
+ ip => "10.0.0.150",
+ });
+ is($ethers->(), "", "$test_name: mapping delete drops the reservation");
+
+ # a record without a MAC has no NIC to serve, so nothing is pushed for it
+ my @warnings;
+ {
+ local $SIG{__WARN__} = sub { push @warnings, @_; };
+ create_ip({ zone => $zoneid, vnet => $vnetid, ip => "10.0.0.160" });
+ delete_ip({ zone => $zoneid, vnet => $vnetid, ip => "10.0.0.160" });
+ }
+ eq_or_diff(\@warnings, [], "$test_name: a record without a MAC warns nothing");
+
+ # a reservation of a MAC the cache does not know is swept by the next
+ # rewrite, and both addresses of the guest go on one line
+ create_dhcp_subnet($vnetid, "8888::/64");
+ PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "aa:bb:cc:dd:ee:ff,10.0.0.77\n");
+ nic_start($vnetid, $mac, "999", "testhostname");
+ is(
+ $ethers->(),
+ "$mac,10.0.0.100,[8888::64]\n",
+ "$test_name: dual-stack reservation on one line, the unknown MAC swept",
+ );
+
+ # a zone confined to other nodes runs no dnsmasq here, so an edit is not
+ # pushed on this node
+ update_zone($zoneid, { nodes => 'other' });
+ create_ip({
+ zone => $zoneid,
+ vnet => $vnetid,
+ mac => $mac,
+ ip => "10.0.0.150",
+ });
+ is(
+ $ethers->(),
+ "$mac,10.0.0.100,[8888::64]\n",
+ "$test_name: a zone confined to other nodes gets no push here",
+ );
+}
+
+run_test(\&test_dnsmasq_mapping_push);
+
+sub test_mapping_edit_target {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $vnetid = "testvnet";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24");
+
+ # the guest behind a MAC is found by its NIC on the vnet, and the config
+ # writes the MAC in upper case
+ $test_state->{vmlist} = {
+ ids => {
+ 100 => { type => 'qemu', node => 'other' },
+ },
+ };
+ $test_state->{guest_nets} = {
+ 1 => { net0 => "virtio=$mac,bridge=$vnetid" },
+ 100 => { net0 => "virtio=" . uc($mac) . ",bridge=$vnetid" },
+ };
+ is(
+ PVE::Network::SDN::Dhcp::guest_node_by_mac($vnetid, $mac),
+ 'other',
+ "$test_name: the MAC resolves to the node of its guest, a guest the list lacks is skipped",
+ );
+
+ # a mapping edit through the API runs on the guest's node
+ my $proxyto = PVE::API2::Network::SDN::Ips->map_method_by_name('ipupdate')->{proxyto_callback};
+ is(
+ $proxyto->(undef, undef, { zone => $zoneid, vnet => $vnetid, mac => $mac }),
+ 'other',
+ "$test_name: a mapping edit runs on the guest's node",
+ );
+ is(
+ $proxyto->(undef, undef, { zone => $zoneid, vnet => $vnetid, mac => '00:11:22:33:44:66' }),
+ 'localhost',
+ "$test_name: one for a MAC without a guest runs here",
+ );
+ delete $test_state->{vmlist};
+ delete $test_state->{guest_nets};
+}
+
+run_test(\&test_mapping_edit_target);
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH pve-network 07/10] sdn: ipam: look a MAC up within the vnet's subnets
2026-11-02 11:05 [PATCH docs/network 00/10] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (5 preceding siblings ...)
2026-11-02 11:06 ` [PATCH pve-network 06/10] sdn: dhcp: update reservations when ip mappings change Hannes Laimer
@ 2026-11-02 11:06 ` Hannes Laimer
2026-11-02 11:06 ` [PATCH pve-network 08/10] sdn: ipam: keep the MAC cache per vnet Hannes Laimer
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: Hannes Laimer @ 2026-11-02 11:06 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..01e7f347d796 100644
--- a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
@@ -379,26 +379,32 @@ sub del_ip {
}
sub get_ips_from_mac {
- my ($class, $plugin_config, $mac, $zoneid, $noerr) = @_;
+ my ($class, $plugin_config, $mac, $zoneid, $subnets, $noerr) = @_;
my $ip4 = undef;
my $ip6 = undef;
- my $data = eval {
- netbox_api_request($plugin_config, "GET", "/ipam/ip-addresses/?description__ic=$mac");
- };
- if ($@) {
- return if $noerr;
- die "could not query ip address entry for mac $mac: $@";
- }
-
- for my $ip (@{ $data->{results} }) {
- if ($ip->{family}->{value} == 4 && !$ip4) {
- ($ip4, undef) = split(/\//, $ip->{address});
+ for my $cidr (sort map { $_->{cidr} } values %{ $subnets // {} }) {
+ my $data = eval {
+ netbox_api_request(
+ $plugin_config,
+ "GET",
+ "/ipam/ip-addresses/?parent=$cidr&description__ic=$mac",
+ );
+ };
+ if ($@) {
+ return if $noerr;
+ die "could not query ip address entry for mac $mac in $cidr: $@";
}
- if ($ip->{family}->{value} == 6 && !$ip6) {
- ($ip6, undef) = split(/\//, $ip->{address});
+ for my $ip (@{ $data->{results} }) {
+ if ($ip->{family}->{value} == 4 && !$ip4) {
+ ($ip4, undef) = split(/\//, $ip->{address});
+ }
+
+ if ($ip->{family}->{value} == 6 && !$ip6) {
+ ($ip6, undef) = split(/\//, $ip->{address});
+ }
}
}
diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index 77a7b2a7c144..43d0a3daaf9a 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -279,7 +279,7 @@ sub del_ip {
}
sub get_ips_from_mac {
- my ($class, $plugin_config, $mac, $zoneid) = @_;
+ my ($class, $plugin_config, $mac, $zoneid, $subnets) = @_;
#just in case, as this should already be cached in local macs.db
@@ -289,13 +289,12 @@ sub get_ips_from_mac {
my $db = read_db();
die "zone $zoneid don't exist in ipam db" if !$db->{zones}->{$zoneid};
my $dbzone = $db->{zones}->{$zoneid};
- my $subnets = $dbzone->{subnets};
- for my $subnet (keys %$subnets) {
+ for my $subnet (sort map { $_->{cidr} } values %{ $subnets // {} }) {
# subnet keys are CIDRs, Net::IP only recognizes bare IPs
my ($network) = split(m|/|, $subnet);
next if Net::IP::ip_is_ipv4($network) ? $ip4 : $ip6;
- my $ips = $subnets->{$subnet}->{ips};
+ my $ips = $dbzone->{subnets}->{$subnet}->{ips} // next;
for my $ip (keys %$ips) {
my $ipobject = $ips->{$ip};
if ($ipobject->{mac} && $ipobject->{mac} eq $mac) {
diff --git a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
index 395d72c9494c..c3f014d45585 100644
--- a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
@@ -260,7 +260,7 @@ sub del_ip {
}
sub get_ips_from_mac {
- my ($class, $plugin_config, $mac, $zoneid) = @_;
+ my ($class, $plugin_config, $mac, $zoneid, $subnets) = @_;
my $url = $plugin_config->{url};
my $token = $plugin_config->{token};
diff --git a/src/PVE/Network/SDN/Ipams/Plugin.pm b/src/PVE/Network/SDN/Ipams/Plugin.pm
index 26d093139703..14f8fe9e67fe 100644
--- a/src/PVE/Network/SDN/Ipams/Plugin.pm
+++ b/src/PVE/Network/SDN/Ipams/Plugin.pm
@@ -138,8 +138,9 @@ sub del_ip {
die "please implement inside plugin";
}
+# the addresses of a MAC within the given subnets of the zone, one per family
sub get_ips_from_mac {
- my ($class, $plugin_config, $mac, $zone) = @_;
+ my ($class, $plugin_config, $mac, $zoneid, $subnets) = @_;
die "please implement inside plugin";
}
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index 8453a2781ae9..b1a2192ac767 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -201,7 +201,8 @@ sub get_ips_from_mac {
return if !$zone->{ipam} || !$zone->{dhcp};
- return PVE::Network::SDN::Ipams::get_ips_from_mac($mac, $zoneid, $zone);
+ my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid);
+ return PVE::Network::SDN::Ipams::get_ips_from_mac($mac, $zoneid, $zone, $subnets);
}
sub del_ips_from_mac {
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index bde61f2d3401..27b1295ed556 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -1276,4 +1276,29 @@ sub test_mapping_edit_target {
run_test(\&test_mapping_edit_target);
+sub test_ipam_per_vnet {
+ my $test_name = (split(/::/, (caller(0))[3]))[-1];
+ my $zoneid = "TESTZONE";
+ my $mac = "da:65:8f:18:9b:6f";
+
+ # the same MAC on two vnets of one zone has an address on each, and a
+ # lookup for a vnet answers with that vnet's address only
+ create_zone({ type => "simple", dhcp => "dnsmasq", ipam => "pve", zone => $zoneid });
+ for my $n (1, 2) {
+ create_dhcp_vnet($zoneid, "vnet$n", "10.$n.0.0/24");
+ create_ip({ zone => $zoneid, vnet => "vnet$n", mac => $mac, ip => "10.$n.0.50" });
+ }
+
+ for my $n (1, 2) {
+ $test_state->{macdb} = {};
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac("vnet$n", $mac)],
+ ["10.$n.0.50", undef],
+ "$test_name: vnet$n answers with its own address",
+ );
+ }
+}
+
+run_test(\&test_ipam_per_vnet);
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH pve-network 08/10] sdn: ipam: keep the MAC cache per vnet
2026-11-02 11:05 [PATCH docs/network 00/10] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (6 preceding siblings ...)
2026-11-02 11:06 ` [PATCH pve-network 07/10] sdn: ipam: look a MAC up within the vnet's subnets Hannes Laimer
@ 2026-11-02 11:06 ` Hannes Laimer
2026-11-02 11:06 ` [PATCH pve-network 09/10] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM Hannes Laimer
2026-11-02 11:06 ` [PATCH pve-docs 10/10] sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM Hannes Laimer
9 siblings, 0 replies; 11+ messages in thread
From: Hannes Laimer @ 2026-11-02 11:06 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.
Entries from before, keyed by the MAC alone, stay as written, since
nodes not yet upgraded still use them. They answer for a vnet only
with addresses in its subnets, so a running guest keeps its
reservation across the upgrade. A node not yet upgraded does not
update the per-vnet entries, so a release checks that the record
still belongs to its MAC and leaves it alone otherwise. The API
refuses to delete such a mapping.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/API2/Network/SDN/Ips.pm | 5 +-
src/PVE/Network/SDN/Dhcp.pm | 2 +-
src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 125 ++++++------
src/PVE/Network/SDN/Dhcp/Plugin.pm | 6 +-
src/PVE/Network/SDN/Ipams.pm | 124 ++++++------
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 11 +-
src/PVE/Network/SDN/Ipams/PVEPlugin.pm | 13 +-
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 16 +-
src/PVE/Network/SDN/Ipams/Plugin.pm | 4 +-
src/PVE/Network/SDN/Subnets.pm | 11 +-
src/PVE/Network/SDN/Vnets.pm | 12 +-
src/test/run_test_ipams.pl | 37 ++++
src/test/run_test_vnets_blackbox.pl | 217 +++++++++++++++++----
13 files changed, 404 insertions(+), 179 deletions(-)
diff --git a/src/PVE/API2/Network/SDN/Ips.pm b/src/PVE/API2/Network/SDN/Ips.pm
index b1faee71c6ba..ecab7fc99b6c 100644
--- a/src/PVE/API2/Network/SDN/Ips.pm
+++ b/src/PVE/API2/Network/SDN/Ips.pm
@@ -62,11 +62,14 @@ __PACKAGE__->register_method({
my $mac = extract_param($param, 'mac');
my $ip = extract_param($param, 'ip');
- eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
+ my $released = eval { PVE::Network::SDN::Vnets::del_ip($vnet, $ip, '', $mac); };
die "$@\n" if $@;
PVE::Network::SDN::Dhcp::update_mapping($vnet, $mac);
+ # the cache entry is cleared, the IPAM record belongs to another MAC
+ die "IP '$ip' is not mapped to $mac\n" if defined($released) && !$released;
+
return undef;
},
});
diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm
index 4650270df09f..28e83d11630b 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, so a failed update only warns
warn "could not update dhcp mapping for $mac: $@" if $@;
diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm
index 4767fb5162ad..1acc7c42318b 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,90 @@ 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
+ # 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;
+ };
+
+ # read under the ethers lock, a read from before it would undo a push
+ # that ran meanwhile. The lines of other MACs are checked against the
+ # cache alone, so under the lock only the changed MAC may ask the IPAM
+ my $reservation = sub {
+ my ($vnet, $line_mac, $cached_only) = @_;
+ my ($line_ip4, $line_ip6) =
+ PVE::Network::SDN::Vnets::get_ips_from_mac($vnet, $line_mac, 1, $cached_only);
+ return (undef, undef) if !$line_ip4 && !$line_ip6;
+ my $line = $line_mac;
+ $line .= ",$line_ip4" if $line_ip4;
+ $line .= ",[$line_ip6]" if $line_ip6;
# an external IPAM spells addresses its own way, while the file holds one spelling
- $reservation = lc($reservation);
- }
+ return (lc($line), $line_ip4);
+ };
file_set_contents($ethers_file, '') if !-e $ethers_file;
open(my $in, '<', $ethers_file) or die "Could not open file '$ethers_file' $!\n";
open(my $out, '>', $ethers_tmp_file)
or die "Could not open file '$ethers_tmp_file' $!\n";
- my $kept = undef;
-
+ my %written;
while (my $line = <$in>) {
chomp($line);
next if $line !~ m/\S/;
- my ($parsed_mac, $parsed_ip1, $parsed_ip2) = split(/,/, $line);
- # a line from before the file was written in lower case keeps its writer's spelling
+ my ($parsed_mac, $parsed_ip) = split(/,/, $line);
$parsed_mac = lc($parsed_mac);
-
- # the MAC's own reservation is rewritten. An unchanged one stays in place
- if ($parsed_mac eq $mac) {
- if (defined($reservation) && !$kept && lc($line) eq $reservation) {
- $kept = 1;
- print $out "$line\n";
- } else {
- $changed = 1;
- }
- next;
- }
-
- my ($parsed_ip4, $parsed_ip6);
- if ($parsed_ip2) {
- $parsed_ip4 = $parsed_ip1;
- $parsed_ip6 = $parsed_ip2;
- } elsif (defined($parsed_ip1) && Net::IP::ip_is_ipv4($parsed_ip1)) {
- $parsed_ip4 = $parsed_ip1;
- } elsif (defined($parsed_ip1)) {
- $parsed_ip6 = $parsed_ip1;
- }
- $parsed_ip6 = $1 if $parsed_ip6 && $parsed_ip6 =~ m/\[(\S+)\]/;
- # a line from before addresses were lower-cased may spell one in upper case
- $parsed_ip6 = lc($parsed_ip6) if $parsed_ip6;
-
- # a line that no longer matches the cache is replaced by the cache's
- # current one. Dropping it left that guest unknown to dnsmasq until
- # its next start
- my $current = $macs->{$parsed_mac};
- if (
- !$current
- || ($parsed_ip4 && $current->{ip4} && $current->{ip4} ne $parsed_ip4)
- || ($parsed_ip6 && $current->{ip6} && lc($current->{ip6}) ne $parsed_ip6)
- ) {
+ $parsed_ip = $1 if defined($parsed_ip) && $parsed_ip =~ m/\[(\S+)\]/;
+ my $vnet = defined($parsed_ip) ? $vnet_of->($parsed_ip) : undef;
+ my $key = defined($vnet) ? "$parsed_mac,$vnet" : undef;
+
+ # a line that no longer matches is replaced, since dropping it left
+ # that guest unknown to dnsmasq until its next start
+ my ($want) =
+ eval { $key && !$written{$key} ? $reservation->($vnet, $parsed_mac, 1) : () };
+ if ($@) {
+ warn "could not read the reservation of $parsed_mac: $@";
+ print $out "$line\n";
+ } elsif (!defined($want)) {
$changed = 1;
- next if !$current;
- my $replacement = $parsed_mac;
- $replacement .= ",$current->{ip4}" if $current->{ip4};
- $replacement .= ",[$current->{ip6}]" if $current->{ip6};
- print $out lc($replacement) . "\n";
next;
+ } elsif (lc($line) ne $want) {
+ $changed = 1;
+ print $out "$want\n";
+ } else {
+ print $out "$line\n";
}
-
- print $out "$line\n";
+ $written{$key}++;
}
- if (defined($reservation) && !$kept) {
- print $out "$reservation\n";
+ # the changed MAC's own line may have been dropped above, since only
+ # the cache was asked for it, so it is written from the IPAM's answer
+ my $want;
+ ($want, $ip4) = $reservation->($vnetid, $mac);
+ if (defined($want) && !$written{"$mac,$vnetid"}) {
+ print $out "$want\n";
$changed = 1;
}
diff --git a/src/PVE/Network/SDN/Dhcp/Plugin.pm b/src/PVE/Network/SDN/Dhcp/Plugin.pm
index a3a5ae24403f..3484df19b6dc 100644
--- a/src/PVE/Network/SDN/Dhcp/Plugin.pm
+++ b/src/PVE/Network/SDN/Dhcp/Plugin.pm
@@ -22,10 +22,10 @@ sub private {
return $defaultData;
}
-# the MAC's records changed, so make the backend serve the current ones. With no
-# record left its reservation goes
+# the MAC's records on the vnet changed, so make the backend serve the current
+# ones. With no record left its reservation on the vnet goes
sub update_ip_mapping {
- my ($class, $dhcpid, $mac) = @_;
+ my ($class, $dhcpid, $vnetid, $mac) = @_;
die 'implement in sub class';
}
diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm
index 9a31eaf39f58..5b2217c45242 100644
--- a/src/PVE/Network/SDN/Ipams.pm
+++ b/src/PVE/Network/SDN/Ipams.pm
@@ -35,40 +35,10 @@ sub json_writer {
return encode_json($data);
}
-# Writers spell a MAC their own way, the generated ones in upper case and the
-# API in either, and nodes not yet upgraded look an entry up under the spelling
-# their caller hands them, so the file keeps its keys as written. Readers get a
-# view with every MAC in lower case. Two spellings that disagree on an address
-# are left out of it, since one of them is stale and nothing here says which,
-# and so is an entry without an address. The next lookup then asks the IPAM
-my sub fold_macdb {
- my ($db) = @_;
-
- my $macs = {};
- my %clash;
- for my $key (sort keys %{ $db->{macs} // {} }) {
- my $entry = $db->{macs}->{$key};
- for my $family (grep { defined($entry->{$_}) } qw(ip4 ip6)) {
- my $seen = $macs->{ lc($key) }->{$family};
- $clash{ lc($key) } = 1 if defined($seen) && lc($seen) ne lc($entry->{$family});
- $macs->{ lc($key) }->{$family} //= $entry->{$family};
- }
- }
- delete $macs->{$_} for keys %clash;
- return { %$db, macs => $macs };
-}
-
-# the file as written, which the writers change and write back
-sub read_macdb_file {
- my () = @_;
-
- return cfs_read_file($macdb_filename);
-}
-
sub read_macdb {
my () = @_;
- return fold_macdb(read_macdb_file());
+ return cfs_read_file($macdb_filename);
}
sub write_macdb {
@@ -77,25 +47,56 @@ 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 {
- my ($db, $mac, $family, $ip) = @_;
+# the map under the MAC alone keeps its keys as written, since nodes not yet
+# upgraded still use it under the MAC as their caller spelled it. Spellings
+# that disagree on an address are both left out, since one of them is stale
+my sub old_cache_entry {
+ my ($db, $mac, $subnets) = @_;
- my $macs = $db->{macs} //= {};
- my @keys = grep { lc($_) eq lc($mac) } keys %$macs;
- $macs->{$_}->{$family} = $ip for (@keys ? @keys : (lc($mac)));
+ my $entry = {};
+ my $macs = $db->{macs} // {};
+ for my $key (grep { lc($_) eq lc($mac) } sort keys %$macs) {
+ for my $family (qw(ip4 ip6)) {
+ my $ip = $macs->{$key}->{$family} // next;
+ return undef if defined($entry->{$family}) && lc($entry->{$family}) ne lc($ip);
+ $entry->{$family} = $ip;
+ }
+ }
+ for my $family (keys %$entry) {
+ delete $entry->{$family}
+ if !eval { PVE::Network::SDN::Subnets::find_ip_subnet($entry->{$family}, $subnets) };
+ }
+ return %$entry ? $entry : undef;
+}
+
+# an entry from before the cache was kept per vnet fills the families a
+# vnet's entry does not have. Where the two disagree, a node still writing
+# one map may have moved the address since, so the lookup is a miss
+my sub cache_entry {
+ my ($db, $mac, $vnetid, $subnets) = @_;
+
+ my $entry = (($db->{vnets} // {})->{$vnetid} // {})->{ lc($mac) };
+ $entry = undef if $entry && !defined($entry->{ip4}) && !defined($entry->{ip6});
+ my $old = old_cache_entry($db, $mac, $subnets);
+ return undef if !$entry && !$old;
+ for my $family (qw(ip4 ip6)) {
+ my ($ip, $old_ip) = ($entry->{$family}, $old->{$family});
+ return undef if defined($ip) && defined($old_ip) && lc($ip) ne lc($old_ip);
+ }
+ return { map { $_ => $entry->{$_} // $old->{$_} } qw(ip4 ip6) };
}
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;
write_macdb($db);
},
);
@@ -103,19 +104,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 old map 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);
},
@@ -169,15 +180,13 @@ sub complete_sdn_vnet {
}
sub get_ips_from_mac {
- my ($mac, $zoneid, $zone, $subnets) = @_;
+ my ($vnetid, $mac, $zoneid, $zone, $subnets, $cached_only) = @_;
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 $cached = cache_entry(read_macdb(), $mac, $vnetid, $subnets);
+ return ($cached->{ip4}, $cached->{ip6}) if $cached;
+ return if $cached_only;
my $plugin_config = get_plugin_config($zone);
my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type});
@@ -195,9 +204,10 @@ 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) } //= {};
+ $entry->{ip4} = $ip4 if defined($ip4);
+ $entry->{ip6} = $ip6 if defined($ip6);
write_macdb($db);
},
);
diff --git a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
index 01e7f347d796..82c9e80b6c6e 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,15 @@ 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/") };
+ my ($owner) = (($record // {})->{description} // '') =~ m/^mac:(\S+)$/;
+ if ($owner && lc($owner) ne lc($mac)) {
+ warn "not releasing IP '$ip' for $mac, it is held by $owner\n";
+ return 0;
+ }
+ }
eval { netbox_api_request($plugin_config, "DELETE", "/ipam/ip-addresses/$ip_id/"); };
if ($@) {
diff --git a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
index 43d0a3daaf9a..86e594a4c604 100644
--- a/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PVEPlugin.pm
@@ -249,11 +249,12 @@ sub add_range_next_freeip {
}
sub del_ip {
- my ($class, $plugin_config, $subnetid, $subnet, $ip) = @_;
+ my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr, $mac) = @_;
my $cidr = $subnet->{cidr};
my $zone = $subnet->{zone};
+ my $released = 1;
cfs_lock_file(
$ipamdb_file,
undef,
@@ -271,11 +272,18 @@ sub del_ip {
warn "IP '$ip' does not exist in IPAM DB\n";
return;
}
+ my $owner = $dbsubnet->{ips}->{$ip}->{mac};
+ if ($mac && $owner && lc($owner) ne lc($mac)) {
+ warn "not releasing IP '$ip' for $mac, it is held by $owner\n";
+ $released = 0;
+ return;
+ }
delete $dbsubnet->{ips}->{$ip};
write_db($db);
},
);
die "$@" if $@;
+ return $released;
}
sub get_ips_from_mac {
@@ -297,7 +305,8 @@ sub get_ips_from_mac {
my $ips = $dbzone->{subnets}->{$subnet}->{ips} // next;
for my $ip (keys %$ips) {
my $ipobject = $ips->{$ip};
- if ($ipobject->{mac} && $ipobject->{mac} eq $mac) {
+ # a record keeps the MAC as its writer spelled it
+ if ($ipobject->{mac} && lc($ipobject->{mac}) eq lc($mac)) {
if (Net::IP::ip_is_ipv4($ip)) {
$ip4 = $ip;
} else {
diff --git a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
index c3f014d45585..e1b30e293ab3 100644
--- a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
@@ -237,7 +237,7 @@ sub add_range_next_freeip {
}
sub del_ip {
- my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr) = @_;
+ my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr, $mac) = @_;
return if !$ip;
@@ -248,6 +248,18 @@ 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,
+ );
+ };
+ my $owner = (($record // {})->{data} // {})->{mac};
+ if ($owner && lc($owner) ne lc($mac)) {
+ warn "not releasing IP '$ip' for $mac, it is held by $owner\n";
+ return 0;
+ }
+ }
eval {
PVE::Network::SDN::api_request(
@@ -257,6 +269,8 @@ sub del_ip {
if ($@) {
die "error delete ip $ip: $@" if !$noerr;
}
+
+ return 1;
}
sub get_ips_from_mac {
diff --git a/src/PVE/Network/SDN/Ipams/Plugin.pm b/src/PVE/Network/SDN/Ipams/Plugin.pm
index 14f8fe9e67fe..6e23d5434d4b 100644
--- a/src/PVE/Network/SDN/Ipams/Plugin.pm
+++ b/src/PVE/Network/SDN/Ipams/Plugin.pm
@@ -132,8 +132,10 @@ sub add_range_next_freeip {
die "please implement inside plugin";
}
+# a MAC names the guest the address is released for, a record holding the
+# address for another MAC is then left alone and 0 returned
sub del_ip {
- my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr) = @_;
+ my ($class, $plugin_config, $subnetid, $subnet, $ip, $noerr, $mac) = @_;
die "please implement inside plugin";
}
diff --git a/src/PVE/Network/SDN/Subnets.pm b/src/PVE/Network/SDN/Subnets.pm
index 1f27fac45db1..4d431f1bd27a 100644
--- a/src/PVE/Network/SDN/Subnets.pm
+++ b/src/PVE/Network/SDN/Subnets.pm
@@ -263,7 +263,7 @@ sub add_next_free_ip {
die $@ if $@;
- eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, $ip); };
+ eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($subnet->{vnet}, $mac, $ip); };
warn $@ if $@;
}
@@ -322,7 +322,7 @@ sub add_ip {
};
die $@ if $@;
- eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, $ip) if $mac; };
+ eval { PVE::Network::SDN::Ipams::add_cache_mac_ip($subnet->{vnet}, $mac, $ip) if $mac; };
warn $@ if $@;
}
@@ -410,14 +410,15 @@ 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 $@;
}
}
@@ -431,6 +432,8 @@ sub del_ip {
if ($@) {
warn $@;
}
+
+ return $released;
}
1;
diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index b1a2192ac767..d8824218d9ef 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -191,18 +191,20 @@ sub del_ip {
}
sub get_ips_from_mac {
- my ($vnetid, $mac) = @_;
+ my ($vnetid, $mac, $running, $cached_only) = @_;
- 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, $cached_only,
+ );
}
sub del_ips_from_mac {
diff --git a/src/test/run_test_ipams.pl b/src/test/run_test_ipams.pl
index f9c44fe1a3b9..0d3fed120d3f 100755
--- a/src/test/run_test_ipams.pl
+++ b/src/test/run_test_ipams.pl
@@ -219,5 +219,42 @@ 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 @deletes;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url) = @_;
+ 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");
+ }
+}
+
done_testing();
diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl
index 27b1295ed556..8f703cec9968 100755
--- a/src/test/run_test_vnets_blackbox.pl
+++ b/src/test/run_test_vnets_blackbox.pl
@@ -194,7 +194,7 @@ $mocked_sdn_ipams->mock(
my ($cfg) = @_;
$test_state->{ipam_config} = $cfg;
},
- read_macdb_file => sub {
+ read_macdb => sub {
return PVE::Network::SDN::Ipams::json_reader(undef, encode_json($test_state->{macdb}));
},
write_macdb => sub {
@@ -491,74 +491,97 @@ 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 => undef, ip6 => undef } },
+ vnets => { $vnetid => { $mac => { ip4 => "10.0.0.50" } } },
+ },
+ "$test_name: the answer is cached, the old entry left as written",
);
# a write to the cache between the miss and the fill, which the file's
# version shows, keeps the IPAM's answer out of the cache
- delete $test_state->{macdb}->{macs}->{$mac};
+ delete $test_state->{macdb}->{vnets};
my $version = 0;
$mocked_pve_cluster_obj->mock(cfs_file_version => sub { return $version++ });
@ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
$mocked_pve_cluster_obj->unmock('cfs_file_version');
is($ips[0], "10.0.0.50", "$test_name: the lookup still answers");
- ok(!exists $test_state->{macdb}->{macs}->{$mac},
- "$test_name: but leaves a changed cache alone");
+ ok(!exists $test_state->{macdb}->{vnets}, "$test_name: but leaves a changed cache alone");
- # an entry written under the config's spelling, as a node not yet upgraded
- # writes it, is read and released like any other and keeps its key
+ # an entry written under the config's spelling before the cache was keyed in
+ # lower case is read and released like any other
my $other = "da:65:8f:18:9b:70";
$test_state->{macdb}->{macs} = {
uc($mac) => { ip4 => '10.0.0.150' },
uc($other) => { ip4 => '10.0.0.170' },
};
my ($ip4) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
- is($ip4, '10.0.0.150', "$test_name: an entry is found however it is spelled");
- PVE::Network::SDN::Ipams::del_cache_mac_ip($mac, '10.0.0.150');
+ is($ip4, '10.0.0.150', "$test_name: an old entry is found however it is spelled");
+ PVE::Network::SDN::Ipams::del_cache_mac_ip($vnetid, $mac, '10.0.0.150');
is_deeply(
$test_state->{macdb}->{macs},
{ uc($other) => { ip4 => '10.0.0.170' } },
"$test_name: and released, the other entry left as written",
);
- # a write in another spelling goes to the spelling the file holds
+ # a write by this code leaves an old entry as a node not yet upgraded
+ # wrote it, so that node still finds it under its own spelling
$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",
+ { uc($mac) => { ip4 => '10.0.0.150' } },
+ "$test_name: a write leaves the old entry as written",
+ );
+ 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
+ $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}->{vnets}->{$vnetid}->{$mac},
+ { ip4 => '10.0.0.50', ip6 => 'fd00::150' },
+ "$test_name: and the answer replaces the vnet's entry",
);
# two spellings that disagree are not trusted, the lookup asks the IPAM
- $test_state->{macdb}->{macs} = {
- uc($mac) => { ip4 => '10.0.0.150' },
- $mac => { ip4 => '10.0.0.151' },
+ $test_state->{macdb} = {
+ macs => {
+ uc($mac) => { ip4 => '10.0.0.150' },
+ $mac => { ip4 => '10.0.0.151' },
+ },
};
@ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
is($ips[0], '10.0.0.50', "$test_name: a clash of spellings counts as a miss");
- is_deeply(
- $test_state->{macdb}->{macs},
- { uc($mac) => { ip4 => '10.0.0.50' }, $mac => { ip4 => '10.0.0.50' } },
- "$test_name: and its write brings both spellings in line",
- );
+
+ # an old entry's address outside the vnet's subnets is another vnet's
+ $test_state->{macdb} = { macs => { uc($mac) => { ip4 => '10.9.0.150' } } };
+ @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac);
+ is($ips[0], '10.0.0.50', "$test_name: an old address of another vnet counts as a miss");
+
+ # the record keeps the MAC as its writer spelled it, a lookup in another
+ # spelling finds it
+ $test_state->{macdb} = {};
+ @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, uc($mac));
+ is($ips[0], '10.0.0.50', "$test_name: a lookup in another spelling finds the record");
# a mapping whose record is gone is still deleted, and its cache entry
# with it
@@ -1128,15 +1151,18 @@ 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' },
+ vnets => {
+ testvnet => {
+ $stale => { ip4 => '10.0.1.11' },
+ $added => { ip4 => '10.0.1.13' },
+ },
},
};
PVE::Tools::file_set_contents($TMP_ETHERS_FILE, "$stale,10.0.1.10\n$gone,10.0.1.12\n");
- PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping('TESTZONE', $added);
+ PVE::Network::SDN::Dhcp::Dnsmasq->update_ip_mapping('TESTZONE', 'testvnet', $added);
is(
PVE::Tools::file_get_contents($TMP_ETHERS_FILE),
@@ -1297,8 +1323,127 @@ 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 any vnet,
+ # and a mapping edit elsewhere does not touch it
+ $test_state->{macdb} = { macs => { $mac => { ip4 => '10.2.0.60' } } };
+ create_ip({ zone => $zoneid, vnet => 'vnet1', mac => $mac, ip => '10.1.0.52' });
+ is_deeply(
+ [PVE::Network::SDN::Vnets::get_ips_from_mac('vnet2', $mac)],
+ ['10.2.0.60', undef],
+ "$test_name: an old entry still answers for the other vnet",
+ );
}
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
+ 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 the
+ # cache does not hold is dropped without the IPAM being asked
+ delete $test_state->{macdb}->{vnets}->{vnet2};
+ 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] 11+ messages in thread
* [PATCH pve-network 09/10] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM
2026-11-02 11:05 [PATCH docs/network 00/10] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (7 preceding siblings ...)
2026-11-02 11:06 ` [PATCH pve-network 08/10] sdn: ipam: keep the MAC cache per vnet Hannes Laimer
@ 2026-11-02 11:06 ` Hannes Laimer
2026-11-02 11:06 ` [PATCH pve-docs 10/10] sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM Hannes Laimer
9 siblings, 0 replies; 11+ messages in thread
From: Hannes Laimer @ 2026-11-02 11:06 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 even when another zone holds the same range elsewhere.
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. A failed section lookup
stops the operation, only a missing section means the zone has none.
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
src/PVE/Network/SDN/Ipams/NetboxPlugin.pm | 157 +++++++++----
src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm | 163 ++++++++++---
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 | 221 ++++++++++++++++++
7 files changed, 478 insertions(+), 71 deletions(-)
diff --git a/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm b/src/PVE/Network/SDN/Ipams/NetboxPlugin.pm
index 82c9e80b6c6e..ee48a19d5358 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;
@@ -78,12 +75,25 @@ sub add_subnet {
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 only the zone's VRF is
+ # searched, not the global table the prefixes from before sit in
+ 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";
}
- eval { netbox_api_request($plugin_config, "POST", "/ipam/prefixes/", { prefix => $cidr }); };
+ $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, vrf => $vrf_id },
+ );
+ };
if ($@) {
return if $noerr;
die "error adding subnet to ipam: $@";
@@ -91,7 +101,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 +132,8 @@ sub update_subnet {
my $to_delete_ids = ();
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr);
+
# delete first so we don't get errors with overlapping ranges
for my $dhcp_range (@$old_dhcp_ranges) {
my $hash = $hash_range->($dhcp_range);
@@ -130,7 +142,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 +157,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 +167,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 +175,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 +188,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 +227,8 @@ sub add_ip {
$description = "mac:$mac";
}
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr);
+
eval {
my $params = {
address => "$ip/$mask",
@@ -222,6 +236,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 +244,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 +274,8 @@ sub update_ip {
$description = "mac:$mac";
}
- my $ip_id = get_ip_id($plugin_config, $ip, $noerr);
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr);
+ my $ip_id = get_ip_id($plugin_config, $ip, $vrf_id, $noerr);
# definedness check, because ID could be 0
if (!defined($ip_id)) {
@@ -274,6 +290,7 @@ sub update_ip {
};
$params->{dns_name} = $hostname if $hostname;
+ $params->{vrf} = $vrf_id if defined($vrf_id);
netbox_api_request($plugin_config, "PATCH", "/ipam/ip-addresses/$ip_id/", $params);
};
@@ -287,7 +304,7 @@ sub add_next_freeip {
my $cidr = $subnet->{cidr};
- my $internalid = get_prefix_id($plugin_config, $cidr, $noerr);
+ my ($internalid) = subnet_prefix($plugin_config, $subnet, $noerr);
# definedness check, because ID could be 0
if (!defined($internalid)) {
@@ -323,7 +340,8 @@ sub add_next_freeip {
sub add_range_next_freeip {
my ($class, $plugin_config, $subnet, $range, $data, $noerr) = @_;
- my $internalid = get_iprange_id($plugin_config, $range, $noerr);
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr);
+ my $internalid = get_iprange_id($plugin_config, $range, $vrf_id, $noerr);
# definedness check, because ID could be 0
if (!defined($internalid)) {
@@ -364,7 +382,8 @@ sub del_ip {
return if !$ip;
- my $ip_id = get_ip_id($plugin_config, $ip, $noerr);
+ my (undef, $vrf_id) = subnet_prefix($plugin_config, $subnet, $noerr);
+ my $ip_id = get_ip_id($plugin_config, $ip, $vrf_id, $noerr);
if (!defined($ip_id)) {
warn "could not find id for ip $ip";
return;
@@ -393,12 +412,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 ($@) {
@@ -435,14 +458,65 @@ 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";
+}
+
+my sub zone_vrf_name {
+ my ($zoneid) = @_;
+
+ return "pve-$zoneid";
+}
+
+sub get_zone_namespace {
+ my ($config, $zoneid, $noerr) = @_;
+
+ my $name = zone_vrf_name($zoneid);
+ my $result = eval { netbox_api_request($config, "GET", "/ipam/vrfs/?name=$name") };
+ if ($@) {
+ return if $noerr;
+ die "could not obtain ID for VRF $name: $@";
+ }
+
+ my $data = @{ $result->{results} }[0];
+ return $data->{id};
+}
+
+sub add_zone_namespace {
+ my ($config, $zoneid, $noerr) = @_;
+
+ my $name = zone_vrf_name($zoneid);
+ my $result = eval { netbox_api_request($config, "POST", "/ipam/vrfs/", { name => $name }) };
+ if ($@) {
+ return if $noerr;
+ die "could not create VRF $name: $@";
+ }
+
+ return $result->{id};
+}
+
+# a prefix from before sits in the global table and keeps its addresses there,
+# so a subnet is looked for in the zone's VRF first and in the global table
+# after it, with none as the VRF of the global table
+sub subnet_prefix {
+ my ($config, $subnet, $noerr) = @_;
+
+ my $vrf_id = get_zone_namespace($config, $subnet->{zone}, $noerr);
+ if (defined($vrf_id)) {
+ my $prefix_id = get_prefix_id($config, $subnet->{cidr}, $vrf_id, $noerr);
+ return ($prefix_id, $vrf_id) if defined($prefix_id);
+ }
+ return (get_prefix_id($config, $subnet->{cidr}, undef, $noerr), undef);
+}
+
+sub get_prefix_id {
+ my ($config, $cidr, $vrf_id, $noerr) = @_;
- my $result = eval { netbox_api_request($config, "GET", "/ipam/prefixes/?q=$ip") };
+ my $vrf = vrf_filter($vrf_id);
+ my $result = eval { netbox_api_request($config, "GET", "/ipam/prefixes/?prefix=$cidr&$vrf") };
if ($@) {
return if $noerr;
die "could not obtain ID for prefix $cidr: $@";
@@ -453,13 +527,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 ($@) {
@@ -473,9 +548,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: $@";
@@ -486,9 +562,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: $@";
@@ -499,9 +576,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 e1b30e293ab3..efc2b15f6a91 100644
--- a/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
+++ b/src/PVE/Network/SDN/Ipams/PhpIpamPlugin.pm
@@ -2,6 +2,8 @@ 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;
@@ -47,15 +49,21 @@ 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 only the zone's section is
+ # searched, not the configured one the subnets from before sit in
+ 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) };
#create subnet
if (!$internalid) {
+ $section //= add_zone_namespace($plugin_config, $subnet->{zone}, $headers, $noerr);
+ return if !defined($section);
my $params = {
subnet => $network,
mask => $mask,
@@ -85,7 +93,7 @@ sub del_subnet {
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
- my $internalid = get_prefix_id($url, $cidr, $headers);
+ my $internalid = subnet_id($plugin_config, $subnet, $headers);
return if !$internalid;
return; #fixme: check that prefix is empty exluding gateway, before delete
@@ -118,7 +126,7 @@ sub add_ip {
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
- my $internalid = get_prefix_id($url, $cidr, $headers);
+ my $internalid = subnet_id($plugin_config, $subnet, $headers);
my $params = {
ip => $ip,
@@ -138,7 +146,7 @@ sub add_ip {
if ($@) {
if ($is_gateway) {
die "error add subnet ip to ipam: ip $ip already exist: $@"
- if !is_ip_gateway($url, $ip, $headers) && !$noerr;
+ if !is_ip_gateway($url, $ip, $headers, $internalid) && !$noerr;
} else {
die "error add subnet ip to ipam: ip $ip already exist: $@" if !$noerr;
}
@@ -165,7 +173,7 @@ sub update_ip {
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
- my $ip_id = get_ip_id($url, $ip, $headers);
+ my $ip_id = get_ip_id($url, $ip, $headers, subnet_id($plugin_config, $subnet, $headers));
die "can't find ip address in ipam\n" if !$ip_id;
my $params = {
@@ -196,7 +204,7 @@ sub add_next_freeip {
my $fingerprint = $plugin_config->{fingerprint};
my $headers = ['Content-Type' => 'application/json; charset=UTF-8', 'Token' => $token];
- my $internalid = get_prefix_id($url, $cidr, $headers);
+ my $internalid = subnet_id($plugin_config, $subnet, $headers);
my $params = {
hostname => $hostname,
@@ -246,7 +254,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 {
@@ -273,6 +281,18 @@ sub del_ip {
return 1;
}
+# 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) = @_;
@@ -284,7 +304,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 = eval { 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",
@@ -295,17 +323,14 @@ sub get_ips_from_mac {
};
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});
- # }
+ 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);
@@ -338,27 +363,109 @@ sub on_update_hook {
#helpers
+# 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 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 $err =~ m/^Invalid response from server: 404 /;
+ die "could not look up section $name: $err";
+ }
+ my $section = $result->{data};
+ die "section $name is not under section $plugin_config->{section}\n"
+ if ($section->{masterSection} // 0) != $plugin_config->{section};
+ return $section->{id};
+}
+
+sub add_zone_namespace {
+ my ($plugin_config, $zoneid, $headers, $noerr) = @_;
+
+ my $name = zone_section_name($zoneid);
+ my $result = eval {
+ PVE::Network::SDN::api_request(
+ "POST",
+ "$plugin_config->{url}/sections/",
+ $headers,
+ { name => $name, masterSection => $plugin_config->{section} },
+ $plugin_config->{fingerprint},
+ );
+ };
+ if ($@) {
+ return if $noerr;
+ die "could not create section $name: $@";
+ }
+ return $result->{id};
+}
+
+# phpIPAM nests sections one level deep, so a configured section that is a
+# sub-section itself gets no zone sections and every zone's subnets stay in it
+sub shared_section {
+ my ($plugin_config, $headers) = @_;
+
+ my $result = PVE::Network::SDN::api_request(
+ "GET",
+ "$plugin_config->{url}/sections/$plugin_config->{section}/",
+ $headers,
+ undef,
+ $plugin_config->{fingerprint},
+ );
+ return ($result->{data}->{masterSection} // 0) ? $plugin_config->{section} : undef;
+}
+
+# a subnet from before sits in the configured section, so a subnet is looked
+# for in the zone's section first and in the configured one after it
+sub subnet_id {
+ my ($plugin_config, $subnet, $headers) = @_;
+
+ my $section = get_zone_namespace($plugin_config, $subnet->{zone}, $headers);
+ my $url = $plugin_config->{url};
+ return get_prefix_id($url, $subnet->{cidr}, $headers, $section)
+ // get_prefix_id($url, $subnet->{cidr}, $headers, $plugin_config->{section});
+}
+
sub get_prefix_id {
- my ($url, $cidr, $headers) = @_;
+ my ($url, $cidr, $headers, $section) = @_;
+ return undef if !defined($section);
my $result = PVE::Network::SDN::api_request("GET", "$url/subnets/cidr/$cidr", $headers);
- my $data = @{ $result->{data} }[0];
- my $internalid = $data->{id};
- return $internalid;
+ my ($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 0d3fed120d3f..4dbc78720163 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,115 @@ 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 range the zone's namespace does not hold is created there, also when a
+# subnet from before holds it in the global table or the configured section
+{
+ 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; },
+ );
+ PVE::Network::SDN::Ipams::NetboxPlugin->add_subnet(
+ { url => 'u', token => 't' },
+ 'z-10.0.0.0-24',
+ $subnet,
+ 1,
+ );
+ is_deeply(
+ $requests[0],
+ ['POST', 'u/ipam/prefixes/', { prefix => '10.0.0.0/24', vrf => 2 }],
+ 'netbox adds a range the global table holds to the zone\'s VRF',
+ );
+
+ @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;
+ },
+ );
+ PVE::Network::SDN::Ipams::PhpIpamPlugin->add_subnet(
+ { url => 'u', token => 't', section => 1 },
+ 'z-10.0.0.0-24',
+ $subnet,
+ 1,
+ );
+ is_deeply(
+ $requests[0],
+ ['POST', 'u/subnets/', { subnet => '10.0.0.0', mask => '24', sectionId => 2 }],
+ 'phpipam adds a range the configured section holds to the zone\'s section',
+ );
+}
+
+# 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');
+}
+
# an address released for a MAC stays when the external IPAM records it
# for another MAC
{
@@ -256,5 +368,114 @@ foreach my $path (@plugins) {
}
}
+# a zone's VRF is found and created under its prefixed name
+{
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return { results => [{ id => 4 }], id => 4 };
+ },
+ );
+ my $config = { url => 'u', token => 't' };
+ PVE::Network::SDN::Ipams::NetboxPlugin::get_zone_namespace($config, 'z');
+ PVE::Network::SDN::Ipams::NetboxPlugin::add_zone_namespace($config, 'z');
+ is_deeply(
+ \@requests,
+ [
+ ['GET', 'u/ipam/vrfs/?name=pve-z', undef],
+ ['POST', 'u/ipam/vrfs/', { name => 'pve-z' }],
+ ],
+ 'netbox finds and creates the zone\'s VRF under the name pve-<zone>',
+ );
+}
+
+# a zone without a section is told apart from a section lookup that failed
+{
+ my $error;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(api_request => sub { die $error; });
+ my $lookup = \&PVE::Network::SDN::Ipams::PhpIpamPlugin::get_zone_namespace;
+ my $config = { url => 'u', token => 't' };
+
+ $error = "Invalid response from server: 404 Not Found\n";
+ is(scalar($lookup->($config, 'z', [])), undef, 'phpipam reports a missing section as none');
+ $error = "Invalid response from server: 503 Service Unavailable\n";
+ eval { $lookup->($config, 'z', []) };
+ like($@, qr/could not look up section pve-z/, 'and dies on any other failure');
+
+ # a section of the zone's name elsewhere in phpIPAM is not the zone's
+ $sdn_module->mock(api_request => sub { return { data => { id => 7, masterSection => 2 } }; });
+ eval { $lookup->({ url => 'u', token => 't', section => 1 }, 'z', []) };
+ like($@, qr/not under section 1/, 'a section outside the configured one is refused');
+ $sdn_module->mock(api_request => sub { return { data => { id => 7, masterSection => 1 } }; });
+ is(
+ $lookup->({ url => 'u', token => 't', section => 1 }, 'z', []),
+ 7,
+ 'and one under it is the zone\'s',
+ );
+}
+
+# a zone's section is created under the configured section
+{
+ my @requests;
+ my $sdn_module = Test::MockModule->new("PVE::Network::SDN");
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return { id => 5 };
+ },
+ );
+ my $phpipam = Test::MockModule->new('PVE::Network::SDN::Ipams::PhpIpamPlugin');
+ $phpipam->mock(
+ get_zone_namespace => sub { return undef; },
+ get_prefix_id => sub { return undef; },
+ );
+ PVE::Network::SDN::Ipams::PhpIpamPlugin->add_subnet(
+ { url => 'u', token => 't', section => 1 },
+ 'z-10.0.0.0/24',
+ { zone => 'z', cidr => '10.0.0.0/24', network => '10.0.0.0', mask => '24' },
+ 1,
+ );
+ is_deeply(
+ [map { [$_->[0], $_->[1], $_->[2]] } @requests],
+ [
+ ['GET', 'u/sections/1/', undef],
+ ['POST', 'u/sections/', { name => 'pve-z', masterSection => 1 }],
+ ['POST', 'u/subnets/', { subnet => '10.0.0.0', mask => '24', sectionId => 5 }],
+ ],
+ 'phpipam creates the zone\'s section under the configured one',
+ );
+
+ # a configured section that is a sub-section itself gets no zone sections
+ @requests = ();
+ $sdn_module->mock(
+ api_request => sub {
+ my ($method, $url, undef, $data) = @_;
+ push @requests, [$method, $url, $data];
+ return $url eq 'u/sections/1/'
+ ? { data => { id => 1, masterSection => 3 } }
+ : { id => 5 };
+ },
+ );
+ PVE::Network::SDN::Ipams::PhpIpamPlugin->add_subnet(
+ { url => 'u', token => 't', section => 1 },
+ 'z-10.0.0.0/24',
+ { zone => 'z', cidr => '10.0.0.0/24', network => '10.0.0.0', mask => '24' },
+ 1,
+ );
+ is_deeply(
+ [map { [$_->[0], $_->[1], $_->[2]] } @requests],
+ [
+ ['GET', 'u/sections/1/', undef],
+ ['POST', 'u/subnets/', { subnet => '10.0.0.0', mask => '24', sectionId => 1 }],
+ ],
+ 'and keeps the subnets in a configured section that is a sub-section itself',
+ );
+}
+
done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH pve-docs 10/10] sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM
2026-11-02 11:05 [PATCH docs/network 00/10] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
` (8 preceding siblings ...)
2026-11-02 11:06 ` [PATCH pve-network 09/10] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM Hannes Laimer
@ 2026-11-02 11:06 ` Hannes Laimer
9 siblings, 0 replies; 11+ messages in thread
From: Hannes Laimer @ 2026-11-02 11:06 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 | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/pvesdn.adoc b/pvesdn.adoc
index 3fd3533ad07f..3516102ce511 100644
--- a/pvesdn.adoc
+++ b/pvesdn.adoc
@@ -1287,6 +1287,13 @@ 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.
+
[[pvesdn_ipam_plugin_phpipam]]
phpIPAM Plugin
@@ -1304,6 +1311,14 @@ 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.
+
[[pvesdn_config_dns]]
DNS
--
2.47.3
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-11-02 11:07 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-02 11:05 [PATCH docs/network 00/10] sdn: improve MAC cache, DHCP reservations and scope both to the vnet Hannes Laimer
2026-11-02 11:05 ` [PATCH pve-network 01/10] sdn: ipam: stop caching IPAM misses Hannes Laimer
2026-11-02 11:05 ` [PATCH pve-network 02/10] sdn: ipam: write the lookup's cache entry under the cluster lock Hannes Laimer
2026-11-02 11:05 ` [PATCH pve-network 03/10] sdn: ipam: read the MAC cache case-insensitively Hannes Laimer
2026-11-02 11:05 ` [PATCH pve-network 04/10] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it Hannes Laimer
2026-11-02 11:05 ` [PATCH pve-network 05/10] sdn: ipam: release an address the built-in IPAM no longer holds Hannes Laimer
2026-11-02 11:06 ` [PATCH pve-network 06/10] sdn: dhcp: update reservations when ip mappings change Hannes Laimer
2026-11-02 11:06 ` [PATCH pve-network 07/10] sdn: ipam: look a MAC up within the vnet's subnets Hannes Laimer
2026-11-02 11:06 ` [PATCH pve-network 08/10] sdn: ipam: keep the MAC cache per vnet Hannes Laimer
2026-11-02 11:06 ` [PATCH pve-network 09/10] sdn: ipam: give each zone a VRF in NetBox and a section in phpIPAM Hannes Laimer
2026-11-02 11:06 ` [PATCH pve-docs 10/10] sdn: ipam: document the per-zone VRF and section in NetBox and phpIPAM Hannes Laimer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox