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 5FB3F1FF0AA for ; Tue, 20 Oct 2026 15:15:31 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id C6AE6216A4; Tue, 20 Oct 2026 15:15:29 +0200 (CEST) Message-ID: <4efa7c58-9770-4cf4-bc1b-6ce8dde80736@proxmox.com> Date: Tue, 20 Oct 2026 15:15:25 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH pve-network v3 04/17] sdn: dhcp: replace a stale dnsmasq reservation instead of dropping it To: pve-devel@lists.proxmox.com References: <20261020080005.470757-1-h.laimer@proxmox.com> <20261020080005.470757-5-h.laimer@proxmox.com> Content-Language: en-US From: Stefan Hanreich In-Reply-To: <20261020080005.470757-5-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.380 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: 75BCHRM272CYBWQUPLONA2MT55VLBBJY X-Message-ID-Hash: 75BCHRM272CYBWQUPLONA2MT55VLBBJY X-MailFrom: s.hanreich@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: Reviewed-by: Stefan Hanreich Tested-by: Stefan Hanreich On 10/20/26 10:00 AM, Hannes Laimer wrote: > 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 > --- > 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 477b700f..5c9496c0 100644 > --- a/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm > +++ b/src/PVE/Network/SDN/Dhcp/Dnsmasq.pm > @@ -88,7 +88,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 > @@ -99,6 +101,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 9f4c424f..36225a92 100755 > --- a/src/test/run_test_vnets_blackbox.pl > +++ b/src/test/run_test_vnets_blackbox.pl > @@ -963,4 +963,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();