From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 12B011FF13E for ; Wed, 01 Jul 2026 12:31:03 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id B8FAB21430; Wed, 01 Jul 2026 12:31:01 +0200 (CEST) From: Gabriel Goller To: pve-devel@lists.proxmox.com Subject: [PATCH pve-network 1/2] sdn: frr: add append-only custom frr config Date: Wed, 1 Jul 2026 12:30:44 +0200 Message-ID: <20260701103051.119422-2-g.goller@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260701103051.119422-1-g.goller@proxmox.com> References: <20260701103051.119422-1-g.goller@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1782901843158 X-SPAM-LEVEL: Spam detection results: 0 DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: XV7S3YGAL3JHTN2O7JHZLDNOKWNNJVGU X-Message-ID-Hash: XV7S3YGAL3JHTN2O7JHZLDNOKWNNJVGU X-MailFrom: g.goller@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Users sometimes need FRR config that SDN does not model yet. Editing /etc/frr/frr.conf is not useful because the file is generated and gets rewritten. frr.conf.local still works, but it is parsed and merged into the SDN-generated config, and that merge can fail for unsupported constructs. Append files from /etc/frr/frr.conf.d/*.conf after the full generated FRR config, including fabric config generated by Rust. Files are read in sorted filename order and copied without parsing or modifying their content. Wrap each appended snippet in begin/end marker comments in the installed configuration. running_config_has_frr() also checks the active /etc/frr/frr.conf for that marker so that removing the last snippet, or the whole frr.conf.d directory, still triggers an FRR rewrite. Without that check, stale appended config remain installed because there would be no remaining SDN or custom FRR input to trigger a regeneratin of frr.conf. Build the full candidate config first and validate it with vtysh -C -f before replacing /etc/frr/frr.conf. If validation fails, keep the old config in place and include the vtysh output in the error. Using the append-only user-configuration we can model about 95% of the possible setups. There are some config constellations that are impossible to generate, we would need user-editable templates for that. The problem is that the templates are quite hard to understand and even harder to correctly modify (i.e. you need to read the code). Signed-off-by: Gabriel Goller --- src/PVE/Network/SDN.pm | 26 ++++++++- src/PVE/Network/SDN/Frr.pm | 115 ++++++++++++++++++++++++++++++++++--- 2 files changed, 130 insertions(+), 11 deletions(-) diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm index 33a3cf352496..aeba89b6b6cb 100644 --- a/src/PVE/Network/SDN.pm +++ b/src/PVE/Network/SDN.pm @@ -113,7 +113,9 @@ sub running_config { =head3 running_config_has_frr(\%running_config) Determines whether C<\%running_config> contains any entities that generate an -FRR configuration. This is used by pve-manager to determine whether a rewrite of +FRR configuration, whether local FRR custom config files or append snippets +exist, or whether the active FRR configuration still contains previously +appended snippets. This is used by pve-manager to determine whether a rewrite of the FRR configuration is required or not. If C<\%running_config> is not provided, it will query the current running @@ -131,8 +133,18 @@ sub running_config_has_frr { my $prefix_lists = $running_config->{'prefix-lists'}->{ids} // {}; my $local_frr_config = PVE::Network::SDN::Frr::local_frr_config_exists(); - - return %$controllers || %$fabrics || %$route_maps || %$prefix_lists || $local_frr_config; + my $append_frr_config = PVE::Network::SDN::Frr::read_append_frr_config(); + my $active_frr_config_has_append = + PVE::Network::SDN::Frr::active_config_has_append_frr_config(); + + return + %$controllers + || %$fabrics + || %$route_maps + || %$prefix_lists + || $local_frr_config + || $append_frr_config ne '' + || $active_frr_config_has_append; } sub pending_config { @@ -449,10 +461,14 @@ sub generate_frr_raw_config { my $frr_config = {}; PVE::Network::SDN::Controllers::generate_frr_config($frr_config, $running_config); + PVE::Network::SDN::Frr::append_local_config($frr_config); my $nodename = PVE::INotify::nodename(); + # This also appends the FRR config generated by fabrics in Rust. The + # frr.conf.d snippets are added later in raw_config_to_string(), after these + # raw lines have been rendered and before the final line vty stanza. return PVE::RS::SDN::get_frr_raw_config( $frr_config->{'frr'}, $prefix_list_config, @@ -490,6 +506,10 @@ sub generate_frr_config { my $needs_restart = PVE::Network::SDN::Frr::set_daemon_status($daemon_status, 1); my $raw_config = PVE::Network::SDN::generate_frr_raw_config($running_config, $fabric_config); + + # write_raw_config() inserts frr.conf.d/*.conf after the rendered config, + # including the fabric FRR config, and before the final line vty stanza. It + # validates the complete candidate before installing it as frr.conf. PVE::Network::SDN::Frr::write_raw_config($raw_config); PVE::Network::SDN::Frr::apply($needs_restart) if $apply; diff --git a/src/PVE/Network/SDN/Frr.pm b/src/PVE/Network/SDN/Frr.pm index f81da325b1ef..9a57eb1a7eb7 100644 --- a/src/PVE/Network/SDN/Frr.pm +++ b/src/PVE/Network/SDN/Frr.pm @@ -22,6 +22,12 @@ frr configuration in this format. This format is also used for merging the local FRR config (a user-defined configuration file) with the controller-generated configuration. +The preferred way to use custom frr config is to paste config into files in +C. The config of these files is then appended +after the generated SDN config and before the final C stanza, sorted +by file name. Validate syntax with C before installing the final +configuration. + =head2 raw config This is generated from the frr config. It is an array where every entry is a @@ -29,10 +35,14 @@ string that is a FRR configuration line. =cut +use File::Temp qw(tempfile); + 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"; +my $FRR_CONF_APPEND_DIR = "/etc/frr/frr.conf.d"; +my $FRR_CONF_APPEND_BEGIN_MARKER = "! Begin user FRR config from "; =head3 local_frr_config_exists @@ -57,8 +67,57 @@ sub read_local_frr_config { return; # undef } +=head3 read_append_frr_config($dir) + +Returns custom FRR config from C<$dir/*.conf>, sorted by file name. If C<$dir> +is not set, C is used. + +=cut + +sub read_append_frr_config { + my ($dir) = @_; + + $dir //= $FRR_CONF_APPEND_DIR; + + my $config = ''; + + return $config if !-d $dir; + + for my $file (grep { -f $_ } sort glob("$dir/*.conf")) { + my $content = file_get_contents($file); + next if $content eq ''; + + $config .= "$FRR_CONF_APPEND_BEGIN_MARKER$file\n!\n"; + $config .= $content; + $config .= "\n" if $content !~ /\n\z/; + $config .= "!\n! End user FRR config from $file\n!\n"; + } + + return $config; +} + my $FRR_CONFIG_FILE = "/etc/frr/frr.conf"; +=head3 active_config_has_append_frr_config($file) + +Checks whether the active FRR config still contains append-only user config. +This is used to trigger a rewrite after the last snippet or the whole +C directory has been removed, so stale appended config gets +removed from C. + +=cut + +sub active_config_has_append_frr_config { + my ($file) = @_; + + $file //= $FRR_CONFIG_FILE; + + return 0 if !-e $file; + + my $config = file_get_contents($file); + return $config =~ /^\Q$FRR_CONF_APPEND_BEGIN_MARKER\E/m ? 1 : 0; +} + =head3 apply() Tries to reload FRR with the frr-reload.py script from frr-pythontools. If that @@ -192,7 +251,7 @@ sub set_daemon_status { Converts a given C<\@raw_config> to a string representing a complete frr configuration, ready to be written to /etc/frr/frr.conf. If raw_config is empty, -returns only the FRR config skeleton. +returns the FRR config skeleton plus any append snippets. =cut @@ -202,6 +261,10 @@ sub raw_config_to_string { my $nodename = PVE::INotify::nodename(); my @final_config = ( + "! This file is generated by Proxmox VE SDN.", + "! Do not edit directly; changes will be overwritten.", + "! For simple custom FRR additions, use /etc/frr/frr.conf.d/*.conf.", + "!", "frr version 10.6.1", "frr defaults datacenter", "hostname $nodename", @@ -211,27 +274,63 @@ sub raw_config_to_string { push @final_config, @$raw_config; - push @final_config, ( - "!", "line vty", "!", - ); + my $config = join("\n", @final_config) . "\n!\n"; + my $append_config = read_append_frr_config(); + + $config .= $append_config; + $config .= "line vty\n!\n"; - return join("\n", @final_config) . "\n"; + return $config; } -=head3 raw_config_to_string(\@raw_config) +=head3 write_raw_config(\@raw_config) -Writes a given C<\@raw_config> to /etc/frr/frr.conf. +Writes a given C<\@raw_config> to /etc/frr/frr.conf. Before replacing the real +configuration, the full candidate (including append snippets) is validated with +C. On validation failure, the frr.conf is not edited. =cut +sub validate_config { + my ($candidate_file) = @_; + + my $stdout = ''; + my $stderr = ''; + eval { + run_command( + ['vtysh', '-C', '-f', $candidate_file], + outfunc => sub { $stdout .= "$_[0]\n"; }, + errfunc => sub { $stderr .= "$_[0]\n"; }, + ); + }; + + if (my $err = $@) { + my $msg = "FRR configuration validation failed with 'vtysh -C -f $candidate_file': $err"; + $msg .= "\n" if $msg !~ /\n\z/; + $msg .= "vtysh stdout:\n$stdout" if $stdout ne ''; + $msg .= "vtysh stderr:\n$stderr" if $stderr ne ''; + die $msg; + } +} + sub write_raw_config { my ($raw_config) = @_; return if !-d "/etc/frr"; return if !$raw_config; - file_set_contents("/etc/frr/frr.conf", raw_config_to_string($raw_config)); + my $candidate = raw_config_to_string($raw_config); + + my ($fh, $candidate_file) = tempfile('frr.conf.XXXXXX', DIR => "/etc/frr"); + print $fh $candidate; + close($fh); + + eval { validate_config($candidate_file); }; + my $err = $@; + unlink $candidate_file; + die $err if $err; + file_set_contents($FRR_CONFIG_FILE, $candidate); } =head3 append_local_config(\%frr_config, $local_config) -- 2.47.3