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 3FAB71FF0AA for ; Tue, 20 Oct 2026 15:17:18 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 08537216C5; Tue, 20 Oct 2026 15:17:17 +0200 (CEST) Message-ID: <4272d5a6-464d-4e75-af52-a69dfb9f5489@proxmox.com> Date: Tue, 20 Oct 2026 15:17:12 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH pve-network v3 08/17] sdn: dhcp: require a backend only if a zone on this node uses it To: pve-devel@lists.proxmox.com References: <20261020080005.470757-1-h.laimer@proxmox.com> <20261020080005.470757-9-h.laimer@proxmox.com> Content-Language: en-US From: Stefan Hanreich In-Reply-To: <20261020080005.470757-9-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.368 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: BRKUH34TYOAJ4GNJP63MMCPOFLQAECRM X-Message-ID-Hash: BRKUH34TYOAJ4GNJP63MMCPOFLQAECRM 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:01 AM, Hannes Laimer wrote: > A network reload failed on a node without the dnsmasq package as soon > as any zone had DHCP, even a zone confined to other nodes. On a node > with dnsmasq, the reload wrote such a zone's configuration and started > an instance for it, although none of the zone's vnets exist there. > > A reload now skips the zones confined to other nodes, so a backend is > required and started only for the zones on the reloading node. > > Signed-off-by: Hannes Laimer > --- > src/PVE/Network/SDN/Dhcp.pm | 13 +++++++-- > src/test/run_test_vnets_blackbox.pl | 43 +++++++++++++++++++++++++++++ > 2 files changed, 53 insertions(+), 3 deletions(-) > > diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm > index ab5d6b5b..2a023534 100644 > --- a/src/PVE/Network/SDN/Dhcp.pm > +++ b/src/PVE/Network/SDN/Dhcp.pm > @@ -63,17 +63,24 @@ sub regenerate_config { > > my $plugins = PVE::Network::SDN::Dhcp::Plugin->lookup_types(); > > - my $any_zone_needs_dhcp = grep { $_->{dhcp} } values $zone_cfg->{ids}->%*; > + my %plugin_needed = (); > + for my $zone (values $zone_cfg->{ids}->%*) { > + next if !$zone->{dhcp}; > + # a zone confined to other nodes runs no backend here > + next if defined($zone->{nodes}) && !$zone->{nodes}->{$nodename}; > + $plugin_needed{ $zone->{dhcp} } = 1; > + } > > foreach my $plugin_name (@$plugins) { > my $plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($plugin_name); > - eval { $plugin->before_regenerate(!$any_zone_needs_dhcp) }; > + eval { $plugin->before_regenerate(!$plugin_needed{$plugin_name}) }; > die "Could not run before_regenerate for DHCP plugin $plugin_name $@\n" if $@; > } > > foreach my $zoneid (sort keys %{ $zone_cfg->{ids} }) { > my $zone = $zone_cfg->{ids}->{$zoneid}; > next if !$zone->{dhcp}; > + next if defined($zone->{nodes}) && !$zone->{nodes}->{$nodename}; > > my $dhcp_plugin_name = $zone->{dhcp}; > my $dhcp_plugin = PVE::Network::SDN::Dhcp::Plugin->lookup($dhcp_plugin_name); > @@ -114,7 +121,7 @@ sub regenerate_config { > warn "Could not configure vnet $vnetid: $@\n" if $@; > } > > - eval { $dhcp_plugin->after_configure($zoneid, !$any_zone_needs_dhcp) }; > + eval { $dhcp_plugin->after_configure($zoneid, !$plugin_needed{$dhcp_plugin_name}) }; > warn "Could not run after_configure for DHCP server $zoneid $@\n" if $@; > > } > diff --git a/src/test/run_test_vnets_blackbox.pl b/src/test/run_test_vnets_blackbox.pl > index dc54bba3..cda4b7ef 100755 > --- a/src/test/run_test_vnets_blackbox.pl > +++ b/src/test/run_test_vnets_blackbox.pl > @@ -104,6 +104,13 @@ my $mocked_pve_sdn; > $mocked_pve_sdn = Test::MockModule->new('PVE::Network::SDN'); > $mocked_pve_sdn->mock( > cfs_lock_file => $mocked_cfs_lock_file, > + running_config => sub { > + return { > + zones => $test_state->{zones_config}, > + vnets => $test_state->{vnets_config}, > + subnets => $test_state->{subnets_config}, > + }; > + }, > ); > > my $mocked_pve_tools = Test::MockModule->new('PVE::Tools'); > @@ -1169,6 +1176,42 @@ sub test_dnsmasq_lease_time { > > run_test(\&test_dnsmasq_lease_time); > > +sub test_dhcp_backend_needed_per_node { > + my $test_name = (split(/::/, (caller(0))[3]))[-1]; > + > + # a backend is told whether a zone on this node uses it. A zone confined > + # to other nodes runs nothing here > + my $asked = []; > + my $configured = []; > + $mocked_sdn_dhcp_dnsmasq->mock( > + before_regenerate => sub { push @$asked, $_[1] ? 'optional' : 'needed'; }, > + after_configure => sub { push @$configured, $_[1]; }, > + ); > + create_zone({ > + type => "simple", > + dhcp => "dnsmasq", > + ipam => "pve", > + zone => "DNSZONE", > + nodes => 'other', > + }); > + PVE::Network::SDN::Dhcp::regenerate_config(); > + eq_or_diff( > + $asked, > + ['optional'], > + "$test_name: a dnsmasq zone confined elsewhere needs no dnsmasq here", > + ); > + eq_or_diff($configured, [], "$test_name: and is not configured here"); > + > + @$asked = (); > + update_zone("DNSZONE", { delete => 'nodes' }); > + PVE::Network::SDN::Dhcp::regenerate_config(); > + eq_or_diff($asked, ['needed'], "$test_name: a dnsmasq zone on this node needs it"); > + eq_or_diff($configured, ['DNSZONE'], "$test_name: and is configured"); > + $mocked_sdn_dhcp_dnsmasq->unmock($_) for qw(before_regenerate after_configure); > +} > + > +run_test(\&test_dhcp_backend_needed_per_node); > + > sub test_ipam_cache_misses { > my $test_name = (split(/::/, (caller(0))[3]))[-1]; > my $zoneid = "TESTZONE";