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 34FFB1FF13A for ; Wed, 13 May 2026 15:01:26 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id CAAC8CE96; Wed, 13 May 2026 15:01:23 +0200 (CEST) Message-ID: Date: Wed, 13 May 2026 15:00:47 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox-ve-rs v4 1/7] sdn: fabric: add BGP protocol support To: Stefan Hanreich , pve-devel@lists.proxmox.com References: <20260512141305.199664-1-h.laimer@proxmox.com> <20260512141305.199664-2-h.laimer@proxmox.com> From: Hannes Laimer Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1778677244347 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.081 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: KKTAEH6NEZPQOLQ4INFP57SDBBOYBKIM X-Message-ID-Hash: KKTAEH6NEZPQOLQ4INFP57SDBBOYBKIM X-MailFrom: h.laimer@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 X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: thanks, will address in v5! On 2026-05-13 14:30, Stefan Hanreich wrote: > On 5/12/26 4:12 PM, Hannes Laimer wrote: >> +impl AddressFamilies { >> + /// Merge another [`AddressFamilies`] into this one. >> + /// >> + /// For each address family: if `self` already has it, extend its neighbors, networks, and >> + /// redistribute lists. If `self` doesn't have it, take it from `other`. >> + pub fn merge(&mut self, other: AddressFamilies) { > > maybe extend is the better name, as it mirrors existing conventions from > std::Vec? > makes sense, can do [..] >> } >> FabricEntry::WireGuard(_) => {} // not a frr fabric >> + FabricEntry::Bgp(bgp_entry) => { >> + let Ok(node) = bgp_entry.node_section(¤t_node) else { >> + continue; >> + }; >> + >> + let BgpNode::Internal(properties) = node.properties() else { >> + continue; >> + }; >> + >> + let fabric = bgp_entry.fabric_section(); >> + >> + let local_asn = properties.asn().as_u32(); > > makes me wonder if implementing AsRef would simplify handling ASN > throughout the code? > good point, probably also `Deref` [..] >> + >> +#[api( >> + type: Integer, >> + minimum: u32::MIN as i64, >> + maximum: u32::MAX as i64, >> +)] >> +#[derive(Debug, Clone, Serialize, Updater, Hash)] >> +/// Autonomous system number as defined by RFC 6793 >> +pub struct ASN(u32); > > potentially something for later, but I think we use ASN in several > places - so we might want to add this to sdn-types instead and reuse it > across the ve-rs crates? > yes, sounds like a good follow-up that covers all places then [..] >> +impl<'de> Deserialize<'de> for ASN { >> + fn deserialize(deserializer: D) -> Result >> + where >> + D: serde::Deserializer<'de>, >> + { >> + use serde::de::{self, Visitor}; >> + >> + struct AsnVisitor; >> + >> + impl<'de> Visitor<'de> for AsnVisitor { >> + type Value = ASN; >> + >> + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { >> + formatter.write_str("a u32 or string containing a u32") >> + } >> + >> + fn visit_i64(self, v: i64) -> Result { >> + u32::try_from(v) >> + .map(ASN) >> + .map_err(|_| E::custom(format!("ASN out of range: {v}"))) >> + } >> + >> + fn visit_u64(self, v: u64) -> Result { >> + u32::try_from(v) >> + .map(ASN) >> + .map_err(|_| E::custom(format!("ASN out of range: {v}"))) >> + } >> + >> + fn visit_str(self, v: &str) -> Result { >> + v.parse::() >> + .map(ASN) >> + .map_err(|_| E::custom(format!("invalid ASN: {v}"))) >> + } >> + } >> + >> + deserializer.deserialize_any(AsnVisitor) >> + } >> +} > > Is there a reason why proxmox_serde::perl::deserialize_u32 doesn't work? > It should work the same afaict. > i remeber there was a problem with the information that this is an int being lost when passed through perlmod, but i tried to reproduce this again and could not. so no reason :) [..]