From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id A63391FF165 for ; Thu, 22 May 2025 18:22:41 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4D9D7B0B8; Thu, 22 May 2025 18:18:31 +0200 (CEST) From: Stefan Hanreich To: pve-devel@lists.proxmox.com Date: Thu, 22 May 2025 18:16:57 +0200 Message-Id: <20250522161731.537011-42-s.hanreich@proxmox.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20250522161731.537011-1-s.hanreich@proxmox.com> References: <20250522161731.537011-1-s.hanreich@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.225 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RDNS_NONE 0.793 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Subject: [pve-devel] [PATCH pve-network v3 08/21] sdn: api: add check for rewriting frr configuration X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" With the old FRR config generation logic, we never wrote an empty FRR configuration if all controllers got deleted. This meant that deleting all controllers still left the previous FRR configuration on the nodes, never disabling BGP / IS-IS. The new logic now writes an empty configuration if there is no controller / fabric configured, fixing this behavior. This has a side effect for users with an existing FRR configuration not managed by SDN, but utilizing other SDN features (zones, vnets, ...). Their manual FRR configuration would get overwritten when applying any SDN configuration. This is particularly an issue with full-mesh Ceph setups, that were set up according to our Wiki guide [1]. User with such a full-mesh setup could get their FRR configuration overwritten when using unrelated SDN features. Since we call the API endpoint in pve-manager for generating and writing configuration files, we cannot directly prevent the FRR configuration from being written in the SDN API call. Instead a new parameter, skip_frr, has been added to the endpoint in pve-manager, that skips writing the FRR configuration. We skip writing the FRR configuration if neither the previous SDN configuration, nor the new SDN configuration contains an entity that requires writing FRR configuration. This should minimize the impact of the change to the FRR config generation on existing FRR setups. [1] https://pve.proxmox.com/mediawiki/index.php?title=Full_Mesh_Network_for_Ceph_Server&oldid=12146 Signed-off-by: Stefan Hanreich --- src/PVE/API2/Network/SDN.pm | 15 ++++++++++++--- src/PVE/Network/SDN.pm | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm index d216e48..910f89a 100644 --- a/src/PVE/API2/Network/SDN.pm +++ b/src/PVE/API2/Network/SDN.pm @@ -82,12 +82,17 @@ __PACKAGE__->register_method({ }}); my $create_reload_network_worker = sub { - my ($nodename) = @_; + my ($nodename, $skip_frr) = @_; + + my @command = ('pvesh', 'set', "/nodes/$nodename/network"); + if ($skip_frr) { + push(@command, '--skip_frr'); + } # FIXME: how to proxy to final node ? my $upid; print "$nodename: reloading network config\n"; - run_command(['pvesh', 'set', "/nodes/$nodename/network"], outfunc => sub { + run_command(\@command, outfunc => sub { my $line = shift; if ($line =~ /["']?(UPID:[^\s"']+)["']?$/) { $upid = $1; @@ -120,14 +125,18 @@ __PACKAGE__->register_method ({ my $rpcenv = PVE::RPCEnvironment::get(); my $authuser = $rpcenv->get_user(); + 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 $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) }; + my $pid = eval { $create_reload_network_worker->($node, $skip_frr) }; warn $@ if $@; } diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm index 382147f..1ab2e4d 100644 --- a/src/PVE/Network/SDN.pm +++ b/src/PVE/Network/SDN.pm @@ -89,6 +89,27 @@ sub running_config { return cfs_read_file($running_cfg); } +=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 +the FRR configuration is required or not. + +If C<\%running_config> is not provided, it will query the current running +configuration and then evaluate it. + +=cut + +sub running_config_has_frr { + my $running_config = PVE::Network::SDN::running_config(); + + # both can be empty if the SDN configuration was never applied + my $controllers = $running_config->{controllers}->{ids} // {}; + my $fabrics = $running_config->{fabrics}->{ids} // {}; + + return %$controllers || %$fabrics; +} + sub pending_config { my ($running_cfg, $cfg, $type) = @_; -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel