all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH manager/network v2 0/2] Improve FRR reloading logic
@ 2025-08-05  8:35 Stefan Hanreich
  2025-08-05  8:35 ` [pve-devel] [PATCH pve-manager v2 1/1] network: improve " Stefan Hanreich
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Stefan Hanreich @ 2025-08-05  8:35 UTC (permalink / raw)
  To: pve-devel

Follow-up to the stop gap fix in [1].

This should handle all cases that we need to consider:

* Do not overwrite existing FRR configurations, unless we need to generate our
own FRR configuration.

* Do not trigger a FRR enable when reloading the host configuration, even though
there is no FRR configuration.

* Overwrite the FRR configuration with an empty configuration if all SDN
entities using FRR got deleted.

* Regenerate the FRR configuration when the host network configuration changes,
since this might affect the generated FRR configuration.

Changes from v1:
* fix typo and improve wording of comments


[1] https://git.proxmox.com/?p=pve-manager.git;a=commit;h=6c5295a1cdb6ec872ddf997cae030817dffff106

pve-manager:

Stefan Hanreich (1):
  network: improve reloading logic

 PVE/API2/Network.pm | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)


pve-network:

Stefan Hanreich (1):
  sdn: api: always send regenerate-frr parameter

 src/PVE/API2/Network/SDN.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


Summary over all repositories:
  2 files changed, 10 insertions(+), 3 deletions(-)

-- 
Generated by git-murpp 0.8.0

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [PATCH pve-manager v2 1/1] network: improve reloading logic
  2025-08-05  8:35 [pve-devel] [PATCH manager/network v2 0/2] Improve FRR reloading logic Stefan Hanreich
@ 2025-08-05  8:35 ` Stefan Hanreich
  2025-08-05  8:35 ` [pve-devel] [PATCH pve-network v2 1/1] sdn: api: always send regenerate-frr parameter Stefan Hanreich
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Stefan Hanreich @ 2025-08-05  8:35 UTC (permalink / raw)
  To: pve-devel

The old behavior was *always* reloading the network configuration,
which worked as long as FRR was not pre-installed, but the change in
db03d261 coupled with the fact that we now ship with FRR caused FRR to
be enabled on applying the network configuration via the Web UI.

The stop gap fix in e1b9466d solved this behavior, but had the issue
that we need to possibly regenerate the FRR configuration if the host
configuration changes, since some controllers generate their
configuration based on the host network configuration.

pve-network now always sends the regenerate-frr parameter, so we can
discern whether a request came from SDN or is a manual request that is
requesting a specific behavior. With this information the reloading
logic can be improved as follows:

* Honor the parameter if it is set
* reload only if there are any FRR entities in the SDN configuration

This should handle all cases that we need to consider:

* Do not overwrite existing FRR configurations, unless we need to
  generate our own FRR configuration.

* Do not trigger a FRR enable when reloading the host configuration,
  even though there is no FRR configuration.

* Overwrite the FRR configuration with an empty configuration if all
  SDN entities using FRR got deleted.

* Regenerate the FRR configuration when the host network configuration
  changes, since this might affect the generated FRR configuration.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 PVE/API2/Network.pm | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm
index b423e19b5..7fb7667ee 100644
--- a/PVE/API2/Network.pm
+++ b/PVE/API2/Network.pm
@@ -932,7 +932,14 @@ __PACKAGE__->register_method({
             };
             PVE::Tools::run_command(['ifreload', '-a'], errfunc => $err);
 
-            if ($have_sdn && $regenerate_frr) {
+            if (defined($regenerate_frr)) {
+                # SDN (or a manual invocation) is requesting a
+                # specific behavior, so we abide by the parameter set.
+                PVE::Network::SDN::generate_frr_config(1) if $regenerate_frr;
+            } elsif (PVE::Network::SDN::running_config_has_frr()) {
+                # Reload is coming from the Web UI (or a manual
+                # invocation requesting no specific behavior), so we
+                # reload if there are FRR entities in the SDN config.
                 PVE::Network::SDN::generate_frr_config(1);
             }
         };
-- 
2.47.2


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] [PATCH pve-network v2 1/1] sdn: api: always send regenerate-frr parameter
  2025-08-05  8:35 [pve-devel] [PATCH manager/network v2 0/2] Improve FRR reloading logic Stefan Hanreich
  2025-08-05  8:35 ` [pve-devel] [PATCH pve-manager v2 1/1] network: improve " Stefan Hanreich
@ 2025-08-05  8:35 ` Stefan Hanreich
  2025-08-05  8:36 ` [pve-devel] superseded: [PATCH manager/network v2 0/2] Improve FRR reloading logic Stefan Hanreich
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Stefan Hanreich @ 2025-08-05  8:35 UTC (permalink / raw)
  To: pve-devel

This makes it possible to check for the definedness of the parameter
and adapt the behavior of the reloading endpoint depending on that
information. See the commit in pve-manager for more information.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/API2/Network/SDN.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm
index 16a8e2c..af00b1a 100644
--- a/src/PVE/API2/Network/SDN.pm
+++ b/src/PVE/API2/Network/SDN.pm
@@ -93,7 +93,7 @@ my $create_reload_network_worker = sub {
     my ($nodename, $regenerate_frr) = @_;
 
     my @command = ('pvesh', 'set', "/nodes/$nodename/network");
-    push(@command, '--regenerate-frr') if $regenerate_frr;
+    push(@command, '--regenerate-frr', $regenerate_frr);
 
     # FIXME: how to proxy to final node ?
     my $upid;
@@ -299,7 +299,7 @@ __PACKAGE__->register_method({
             $lock_token,
         );
 
-        my $regenerate_frr = $previous_config_has_frr || $new_config_has_frr;
+        my $regenerate_frr = ($previous_config_has_frr || $new_config_has_frr) ? 1 : 0;
 
         my $code = sub {
             $rpcenv->{type} = 'priv'; # to start tasks in background
-- 
2.47.2


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pve-devel] superseded: [PATCH manager/network v2 0/2] Improve FRR reloading logic
  2025-08-05  8:35 [pve-devel] [PATCH manager/network v2 0/2] Improve FRR reloading logic Stefan Hanreich
  2025-08-05  8:35 ` [pve-devel] [PATCH pve-manager v2 1/1] network: improve " Stefan Hanreich
  2025-08-05  8:35 ` [pve-devel] [PATCH pve-network v2 1/1] sdn: api: always send regenerate-frr parameter Stefan Hanreich
@ 2025-08-05  8:36 ` Stefan Hanreich
  2025-08-05  8:49   ` Stefan Hanreich
  2025-08-05  9:22 ` [pve-devel] " Gabriel Goller
  2025-08-05  9:43 ` Hannes Duerr
  4 siblings, 1 reply; 7+ messages in thread
From: Stefan Hanreich @ 2025-08-05  8:36 UTC (permalink / raw)
  To: pve-devel

https://lore.proxmox.com/pve-devel/20250805083504.55378-1-s.hanreich@proxmox.com/T/#t

On 8/5/25 10:34 AM, Stefan Hanreich wrote:
> Follow-up to the stop gap fix in [1].
> 
> This should handle all cases that we need to consider:
> 
> * Do not overwrite existing FRR configurations, unless we need to generate our
> own FRR configuration.
> 
> * Do not trigger a FRR enable when reloading the host configuration, even though
> there is no FRR configuration.
> 
> * Overwrite the FRR configuration with an empty configuration if all SDN
> entities using FRR got deleted.
> 
> * Regenerate the FRR configuration when the host network configuration changes,
> since this might affect the generated FRR configuration.
> 
> Changes from v1:
> * fix typo and improve wording of comments
> 
> 
> [1] https://git.proxmox.com/?p=pve-manager.git;a=commit;h=6c5295a1cdb6ec872ddf997cae030817dffff106
> 
> pve-manager:
> 
> Stefan Hanreich (1):
>   network: improve reloading logic
> 
>  PVE/API2/Network.pm | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> 
> pve-network:
> 
> Stefan Hanreich (1):
>   sdn: api: always send regenerate-frr parameter
> 
>  src/PVE/API2/Network/SDN.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> 
> Summary over all repositories:
>   2 files changed, 10 insertions(+), 3 deletions(-)
> 



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pve-devel] superseded: [PATCH manager/network v2 0/2] Improve FRR reloading logic
  2025-08-05  8:36 ` [pve-devel] superseded: [PATCH manager/network v2 0/2] Improve FRR reloading logic Stefan Hanreich
@ 2025-08-05  8:49   ` Stefan Hanreich
  0 siblings, 0 replies; 7+ messages in thread
From: Stefan Hanreich @ 2025-08-05  8:49 UTC (permalink / raw)
  To: pve-devel

On 8/5/25 10:35 AM, Stefan Hanreich wrote:
> https://lore.proxmox.com/pve-devel/20250805083504.55378-1-s.hanreich@proxmox.com/T/#t

misclicked on the v2 when sending the superseded, sorry -.-
this is the correct one


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pve-devel] [PATCH manager/network v2 0/2] Improve FRR reloading logic
  2025-08-05  8:35 [pve-devel] [PATCH manager/network v2 0/2] Improve FRR reloading logic Stefan Hanreich
                   ` (2 preceding siblings ...)
  2025-08-05  8:36 ` [pve-devel] superseded: [PATCH manager/network v2 0/2] Improve FRR reloading logic Stefan Hanreich
@ 2025-08-05  9:22 ` Gabriel Goller
  2025-08-05  9:43 ` Hannes Duerr
  4 siblings, 0 replies; 7+ messages in thread
From: Gabriel Goller @ 2025-08-05  9:22 UTC (permalink / raw)
  To: Stefan Hanreich; +Cc: pve-devel

Consider:

Tested-by: Gabriel Goller <g.goller@proxmox.com>
Reviewed-by: Gabriel Goller <g.goller@proxmox.com>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pve-devel] [PATCH manager/network v2 0/2] Improve FRR reloading logic
  2025-08-05  8:35 [pve-devel] [PATCH manager/network v2 0/2] Improve FRR reloading logic Stefan Hanreich
                   ` (3 preceding siblings ...)
  2025-08-05  9:22 ` [pve-devel] " Gabriel Goller
@ 2025-08-05  9:43 ` Hannes Duerr
  4 siblings, 0 replies; 7+ messages in thread
From: Hannes Duerr @ 2025-08-05  9:43 UTC (permalink / raw)
  To: Proxmox VE development discussion; +Cc: pve-devel

Tested it by upgrading existing ceph full mesh cluster (without
fabrics) and reloading the network after succesful upgrade

Tested-by: Hannes Duerr <h.duerr@proxmox.com>

On Tue Aug 5, 2025 at 10:35 AM CEST, Stefan Hanreich wrote:
> Follow-up to the stop gap fix in [1].
>
> This should handle all cases that we need to consider:
>
> * Do not overwrite existing FRR configurations, unless we need to generate our
> own FRR configuration.
>
> * Do not trigger a FRR enable when reloading the host configuration, even though
> there is no FRR configuration.
>
> * Overwrite the FRR configuration with an empty configuration if all SDN
> entities using FRR got deleted.
>
> * Regenerate the FRR configuration when the host network configuration changes,
> since this might affect the generated FRR configuration.
>
> Changes from v1:
> * fix typo and improve wording of comments
>
>
> [1] https://git.proxmox.com/?p=pve-manager.git;a=commit;h=6c5295a1cdb6ec872ddf997cae030817dffff106
>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-08-05  9:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-05  8:35 [pve-devel] [PATCH manager/network v2 0/2] Improve FRR reloading logic Stefan Hanreich
2025-08-05  8:35 ` [pve-devel] [PATCH pve-manager v2 1/1] network: improve " Stefan Hanreich
2025-08-05  8:35 ` [pve-devel] [PATCH pve-network v2 1/1] sdn: api: always send regenerate-frr parameter Stefan Hanreich
2025-08-05  8:36 ` [pve-devel] superseded: [PATCH manager/network v2 0/2] Improve FRR reloading logic Stefan Hanreich
2025-08-05  8:49   ` Stefan Hanreich
2025-08-05  9:22 ` [pve-devel] " Gabriel Goller
2025-08-05  9:43 ` Hannes Duerr

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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal