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 048581FF09B for ; Mon, 31 Aug 2026 15:54:42 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 5354F212E0; Mon, 31 Aug 2026 15:54:41 +0200 (CEST) From: Daniel Herzig To: Hannes Laimer Subject: Re: [pve-network v2 4/4] dhcp: make use of get_subnets_with_config In-Reply-To: References: <20260831110116.300798-1-d.herzig@proxmox.com> <20260831110116.300798-5-d.herzig@proxmox.com> Date: Mon, 31 Aug 2026 15:54:38 +0200 Message-ID: <87ik4q1jkx.fsf@proxmox.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1788184465428 X-SPAM-LEVEL: Spam detection results: 0 AWL 1.663 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: OWFP5OENYRCQQPQCDUPLODDWB3TDA5LY X-Message-ID-Hash: OWFP5OENYRCQQPQCDUPLODDWB3TDA5LY X-MailFrom: d.herzig@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 CC: pve-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Hannes Laimer writes: > On 2026-08-31 13:01, Daniel Herzig wrote: >> At the given point we already have the subnet configuration by hand, >> so make sure to use it, instead of inherently falling back to >> re-reading '/etc/pve/sdn/.running-config' via the original >> 'get_subnets($vnet_id, 1)'. >> > > before this was `get_subnets($vnetid)`, so it did not take it from > running, now, it does since `$subnet_cfg` is the running config that has > the changes commited to it. this is a behaviour change, arguably this is > more correct here.. currently the node PUT `../network` endpoint also > calls this generate, that applies still pending configs, which is > probably a bug tbh > > so chnage is good, but the commit message should be updated Thanks for catching this. I was scanning callers that have `$subnet_cfg` available at calltime and admittedly missed that we're calling without `$running=1` here. So this one here is essentially superflouus, without `$running=1` we're reading from (already updated) '/etc/pve/sdn/subnets.cfg' file via `Subnets::config` (without `$running` given) anyway. ATM, I'm not sure what's better -- calling the original `get_subnets($vnetid)` without `$running` should yield the same as `$subnets` as calling `get_subnets_with_config($vnetid,$subnet_cfg)` at this point. For general noise reduction reasons I'd think about dropping this patch altogether, on the other hand, with it, we'd once less look into the file, which doesn't really seem to be necessary at this point. > >> Signed-off-by: Daniel Herzig >> --- >> src/PVE/Network/SDN/Dhcp.pm | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/PVE/Network/SDN/Dhcp.pm b/src/PVE/Network/SDN/Dhcp.pm >> index 65e40d4..1fdec89 100644 >> --- a/src/PVE/Network/SDN/Dhcp.pm >> +++ b/src/PVE/Network/SDN/Dhcp.pm >> @@ -94,7 +94,7 @@ sub regenerate_config { >> next if $vnet->{zone} ne $zoneid; >> >> my $config = []; >> - my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid); >> + my $subnets = PVE::Network::SDN::Vnets::get_subnets_with_config($vnetid, $subnet_cfg); >> >> foreach my $subnet_id (sort keys %{$subnets}) { >> my $subnet_config = $subnets->{$subnet_id}; >> -- >> 2.47.3 >> >> >> >>