Re: [PATCH proxmox-ve-rs v7 05/21] frr: add template serializer and serialize fabrics using templates

[Gabriel Goller <g.goller@proxmox.com>]

On 27.03.2026 01:37, Thomas Lamprecht wrote:
> > +++ b/proxmox-frr/src/ser/openfabric.rs
> > +impl std::fmt::Display for OpenfabricRouterName {
> > +    fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
> > +        self.0.fmt(f)
> 
> This calls Debug::fmt now, since FrrWord's Display impl was removed earlier
> in this patch. Result: format!("{}", name) gives FrrWord("fabric1") instead
> of "fabric1".
> 
> The old code was `write!(f, "openfabric {}", self.0)` which used FrrWord's
> Display (just the inner string). Now that Display is gone, self.0.fmt(f)
> resolves to Debug::fmt.
> 
> Templates use Serialize so config generation isn't affected, but anything
> using Display (logging, errors) gets garbage. Simplest fix: use
> write!(f, "{}", self.0.as_ref()) or re-add a trivial Display to FrrWord.

Added `write!(f, "{}", self.0.as_ref())`.

> > +++ b/proxmox-frr/src/ser/route_map.rs
> > +#[serde(tag = "protocol_type")]
> > +pub enum RouteMapMatch {
> > +    #[serde(rename = "ip")]
> > +    V4(RouteMapMatchInner),
> > +    #[serde(rename = "ipv6")]
> > +    V6(RouteMapMatchInner),
> > +    #[serde(rename = "vni")]
> > +    Vni(u32),
> 
> #[serde(tag = "...")] (internally tagged) on a variant wrapping a bare u32
> cannot really work as serde can't inject the tag field into a primitive.
> This will panic at serialization time IIRC.
> 
> btw. The variant also has no corresponding branch in route_maps.jinja (just
> in case I forget to write it there).
>                                  
> Since it's unreachable from the Perl side today, nothing is broken, but
> if it's not needed then I'd suggest removing it until fully wired up, or
> switching  to Vni(VniMatch) (the struct already exists) (and adding a template  
> branch in the jinja template).

Oh, you're right. This is horrible code anyway, Stefan makes this a lot better
in his new route-map series, so I'll just remove the Vni case here.

> > +pub enum SetIpNextHopValue {
> 
> > +pub enum SetTagValue {
> 
> > +pub struct VniMatch {
>                                  
> These three types plus AccessList and PrefixList (the wrapper structs) seem
> to be unused - FrrConfig uses BTreeMap<Name, Vec<Rule>> directly now.

See above, I'll remove these three as well.

Thanks for the review!