[pve-network v2 0/4] make sure to use subnet config if available at point

[pve-network v2 0/4] make sure to use subnet config if available at point

[Daniel Herzig]

At certain places (SimplePlugin, EvpnPlugin) subnets were read from the
running config instead of the one passed in, so callers that generate without
committing first, like the dry-run, never saw pending subnet changes in the result.

This series changes this behaviour by introducing a function that makes use of
an existing subnet configuration if available at point. Additionally it adds
use of this function in Dhcp.pm, as we have the subnet configuration by hand
there as well.

To avoid code duplication, the function is also used from the original 'get_subnets'
for by callers, that do not have the subnet configuration by hand, here passing in the
configuration retrieved by the the original call to Subnets::config.

Changes since v1:
* use 'get_subnets_with_config' in original 'get_subnets' as well.
* use 'get_subnets_with_config' in EvpnPlugin and Dhcp in addition to SimplePlugin.

Daniel Herzig (4):
  vnets: introduce get_subnets_with_config
  fix #7837: simpleplugin: make use of get_subnets_with_config
  evpnplugin: make use of get_subnets_with_config
  dhcp: make use of get_subnets_with_config

 src/PVE/Network/SDN/Dhcp.pm               | 2 +-
 src/PVE/Network/SDN/Vnets.pm              | 8 +++++++-
 src/PVE/Network/SDN/Zones/EvpnPlugin.pm   | 2 +-
 src/PVE/Network/SDN/Zones/SimplePlugin.pm | 2 +-
 4 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.47.3

[pve-network v2 1/4] vnets: introduce get_subnets_with_config

[Daniel Herzig]

Add 'get_subnets_with_config' to return the subnets of a vnet with
certain subnet configuration and integrate into the current
'get_subnets'.

This allows to keep the original behaviour of 'get_subnets' --
retrieving subnet configurations (retreiving it either from the
running sdn-config, or from the 'subnets.cfg' config file) when it's
not available to the caller -- without doubling doubling up the code
to to loop through the subnet ids to handle the other case (subnet
configuration already present at the callsite).

Suggested-by: Gabriel Goller <g.goller@proxmox.com>
Suggested-by: Hannes Laimer <h.laimer@proxmox.com>
Signed-off-by: Daniel Herzig <d.herzig@proxmox.com>
---
 src/PVE/Network/SDN/Vnets.pm | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
index c327a4b..aa2e842 100644
--- a/src/PVE/Network/SDN/Vnets.pm
+++ b/src/PVE/Network/SDN/Vnets.pm
@@ -70,9 +70,15 @@ sub get_vnet {
 sub get_subnets {
     my ($vnetid, $running) = @_;
 
-    my $subnets = undef;
     my $subnets_cfg = PVE::Network::SDN::Subnets::config($running);
+    return get_subnets_with_config($vnetid, $subnets_cfg);
+
+}
 
+sub get_subnets_with_config {
+    my ($vnetid, $subnets_cfg) = @_;
+
+    my $subnets = undef;
     foreach my $subnetid (sort keys %{ $subnets_cfg->{ids} }) {
         my $subnet = PVE::Network::SDN::Subnets::sdn_subnets_config($subnets_cfg, $subnetid);
         next if !$subnet->{vnet} || ($vnetid && $subnet->{vnet} ne $vnetid);
-- 
2.47.3

[pve-network v2 2/4] fix #7837: simpleplugin: make use of get_subnets_with_config

[Daniel Herzig]

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

Suggested-by: Gabriel Goller <g.goller@proxmox.com>
Signed-off-by: Daniel Herzig <d.herzig@proxmox.com>
---
 src/PVE/Network/SDN/Zones/SimplePlugin.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/Network/SDN/Zones/SimplePlugin.pm b/src/PVE/Network/SDN/Zones/SimplePlugin.pm
index 347eee9..d922035 100644
--- a/src/PVE/Network/SDN/Zones/SimplePlugin.pm
+++ b/src/PVE/Network/SDN/Zones/SimplePlugin.pm
@@ -75,7 +75,7 @@ sub generate_sdn_config {
     my @iface_config = ();
 
     my $address = {};
-    my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid, 1);
+    my $subnets = PVE::Network::SDN::Vnets::get_subnets_with_config($vnetid, $subnet_cfg);
 
     my $ipv4 = undef;
     my $ipv6 = undef;
-- 
2.47.3

[pve-network v2 3/4] evpnplugin: make use of get_subnets_with_config

[Daniel Herzig]

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

Suggested-by: Hannes Laimer <h.laimer@proxmox.com>
Signed-off-by: Daniel Herzig <d.herzig@proxmox.com>
---
 src/PVE/Network/SDN/Zones/EvpnPlugin.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
index 0e79707..04fef78 100644
--- a/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
+++ b/src/PVE/Network/SDN/Zones/EvpnPlugin.pm
@@ -239,7 +239,7 @@ sub generate_sdn_config {
     my $ipv6 = undef;
     my $enable_forward_v4 = undef;
     my $enable_forward_v6 = undef;
-    my $subnets = PVE::Network::SDN::Vnets::get_subnets($vnetid, 1);
+    my $subnets = PVE::Network::SDN::Vnets::get_subnets_with_config($vnetid, $subnet_cfg);
     foreach my $subnetid (sort keys %{$subnets}) {
         my $subnet = $subnets->{$subnetid};
         my $cidr = $subnet->{cidr};
-- 
2.47.3

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

[Daniel Herzig]

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

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

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

[Hannes Laimer]

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

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

Re: [pve-network v2 1/4] vnets: introduce get_subnets_with_config

[Hannes Laimer]

On 2026-08-31 13:01, Daniel Herzig wrote:
> Add 'get_subnets_with_config' to return the subnets of a vnet with
> certain subnet configuration and integrate into the current
> 'get_subnets'.
> 
> This allows to keep the original behaviour of 'get_subnets' --
> retrieving subnet configurations (retreiving it either from the
> running sdn-config, or from the 'subnets.cfg' config file) when it's
> not available to the caller -- without doubling doubling up the code
> to to loop through the subnet ids to handle the other case (subnet
> configuration already present at the callsite).
> 
> Suggested-by: Gabriel Goller <g.goller@proxmox.com>
> Suggested-by: Hannes Laimer <h.laimer@proxmox.com>
> Signed-off-by: Daniel Herzig <d.herzig@proxmox.com>
> ---
>  src/PVE/Network/SDN/Vnets.pm | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
> index c327a4b..aa2e842 100644
> --- a/src/PVE/Network/SDN/Vnets.pm
> +++ b/src/PVE/Network/SDN/Vnets.pm
> @@ -70,9 +70,15 @@ sub get_vnet {
>  sub get_subnets {
>      my ($vnetid, $running) = @_;
>  
> -    my $subnets = undef;
>      my $subnets_cfg = PVE::Network::SDN::Subnets::config($running);
> +    return get_subnets_with_config($vnetid, $subnets_cfg);
> +
> +}
>  
> +sub get_subnets_with_config {
> +    my ($vnetid, $subnets_cfg) = @_;
> +

we should probably `die` here if the passed cfg is undef, for the
deletion `ids` will be empty, and an empty conf will be generated,
that's fine. but we dont want undef here.. on that note, the guard in
Dhcp.pm should maybe be updated

> +    my $subnets = undef;
>      foreach my $subnetid (sort keys %{ $subnets_cfg->{ids} }) {
>          my $subnet = PVE::Network::SDN::Subnets::sdn_subnets_config($subnets_cfg, $subnetid);
>          next if !$subnet->{vnet} || ($vnetid && $subnet->{vnet} ne $vnetid);
> -- 
> 2.47.3
> 
> 

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

[Daniel Herzig]

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.

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

Re: [pve-network v2 1/4] vnets: introduce get_subnets_with_config

[Daniel Herzig]

Hannes Laimer <h.laimer@proxmox.com> writes:

> On 2026-08-31 13:01, Daniel Herzig wrote:
>> Add 'get_subnets_with_config' to return the subnets of a vnet with
>> certain subnet configuration and integrate into the current
>> 'get_subnets'.
>> 
>> This allows to keep the original behaviour of 'get_subnets' --
>> retrieving subnet configurations (retreiving it either from the
>> running sdn-config, or from the 'subnets.cfg' config file) when it's
>> not available to the caller -- without doubling doubling up the code
>> to to loop through the subnet ids to handle the other case (subnet
>> configuration already present at the callsite).
>> 
>> Suggested-by: Gabriel Goller <g.goller@proxmox.com>
>> Suggested-by: Hannes Laimer <h.laimer@proxmox.com>
>> Signed-off-by: Daniel Herzig <d.herzig@proxmox.com>
>> ---
>>  src/PVE/Network/SDN/Vnets.pm | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
>> index c327a4b..aa2e842 100644
>> --- a/src/PVE/Network/SDN/Vnets.pm
>> +++ b/src/PVE/Network/SDN/Vnets.pm
>> @@ -70,9 +70,15 @@ sub get_vnet {
>>  sub get_subnets {
>>      my ($vnetid, $running) = @_;
>>  
>> -    my $subnets = undef;
>>      my $subnets_cfg = PVE::Network::SDN::Subnets::config($running);
>> +    return get_subnets_with_config($vnetid, $subnets_cfg);
>> +
>> +}
>>  
>> +sub get_subnets_with_config {
>> +    my ($vnetid, $subnets_cfg) = @_;
>> +
>
> we should probably `die` here if the passed cfg is undef, for the
> deletion `ids` will be empty, and an empty conf will be generated,
> that's fine. but we dont want undef here..

I guess we could possibly also fall back to re-reading
`/etc/pve/sdn/subnets.cfg` in case anything got lost in transmission, or
in a disaster-situation, where the file got lost alltogether, fall back
to get the values from `.running-config` and issue a warning instead of
`die`-ing here. I need to wrap my head around it, but I'm not sure if
this is a good place to `die`.

> on that note, the guard in
> Dhcp.pm should maybe be updated
>

I've sent a separate answer regarding the patch in Dhcp.pm -- in a
nutshell, we could possibly just skip that one (also, because it's not
really related to the dry-run topic).


>> +    my $subnets = undef;
>>      foreach my $subnetid (sort keys %{ $subnets_cfg->{ids} }) {
>>          my $subnet = PVE::Network::SDN::Subnets::sdn_subnets_config($subnets_cfg, $subnetid);
>>          next if !$subnet->{vnet} || ($vnetid && $subnet->{vnet} ne $vnetid);
>> -- 
>> 2.47.3
>> 
>> 

Re: [pve-network v2 1/4] vnets: introduce get_subnets_with_config

[Hannes Laimer]

On 2026-08-31 16:42, Daniel Herzig wrote:
> 
> 
> Hannes Laimer <h.laimer@proxmox.com> writes:
> 
> > On 2026-08-31 13:01, Daniel Herzig wrote:
> >> Add 'get_subnets_with_config' to return the subnets of a vnet with
> >> certain subnet configuration and integrate into the current
> >> 'get_subnets'.
> >> 
> >> This allows to keep the original behaviour of 'get_subnets' --
> >> retrieving subnet configurations (retreiving it either from the
> >> running sdn-config, or from the 'subnets.cfg' config file) when it's
> >> not available to the caller -- without doubling doubling up the code
> >> to to loop through the subnet ids to handle the other case (subnet
> >> configuration already present at the callsite).
> >> 
> >> Suggested-by: Gabriel Goller <g.goller@proxmox.com>
> >> Suggested-by: Hannes Laimer <h.laimer@proxmox.com>
> >> Signed-off-by: Daniel Herzig <d.herzig@proxmox.com>
> >> ---
> >>  src/PVE/Network/SDN/Vnets.pm | 8 +++++++-
> >>  1 file changed, 7 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm
> >> index c327a4b..aa2e842 100644
> >> --- a/src/PVE/Network/SDN/Vnets.pm
> >> +++ b/src/PVE/Network/SDN/Vnets.pm
> >> @@ -70,9 +70,15 @@ sub get_vnet {
> >>  sub get_subnets {
> >>      my ($vnetid, $running) = @_;
> >>  
> >> -    my $subnets = undef;
> >>      my $subnets_cfg = PVE::Network::SDN::Subnets::config($running);
> >> +    return get_subnets_with_config($vnetid, $subnets_cfg);
> >> +
> >> +}
> >>  
> >> +sub get_subnets_with_config {
> >> +    my ($vnetid, $subnets_cfg) = @_;
> >> +
> >
> > we should probably `die` here if the passed cfg is undef, for the
> > deletion `ids` will be empty, and an empty conf will be generated,
> > that's fine. but we dont want undef here..
> 
> I guess we could possibly also fall back to re-reading
> `/etc/pve/sdn/subnets.cfg` in case anything got lost in transmission, or
> in a disaster-situation, where the file got lost alltogether, fall back
> to get the values from `.running-config` and issue a warning instead of
> `die`-ing here. I need to wrap my head around it, but I'm not sure if
> this is a good place to `die`.
> 

not really, at least i don't think we should. the premis of this sub is,
'give me a config, and ill get you its subnets', the assertion here is
'you give me a config'. this should not go and fetch its own..

> > on that note, the guard in
> > Dhcp.pm should maybe be updated
> >
> 
> I've sent a separate answer regarding the patch in Dhcp.pm -- in a
> nutshell, we could possibly just skip that one (also, because it's not
> really related to the dry-run topic).
> 
> 
> >> +    my $subnets = undef;
> >>      foreach my $subnetid (sort keys %{ $subnets_cfg->{ids} }) {
> >>          my $subnet = PVE::Network::SDN::Subnets::sdn_subnets_config($subnets_cfg, $subnetid);
> >>          next if !$subnet->{vnet} || ($vnetid && $subnet->{vnet} ne $vnetid);
> >> -- 
> >> 2.47.3
> >> 
> >> 

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

[Hannes Laimer]

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

Re: [pve-network v2 0/4] make sure to use subnet config if available at point

[Gabriel Goller]

For the record, I’ve been working on a patch that reads the config only once,
when starting an apply or dry-run, and then passes it through the entire SDN config application
process.

The patch adds around 300 lines though and touches a lot of critical code, so it will
take some time to polish.

In the meantime, IMO we can merge this targeted subnet fix.