Re: [PATCH manager/network/proxmox-ve-rs 0/9] Implement route redistribution for OSPF

Gabriel Goller
2026-05-07T11:58:25Z

LGTM!
Just a few things:

Setting a route-map on a redistribution didn't work, the api property was
missing:
(pve-network)

diff --git a/src/PVE/Network/SDN/Fabrics.pm b/src/PVE/Network/SDN/Fabrics.pm
index d5ab808c3a..f7140187ad 100644
--- a/src/PVE/Network/SDN/Fabrics.pm
+++ b/src/PVE/Network/SDN/Fabrics.pm
@@ -313,6 +313,12 @@
'static',
],
},
+ route_map => {
+ type => 'string',
+ description =>
+ 'The route-map to filter the redistribution.',
+ optional => 1,
+ },
},
},
optional => 1,

The route_map property also has a wrong serde rename_all type (proxmox-ve-rs):

diff --git a/proxmox-ve-config/src/sdn/fabric/section_config/protocol/ospf.rs b/proxmox-ve-config/src/sdn/fabric/section_config/protocol/ospf.rs
index 2e1f37b278..66297993fe 100644
--- a/proxmox-ve-config/src/sdn/fabric/section_config/protocol/ospf.rs
+++ b/proxmox-ve-config/src/sdn/fabric/section_config/protocol/ospf.rs
@@ -36,7 +36,7 @@

#[api]
#[derive(Debug, Clone, Serialize, Deserialize, Hash)]
-#[serde(rename_all = "kebab-case")]
+#[serde(rename_all = "lowercase")]
/// An OSPF redistribution
pub struct OspfRedistribution {
/// The source protocol for this redistribution

There is also an additional whitespace when specifying a route-map:

diff --git a/proxmox-frr-templates/templates/ospfd.jinja b/proxmox-frr-templates/templates/ospfd.jinja
index 4ba609f1e5..427ed7f9dd 100644
--- a/proxmox-frr-templates/templates/ospfd.jinja
+++ b/proxmox-frr-templates/templates/ospfd.jinja
@@ -4,7 +4,7 @@
router ospf
ospf router-id {{ ospf.router.router_id }}
{% for redistribution in ospf.router.redistribute %}
- redistribute {{ redistribution.source }}{% if redistribution.metric is defined %} metric {{ redistribution.metric }}{% endif %} {% if redistribution.route_map is defined %} route-map {{ redistribution.route_map }}{% endif %}
+ redistribute {{ redistribution.source }}{% if redistribution.metric is defined %} metric {{ redistribution.metric }}{% endif %}{% if redistribution.route_map is defined %} route-map {{ redistribution.route_map }}{% endif %}

{% endfor %}
exit

And in pve-manager, IMO it looks better when the "Add" button is at the bottom of the table:

diff --git a/www/manager6/sdn/fabrics/RedistributionGrid.js b/www/manager6/sdn/fabrics/RedistributionGrid.js
index 8580c47939..cb7c374d45 100644
--- a/www/manager6/sdn/fabrics/RedistributionGrid.js
+++ b/www/manager6/sdn/fabrics/RedistributionGrid.js
@@ -20,7 +20,7 @@
}
},

- tbar: [
+ bbar: [
'->',
{
text: gettext('Add'),

Another small thing: I'm not sure if "static" is a bit confusing for the users,
because it refers to routes from the "staticd" daemon in frr and not static
routes on the host. But I can't think of a better name to be honest ...

The rest works great!
Tested redistributing every type of routes and tested filtering them by route-maps.