From: Gabriel Goller <g.goller@proxmox.com>
To: Stefan Hanreich <s.hanreich@proxmox.com>
Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH network] sdn: write frr config if the frr.conf.local file exists
Date: Thu, 2 Oct 2025 14:47:28 +0200 [thread overview]
Message-ID: <vbzglowzpbyfftz5lrko4ky4gmpvamaquj2wxr5lkozchxegos@5mo7u3q7mrlo> (raw)
In-Reply-To: <40a85883-b25c-4042-b842-10f013836efc@proxmox.com>
On 01.10.2025 12:06, Stefan Hanreich wrote:
>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:
>> [snip]
>> 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?
good point, will send a v2 soon!
>+ return file_get_contents($FRR_CONF_LOCAL_FILE);
>> }
>> }
Thanks for the review!
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
next prev parent reply other threads:[~2025-10-02 12:47 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-19 13:24 Gabriel Goller
2025-10-01 10:06 ` Stefan Hanreich
2025-10-02 12:47 ` Gabriel Goller [this message]
2025-10-02 13:11 ` Gabriel Goller
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=vbzglowzpbyfftz5lrko4ky4gmpvamaquj2wxr5lkozchxegos@5mo7u3q7mrlo \
--to=g.goller@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=s.hanreich@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 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.