[pve-devel] [PATCH docs/network 0/2] improve behavior of frr service when pre-installed

[pve-devel] [PATCH docs/network 0/2] improve behavior of frr service when pre-installed

[Stefan Hanreich]

Fixes the issue of the FRR service getting started, but not enabled when
applying an SDN configuration and updates the docs to reflect the new behavior.

pve-network:

Stefan Hanreich (1):
  frr: enable frr service on reloading the controller config

 src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 1 +
 1 file changed, 1 insertion(+)


pve-docs:

Stefan Hanreich (1):
  sdn: frr update documentation for installing frr package

 pvesdn.adoc | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)


Summary over all repositories:
  2 files changed, 10 insertions(+), 12 deletions(-)

-- 
Generated by git-murpp 0.8.0

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH pve-network 1/1] frr: enable frr service on reloading the controller config

[Stefan Hanreich]

Since we now ship frr with Proxmox VE, the frr service is available on
the nodes but disabled on install. Prior to that users had to manually
install frr, which automatically enabled the service. When applying a
SDN configuration with an EVPN controller, we invoke systemctl restart
frr, which leads to the service running but still being in the
disabled state. This means that the EVPN setup is working until the
next reboot. To avoid the situation where users configure an EVPN
controller and everything seems to be working, until a restart breaks
the EVPN setup, additionally enable the frr service before restarting
it.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
index c245ea2..4249cc5 100644
--- a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
+++ b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
@@ -638,6 +638,7 @@ sub reload_controller {
 	};
 	if ($@) {
 	    warn "frr reload command fail. Restarting frr.";
+	    run_command(['systemctl', 'enable', 'frr']);
 	    eval { run_command(['systemctl', 'restart', 'frr']); };
 	}
     }
-- 
2.39.5


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH pve-network 1/1] frr: enable frr service on reloading the controller config

[Thomas Lamprecht]

On 08/04/2025 18:32, Stefan Hanreich wrote:
> Since we now ship frr with Proxmox VE, the frr service is available on
> the nodes but disabled on install. Prior to that users had to manually
> install frr, which automatically enabled the service. When applying a
> SDN configuration with an EVPN controller, we invoke systemctl restart
> frr, which leads to the service running but still being in the
> disabled state. This means that the EVPN setup is working until the
> next reboot. To avoid the situation where users configure an EVPN
> controller and everything seems to be working, until a restart breaks
> the EVPN setup, additionally enable the frr service before restarting
> it.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
> index c245ea2..4249cc5 100644
> --- a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
> +++ b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
> @@ -638,6 +638,7 @@ sub reload_controller {
>  	};
>  	if ($@) {
>  	    warn "frr reload command fail. Restarting frr.";
> +	    run_command(['systemctl', 'enable', 'frr']);

can we guard this with an  file exists check for
"/etc/systemd/system/multi-user.target.wants/frr.service"? Not a must, but does
not feel right to unconditionally call systemctl enable.

>  	    eval { run_command(['systemctl', 'restart', 'frr']); };
>  	}
>      }



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH pve-network 1/1] frr: enable frr service on reloading the controller config

[Stefan Hanreich]

> On 08.04.2025 20:43 CEST Thomas Lamprecht <t.lamprecht@proxmox.com> wrote:
> 
>  
> On 08/04/2025 18:32, Stefan Hanreich wrote:
> > Since we now ship frr with Proxmox VE, the frr service is available on
> > the nodes but disabled on install. Prior to that users had to manually
> > install frr, which automatically enabled the service. When applying a
> > SDN configuration with an EVPN controller, we invoke systemctl restart
> > frr, which leads to the service running but still being in the
> > disabled state. This means that the EVPN setup is working until the
> > next reboot. To avoid the situation where users configure an EVPN
> > controller and everything seems to be working, until a restart breaks
> > the EVPN setup, additionally enable the frr service before restarting
> > it.
> > 
> > Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> > ---
> >  src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
> > index c245ea2..4249cc5 100644
> > --- a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
> > +++ b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
> > @@ -638,6 +638,7 @@ sub reload_controller {
> >  	};
> >  	if ($@) {
> >  	    warn "frr reload command fail. Restarting frr.";
> > +	    run_command(['systemctl', 'enable', 'frr']);
> 
> can we guard this with an  file exists check for
> "/etc/systemd/system/multi-user.target.wants/frr.service"? Not a must, but does
> not feel right to unconditionally call systemctl enable.

probably a bit too convoluted and unclear, but this is guarded by the existence of frr-reload.py already, which requires frr to be installed (above in the if with an early return).

I can certainly whip up a v2 quite early tomorrow, shouldn't take long. Do you have any input on pre-installing frr-pythontools as well?

> >  	    eval { run_command(['systemctl', 'restart', 'frr']); };
> >  	}
> >      }


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH pve-network 1/1] frr: enable frr service on reloading the controller config

[Stoiko Ivanov]

On Tue, 8 Apr 2025 20:43:17 +0200
Thomas Lamprecht <t.lamprecht@proxmox.com> wrote:

> On 08/04/2025 18:32, Stefan Hanreich wrote:
> > Since we now ship frr with Proxmox VE, the frr service is available on
> > the nodes but disabled on install. Prior to that users had to manually
> > install frr, which automatically enabled the service. When applying a
> > SDN configuration with an EVPN controller, we invoke systemctl restart
> > frr, which leads to the service running but still being in the
> > disabled state. This means that the EVPN setup is working until the
> > next reboot. To avoid the situation where users configure an EVPN
> > controller and everything seems to be working, until a restart breaks
> > the EVPN setup, additionally enable the frr service before restarting
> > it.
> > 
> > Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> > ---
> >  src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
> > index c245ea2..4249cc5 100644
> > --- a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
> > +++ b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
> > @@ -638,6 +638,7 @@ sub reload_controller {
> >  	};
> >  	if ($@) {
> >  	    warn "frr reload command fail. Restarting frr.";
> > +	    run_command(['systemctl', 'enable', 'frr']);  
> 
> can we guard this with an  file exists check for
> "/etc/systemd/system/multi-user.target.wants/frr.service"? Not a must, but does
> not feel right to unconditionally call systemctl enable.
while talking off-list with Gabriel and Stefan I argued that `systemctl
is-enabled` probably costs as much as running `systemctl enable` for a
service (open socket - tell pid 1 to do stuff, wait for result) - so 
now took the time to look into it (with strace, and ignoring what pid 1
does) - in this case the output of `strace -yyttf systemctl enable frr`
vs. `strace -yyttf systemctl is-enabled frr` is around 2.5 orders of
magnitude (58k vs 9.9M) - and even for a service which does not ship an
init-script anymore (thus causing a few forks for systemd-sysv-install),
it's 56k vs 3.3M.

in any-case a `-e /etc/systemd/system/multi-user.target.wants/frr.service`
is probably the most economic version.
I tried figuring out if this check could break due to external
cirumstances - if the service is started as part of a target and that
target is pulled into multi-user.target - the symlink is not present
(e.g. zfs-zed) - but even then we'd fall back to the "expensive" enabling.

summing up - the existence check seems sensible to me as well.


> 
> >  	    eval { run_command(['systemctl', 'restart', 'frr']); };
> >  	}
> >      }  
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH pve-network 1/1] frr: enable frr service on reloading the controller config

[Stefan Hanreich]

> On 08.04.2025 21:42 CEST Stoiko Ivanov <s.ivanov@proxmox.com> wrote:
> 
>  
> On Tue, 8 Apr 2025 20:43:17 +0200
> Thomas Lamprecht <t.lamprecht@proxmox.com> wrote:
> 
> > On 08/04/2025 18:32, Stefan Hanreich wrote:
> > > Since we now ship frr with Proxmox VE, the frr service is available on
> > > the nodes but disabled on install. Prior to that users had to manually
> > > install frr, which automatically enabled the service. When applying a
> > > SDN configuration with an EVPN controller, we invoke systemctl restart
> > > frr, which leads to the service running but still being in the
> > > disabled state. This means that the EVPN setup is working until the
> > > next reboot. To avoid the situation where users configure an EVPN
> > > controller and everything seems to be working, until a restart breaks
> > > the EVPN setup, additionally enable the frr service before restarting
> > > it.
> > > 
> > > Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> > > ---
> > >  src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
> > > index c245ea2..4249cc5 100644
> > > --- a/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
> > > +++ b/src/PVE/Network/SDN/Controllers/EvpnPlugin.pm
> > > @@ -638,6 +638,7 @@ sub reload_controller {
> > >  	};
> > >  	if ($@) {
> > >  	    warn "frr reload command fail. Restarting frr.";
> > > +	    run_command(['systemctl', 'enable', 'frr']);  
> > 
> > can we guard this with an  file exists check for
> > "/etc/systemd/system/multi-user.target.wants/frr.service"? Not a must, but does
> > not feel right to unconditionally call systemctl enable.
> while talking off-list with Gabriel and Stefan I argued that `systemctl
> is-enabled` probably costs as much as running `systemctl enable` for a
> service (open socket - tell pid 1 to do stuff, wait for result) - so 
> now took the time to look into it (with strace, and ignoring what pid 1
> does) - in this case the output of `strace -yyttf systemctl enable frr`
> vs. `strace -yyttf systemctl is-enabled frr` is around 2.5 orders of
> magnitude (58k vs 9.9M) - and even for a service which does not ship an
> init-script anymore (thus causing a few forks for systemd-sysv-install),
> it's 56k vs 3.3M.
> 
> in any-case a `-e /etc/systemd/system/multi-user.target.wants/frr.service`
> is probably the most economic version.
> I tried figuring out if this check could break due to external
> cirumstances - if the service is started as part of a target and that
> target is pulled into multi-user.target - the symlink is not present
> (e.g. zfs-zed) - but even then we'd fall back to the "expensive" enabling.
> 
> summing up - the existence check seems sensible to me as well.

It certainly wouldn't hurt and your points sound sensible, I'll send
a v2 early tomorrow. Thanks for looking into this further!

> > 
> > >  	    eval { run_command(['systemctl', 'restart', 'frr']); };
> > >  	}
> > >      }  
> > 
> > 
> > 
> > _______________________________________________
> > pve-devel mailing list
> > pve-devel@lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> > 
> >


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH pve-docs 1/1] sdn: frr update documentation for installing frr package

[Stefan Hanreich]

The frr service now gets automatically enabled when applying an SDN
configuration with an EVPN controller present. Reflect this change in
the documentation. Also add a note for users upgrading from prior
versions, that they still need to install frr before applying the SDN
configuration.

Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
---
 pvesdn.adoc | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/pvesdn.adoc b/pvesdn.adoc
index 5e58cc3..eef2abf 100644
--- a/pvesdn.adoc
+++ b/pvesdn.adoc
@@ -115,20 +115,17 @@ FRRouting
 ~~~~~~~~~
 
 The {pve} SDN stack uses the https://frrouting.org/[FRRouting] project for
-advanced setups. This is currently opt-in.
+advanced setups. Since version 8.4 FRR is automatically included in {pve}
+and gets automatically enabled when applying an SDN configuration with an EVPN
+controller configured.
 
-To use the SDN routing integration you need to install the `frr-pythontools`
-package on all nodes:
+If you upgraded from an earlier version, you need to install the frr and
+frr-pythontools packages before applying an SDN configuration with an EVPN
+controller:
 
 ----
 apt update
-apt install frr-pythontools
-----
-
-Then enable the frr service on all nodes:
-
-----
-systemctl enable frr.service
+apt install frr frr-pythontools
 ----
 
 [[pvesdn_main_configuration]]
@@ -450,8 +447,8 @@ EVPN Controller
 The `EVPN`, zone requires an external controller to manage the control plane.
 The EVPN controller plugin configures the Free Range Routing (frr) router.
 
-To enable the EVPN controller, you need to enable FRR on every node, see
-xref:pvesdn_install_frrouting[install FRRouting].
+If you upgraded {pve} from a version earlier than 8.4, you need to install FRR,
+see xref:pvesdn_install_frrouting[install FRRouting].
 
 EVPN controller configuration options:
 
-- 
2.39.5


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH docs/network 0/2] improve behavior of frr service when pre-installed

[Stefan Hanreich]

We should also consider adding frr-pythontools to the ISO, since
otherwise applying doesn't work ootb.


On 4/8/25 18:32, Stefan Hanreich wrote:
> Fixes the issue of the FRR service getting started, but not enabled when
> applying an SDN configuration and updates the docs to reflect the new behavior.
> 
> pve-network:
> 
> Stefan Hanreich (1):
>   frr: enable frr service on reloading the controller config
> 
>  src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
> 
> pve-docs:
> 
> Stefan Hanreich (1):
>   sdn: frr update documentation for installing frr package
> 
>  pvesdn.adoc | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> 
> Summary over all repositories:
>   2 files changed, 10 insertions(+), 12 deletions(-)
> 



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH docs/network 0/2] improve behavior of frr service when pre-installed

[Friedrich Weber]

On 08/04/2025 18:32, Stefan Hanreich wrote:
> Fixes the issue of the FRR service getting started, but not enabled when
> applying an SDN configuration and updates the docs to reflect the new behavior.

- started with fresh ISO with sha256sum:
695c1c9b247e62d851d29d39c3e9399d3a15dd7c1bdca68b357b6bd578ee9925

- set up evpn controller and zone, clicked apply
 => frr.service is disabled but running

- reboot => frr.service is not running

- applied this patch, clicked apply
 => frr.service is now enabled and running

- reboot => frr.service is running

So this looks good to me. I was wondering whether enabling frr may have
any side effects on existing setups, but since we only enable if we
restart anyway, I don't think this should cause any issues.

Consider this

Tested-by: Friedrich Weber <f.weber@proxmox.com>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel