Re: [PATCH docs/gui-tests/manager/network/proxmox{-ve-rs,-perl-rs} v3 00/13] Add IS-IS protocol to fabrics

[Hannes Laimer <h.laimer@proxmox.com>]

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 <h.laimer@proxmox.com>
> > > Reviewed-by: Hannes Laimer <h.laimer@proxmox.com>
> > 
> > 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]