* [pve-network 1/2] vnets: introduce get_subnets_with_config @ 2026-08-26 15:43 Daniel Herzig 2026-08-26 15:43 ` [pve-network 2/2] fix #7837: simpleplugin: make use of get_subnets_with_config Daniel Herzig 2026-08-27 13:58 ` [pve-network 1/2] vnets: introduce get_subnets_with_config Hannes Laimer 0 siblings, 2 replies; 6+ messages in thread From: Daniel Herzig @ 2026-08-26 15:43 UTC (permalink / raw) To: pve-devel Add a sub to return the subnets of a vnet with certain subnet configuration given. Signed-off-by: Daniel Herzig <d.herzig@proxmox.com> --- src/PVE/Network/SDN/Vnets.pm | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm index c327a4b..fdb2dd9 100644 --- a/src/PVE/Network/SDN/Vnets.pm +++ b/src/PVE/Network/SDN/Vnets.pm @@ -82,6 +82,19 @@ sub get_subnets { return $subnets; } +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); + $subnets->{$subnetid} = $subnet; + } + + return $subnets; +} + sub get_subnet_from_vnet_ip { my ($vnetid, $ip) = @_; -- 2.47.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [pve-network 2/2] fix #7837: simpleplugin: make use of get_subnets_with_config 2026-08-26 15:43 [pve-network 1/2] vnets: introduce get_subnets_with_config Daniel Herzig @ 2026-08-26 15:43 ` Daniel Herzig 2026-08-27 13:56 ` Hannes Laimer 2026-08-27 13:58 ` [pve-network 1/2] vnets: introduce get_subnets_with_config Hannes Laimer 1 sibling, 1 reply; 6+ messages in thread From: Daniel Herzig @ 2026-08-26 15:43 UTC (permalink / raw) To: pve-devel 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'. 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [pve-network 2/2] fix #7837: simpleplugin: make use of get_subnets_with_config 2026-08-26 15:43 ` [pve-network 2/2] fix #7837: simpleplugin: make use of get_subnets_with_config Daniel Herzig @ 2026-08-27 13:56 ` Hannes Laimer 2026-08-28 8:41 ` Daniel Herzig 0 siblings, 1 reply; 6+ messages in thread From: Hannes Laimer @ 2026-08-27 13:56 UTC (permalink / raw) To: Daniel Herzig, pve-devel Generally this LGTM! EVPN has the same problem[1], so fixing it there as well would make sense for this series. Aside from that, for more than one patch a `--cover-letter` would be nice, that simplifies answering. Also allows to add some extra context if needed. nit: the commit messages mostly describe what the code does, ideally they would include more of the _why_, something like: ``` 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. ``` for the commit that mentions the bugzilla issue. [1] https://git.proxmox.com/?p=pve-network.git;a=blob;f=src/PVE/Network/SDN/Zones/EvpnPlugin.pm;h=0e79707c374609f2505d842f99633e6311794f15;hb=refs/heads/master#l242 On 2026-08-26 17:43, 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'. > > 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; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-network 2/2] fix #7837: simpleplugin: make use of get_subnets_with_config 2026-08-27 13:56 ` Hannes Laimer @ 2026-08-28 8:41 ` Daniel Herzig 0 siblings, 0 replies; 6+ messages in thread From: Daniel Herzig @ 2026-08-28 8:41 UTC (permalink / raw) To: Hannes Laimer; +Cc: pve-devel Hannes Laimer <h.laimer@proxmox.com> writes: > Generally this LGTM! EVPN has the same problem[1], so fixing it > there as well would make sense for this series. > > Aside from that, for more than one patch a `--cover-letter` would be > nice, that simplifies answering. Also allows to add some extra context > if needed. > > nit: the commit messages mostly describe what the code does, ideally > they would include more of the _why_, something like: > > ``` > 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. > ``` > > for the commit that mentions the bugzilla issue. > Agreed, apparently I was too deep in it so I missed adding the cover letter. Absolutely makes sense, will be added in v2. I'll also look into the call from the Evpnplugin module, thanks for the headsup! > > [1] > https://git.proxmox.com/?p=pve-network.git;a=blob;f=src/PVE/Network/SDN/Zones/EvpnPlugin.pm;h=0e79707c374609f2505d842f99633e6311794f15;hb=refs/heads/master#l242 > > On 2026-08-26 17:43, 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'. >> >> 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; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-network 1/2] vnets: introduce get_subnets_with_config 2026-08-26 15:43 [pve-network 1/2] vnets: introduce get_subnets_with_config Daniel Herzig 2026-08-26 15:43 ` [pve-network 2/2] fix #7837: simpleplugin: make use of get_subnets_with_config Daniel Herzig @ 2026-08-27 13:58 ` Hannes Laimer 2026-08-28 8:35 ` Daniel Herzig 1 sibling, 1 reply; 6+ messages in thread From: Hannes Laimer @ 2026-08-27 13:58 UTC (permalink / raw) To: Daniel Herzig, pve-devel On 2026-08-26 17:43, Daniel Herzig wrote: > Add a sub to return the subnets of a vnet with certain subnet configuration given. > > Signed-off-by: Daniel Herzig <d.herzig@proxmox.com> > --- > src/PVE/Network/SDN/Vnets.pm | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm > index c327a4b..fdb2dd9 100644 > --- a/src/PVE/Network/SDN/Vnets.pm > +++ b/src/PVE/Network/SDN/Vnets.pm > @@ -82,6 +82,19 @@ sub get_subnets { > return $subnets; > } > > +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); > + $subnets->{$subnetid} = $subnet; > + } > + this loop is the same as in `get_subnets`, `get_subnets` could just also call this helper, so filtering is only done in one place > + return $subnets; > +} > + > sub get_subnet_from_vnet_ip { > my ($vnetid, $ip) = @_; > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-network 1/2] vnets: introduce get_subnets_with_config 2026-08-27 13:58 ` [pve-network 1/2] vnets: introduce get_subnets_with_config Hannes Laimer @ 2026-08-28 8:35 ` Daniel Herzig 0 siblings, 0 replies; 6+ messages in thread From: Daniel Herzig @ 2026-08-28 8:35 UTC (permalink / raw) To: Hannes Laimer; +Cc: pve-devel Thanks for looking into this. Hannes Laimer <h.laimer@proxmox.com> writes: > On 2026-08-26 17:43, Daniel Herzig wrote: >> Add a sub to return the subnets of a vnet with certain subnet configuration given. >> >> Signed-off-by: Daniel Herzig <d.herzig@proxmox.com> >> --- >> src/PVE/Network/SDN/Vnets.pm | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/src/PVE/Network/SDN/Vnets.pm b/src/PVE/Network/SDN/Vnets.pm >> index c327a4b..fdb2dd9 100644 >> --- a/src/PVE/Network/SDN/Vnets.pm >> +++ b/src/PVE/Network/SDN/Vnets.pm >> @@ -82,6 +82,19 @@ sub get_subnets { >> return $subnets; >> } >> >> +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); >> + $subnets->{$subnetid} = $subnet; >> + } >> + > > this loop is the same as in `get_subnets`, `get_subnets` could just also > call this helper, so filtering is only done in one place > Thanks for catching this -- I agree that doubling up the procedures here is not nice. Will send a v2 with calling it from the original `get_subnets`. >> + return $subnets; >> +} >> + >> sub get_subnet_from_vnet_ip { >> my ($vnetid, $ip) = @_; >> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-28 8:41 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-26 15:43 [pve-network 1/2] vnets: introduce get_subnets_with_config Daniel Herzig 2026-08-26 15:43 ` [pve-network 2/2] fix #7837: simpleplugin: make use of get_subnets_with_config Daniel Herzig 2026-08-27 13:56 ` Hannes Laimer 2026-08-28 8:41 ` Daniel Herzig 2026-08-27 13:58 ` [pve-network 1/2] vnets: introduce get_subnets_with_config Hannes Laimer 2026-08-28 8:35 ` Daniel Herzig
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.