On 2/25/26 5:29 PM, Gabriel Goller wrote:
> On 25.02.2026 14:43, Stefan Hanreich wrote:
>> A lot of structs here derive Builder, but we don't use any of the
>> generated builder functionality atm and just instantiate the structs
>> directly in ve-config since all the properties are pub. We should imo
>> settle on one approach, preferably now, and then use that consistently.
>>
>> We use many collection/map types here, and with bon we'd have to write
>> some boilerplate for those [1] and add validation for e.g. duplicate
>> route_maps [2]. In the case of the BGP router this could become quite
>> tricky imo, since for instance the value of ASN / remote-as / .. depends
>> on whether we have a BGP controller / fabric / ...
>>
>> Additionally, we're creating an instance of FrrConfig in the Perl part,
>> by just building the hash in a way that serializes / deserializes into
>> the FrrConfig in Rust. With the builder pattern we'd have to expose the
>> builder functionality to the perl side somehow. I think directly
>> instantiating/mutating + (de)serializing is the better way here for that
>> reason, since we get a partial FrrConfig from Perl already and then just
>> incrementally add stuff in Rust. So it'd make sense to remove all
>> Builder functionality from here for now to save on dependencies /
>> compile-time?
>>
>> For future additions, e.g. route-maps, we could utilize the Builder
>> pattern, since it is entirely contained to Rust and would make sense
>> there - but I'd then introduce it in that patch series.
>>
>> [1] https://bon-rs.com/guide/typestate-api/builder-fields#custom-fields
>> [2] https://bon-rs.com/guide/patterns/fallible-builders#fallible-builders
>
> Good point -- I'm definitely going through and removing the derive where it
> isn't needed.
>
> As discussed offlist, this is not super-urgent as the api of proxmox-frr isn't
> really stable or critical or something. Nevertheless we should check how much
> the compilation-time overhead is for deriving bon for every struct, as bon is
> not really lighweight (uses the type-state builder pattern).
>
> We also need to check if we want to add some validation here or if we don't
> need
> that. If we need some validation, I'd go with bon, because it's a uniform
> method
> of doing this, instead of mixing new() methods and TryInto,TryFrom
> implementations, etc.
> Quickly went over the build_fabrics code and didn't notice any struct that
> needs
> "different" validations, so e.g. sometimes append, sometimes overwrite (so bon
> would still work and we wouldn't need two creation methods).
>
> Don't know if we should clean this up and continue with bon, or rip it out
> (which wouldn't be much work because we don't use it that heavy) and maybe add
> this later on.
>
> Open for some other arguments or opinions!
Had some further off-list discussion and settled on the current
approach, without utilizing bon::Builder for now - mostly because it's
hard to expose for the Perl side atm and we build part of the FRR
configuration there still.
