From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 7AA00C1A8B for ; Wed, 17 Jan 2024 10:51:04 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5CCDB3FE50 for ; Wed, 17 Jan 2024 10:50:34 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 17 Jan 2024 10:50:33 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 6283949196 for ; Wed, 17 Jan 2024 10:50:33 +0100 (CET) Message-ID: <80251063-623f-4162-960c-963a0984a7f3@proxmox.com> Date: Wed, 17 Jan 2024 10:50:32 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Lukas Wagner To: Proxmox Backup Server development discussion , Stefan Lendl References: <20240111155303.1072675-1-s.lendl@proxmox.com> <20240111155303.1072675-15-s.lendl@proxmox.com> Content-Language: de-AT, en-US In-Reply-To: <20240111155303.1072675-15-s.lendl@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.004 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 T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pbs-devel] [PATCH proxmox-backup 07/10] api: create and update vlan interfaces X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Jan 2024 09:51:04 -0000 On 1/11/24 16:53, Stefan Lendl wrote: > * Implement setting vlan-id and vlan-raw-device in create_ and update_interface. > * Checking if vlan-raw-device exists > * Moved VLAN_INTERFACE_REGEX to top level network module to use in > checking functions there. Changed to match with named capture groups. > * Unit tests to verify parsing vlan_id and vlan_raw_device from name > > Signed-off-by: Stefan Lendl > --- > pbs-config/src/network/mod.rs | 35 +++++++++++++++++++ > pbs-config/src/network/parser.rs | 3 +- > src/api2/node/network.rs | 58 +++++++++++++++++++++++++++++++- > 3 files changed, 94 insertions(+), 2 deletions(-) > > diff --git a/pbs-config/src/network/mod.rs b/pbs-config/src/network/mod.rs > index 9ecc66b8..bc49aded 100644 > --- a/pbs-config/src/network/mod.rs > +++ b/pbs-config/src/network/mod.rs > @@ -25,6 +25,8 @@ use crate::{open_backup_lockfile, BackupLockGuard}; > > lazy_static! { > static ref PHYSICAL_NIC_REGEX: Regex = Regex::new(r"^(?:eth\d+|en[^:.]+|ib\d+)$").unwrap(); > + static ref VLAN_INTERFACE_REGEX: Regex = > + Regex::new(r"^(?P\S+)\.(?P\d+)|vlan(?P\d+)$").unwrap(); > } > > pub fn is_physical_nic(iface: &str) -> bool { > @@ -41,6 +43,21 @@ pub fn bond_xmit_hash_policy_from_str(s: &str) -> Result .map_err(|_: value::Error| format_err!("invalid bond_xmit_hash_policy '{}'", s)) > } > > +pub fn parse_vlan_id_from_name(iface_name: &str) -> Option { > + VLAN_INTERFACE_REGEX.captures(iface_name).and_then(|cap| { > + cap.name("vlan_id") > + .or(cap.name("vlan_id2")) > + .and_then(|id| id.as_str().parse::().ok()) > + }) > +} > + > +pub fn parse_vlan_raw_device_from_name(iface_name: &str) -> Option<&str> { > + VLAN_INTERFACE_REGEX > + .captures(iface_name) > + .and_then(|cap| cap.name("vlan_raw_device")) > + .map(Into::into) > +} > + > // Write attributes not depending on address family > fn write_iface_attributes(iface: &Interface, w: &mut dyn Write) -> Result<(), Error> { > static EMPTY_LIST: Vec = Vec::new(); > @@ -652,4 +669,22 @@ iface individual_name inet manual > .trim() > ); > } > + > + #[test] > + fn test_vlan_parse_vlan_id_from_name() { > + assert_eq!(parse_vlan_id_from_name("vlan100"), Some(100)); > + assert_eq!(parse_vlan_id_from_name("vlan"), None); > + assert_eq!(parse_vlan_id_from_name("arbitrary"), None); > + assert_eq!(parse_vlan_id_from_name("vmbr0.100"), Some(100)); > + assert_eq!(parse_vlan_id_from_name("vmbr0"), None); > + // assert_eq!(parse_vlan_id_from_name("vmbr0.1.400"), Some(400)); // NOTE ifupdown2 does actually support this > + } > + > + #[test] > + fn test_vlan_parse_vlan_raw_device_from_name() { > + assert_eq!(parse_vlan_raw_device_from_name("vlan100"), None); > + assert_eq!(parse_vlan_raw_device_from_name("arbitrary"), None); > + assert_eq!(parse_vlan_raw_device_from_name("vmbr0"), None); > + assert_eq!(parse_vlan_raw_device_from_name("vmbr0.200"), Some("vmbr0")); > + } > } > diff --git a/pbs-config/src/network/parser.rs b/pbs-config/src/network/parser.rs > index 0c178d9b..d66267b3 100644 > --- a/pbs-config/src/network/parser.rs > +++ b/pbs-config/src/network/parser.rs > @@ -1,3 +1,5 @@ > +use crate::network::VLAN_INTERFACE_REGEX; > + > use std::collections::{HashMap, HashSet}; > use std::io::BufRead; > use std::iter::{Iterator, Peekable}; > @@ -536,7 +538,6 @@ impl NetworkParser { > > lazy_static! { > static ref INTERFACE_ALIAS_REGEX: Regex = Regex::new(r"^\S+:\d+$").unwrap(); > - static ref VLAN_INTERFACE_REGEX: Regex = Regex::new(r"^\S+\.\d+|vlan\d+$").unwrap(); > } > > if let Some(existing_interfaces) = existing_interfaces { > diff --git a/src/api2/node/network.rs b/src/api2/node/network.rs > index 187b27a0..d1393103 100644 > --- a/src/api2/node/network.rs > +++ b/src/api2/node/network.rs > @@ -12,7 +12,9 @@ use pbs_api_types::{ > NETWORK_INTERFACE_ARRAY_SCHEMA, NETWORK_INTERFACE_LIST_SCHEMA, NETWORK_INTERFACE_NAME_SCHEMA, > NODE_SCHEMA, PRIV_SYS_AUDIT, PRIV_SYS_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA, > }; > -use pbs_config::network::{self, NetworkConfig}; > +use pbs_config::network::{ > + self, parse_vlan_id_from_name, parse_vlan_raw_device_from_name, NetworkConfig, > +}; > > use proxmox_rest_server::WorkerTask; > > @@ -231,6 +233,15 @@ pub fn read_interface(iface: String) -> Result { > type: bool, > optional: true, > }, > + "vlan-id": { > + description: "VLAN ID.", > + type: u16, > + optional: true, > + }, > + "vlan-raw-device": { > + schema: NETWORK_INTERFACE_NAME_SCHEMA, > + optional: true, > + }, > bond_mode: { > type: LinuxBondMode, > optional: true, > @@ -269,6 +280,8 @@ pub fn create_interface( > mtu: Option, > bridge_ports: Option, > bridge_vlan_aware: Option, > + vlan_id: Option, > + vlan_raw_device: Option, > bond_mode: Option, > bond_primary: Option, > bond_xmit_hash_policy: Option, > @@ -373,6 +386,22 @@ pub fn create_interface( > set_bond_slaves(&mut interface, slaves)?; > } > } > + NetworkInterfaceType::Vlan => { > + if vlan_id.is_none() && parse_vlan_id_from_name(&iface).is_none() { > + bail!("vlan-id must be set"); > + } > + interface.vlan_id = vlan_id; > + > + if let Some(dev) = vlan_raw_device > + .as_deref() > + .or_else(|| parse_vlan_raw_device_from_name(&iface)) > + { > + if !config.interfaces.contains_key(dev) { > + bail!("vlan-raw-device {dev} does not exist"); > + } > + } > + interface.vlan_raw_device = vlan_raw_device; > + } > _ => bail!( > "creating network interface type '{:?}' is not supported", > interface_type > @@ -507,6 +536,15 @@ pub enum DeletableProperty { > type: bool, > optional: true, > }, > + "vlan-id": { > + description: "VLAN ID.", > + type: u16, > + optional: true, > + }, > + "vlan-raw-device": { > + schema: NETWORK_INTERFACE_NAME_SCHEMA, > + optional: true, > + }, > bond_mode: { > type: LinuxBondMode, > optional: true, > @@ -557,6 +595,8 @@ pub fn update_interface( > mtu: Option, > bridge_ports: Option, > bridge_vlan_aware: Option, > + vlan_id: Option, > + vlan_raw_device: Option, > bond_mode: Option, > bond_primary: Option, > bond_xmit_hash_policy: Option, > @@ -581,6 +621,19 @@ pub fn update_interface( > check_duplicate_gateway_v6(&config, &iface)?; > } > > + if let Some(dev) = vlan_raw_device > + .as_deref() > + .or_else(|| parse_vlan_raw_device_from_name(&iface)) > + { > + if !config.interfaces.contains_key(dev) { > + bail!("vlan-raw-device {dev} does not exist"); > + } > + } > + > + if vlan_id.is_none() && parse_vlan_id_from_name(&iface).is_none() { > + bail!("vlan-id must be set"); > + } This seems to break updating existing interfaces/bridge. E.g. when updating 'comment' for a bridge/interface, this seems to be triggered. I guess you would need to check first if this is actually a VLAN device? > + > let interface = config.lookup_mut(&iface)?; > > if let Some(interface_type) = param.get("type") { > @@ -734,6 +787,9 @@ pub fn update_interface( > interface.method6 = Some(NetworkConfigMethod::Manual); > } > > + interface.vlan_id = vlan_id; > + interface.vlan_raw_device = vlan_raw_device; > + > network::save_config(&config)?; > > Ok(()) -- - Lukas