From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 5F6611FF09C for ; Mon, 02 Nov 2026 15:57:14 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id C57A12171F; Mon, 02 Nov 2026 15:56:53 +0100 (CET) From: Hannes Laimer To: pve-devel@lists.proxmox.com Subject: [PATCH pve-network v2 03/11] sdn: ipam: read the MAC cache case-insensitively Date: Mon, 2 Nov 2026 15:56:22 +0100 Message-ID: <20261102145630.1555953-4-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: 1793631397119 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.363 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: 6AXD7TMPRK2YK4PAMRTHQCQQLN4XUNJQ X-Message-ID-Hash: 6AXD7TMPRK2YK4PAMRTHQCQQLN4XUNJQ 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: 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 --- src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 3 ++ src/PVE/Network/SDN/Ipams.pm | 72 +++++++++++++++++++++-------- src/test/run_test_vnets_blackbox.pl | 51 +++++++++++++++++++- 3 files changed, 107 insertions(+), 19 deletions(-) diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm index 477b700f2429..d3b78fe9a894 100644 --- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm +++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm @@ -53,6 +53,7 @@ sub update_lease { sub add_ip_mapping { my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_; + $mac = lc($mac); my $ethers_file = ethers_file($dhcpid); my $ethers_tmp_file = "$ethers_file.tmp"; @@ -76,6 +77,8 @@ sub add_ip_mapping { warn "failed to parse MAC from $dhcpid ethers file on line $line_no: '$line'\n"; next; } + # the cache is keyed in lower case, a line from before keeps its writer's spelling + $parsed_mac = lc($parsed_mac); my ($parsed_ip4, $parsed_ip6); if ($parsed_ip2) { diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm index ae2c4337ef8d..1476c4236be0 100644 --- a/src/PVE/Network/SDN/Ipams.pm +++ b/src/PVE/Network/SDN/Ipams.pm @@ -35,18 +35,58 @@ sub json_writer { return encode_json($data); } -sub read_macdb { +# Writers spell a MAC their own way, the generated ones in upper case and the +# API in either, and nodes not yet upgraded look an entry up under the spelling +# their caller hands them, so the file keeps its keys as written. Readers get a +# view with every MAC in lower case. Two spellings that disagree on an address +# are left out of it, since one of them is stale and nothing here says which, +# and so is an entry without an address. The next lookup then asks the IPAM +my sub fold_macdb { + my ($db) = @_; + + my $macs = {}; + my %clash; + for my $key (sort keys %{ $db->{macs} // {} }) { + my $entry = $db->{macs}->{$key}; + for my $family (grep { defined($entry->{$_}) } qw(ip4 ip6)) { + my $seen = $macs->{ lc($key) }->{$family}; + $clash{ lc($key) } = 1 if defined($seen) && lc($seen) ne lc($entry->{$family}); + $macs->{ lc($key) }->{$family} //= $entry->{$family}; + } + } + delete $macs->{$_} for keys %clash; + return { %$db, macs => $macs }; +} + +# the file as written, which the writers change and write back +sub read_macdb_file { my () = @_; return cfs_read_file($macdb_filename); } +sub read_macdb { + my () = @_; + + return fold_macdb(read_macdb_file()); +} + sub write_macdb { my ($data) = @_; cfs_write_file($macdb_filename, $data); } +# a write goes to every spelling the file holds for the MAC, so each reader +# finds it under its own, and a MAC new to the file is written in lower case +my sub set_cache_entry { + my ($db, $mac, $family, $ip) = @_; + + my $macs = $db->{macs} //= {}; + my @keys = grep { lc($_) eq lc($mac) } keys %$macs; + $macs->{$_}->{$family} = $ip for (@keys ? @keys : (lc($mac))); +} + sub add_cache_mac_ip { my ($mac, $ip) = @_; @@ -54,12 +94,8 @@ sub add_cache_mac_ip { $macdb_filename, undef, sub { - my $db = read_macdb(); - if (Net::IP::ip_is_ipv4($ip)) { - $db->{macs}->{$mac}->{ip4} = $ip; - } else { - $db->{macs}->{$mac}->{ip6} = $ip; - } + my $db = read_macdb_file(); + set_cache_entry($db, $mac, Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6', $ip); write_macdb($db); }, ); @@ -73,14 +109,14 @@ sub del_cache_mac_ip { $macdb_filename, undef, sub { - my $db = read_macdb(); - if (Net::IP::ip_is_ipv4($ip)) { - delete $db->{macs}->{$mac}->{ip4}; - } else { - delete $db->{macs}->{$mac}->{ip6}; + my $db = read_macdb_file(); + my $family = Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6'; + my $macs = $db->{macs} // {}; + for my $key (grep { lc($_) eq lc($mac) } keys %$macs) { + delete $macs->{$key}->{$family}; + delete $macs->{$key} + if !defined($macs->{$key}->{ip4}) && !defined($macs->{$key}->{ip6}); } - delete $db->{macs}->{$mac} - if !defined($db->{macs}->{$mac}->{ip4}) && !defined($db->{macs}->{$mac}->{ip6}); write_macdb($db); }, ); @@ -137,7 +173,7 @@ sub get_ips_from_mac { PVE::Cluster::cfs_update(); # the lock refreshes, so the snapshot must too my $version = PVE::Cluster::cfs_file_version($macdb_filename); - my $cached = read_macdb()->{macs}->{$mac}; + my $cached = read_macdb()->{macs}->{ lc($mac) }; # an entry without an address was left behind by a lookup that missed, and # says nothing about its MAC return ($cached->{ip4}, $cached->{ip6}) @@ -159,9 +195,9 @@ sub get_ips_from_mac { return if (PVE::Cluster::cfs_file_version($macdb_filename) // '') ne ($version // ''); - my $db = read_macdb(); - $db->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4); - $db->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6); + my $db = read_macdb_file(); + set_cache_entry($db, $mac, 'ip4', $ip4) if defined($ip4); + set_cache_entry($db, $mac, 'ip6', $ip6) if defined($ip6); write_macdb($db); }, ); diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl index 0362b7ca2f8d..e9c9bbb6be32 100755 --- a/src/test/run_test_vnets_blackbox.pl +++ b/src/test/run_test_vnets_blackbox.pl @@ -193,7 +193,7 @@ $mocked_sdn_ipams->mock( my ($cfg) = @_; $test_state->{ipam_config} = $cfg; }, - read_macdb => sub { + read_macdb_file => sub { return PVE::Network::SDN::Ipams::json_reader(undef, encode_json($test_state->{macdb})); }, write_macdb => sub { @@ -484,6 +484,55 @@ sub test_ipam_cache_lookup { is($ips[0], "10.0.0.50", "$test_name: the lookup still answers"); ok(!exists $test_state->{macdb}->{macs}->{$mac}, "$test_name: but leaves a changed cache alone"); + + # an entry written under the config's spelling, as a node not yet upgraded + # writes it, is read and released like any other and keeps its key + my $other = "da:65:8f:18:9b:70"; + $test_state->{macdb}->{macs} = { + uc($mac) => { ip4 => '10.0.0.150' }, + uc($other) => { ip4 => '10.0.0.170' }, + }; + my ($ip4) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac); + is($ip4, '10.0.0.150', "$test_name: an entry is found however it is spelled"); + PVE::Network::SDN::Ipams::del_cache_mac_ip($mac, '10.0.0.150'); + is_deeply( + $test_state->{macdb}->{macs}, + { uc($other) => { ip4 => '10.0.0.170' } }, + "$test_name: and released, the other entry left as written", + ); + + # a write in another spelling goes to the spelling the file holds + $test_state->{macdb}->{macs} = { uc($mac) => { ip4 => '10.0.0.150' } }; + PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, 'fd00::150'); + is_deeply( + $test_state->{macdb}->{macs}, + { uc($mac) => { ip4 => '10.0.0.150', ip6 => 'fd00::150' } }, + "$test_name: a write keeps the key as written", + ); + + # a lookup past an address-less entry asks the IPAM and fills the entry + # under its spelling + $test_state->{macdb}->{macs} = { uc($mac) => { ip4 => undef, ip6 => undef } }; + @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac); + is($ips[0], '10.0.0.50', "$test_name: a lookup past an address-less entry asks the IPAM"); + is_deeply( + $test_state->{macdb}->{macs}, + { uc($mac) => { ip4 => '10.0.0.50', ip6 => undef } }, + "$test_name: and fills the entry under its spelling", + ); + + # two spellings that disagree are not trusted, the lookup asks the IPAM + $test_state->{macdb}->{macs} = { + uc($mac) => { ip4 => '10.0.0.150' }, + $mac => { ip4 => '10.0.0.151' }, + }; + @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac); + is($ips[0], '10.0.0.50', "$test_name: a clash of spellings counts as a miss"); + is_deeply( + $test_state->{macdb}->{macs}, + { uc($mac) => { ip4 => '10.0.0.50' }, $mac => { ip4 => '10.0.0.50' } }, + "$test_name: and its write brings both spellings in line", + ); } run_test(\&test_ipam_cache_lookup); -- 2.47.3