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(-) >
