From: Hannes Laimer <h.laimer@proxmox.com>
To: Gabriel Goller <g.goller@proxmox.com>, pve-devel@lists.proxmox.com
Subject: Re: [PATCH manager/network/proxmox{-ve-rs,-perl-rs} v5 00/19] Generate frr config using jinja templates and rust types
Date: Thu, 12 Mar 2026 14:35:26 +0100 [thread overview]
Message-ID: <f279aac9-7006-4676-ad7a-42d94add46b9@proxmox.com> (raw)
In-Reply-To: <20260310120705.150425-1-g.goller@proxmox.com>
did comment on a few things on the individual patches, the rest looks
good code-wise!
On 2026-03-10 13:09, Gabriel Goller wrote:
> Previously we generated the frr config using one big perl hash, where every
> controller-plugin and zone-plugin would push their stuff. This is not pretty
> and also tricky to unite with our new rust-based fabrics. Furthermore the only
> way to edit or override the frr config is currently the frr.conf.local file,
> which is merged with the perl-hash in a very janky manner, which has sprouted
> numerous forum threads. The main problem with the frr.conf.local is the limited
> control which the user has as to where the override or additional config gets
> placed. There are also a few config overrides or additions to frr.conf.local
> that are currently impossible or generate invalid frr config.
>
> To improve this we now ship templates, which we use to generate the frr config.
> This is the way it is done in e.g. sonic and vyos. These jinja2 templates are
> then populated using rust-structs. We changed the perl code to generate
> bgp/evpn and isis config that can be deserialized by the rust types and then
> rendered into a frr configuration using the templates.
>
> # Versioning
>
> The templates are in the proxmox-frr-templates debian package which, when
> installed, copies the template into `/usr/share/proxmox-frr/templates`, where
> they are read from using `include_str!`. This means the proxmox-frr-templates
> package is only used for development and to version the templates. The user
> only gets them in the binary of proxmox-frr (which, by extension, is in the
> perl-rs shared library).
>
> # frr.conf.local
>
> The frr.conf.local merging code has been adjusted so that the frr.conf.local
> still works as before.
>
>
> Changelog:
>
> v5 (thanks @Hannes):
> * Use Sys.Audit instead of Sys.Modify on dry-run api call
> * Fix dry-run config reading (the running-config was read twice)
>
> v4 (thanks @Stefan):
> * Fix completely new bgp router (global 'custom_frr_config') not having a 'exit' statement
> * Add more testcases for frr.conf.local merging (vrf, ip-access list, ip prefix-list, etc.)
> * Add more broken testcases (frr_local_merge_router_without_exit)
> * Chang dry-run api method to GET
> * Improv dry-run modal (wrap in panel)
> * Improv commit messages
>
> v3 (thanks @Stefan):
> * Fix frr.conf.local merging (add routers of other protocols, fix routemap
> sequence numbers, error on empty custom_frr_config, various perl style
> changes)
> * Add some more stuff to the frr_local_merge test.
> * Reorder commits so that we have a commit that adds the tests at the
> beginning and then a separate commit that changes the tests
>
> v2 (thanks @Hannes and @Stefan):
> * Removed user overrides (/etc/proxmox-frr/templates/*) and the pvesdn cli
> tool
> * Render frr.conf.local configuration at the correct position in the frr.conf
> (In the middle after the bgp controller)
> * Fix the default setting for `no bgp default ipv4-unicast` in the bgp
> controller
> * Add the ifupdown2 config diff to the dry-run
> * Improve the dry-run diff view in the ui
> * Expose `bgp hard-administrative-reset` and `bgp graceful-restart
> notification`, don't hardcode it in the templates
> * Add more extensive tests in pve-network (cover more evpn/bgp options, add
> bgp-only test)
> * Remove the `bon` dependency and the `Builder` derives (it's not needed right
> now -- maybe we'll add it again later)
> * In pve-network, add a undef check for the routemaps in the
> `fix_routemap_seqs` function
> * Updated implementation of `InterfaceName` -- use less complicated trait
> bounds, improve api
> * Remove obsolete code comments
> * Squash the `phf` commit into the previous one
>
>
> Also thanks to Stefan Hanreich as always :)
>
> proxmox-ve-rs:
>
> Gabriel Goller (8):
> ve-config: firewall: cargo fmt
> frr: add proxmox-frr-templates package that contains templates
> ve-config: remove FrrConfigBuilder struct
> sdn-types: support variable-length NET identifier
> frr: add template serializer and serialize fabrics using templates
> frr: add isis configuration and templates
> frr: support custom frr configuration lines
> frr: add bgp support with templates and serialization
>
> Makefile | 8 +
> proxmox-frr-templates/.gitignore | 1 +
> proxmox-frr-templates/Makefile | 50 +++
> proxmox-frr-templates/debian/changelog | 5 +
> proxmox-frr-templates/debian/control | 17 +
> proxmox-frr-templates/debian/copyright | 18 ++
> .../debian/proxmox-frr-templates.install | 1 +
> proxmox-frr-templates/debian/rules | 5 +
> .../templates/access_list.jinja | 6 +
> .../templates/access_lists.jinja | 6 +
> .../templates/bgp_router.jinja | 128 ++++++++
> proxmox-frr-templates/templates/bgpd.jinja | 35 ++
> proxmox-frr-templates/templates/fabricd.jinja | 29 ++
> .../templates/frr.conf.jinja | 12 +
> .../templates/interface.jinja | 9 +
> .../templates/ip_routes.jinja | 8 +
> proxmox-frr-templates/templates/isisd.jinja | 32 ++
> proxmox-frr-templates/templates/ospfd.jinja | 18 ++
> .../templates/prefix_lists.jinja | 6 +
> .../templates/protocol_routemaps.jinja | 10 +
> .../templates/route_maps.jinja | 20 ++
> proxmox-frr/Cargo.toml | 3 +
> proxmox-frr/debian/control | 12 +
> proxmox-frr/src/ser/bgp.rs | 185 +++++++++++
> proxmox-frr/src/ser/isis.rs | 47 +++
> proxmox-frr/src/ser/mod.rs | 303 +++++++++---------
> proxmox-frr/src/ser/openfabric.rs | 37 +--
> proxmox-frr/src/ser/ospf.rs | 78 +----
> proxmox-frr/src/ser/route_map.rs | 212 ++++--------
> proxmox-frr/src/ser/serializer.rs | 232 +++-----------
> proxmox-sdn-types/src/net.rs | 140 +++++++-
> proxmox-ve-config/src/common/valid.rs | 4 +-
> proxmox-ve-config/src/firewall/cluster.rs | 3 +-
> proxmox-ve-config/src/firewall/types/ipset.rs | 2 +-
> proxmox-ve-config/src/sdn/fabric/frr.rs | 284 +++++++++-------
> proxmox-ve-config/src/sdn/frr.rs | 42 ---
> proxmox-ve-config/src/sdn/mod.rs | 2 -
> proxmox-ve-config/tests/fabric/main.rs | 101 +++---
> .../fabric__openfabric_default_pve.snap | 2 +-
> .../fabric__openfabric_default_pve1.snap | 2 +-
> .../fabric__openfabric_dualstack_pve.snap | 13 +-
> .../fabric__openfabric_ipv6_only_pve.snap | 4 +-
> .../fabric__openfabric_multi_fabric_pve1.snap | 2 +-
> .../snapshots/fabric__ospf_default_pve.snap | 2 +-
> .../snapshots/fabric__ospf_default_pve1.snap | 2 +-
> .../fabric__ospf_multi_fabric_pve1.snap | 2 +-
> 46 files changed, 1331 insertions(+), 809 deletions(-)
> create mode 100644 proxmox-frr-templates/.gitignore
> create mode 100644 proxmox-frr-templates/Makefile
> create mode 100644 proxmox-frr-templates/debian/changelog
> create mode 100644 proxmox-frr-templates/debian/control
> create mode 100644 proxmox-frr-templates/debian/copyright
> create mode 100644 proxmox-frr-templates/debian/proxmox-frr-templates.install
> create mode 100755 proxmox-frr-templates/debian/rules
> create mode 100644 proxmox-frr-templates/templates/access_list.jinja
> create mode 100644 proxmox-frr-templates/templates/access_lists.jinja
> create mode 100644 proxmox-frr-templates/templates/bgp_router.jinja
> create mode 100644 proxmox-frr-templates/templates/bgpd.jinja
> create mode 100644 proxmox-frr-templates/templates/fabricd.jinja
> create mode 100644 proxmox-frr-templates/templates/frr.conf.jinja
> create mode 100644 proxmox-frr-templates/templates/interface.jinja
> create mode 100644 proxmox-frr-templates/templates/ip_routes.jinja
> create mode 100644 proxmox-frr-templates/templates/isisd.jinja
> create mode 100644 proxmox-frr-templates/templates/ospfd.jinja
> create mode 100644 proxmox-frr-templates/templates/prefix_lists.jinja
> create mode 100644 proxmox-frr-templates/templates/protocol_routemaps.jinja
> create mode 100644 proxmox-frr-templates/templates/route_maps.jinja
> create mode 100644 proxmox-frr/src/ser/bgp.rs
> create mode 100644 proxmox-frr/src/ser/isis.rs
> delete mode 100644 proxmox-ve-config/src/sdn/frr.rs
>
>
> proxmox-perl-rs:
>
> Gabriel Goller (1):
> sdn: add function to generate the frr config for all daemons
>
> pve-rs/Makefile | 1 +
> pve-rs/src/bindings/sdn/fabrics.rs | 25 +++----------------------
> pve-rs/src/bindings/sdn/mod.rs | 28 ++++++++++++++++++++++++++++
> 3 files changed, 32 insertions(+), 22 deletions(-)
>
>
> pve-network:
>
> Gabriel Goller (9):
> tests: use Test::Differences to make test assertions
> test: add tests for frr.conf.local merging
> test: bgp: add some various integration tests
> sdn: write structured frr config that can be rendered using templates
> sdn: remove duplicate comment line '!' in frr config
> tests: rearrange some statements in the frr config
> sdn: adjust frr.conf.local merging to rust template types
> test: adjust frr_local_merge test for new template generation
> api: add dry-run endpoint for sdn apply to preview changes
>
> debian/control | 1 +
> src/PVE/API2/Network/SDN.pm | 86 ++++
> src/PVE/Network/SDN.pm | 46 +-
> src/PVE/Network/SDN/Controllers/BgpPlugin.pm | 104 +++--
> src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 378 ++++++++--------
> src/PVE/Network/SDN/Controllers/IsisPlugin.pm | 28 +-
> src/PVE/Network/SDN/Fabrics.pm | 24 +-
> src/PVE/Network/SDN/Frr.pm | 404 +++++++++---------
> src/PVE/Network/SDN/Zones.pm | 3 +-
> src/test/debug/generateconfig.pl | 3 +-
> src/test/run_test_dns.pl | 15 +-
> src/test/run_test_ipams.pl | 13 +-
> src/test/run_test_subnets.pl | 31 +-
> src/test/run_test_vnets_blackbox.pl | 23 +-
> src/test/run_test_zones.pl | 23 +-
> .../expected_controller_config | 1 -
> .../evpn/bgp_ebgp/expected_controller_config | 54 +++
> .../evpn/bgp_ebgp/expected_sdn_interfaces | 41 ++
> src/test/zones/evpn/bgp_ebgp/interfaces | 7 +
> src/test/zones/evpn/bgp_ebgp/sdn_config | 49 +++
> .../expected_controller_config | 67 +++
> .../bgp_ebgp_multihop/expected_sdn_interfaces | 41 ++
> .../zones/evpn/bgp_ebgp_multihop/interfaces | 10 +
> .../zones/evpn/bgp_ebgp_multihop/sdn_config | 51 +++
> .../expected_controller_config | 52 +++
> .../expected_sdn_interfaces | 41 ++
> .../evpn/bgp_ebgp_reverse_order/interfaces | 7 +
> .../evpn/bgp_ebgp_reverse_order/sdn_config | 49 +++
> .../bgp_loopback/expected_controller_config | 65 +++
> .../evpn/bgp_loopback/expected_sdn_interfaces | 41 ++
> src/test/zones/evpn/bgp_loopback/interfaces | 10 +
> src/test/zones/evpn/bgp_loopback/sdn_config | 49 +++
> .../expected_controller_config | 55 +++
> .../expected_sdn_interfaces | 41 ++
> .../zones/evpn/bgp_multipath_relax/interfaces | 7 +
> .../zones/evpn/bgp_multipath_relax/sdn_config | 49 +++
> .../expected_controller_config | 76 ++++
> .../combined_bgp_isis/expected_sdn_interfaces | 41 ++
> .../zones/evpn/combined_bgp_isis/interfaces | 10 +
> .../zones/evpn/combined_bgp_isis/sdn_config | 57 +++
> .../expected_controller_config | 1 -
> .../evpn/ebgp/expected_controller_config | 1 -
> .../ebgp_loopback/expected_controller_config | 3 +-
> .../evpn/ebgp_only/expected_controller_config | 25 ++
> .../evpn/ebgp_only/expected_sdn_interfaces | 1 +
> src/test/zones/evpn/ebgp_only/interfaces | 7 +
> src/test/zones/evpn/ebgp_only/sdn_config | 19 +
> .../evpn/exitnode/expected_controller_config | 1 -
> .../expected_controller_config | 1 -
> .../expected_controller_config | 1 -
> .../exitnode_snat/expected_controller_config | 1 -
> .../expected_controller_config | 1 -
> .../expected_controller_config | 184 ++++++++
> .../frr_local_merge/expected_sdn_interfaces | 53 +++
> .../zones/evpn/frr_local_merge/frr.conf.local | 120 ++++++
> .../zones/evpn/frr_local_merge/interfaces | 7 +
> .../zones/evpn/frr_local_merge/sdn_config | 81 ++++
> .../expected_controller_config | 123 ++++++
> .../expected_sdn_interfaces | 38 ++
> .../frr.conf.local | 90 ++++
> .../interfaces | 0
> .../sdn_config | 53 +++
> .../evpn/ipv4/expected_controller_config | 1 -
> .../evpn/ipv4ipv6/expected_controller_config | 1 -
> .../expected_controller_config | 1 -
> .../evpn/ipv6/expected_controller_config | 1 -
> .../ipv6underlay/expected_controller_config | 1 -
> .../evpn/isis/expected_controller_config | 15 +-
> .../isis_loopback/expected_controller_config | 15 +-
> .../expected_controller_config | 13 +-
> .../expected_controller_config | 3 +-
> .../multiplezones/expected_controller_config | 1 -
> .../expected_controller_config | 13 +-
> .../ospf_fabric/expected_controller_config | 13 +-
> .../evpn/rt_import/expected_controller_config | 1 -
> .../evpn/vxlanport/expected_controller_config | 1 -
> 76 files changed, 2471 insertions(+), 573 deletions(-)
> create mode 100644 src/test/zones/evpn/bgp_ebgp/expected_controller_config
> create mode 100644 src/test/zones/evpn/bgp_ebgp/expected_sdn_interfaces
> create mode 100644 src/test/zones/evpn/bgp_ebgp/interfaces
> create mode 100644 src/test/zones/evpn/bgp_ebgp/sdn_config
> create mode 100644 src/test/zones/evpn/bgp_ebgp_multihop/expected_controller_config
> create mode 100644 src/test/zones/evpn/bgp_ebgp_multihop/expected_sdn_interfaces
> create mode 100644 src/test/zones/evpn/bgp_ebgp_multihop/interfaces
> create mode 100644 src/test/zones/evpn/bgp_ebgp_multihop/sdn_config
> create mode 100644 src/test/zones/evpn/bgp_ebgp_reverse_order/expected_controller_config
> create mode 100644 src/test/zones/evpn/bgp_ebgp_reverse_order/expected_sdn_interfaces
> create mode 100644 src/test/zones/evpn/bgp_ebgp_reverse_order/interfaces
> create mode 100644 src/test/zones/evpn/bgp_ebgp_reverse_order/sdn_config
> create mode 100644 src/test/zones/evpn/bgp_loopback/expected_controller_config
> create mode 100644 src/test/zones/evpn/bgp_loopback/expected_sdn_interfaces
> create mode 100644 src/test/zones/evpn/bgp_loopback/interfaces
> create mode 100644 src/test/zones/evpn/bgp_loopback/sdn_config
> create mode 100644 src/test/zones/evpn/bgp_multipath_relax/expected_controller_config
> create mode 100644 src/test/zones/evpn/bgp_multipath_relax/expected_sdn_interfaces
> create mode 100644 src/test/zones/evpn/bgp_multipath_relax/interfaces
> create mode 100644 src/test/zones/evpn/bgp_multipath_relax/sdn_config
> create mode 100644 src/test/zones/evpn/combined_bgp_isis/expected_controller_config
> create mode 100644 src/test/zones/evpn/combined_bgp_isis/expected_sdn_interfaces
> create mode 100644 src/test/zones/evpn/combined_bgp_isis/interfaces
> create mode 100644 src/test/zones/evpn/combined_bgp_isis/sdn_config
> create mode 100644 src/test/zones/evpn/ebgp_only/expected_controller_config
> create mode 100644 src/test/zones/evpn/ebgp_only/expected_sdn_interfaces
> create mode 100644 src/test/zones/evpn/ebgp_only/interfaces
> create mode 100644 src/test/zones/evpn/ebgp_only/sdn_config
> create mode 100644 src/test/zones/evpn/frr_local_merge/expected_controller_config
> create mode 100644 src/test/zones/evpn/frr_local_merge/expected_sdn_interfaces
> create mode 100644 src/test/zones/evpn/frr_local_merge/frr.conf.local
> create mode 100644 src/test/zones/evpn/frr_local_merge/interfaces
> create mode 100644 src/test/zones/evpn/frr_local_merge/sdn_config
> create mode 100644 src/test/zones/evpn/frr_local_merge_router_without_exit/expected_controller_config
> create mode 100644 src/test/zones/evpn/frr_local_merge_router_without_exit/expected_sdn_interfaces
> create mode 100644 src/test/zones/evpn/frr_local_merge_router_without_exit/frr.conf.local
> create mode 100644 src/test/zones/evpn/frr_local_merge_router_without_exit/interfaces
> create mode 100644 src/test/zones/evpn/frr_local_merge_router_without_exit/sdn_config
>
>
> pve-manager:
>
> Gabriel Goller (1):
> sdn: add dry-run diff view for sdn apply
>
> www/manager6/Makefile | 1 +
> www/manager6/sdn/SdnDiffView.js | 149 ++++++++++++++++++++++++++++++++
> www/manager6/sdn/StatusView.js | 8 ++
> 3 files changed, 158 insertions(+)
> create mode 100644 www/manager6/sdn/SdnDiffView.js
>
>
> Summary over all repositories:
> 128 files changed, 3992 insertions(+), 1404 deletions(-)
>
next prev parent reply other threads:[~2026-03-12 13:35 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-10 12:06 Gabriel Goller
2026-03-10 12:06 ` [PATCH proxmox-ve-rs v5 1/8] ve-config: firewall: cargo fmt Gabriel Goller
2026-03-10 12:06 ` [PATCH proxmox-ve-rs v5 2/8] frr: add proxmox-frr-templates package that contains templates Gabriel Goller
2026-03-12 8:01 ` Hannes Laimer
2026-03-12 10:03 ` Gabriel Goller
2026-03-12 8:19 ` Hannes Laimer
2026-03-12 9:30 ` Gabriel Goller
2026-03-12 8:33 ` Hannes Laimer
2026-03-12 9:31 ` Gabriel Goller
2026-03-10 12:06 ` [PATCH proxmox-ve-rs v5 3/8] ve-config: remove FrrConfigBuilder struct Gabriel Goller
2026-03-10 12:06 ` [PATCH proxmox-ve-rs v5 4/8] sdn-types: support variable-length NET identifier Gabriel Goller
2026-03-10 12:06 ` [PATCH proxmox-ve-rs v5 5/8] frr: add template serializer and serialize fabrics using templates Gabriel Goller
2026-03-12 8:44 ` Hannes Laimer
2026-03-12 10:07 ` Gabriel Goller
2026-03-12 9:12 ` Hannes Laimer
2026-03-12 10:11 ` Gabriel Goller
2026-03-10 12:06 ` [PATCH proxmox-ve-rs v5 6/8] frr: add isis configuration and templates Gabriel Goller
2026-03-10 12:06 ` [PATCH proxmox-ve-rs v5 7/8] frr: support custom frr configuration lines Gabriel Goller
2026-03-10 12:06 ` [PATCH proxmox-ve-rs v5 8/8] frr: add bgp support with templates and serialization Gabriel Goller
2026-03-12 8:30 ` Hannes Laimer
2026-03-12 9:35 ` Gabriel Goller
2026-03-10 12:06 ` [PATCH proxmox-perl-rs v5 1/1] sdn: add function to generate the frr config for all daemons Gabriel Goller
2026-03-10 12:06 ` [PATCH pve-network v5 1/9] tests: use Test::Differences to make test assertions Gabriel Goller
2026-03-10 12:06 ` [PATCH pve-network v5 2/9] test: add tests for frr.conf.local merging Gabriel Goller
2026-03-10 12:06 ` [PATCH pve-network v5 3/9] test: bgp: add some various integration tests Gabriel Goller
2026-03-10 12:06 ` [PATCH pve-network v5 4/9] sdn: write structured frr config that can be rendered using templates Gabriel Goller
2026-03-12 9:20 ` Hannes Laimer
2026-03-12 10:20 ` Gabriel Goller
2026-03-10 12:06 ` [PATCH pve-network v5 5/9] sdn: remove duplicate comment line '!' in frr config Gabriel Goller
2026-03-10 12:06 ` [PATCH pve-network v5 6/9] tests: rearrange some statements in the " Gabriel Goller
2026-03-10 12:06 ` [PATCH pve-network v5 7/9] sdn: adjust frr.conf.local merging to rust template types Gabriel Goller
2026-03-10 12:06 ` [PATCH pve-network v5 8/9] test: adjust frr_local_merge test for new template generation Gabriel Goller
2026-03-10 12:06 ` [PATCH pve-network v5 9/9] api: add dry-run endpoint for sdn apply to preview changes Gabriel Goller
2026-03-10 12:06 ` [PATCH pve-manager v5 1/1] sdn: add dry-run diff view for sdn apply Gabriel Goller
2026-03-12 7:58 ` Hannes Laimer
2026-03-12 10:22 ` Gabriel Goller
2026-03-11 17:22 ` [PATCH manager/network/proxmox{-ve-rs,-perl-rs} v5 00/19] Generate frr config using jinja templates and rust types Stefan Hanreich
2026-03-12 13:35 ` Hannes Laimer [this message]
2026-03-12 14:28 ` Gabriel Goller
2026-03-12 14:31 ` 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=f279aac9-7006-4676-ad7a-42d94add46b9@proxmox.com \
--to=h.laimer@proxmox.com \
--cc=g.goller@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