From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 36BF51FF13C for ; Thu, 02 Apr 2026 15:36:15 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C625D1B05E; Thu, 2 Apr 2026 15:36:44 +0200 (CEST) Date: Thu, 2 Apr 2026 15:36:02 +0200 From: Wolfgang Bumiller To: Stefan Hanreich Subject: Re: [PATCH proxmox-ve-rs v2 05/34] sdn-types: add common route-map helper types Message-ID: References: <20260401143957.386809-1-s.hanreich@proxmox.com> <20260401143957.386809-6-s.hanreich@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260401143957.386809-6-s.hanreich@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1775136904145 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.088 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: B3LN6VFZXMQL7QTNZBHI7ZDQ4B4CKTEX X-Message-ID-Hash: B3LN6VFZXMQL7QTNZBHI7ZDQ4B4CKTEX X-MailFrom: w.bumiller@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: pve-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Wed, Apr 01, 2026 at 04:39:14PM +0200, Stefan Hanreich wrote: > The reason for including those types here is that they are used in > proxmox-frr for generating the FRR configuration as well as > proxmox-ve-config for utilizing them in the section config types. > > For some values in route maps FRR supports specifying either an > absolute value or a value relative to the existing value. When > modifying the metric of a route via a route map, '123' would set the > value to 123, whereas '+/-123' would add/subtract 123 from the > existing value. IntegerWithSign can be used to represent such a value > in the section config. > > A custom deserializer is implemented, so primitive numerical values > can be used in addition to strings that contain the respective signs. > They deserialize into absolute values. > > Signed-off-by: Stefan Hanreich > --- > proxmox-sdn-types/src/bgp.rs | 62 ++++++++++++ > proxmox-sdn-types/src/lib.rs | 179 +++++++++++++++++++++++++++++++++++ > 2 files changed, 241 insertions(+) > create mode 100644 proxmox-sdn-types/src/bgp.rs > > diff --git a/proxmox-sdn-types/src/bgp.rs b/proxmox-sdn-types/src/bgp.rs > new file mode 100644 > index 0000000..6df3022 > --- /dev/null > +++ b/proxmox-sdn-types/src/bgp.rs > @@ -0,0 +1,62 @@ > +use serde::{Deserialize, Serialize}; > + > +use crate::IntegerWithSign; > + > +/// Represents a BGP metric value, as used in FRR. > +/// > +/// A metric can either be a numeric value, or certain 'magic' values. For more information see the > +/// respective enum variants. > +#[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq)] > +pub enum SetMetricValue { > + /// Set the metric to the round-trip-time. > + #[serde(rename = "rtt")] > + Rtt, > + /// Add the round-trip-time to the metric. > + #[serde(rename = "+rtt")] > + AddRtt, > + /// Subtract the round-trip-time from the metric. > + #[serde(rename = "-rtt")] > + SubtractRtt, > + /// Use the IGP value when importing from another IGP. > + #[serde(rename = "igp")] > + Igp, > + /// Use the accumulated IGP value when importing from another IGP. > + #[serde(rename = "aigp")] > + Aigp, > + /// Set the metric to a fixed numeric value. > + #[serde(untagged)] > + Numeric(IntegerWithSign), > +} > + > +impl> From for SetMetricValue { > + fn from(value: T) -> Self { > + Self::Numeric(value.into()) > + } > +} > + > +/// An EVPN route-type, as used in the FRR route maps. > +#[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq)] > +#[serde(rename_all = "lowercase")] > +pub enum EvpnRouteType { > + Ead, > + MacIp, > + Multicast, > + #[serde(rename = "es")] > + EthernetSegment, > + Prefix, > +} > + > +/// An tag value, as used in the FRR route maps. > +#[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq)] > +#[serde(rename_all = "lowercase")] > +pub enum SetTagValue { > + Untagged, > + #[serde(untagged)] > + Numeric(#[serde(deserialize_with = "proxmox_serde::perl::deserialize_u32")] u32), > +} > + > +impl SetTagValue { > + pub fn new(value: u32) -> Self { > + SetTagValue::Numeric(value) > + } > +} > diff --git a/proxmox-sdn-types/src/lib.rs b/proxmox-sdn-types/src/lib.rs > index 1656f1d..9795857 100644 > --- a/proxmox-sdn-types/src/lib.rs > +++ b/proxmox-sdn-types/src/lib.rs > @@ -1,3 +1,182 @@ > pub mod area; > +pub mod bgp; > pub mod net; > pub mod openfabric; > + > +use serde::de::{Error, Visitor}; > +use serde::{Deserialize, Serialize}; > + > +use proxmox_schema::api; > + > +/// Enum for representing signedness of Integer in [`IntegerWithSign`]. > +#[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq)] > +pub enum Sign { > + #[serde(rename = "-")] > + Negative, > + #[serde(rename = "+")] > + Positive, > +} > + > +proxmox_serde::forward_display_to_serialize!(Sign); > +proxmox_serde::forward_from_str_to_deserialize!(Sign); > + > +/// An Integer with an optional [`Sign`]. > +/// > +/// This is used for representing certain keys in the FRR route maps (e.g. metric). They can be set > +/// to either a static value (no sign) or to a value relative to the existing value (with sign). > +/// For instance, a value of 50 would set the metric to 50, but a value of +50 would add 50 to the > +/// existing metric value. > +#[derive(Debug, Clone, Eq, PartialEq)] > +pub struct IntegerWithSign { > + pub(crate) sign: Option, > + pub(crate) n: u32, > +} > + Summary of a short off-list discussion: - I found the name somewhat arbitrary (and the `Sign` part is optional) - It's supposed to say "Set to X" or "Modify *by* X", so it's probably nicer to have an `enum ModifyNumber { Absolute(u32), Relative(i32) }` (name and exact types to be determined) so the name and content actually reflect the description found in the doc comment. Also, since this comes from the API and flows through perl (integer/string confusion) and the leading plus sign is significant here, it should be an only-string type (which means the `Deserialize` impl can be forwarded to `FromStr` and the serde `Visitor` boiler plate can be dropped).