public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Gabriel Goller <g.goller@proxmox.com>
To: Hannes Laimer <h.laimer@proxmox.com>
Cc: pve-devel@lists.proxmox.com
Subject: Re: [PATCH proxmox-ve-rs 7/9] frr: support custom frr configuration lines
Date: Thu, 19 Feb 2026 16:01:53 +0100	[thread overview]
Message-ID: <kzfrer7p3jx362zhjewi7pymshsfda4tdoo5f6sjkucf5rbkl2@uvn7yrbqzyhj> (raw)
In-Reply-To: <e77d3a9e-48b7-4c45-8ca2-a4a9543f52e7@proxmox.com>

On 19.02.2026 13:17, Hannes Laimer wrote:
> currently we place the custom stuff from `frr.conf.local` at the top of
> `frr.conf`. I don't think it should be a problem having it at the
> bottom, but I'm not super sure if there maybe exist some options we
> don't merge that care about the ordering. I couldn't find any, but in
> case I missed something, wanted to note it here.

This should be possible. It's a bit weird, as it's before the route-maps
right in the middle of the config and not all the statements can be
overridden by the frr.conf.local -- but it's better than not being
backwards-compatible :).

I'll add the following hunk and fix the tests in pve-network:

diff --git a/proxmox-frr-templates/templates/frr.conf.jinja b/proxmox-frr-templates/templates/frr.conf.jinja
index f9ca858907..5731781cc5 100644
--- a/proxmox-frr-templates/templates/frr.conf.jinja
+++ b/proxmox-frr-templates/templates/frr.conf.jinja
@@ -4,9 +4,9 @@
 {% include "ospfd.jinja" %}
 {% include "access_lists.jinja" %}
 {% include "prefix_lists.jinja" %}
-{% include "route_maps.jinja" %}
-{% include "ip_routes.jinja" %}
-{% include "protocol_routemaps.jinja" %}
 {% for line in custom_frr_config %}
 {{ line }}
 {% endfor %}
+{% include "route_maps.jinja" %}
+{% include "ip_routes.jinja" %}
+{% include "protocol_routemaps.jinja" %}


I locally have a test for the frr conf local merging (I'll add it in the
next version of this patch series), which when applied to the current
version of pve-network shows the following diff:
@@ -1,10 +1,9 @@
-#          got: 'frr version 10.4.1
+#     expected: 'frr version 10.4.1
 # frr defaults datacenter
 # hostname localhost
 # log syslog informational
 # service integrated-vtysh-config
 # !
-# !
 # vrf vrf_myzone
 #  vni 1000
 # exit-vrf
@@ -22,7 +21,6 @@
 #  neighbor 192.168.0.3 peer-group VTEP
 #  neighbor 192.168.1.1 remote-as 65001
 #  neighbor 192.168.1.1 description "External Peer"
-#  !
 #  address-family ipv4 unicast
 #   neighbor VTEP activate
 #  exit-address-family
@@ -31,8 +29,8 @@
 #   neighbor VTEP activate
 #   neighbor VTEP route-map MAP_VTEP_IN in
 #   neighbor VTEP route-map MAP_VTEP_OUT out
-#   advertise-all-vni
 #   advertise-svi-ip
+#   advertise-all-vni
 #  exit-address-family
 # exit
 # !
@@ -41,29 +39,25 @@
 #  no bgp hard-administrative-reset
 #  no bgp graceful-restart notification
 # exit
+# route-map MAP_VTEP_IN permit 2
+#  set community 65000:200
+# exit
 # !
 # ip prefix-list PL_ALLOW seq 10 permit 10.0.0.0/8 le 24
-# !
-# bgp community-list standard CL_LOCAL permit 65000:200
-# !
-# route-map CUSTOM_MAP permit 1
+# route-map CUSTOM_MAP permit 10
 #  match ip address prefix-list PL_ALLOW
 # exit
 # !
-# route-map MAP_VTEP_IN permit 1
-# exit
+# bgp community-list standard CL_LOCAL permit 65000:200
 # !
-# route-map MAP_VTEP_IN permit 2
-#  set community 65000:200
+# route-map MAP_VTEP_IN permit 1
 # exit
 # !
 # route-map MAP_VTEP_OUT permit 1
-# exit
-# !
-# route-map MAP_VTEP_OUT permit 2
 #  set community 65000:100
 # exit
 # !
 # line vty
 # !
 # '
+

IMO this is quite good.

Thanks for the review!

> On 2026-02-03 17:01, Gabriel Goller wrote:
> > When merging the frr.conf.local with the frr.conf, some lines cannot be
> > merged and we need to add custom frr config lines to the rust
> > configuration. Add the vec of lines and just dump them into the
> > template.
> > 
> > Co-authored-by: Stefan Hanreich <s.hanreich@proxmox.com>
> > Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> > ---
> >  proxmox-frr-templates/templates/frr.conf.jinja | 3 +++
> >  proxmox-frr/src/ser/mod.rs                     | 3 +++
> >  2 files changed, 6 insertions(+)
> > 
> > diff --git a/proxmox-frr-templates/templates/frr.conf.jinja b/proxmox-frr-templates/templates/frr.conf.jinja
> > index c8495b417990..6d60ad2a4c4c 100644
> > --- a/proxmox-frr-templates/templates/frr.conf.jinja
> > +++ b/proxmox-frr-templates/templates/frr.conf.jinja
> > @@ -4,3 +4,6 @@
> >  {% include "access_lists.jinja" %}
> >  {% include "route_maps.jinja" %}
> >  {% include "protocol_routemaps.jinja" %}
> > +{% for line in custom_frr_config %}
> > +{{ line }}
> > +{% endfor %}
> > diff --git a/proxmox-frr/src/ser/mod.rs b/proxmox-frr/src/ser/mod.rs
> > index 9aaee74d7af0..3baa0a318fb0 100644
> > --- a/proxmox-frr/src/ser/mod.rs
> > +++ b/proxmox-frr/src/ser/mod.rs
> > @@ -185,6 +185,9 @@ pub struct FrrConfig {
> >      #[builder(default)]
> >      #[serde(default)]
> >      pub access_lists: BTreeMap<AccessListName, Vec<AccessListRule>>,
> > +    #[builder(default)]
> > +    #[serde(default)]
> > +    pub custom_frr_config: Vec<String>,
> >  }
> >  
> >  #[derive(Clone, Debug, PartialEq, Eq, Default, Serialize, Deserialize)]
> 




  reply	other threads:[~2026-02-19 15:00 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-03 16:01 [PATCH docs/manager/network/proxmox{-ve-rs,-perl-rs} 00/23] Generate frr config using jinja templates and rust types Gabriel Goller
2026-02-03 16:01 ` [PATCH proxmox-ve-rs 1/9] ve-config: firewall: cargo fmt Gabriel Goller
2026-02-03 16:01 ` [PATCH proxmox-ve-rs 2/9] frr: add proxmox-frr-templates package that contains templates Gabriel Goller
2026-02-03 16:01 ` [PATCH proxmox-ve-rs 3/9] ve-config: remove FrrConfigBuilder struct Gabriel Goller
2026-02-03 16:01 ` [PATCH proxmox-ve-rs 4/9] sdn-types: support variable-length NET identifier Gabriel Goller
2026-02-03 16:01 ` [PATCH proxmox-ve-rs 5/9] frr: add template serializer and serialize fabrics using templates Gabriel Goller
2026-02-03 16:01 ` [PATCH proxmox-ve-rs 6/9] frr: add isis configuration and templates Gabriel Goller
2026-02-03 16:01 ` [PATCH proxmox-ve-rs 7/9] frr: support custom frr configuration lines Gabriel Goller
2026-02-19 12:17   ` Hannes Laimer
2026-02-19 15:01     ` Gabriel Goller [this message]
2026-02-03 16:01 ` [PATCH proxmox-ve-rs 8/9] frr: add bgp support with templates and serialization Gabriel Goller
2026-02-03 16:01 ` [PATCH proxmox-ve-rs 9/9] frr: store frr template content as a const map Gabriel Goller
2026-02-03 16:01 ` [PATCH proxmox-perl-rs 1/2] sdn: add function to generate the frr config for all daemons Gabriel Goller
2026-02-03 16:01 ` [PATCH proxmox-perl-rs 2/2] sdn: add method to get a frr template Gabriel Goller
2026-02-03 16:01 ` [PATCH pve-network 01/10] sdn: remove duplicate comment line '!' in frr config Gabriel Goller
2026-02-03 16:01 ` [PATCH pve-network 02/10] sdn: tests: add missing comment " Gabriel Goller
2026-02-03 16:01 ` [PATCH pve-network 03/10] tests: use Test::Differences to make test assertions Gabriel Goller
2026-02-03 16:01 ` [PATCH pve-network 04/10] sdn: write structured frr config that can be rendered using templates Gabriel Goller
2026-02-19 13:52   ` Hannes Laimer
2026-02-19 15:36     ` Gabriel Goller
2026-02-19 15:44       ` Gabriel Goller
2026-02-03 16:01 ` [PATCH pve-network 05/10] tests: rearrange some statements in the frr config Gabriel Goller
2026-02-03 16:01 ` [PATCH pve-network 06/10] sdn: adjust frr.conf.local merging to rust template types Gabriel Goller
2026-02-03 16:01 ` [PATCH pve-network 07/10] cli: add pvesdn cli tool for managing frr template overrides Gabriel Goller
2026-02-19 12:39   ` Hannes Laimer
2026-02-19 15:49     ` Gabriel Goller
2026-02-03 16:01 ` [PATCH pve-network 08/10] debian: handle user modifications to FRR templates via ucf Gabriel Goller
2026-02-03 16:01 ` [PATCH pve-network 09/10] api: add dry-run endpoint for sdn apply to preview changes Gabriel Goller
2026-02-03 16:01 ` [PATCH pve-network 10/10] test: add test for frr.conf.local merging Gabriel Goller
2026-02-03 16:01 ` [PATCH pve-manager 1/1] sdn: add dry-run view for sdn apply Gabriel Goller
2026-02-03 16:01 ` [PATCH pve-docs 1/1] docs: add man page for the `pvesdn` cli Gabriel Goller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=kzfrer7p3jx362zhjewi7pymshsfda4tdoo5f6sjkucf5rbkl2@uvn7yrbqzyhj \
    --to=g.goller@proxmox.com \
    --cc=h.laimer@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal