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 B98191FF0AE for ; Tue, 01 Sep 2026 09:09:52 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 7E10F214D5; Tue, 01 Sep 2026 09:09:51 +0200 (CEST) Date: Tue, 1 Sep 2026 09:09:43 +0200 From: Hannes Laimer To: pve-devel@lists.proxmox.com Subject: Re: [PATCH docs/gui-tests/manager/network/proxmox{-ve-rs,-perl-rs} v3 00/13] Add IS-IS protocol to fabrics Message-ID: References: <20260828113255.176546-1-g.goller@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1788246581714 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.786 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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: F7A4J5IUFSNX6G3S2HIR4CNHWW7VKUYH X-Message-ID-Hash: F7A4J5IUFSNX6G3S2HIR4CNHWW7VKUYH X-MailFrom: h.laimer@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: On 2026-08-31 14:59, Gabriel Goller wrote: > On 31.08.2026 14:27, Gabriel Goller wrote: > > On 31.08.2026 10:48, Hannes Laimer wrote: > > > Aside from these very tiny nits/questions, looks very good to me: > > > - could we have the same `network_type`-nob here that we introduced for > > > OSPF? > > > > Yes, makes sense. ISIS supports network-type broadcast (unlike openfabric) and > > even multi-access (e.g. multiple devices behind a switch). > > Oh, no wait I won't be adding this. ISIS only has broadcast and point-to-point > available, and the default is broadcast. We use broadcast when an interface ip > is set and point-to-point when no ip is set. > yes, but we only need a specified ip for v4 broadcast, v6 does not. so unnumbered v6 broadcast is not possible currently the ip for the nic is taken from the node config, so leaving the nic ip fields empty doesn't necesarrily mean do p2p, p2p still needs a next-htop target and for v4 this has to be specified. we just take the ip from the node, which is fine. but leaving the ip field empty does not imply do `p2p`, just: use the node-ip for either(p2p or broadcast). for v6 we could actually never copy the node-ip to the nic(we do that now always), this would allow configuring unnumbered v6 broadcast or p2p, which are both possible with v6. does isisd support v4-via-v6 routes? if it does we could just do v6 unnumbered in case no nic ip is specified and skip configuring the nic ourself.. either way, I still think the nob makes sense here, cause the the existance of a specified ip on the nic doesn't really point either way (bc or p2p), both need one, and for v6 can work without one > > > - the warning in the UI could be warpped in a `gettext()` > > > > Agree. > > > > > - could we put `isisd => 1` behind an IS-IS fabric presence check? i > > > guess that could break setups that have it enabled manually though.. > > > > Hmm you're right I missed this. I somehow thought we always have it enabled like > > bgpd, but turns out the user needs to manually enable it. > > > > What if we set it to 0 and add something like this: > > > > diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm > > index 33a3cf3524..8737e187cf 100644 > > --- a/src/PVE/Network/SDN.pm > > +++ b/src/PVE/Network/SDN.pm > > @@ -462,7 +462,7 @@ sub generate_frr_raw_config { > > ); > > } > > > > -=head3 get_frr_daemon_status(\%fabric_config) > > +=head3 get_frr_daemon_status(\%fabric_config, \%running_config) > > > > Returns a hash that indicates which FRR daemons, that are managed by SDN, should > > be enabled / disabled. > > @@ -470,9 +470,20 @@ =head3 get_frr_daemon_status(\%fabric_config, \%running_config) > > =cut > > > > sub get_frr_daemon_status { > > - my ($fabric_config) = @_; > > - > > - return PVE::Network::SDN::Fabrics::get_frr_daemon_status($fabric_config); > > + my ($fabric_config, $running_config) = @_; > > + > > + my $daemon_status = PVE::Network::SDN::Fabrics::get_frr_daemon_status($fabric_config); > > + my $nodename = PVE::INotify::nodename(); > > + > > + for my $controller (values %{ $running_config->{controllers}->{ids} // {} }) { > > + next if $controller->{type} ne 'isis'; > > + next if $controller->{node} ne $nodename; > > + > > + $daemon_status->{isisd} = 1; > > + last; > > + } > > + > > + return $daemon_status; > > } > > > > > > > Consider this: > > > > > > Tested-by: Hannes Laimer > > > Reviewed-by: Hannes Laimer > > > > Thanks for the review! > > > > > On 2026-08-28 13:32, Gabriel Goller wrote: > > > > As part of our effort to deprecate the existing controllers and transition > > > > toward a fabric-based architecture, we are introducing an IS-IS fabric. To > > > > centralize all routing protocols, we plan to consolidate them under the > > > > "Fabrics" sidebar menu. Additionally, we aim to generate all routing protocol > > > > configurations in a unified location using Rust. > > > > > > > > Following the integration of OpenFabric and OSPF, the next logical step is to > > > > migrate the IS-IS controller to the fabrics framework. > > > > > > > > This patch series adds an IS-IS Fabric option to the "Fabrics" menu while > > > > retaining the original IS-IS controller for backward compatibility. Since the > > > > two do not interfere, we will include a warning recommending the use of the new > > > > IS-IS Fabrics instead. The primary difference between the IS-IS Controller and > > > > the IS-IS Fabric is that the Fabric does not automatically redistribute > > > > connected routes into the Link State DB. This change is intentional, as we > > > > intend to implement a more advanced redistribution and route-map mechanism in > > > > the future, rather than relying on a simple checkbox. Users who need to > > > > redistribute connected routes with the new IS-IS Fabrics can do so by manually > > > > adding the appropriate FRR statement to the /etc/frr/frr.conf.local file. > > > > > > > > Changelog: > > > > v2: > > > > * rebase ontop of master > > > > v1: > > > > * rebase ontop of template series > > > > > > > > [snip]