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 8A2591FF0C0 for ; Wed, 04 Nov 2026 10:56:09 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 2510721668; Wed, 04 Nov 2026 10:55:43 +0100 (CET) From: Hannes Laimer To: pve-devel@lists.proxmox.com Subject: [PATCH pve-network v5 03/13] sdn: ipam: read the MAC cache case-insensitively Date: Wed, 4 Nov 2026 10:55:10 +0100 Message-ID: <20261104095520.398544-4-h.laimer@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261104095520.398544-1-h.laimer@proxmox.com> References: <20261104095520.398544-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: 1793786129174 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.315 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: TWCNYQDTXIUIU7USOH7O3CFI2NBZ2XCE X-Message-ID-Hash: TWCNYQDTXIUIU7USOH7O3CFI2NBZ2XCE 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. The cache stored each MAC as its writer spelled it. A deletion in the other spelling missed the entry, so the cache kept answering with the deleted address. Lookups now match a MAC in any spelling, and a write or a deletion reaches every spelling of it in the cache file. The file keeps its keys as written, since nodes not yet upgraded look a MAC up by its exact spelling. A MAC new to the file is written in lower case. If two spellings disagree on an address, the cache cannot tell which is stale. A lookup then ignores them, asks the IPAM and writes its answer under both. Adding a dnsmasq reservation checks every line of its file against the cache and drops the ones that do not match. The check ignores case as well, since an existing line keeps its writer's spelling. Signed-off-by: Hannes Laimer --- src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 3 ++ src/PVE/Network/SDN/Ipams.pm | 72 +++++++++++++++++++++-------- src/test/run_test_vnets_blackbox.pl | 51 +++++++++++++++++++- 3 files changed, 107 insertions(+), 19 deletions(-) diff --git a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm index 477b700f2429..dc70b9a99a2e 100644 --- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm +++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm @@ -53,6 +53,7 @@ sub update_lease { sub add_ip_mapping { my ($class, $dhcpid, $macdb, $mac, $ip4, $ip6) = @_; + $mac = lc($mac); my $ethers_file = ethers_file($dhcpid); my $ethers_tmp_file = "$ethers_file.tmp"; @@ -76,6 +77,8 @@ sub add_ip_mapping { warn "failed to parse MAC from $dhcpid ethers file on line $line_no: '$line'\n"; next; } + # the cache is read in lower case, a line from before keeps its writer's spelling + $parsed_mac = lc($parsed_mac); my ($parsed_ip4, $parsed_ip6); if ($parsed_ip2) { diff --git a/src/PVE/Network/SDN/Ipams.pm b/src/PVE/Network/SDN/Ipams.pm index ae2c4337ef8d..1476c4236be0 100644 --- a/src/PVE/Network/SDN/Ipams.pm +++ b/src/PVE/Network/SDN/Ipams.pm @@ -35,18 +35,58 @@ sub json_writer { return encode_json($data); } -sub read_macdb { +# Writers spell a MAC their own way, the generated ones in upper case and the +# API in either, and nodes not yet upgraded look an entry up under the spelling +# their caller hands them, so the file keeps its keys as written. Readers get a +# view with every MAC in lower case. Two spellings that disagree on an address +# are left out of it, since one of them is stale and nothing here says which, +# and so is an entry without an address. The next lookup then asks the IPAM +my sub fold_macdb { + my ($db) = @_; + + my $macs = {}; + my %clash; + for my $key (sort keys %{ $db->{macs} // {} }) { + my $entry = $db->{macs}->{$key}; + for my $family (grep { defined($entry->{$_}) } qw(ip4 ip6)) { + my $seen = $macs->{ lc($key) }->{$family}; + $clash{ lc($key) } = 1 if defined($seen) && lc($seen) ne lc($entry->{$family}); + $macs->{ lc($key) }->{$family} //= $entry->{$family}; + } + } + delete $macs->{$_} for keys %clash; + return { %$db, macs => $macs }; +} + +# the file as written, which the writers change and write back +sub read_macdb_file { my () = @_; return cfs_read_file($macdb_filename); } +sub read_macdb { + my () = @_; + + return fold_macdb(read_macdb_file()); +} + sub write_macdb { my ($data) = @_; cfs_write_file($macdb_filename, $data); } +# a write goes to every spelling the file holds for the MAC, so each reader +# finds it under its own, and a MAC new to the file is written in lower case +my sub set_cache_entry { + my ($db, $mac, $family, $ip) = @_; + + my $macs = $db->{macs} //= {}; + my @keys = grep { lc($_) eq lc($mac) } keys %$macs; + $macs->{$_}->{$family} = $ip for (@keys ? @keys : (lc($mac))); +} + sub add_cache_mac_ip { my ($mac, $ip) = @_; @@ -54,12 +94,8 @@ sub add_cache_mac_ip { $macdb_filename, undef, sub { - my $db = read_macdb(); - if (Net::IP::ip_is_ipv4($ip)) { - $db->{macs}->{$mac}->{ip4} = $ip; - } else { - $db->{macs}->{$mac}->{ip6} = $ip; - } + my $db = read_macdb_file(); + set_cache_entry($db, $mac, Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6', $ip); write_macdb($db); }, ); @@ -73,14 +109,14 @@ sub del_cache_mac_ip { $macdb_filename, undef, sub { - my $db = read_macdb(); - if (Net::IP::ip_is_ipv4($ip)) { - delete $db->{macs}->{$mac}->{ip4}; - } else { - delete $db->{macs}->{$mac}->{ip6}; + my $db = read_macdb_file(); + my $family = Net::IP::ip_is_ipv4($ip) ? 'ip4' : 'ip6'; + my $macs = $db->{macs} // {}; + for my $key (grep { lc($_) eq lc($mac) } keys %$macs) { + delete $macs->{$key}->{$family}; + delete $macs->{$key} + if !defined($macs->{$key}->{ip4}) && !defined($macs->{$key}->{ip6}); } - delete $db->{macs}->{$mac} - if !defined($db->{macs}->{$mac}->{ip4}) && !defined($db->{macs}->{$mac}->{ip6}); write_macdb($db); }, ); @@ -137,7 +173,7 @@ sub get_ips_from_mac { PVE::Cluster::cfs_update(); # the lock refreshes, so the snapshot must too my $version = PVE::Cluster::cfs_file_version($macdb_filename); - my $cached = read_macdb()->{macs}->{$mac}; + my $cached = read_macdb()->{macs}->{ lc($mac) }; # an entry without an address was left behind by a lookup that missed, and # says nothing about its MAC return ($cached->{ip4}, $cached->{ip6}) @@ -159,9 +195,9 @@ sub get_ips_from_mac { return if (PVE::Cluster::cfs_file_version($macdb_filename) // '') ne ($version // ''); - my $db = read_macdb(); - $db->{macs}->{$mac}->{ip4} = $ip4 if defined($ip4); - $db->{macs}->{$mac}->{ip6} = $ip6 if defined($ip6); + my $db = read_macdb_file(); + set_cache_entry($db, $mac, 'ip4', $ip4) if defined($ip4); + set_cache_entry($db, $mac, 'ip6', $ip6) if defined($ip6); write_macdb($db); }, ); diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl index 927ab16adba9..be4c581e414e 100755 --- a/src/test/run_test_vnets_blackbox.pl +++ b/src/test/run_test_vnets_blackbox.pl @@ -194,7 +194,7 @@ $mocked_sdn_ipams->mock( my ($cfg) = @_; $test_state->{ipam_config} = $cfg; }, - read_macdb => sub { + read_macdb_file => sub { return PVE::Network::SDN::Ipams::json_reader(undef, encode_json($test_state->{macdb})); }, write_macdb => sub { @@ -498,6 +498,55 @@ sub test_ipam_cache_lookup { is($ips[0], "10.0.0.50", "$test_name: the lookup still answers"); ok(!exists $test_state->{macdb}->{macs}->{$mac}, "$test_name: but leaves a changed cache alone"); + + # an entry written under the config's spelling, as a node not yet upgraded + # writes it, is read and released like any other and keeps its key + my $other = "da:65:8f:18:9b:70"; + $test_state->{macdb}->{macs} = { + uc($mac) => { ip4 => '10.0.0.150' }, + uc($other) => { ip4 => '10.0.0.170' }, + }; + my ($ip4) = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac); + is($ip4, '10.0.0.150', "$test_name: an entry is found however it is spelled"); + PVE::Network::SDN::Ipams::del_cache_mac_ip($mac, '10.0.0.150'); + is_deeply( + $test_state->{macdb}->{macs}, + { uc($other) => { ip4 => '10.0.0.170' } }, + "$test_name: and released, the other entry left as written", + ); + + # a write in another spelling goes to the spelling the file holds + $test_state->{macdb}->{macs} = { uc($mac) => { ip4 => '10.0.0.150' } }; + PVE::Network::SDN::Ipams::add_cache_mac_ip($mac, 'fd00::150'); + is_deeply( + $test_state->{macdb}->{macs}, + { uc($mac) => { ip4 => '10.0.0.150', ip6 => 'fd00::150' } }, + "$test_name: a write keeps the key as written", + ); + + # a lookup past an address-less entry asks the IPAM and fills the entry + # under its spelling + $test_state->{macdb}->{macs} = { uc($mac) => { ip4 => undef, ip6 => undef } }; + @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac); + is($ips[0], '10.0.0.50', "$test_name: a lookup past an address-less entry asks the IPAM"); + is_deeply( + $test_state->{macdb}->{macs}, + { uc($mac) => { ip4 => '10.0.0.50', ip6 => undef } }, + "$test_name: and fills the entry under its spelling", + ); + + # two spellings that disagree are not trusted, the lookup asks the IPAM + $test_state->{macdb}->{macs} = { + uc($mac) => { ip4 => '10.0.0.150' }, + $mac => { ip4 => '10.0.0.151' }, + }; + @ips = PVE::Network::SDN::Vnets::get_ips_from_mac($vnetid, $mac); + is($ips[0], '10.0.0.50', "$test_name: a clash of spellings counts as a miss"); + is_deeply( + $test_state->{macdb}->{macs}, + { uc($mac) => { ip4 => '10.0.0.50' }, $mac => { ip4 => '10.0.0.50' } }, + "$test_name: and its write brings both spellings in line", + ); } run_test(\&test_ipam_cache_lookup); -- 2.47.3