Re: [pve-network v2 4/4] dhcp: make use of get_subnets_with_config

[Hannes Laimer <h.laimer@proxmox.com>]

On 2026-08-31 15:54, Daniel Herzig wrote:
> Hannes Laimer <h.laimer@proxmox.com> 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.

this should be the running conf, not the config file, we don't want to
skip sdn apply step ..

> 
> 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.
>

.. doing this separate sounds fine, something along the lines of 'avoid
applying pending dhcp config without sdn commit'

> 
> >
> >> Signed-off-by: Daniel Herzig <d.herzig@proxmox.com>
> >> ---
> >>  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
> >> 
> >> 
> >> 
> >>