public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH network] sdn: write frr config if the frr.conf.local file exists
@ 2025-09-19 13:24 Gabriel Goller
  2025-10-01 10:06 ` Stefan Hanreich
  0 siblings, 1 reply; 2+ messages in thread
From: Gabriel Goller @ 2025-09-19 13:24 UTC (permalink / raw)
  To: pve-devel

Currently we check if there is a fabric or a controller existing to
decide if we write the frr.conf file. Also add a check if the
frr.conf.local exists. This way it's possible to have a custom config in
the frr.conf.local file without any SDN objects configured and still
be able to apply the config.
By not checking the content we can also clear the frr config by clearing
the frr.conf.local file.

Reported-by: Hannes Dürr <h.duerr@proxmox.com>
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 src/PVE/Network/SDN.pm     |  3 ++-
 src/PVE/Network/SDN/Frr.pm | 20 ++++++++++++++++++--
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
index f2ecd4ab7318..e867d1ab886e 100644
--- a/src/PVE/Network/SDN.pm
+++ b/src/PVE/Network/SDN.pm
@@ -123,8 +123,9 @@ sub running_config_has_frr {
     # both can be empty if the SDN configuration was never applied
     my $controllers = $running_config->{controllers}->{ids} // {};
     my $fabrics = $running_config->{fabrics}->{ids} // {};
+    my $local_frr_config = PVE::Network::SDN::Frr::local_frr_config_exists();
 
-    return %$controllers || %$fabrics;
+    return %$controllers || %$fabrics || $local_frr_config;
 }
 
 sub pending_config {
diff --git a/src/PVE/Network/SDN/Frr.pm b/src/PVE/Network/SDN/Frr.pm
index b607b32c248d..e77eba182177 100644
--- a/src/PVE/Network/SDN/Frr.pm
+++ b/src/PVE/Network/SDN/Frr.pm
@@ -32,6 +32,22 @@ string that is a FRR configuration line.
 use PVE::RESTEnvironment qw(log_warn);
 use PVE::Tools qw(file_get_contents file_set_contents run_command);
 
+my $FRR_CONF_LOCAL_FILE = "/etc/frr/frr.conf.local";
+
+=head3 local_frr_config_exists
+
+Returns true if the `/etc/frr/frr.conf.local` file exists, otherwise false.
+
+=cut
+
+sub local_frr_config_exists {
+    if (-e $FRR_CONF_LOCAL_FILE) {
+        return 1;
+    } else {
+        return 0;
+    }
+}
+
 =head3 read_local_frr_config
 
 Returns the contents of `/etc/frr/frr.conf.local` as a string if it exists, otherwise undef.
@@ -39,8 +55,8 @@ Returns the contents of `/etc/frr/frr.conf.local` as a string if it exists, othe
 =cut
 
 sub read_local_frr_config {
-    if (-e "/etc/frr/frr.conf.local") {
-        return file_get_contents("/etc/frr/frr.conf.local");
+    if (-e $FRR_CONF_LOCAL_FILE) {
+        return file_get_contents($FRR_CONF_LOCAL_FILE);
     }
 }
 
-- 
2.47.3



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

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

* Re: [pve-devel] [PATCH network] sdn: write frr config if the frr.conf.local file exists
  2025-09-19 13:24 [pve-devel] [PATCH network] sdn: write frr config if the frr.conf.local file exists Gabriel Goller
@ 2025-10-01 10:06 ` Stefan Hanreich
  0 siblings, 0 replies; 2+ messages in thread
From: Stefan Hanreich @ 2025-10-01 10:06 UTC (permalink / raw)
  To: Proxmox VE development discussion, Gabriel Goller

Tested this on my machine, works as advertised. The general idea of
regenerating the FRR config anytime we reload SDN also makes sense to
me, since it allows users to have a FRR config and potentially create
controllers / fabrics at a later time and not have to worry about their
frr.conf getting overwritten accidentally.

Reviewed-by: Stefan Hanreich <s.hanreich@proxmox.com>
Tested-by: Stefan Hanreich <s.hanreich@proxmox.com>


On 9/19/25 3:25 PM, Gabriel Goller wrote:
> Currently we check if there is a fabric or a controller existing to
> decide if we write the frr.conf file. Also add a check if the
> frr.conf.local exists. This way it's possible to have a custom config in
> the frr.conf.local file without any SDN objects configured and still
> be able to apply the config.
> By not checking the content we can also clear the frr config by clearing
> the frr.conf.local file.
> 
> Reported-by: Hannes Dürr <h.duerr@proxmox.com>
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  src/PVE/Network/SDN.pm     |  3 ++-
>  src/PVE/Network/SDN/Frr.pm | 20 ++++++++++++++++++--
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
> index f2ecd4ab7318..e867d1ab886e 100644
> --- a/src/PVE/Network/SDN.pm
> +++ b/src/PVE/Network/SDN.pm
> @@ -123,8 +123,9 @@ sub running_config_has_frr {
>      # both can be empty if the SDN configuration was never applied
>      my $controllers = $running_config->{controllers}->{ids} // {};
>      my $fabrics = $running_config->{fabrics}->{ids} // {};
> +    my $local_frr_config = PVE::Network::SDN::Frr::local_frr_config_exists();
>  
> -    return %$controllers || %$fabrics;
> +    return %$controllers || %$fabrics || $local_frr_config;
>  }
>  
>  sub pending_config {
> diff --git a/src/PVE/Network/SDN/Frr.pm b/src/PVE/Network/SDN/Frr.pm
> index b607b32c248d..e77eba182177 100644
> --- a/src/PVE/Network/SDN/Frr.pm
> +++ b/src/PVE/Network/SDN/Frr.pm
> @@ -32,6 +32,22 @@ string that is a FRR configuration line.
>  use PVE::RESTEnvironment qw(log_warn);
>  use PVE::Tools qw(file_get_contents file_set_contents run_command);
>  
> +my $FRR_CONF_LOCAL_FILE = "/etc/frr/frr.conf.local";
> +
> +=head3 local_frr_config_exists
> +
> +Returns true if the `/etc/frr/frr.conf.local` file exists, otherwise false.
> +
> +=cut
> +
> +sub local_frr_config_exists {
> +    if (-e $FRR_CONF_LOCAL_FILE) {
> +        return 1;
> +    } else {
> +        return 0;
> +    }
> +}
> +
>  =head3 read_local_frr_config
>  
>  Returns the contents of `/etc/frr/frr.conf.local` as a string if it exists, otherwise undef.
> @@ -39,8 +55,8 @@ Returns the contents of `/etc/frr/frr.conf.local` as a string if it exists, othe
>  =cut
>  
>  sub read_local_frr_config {
> -    if (-e "/etc/frr/frr.conf.local") {
> -        return file_get_contents("/etc/frr/frr.conf.local");
> +    if (-e $FRR_CONF_LOCAL_FILE) {

small nit: potentially use the helper here, if we define one anyway?

+        return file_get_contents($FRR_CONF_LOCAL_FILE);
>      }
>  }
>  



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

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

end of thread, other threads:[~2025-10-01 10:06 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-19 13:24 [pve-devel] [PATCH network] sdn: write frr config if the frr.conf.local file exists Gabriel Goller
2025-10-01 10:06 ` Stefan Hanreich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal