[pve-devel] [PATCH manager/network v2 0/2] do not apply FRR configuration when reloading host network configuration

[pve-devel] [PATCH manager/network v2 0/2] do not apply FRR configuration when reloading host network configuration

[Stefan Hanreich]

The FRR apply function was called when omitting the skip_frr parameter, which is
the case when saving the host network configuration via the Web UI, leading to
an enabled FRR daemon after saving the network configuration.

In a follow-up we should also consider checking if the previous configuration
contained FRR entities, but the new one doesn't and disable the FRR daemon in
this case.

Changes from v1 to v2:
* rebase on top of current master

pve-manager:

Stefan Hanreich (1):
  api: network: default to not regenerating the frr configuration

 PVE/API2/Network.pm | 7 ++++---
 bin/pve-sdn-commit  | 4 ++--
 2 files changed, 6 insertions(+), 5 deletions(-)


pve-network:

Stefan Hanreich (1):
  sdn: api: rename parameter from skip-frr to regenerate-frr

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


Summary over all repositories:
  3 files changed, 10 insertions(+), 11 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

[pve-devel] [PATCH pve-manager v2 1/1] api: network: default to not regenerating the frr configuration

[Stefan Hanreich]

Default to not regenerating the FRR configuration, unless explicitly
requested. Otherwise applying the host network configuration would
reload and enable the FRR service. Invert the boolean from skip to
regenerate, since the logic is less convoluted this way.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 PVE/API2/Network.pm | 7 ++++---
 bin/pve-sdn-commit  | 4 ++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm
index 93f441bab..b423e19b5 100644
--- a/PVE/API2/Network.pm
+++ b/PVE/API2/Network.pm
@@ -891,10 +891,11 @@ __PACKAGE__->register_method({
         additionalProperties => 0,
         properties => {
             node => get_standard_option('pve-node'),
-            skip_frr => {
+            'regenerate-frr' => {
                 type => 'boolean',
                 description => 'Whether FRR config generation should get skipped or not.',
                 optional => 1,
+                default => 0,
             },
         },
     },
@@ -910,7 +911,7 @@ __PACKAGE__->register_method({
         my $current_config_file = "/etc/network/interfaces";
         my $new_config_file = "/etc/network/interfaces.new";
 
-        my $skip_frr = extract_param($param, 'skip_frr');
+        my $regenerate_frr = extract_param($param, 'regenerate-frr');
 
         assert_ifupdown2_installed();
 
@@ -931,7 +932,7 @@ __PACKAGE__->register_method({
             };
             PVE::Tools::run_command(['ifreload', '-a'], errfunc => $err);
 
-            if ($have_sdn && !$skip_frr) {
+            if ($have_sdn && $regenerate_frr) {
                 PVE::Network::SDN::generate_frr_config(1);
             }
         };
diff --git a/bin/pve-sdn-commit b/bin/pve-sdn-commit
index 492963baf..6eeba301c 100644
--- a/bin/pve-sdn-commit
+++ b/bin/pve-sdn-commit
@@ -83,7 +83,7 @@ my $previous_config_has_frr = PVE::Network::SDN::running_config_has_frr();
 PVE::Network::SDN::commit_config();
 
 my $new_config_has_frr = PVE::Network::SDN::running_config_has_frr();
-my $skip_frr = !($previous_config_has_frr || $new_config_has_frr);
+my $regenerate_frr = ($previous_config_has_frr || $new_config_has_frr);
 
 PVE::Network::SDN::generate_etc_network_config();
 PVE::Network::SDN::generate_dhcp_config();
@@ -97,6 +97,6 @@ my $err = sub {
 
 PVE::Tools::run_command(['ifreload', '-a'], errfunc => $err);
 
-PVE::Network::SDN::generate_frr_config(1) if !$skip_frr;
+PVE::Network::SDN::generate_frr_config(1) if $regenerate_frr;
 
 exit 0;
-- 
2.47.2


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

[pve-devel] [PATCH pve-network v2 1/1] sdn: api: rename parameter from skip-frr to regenerate-frr

[Stefan Hanreich]

Parameter has moved from skip_frr to regenerate-frr in pve-manager.
Also invert the check for generating the boolean, since the logic is
inverted now.

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

diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm
index 7c91902..16a8e2c 100644
--- a/src/PVE/API2/Network/SDN.pm
+++ b/src/PVE/API2/Network/SDN.pm
@@ -90,12 +90,10 @@ __PACKAGE__->register_method({
 });
 
 my $create_reload_network_worker = sub {
-    my ($nodename, $skip_frr) = @_;
+    my ($nodename, $regenerate_frr) = @_;
 
     my @command = ('pvesh', 'set', "/nodes/$nodename/network");
-    if ($skip_frr) {
-        push(@command, '--skip_frr');
-    }
+    push(@command, '--regenerate-frr') if $regenerate_frr;
 
     # FIXME: how to proxy to final node ?
     my $upid;
@@ -301,14 +299,14 @@ __PACKAGE__->register_method({
             $lock_token,
         );
 
-        my $skip_frr = !($previous_config_has_frr || $new_config_has_frr);
+        my $regenerate_frr = $previous_config_has_frr || $new_config_has_frr;
 
         my $code = sub {
             $rpcenv->{type} = 'priv'; # to start tasks in background
             PVE::Cluster::check_cfs_quorum();
             my $nodelist = PVE::Cluster::get_nodelist();
             for my $node (@$nodelist) {
-                my $pid = eval { $create_reload_network_worker->($node, $skip_frr) };
+                my $pid = eval { $create_reload_network_worker->($node, $regenerate_frr) };
                 warn $@ if $@;
             }
 
-- 
2.47.2


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

Re: [pve-devel] [PATCH manager/network v2 0/2] do not apply FRR configuration when reloading host network configuration

[Friedrich Weber]

On 04/08/2025 16:01, Stefan Hanreich wrote:
> The FRR apply function was called when omitting the skip_frr parameter, which is
> the case when saving the host network configuration via the Web UI, leading to
> an enabled FRR daemon after saving the network configuration.
> 
> In a follow-up we should also consider checking if the previous configuration
> contained FRR entities, but the new one doesn't and disable the FRR daemon in
> this case.

Thanks for the fix! I'm not using fabrics, and after applying these
patches, re-applying the host network configuration doesn't enable+start
frr anymore:

Tested-by: Friedrich Weber <f.weber@proxmox.com>


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

[pve-devel] applied-series: [PATCH manager/network v2 0/2] do not apply FRR configuration when reloading host network configuration

[Thomas Lamprecht]

Am 04.08.25 um 16:01 schrieb Stefan Hanreich:
> The FRR apply function was called when omitting the skip_frr parameter, which is
> the case when saving the host network configuration via the Web UI, leading to
> an enabled FRR daemon after saving the network configuration.
> 
> In a follow-up we should also consider checking if the previous configuration
> contained FRR entities, but the new one doesn't and disable the FRR daemon in
> this case.
> 
> Changes from v1 to v2:
> * rebase on top of current master
> 
> pve-manager:
> 
> Stefan Hanreich (1):
>   api: network: default to not regenerating the frr configuration
> 
>  PVE/API2/Network.pm | 7 ++++---
>  bin/pve-sdn-commit  | 4 ++--
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> 
> pve-network:
> 
> Stefan Hanreich (1):
>   sdn: api: rename parameter from skip-frr to regenerate-frr
> 
>  src/PVE/API2/Network/SDN.pm | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> 
> Summary over all repositories:
>   3 files changed, 10 insertions(+), 11 deletions(-)
> 


applied, thanks!


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