From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id AD7C61FF09C for ; Mon, 02 Nov 2026 15:56:47 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 7F989216BF; Mon, 02 Nov 2026 15:56:39 +0100 (CET) From: Hannes Laimer To: pve-devel@lists.proxmox.com Subject: [PATCH pve-network v2 01/11] sdn: ipam: stop caching IPAM misses Date: Mon, 2 Nov 2026 15:56:20 +0100 Message-ID: <20261102145630.1555953-2-h.laimer@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261102145630.1555953-1-h.laimer@proxmox.com> References: <20261102145630.1555953-1-h.laimer@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793631394952 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.366 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: F5HZ3OK7SZTDLJPRIZ3CASQ3KSNVYVOS X-Message-ID-Hash: F5HZ3OK7SZTDLJPRIZ3CASQ3KSNVYVOS X-MailFrom: h.laimer@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- src/PVE/Network/SDN/Ipams.pm | 16 ++++++-- src/test/run_test_vnets_blackbox.pl | 58 ++++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm index 179bdf76defc..7e7bec27b5ed 100644 --- a/src/PVE/Network/SDN/Ipams.pm +++ b/src/PVE/Network/SDN/Ipams.pm @@ -136,16 +136,24 @@ sub get_ips_from_mac { my ($mac, $zoneid, $zone) = @_; my $macdb = read_macdb(); - return ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6}) if $macdb->{macs}->{$mac}; + my $cached = $macdb->{macs}->{$mac}; + # an entry without an address was left behind by a lookup that missed, and + # says nothing about its MAC + return ($cached->{ip4}, $cached->{ip6}) + if $cached && (defined($cached->{ip4}) || defined($cached->{ip6})); my $plugin_config = get_plugin_config($zone); my $plugin = PVE::Network::SDN::Ipams::Plugin->lookup($plugin_config->{type}); - ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6}) = - $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid); + my ($ip4, $ip6) = $plugin->get_ips_from_mac($plugin_config, $mac, $zoneid); + # an empty answer is not cached, since the record may simply not exist yet + return if !defined($ip4) && !defined($ip6); + + $macdb->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4); + $macdb->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6); write_macdb($macdb); - return ($macdb->{macs}->{$mac}->{ip4}, $macdb->{macs}->{$mac}->{ip6}); + return ($ip4, $ip6); } 1; diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl index 9f4c424f3d2f..fd3cfa949108 100755 --- a/src/test/run_test_vnets_blackbox.pl +++ b/src/test/run_test_vnets_blackbox.pl @@ -5,6 +5,7 @@ use warnings; use lib qw(..); use File::Slurp; +use JSON; use List::Util qw(first all); use NetAddr::IP qw(:lower); @@ -193,7 +194,7 @@ $mocked_sdn_ipams->mock( $test_state->{ipam_config} = $cfg; }, read_macdb => sub { - return $test_state->{macdb}; + return PVE::Network::SDN::Ipams::json_reader(undef, encode_json($test_state->{macdb})); }, write_macdb => sub { my ($cfg) = @_; @@ -321,6 +322,34 @@ sub create_subnet { PVE::API2::Network::SDN::Subnets->create($params); } +# a DHCP zone with one vnet, each subnet with its first address as the +# gateway and a range over its hosts 100 to 200 +sub create_dhcp_zone { + my ($zoneid, $vnetid, @cidrs) = @_; + create_zone({ type => "simple", dhcp => "dnsmasq", ipam => "pve", zone => $zoneid }); + create_dhcp_vnet($zoneid, $vnetid, @cidrs); +} + +sub create_dhcp_vnet { + my ($zoneid, $vnetid, @cidrs) = @_; + create_vnet({ type => "vnet", zone => $zoneid, vnet => $vnetid }); + create_dhcp_subnet($vnetid, $_) for @cidrs; +} + +sub create_dhcp_subnet { + my ($vnetid, $cidr) = @_; + my $net = NetAddr::IP->new($cidr); + my $addr = sub { my ($ip) = @_; return $ip->version() == 4 ? $ip->addr() : $ip->short(); }; + create_subnet({ + type => "subnet", + vnet => $vnetid, + subnet => $cidr, + gateway => $addr->($net->first()), + 'dhcp-range' => + ["start-address=" . $addr->($net + 100) . ",end-address=" . $addr->($net + 200)], + }); +} + sub get_ipam_entries { return PVE::API2::Network::SDN::Ipams->ipamindex({ ipam => "pve" }); } @@ -421,6 +450,33 @@ sub test_without_subnet { } run_test(\&test_without_subnet); +sub test_ipam_cache_lookup { + my $test_name = (split(/::/, (caller(0))[3]))[-1]; + my $zoneid = "TESTZONE"; + my $vnetid = "testvnet"; + my $mac = "da:65:8f:18:9b:6f"; + + create_dhcp_zone($zoneid, $vnetid, "10.0.0.0/24"); + + my $cache = decode_json(encode_json($test_state->{macdb})); + PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac); + is_deeply($test_state->{macdb}, $cache, "$test_name: the miss is not cached"); + + # an entry holding no address, left behind by an earlier miss, does not + # hide a record created since + create_ip({ zone => $zoneid, vnet => $vnetid, mac => $mac, ip => "10.0.0.50" }); + $test_state->{macdb}->{macs}->{$mac} = { ip4 => undef, ip6 => undef }; + my @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac); + is($ips[0], "10.0.0.50", "$test_name: an address-less entry counts as a miss"); + is( + $test_state->{macdb}->{macs}->{$mac}->{ip4}, + "10.0.0.50", + "$test_name: the answer is cached", + ); +} + +run_test(\&test_ipam_cache_lookup); + sub test_nic_join { my ($test_name, $subnets) = @_; -- 2.47.3