public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Stefan Hanreich <s.hanreich@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Gabriel Goller <g.goller@proxmox.com>
Subject: Re: [pve-devel] [PATCH network] sdn: write frr config if the frr.conf.local file exists
Date: Wed, 1 Oct 2025 12:06:20 +0200	[thread overview]
Message-ID: <40a85883-b25c-4042-b842-10f013836efc@proxmox.com> (raw)
In-Reply-To: <20250919132438.167861-1-g.goller@proxmox.com>

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

      reply	other threads:[~2025-10-01 10:06 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-19 13:24 Gabriel Goller
2025-10-01 10:06 ` Stefan Hanreich [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=40a85883-b25c-4042-b842-10f013836efc@proxmox.com \
    --to=s.hanreich@proxmox.com \
    --cc=g.goller@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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