From 256c076c75defb21983aef09d44b741ae67777ea Mon Sep 17 00:00:00 2001 From: Clemens Famulla-Conrad Date: Tue, 26 Sep 2023 14:03:35 +0200 Subject: [PATCH 01/58] Add Bonding model --- rust/agama-dbus-server/src/network/error.rs | 2 + rust/agama-dbus-server/src/network/model.rs | 53 ++++++++++++++++++++- rust/agama-lib/src/network/types.rs | 2 + 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/rust/agama-dbus-server/src/network/error.rs b/rust/agama-dbus-server/src/network/error.rs index 8c62c05f24..06bafcf826 100644 --- a/rust/agama-dbus-server/src/network/error.rs +++ b/rust/agama-dbus-server/src/network/error.rs @@ -16,6 +16,8 @@ pub enum NetworkStateError { #[error("Invalid wireless mode: '{0}'")] InvalidWirelessMode(String), #[error("Connection '{0}' already exists")] + UnknownParentKind(String), + #[error("Connection '{0}' already exists")] ConnectionExists(Uuid), #[error("Invalid security wireless protocol: '{0}'")] InvalidSecurityProtocol(String), diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 88a5416180..bccbabc32d 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -10,7 +10,7 @@ use std::{ default::Default, fmt, net::IpAddr, - str::{self, FromStr}, + str::{self, FromStr}, collections::HashMap, }; use thiserror::Error; use uuid::Uuid; @@ -55,6 +55,7 @@ impl NetworkState { self.connections.iter_mut().find(|c| c.id() == id) } + /// Adds a new connection. /// /// It uses the `id` to decide whether the connection already exists. @@ -221,6 +222,7 @@ pub enum Connection { Ethernet(EthernetConnection), Wireless(WirelessConnection), Loopback(LoopbackConnection), + Bond(BondConnection), } impl Connection { @@ -236,6 +238,10 @@ impl Connection { }), DeviceType::Loopback => Connection::Loopback(LoopbackConnection { base }), DeviceType::Ethernet => Connection::Ethernet(EthernetConnection { base }), + DeviceType::Bond => Connection::Bond(BondConnection { + base, + ..Default::default() + }), } } @@ -246,6 +252,7 @@ impl Connection { Connection::Ethernet(conn) => &conn.base, Connection::Wireless(conn) => &conn.base, Connection::Loopback(conn) => &conn.base, + Connection::Bond(conn) => &conn.base, } } @@ -254,6 +261,7 @@ impl Connection { Connection::Ethernet(conn) => &mut conn.base, Connection::Wireless(conn) => &mut conn.base, Connection::Loopback(conn) => &mut conn.base, + Connection::Bond(conn) => &mut conn.base, } } @@ -308,6 +316,37 @@ impl Connection { } } +#[derive(Debug, PartialEq, Clone)] +pub enum ParentKind { + Bond, +} + +impl fmt::Display for ParentKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let name = match &self { + ParentKind::Bond => "bond", + }; + write!(f, "{}", name) + } +} + +impl FromStr for ParentKind { + type Err = NetworkStateError; + + fn from_str(s: &str) -> Result { + match s { + "bond" => Ok(ParentKind::Bond), + _ => Err(NetworkStateError::UnknownParentKind(s.to_string())), + } + } +} + +#[derive(Debug, Clone)] +pub struct Parent { + pub interface: String, + pub kind: ParentKind, +} + #[derive(Debug, Default, Clone)] pub struct BaseConnection { pub id: String, @@ -316,6 +355,7 @@ pub struct BaseConnection { pub status: Status, pub interface: String, pub match_config: MatchConfig, + pub parent: Option, } impl PartialEq for BaseConnection { @@ -480,6 +520,17 @@ pub struct LoopbackConnection { } #[derive(Debug, Default, PartialEq, Clone)] +pub struct BondConnection { + pub base: BaseConnection, + pub bond: BondConfig, +} + +#[derive(Debug, Default, PartialEq, Clone)] +pub struct BondConfig { + pub options: HashMap +} + +#[derive(Debug, Default, Clone, PartialEq)] pub struct WirelessConfig { pub mode: WirelessMode, pub ssid: SSID, diff --git a/rust/agama-lib/src/network/types.rs b/rust/agama-lib/src/network/types.rs index 5aa56c5145..729f68bc10 100644 --- a/rust/agama-lib/src/network/types.rs +++ b/rust/agama-lib/src/network/types.rs @@ -29,6 +29,7 @@ pub enum DeviceType { Loopback = 0, Ethernet = 1, Wireless = 2, + Bond = 3, } #[derive(Debug, Error, PartialEq)] @@ -43,6 +44,7 @@ impl TryFrom for DeviceType { 0 => Ok(DeviceType::Loopback), 1 => Ok(DeviceType::Ethernet), 2 => Ok(DeviceType::Wireless), + 3 => Ok(DeviceType::Bond), _ => Err(InvalidDeviceType(value)), } } From 2f4e8b906393ef35cd32509c3be4898151303de4 Mon Sep 17 00:00:00 2001 From: Clemens Famulla-Conrad Date: Tue, 26 Sep 2023 14:06:00 +0200 Subject: [PATCH 02/58] nm-dbus: use WIRELESS_KEY instead of slice --- rust/agama-dbus-server/src/network/nm/dbus.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index d601291e9f..fd6f3b9be6 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -33,7 +33,7 @@ pub fn connection_to_dbus(conn: &Connection) -> NestedHash { result.insert("match", match_config_to_dbus(conn.match_config())); if let Connection::Wireless(wireless) = conn { - connection_dbus.insert("type", "802-11-wireless".into()); + connection_dbus.insert("type", WIRELESS_KEY.into()); let wireless_dbus = wireless_config_to_dbus(wireless); for (k, v) in wireless_dbus { result.insert(k, v); From 5053b8704fa3b77afc3bbae2c792b77c01a0fd64 Mon Sep 17 00:00:00 2001 From: Clemens Famulla-Conrad Date: Tue, 26 Sep 2023 14:17:37 +0200 Subject: [PATCH 03/58] nm-dbus: initial bonding to dbus impl --- rust/agama-dbus-server/src/network/model.rs | 1 - rust/agama-dbus-server/src/network/nm/dbus.rs | 40 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index bccbabc32d..663ba2b457 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -55,7 +55,6 @@ impl NetworkState { self.connections.iter_mut().find(|c| c.id() == id) } - /// Adds a new connection. /// /// It uses the `id` to decide whether the connection already exists. diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index fd6f3b9be6..91679ba01a 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -14,6 +14,7 @@ use uuid::Uuid; use zbus::zvariant::{self, OwnedValue, Value}; const ETHERNET_KEY: &str = "802-3-ethernet"; +const BOND_KEY: &str = "bond"; const WIRELESS_KEY: &str = "802-11-wireless"; const WIRELESS_SECURITY_KEY: &str = "802-11-wireless-security"; const LOOPBACK_KEY: &str = "loopback"; @@ -28,6 +29,10 @@ pub fn connection_to_dbus(conn: &Connection) -> NestedHash { ("type", ETHERNET_KEY.into()), ("interface-name", conn.interface().into()), ]); + if let Some(parent) = &conn.base().parent { + connection_dbus.insert("master", parent.interface.clone().into()); + connection_dbus.insert("slave-type", parent.kind.to_string().into()); + } result.insert("ipv4", ip_config_to_ipv4_dbus(conn.ip_config())); result.insert("ipv6", ip_config_to_ipv6_dbus(conn.ip_config())); result.insert("match", match_config_to_dbus(conn.match_config())); @@ -40,6 +45,14 @@ pub fn connection_to_dbus(conn: &Connection) -> NestedHash { } } + if let Connection::Bond(bond) = conn { + connection_dbus.insert("type", BOND_KEY.into()); + let bond_dbus = bond_config_to_dbus(bond); + for (k, v) in bond_dbus { + result.insert(k, v); + } + } + result.insert("connection", connection_dbus); result } @@ -57,6 +70,13 @@ pub fn connection_from_dbus(conn: OwnedNestedHash) -> Option { })); } + if let Some(bond_config) = bond_config_from_dbus(&conn) { + return Some(Connection::Bond(BondConnection { + base, + bond: bond_config, + })); + } + if conn.get(LOOPBACK_KEY).is_some() { return Some(Connection::Loopback(LoopbackConnection { base })); }; @@ -233,6 +253,17 @@ fn wireless_config_to_dbus(conn: &WirelessConnection) -> NestedHash { ]) } +fn bond_config_to_dbus(conn: &BondConnection) -> NestedHash { + let config = &conn.bond; + let bond: HashMap<&str, zvariant::Value> = HashMap::from([ + ("options", Value::new(config.options.clone())) + ]); + + NestedHash::from([ + ("bond", bond ), + ]) +} + /// Converts a MatchConfig struct into a HashMap that can be sent over D-Bus. /// /// * `match_config`: MatchConfig to convert. @@ -470,6 +501,15 @@ fn wireless_config_from_dbus(conn: &OwnedNestedHash) -> Option { Some(wireless_config) } +fn bond_config_from_dbus(conn: &OwnedNestedHash) -> Option { + let Some(_bond) = conn.get(BOND_KEY) else { + return None; + }; + + // TODO + None +} + /// Determines whether a value is empty. /// /// TODO: Generalize for other kind of values, like dicts or arrays. From a241308d424ff741c1aa91078656d65d6f6a966a Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Tue, 14 Nov 2023 10:48:39 +0000 Subject: [PATCH 04/58] Added bonding configuration support --- rust/agama-dbus-server/src/network/action.rs | 2 + .../src/network/dbus/interfaces.rs | 76 ++++++++++++++- .../src/network/dbus/tree.rs | 9 ++ rust/agama-dbus-server/src/network/model.rs | 96 ++++++++++++------- .../src/network/nm/client.rs | 72 +++++++++++++- rust/agama-dbus-server/src/network/nm/dbus.rs | 45 +++++---- .../agama-dbus-server/src/network/nm/model.rs | 1 + rust/agama-dbus-server/src/network/system.rs | 3 + rust/agama-lib/share/examples/profile.json | 2 +- rust/agama-lib/src/network/client.rs | 39 +++++++- rust/agama-lib/src/network/proxies.rs | 51 ++++++++++ rust/agama-lib/src/network/settings.rs | 25 +++++ rust/agama-lib/src/network/types.rs | 9 +- 13 files changed, 369 insertions(+), 61 deletions(-) diff --git a/rust/agama-dbus-server/src/network/action.rs b/rust/agama-dbus-server/src/network/action.rs index 9f935f70f1..025e9ba53a 100644 --- a/rust/agama-dbus-server/src/network/action.rs +++ b/rust/agama-dbus-server/src/network/action.rs @@ -11,6 +11,8 @@ pub enum Action { AddConnection(String, DeviceType), /// Update a connection (replacing the old one). UpdateConnection(Connection), + /// Update a controller connection (replacing the old one). + UpdateControllerConnection(Connection, Vec), /// Remove the connection with the given Uuid. RemoveConnection(String), /// Apply the current configuration. diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index 682dbbfe10..454a7ee5a1 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -6,7 +6,10 @@ use super::ObjectsRegistry; use crate::network::{ action::Action, error::NetworkStateError, - model::{Connection as NetworkConnection, Device as NetworkDevice, WirelessConnection}, + model::{ + BondConnection, Connection as NetworkConnection, Device as NetworkDevice, + WirelessConnection, + }, }; use agama_lib::network::types::SSID; @@ -281,6 +284,77 @@ impl Match { } } +/// D-Bus interface for Bond settings +pub struct Bond { + actions: Arc>>, + connection: Arc>, +} + +impl Bond { + /// Creates a Match Settings interface object. + /// + /// * `actions`: sending-half of a channel to send actions. + /// * `connection`: connection to expose over D-Bus. + pub fn new(actions: Sender, connection: Arc>) -> Self { + Self { + actions: Arc::new(Mutex::new(actions)), + connection, + } + } + + /// Gets the bond connection. + /// + /// Beware that it crashes when it is not a bond connection. + async fn get_bond(&self) -> MappedMutexGuard { + MutexGuard::map(self.connection.lock().await, |c| match c { + NetworkConnection::Bond(config) => config, + _ => panic!("Not a bond connection. This is most probably a bug."), + }) + } + + /// Updates the controller connection data in the NetworkSystem. + /// + /// * `connection`: Updated connection. + async fn update_controller_connection<'a>( + &self, + connection: MappedMutexGuard<'a, NetworkConnection, BondConnection>, + ports: Vec, + ) -> zbus::fdo::Result<()> { + let actions = self.actions.lock().await; + let connection = NetworkConnection::Bond(connection.clone()); + actions + .send(Action::UpdateControllerConnection( + connection.clone(), + ports.clone(), + )) + .await + .unwrap(); + Ok(()) + } +} + +#[dbus_interface(name = "org.opensuse.Agama1.Network.Connection.Bond")] +impl Bond { + /// List of bond ports. + #[dbus_interface(property)] + pub async fn ports(&self) -> Vec { + let connection = self.get_bond().await; + connection + .bond + .ports + .iter() + .map(|port| port.base().interface.to_string()) + .collect() + } + + #[dbus_interface(property)] + pub async fn set_ports(&mut self, ports: Vec) -> zbus::fdo::Result<()> { + let connection = self.get_bond().await; + self.update_controller_connection(connection, ports.clone()) + .await + } +} + #[dbus_interface(name = "org.opensuse.Agama1.Network.Connection.Match")] impl Match { /// List of driver names to match. diff --git a/rust/agama-dbus-server/src/network/dbus/tree.rs b/rust/agama-dbus-server/src/network/dbus/tree.rs index 28a9e34053..e56f51638d 100644 --- a/rust/agama-dbus-server/src/network/dbus/tree.rs +++ b/rust/agama-dbus-server/src/network/dbus/tree.rs @@ -112,6 +112,15 @@ impl Tree { interfaces::Match::new(self.actions.clone(), Arc::clone(&cloned)), ) .await?; + + if let Connection::Bond(_) = conn { + self.add_interface( + &path, + interfaces::Bond::new(self.actions.clone(), Arc::clone(&cloned)), + ) + .await?; + } + if let Connection::Wireless(_) = conn { self.add_interface( &path, diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 663ba2b457..06cbe56737 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -10,7 +10,7 @@ use std::{ default::Default, fmt, net::IpAddr, - str::{self, FromStr}, collections::HashMap, + str::{self, FromStr}, }; use thiserror::Error; use uuid::Uuid; @@ -55,6 +55,13 @@ impl NetworkState { self.connections.iter_mut().find(|c| c.id() == id) } + /// Get connection by name + /// + /// * `name`: connection name + pub fn get_connection_by_name(&self, id: &str) -> Option<&Connection> { + self.connections.iter().find(|c| c.interface() == id) + } + /// Adds a new connection. /// /// It uses the `id` to decide whether the connection already exists. @@ -81,6 +88,48 @@ impl NetworkState { Ok(()) } + /// Updates a controller connection with a new one. + /// + /// It uses the `id` to decide which connection to update. + /// + /// Additionally, it registers the connection to be removed when the changes are applied. + pub fn update_controller_connection( + &mut self, + conn: Connection, + ports: Vec, + ) -> Result<(), NetworkStateError> { + // let Some(old_conn) = self.get_connection_mut(conn.id()) else { + // return Err(NetworkStateError::UnknownConnection(conn.id().to_string())); + // }; + // + let mut new_ports = vec![]; + + if let Connection::Bond(mut bond) = conn { + // if let Connection::Bond(old_bond) = old_conn { + // let moved_ports = old_bond.bond.ports.into_iter().filter(|p| ports.contains(&p.base().interface)); + // for port in old_bond.bond.ports.iter() { + // if ports.contains(&port.base().interface) { + // bond.bond.ports.push(port.clone()) + // } + // } + //} + + for port in ports.into_iter() { + new_ports.push( + if let Some(new_port) = bond.bond.ports.iter().find(|c| c.interface() == port) { + new_port.clone() + } else { + Connection::new(port, DeviceType::Ethernet) + }, + ); + } + + bond.bond.ports = new_ports; + } + + Ok(()) + } + /// Removes a connection from the state. /// /// Additionally, it registers the connection to be removed when the changes are applied. @@ -280,11 +329,18 @@ impl Connection { self.base_mut().interface = interface.to_string() } + pub fn controller(&self) -> &str { + self.base().controller.as_str() + } + + pub fn set_controller(&mut self, controller: &str) { + self.base_mut().controller = controller.to_string() + } + pub fn uuid(&self) -> Uuid { self.base().uuid } - /// FIXME: rename to ip_config pub fn ip_config(&self) -> &IpConfig { &self.base().ip_config } @@ -315,37 +371,6 @@ impl Connection { } } -#[derive(Debug, PartialEq, Clone)] -pub enum ParentKind { - Bond, -} - -impl fmt::Display for ParentKind { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let name = match &self { - ParentKind::Bond => "bond", - }; - write!(f, "{}", name) - } -} - -impl FromStr for ParentKind { - type Err = NetworkStateError; - - fn from_str(s: &str) -> Result { - match s { - "bond" => Ok(ParentKind::Bond), - _ => Err(NetworkStateError::UnknownParentKind(s.to_string())), - } - } -} - -#[derive(Debug, Clone)] -pub struct Parent { - pub interface: String, - pub kind: ParentKind, -} - #[derive(Debug, Default, Clone)] pub struct BaseConnection { pub id: String, @@ -353,8 +378,8 @@ pub struct BaseConnection { pub ip_config: IpConfig, pub status: Status, pub interface: String, + pub controller: String, pub match_config: MatchConfig, - pub parent: Option, } impl PartialEq for BaseConnection { @@ -526,7 +551,8 @@ pub struct BondConnection { #[derive(Debug, Default, PartialEq, Clone)] pub struct BondConfig { - pub options: HashMap + pub ports: Vec, + pub options: HashMap, } #[derive(Debug, Default, Clone, PartialEq)] diff --git a/rust/agama-dbus-server/src/network/nm/client.rs b/rust/agama-dbus-server/src/network/nm/client.rs index 9c859b586d..8a5989caf4 100644 --- a/rust/agama-dbus-server/src/network/nm/client.rs +++ b/rust/agama-dbus-server/src/network/nm/client.rs @@ -1,5 +1,9 @@ //! NetworkManager client. -use super::dbus::{connection_from_dbus, connection_to_dbus, merge_dbus_connections}; +use std::collections::HashMap; + +use super::dbus::{ + connection_from_dbus, connection_to_dbus, controller_from_dbus, merge_dbus_connections, +}; use super::model::NmDeviceType; use super::proxies::{ConnectionProxy, DeviceProxy, NetworkManagerProxy, SettingsProxy}; use crate::network::model::{Connection, Device}; @@ -71,6 +75,7 @@ impl<'a> NetworkManagerClient<'a> { let proxy = SettingsProxy::new(&self.connection).await?; let paths = proxy.list_connections().await?; let mut connections: Vec = Vec::with_capacity(paths.len()); + let mut controllers: HashMap> = HashMap::new(); for path in paths { let proxy = ConnectionProxy::builder(&self.connection) .path(path.as_str())? @@ -78,10 +83,25 @@ impl<'a> NetworkManagerClient<'a> { .await?; let settings = proxy.get_settings().await?; // TODO: log an error if a connection is not found - if let Some(connection) = connection_from_dbus(settings) { - connections.push(connection); + + if let Some(connection) = connection_from_dbus(settings.clone()) { + if let Some(controller) = controller_from_dbus(settings) { + controllers + .entry(controller) + .or_insert_with(Vec::new) + .push(connection) + } else { + connections.push(connection); + } + } + } + for (controller, conns) in controllers { + dbg!(controller.to_string()); + for conn in conns { + dbg!(conn.id().to_string()); } } + Ok(connections) } @@ -90,6 +110,7 @@ impl<'a> NetworkManagerClient<'a> { /// * `conn`: connection to add or update. pub async fn add_or_update_connection(&self, conn: &Connection) -> Result<(), ServiceError> { let new_conn = connection_to_dbus(conn); + let path = if let Ok(proxy) = self.get_connection_proxy(conn.uuid()).await { let original = proxy.get_settings().await?; let merged = merge_dbus_connections(&original, &new_conn); @@ -99,6 +120,51 @@ impl<'a> NetworkManagerClient<'a> { let proxy = SettingsProxy::new(&self.connection).await?; proxy.add_connection(new_conn).await? }; + + if let Connection::Bond(bond) = conn { + let _ = bond.bond.ports.iter().map(|port| { + self.add_or_update_port_connection( + port, + bond.base.interface.to_string(), + "bond".to_string(), + ) + }); + } + self.activate_connection(path).await?; + Ok(()) + } + + pub async fn add_or_update_port_connection( + &self, + conn: &Connection, + controller: String, + port_type: String, + ) -> Result<(), ServiceError> { + let mut dbus_conn = connection_to_dbus(conn); + if let Some(new_conn) = dbus_conn.get_mut("connection") { + new_conn.insert("slave-type", port_type.to_string().into()); + new_conn.insert("master", controller.to_string().into()); + } + + let path = if let Ok(proxy) = self.get_connection_proxy(conn.uuid()).await { + let original = proxy.get_settings().await?; + let merged = merge_dbus_connections(&original, &dbus_conn); + proxy.update(merged).await?; + OwnedObjectPath::from(proxy.path().to_owned()) + } else { + let proxy = SettingsProxy::new(&self.connection).await?; + proxy.add_connection(dbus_conn).await? + }; + + if let Connection::Bond(bond) = conn { + let _ = bond.bond.ports.iter().map(|port| { + self.add_or_update_port_connection( + port, + bond.base.interface.to_string(), + "bond".to_string(), + ) + }); + } self.activate_connection(path).await?; Ok(()) } diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index 91679ba01a..a4f9167a0a 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -29,10 +29,7 @@ pub fn connection_to_dbus(conn: &Connection) -> NestedHash { ("type", ETHERNET_KEY.into()), ("interface-name", conn.interface().into()), ]); - if let Some(parent) = &conn.base().parent { - connection_dbus.insert("master", parent.interface.clone().into()); - connection_dbus.insert("slave-type", parent.kind.to_string().into()); - } + result.insert("ipv4", ip_config_to_ipv4_dbus(conn.ip_config())); result.insert("ipv6", ip_config_to_ipv6_dbus(conn.ip_config())); result.insert("match", match_config_to_dbus(conn.match_config())); @@ -47,10 +44,7 @@ pub fn connection_to_dbus(conn: &Connection) -> NestedHash { if let Connection::Bond(bond) = conn { connection_dbus.insert("type", BOND_KEY.into()); - let bond_dbus = bond_config_to_dbus(bond); - for (k, v) in bond_dbus { - result.insert(k, v); - } + result.insert("bond", bond_config_to_dbus(bond)); } result.insert("connection", connection_dbus); @@ -71,6 +65,7 @@ pub fn connection_from_dbus(conn: OwnedNestedHash) -> Option { } if let Some(bond_config) = bond_config_from_dbus(&conn) { + println!("Returning bond config"); return Some(Connection::Bond(BondConnection { base, bond: bond_config, @@ -253,15 +248,10 @@ fn wireless_config_to_dbus(conn: &WirelessConnection) -> NestedHash { ]) } -fn bond_config_to_dbus(conn: &BondConnection) -> NestedHash { +fn bond_config_to_dbus(conn: &BondConnection) -> HashMap<&str, zvariant::Value> { let config = &conn.bond; - let bond: HashMap<&str, zvariant::Value> = HashMap::from([ - ("options", Value::new(config.options.clone())) - ]); - NestedHash::from([ - ("bond", bond ), - ]) + HashMap::from([("options", Value::new(config.options.clone()))]) } /// Converts a MatchConfig struct into a HashMap that can be sent over D-Bus. @@ -298,6 +288,19 @@ fn match_config_to_dbus(match_config: &MatchConfig) -> HashMap<&str, zvariant::V ("interface-name", interfaces), ]) } +pub fn controller_from_dbus(conn: OwnedNestedHash) -> Option { + let Some(connection) = conn.get("connection") else { + return None; + }; + + let Some(controller) = connection.get("master") else { + return None; + }; + + let controller: &str = controller.downcast_ref()?; + + Some(controller.to_string()) +} fn base_connection_from_dbus(conn: &OwnedNestedHash) -> Option { let Some(connection) = conn.get("connection") else { @@ -307,6 +310,7 @@ fn base_connection_from_dbus(conn: &OwnedNestedHash) -> Option { let id: &str = connection.get("id")?.downcast_ref()?; let uuid: &str = connection.get("uuid")?.downcast_ref()?; let uuid: Uuid = uuid.try_into().ok()?; + let mut base_connection = BaseConnection { id: id.to_string(), uuid, @@ -502,12 +506,17 @@ fn wireless_config_from_dbus(conn: &OwnedNestedHash) -> Option { } fn bond_config_from_dbus(conn: &OwnedNestedHash) -> Option { - let Some(_bond) = conn.get(BOND_KEY) else { + let Some(bond) = conn.get(BOND_KEY) else { return None; }; - // TODO - None + let dict: &zvariant::Dict = bond.get("options")?.downcast_ref()?; + let options = >::try_from(dict.clone()).unwrap(); + + Some(BondConfig { + options: options, + ..Default::default() + }) } /// Determines whether a value is empty. diff --git a/rust/agama-dbus-server/src/network/nm/model.rs b/rust/agama-dbus-server/src/network/nm/model.rs index 93ce9f69c8..3820083bf8 100644 --- a/rust/agama-dbus-server/src/network/nm/model.rs +++ b/rust/agama-dbus-server/src/network/nm/model.rs @@ -83,6 +83,7 @@ impl TryFrom for DeviceType { NmDeviceType(0) => Ok(DeviceType::Loopback), NmDeviceType(1) => Ok(DeviceType::Ethernet), NmDeviceType(2) => Ok(DeviceType::Wireless), + NmDeviceType(10) => Ok(DeviceType::Bond), NmDeviceType(_) => Err(NmError::UnsupportedDeviceType(value.into())), } } diff --git a/rust/agama-dbus-server/src/network/system.rs b/rust/agama-dbus-server/src/network/system.rs index fcfb3e170e..ded163ab57 100644 --- a/rust/agama-dbus-server/src/network/system.rs +++ b/rust/agama-dbus-server/src/network/system.rs @@ -73,6 +73,9 @@ impl NetworkSystem { Action::UpdateConnection(conn) => { self.state.update_connection(conn)?; } + Action::UpdateControllerConnection(conn, ports) => { + self.state.update_controller_connection(conn, ports)?; + } Action::RemoveConnection(id) => { self.tree.remove_connection(&id).await?; self.state.remove_connection(&id)?; diff --git a/rust/agama-lib/share/examples/profile.json b/rust/agama-lib/share/examples/profile.json index 0aae970862..f87db83a76 100644 --- a/rust/agama-lib/share/examples/profile.json +++ b/rust/agama-lib/share/examples/profile.json @@ -34,7 +34,7 @@ "nameservers": [ "192.168.122.1", "2001:4860:4860::8888" - ] + ], } ] } diff --git a/rust/agama-lib/src/network/client.rs b/rust/agama-lib/src/network/client.rs index 485baee516..660d97d824 100644 --- a/rust/agama-lib/src/network/client.rs +++ b/rust/agama-lib/src/network/client.rs @@ -1,6 +1,9 @@ -use super::proxies::{ConnectionProxy, ConnectionsProxy, IPProxy, MatchProxy, WirelessProxy}; +use super::proxies::{ + ConnectionProxy, ConnectionsProxy, DeviceProxy, DevicesProxy, IPProxy, MatchProxy, + WirelessProxy, +}; use super::settings::{MatchSettings, NetworkConnection, WirelessSettings}; -use super::types::SSID; +use super::types::{Device, DeviceType, SSID}; use crate::error::ServiceError; use tokio_stream::StreamExt; use zbus::zvariant::OwnedObjectPath; @@ -10,12 +13,14 @@ use zbus::Connection; pub struct NetworkClient<'a> { pub connection: Connection, connections_proxy: ConnectionsProxy<'a>, + devices_proxy: DevicesProxy<'a>, } impl<'a> NetworkClient<'a> { pub async fn new(connection: Connection) -> Result, ServiceError> { Ok(Self { connections_proxy: ConnectionsProxy::new(&connection).await?, + devices_proxy: DevicesProxy::new(&connection).await?, connection, }) } @@ -25,6 +30,19 @@ impl<'a> NetworkClient<'a> { Ok(self.connection_from(path.as_str()).await?) } + pub async fn available_devices(&self) -> Result, ServiceError> { + let devices_paths = self.devices_proxy.get_devices().await?; + let mut devices = vec![]; + + for path in devices_paths { + let device = self.device_from(path.as_str()).await?; + + devices.push(device); + } + + Ok(devices) + } + /// Returns an array of network connections pub async fn connections(&self) -> Result, ServiceError> { let connection_paths = self.connections_proxy.get_connections().await?; @@ -54,6 +72,23 @@ impl<'a> NetworkClient<'a> { Ok(()) } + /// Returns the NetworkDevice for the given device path + /// + /// * `path`: the connections path to get the config from + async fn device_from(&self, path: &str) -> Result { + let device_proxy = DeviceProxy::builder(&self.connection) + .path(path)? + .build() + .await?; + let name = device_proxy.name().await?; + let device_type = device_proxy.device_type().await?; + + Ok(Device { + name, + type_: DeviceType::try_from(device_type).unwrap(), + }) + } + /// Returns the NetworkConnection for the given connection path /// /// * `path`: the connections path to get the config from diff --git a/rust/agama-lib/src/network/proxies.rs b/rust/agama-lib/src/network/proxies.rs index 2b43124722..207a45dfa5 100644 --- a/rust/agama-lib/src/network/proxies.rs +++ b/rust/agama-lib/src/network/proxies.rs @@ -3,6 +3,29 @@ //! This code was generated by `zbus-xmlgen` `3.1.0` from DBus introspection data.`. use zbus::dbus_proxy; +#[dbus_proxy( + interface = "org.opensuse.Agama1.Network.Devices", + default_service = "org.opensuse.Agama1", + default_path = "/org/opensuse/Agama1/Network/devices" +)] +trait Devices { + /// GetConnections method + fn get_devices(&self) -> zbus::Result>; +} + +#[dbus_proxy( + interface = "org.opensuse.Agama1.Network.Device", + default_service = "org.opensuse.Agama1", + default_path = "/org/opensuse/Agama1/Network" +)] +trait Device { + /// Id property + #[dbus_proxy(property)] + fn name(&self) -> zbus::Result; + #[dbus_proxy(property)] + fn device_type(&self) -> zbus::Result; +} + #[dbus_proxy( interface = "org.opensuse.Agama1.Network.Connections", default_service = "org.opensuse.Agama1", @@ -137,10 +160,12 @@ trait Match { /// Driver property #[dbus_proxy(property)] fn driver(&self) -> zbus::Result>; + fn set_driver(&self, value: &[&str]) -> zbus::Result<()>; /// Interface property #[dbus_proxy(property)] fn interface(&self) -> zbus::Result>; + fn set_interface(&self, value: &[&str]) -> zbus::Result<()>; /// Path property #[dbus_proxy(property)] @@ -151,4 +176,30 @@ trait Match { /// Path property #[dbus_proxy(property)] fn kernel(&self) -> zbus::Result>; + fn set_kernel(&self, value: &[&str]) -> zbus::Result<()>; +} + +#[dbus_proxy( + interface = "org.opensuse.Agama1.Network.Connection.Bond", + default_service = "org.opensuse.Agama1", + default_path = "/org/opensuse/Agama1/Network" +)] +trait Bond { + /// Mode property + #[dbus_proxy(property)] + fn mode(&self) -> zbus::Result; + #[dbus_proxy(property)] + fn set_mode(&self, value: &str) -> zbus::Result<()>; + + /// ports property + #[dbus_proxy(property)] + fn options(&self) -> zbus::Result>; + #[dbus_proxy(property)] + fn set_options(&self, value: &[&str]) -> zbus::Result<()>; + + /// ports property + #[dbus_proxy(property)] + fn ports(&self) -> zbus::Result>; + #[dbus_proxy(property)] + fn set_ports(&self, value: &[&str]) -> zbus::Result<()>; } diff --git a/rust/agama-lib/src/network/settings.rs b/rust/agama-lib/src/network/settings.rs index cd344ef323..4d9db9106d 100644 --- a/rust/agama-lib/src/network/settings.rs +++ b/rust/agama-lib/src/network/settings.rs @@ -48,6 +48,18 @@ pub struct WirelessSettings { pub mode: String, } +#[derive(Clone, Debug, Default, Serialize, Deserialize)] +pub struct BondSettings { + pub options: String, + pub ports: Vec, +} + +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct NetworkDevice { + pub id: String, + pub type_: DeviceType, +} + #[derive(Clone, Debug, Default, Serialize, Deserialize)] pub struct NetworkConnection { pub id: String, @@ -69,6 +81,10 @@ pub struct NetworkConnection { pub interface: Option, #[serde(skip_serializing_if = "Option::is_none")] pub match_settings: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub parent: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub bond: Option, } impl NetworkConnection { @@ -79,6 +95,8 @@ impl NetworkConnection { pub fn device_type(&self) -> DeviceType { if self.wireless.is_some() { DeviceType::Wireless + } else if self.bond.is_some() { + DeviceType::Bond } else { DeviceType::Ethernet } @@ -123,7 +141,14 @@ mod tests { wireless: Some(WirelessSettings::default()), ..Default::default() }; + + let bond = NetworkConnection { + bond: Some(BondSettings::default()), + ..Default::default() + }; + assert_eq!(wlan.device_type(), DeviceType::Wireless); + assert_eq!(bond.device_type(), DeviceType::Bond); } #[test] diff --git a/rust/agama-lib/src/network/types.rs b/rust/agama-lib/src/network/types.rs index 729f68bc10..e51bbcf975 100644 --- a/rust/agama-lib/src/network/types.rs +++ b/rust/agama-lib/src/network/types.rs @@ -3,6 +3,13 @@ use std::{fmt, str}; use thiserror::Error; use zbus; +/// Network device +#[derive(Debug, Clone)] +pub struct Device { + pub name: String, + pub type_: DeviceType, +} + #[derive(Debug, Default, PartialEq, Clone, Serialize, Deserialize)] pub struct SSID(pub Vec); @@ -24,7 +31,7 @@ impl From for Vec { } } -#[derive(Debug, PartialEq, Copy, Clone)] +#[derive(Debug, PartialEq, Copy, Clone, Serialize, Deserialize)] pub enum DeviceType { Loopback = 0, Ethernet = 1, From 48480fa1abc1624e7de17fe93058ba35befd80f7 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Wed, 15 Nov 2023 12:26:31 +0000 Subject: [PATCH 05/58] Mutable connection --- rust/agama-dbus-server/src/network/model.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 06cbe56737..537f638345 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -95,7 +95,7 @@ impl NetworkState { /// Additionally, it registers the connection to be removed when the changes are applied. pub fn update_controller_connection( &mut self, - conn: Connection, + mut conn: Connection, ports: Vec, ) -> Result<(), NetworkStateError> { // let Some(old_conn) = self.get_connection_mut(conn.id()) else { @@ -104,7 +104,7 @@ impl NetworkState { // let mut new_ports = vec![]; - if let Connection::Bond(mut bond) = conn { + if let Connection::Bond(ref mut bond) = conn { // if let Connection::Bond(old_bond) = old_conn { // let moved_ports = old_bond.bond.ports.into_iter().filter(|p| ports.contains(&p.base().interface)); // for port in old_bond.bond.ports.iter() { From 9d0369c117663045b47669f1a4428d08fe86fee9 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Fri, 17 Nov 2023 23:31:04 +0000 Subject: [PATCH 06/58] Bond schema and settings --- .../src/network/dbus/interfaces.rs | 36 ++++++++++++++++--- rust/agama-lib/share/examples/profile.jsonnet | 8 +++++ rust/agama-lib/share/profile.schema.json | 18 ++++++++++ rust/agama-lib/src/network/client.rs | 27 ++++++++++++-- rust/agama-lib/src/network/proxies.rs | 4 +-- 5 files changed, 84 insertions(+), 9 deletions(-) diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index 454a7ee5a1..304a5c5167 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -2,6 +2,8 @@ //! //! This module contains the set of D-Bus interfaces that are exposed by [D-Bus network //! service](crate::NetworkService). +use std::collections::HashMap; + use super::ObjectsRegistry; use crate::network::{ action::Action, @@ -318,14 +320,14 @@ impl Bond { async fn update_controller_connection<'a>( &self, connection: MappedMutexGuard<'a, NetworkConnection, BondConnection>, - ports: Vec, + settings: HashMap, ) -> zbus::fdo::Result<()> { let actions = self.actions.lock().await; let connection = NetworkConnection::Bond(connection.clone()); actions .send(Action::UpdateControllerConnection( connection.clone(), - ports.clone(), + settings, )) .await .unwrap(); @@ -333,8 +335,33 @@ impl Bond { } } +enum BondSettings { + Ports(Vec), + Options(String), +} + #[dbus_interface(name = "org.opensuse.Agama1.Network.Connection.Bond")] impl Bond { + /// List of bond ports. + #[dbus_interface(property)] + pub async fn options(&self) -> String { + let connection = self.get_bond().await; + let mut opts = vec![]; + for (key, value) in &connection.bond.options { + opts.push(format!("{key}={value}")); + } + + opts.join(" ") + } + + #[dbus_interface(property)] + pub async fn set_options(&self, opts: String) -> zbus::fdo::Result<()> { + let connection = self.get_bond().await; + + self.update_controller_connection(connection, HashMap::from(["options", opts.clone()])) + .await + } + /// List of bond ports. #[dbus_interface(property)] pub async fn ports(&self) -> Vec { @@ -350,7 +377,7 @@ impl Bond { #[dbus_interface(property)] pub async fn set_ports(&mut self, ports: Vec) -> zbus::fdo::Result<()> { let connection = self.get_bond().await; - self.update_controller_connection(connection, ports.clone()) + self.update_controller_connection(connection, HashMap::from(["ports", ports.clone()])) .await } } @@ -536,7 +563,6 @@ impl Wireless { connection.wireless.security = security .try_into() .map_err(|_| NetworkStateError::InvalidSecurityProtocol(security.to_string()))?; - self.update_connection(connection).await?; - Ok(()) + self.update_connection(connection).await } } diff --git a/rust/agama-lib/share/examples/profile.jsonnet b/rust/agama-lib/share/examples/profile.jsonnet index cd6b63f5bf..e5ae5d01aa 100644 --- a/rust/agama-lib/share/examples/profile.jsonnet +++ b/rust/agama-lib/share/examples/profile.jsonnet @@ -63,6 +63,14 @@ local findBiggestDisk(disks) = match: { path: ["pci-0000:00:19.0"] } + }, + { + id: 'bond0', + bond: { + ports: ['eth0', 'eth1'], + options: "mode=active-backup primary=eth1" + + } } ] } diff --git a/rust/agama-lib/share/profile.schema.json b/rust/agama-lib/share/profile.schema.json index 2d9cb33b0a..fa24143bdf 100644 --- a/rust/agama-lib/share/profile.schema.json +++ b/rust/agama-lib/share/profile.schema.json @@ -105,6 +105,24 @@ } } }, + "bond": { + "type": "object", + "description": "Bonding configuration", + "additionalProperties": false, + "properties": { + "options": { + "type": "string" + }, + "ports": { + "type": "array", + "items": { + "description": "A list of the ports to be bonded", + "type": "string", + "additionalProperties": false + } + } + } + }, "match": { "type": "object", "description": "Match settings", diff --git a/rust/agama-lib/src/network/client.rs b/rust/agama-lib/src/network/client.rs index 660d97d824..837fe7e02f 100644 --- a/rust/agama-lib/src/network/client.rs +++ b/rust/agama-lib/src/network/client.rs @@ -1,8 +1,8 @@ use super::proxies::{ - ConnectionProxy, ConnectionsProxy, DeviceProxy, DevicesProxy, IPProxy, MatchProxy, + BondProxy, ConnectionProxy, ConnectionsProxy, DeviceProxy, DevicesProxy, IPProxy, MatchProxy, WirelessProxy, }; -use super::settings::{MatchSettings, NetworkConnection, WirelessSettings}; +use super::settings::{BondSettings, MatchSettings, NetworkConnection, WirelessSettings}; use super::types::{Device, DeviceType, SSID}; use crate::error::ServiceError; use tokio_stream::StreamExt; @@ -226,6 +226,10 @@ impl<'a> NetworkClient<'a> { self.update_ip_settings(path, conn).await?; + if let Some(ref bond) = conn.bond { + self.update_bond_settings(path, bond).await?; + } + if let Some(ref wireless) = conn.wireless { self.update_wireless_settings(path, wireless).await?; } @@ -276,6 +280,25 @@ impl<'a> NetworkClient<'a> { Ok(()) } + /// Updates the bond settings for network connection. + /// + /// * `path`: connection D-Bus path. + /// * `bond`: bond settings of the network connection. + async fn update_bond_settings( + &self, + path: &OwnedObjectPath, + bond: &BondSettings, + ) -> Result<(), ServiceError> { + let proxy = BondProxy::builder(&self.connection) + .path(path)? + .build() + .await?; + + let ports: Vec<_> = bond.ports.iter().map(String::as_ref).collect(); + proxy.set_ports(ports.as_slice()).await?; + proxy.set_options(bond.options.to_string().as_str()).await?; + Ok(()) + } /// Updates the wireless settings for network connection. /// /// * `path`: connection D-Bus path. diff --git a/rust/agama-lib/src/network/proxies.rs b/rust/agama-lib/src/network/proxies.rs index 207a45dfa5..84335b6996 100644 --- a/rust/agama-lib/src/network/proxies.rs +++ b/rust/agama-lib/src/network/proxies.rs @@ -193,9 +193,9 @@ trait Bond { /// ports property #[dbus_proxy(property)] - fn options(&self) -> zbus::Result>; + fn options(&self) -> zbus::Result; #[dbus_proxy(property)] - fn set_options(&self, value: &[&str]) -> zbus::Result<()>; + fn set_options(&self, value: &str) -> zbus::Result<()>; /// ports property #[dbus_proxy(property)] From 9a4f52134c8c850f0fd59e5c3a7729128acb55fd Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Tue, 21 Nov 2023 10:17:32 +0000 Subject: [PATCH 07/58] Bonding DBUS API fixes --- rust/agama-dbus-server/src/network/action.rs | 11 ++- rust/agama-dbus-server/src/network/dbus.rs | 1 + .../src/network/dbus/interfaces.rs | 67 ++++++++++------- rust/agama-dbus-server/src/network/model.rs | 72 +++++++++++++------ rust/agama-dbus-server/src/network/nm/dbus.rs | 5 +- rust/agama-dbus-server/src/network/system.rs | 11 ++- 6 files changed, 114 insertions(+), 53 deletions(-) diff --git a/rust/agama-dbus-server/src/network/action.rs b/rust/agama-dbus-server/src/network/action.rs index 025e9ba53a..184daea941 100644 --- a/rust/agama-dbus-server/src/network/action.rs +++ b/rust/agama-dbus-server/src/network/action.rs @@ -1,5 +1,10 @@ +use crate::network::dbus::ControllerSettings; use crate::network::model::Connection; use agama_lib::network::types::DeviceType; +use std::collections::HashMap; +use tokio::sync::oneshot; + +use super::error::NetworkStateError; /// Networking actions, like adding, updating or removing connections. /// @@ -12,7 +17,11 @@ pub enum Action { /// Update a connection (replacing the old one). UpdateConnection(Connection), /// Update a controller connection (replacing the old one). - UpdateControllerConnection(Connection, Vec), + UpdateControllerConnection( + Connection, + HashMap, + oneshot::Sender>, + ), /// Remove the connection with the given Uuid. RemoveConnection(String), /// Apply the current configuration. diff --git a/rust/agama-dbus-server/src/network/dbus.rs b/rust/agama-dbus-server/src/network/dbus.rs index ba95f4c9ac..08722762fc 100644 --- a/rust/agama-dbus-server/src/network/dbus.rs +++ b/rust/agama-dbus-server/src/network/dbus.rs @@ -7,5 +7,6 @@ mod interfaces; pub mod service; mod tree; +pub use interfaces::ControllerSettings; pub use service::NetworkService; pub(crate) use tree::{ObjectsRegistry, Tree}; diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index 304a5c5167..afc79e4c6c 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -16,7 +16,7 @@ use crate::network::{ use agama_lib::network::types::SSID; use std::sync::Arc; -use tokio::sync::mpsc::UnboundedSender; +use tokio::sync::{mpsc::UnboundedSender, oneshot}; use tokio::sync::{MappedMutexGuard, Mutex, MutexGuard}; use zbus::{ dbus_interface, @@ -288,7 +288,7 @@ impl Match { /// D-Bus interface for Bond settings pub struct Bond { - actions: Arc>>, + actions: Arc>>, connection: Arc>, } @@ -297,7 +297,10 @@ impl Bond { /// /// * `actions`: sending-half of a channel to send actions. /// * `connection`: connection to expose over D-Bus. - pub fn new(actions: Sender, connection: Arc>) -> Self { + pub fn new( + actions: UnboundedSender, + connection: Arc>, + ) -> Self { Self { actions: Arc::new(Mutex::new(actions)), connection, @@ -307,7 +310,7 @@ impl Bond { /// Gets the bond connection. /// /// Beware that it crashes when it is not a bond connection. - async fn get_bond(&self) -> MappedMutexGuard { + async fn get_bond(&self) -> MappedMutexGuard { MutexGuard::map(self.connection.lock().await, |c| match c { NetworkConnection::Bond(config) => config, _ => panic!("Not a bond connection. This is most probably a bug."), @@ -319,23 +322,22 @@ impl Bond { /// * `connection`: Updated connection. async fn update_controller_connection<'a>( &self, - connection: MappedMutexGuard<'a, NetworkConnection, BondConnection>, - settings: HashMap, - ) -> zbus::fdo::Result<()> { + connection: MappedMutexGuard<'a, BondConnection>, + settings: HashMap, + ) -> Result { let actions = self.actions.lock().await; let connection = NetworkConnection::Bond(connection.clone()); + let (tx, rx) = oneshot::channel(); actions - .send(Action::UpdateControllerConnection( - connection.clone(), - settings, - )) - .await + .send(Action::UpdateControllerConnection(connection, settings, tx)) .unwrap(); - Ok(()) + + rx.await.unwrap() } } -enum BondSettings { +#[derive(Debug, PartialEq, Clone)] +pub enum ControllerSettings { Ports(Vec), Options(String), } @@ -346,20 +348,24 @@ impl Bond { #[dbus_interface(property)] pub async fn options(&self) -> String { let connection = self.get_bond().await; - let mut opts = vec![]; - for (key, value) in &connection.bond.options { - opts.push(format!("{key}={value}")); - } - opts.join(" ") + connection.bond.options.to_string() } #[dbus_interface(property)] - pub async fn set_options(&self, opts: String) -> zbus::fdo::Result<()> { + pub async fn set_options(&mut self, opts: String) -> zbus::fdo::Result<()> { let connection = self.get_bond().await; - - self.update_controller_connection(connection, HashMap::from(["options", opts.clone()])) - .await + let result = self + .update_controller_connection( + connection, + HashMap::from([( + "options".to_string(), + ControllerSettings::Options(opts.clone()), + )]), + ) + .await; + self.connection = Arc::new(Mutex::new(result.unwrap())); + Ok(()) } /// List of bond ports. @@ -370,15 +376,24 @@ impl Bond { .bond .ports .iter() - .map(|port| port.base().interface.to_string()) + .map(|port| port.base().id.to_string()) .collect() } #[dbus_interface(property)] pub async fn set_ports(&mut self, ports: Vec) -> zbus::fdo::Result<()> { let connection = self.get_bond().await; - self.update_controller_connection(connection, HashMap::from(["ports", ports.clone()])) - .await + let result = self + .update_controller_connection( + connection, + HashMap::from([( + "ports".to_string(), + ControllerSettings::Ports(ports.clone()), + )]), + ) + .await; + self.connection = Arc::new(Mutex::new(result.unwrap())); + Ok(()) } } diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 537f638345..5868909219 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -16,6 +16,8 @@ use thiserror::Error; use uuid::Uuid; use zbus::zvariant::Value; +use super::dbus::ControllerSettings; + #[derive(Default, Clone)] pub struct NetworkState { pub devices: Vec, @@ -96,7 +98,7 @@ impl NetworkState { pub fn update_controller_connection( &mut self, mut conn: Connection, - ports: Vec, + settings: HashMap, ) -> Result<(), NetworkStateError> { // let Some(old_conn) = self.get_connection_mut(conn.id()) else { // return Err(NetworkStateError::UnknownConnection(conn.id().to_string())); @@ -105,29 +107,28 @@ impl NetworkState { let mut new_ports = vec![]; if let Connection::Bond(ref mut bond) = conn { - // if let Connection::Bond(old_bond) = old_conn { - // let moved_ports = old_bond.bond.ports.into_iter().filter(|p| ports.contains(&p.base().interface)); - // for port in old_bond.bond.ports.iter() { - // if ports.contains(&port.base().interface) { - // bond.bond.ports.push(port.clone()) - // } - // } - //} - - for port in ports.into_iter() { - new_ports.push( - if let Some(new_port) = bond.bond.ports.iter().find(|c| c.interface() == port) { - new_port.clone() - } else { - Connection::new(port, DeviceType::Ethernet) - }, - ); + if let Some(ControllerSettings::Options(opts)) = settings.get("options") { + bond.bond.options = BondOptions::try_from(opts.clone()).unwrap() } - bond.bond.ports = new_ports; + if let Some(ControllerSettings::Ports(ports)) = settings.get("ports") { + for port in ports.iter() { + new_ports.push( + if let Some(new_port) = + bond.bond.ports.iter().find(|c| c.interface() == port) + { + new_port.clone() + } else { + Connection::new(port.to_string(), DeviceType::Ethernet) + }, + ); + } + + bond.bond.ports = new_ports; + } } - Ok(()) + self.update_connection(conn) } /// Removes a connection from the state. @@ -549,10 +550,39 @@ pub struct BondConnection { pub bond: BondConfig, } +#[derive(Debug, Default, Clone, PartialEq)] +pub struct BondOptions(pub HashMap); + +impl TryFrom for BondOptions { + type Error = NetworkStateError; + + fn try_from(value: String) -> Result { + let mut options = HashMap::new(); + + for opt in value.split_whitespace() { + let opt_word: Vec<&str> = opt.trim().split('=').collect(); + options.insert(opt_word[0].to_string(), opt_word[1].to_string()); + } + + Ok(BondOptions(options)) + } +} + +impl fmt::Display for BondOptions { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let mut opts = vec![]; + for (key, value) in &self.0 { + opts.push(format!("{key}={value}")); + } + + write!(f, "{}", opts.join(" ")) + } +} + #[derive(Debug, Default, PartialEq, Clone)] pub struct BondConfig { pub ports: Vec, - pub options: HashMap, + pub options: BondOptions, } #[derive(Debug, Default, Clone, PartialEq)] diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index a4f9167a0a..2e2a350262 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -65,7 +65,6 @@ pub fn connection_from_dbus(conn: OwnedNestedHash) -> Option { } if let Some(bond_config) = bond_config_from_dbus(&conn) { - println!("Returning bond config"); return Some(Connection::Bond(BondConnection { base, bond: bond_config, @@ -251,7 +250,7 @@ fn wireless_config_to_dbus(conn: &WirelessConnection) -> NestedHash { fn bond_config_to_dbus(conn: &BondConnection) -> HashMap<&str, zvariant::Value> { let config = &conn.bond; - HashMap::from([("options", Value::new(config.options.clone()))]) + HashMap::from([("options", Value::new(config.options.0.clone()))]) } /// Converts a MatchConfig struct into a HashMap that can be sent over D-Bus. @@ -514,7 +513,7 @@ fn bond_config_from_dbus(conn: &OwnedNestedHash) -> Option { let options = >::try_from(dict.clone()).unwrap(); Some(BondConfig { - options: options, + options: BondOptions(options), ..Default::default() }) } diff --git a/rust/agama-dbus-server/src/network/system.rs b/rust/agama-dbus-server/src/network/system.rs index ded163ab57..eb7b757f03 100644 --- a/rust/agama-dbus-server/src/network/system.rs +++ b/rust/agama-dbus-server/src/network/system.rs @@ -73,8 +73,15 @@ impl NetworkSystem { Action::UpdateConnection(conn) => { self.state.update_connection(conn)?; } - Action::UpdateControllerConnection(conn, ports) => { - self.state.update_controller_connection(conn, ports)?; + Action::UpdateControllerConnection(conn, settings, tx) => { + let id = &conn.clone(); + let id = id.id(); + self.state.update_controller_connection(conn, settings)?; + if let Some(conn) = self.state.get_connection(id) { + tx.send(Ok(conn.clone())).unwrap(); + } + + dbg!(&self.state.connections); } Action::RemoveConnection(id) => { self.tree.remove_connection(&id).await?; From 00c41779cce6d9f6e371a4fdc8297a2b867597a9 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Wed, 22 Nov 2023 09:24:49 +0000 Subject: [PATCH 08/58] Fix device type DBUS method --- rust/agama-lib/src/network/client.rs | 2 +- rust/agama-lib/src/network/proxies.rs | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/rust/agama-lib/src/network/client.rs b/rust/agama-lib/src/network/client.rs index 837fe7e02f..836a95dff2 100644 --- a/rust/agama-lib/src/network/client.rs +++ b/rust/agama-lib/src/network/client.rs @@ -81,7 +81,7 @@ impl<'a> NetworkClient<'a> { .build() .await?; let name = device_proxy.name().await?; - let device_type = device_proxy.device_type().await?; + let device_type = device_proxy.type_().await?; Ok(Device { name, diff --git a/rust/agama-lib/src/network/proxies.rs b/rust/agama-lib/src/network/proxies.rs index 84335b6996..d37f9ca565 100644 --- a/rust/agama-lib/src/network/proxies.rs +++ b/rust/agama-lib/src/network/proxies.rs @@ -16,14 +16,15 @@ trait Devices { #[dbus_proxy( interface = "org.opensuse.Agama1.Network.Device", default_service = "org.opensuse.Agama1", - default_path = "/org/opensuse/Agama1/Network" + default_path = "/org/opensuse/Agama1/Network/devices/1" )] trait Device { - /// Id property + /// Name property #[dbus_proxy(property)] fn name(&self) -> zbus::Result; + /// Type property #[dbus_proxy(property)] - fn device_type(&self) -> zbus::Result; + fn type_(&self) -> zbus::Result; } #[dbus_proxy( From 802dbb606f9ba412e26bb2e5a0e4549cb645a230 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Wed, 22 Nov 2023 13:00:54 +0000 Subject: [PATCH 09/58] Add controller and interface name --- rust/agama-dbus-server/src/network/model.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 5868909219..966173b2d3 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -105,6 +105,7 @@ impl NetworkState { // }; // let mut new_ports = vec![]; + let controller = conn.clone(); if let Connection::Bond(ref mut bond) = conn { if let Some(ControllerSettings::Options(opts)) = settings.get("options") { @@ -119,7 +120,11 @@ impl NetworkState { { new_port.clone() } else { - Connection::new(port.to_string(), DeviceType::Ethernet) + let mut port_conn = + Connection::new(port.to_string(), DeviceType::Ethernet); + port_conn.set_interface(port); + port_conn.set_controller(controller.id()); + port_conn }, ); } From 25df7cffd3ccf00e59495ce1c821316dac1839d7 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Wed, 22 Nov 2023 20:55:39 +0000 Subject: [PATCH 10/58] Cleanup empty fields --- rust/agama-dbus-server/src/network/nm/client.rs | 1 + rust/agama-dbus-server/src/network/nm/dbus.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/rust/agama-dbus-server/src/network/nm/client.rs b/rust/agama-dbus-server/src/network/nm/client.rs index 8a5989caf4..ac3eef5586 100644 --- a/rust/agama-dbus-server/src/network/nm/client.rs +++ b/rust/agama-dbus-server/src/network/nm/client.rs @@ -130,6 +130,7 @@ impl<'a> NetworkManagerClient<'a> { ) }); } + self.activate_connection(path).await?; Ok(()) } diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index 2e2a350262..d6efe7a60e 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -48,6 +48,7 @@ pub fn connection_to_dbus(conn: &Connection) -> NestedHash { } result.insert("connection", connection_dbus); + cleanup_dbus_connection(&mut result); result } From 8284e5764d8e664d9f63b21e3d01ed2cd61cdbb0 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Thu, 23 Nov 2023 07:07:54 +0000 Subject: [PATCH 11/58] Wait for ports to be added --- rust/agama-dbus-server/src/network/nm/client.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/rust/agama-dbus-server/src/network/nm/client.rs b/rust/agama-dbus-server/src/network/nm/client.rs index ac3eef5586..2f94d4b797 100644 --- a/rust/agama-dbus-server/src/network/nm/client.rs +++ b/rust/agama-dbus-server/src/network/nm/client.rs @@ -95,12 +95,6 @@ impl<'a> NetworkManagerClient<'a> { } } } - for (controller, conns) in controllers { - dbg!(controller.to_string()); - for conn in conns { - dbg!(conn.id().to_string()); - } - } Ok(connections) } @@ -122,13 +116,14 @@ impl<'a> NetworkManagerClient<'a> { }; if let Connection::Bond(bond) = conn { - let _ = bond.bond.ports.iter().map(|port| { + for port in bond.bond.ports.clone() { self.add_or_update_port_connection( - port, + &port, bond.base.interface.to_string(), "bond".to_string(), ) - }); + .await?; + } } self.activate_connection(path).await?; @@ -142,6 +137,7 @@ impl<'a> NetworkManagerClient<'a> { port_type: String, ) -> Result<(), ServiceError> { let mut dbus_conn = connection_to_dbus(conn); + if let Some(new_conn) = dbus_conn.get_mut("connection") { new_conn.insert("slave-type", port_type.to_string().into()); new_conn.insert("master", controller.to_string().into()); From 4c0e1e8d2c5dccf1cc049c9a645c521f51d4bc8e Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Fri, 24 Nov 2023 09:39:10 +0000 Subject: [PATCH 12/58] Cleanup BondConfig code --- rust/agama-dbus-server/src/network/model.rs | 98 +++++++++++++++++---- 1 file changed, 79 insertions(+), 19 deletions(-) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 966173b2d3..d791c656a4 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -104,32 +104,28 @@ impl NetworkState { // return Err(NetworkStateError::UnknownConnection(conn.id().to_string())); // }; // - let mut new_ports = vec![]; let controller = conn.clone(); if let Connection::Bond(ref mut bond) = conn { if let Some(ControllerSettings::Options(opts)) = settings.get("options") { - bond.bond.options = BondOptions::try_from(opts.clone()).unwrap() + bond.set_options(opts.to_owned()); } if let Some(ControllerSettings::Ports(ports)) = settings.get("ports") { - for port in ports.iter() { - new_ports.push( - if let Some(new_port) = - bond.bond.ports.iter().find(|c| c.interface() == port) - { - new_port.clone() - } else { - let mut port_conn = - Connection::new(port.to_string(), DeviceType::Ethernet); - port_conn.set_interface(port); - port_conn.set_controller(controller.id()); - port_conn - }, - ); - } - - bond.bond.ports = new_ports; + let new_ports: Vec<_> = ports + .iter() + .map(|port| { + bond.find_port(port).cloned().unwrap_or_else(|| { + ConnectionBuilder::new(port) + .with_type(DeviceType::Ethernet) + .with_interface(port) + .with_controller(controller.id()) + .build() + }) + }) + .collect(); + + bond.set_ports(new_ports); } } @@ -270,6 +266,52 @@ pub struct Device { pub type_: DeviceType, } +#[derive(Debug, Default)] +pub struct ConnectionBuilder { + id: String, + interface: Option, + controller: Option, + type_: Option, +} + +impl ConnectionBuilder { + pub fn new(id: &str) -> Self { + Self { + id: id.to_string(), + ..Default::default() + } + } + + pub fn with_interface(mut self, interface: &str) -> Self { + self.interface = Some(interface.to_string()); + self + } + + pub fn with_controller(mut self, controller: &str) -> Self { + self.controller = Some(controller.to_string()); + self + } + + pub fn with_type(mut self, type_: DeviceType) -> Self { + self.type_ = Some(type_); + self + } + + pub fn build(self) -> Connection { + let mut conn = Connection::new(self.id, self.type_.unwrap_or(DeviceType::Ethernet)); + + if let Some(interface) = self.interface { + conn.set_interface(&interface); + } + + if let Some(controller) = self.controller { + conn.set_controller(&controller); + } + + conn + } +} + /// Represents an available network connection #[derive(Debug, PartialEq, Clone)] pub enum Connection { @@ -555,6 +597,24 @@ pub struct BondConnection { pub bond: BondConfig, } +impl BondConnection { + pub fn find_port(&self, name: &str) -> Option<&Connection> { + self.bond.ports.iter().find(|c| c.interface() == name) + } + + pub fn set_ports(&mut self, ports: Vec) { + self.bond.ports = ports; + } + + pub fn set_options(&mut self, options: T) + where + T: TryInto, + >::Error: std::fmt::Debug, + { + self.bond.options = options.try_into().unwrap() + } +} + #[derive(Debug, Default, Clone, PartialEq)] pub struct BondOptions(pub HashMap); From 0ccfff5c049b74f82dc63c6dba2742356574b08a Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Fri, 24 Nov 2023 10:50:00 +0000 Subject: [PATCH 13/58] Some fixes and adding a bond connection unit test --- .../src/network/nm/client.rs | 6 ++- rust/agama-dbus-server/src/network/nm/dbus.rs | 3 +- rust/agama-dbus-server/src/network/system.rs | 2 - rust/agama-dbus-server/tests/network.rs | 43 ++++++++++++++++++- rust/agama-lib/src/network/client.rs | 20 +++++++++ 5 files changed, 66 insertions(+), 8 deletions(-) diff --git a/rust/agama-dbus-server/src/network/nm/client.rs b/rust/agama-dbus-server/src/network/nm/client.rs index 2f94d4b797..7adacbafc4 100644 --- a/rust/agama-dbus-server/src/network/nm/client.rs +++ b/rust/agama-dbus-server/src/network/nm/client.rs @@ -2,7 +2,8 @@ use std::collections::HashMap; use super::dbus::{ - connection_from_dbus, connection_to_dbus, controller_from_dbus, merge_dbus_connections, + cleanup_dbus_connection, connection_from_dbus, connection_to_dbus, controller_from_dbus, + merge_dbus_connections, }; use super::model::NmDeviceType; use super::proxies::{ConnectionProxy, DeviceProxy, NetworkManagerProxy, SettingsProxy}; @@ -103,7 +104,7 @@ impl<'a> NetworkManagerClient<'a> { /// /// * `conn`: connection to add or update. pub async fn add_or_update_connection(&self, conn: &Connection) -> Result<(), ServiceError> { - let new_conn = connection_to_dbus(conn); + let mut new_conn = connection_to_dbus(conn); let path = if let Ok(proxy) = self.get_connection_proxy(conn.uuid()).await { let original = proxy.get_settings().await?; @@ -112,6 +113,7 @@ impl<'a> NetworkManagerClient<'a> { OwnedObjectPath::from(proxy.path().to_owned()) } else { let proxy = SettingsProxy::new(&self.connection).await?; + cleanup_dbus_connection(&mut new_conn); proxy.add_connection(new_conn).await? }; diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index d6efe7a60e..845c67bb8e 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -48,7 +48,6 @@ pub fn connection_to_dbus(conn: &Connection) -> NestedHash { } result.insert("connection", connection_dbus); - cleanup_dbus_connection(&mut result); result } @@ -117,7 +116,7 @@ pub fn merge_dbus_connections<'a>( /// replaced with "address-data". However, if "addresses" is present, it takes precedence. /// /// * `conn`: connection represented as a NestedHash. -fn cleanup_dbus_connection(conn: &mut NestedHash) { +pub fn cleanup_dbus_connection(conn: &mut NestedHash) { if let Some(connection) = conn.get_mut("connection") { if connection .get("interface-name") diff --git a/rust/agama-dbus-server/src/network/system.rs b/rust/agama-dbus-server/src/network/system.rs index eb7b757f03..bccabc7334 100644 --- a/rust/agama-dbus-server/src/network/system.rs +++ b/rust/agama-dbus-server/src/network/system.rs @@ -80,8 +80,6 @@ impl NetworkSystem { if let Some(conn) = self.state.get_connection(id) { tx.send(Ok(conn.clone())).unwrap(); } - - dbg!(&self.state.connections); } Action::RemoveConnection(id) => { self.tree.remove_connection(&id).await?; diff --git a/rust/agama-dbus-server/tests/network.rs b/rust/agama-dbus-server/tests/network.rs index 37d404210a..bf2923cd44 100644 --- a/rust/agama-dbus-server/tests/network.rs +++ b/rust/agama-dbus-server/tests/network.rs @@ -3,10 +3,14 @@ mod common; use self::common::{async_retry, DBusServer}; use agama_dbus_server::network::{ self, - model::{self, Ipv4Method, Ipv6Method}, + model::{self, BondConfig, Ipv4Method, Ipv6Method}, Adapter, NetworkService, NetworkState, }; -use agama_lib::network::{settings, types::DeviceType, NetworkClient}; +use agama_lib::network::{ + settings::{self, BondSettings}, + types::DeviceType, + NetworkClient, +}; use cidr::IpInet; use std::error::Error; use tokio::test; @@ -86,6 +90,41 @@ async fn test_add_connection() -> Result<(), Box> { assert_eq!(method4, &Ipv4Method::Auto.to_string()); let method6 = conn.method6.as_ref().unwrap(); assert_eq!(method6, &Ipv6Method::Disabled.to_string()); + + Ok(()) +} + +#[test] +async fn test_add_bond_connection() -> Result<(), Box> { + let mut server = DBusServer::new().start().await?; + + let adapter = NetworkTestAdapter(NetworkState::default()); + + let _service = NetworkService::start(&server.connection(), adapter).await?; + server.request_name().await?; + + let client = NetworkClient::new(server.connection().clone()).await?; + let bond0 = settings::NetworkConnection { + id: "bond0".to_string(), + method4: Some("auto".to_string()), + method6: Some("disabled".to_string()), + interface: Some("bond0".to_string()), + bond: Some(settings::BondSettings { + ports: vec!["eth0".to_string(), "eth1".to_string()], + options: "mode=active-backup primary=eth1".to_string(), + }), + ..Default::default() + }; + + client.add_or_update_connection(&bond0).await?; + println!("FETCHING CONNECTIONS"); + let conns = async_retry(|| client.connections()).await?; + assert_eq!(conns.len(), 1); + + let conn = conns.iter().find(|c| c.id == "bond0".to_string()).unwrap(); + assert_eq!(conn.id, "bond0"); + assert_eq!(conn.device_type(), DeviceType::Bond); + Ok(()) } diff --git a/rust/agama-lib/src/network/client.rs b/rust/agama-lib/src/network/client.rs index 836a95dff2..c46ff9e71b 100644 --- a/rust/agama-lib/src/network/client.rs +++ b/rust/agama-lib/src/network/client.rs @@ -51,6 +51,9 @@ impl<'a> NetworkClient<'a> { for path in connection_paths { let mut connection = self.connection_from(path.as_str()).await?; + if let Ok(bond) = self.bond_from(path.as_str()).await { + connection.bond = Some(bond); + } if let Ok(wireless) = self.wireless_from(path.as_str()).await { connection.wireless = Some(wireless); } @@ -130,6 +133,21 @@ impl<'a> NetworkClient<'a> { }) } + /// Returns the [bond settings][BondSettings] for the given connection + /// + /// * `path`: the connections path to get the wireless config from + async fn bond_from(&self, path: &str) -> Result { + let bond_proxy = BondProxy::builder(&self.connection) + .path(path)? + .build() + .await?; + let bond = BondSettings { + options: bond_proxy.options().await?, + ports: bond_proxy.ports().await?, + }; + + Ok(bond) + } /// Returns the [wireless settings][WirelessSettings] for the given connection /// /// * `path`: the connections path to get the wireless config from @@ -179,6 +197,7 @@ impl<'a> NetworkClient<'a> { Ok(path) => path, Err(_) => self.add_connection(conn).await?, }; + self.update_connection(&path, conn).await?; Ok(()) } @@ -297,6 +316,7 @@ impl<'a> NetworkClient<'a> { let ports: Vec<_> = bond.ports.iter().map(String::as_ref).collect(); proxy.set_ports(ports.as_slice()).await?; proxy.set_options(bond.options.to_string().as_str()).await?; + Ok(()) } /// Updates the wireless settings for network connection. From 729f5bda5eee65e9b55182fad6ad9b762306714b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 24 Nov 2023 15:25:54 +0000 Subject: [PATCH 14/58] Handle potential problems parsing bonding options --- rust/agama-dbus-server/src/network/error.rs | 2 ++ rust/agama-dbus-server/src/network/model.rs | 13 ++++++------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/rust/agama-dbus-server/src/network/error.rs b/rust/agama-dbus-server/src/network/error.rs index 06bafcf826..694028a1db 100644 --- a/rust/agama-dbus-server/src/network/error.rs +++ b/rust/agama-dbus-server/src/network/error.rs @@ -23,6 +23,8 @@ pub enum NetworkStateError { InvalidSecurityProtocol(String), #[error("Adapter error: '{0}'")] AdapterError(String), + #[error("Invalid bond options")] + InvalidBondOptions, } impl From for zbus::fdo::Error { diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index d791c656a4..1a4138a415 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -100,15 +100,11 @@ impl NetworkState { mut conn: Connection, settings: HashMap, ) -> Result<(), NetworkStateError> { - // let Some(old_conn) = self.get_connection_mut(conn.id()) else { - // return Err(NetworkStateError::UnknownConnection(conn.id().to_string())); - // }; - // let controller = conn.clone(); if let Connection::Bond(ref mut bond) = conn { if let Some(ControllerSettings::Options(opts)) = settings.get("options") { - bond.set_options(opts.to_owned()); + bond.set_options(opts.to_owned())?; } if let Some(ControllerSettings::Ports(ports)) = settings.get("ports") { @@ -606,12 +602,15 @@ impl BondConnection { self.bond.ports = ports; } - pub fn set_options(&mut self, options: T) + pub fn set_options(&mut self, options: T) -> Result<(), NetworkStateError> where T: TryInto, >::Error: std::fmt::Debug, { - self.bond.options = options.try_into().unwrap() + self.bond.options = options + .try_into() + .map_err(|e| NetworkStateError::InvalidBondOptions)?; + Ok(()) } } From 7aea974e8777c33b0c29156624be9d5a059843a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 24 Nov 2023 15:33:40 +0000 Subject: [PATCH 15/58] controller_from_dbus does not consume the argument * Just for consistency reasons. --- rust/agama-dbus-server/src/network/nm/client.rs | 2 +- rust/agama-dbus-server/src/network/nm/dbus.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/agama-dbus-server/src/network/nm/client.rs b/rust/agama-dbus-server/src/network/nm/client.rs index 7adacbafc4..14a74c2c92 100644 --- a/rust/agama-dbus-server/src/network/nm/client.rs +++ b/rust/agama-dbus-server/src/network/nm/client.rs @@ -86,7 +86,7 @@ impl<'a> NetworkManagerClient<'a> { // TODO: log an error if a connection is not found if let Some(connection) = connection_from_dbus(settings.clone()) { - if let Some(controller) = controller_from_dbus(settings) { + if let Some(controller) = controller_from_dbus(&settings) { controllers .entry(controller) .or_insert_with(Vec::new) diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index 845c67bb8e..5975aa539a 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -287,7 +287,7 @@ fn match_config_to_dbus(match_config: &MatchConfig) -> HashMap<&str, zvariant::V ("interface-name", interfaces), ]) } -pub fn controller_from_dbus(conn: OwnedNestedHash) -> Option { +pub fn controller_from_dbus(conn: &OwnedNestedHash) -> Option { let Some(connection) = conn.get("connection") else { return None; }; From 864376f2d756725c2fa56eade0c7e26627f6ada5 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Fri, 24 Nov 2023 16:18:29 +0000 Subject: [PATCH 16/58] Some small fixed based on CR --- rust/agama-dbus-server/src/network/model.rs | 24 +++++++++----------- rust/agama-dbus-server/src/network/system.rs | 5 ++-- rust/agama-lib/src/network/client.rs | 1 + 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 1a4138a415..7cdbcf87bc 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -57,13 +57,6 @@ impl NetworkState { self.connections.iter_mut().find(|c| c.id() == id) } - /// Get connection by name - /// - /// * `name`: connection name - pub fn get_connection_by_name(&self, id: &str) -> Option<&Connection> { - self.connections.iter().find(|c| c.interface() == id) - } - /// Adds a new connection. /// /// It uses the `id` to decide whether the connection already exists. @@ -373,10 +366,14 @@ impl Connection { self.base_mut().interface = interface.to_string() } + /// Ports controller name, e.g.: bond0, br0 pub fn controller(&self) -> &str { self.base().controller.as_str() } + /// Sets the ports controller. + /// + /// `controller`: Name of the controller (Bridge, Bond, Team), e.g.: bond0. pub fn set_controller(&mut self, controller: &str) { self.base_mut().controller = controller.to_string() } @@ -624,8 +621,8 @@ impl TryFrom for BondOptions { let mut options = HashMap::new(); for opt in value.split_whitespace() { - let opt_word: Vec<&str> = opt.trim().split('=').collect(); - options.insert(opt_word[0].to_string(), opt_word[1].to_string()); + let (opt_key, opt_value) = opt.trim().split_once('=').unwrap(); + options.insert(opt_key.to_string(), opt_value.to_string()); } Ok(BondOptions(options)) @@ -634,10 +631,11 @@ impl TryFrom for BondOptions { impl fmt::Display for BondOptions { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut opts = vec![]; - for (key, value) in &self.0 { - opts.push(format!("{key}={value}")); - } + let opts = &self + .0 + .iter() + .map(|(key, value)| format!("{key}={value}")) + .collect::>(); write!(f, "{}", opts.join(" ")) } diff --git a/rust/agama-dbus-server/src/network/system.rs b/rust/agama-dbus-server/src/network/system.rs index bccabc7334..134833c58d 100644 --- a/rust/agama-dbus-server/src/network/system.rs +++ b/rust/agama-dbus-server/src/network/system.rs @@ -74,10 +74,9 @@ impl NetworkSystem { self.state.update_connection(conn)?; } Action::UpdateControllerConnection(conn, settings, tx) => { - let id = &conn.clone(); - let id = id.id(); + let id = conn.id().to_owned(); self.state.update_controller_connection(conn, settings)?; - if let Some(conn) = self.state.get_connection(id) { + if let Some(conn) = self.state.get_connection(&id) { tx.send(Ok(conn.clone())).unwrap(); } } diff --git a/rust/agama-lib/src/network/client.rs b/rust/agama-lib/src/network/client.rs index c46ff9e71b..f0bb5534e4 100644 --- a/rust/agama-lib/src/network/client.rs +++ b/rust/agama-lib/src/network/client.rs @@ -54,6 +54,7 @@ impl<'a> NetworkClient<'a> { if let Ok(bond) = self.bond_from(path.as_str()).await { connection.bond = Some(bond); } + if let Ok(wireless) = self.wireless_from(path.as_str()).await { connection.wireless = Some(wireless); } From d278bb6c3c0ee915a9d66e3e494b0a0735dd5e23 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Fri, 24 Nov 2023 16:31:27 +0000 Subject: [PATCH 17/58] Prefix unused error variable --- rust/agama-dbus-server/src/network/model.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 7cdbcf87bc..69d093f1a3 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -606,7 +606,7 @@ impl BondConnection { { self.bond.options = options .try_into() - .map_err(|e| NetworkStateError::InvalidBondOptions)?; + .map_err(|_e| NetworkStateError::InvalidBondOptions)?; Ok(()) } } From 8b221e8df2aefe7ecd027239dda7367b5523ca8b Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Mon, 27 Nov 2023 09:05:55 +0000 Subject: [PATCH 18/58] Use the WIRELESS constants --- rust/agama-dbus-server/src/network/nm/dbus.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index 5975aa539a..173675bb1a 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -241,10 +241,7 @@ fn wireless_config_to_dbus(conn: &WirelessConnection) -> NestedHash { security.insert("psk", password.to_string().into()); } - NestedHash::from([ - ("802-11-wireless", wireless), - ("802-11-wireless-security", security), - ]) + NestedHash::from([(WIRELESS_KEY, wireless), (WIRELESS_SECURITY_KEY, security)]) } fn bond_config_to_dbus(conn: &BondConnection) -> HashMap<&str, zvariant::Value> { @@ -537,7 +534,10 @@ mod test { connection_from_dbus, connection_to_dbus, merge_dbus_connections, NestedHash, OwnedNestedHash, }; - use crate::network::{model::*, nm::dbus::ETHERNET_KEY}; + use crate::network::{ + model::*, + nm::dbus::{ETHERNET_KEY, WIRELESS_KEY, WIRELESS_SECURITY_KEY}, + }; use agama_lib::network::types::SSID; use cidr::IpInet; use std::{collections::HashMap, net::IpAddr, str::FromStr}; @@ -695,8 +695,8 @@ mod test { let dbus_conn = HashMap::from([ ("connection".to_string(), connection_section), - ("802-11-wireless".to_string(), wireless_section), - ("802-11-wireless-security".to_string(), security_section), + (WIRELESS_KEY.to_string(), wireless_section), + (WIRELESS_SECURITY_KEY.to_string(), security_section), ]); let connection = connection_from_dbus(dbus_conn).unwrap(); @@ -724,7 +724,7 @@ mod test { let wireless = Connection::Wireless(wireless); let wireless_dbus = connection_to_dbus(&wireless); - let wireless = wireless_dbus.get("802-11-wireless").unwrap(); + let wireless = wireless_dbus.get(WIRELESS_KEY).unwrap(); let mode: &str = wireless.get("mode").unwrap().downcast_ref().unwrap(); assert_eq!(mode, "infrastructure"); @@ -736,7 +736,7 @@ mod test { .collect(); assert_eq!(ssid, "agama".as_bytes()); - let security = wireless_dbus.get("802-11-wireless-security").unwrap(); + let security = wireless_dbus.get(WIRELESS_SECURITY_KEY).unwrap(); let key_mgmt: &str = security.get("key-mgmt").unwrap().downcast_ref().unwrap(); assert_eq!(key_mgmt, "wpa-psk"); } From f1699fad0449d7ffe52683b864b39b4ff643e170 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Mon, 27 Nov 2023 09:13:28 +0000 Subject: [PATCH 19/58] Moved ControllerSettings from dbus to the model --- rust/agama-dbus-server/src/network/action.rs | 3 +-- rust/agama-dbus-server/src/network/dbus.rs | 1 - rust/agama-dbus-server/src/network/dbus/interfaces.rs | 10 ++-------- rust/agama-dbus-server/src/network/model.rs | 9 +++++++-- 4 files changed, 10 insertions(+), 13 deletions(-) diff --git a/rust/agama-dbus-server/src/network/action.rs b/rust/agama-dbus-server/src/network/action.rs index 184daea941..7cfe8b1c52 100644 --- a/rust/agama-dbus-server/src/network/action.rs +++ b/rust/agama-dbus-server/src/network/action.rs @@ -1,5 +1,4 @@ -use crate::network::dbus::ControllerSettings; -use crate::network::model::Connection; +use crate::network::model::{Connection, ControllerSettings}; use agama_lib::network::types::DeviceType; use std::collections::HashMap; use tokio::sync::oneshot; diff --git a/rust/agama-dbus-server/src/network/dbus.rs b/rust/agama-dbus-server/src/network/dbus.rs index 08722762fc..ba95f4c9ac 100644 --- a/rust/agama-dbus-server/src/network/dbus.rs +++ b/rust/agama-dbus-server/src/network/dbus.rs @@ -7,6 +7,5 @@ mod interfaces; pub mod service; mod tree; -pub use interfaces::ControllerSettings; pub use service::NetworkService; pub(crate) use tree::{ObjectsRegistry, Tree}; diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index afc79e4c6c..b8299ff114 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -9,8 +9,8 @@ use crate::network::{ action::Action, error::NetworkStateError, model::{ - BondConnection, Connection as NetworkConnection, Device as NetworkDevice, - WirelessConnection, + BondConnection, Connection as NetworkConnection, ControllerSettings, + Device as NetworkDevice, WirelessConnection, }, }; @@ -336,12 +336,6 @@ impl Bond { } } -#[derive(Debug, PartialEq, Clone)] -pub enum ControllerSettings { - Ports(Vec), - Options(String), -} - #[dbus_interface(name = "org.opensuse.Agama1.Network.Connection.Bond")] impl Bond { /// List of bond ports. diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 69d093f1a3..ac148eeea6 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -16,8 +16,6 @@ use thiserror::Error; use uuid::Uuid; use zbus::zvariant::Value; -use super::dbus::ControllerSettings; - #[derive(Default, Clone)] pub struct NetworkState { pub devices: Vec, @@ -647,6 +645,13 @@ pub struct BondConfig { pub options: BondOptions, } +/// Controller settings payload for updating a controller's connection (Bond, Bridge) +#[derive(Debug, PartialEq, Clone)] +pub enum ControllerSettings { + Ports(Vec), + Options(String), +} + #[derive(Debug, Default, Clone, PartialEq)] pub struct WirelessConfig { pub mode: WirelessMode, From cdfe151759bbcce3f56447ddd9dc3d4f4b64ec19 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Mon, 27 Nov 2023 17:41:53 +0000 Subject: [PATCH 20/58] Renamed as suggested during CR --- rust/agama-dbus-server/src/network/action.rs | 4 ++-- rust/agama-dbus-server/src/network/dbus/interfaces.rs | 8 ++++---- rust/agama-dbus-server/src/network/model.rs | 10 +++++----- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/rust/agama-dbus-server/src/network/action.rs b/rust/agama-dbus-server/src/network/action.rs index 7cfe8b1c52..f56459e753 100644 --- a/rust/agama-dbus-server/src/network/action.rs +++ b/rust/agama-dbus-server/src/network/action.rs @@ -1,4 +1,4 @@ -use crate::network::model::{Connection, ControllerSettings}; +use crate::network::model::{Connection, ControllerConfig}; use agama_lib::network::types::DeviceType; use std::collections::HashMap; use tokio::sync::oneshot; @@ -18,7 +18,7 @@ pub enum Action { /// Update a controller connection (replacing the old one). UpdateControllerConnection( Connection, - HashMap, + HashMap, oneshot::Sender>, ), /// Remove the connection with the given Uuid. diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index b8299ff114..91ff94225b 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -9,7 +9,7 @@ use crate::network::{ action::Action, error::NetworkStateError, model::{ - BondConnection, Connection as NetworkConnection, ControllerSettings, + BondConnection, Connection as NetworkConnection, ControllerConfig, Device as NetworkDevice, WirelessConnection, }, }; @@ -323,7 +323,7 @@ impl Bond { async fn update_controller_connection<'a>( &self, connection: MappedMutexGuard<'a, BondConnection>, - settings: HashMap, + settings: HashMap, ) -> Result { let actions = self.actions.lock().await; let connection = NetworkConnection::Bond(connection.clone()); @@ -354,7 +354,7 @@ impl Bond { connection, HashMap::from([( "options".to_string(), - ControllerSettings::Options(opts.clone()), + ControllerConfig::Options(opts.clone()), )]), ) .await; @@ -382,7 +382,7 @@ impl Bond { connection, HashMap::from([( "ports".to_string(), - ControllerSettings::Ports(ports.clone()), + ControllerConfig::Ports(ports.clone()), )]), ) .await; diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index ac148eeea6..74897899a0 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -89,16 +89,16 @@ impl NetworkState { pub fn update_controller_connection( &mut self, mut conn: Connection, - settings: HashMap, + settings: HashMap, ) -> Result<(), NetworkStateError> { let controller = conn.clone(); if let Connection::Bond(ref mut bond) = conn { - if let Some(ControllerSettings::Options(opts)) = settings.get("options") { + if let Some(ControllerConfig::Options(opts)) = settings.get("options") { bond.set_options(opts.to_owned())?; } - if let Some(ControllerSettings::Ports(ports)) = settings.get("ports") { + if let Some(ControllerConfig::Ports(ports)) = settings.get("ports") { let new_ports: Vec<_> = ports .iter() .map(|port| { @@ -645,9 +645,9 @@ pub struct BondConfig { pub options: BondOptions, } -/// Controller settings payload for updating a controller's connection (Bond, Bridge) +/// Controller config payload for updating a controller's connection (Bond, Bridge) #[derive(Debug, PartialEq, Clone)] -pub enum ControllerSettings { +pub enum ControllerConfig { Ports(Vec), Options(String), } From e958fd2548c23653e232b4435d30dd880688d607 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Tue, 28 Nov 2023 11:15:11 +0000 Subject: [PATCH 21/58] Fix lint formatting reported errors --- rust/agama-dbus-server/src/network/dbus/interfaces.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index 91ff94225b..0dc82d84e1 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -9,8 +9,8 @@ use crate::network::{ action::Action, error::NetworkStateError, model::{ - BondConnection, Connection as NetworkConnection, ControllerConfig, - Device as NetworkDevice, WirelessConnection, + BondConnection, Connection as NetworkConnection, ControllerConfig, Device as NetworkDevice, + WirelessConnection, }, }; @@ -380,10 +380,7 @@ impl Bond { let result = self .update_controller_connection( connection, - HashMap::from([( - "ports".to_string(), - ControllerConfig::Ports(ports.clone()), - )]), + HashMap::from([("ports".to_string(), ControllerConfig::Ports(ports.clone()))]), ) .await; self.connection = Arc::new(Mutex::new(result.unwrap())); From b917e3e53b9812420f22c8976682e7e345c00762 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Thu, 30 Nov 2023 09:24:16 +0000 Subject: [PATCH 22/58] Added the Bond mode explicitly to the settings --- .../src/network/dbus/interfaces.rs | 21 +++++ rust/agama-dbus-server/src/network/error.rs | 2 + rust/agama-dbus-server/src/network/model.rs | 28 ++++-- rust/agama-dbus-server/src/network/nm/dbus.rs | 48 ++++++++-- rust/agama-dbus-server/tests/network.rs | 6 +- rust/agama-lib/src/network/client.rs | 8 +- rust/agama-lib/src/network/settings.rs | 26 +++++- rust/agama-lib/src/network/types.rs | 90 +++++++++++++++++++ 8 files changed, 210 insertions(+), 19 deletions(-) diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index 0dc82d84e1..f356753006 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -338,6 +338,27 @@ impl Bond { #[dbus_interface(name = "org.opensuse.Agama1.Network.Connection.Bond")] impl Bond { + /// Bonding mode + #[dbus_interface(property)] + pub async fn mode(&self) -> String { + let connection = self.get_bond().await; + + connection.bond.mode.to_string() + } + + #[dbus_interface(property)] + pub async fn set_mode(&mut self, mode: String) -> zbus::fdo::Result<()> { + let connection = self.get_bond().await; + let result = self + .update_controller_connection( + connection, + HashMap::from([("mode".to_string(), ControllerConfig::Mode(mode.clone()))]), + ) + .await; + self.connection = Arc::new(Mutex::new(result.unwrap())); + Ok(()) + } + /// List of bond ports. #[dbus_interface(property)] pub async fn options(&self) -> String { diff --git a/rust/agama-dbus-server/src/network/error.rs b/rust/agama-dbus-server/src/network/error.rs index 694028a1db..bf2b4d6242 100644 --- a/rust/agama-dbus-server/src/network/error.rs +++ b/rust/agama-dbus-server/src/network/error.rs @@ -23,6 +23,8 @@ pub enum NetworkStateError { InvalidSecurityProtocol(String), #[error("Adapter error: '{0}'")] AdapterError(String), + #[error("Invalid bond mode '{0}'")] + InvalidBondMode(String), #[error("Invalid bond options")] InvalidBondOptions, } diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 74897899a0..9edf844eba 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -3,7 +3,7 @@ //! * This module contains the types that represent the network concepts. They are supposed to be //! agnostic from the real network service (e.g., NetworkManager). use crate::network::error::NetworkStateError; -use agama_lib::network::types::{DeviceType, SSID}; +use agama_lib::network::types::{BondMode, DeviceType, SSID}; use cidr::IpInet; use std::{ collections::HashMap, @@ -94,8 +94,11 @@ impl NetworkState { let controller = conn.clone(); if let Connection::Bond(ref mut bond) = conn { + if let Some(ControllerConfig::Mode(mode)) = settings.get("mode") { + bond.set_mode(mode)?; + } if let Some(ControllerConfig::Options(opts)) = settings.get("options") { - bond.set_options(opts.to_owned())?; + bond.set_options(opts.as_str())?; } if let Some(ControllerConfig::Ports(ports)) = settings.get("ports") { @@ -597,6 +600,12 @@ impl BondConnection { self.bond.ports = ports; } + pub fn set_mode(&mut self, mode: &str) -> Result<(), NetworkStateError> { + self.bond.mode = BondMode::try_from(mode) + .map_err(|_e| NetworkStateError::InvalidBondMode(mode.to_string()))?; + Ok(()) + } + pub fn set_options(&mut self, options: T) -> Result<(), NetworkStateError> where T: TryInto, @@ -612,15 +621,18 @@ impl BondConnection { #[derive(Debug, Default, Clone, PartialEq)] pub struct BondOptions(pub HashMap); -impl TryFrom for BondOptions { +impl TryFrom<&str> for BondOptions { type Error = NetworkStateError; - fn try_from(value: String) -> Result { + fn try_from(value: &str) -> Result { let mut options = HashMap::new(); for opt in value.split_whitespace() { - let (opt_key, opt_value) = opt.trim().split_once('=').unwrap(); - options.insert(opt_key.to_string(), opt_value.to_string()); + let (key, value) = opt + .trim() + .split_once('=') + .ok_or(NetworkStateError::InvalidBondOptions)?; + options.insert(key.to_string(), value.to_string()); } Ok(BondOptions(options)) @@ -641,6 +653,7 @@ impl fmt::Display for BondOptions { #[derive(Debug, Default, PartialEq, Clone)] pub struct BondConfig { + pub mode: BondMode, pub ports: Vec, pub options: BondOptions, } @@ -648,8 +661,9 @@ pub struct BondConfig { /// Controller config payload for updating a controller's connection (Bond, Bridge) #[derive(Debug, PartialEq, Clone)] pub enum ControllerConfig { - Ports(Vec), + Mode(String), Options(String), + Ports(Vec), } #[derive(Debug, Default, Clone, PartialEq)] diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index 173675bb1a..aebe63b48f 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -6,7 +6,7 @@ use super::model::*; use crate::network::model::*; use agama_lib::{ dbus::{NestedHash, OwnedNestedHash}, - network::types::SSID, + network::types::{BondMode, SSID}, }; use cidr::IpInet; use std::{collections::HashMap, net::IpAddr, str::FromStr}; @@ -507,12 +507,20 @@ fn bond_config_from_dbus(conn: &OwnedNestedHash) -> Option { }; let dict: &zvariant::Dict = bond.get("options")?.downcast_ref()?; - let options = >::try_from(dict.clone()).unwrap(); - Some(BondConfig { + let mut options = >::try_from(dict.clone()).unwrap(); + let mode = options.remove("mode"); + + let mut bond = BondConfig { options: BondOptions(options), ..Default::default() - }) + }; + + if let Some(mode) = mode { + bond.mode = BondMode::try_from(mode.as_str()).unwrap_or_default(); + } + + Some(bond) } /// Determines whether a value is empty. @@ -536,9 +544,9 @@ mod test { }; use crate::network::{ model::*, - nm::dbus::{ETHERNET_KEY, WIRELESS_KEY, WIRELESS_SECURITY_KEY}, + nm::dbus::{BOND_KEY, ETHERNET_KEY, WIRELESS_KEY, WIRELESS_SECURITY_KEY}, }; - use agama_lib::network::types::SSID; + use agama_lib::network::types::{BondMode, SSID}; use cidr::IpInet; use std::{collections::HashMap, net::IpAddr, str::FromStr}; use uuid::Uuid; @@ -708,6 +716,34 @@ mod test { } } + #[test] + fn test_connection_from_dbus_bonding() { + let uuid = Uuid::new_v4().to_string(); + let connection_section = HashMap::from([ + ("id".to_string(), Value::new("bond0").to_owned()), + ("uuid".to_string(), Value::new(uuid).to_owned()), + ]); + + let bond_options = Value::new(HashMap::from([( + "options".to_string(), + HashMap::from([("mode".to_string(), Value::new("active-backup").to_owned())]), + )])); + + let dbus_conn = HashMap::from([ + ( + "connection".to_string(), + connection_section.try_into().unwrap(), + ), + (BOND_KEY.to_string(), bond_options.try_into().unwrap()), + ]); + + let connection = connection_from_dbus(dbus_conn).unwrap(); + assert!(matches!(connection, Connection::Bond(_))); + if let Connection::Bond(connection) = connection { + assert_eq!(connection.bond.mode, BondMode::ActiveBackup); + } + } + #[test] fn test_dbus_from_wireless_connection() { let config = WirelessConfig { diff --git a/rust/agama-dbus-server/tests/network.rs b/rust/agama-dbus-server/tests/network.rs index bf2923cd44..3557d66f7b 100644 --- a/rust/agama-dbus-server/tests/network.rs +++ b/rust/agama-dbus-server/tests/network.rs @@ -110,20 +110,22 @@ async fn test_add_bond_connection() -> Result<(), Box> { method6: Some("disabled".to_string()), interface: Some("bond0".to_string()), bond: Some(settings::BondSettings { + mode: "active-backup".to_string(), ports: vec!["eth0".to_string(), "eth1".to_string()], - options: "mode=active-backup primary=eth1".to_string(), + options: Some("primary=eth1".to_string()), }), ..Default::default() }; client.add_or_update_connection(&bond0).await?; - println!("FETCHING CONNECTIONS"); let conns = async_retry(|| client.connections()).await?; assert_eq!(conns.len(), 1); let conn = conns.iter().find(|c| c.id == "bond0".to_string()).unwrap(); assert_eq!(conn.id, "bond0"); assert_eq!(conn.device_type(), DeviceType::Bond); + let bond = conn.bond.clone().unwrap(); + assert_eq!(bond.mode, "active-backup"); Ok(()) } diff --git a/rust/agama-lib/src/network/client.rs b/rust/agama-lib/src/network/client.rs index f0bb5534e4..36ceba5e65 100644 --- a/rust/agama-lib/src/network/client.rs +++ b/rust/agama-lib/src/network/client.rs @@ -143,7 +143,8 @@ impl<'a> NetworkClient<'a> { .build() .await?; let bond = BondSettings { - options: bond_proxy.options().await?, + mode: bond_proxy.mode().await?, + options: Some(bond_proxy.options().await?), ports: bond_proxy.ports().await?, }; @@ -316,7 +317,10 @@ impl<'a> NetworkClient<'a> { let ports: Vec<_> = bond.ports.iter().map(String::as_ref).collect(); proxy.set_ports(ports.as_slice()).await?; - proxy.set_options(bond.options.to_string().as_str()).await?; + if let Some(ref options) = bond.options { + proxy.set_options(options.to_string().as_str()).await?; + } + proxy.set_mode(bond.mode.as_str()).await?; Ok(()) } diff --git a/rust/agama-lib/src/network/settings.rs b/rust/agama-lib/src/network/settings.rs index 4d9db9106d..22659452bf 100644 --- a/rust/agama-lib/src/network/settings.rs +++ b/rust/agama-lib/src/network/settings.rs @@ -48,12 +48,25 @@ pub struct WirelessSettings { pub mode: String, } -#[derive(Clone, Debug, Default, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct BondSettings { - pub options: String, + pub mode: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub options: Option, + #[serde(skip_serializing_if = "Vec::is_empty", default)] pub ports: Vec, } +impl Default for BondSettings { + fn default() -> Self { + Self { + mode: "round-robin".to_string(), + options: None, + ports: vec![], + } + } +} + #[derive(Clone, Debug, Serialize, Deserialize)] pub struct NetworkDevice { pub id: String, @@ -151,6 +164,15 @@ mod tests { assert_eq!(bond.device_type(), DeviceType::Bond); } + #[test] + fn test_bonding_defaults() { + let bond = BondSettings::default(); + + assert_eq!(bond.mode, "round-robin".to_string()); + assert_eq!(bond.ports.len(), 0); + assert_eq!(bond.options, None); + } + #[test] fn test_add_connection_to_setting() { let name = SettingValue("Ethernet 1".to_string()); diff --git a/rust/agama-lib/src/network/types.rs b/rust/agama-lib/src/network/types.rs index e51bbcf975..30969e3238 100644 --- a/rust/agama-lib/src/network/types.rs +++ b/rust/agama-lib/src/network/types.rs @@ -38,6 +38,90 @@ pub enum DeviceType { Wireless = 2, Bond = 3, } +/// Bond mode +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Copy)] +pub enum BondMode { + #[serde(rename = "balance-rr")] + RoundRobin = 0, + #[serde(rename = "active-backup")] + ActiveBackup = 1, + #[serde(rename = "balance-xor")] + BalanceXOR = 2, + #[serde(rename = "broadcast")] + Broadcast = 3, + #[serde(rename = "802.3ad")] + LACP = 4, + #[serde(rename = "balance-tlb")] + BalanceTLB = 5, + #[serde(rename = "balance-alb")] + BalanceALB = 6, +} +impl Default for BondMode { + fn default() -> Self { + Self::RoundRobin + } +} + +impl std::fmt::Display for BondMode { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{}", + match self { + BondMode::RoundRobin => "balance-rr", + BondMode::ActiveBackup => "active-backup", + BondMode::BalanceXOR => "balance-xor", + BondMode::Broadcast => "broadcast", + BondMode::LACP => "802.3ad", + BondMode::BalanceTLB => "balance-tlb", + BondMode::BalanceALB => "balance-alb", + } + ) + } +} + +#[derive(Debug, Error, PartialEq)] +#[error("Invalid bond mode: {0}")] +pub struct InvalidBondMode(String); + +impl TryFrom<&str> for BondMode { + type Error = InvalidBondMode; + + fn try_from(value: &str) -> Result { + match value { + "round-robin" => Ok(BondMode::RoundRobin), + "active-backup" => Ok(BondMode::ActiveBackup), + "balance-xor" => Ok(BondMode::BalanceXOR), + "broadcast" => Ok(BondMode::Broadcast), + "802.3ad" => Ok(BondMode::LACP), + "balance-tlb" => Ok(BondMode::BalanceTLB), + "balance-alb" => Ok(BondMode::BalanceALB), + _ => Err(InvalidBondMode(value.to_string())), + } + } +} +impl TryFrom for BondMode { + type Error = InvalidBondMode; + + fn try_from(value: u8) -> Result { + match value { + 0 => Ok(BondMode::RoundRobin), + 1 => Ok(BondMode::ActiveBackup), + 2 => Ok(BondMode::BalanceXOR), + 3 => Ok(BondMode::Broadcast), + 4 => Ok(BondMode::LACP), + 5 => Ok(BondMode::BalanceTLB), + 6 => Ok(BondMode::BalanceALB), + _ => Err(InvalidBondMode(value.to_string())), + } + } +} + +impl From for zbus::fdo::Error { + fn from(value: InvalidBondMode) -> zbus::fdo::Error { + zbus::fdo::Error::Failed(format!("Network error: {value}")) + } +} #[derive(Debug, Error, PartialEq)] #[error("Invalid device type: {0}")] @@ -88,4 +172,10 @@ mod tests { let dtype = DeviceType::try_from(128); assert_eq!(dtype, Err(InvalidDeviceType(128))); } + + #[test] + fn test_display_bond_mode() { + let mode = BondMode::try_from(1).unwrap(); + assert_eq!(format!("{}", mode), "active-backup"); + } } From c85c68b849f078e5f27c7daed7ff727a14145125 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Thu, 30 Nov 2023 10:00:53 +0000 Subject: [PATCH 23/58] Add the bonding mode to the options --- rust/agama-dbus-server/src/network/nm/dbus.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index aebe63b48f..c6cabdf562 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -247,7 +247,10 @@ fn wireless_config_to_dbus(conn: &WirelessConnection) -> NestedHash { fn bond_config_to_dbus(conn: &BondConnection) -> HashMap<&str, zvariant::Value> { let config = &conn.bond; - HashMap::from([("options", Value::new(config.options.0.clone()))]) + let mut options = config.options.0.clone(); + options.insert("mode".to_string(), config.mode.to_string()); + + HashMap::from([("options", Value::new(options))]) } /// Converts a MatchConfig struct into a HashMap that can be sent over D-Bus. From ae62b01736926f4ffb4c80a466c1f2dc31259323 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Thu, 30 Nov 2023 12:58:21 +0000 Subject: [PATCH 24/58] Handle better update controller errors --- .../src/network/dbus/interfaces.rs | 2 +- rust/agama-dbus-server/src/network/system.rs | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index f356753006..b6a3e8b4c6 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -355,7 +355,7 @@ impl Bond { HashMap::from([("mode".to_string(), ControllerConfig::Mode(mode.clone()))]), ) .await; - self.connection = Arc::new(Mutex::new(result.unwrap())); + self.connection = Arc::new(Mutex::new(result?)); Ok(()) } diff --git a/rust/agama-dbus-server/src/network/system.rs b/rust/agama-dbus-server/src/network/system.rs index 134833c58d..c70d364b18 100644 --- a/rust/agama-dbus-server/src/network/system.rs +++ b/rust/agama-dbus-server/src/network/system.rs @@ -75,10 +75,16 @@ impl NetworkSystem { } Action::UpdateControllerConnection(conn, settings, tx) => { let id = conn.id().to_owned(); - self.state.update_controller_connection(conn, settings)?; - if let Some(conn) = self.state.get_connection(&id) { - tx.send(Ok(conn.clone())).unwrap(); - } + let result = self + .state + .update_controller_connection(conn, settings) + .and_then(|_| { + self.state.get_connection(&id).ok_or( + super::error::NetworkStateError::UnknownConnection(id.to_string()), + ) + }); + + tx.send(result.cloned()).unwrap(); } Action::RemoveConnection(id) => { self.tree.remove_connection(&id).await?; From 89046e113653f6dc9cbb6d38548691b7259ffe8a Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Fri, 1 Dec 2023 14:29:37 +0000 Subject: [PATCH 25/58] Remove options and mode from the ControllerConfig --- .../src/network/dbus/interfaces.rs | 46 +++++++++---------- rust/agama-dbus-server/src/network/model.rs | 9 ---- 2 files changed, 23 insertions(+), 32 deletions(-) diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index b6a3e8b4c6..5eb77a4d81 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -317,6 +317,19 @@ impl Bond { }) } + /// Updates the connection data in the NetworkSystem. + /// + /// * `connection`: Updated connection. + async fn update_connection<'a>( + &self, + connection: MappedMutexGuard<'a, BondConnection>, + ) -> zbus::fdo::Result<()> { + let actions = self.actions.lock().await; + let connection = NetworkConnection::Bond(connection.clone()); + actions.send(Action::UpdateConnection(connection)).unwrap(); + Ok(()) + } + /// Updates the controller connection data in the NetworkSystem. /// /// * `connection`: Updated connection. @@ -347,16 +360,11 @@ impl Bond { } #[dbus_interface(property)] - pub async fn set_mode(&mut self, mode: String) -> zbus::fdo::Result<()> { - let connection = self.get_bond().await; - let result = self - .update_controller_connection( - connection, - HashMap::from([("mode".to_string(), ControllerConfig::Mode(mode.clone()))]), - ) - .await; - self.connection = Arc::new(Mutex::new(result?)); - Ok(()) + pub async fn set_mode(&mut self, mode: &str) -> zbus::fdo::Result<()> { + let mut connection = self.get_bond().await; + connection.set_mode(mode)?; + + self.update_connection(connection).await } /// List of bond ports. @@ -368,19 +376,11 @@ impl Bond { } #[dbus_interface(property)] - pub async fn set_options(&mut self, opts: String) -> zbus::fdo::Result<()> { - let connection = self.get_bond().await; - let result = self - .update_controller_connection( - connection, - HashMap::from([( - "options".to_string(), - ControllerConfig::Options(opts.clone()), - )]), - ) - .await; - self.connection = Arc::new(Mutex::new(result.unwrap())); - Ok(()) + pub async fn set_options(&mut self, opts: &str) -> zbus::fdo::Result<()> { + let mut connection = self.get_bond().await; + connection.set_options(opts)?; + + self.update_connection(connection).await } /// List of bond ports. diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 9edf844eba..bbb4ad166f 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -94,13 +94,6 @@ impl NetworkState { let controller = conn.clone(); if let Connection::Bond(ref mut bond) = conn { - if let Some(ControllerConfig::Mode(mode)) = settings.get("mode") { - bond.set_mode(mode)?; - } - if let Some(ControllerConfig::Options(opts)) = settings.get("options") { - bond.set_options(opts.as_str())?; - } - if let Some(ControllerConfig::Ports(ports)) = settings.get("ports") { let new_ports: Vec<_> = ports .iter() @@ -661,8 +654,6 @@ pub struct BondConfig { /// Controller config payload for updating a controller's connection (Bond, Bridge) #[derive(Debug, PartialEq, Clone)] pub enum ControllerConfig { - Mode(String), - Options(String), Ports(Vec), } From 1f7795112e13cbb8bff1306119c0646479c00c1f Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Tue, 5 Dec 2023 10:15:37 +0000 Subject: [PATCH 26/58] Ports will be in the State connections referencing its controller by name --- rust/agama-dbus-server/src/network/action.rs | 12 +-- .../src/network/dbus/interfaces.rs | 50 ++---------- rust/agama-dbus-server/src/network/model.rs | 81 ++++++++----------- .../src/network/nm/adapter.rs | 6 +- .../src/network/nm/client.rs | 63 +-------------- rust/agama-dbus-server/src/network/system.rs | 13 --- rust/agama-dbus-server/tests/network.rs | 4 +- rust/agama-lib/share/profile.schema.json | 5 +- 8 files changed, 54 insertions(+), 180 deletions(-) diff --git a/rust/agama-dbus-server/src/network/action.rs b/rust/agama-dbus-server/src/network/action.rs index f56459e753..9f935f70f1 100644 --- a/rust/agama-dbus-server/src/network/action.rs +++ b/rust/agama-dbus-server/src/network/action.rs @@ -1,9 +1,5 @@ -use crate::network::model::{Connection, ControllerConfig}; +use crate::network::model::Connection; use agama_lib::network::types::DeviceType; -use std::collections::HashMap; -use tokio::sync::oneshot; - -use super::error::NetworkStateError; /// Networking actions, like adding, updating or removing connections. /// @@ -15,12 +11,6 @@ pub enum Action { AddConnection(String, DeviceType), /// Update a connection (replacing the old one). UpdateConnection(Connection), - /// Update a controller connection (replacing the old one). - UpdateControllerConnection( - Connection, - HashMap, - oneshot::Sender>, - ), /// Remove the connection with the given Uuid. RemoveConnection(String), /// Apply the current configuration. diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index 5eb77a4d81..17a2c00733 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -2,21 +2,20 @@ //! //! This module contains the set of D-Bus interfaces that are exposed by [D-Bus network //! service](crate::NetworkService). -use std::collections::HashMap; use super::ObjectsRegistry; use crate::network::{ action::Action, error::NetworkStateError, model::{ - BondConnection, Connection as NetworkConnection, ControllerConfig, Device as NetworkDevice, + BondConnection, Connection as NetworkConnection, Device as NetworkDevice, WirelessConnection, }, }; use agama_lib::network::types::SSID; use std::sync::Arc; -use tokio::sync::{mpsc::UnboundedSender, oneshot}; +use tokio::sync::mpsc::UnboundedSender; use tokio::sync::{MappedMutexGuard, Mutex, MutexGuard}; use zbus::{ dbus_interface, @@ -329,24 +328,6 @@ impl Bond { actions.send(Action::UpdateConnection(connection)).unwrap(); Ok(()) } - - /// Updates the controller connection data in the NetworkSystem. - /// - /// * `connection`: Updated connection. - async fn update_controller_connection<'a>( - &self, - connection: MappedMutexGuard<'a, BondConnection>, - settings: HashMap, - ) -> Result { - let actions = self.actions.lock().await; - let connection = NetworkConnection::Bond(connection.clone()); - let (tx, rx) = oneshot::channel(); - actions - .send(Action::UpdateControllerConnection(connection, settings, tx)) - .unwrap(); - - rx.await.unwrap() - } } #[dbus_interface(name = "org.opensuse.Agama1.Network.Connection.Bond")] @@ -355,15 +336,13 @@ impl Bond { #[dbus_interface(property)] pub async fn mode(&self) -> String { let connection = self.get_bond().await; - connection.bond.mode.to_string() } #[dbus_interface(property)] pub async fn set_mode(&mut self, mode: &str) -> zbus::fdo::Result<()> { let mut connection = self.get_bond().await; - connection.set_mode(mode)?; - + connection.set_mode(mode.try_into()?); self.update_connection(connection).await } @@ -371,15 +350,13 @@ impl Bond { #[dbus_interface(property)] pub async fn options(&self) -> String { let connection = self.get_bond().await; - connection.bond.options.to_string() } #[dbus_interface(property)] pub async fn set_options(&mut self, opts: &str) -> zbus::fdo::Result<()> { let mut connection = self.get_bond().await; - connection.set_options(opts)?; - + connection.set_options(opts.try_into()?); self.update_connection(connection).await } @@ -387,25 +364,14 @@ impl Bond { #[dbus_interface(property)] pub async fn ports(&self) -> Vec { let connection = self.get_bond().await; - connection - .bond - .ports - .iter() - .map(|port| port.base().id.to_string()) - .collect() + connection.bond.ports.clone() } #[dbus_interface(property)] pub async fn set_ports(&mut self, ports: Vec) -> zbus::fdo::Result<()> { - let connection = self.get_bond().await; - let result = self - .update_controller_connection( - connection, - HashMap::from([("ports".to_string(), ControllerConfig::Ports(ports.clone()))]), - ) - .await; - self.connection = Arc::new(Mutex::new(result.unwrap())); - Ok(()) + let mut connection = self.get_bond().await; + connection.set_ports(ports); + self.update_connection(connection).await } } diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index bbb4ad166f..97c15bf68c 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -41,9 +41,9 @@ impl NetworkState { self.devices.iter().find(|d| d.name == name) } - /// Get connection by UUID + /// Get connection by ID /// - /// * `uuid`: connection UUID + /// * `id`: connection ID pub fn get_connection(&self, id: &str) -> Option<&Connection> { self.connections.iter().find(|c| c.id() == id) } @@ -78,6 +78,16 @@ impl NetworkState { }; *old_conn = conn; + + match old_conn { + Connection::Bond(ref bond) => { + let id = old_conn.id().to_string(); + let ports = bond.bond.ports.clone(); + self.update_controller_ports(id, ports)?; + } + _ => {} + }; + Ok(()) } @@ -86,33 +96,17 @@ impl NetworkState { /// It uses the `id` to decide which connection to update. /// /// Additionally, it registers the connection to be removed when the changes are applied. - pub fn update_controller_connection( + pub fn update_controller_ports( &mut self, - mut conn: Connection, - settings: HashMap, + controller_id: String, + ports: Vec, ) -> Result<(), NetworkStateError> { - let controller = conn.clone(); - - if let Connection::Bond(ref mut bond) = conn { - if let Some(ControllerConfig::Ports(ports)) = settings.get("ports") { - let new_ports: Vec<_> = ports - .iter() - .map(|port| { - bond.find_port(port).cloned().unwrap_or_else(|| { - ConnectionBuilder::new(port) - .with_type(DeviceType::Ethernet) - .with_interface(port) - .with_controller(controller.id()) - .build() - }) - }) - .collect(); - - bond.set_ports(new_ports); + for conn in self.connections.iter_mut() { + if ports.contains(&conn.id().to_string()) { + conn.set_controller(&controller_id); } } - - self.update_connection(conn) + Ok(()) } /// Removes a connection from the state. @@ -361,15 +355,19 @@ impl Connection { } /// Ports controller name, e.g.: bond0, br0 - pub fn controller(&self) -> &str { - self.base().controller.as_str() + pub fn is_controlled(&self) -> bool { + self.base().controller.is_some() + } + /// Ports controller name, e.g.: bond0, br0 + pub fn controller(&self) -> Option<&String> { + self.base().controller.as_ref() } /// Sets the ports controller. /// /// `controller`: Name of the controller (Bridge, Bond, Team), e.g.: bond0. pub fn set_controller(&mut self, controller: &str) { - self.base_mut().controller = controller.to_string() + self.base_mut().controller = Some(controller.to_string()); } pub fn uuid(&self) -> Uuid { @@ -413,7 +411,7 @@ pub struct BaseConnection { pub ip_config: IpConfig, pub status: Status, pub interface: String, - pub controller: String, + pub controller: Option, pub match_config: MatchConfig, } @@ -585,29 +583,16 @@ pub struct BondConnection { } impl BondConnection { - pub fn find_port(&self, name: &str) -> Option<&Connection> { - self.bond.ports.iter().find(|c| c.interface() == name) - } - - pub fn set_ports(&mut self, ports: Vec) { + pub fn set_ports(&mut self, ports: Vec) { self.bond.ports = ports; } - pub fn set_mode(&mut self, mode: &str) -> Result<(), NetworkStateError> { - self.bond.mode = BondMode::try_from(mode) - .map_err(|_e| NetworkStateError::InvalidBondMode(mode.to_string()))?; - Ok(()) + pub fn set_mode(&mut self, mode: BondMode) { + self.bond.mode = mode; } - pub fn set_options(&mut self, options: T) -> Result<(), NetworkStateError> - where - T: TryInto, - >::Error: std::fmt::Debug, - { - self.bond.options = options - .try_into() - .map_err(|_e| NetworkStateError::InvalidBondOptions)?; - Ok(()) + pub fn set_options(&mut self, options: BondOptions) { + self.bond.options = options; } } @@ -647,7 +632,7 @@ impl fmt::Display for BondOptions { #[derive(Debug, Default, PartialEq, Clone)] pub struct BondConfig { pub mode: BondMode, - pub ports: Vec, + pub ports: Vec, pub options: BondOptions, } diff --git a/rust/agama-dbus-server/src/network/nm/adapter.rs b/rust/agama-dbus-server/src/network/nm/adapter.rs index a5d3c4d811..0fc3f62951 100644 --- a/rust/agama-dbus-server/src/network/nm/adapter.rs +++ b/rust/agama-dbus-server/src/network/nm/adapter.rs @@ -52,8 +52,10 @@ impl<'a> Adapter for NetworkManagerAdapter<'a> { if let Err(e) = self.client.remove_connection(conn.uuid()).await { log::error!("Could not remove the connection {}: {}", conn.id(), e); } - } else if let Err(e) = self.client.add_or_update_connection(conn).await { - log::error!("Could not add/update the connection {}: {}", conn.id(), e); + } else if !conn.is_controlled() { + if let Err(e) = self.client.add_or_update_connection(conn).await { + log::error!("Could not add/update the connection {}: {}", conn.id(), e); + } } } }) diff --git a/rust/agama-dbus-server/src/network/nm/client.rs b/rust/agama-dbus-server/src/network/nm/client.rs index 14a74c2c92..d6b7ac22c1 100644 --- a/rust/agama-dbus-server/src/network/nm/client.rs +++ b/rust/agama-dbus-server/src/network/nm/client.rs @@ -1,9 +1,6 @@ //! NetworkManager client. -use std::collections::HashMap; - use super::dbus::{ - cleanup_dbus_connection, connection_from_dbus, connection_to_dbus, controller_from_dbus, - merge_dbus_connections, + cleanup_dbus_connection, connection_from_dbus, connection_to_dbus, merge_dbus_connections, }; use super::model::NmDeviceType; use super::proxies::{ConnectionProxy, DeviceProxy, NetworkManagerProxy, SettingsProxy}; @@ -76,7 +73,6 @@ impl<'a> NetworkManagerClient<'a> { let proxy = SettingsProxy::new(&self.connection).await?; let paths = proxy.list_connections().await?; let mut connections: Vec = Vec::with_capacity(paths.len()); - let mut controllers: HashMap> = HashMap::new(); for path in paths { let proxy = ConnectionProxy::builder(&self.connection) .path(path.as_str())? @@ -84,16 +80,8 @@ impl<'a> NetworkManagerClient<'a> { .await?; let settings = proxy.get_settings().await?; // TODO: log an error if a connection is not found - if let Some(connection) = connection_from_dbus(settings.clone()) { - if let Some(controller) = controller_from_dbus(&settings) { - controllers - .entry(controller) - .or_insert_with(Vec::new) - .push(connection) - } else { - connections.push(connection); - } + connections.push(connection); } } @@ -117,53 +105,6 @@ impl<'a> NetworkManagerClient<'a> { proxy.add_connection(new_conn).await? }; - if let Connection::Bond(bond) = conn { - for port in bond.bond.ports.clone() { - self.add_or_update_port_connection( - &port, - bond.base.interface.to_string(), - "bond".to_string(), - ) - .await?; - } - } - - self.activate_connection(path).await?; - Ok(()) - } - - pub async fn add_or_update_port_connection( - &self, - conn: &Connection, - controller: String, - port_type: String, - ) -> Result<(), ServiceError> { - let mut dbus_conn = connection_to_dbus(conn); - - if let Some(new_conn) = dbus_conn.get_mut("connection") { - new_conn.insert("slave-type", port_type.to_string().into()); - new_conn.insert("master", controller.to_string().into()); - } - - let path = if let Ok(proxy) = self.get_connection_proxy(conn.uuid()).await { - let original = proxy.get_settings().await?; - let merged = merge_dbus_connections(&original, &dbus_conn); - proxy.update(merged).await?; - OwnedObjectPath::from(proxy.path().to_owned()) - } else { - let proxy = SettingsProxy::new(&self.connection).await?; - proxy.add_connection(dbus_conn).await? - }; - - if let Connection::Bond(bond) = conn { - let _ = bond.bond.ports.iter().map(|port| { - self.add_or_update_port_connection( - port, - bond.base.interface.to_string(), - "bond".to_string(), - ) - }); - } self.activate_connection(path).await?; Ok(()) } diff --git a/rust/agama-dbus-server/src/network/system.rs b/rust/agama-dbus-server/src/network/system.rs index c70d364b18..fcfb3e170e 100644 --- a/rust/agama-dbus-server/src/network/system.rs +++ b/rust/agama-dbus-server/src/network/system.rs @@ -73,19 +73,6 @@ impl NetworkSystem { Action::UpdateConnection(conn) => { self.state.update_connection(conn)?; } - Action::UpdateControllerConnection(conn, settings, tx) => { - let id = conn.id().to_owned(); - let result = self - .state - .update_controller_connection(conn, settings) - .and_then(|_| { - self.state.get_connection(&id).ok_or( - super::error::NetworkStateError::UnknownConnection(id.to_string()), - ) - }); - - tx.send(result.cloned()).unwrap(); - } Action::RemoveConnection(id) => { self.tree.remove_connection(&id).await?; self.state.remove_connection(&id)?; diff --git a/rust/agama-dbus-server/tests/network.rs b/rust/agama-dbus-server/tests/network.rs index 3557d66f7b..bf40144092 100644 --- a/rust/agama-dbus-server/tests/network.rs +++ b/rust/agama-dbus-server/tests/network.rs @@ -3,11 +3,11 @@ mod common; use self::common::{async_retry, DBusServer}; use agama_dbus_server::network::{ self, - model::{self, BondConfig, Ipv4Method, Ipv6Method}, + model::{self, Ipv4Method, Ipv6Method}, Adapter, NetworkService, NetworkState, }; use agama_lib::network::{ - settings::{self, BondSettings}, + settings::{self}, types::DeviceType, NetworkClient, }; diff --git a/rust/agama-lib/share/profile.schema.json b/rust/agama-lib/share/profile.schema.json index fa24143bdf..224ba8faed 100644 --- a/rust/agama-lib/share/profile.schema.json +++ b/rust/agama-lib/share/profile.schema.json @@ -110,13 +110,16 @@ "description": "Bonding configuration", "additionalProperties": false, "properties": { + "mode": { + "type": "string" + }, "options": { "type": "string" }, "ports": { "type": "array", "items": { - "description": "A list of the ports to be bonded", + "description": "A list of the interfaces or connections to be bonded", "type": "string", "additionalProperties": false } From 25263405e549a8f3e0c1684e7938de1e5a19973f Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Tue, 5 Dec 2023 13:03:27 +0000 Subject: [PATCH 27/58] Adapted NM adapter and client for writing the controlled connections --- rust/agama-dbus-server/src/network/model.rs | 1 + .../src/network/nm/adapter.rs | 66 ++++++++++++++++--- .../src/network/nm/client.rs | 8 ++- rust/agama-dbus-server/src/network/nm/dbus.rs | 17 +++-- 4 files changed, 76 insertions(+), 16 deletions(-) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 97c15bf68c..878b325507 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -358,6 +358,7 @@ impl Connection { pub fn is_controlled(&self) -> bool { self.base().controller.is_some() } + /// Ports controller name, e.g.: bond0, br0 pub fn controller(&self) -> Option<&String> { self.base().controller.as_ref() diff --git a/rust/agama-dbus-server/src/network/nm/adapter.rs b/rust/agama-dbus-server/src/network/nm/adapter.rs index 0fc3f62951..3aa2c0393a 100644 --- a/rust/agama-dbus-server/src/network/nm/adapter.rs +++ b/rust/agama-dbus-server/src/network/nm/adapter.rs @@ -1,4 +1,5 @@ use crate::network::{ + error::NetworkStateError, model::{Connection, NetworkState}, nm::NetworkManagerClient, Adapter, @@ -25,6 +26,61 @@ impl<'a> NetworkManagerAdapter<'a> { fn is_writable(conn: &Connection) -> bool { !conn.is_loopback() } + + /// Writes the connections to NetworkManager. + /// + /// Internally, it creates and order list of connections before processing them. The reason is + /// that using async recursive functions is giving us some troubles, so we decided to go with a + /// simpler approach. + /// + /// * `network`: network model. + async fn write_connections(&self, network: &NetworkState) { + let conns = self.ordered_connections(&network); + println!("Connections to write: {:?}", &conns); + + for conn in &conns { + let result = if conn.is_removed() { + self.client.remove_connection(conn.uuid()).await + } else { + let ctrl = conn.controller().and_then(|id| network.get_connection(&id)); + self.client.add_or_update_connection(&conn, ctrl).await + }; + + if let Err(e) = result { + log::error!("Could not process the connection {}: {}", conn.id(), e); + } + } + } + + /// Returns the connections in the order they should be processed. + /// * `network`: network model. + /// + fn ordered_connections<'b>(&self, network: &'b NetworkState) -> Vec<&'b Connection> { + let mut conns: Vec<&Connection> = vec![]; + for conn in &network.connections { + if !conn.is_controlled() { + self.add_ordered_connections(conn, network, &mut conns); + } + } + conns + } + + fn add_ordered_connections<'b>( + &self, + conn: &'b Connection, + network: &'b NetworkState, + conns: &mut Vec<&'b Connection>, + ) { + conns.push(conn); + + if let Connection::Bond(bond) = &conn { + for port in &bond.bond.ports { + if let Some(port_connection) = network.get_connection(port.as_str()) { + self.add_ordered_connections(port_connection, network, conns); + } + } + } + } } impl<'a> Adapter for NetworkManagerAdapter<'a> { @@ -48,15 +104,7 @@ impl<'a> Adapter for NetworkManagerAdapter<'a> { if !Self::is_writable(conn) { continue; } - if conn.is_removed() { - if let Err(e) = self.client.remove_connection(conn.uuid()).await { - log::error!("Could not remove the connection {}: {}", conn.id(), e); - } - } else if !conn.is_controlled() { - if let Err(e) = self.client.add_or_update_connection(conn).await { - log::error!("Could not add/update the connection {}: {}", conn.id(), e); - } - } + self.write_connections(network).await; } }) }); diff --git a/rust/agama-dbus-server/src/network/nm/client.rs b/rust/agama-dbus-server/src/network/nm/client.rs index d6b7ac22c1..23698f3e15 100644 --- a/rust/agama-dbus-server/src/network/nm/client.rs +++ b/rust/agama-dbus-server/src/network/nm/client.rs @@ -91,8 +91,12 @@ impl<'a> NetworkManagerClient<'a> { /// Adds or updates a connection if it already exists. /// /// * `conn`: connection to add or update. - pub async fn add_or_update_connection(&self, conn: &Connection) -> Result<(), ServiceError> { - let mut new_conn = connection_to_dbus(conn); + pub async fn add_or_update_connection( + &self, + conn: &Connection, + controller: Option<&Connection>, + ) -> Result<(), ServiceError> { + let mut new_conn = connection_to_dbus(conn, controller); let path = if let Ok(proxy) = self.get_connection_proxy(conn.uuid()).await { let original = proxy.get_settings().await?; diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index c6cabdf562..d22e9f4cff 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -22,13 +22,20 @@ const LOOPBACK_KEY: &str = "loopback"; /// Converts a connection struct into a HashMap that can be sent over D-Bus. /// /// * `conn`: Connection to convert. -pub fn connection_to_dbus(conn: &Connection) -> NestedHash { +pub fn connection_to_dbus<'a>( + conn: &'a Connection, + controller: Option<&'a Connection>, +) -> NestedHash<'a> { let mut result = NestedHash::new(); let mut connection_dbus = HashMap::from([ ("id", conn.id().into()), ("type", ETHERNET_KEY.into()), ("interface-name", conn.interface().into()), ]); + if let Some(controller) = controller { + connection_dbus.insert("slave-type", "bond".into()); // TODO: only 'bond' is supported + connection_dbus.insert("master", controller.id().into()); + } result.insert("ipv4", ip_config_to_ipv4_dbus(conn.ip_config())); result.insert("ipv6", ip_config_to_ipv6_dbus(conn.ip_config())); @@ -761,7 +768,7 @@ mod test { ..Default::default() }; let wireless = Connection::Wireless(wireless); - let wireless_dbus = connection_to_dbus(&wireless); + let wireless_dbus = connection_to_dbus(&wireless, None); let wireless = wireless_dbus.get(WIRELESS_KEY).unwrap(); let mode: &str = wireless.get("mode").unwrap().downcast_ref().unwrap(); @@ -783,7 +790,7 @@ mod test { #[test] fn test_dbus_from_ethernet_connection() { let ethernet = build_ethernet_connection(); - let ethernet_dbus = connection_to_dbus(ðernet); + let ethernet_dbus = connection_to_dbus(ðernet, None); check_dbus_base_connection(ðernet_dbus); } @@ -841,7 +848,7 @@ mod test { ..Default::default() }; let updated = Connection::Ethernet(ethernet); - let updated = connection_to_dbus(&updated); + let updated = connection_to_dbus(&updated, None); let merged = merge_dbus_connections(&original, &updated); let connection = merged.get("connection").unwrap(); @@ -895,7 +902,7 @@ mod test { let mut updated = Connection::Ethernet(EthernetConnection::default()); updated.set_interface(""); - let updated = connection_to_dbus(&updated); + let updated = connection_to_dbus(&updated, None); let merged = merge_dbus_connections(&original, &updated); let connection = merged.get("connection").unwrap(); From 7d5124bd94c781fda73408ab248947463c70749d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 5 Dec 2023 16:45:59 +0000 Subject: [PATCH 28/58] Extend the BuilderConnection to handle ports * By the way, the builder is moved to its own file. --- rust/agama-dbus-server/src/network/model.rs | 47 +------------- .../src/network/model/builder.rs | 63 +++++++++++++++++++ 2 files changed, 64 insertions(+), 46 deletions(-) create mode 100644 rust/agama-dbus-server/src/network/model/builder.rs diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 878b325507..805f4c8d65 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -15,6 +15,7 @@ use std::{ use thiserror::Error; use uuid::Uuid; use zbus::zvariant::Value; +mod builder; #[derive(Default, Clone)] pub struct NetworkState { @@ -243,52 +244,6 @@ pub struct Device { pub type_: DeviceType, } -#[derive(Debug, Default)] -pub struct ConnectionBuilder { - id: String, - interface: Option, - controller: Option, - type_: Option, -} - -impl ConnectionBuilder { - pub fn new(id: &str) -> Self { - Self { - id: id.to_string(), - ..Default::default() - } - } - - pub fn with_interface(mut self, interface: &str) -> Self { - self.interface = Some(interface.to_string()); - self - } - - pub fn with_controller(mut self, controller: &str) -> Self { - self.controller = Some(controller.to_string()); - self - } - - pub fn with_type(mut self, type_: DeviceType) -> Self { - self.type_ = Some(type_); - self - } - - pub fn build(self) -> Connection { - let mut conn = Connection::new(self.id, self.type_.unwrap_or(DeviceType::Ethernet)); - - if let Some(interface) = self.interface { - conn.set_interface(&interface); - } - - if let Some(controller) = self.controller { - conn.set_controller(&controller); - } - - conn - } -} - /// Represents an available network connection #[derive(Debug, PartialEq, Clone)] pub enum Connection { diff --git a/rust/agama-dbus-server/src/network/model/builder.rs b/rust/agama-dbus-server/src/network/model/builder.rs new file mode 100644 index 0000000000..6994b2d808 --- /dev/null +++ b/rust/agama-dbus-server/src/network/model/builder.rs @@ -0,0 +1,63 @@ +use super::{Connection, DeviceType}; + +// TODO: improve the implementation to allow calling methods only where it makes sense +// depending on the type of the connection. + +#[derive(Debug, Default)] +pub struct ConnectionBuilder { + id: String, + interface: Option, + controller: Option, + type_: Option, + ports: Vec, +} + +impl ConnectionBuilder { + pub fn new(id: &str) -> Self { + Self { + id: id.to_string(), + ..Default::default() + } + } + + pub fn with_interface(mut self, interface: &str) -> Self { + self.interface = Some(interface.to_string()); + self + } + + pub fn with_controller(mut self, controller: &str) -> Self { + self.controller = Some(controller.to_string()); + self + } + + pub fn with_type(mut self, type_: DeviceType) -> Self { + self.type_ = Some(type_); + self + } + + pub fn with_ports(mut self, ports: Vec) -> Self { + self.ports = ports; + self + } + + pub fn build(self) -> Connection { + let mut conn = Connection::new(self.id, self.type_.unwrap_or(DeviceType::Ethernet)); + + if let Some(interface) = self.interface { + conn.set_interface(&interface); + } + + if let Some(controller) = self.controller { + conn.set_controller(&controller); + } + + match &mut conn { + Connection::Bond(bond) => { + bond.set_ports(self.ports); + } + _ => {} + } + + conn + } +} From f4a63418020aabd19c5d1fadc2d317fc59f5c483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 5 Dec 2023 17:08:08 +0000 Subject: [PATCH 29/58] Improve handling of bond ports * It is still a WIP. --- rust/agama-dbus-server/src/network/model.rs | 54 ++++++++++++++++++--- 1 file changed, 48 insertions(+), 6 deletions(-) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 805f4c8d65..a64a35e6ce 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -16,8 +16,9 @@ use thiserror::Error; use uuid::Uuid; use zbus::zvariant::Value; mod builder; +pub use builder::ConnectionBuilder; -#[derive(Default, Clone)] +#[derive(Default, Clone, Debug)] pub struct NetworkState { pub devices: Vec, pub connections: Vec, @@ -64,7 +65,21 @@ impl NetworkState { return Err(NetworkStateError::ConnectionExists(conn.uuid())); } + // FIXME: unify this code. + match &conn { + Connection::Bond(bond) => { + let id = conn.id().to_string(); + // FIXME: We should implement a trait Controller + // so those that acts as a controller offer the same API + // and we do not need to access "bond.bond". + let ports = bond.bond.ports.clone(); + self.update_controller_ports(id, ports)?; + } + _ => {} + }; + self.connections.push(conn); + Ok(()) } @@ -80,8 +95,8 @@ impl NetworkState { *old_conn = conn; - match old_conn { - Connection::Bond(ref bond) => { + match &old_conn { + Connection::Bond(bond) => { let id = old_conn.id().to_string(); let ports = bond.bond.ports.clone(); self.update_controller_ports(id, ports)?; @@ -92,11 +107,12 @@ impl NetworkState { Ok(()) } - /// Updates a controller connection with a new one. + /// Updates controller ports. /// - /// It uses the `id` to decide which connection to update. + /// It sets or unsets the controller of each connection according to the list of given ports. /// - /// Additionally, it registers the connection to be removed when the changes are applied. + /// * `controller_id`: controller to use. + /// * `ports`: devices that should belong to the given controller. pub fn update_controller_ports( &mut self, controller_id: String, @@ -105,6 +121,8 @@ impl NetworkState { for conn in self.connections.iter_mut() { if ports.contains(&conn.id().to_string()) { conn.set_controller(&controller_id); + } else if conn.controller() == Some(&controller_id) { + conn.unset_controller(); } } Ok(()) @@ -127,6 +145,7 @@ impl NetworkState { mod tests { use uuid::Uuid; + use super::builder::ConnectionBuilder; use super::*; use crate::network::error::NetworkStateError; @@ -235,6 +254,25 @@ mod tests { let conn = Connection::Loopback(LoopbackConnection { base }); assert!(conn.is_loopback()); } + + #[test] + fn test_add_bonding() { + let mut state = NetworkState::default(); + let eth0 = ConnectionBuilder::new("eth0") + .with_type(DeviceType::Ethernet) + .build(); + state.add_connection(eth0).unwrap(); + + let bond0 = ConnectionBuilder::new("bond0") + .with_type(DeviceType::Bond) + .with_ports(vec!["eth0".to_string()]) + .build(); + + state.add_connection(bond0).unwrap(); + + let eth0 = state.get_connection("eth0").unwrap(); + assert_eq!(eth0.controller().unwrap(), "bond0"); + } } /// Network device @@ -326,6 +364,10 @@ impl Connection { self.base_mut().controller = Some(controller.to_string()); } + pub fn unset_controller(&mut self) { + self.base_mut().controller = None; + } + pub fn uuid(&self) -> Uuid { self.base().uuid } From 894a647ff124ba280c97650d9a74139df4857590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 5 Dec 2023 17:08:23 +0000 Subject: [PATCH 30/58] Drop some unused code --- rust/agama-dbus-server/src/network/nm/adapter.rs | 1 - rust/agama-dbus-server/src/network/nm/dbus.rs | 13 ------------- 2 files changed, 14 deletions(-) diff --git a/rust/agama-dbus-server/src/network/nm/adapter.rs b/rust/agama-dbus-server/src/network/nm/adapter.rs index 3aa2c0393a..b7273e60ae 100644 --- a/rust/agama-dbus-server/src/network/nm/adapter.rs +++ b/rust/agama-dbus-server/src/network/nm/adapter.rs @@ -1,5 +1,4 @@ use crate::network::{ - error::NetworkStateError, model::{Connection, NetworkState}, nm::NetworkManagerClient, Adapter, diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index d22e9f4cff..f78a025e3e 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -294,19 +294,6 @@ fn match_config_to_dbus(match_config: &MatchConfig) -> HashMap<&str, zvariant::V ("interface-name", interfaces), ]) } -pub fn controller_from_dbus(conn: &OwnedNestedHash) -> Option { - let Some(connection) = conn.get("connection") else { - return None; - }; - - let Some(controller) = connection.get("master") else { - return None; - }; - - let controller: &str = controller.downcast_ref()?; - - Some(controller.to_string()) -} fn base_connection_from_dbus(conn: &OwnedNestedHash) -> Option { let Some(connection) = conn.get("connection") else { From cce1dcac7bd972fbe9cfa8bf267e976e0012f915 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 6 Dec 2023 11:13:51 +0000 Subject: [PATCH 31/58] Update bonding ports --- rust/agama-dbus-server/src/network/model.rs | 65 +++++++-------------- 1 file changed, 20 insertions(+), 45 deletions(-) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index a64a35e6ce..aef19969c7 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -65,19 +65,7 @@ impl NetworkState { return Err(NetworkStateError::ConnectionExists(conn.uuid())); } - // FIXME: unify this code. - match &conn { - Connection::Bond(bond) => { - let id = conn.id().to_string(); - // FIXME: We should implement a trait Controller - // so those that acts as a controller offer the same API - // and we do not need to access "bond.bond". - let ports = bond.bond.ports.clone(); - self.update_controller_ports(id, ports)?; - } - _ => {} - }; - + self.update_controller_ports(&conn); self.connections.push(conn); Ok(()) @@ -88,43 +76,14 @@ impl NetworkState { /// It uses the `id` to decide which connection to update. /// /// Additionally, it registers the connection to be removed when the changes are applied. - pub fn update_connection(&mut self, conn: Connection) -> Result<(), NetworkStateError> { + pub fn update_connection(&mut self, mut conn: Connection) -> Result<(), NetworkStateError> { let Some(old_conn) = self.get_connection_mut(conn.id()) else { return Err(NetworkStateError::UnknownConnection(conn.id().to_string())); }; - *old_conn = conn; - - match &old_conn { - Connection::Bond(bond) => { - let id = old_conn.id().to_string(); - let ports = bond.bond.ports.clone(); - self.update_controller_ports(id, ports)?; - } - _ => {} - }; - - Ok(()) - } + std::mem::swap(old_conn, &mut conn); + self.update_controller_ports(&conn); - /// Updates controller ports. - /// - /// It sets or unsets the controller of each connection according to the list of given ports. - /// - /// * `controller_id`: controller to use. - /// * `ports`: devices that should belong to the given controller. - pub fn update_controller_ports( - &mut self, - controller_id: String, - ports: Vec, - ) -> Result<(), NetworkStateError> { - for conn in self.connections.iter_mut() { - if ports.contains(&conn.id().to_string()) { - conn.set_controller(&controller_id); - } else if conn.controller() == Some(&controller_id) { - conn.unset_controller(); - } - } Ok(()) } @@ -139,6 +98,22 @@ impl NetworkState { conn.remove(); Ok(()) } + + /// It does not check whether the ports exist. + /// + /// TODO: check whether a port is missing. + fn update_controller_ports(&mut self, controller: &Connection) { + if let Connection::Bond(bond) = controller { + let controller_id = controller.id().to_string(); + for conn in self.connections.iter_mut() { + if bond.bond.ports.contains(&conn.id().to_string()) { + conn.set_controller(&controller_id); + } else if conn.controller() == Some(&controller_id) { + conn.unset_controller(); + } + } + } + } } #[cfg(test)] From aa5ab03f1eb73839d914d37b40100df5a40b4bc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 11 Dec 2023 11:45:10 +0000 Subject: [PATCH 32/58] Make rustfmt happy --- rust/agama-dbus-server/src/network/dbus/interfaces.rs | 3 ++- rust/agama-dbus-server/src/network/nm/adapter.rs | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index 792cf6a4d9..b8d66deba1 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -8,7 +8,8 @@ use crate::network::{ action::Action, error::NetworkStateError, model::{ - BondConnection, Connection as NetworkConnection, Device as NetworkDevice, MacAddress, WirelessConnection, + BondConnection, Connection as NetworkConnection, Device as NetworkDevice, MacAddress, + WirelessConnection, }, }; diff --git a/rust/agama-dbus-server/src/network/nm/adapter.rs b/rust/agama-dbus-server/src/network/nm/adapter.rs index b7273e60ae..82ee5dd983 100644 --- a/rust/agama-dbus-server/src/network/nm/adapter.rs +++ b/rust/agama-dbus-server/src/network/nm/adapter.rs @@ -1,5 +1,5 @@ use crate::network::{ - model::{Connection, NetworkState}, + model::{BondConnection, Connection, NetworkState}, nm::NetworkManagerClient, Adapter, }; @@ -52,8 +52,8 @@ impl<'a> NetworkManagerAdapter<'a> { } /// Returns the connections in the order they should be processed. - /// * `network`: network model. /// + /// * `network`: network model. fn ordered_connections<'b>(&self, network: &'b NetworkState) -> Vec<&'b Connection> { let mut conns: Vec<&Connection> = vec![]; for conn in &network.connections { @@ -72,8 +72,8 @@ impl<'a> NetworkManagerAdapter<'a> { ) { conns.push(conn); - if let Connection::Bond(bond) = &conn { - for port in &bond.bond.ports { + if let Connection::Bond(BondConnection { bond, .. }) = &conn { + for port in &bond.ports { if let Some(port_connection) = network.get_connection(port.as_str()) { self.add_ordered_connections(port_connection, network, conns); } From 69eec5744ae82da98bb3ca17bc04729c23ac1593 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 11 Dec 2023 11:45:39 +0000 Subject: [PATCH 33/58] Better matching on update_controller_ports --- rust/agama-dbus-server/src/network/model.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 2946908a59..c6d2eb780d 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -103,10 +103,10 @@ impl NetworkState { /// /// TODO: check whether a port is missing. fn update_controller_ports(&mut self, controller: &Connection) { - if let Connection::Bond(bond) = controller { + if let Connection::Bond(BondConnection { bond, .. }) = controller { let controller_id = controller.id().to_string(); for conn in self.connections.iter_mut() { - if bond.bond.ports.contains(&conn.id().to_string()) { + if bond.ports.contains(&conn.id().to_string()) { conn.set_controller(&controller_id); } else if conn.controller() == Some(&controller_id) { conn.unset_controller(); From de0edd7c50b5b38e4f81947ca9933633b04fbe6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 12 Dec 2023 09:16:56 +0000 Subject: [PATCH 34/58] Fix the assignment of bond ports --- rust/agama-dbus-server/src/network/model.rs | 43 +++++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index c6d2eb780d..f4bc419a0f 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -76,13 +76,15 @@ impl NetworkState { /// It uses the `id` to decide which connection to update. /// /// Additionally, it registers the connection to be removed when the changes are applied. - pub fn update_connection(&mut self, mut conn: Connection) -> Result<(), NetworkStateError> { + pub fn update_connection(&mut self, conn: Connection) -> Result<(), NetworkStateError> { let Some(old_conn) = self.get_connection_mut(conn.id()) else { return Err(NetworkStateError::UnknownConnection(conn.id().to_string())); }; - std::mem::swap(old_conn, &mut conn); - self.update_controller_ports(&conn); + // workaround the borrow checker + let clone = conn.clone(); + *old_conn = conn; + self.update_controller_ports(&clone); Ok(()) } @@ -102,9 +104,9 @@ impl NetworkState { /// It does not check whether the ports exist. /// /// TODO: check whether a port is missing. - fn update_controller_ports(&mut self, controller: &Connection) { - if let Connection::Bond(BondConnection { bond, .. }) = controller { - let controller_id = controller.id().to_string(); + fn update_controller_ports(&mut self, updated: &Connection) { + if let Connection::Bond(BondConnection { bond, .. }) = &updated { + let controller_id = updated.id().to_string(); for conn in self.connections.iter_mut() { if bond.ports.contains(&conn.id().to_string()) { conn.set_controller(&controller_id); @@ -118,11 +120,10 @@ impl NetworkState { #[cfg(test)] mod tests { - use uuid::Uuid; - use super::builder::ConnectionBuilder; use super::*; use crate::network::error::NetworkStateError; + use uuid::Uuid; #[test] fn test_add_connection() { @@ -248,6 +249,32 @@ mod tests { let eth0 = state.get_connection("eth0").unwrap(); assert_eq!(eth0.controller().unwrap(), "bond0"); } + + #[test] + fn test_update_bonding() { + let mut state = NetworkState::default(); + let eth0 = ConnectionBuilder::new("eth0").build(); + let eth1 = ConnectionBuilder::new("eth1").build(); + state.add_connection(eth0).unwrap(); + state.add_connection(eth1).unwrap(); + + let bond0 = ConnectionBuilder::new("bond0") + .with_type(DeviceType::Bond) + .with_ports(vec!["eth0".to_string()]) + .build(); + state.add_connection(bond0).unwrap(); + + let bond0 = ConnectionBuilder::new("bond0") + .with_type(DeviceType::Bond) + .with_ports(vec!["eth1".to_string()]) + .build(); + state.update_connection(bond0).unwrap(); + + let eth1_found = state.get_connection("eth1").unwrap(); + assert_eq!(eth1_found.controller(), Some(&"bond0".to_string())); + let eth0_found = state.get_connection("eth0").unwrap(); + assert_eq!(eth0_found.controller(), None); + } } /// Network device From f08b2853f5a42a00fe77e20e2ba11470c1df0a26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 12 Dec 2023 11:34:21 +0000 Subject: [PATCH 35/58] Use a channel to get the connection in the Bond interface --- rust/agama-dbus-server/src/network/action.rs | 6 ++++ .../src/network/dbus/interfaces.rs | 28 +++++++++++-------- .../src/network/dbus/tree.rs | 3 +- rust/agama-dbus-server/src/network/model.rs | 11 ++++++-- rust/agama-dbus-server/src/network/system.rs | 4 +++ 5 files changed, 38 insertions(+), 14 deletions(-) diff --git a/rust/agama-dbus-server/src/network/action.rs b/rust/agama-dbus-server/src/network/action.rs index 9f935f70f1..830d371167 100644 --- a/rust/agama-dbus-server/src/network/action.rs +++ b/rust/agama-dbus-server/src/network/action.rs @@ -1,5 +1,9 @@ use crate::network::model::Connection; use agama_lib::network::types::DeviceType; +use tokio::sync::oneshot; +use uuid::Uuid; + +pub type Responder = oneshot::Sender; /// Networking actions, like adding, updating or removing connections. /// @@ -9,6 +13,8 @@ use agama_lib::network::types::DeviceType; pub enum Action { /// Add a new connection with the given name and type. AddConnection(String, DeviceType), + /// Gets a connection + GetConnection(Uuid, Responder>), /// Update a connection (replacing the old one). UpdateConnection(Connection), /// Remove the connection with the given Uuid. diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index b8d66deba1..d7dd9f666c 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -14,8 +14,9 @@ use crate::network::{ }; use agama_lib::network::types::SSID; +use uuid::Uuid; use std::{str::FromStr, sync::Arc}; -use tokio::sync::mpsc::UnboundedSender; +use tokio::sync::{mpsc::UnboundedSender, oneshot}; use tokio::sync::{MappedMutexGuard, Mutex, MutexGuard}; use zbus::{ dbus_interface, @@ -301,32 +302,37 @@ impl Match { /// D-Bus interface for Bond settings pub struct Bond { actions: Arc>>, - connection: Arc>, + uuid: Uuid, } impl Bond { /// Creates a Match Settings interface object. /// /// * `actions`: sending-half of a channel to send actions. - /// * `connection`: connection to expose over D-Bus. + /// * `uuid`: connection UUID. pub fn new( actions: UnboundedSender, - connection: Arc>, + uuid: &Uuid, ) -> Self { Self { actions: Arc::new(Mutex::new(actions)), - connection, + uuid: uuid.to_owned() } } /// Gets the bond connection. /// /// Beware that it crashes when it is not a bond connection. - async fn get_bond(&self) -> MappedMutexGuard { - MutexGuard::map(self.connection.lock().await, |c| match c { - NetworkConnection::Bond(config) => config, - _ => panic!("Not a bond connection. This is most probably a bug."), - }) + async fn get_bond(&self) -> BondConnection { + let actions = self.actions.lock().await; + let (tx, rx) = oneshot::channel(); + actions.send(Action::GetConnection(self.uuid.clone(), tx)).unwrap(); + let connection = rx.await.unwrap(); + + match connection { + Some(NetworkConnection::Bond(config)) => config, + _ => panic!("Not a bond connection. This is most probably a bug.") + } } /// Updates the connection data in the NetworkSystem. @@ -334,7 +340,7 @@ impl Bond { /// * `connection`: Updated connection. async fn update_connection<'a>( &self, - connection: MappedMutexGuard<'a, BondConnection>, + connection: BondConnection, ) -> zbus::fdo::Result<()> { let actions = self.actions.lock().await; let connection = NetworkConnection::Bond(connection.clone()); diff --git a/rust/agama-dbus-server/src/network/dbus/tree.rs b/rust/agama-dbus-server/src/network/dbus/tree.rs index e56f51638d..975dd984b4 100644 --- a/rust/agama-dbus-server/src/network/dbus/tree.rs +++ b/rust/agama-dbus-server/src/network/dbus/tree.rs @@ -88,6 +88,7 @@ impl Tree { let mut objects = self.objects.lock().await; let orig_id = conn.id().to_owned(); + let uuid = conn.uuid(); let (id, path) = objects.register_connection(conn); if id != conn.id() { conn.set_id(&id) @@ -116,7 +117,7 @@ impl Tree { if let Connection::Bond(_) = conn { self.add_interface( &path, - interfaces::Bond::new(self.actions.clone(), Arc::clone(&cloned)), + interfaces::Bond::new(self.actions.clone(), &uuid), ) .await?; } diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index f4bc419a0f..709f846e25 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -43,6 +43,13 @@ impl NetworkState { self.devices.iter().find(|d| d.name == name) } + /// Get connection by UUID + /// + /// * `id`: connection UUID + pub fn get_connection_by_uuid(&self, uuid: &Uuid) -> Option<&Connection> { + self.connections.iter().find(|c| c.uuid() == *uuid) + } + /// Get connection by ID /// /// * `id`: connection ID @@ -50,9 +57,9 @@ impl NetworkState { self.connections.iter().find(|c| c.id() == id) } - /// Get connection by UUID as mutable + /// Get connection by ID as mutable /// - /// * `uuid`: connection UUID + /// * `id`: connection ID pub fn get_connection_mut(&mut self, id: &str) -> Option<&mut Connection> { self.connections.iter_mut().find(|c| c.id() == id) } diff --git a/rust/agama-dbus-server/src/network/system.rs b/rust/agama-dbus-server/src/network/system.rs index fcfb3e170e..c79757d5ff 100644 --- a/rust/agama-dbus-server/src/network/system.rs +++ b/rust/agama-dbus-server/src/network/system.rs @@ -70,6 +70,10 @@ impl NetworkSystem { self.tree.add_connection(&mut conn, true).await?; self.state.add_connection(conn)?; } + Action::GetConnection(uuid, rx) => { + let conn = self.state.get_connection_by_uuid(&uuid); + rx.send(conn.cloned()).unwrap(); + } Action::UpdateConnection(conn) => { self.state.update_connection(conn)?; } From 79fc75386a08ae93fd7abc7caa03dcb2031acb37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 12 Dec 2023 14:47:20 +0000 Subject: [PATCH 36/58] Clean unused code --- rust/agama-dbus-server/src/network/model.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 709f846e25..3c043ffc96 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -718,12 +718,6 @@ pub struct BondConfig { pub options: BondOptions, } -/// Controller config payload for updating a controller's connection (Bond, Bridge) -#[derive(Debug, PartialEq, Clone)] -pub enum ControllerConfig { - Ports(Vec), -} - #[derive(Debug, Default, PartialEq, Clone)] pub struct WirelessConfig { pub mode: WirelessMode, From b72cf2383c8bd0a314d78df3faab32a046469796 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 12 Dec 2023 14:52:53 +0000 Subject: [PATCH 37/58] Do not keep the ports in the bond connection --- rust/agama-dbus-server/src/network/action.rs | 10 ++ .../src/network/dbus/interfaces.rs | 38 ++++---- .../src/network/dbus/tree.rs | 7 +- rust/agama-dbus-server/src/network/model.rs | 92 ++++++++----------- .../src/network/model/builder.rs | 22 +---- .../src/network/nm/adapter.rs | 18 ++-- rust/agama-dbus-server/src/network/system.rs | 20 +++- 7 files changed, 102 insertions(+), 105 deletions(-) diff --git a/rust/agama-dbus-server/src/network/action.rs b/rust/agama-dbus-server/src/network/action.rs index 830d371167..bfc2651926 100644 --- a/rust/agama-dbus-server/src/network/action.rs +++ b/rust/agama-dbus-server/src/network/action.rs @@ -3,7 +3,10 @@ use agama_lib::network::types::DeviceType; use tokio::sync::oneshot; use uuid::Uuid; +use super::error::NetworkStateError; + pub type Responder = oneshot::Sender; +pub type ControllerConnection = (Connection, Vec); /// Networking actions, like adding, updating or removing connections. /// @@ -15,6 +18,13 @@ pub enum Action { AddConnection(String, DeviceType), /// Gets a connection GetConnection(Uuid, Responder>), + /// Gets a controller connection + GetController( + Uuid, + Responder>, + ), + /// Sets a controller ports + SetPorts(Uuid, Vec, Responder>), /// Update a connection (replacing the old one). UpdateConnection(Connection), /// Remove the connection with the given Uuid. diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index d7dd9f666c..e712b91110 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -14,10 +14,10 @@ use crate::network::{ }; use agama_lib::network::types::SSID; -use uuid::Uuid; use std::{str::FromStr, sync::Arc}; use tokio::sync::{mpsc::UnboundedSender, oneshot}; use tokio::sync::{MappedMutexGuard, Mutex, MutexGuard}; +use uuid::Uuid; use zbus::{ dbus_interface, zvariant::{ObjectPath, OwnedObjectPath}, @@ -310,13 +310,10 @@ impl Bond { /// /// * `actions`: sending-half of a channel to send actions. /// * `uuid`: connection UUID. - pub fn new( - actions: UnboundedSender, - uuid: &Uuid, - ) -> Self { + pub fn new(actions: UnboundedSender, uuid: Uuid) -> Self { Self { actions: Arc::new(Mutex::new(actions)), - uuid: uuid.to_owned() + uuid, } } @@ -326,22 +323,19 @@ impl Bond { async fn get_bond(&self) -> BondConnection { let actions = self.actions.lock().await; let (tx, rx) = oneshot::channel(); - actions.send(Action::GetConnection(self.uuid.clone(), tx)).unwrap(); + actions.send(Action::GetConnection(self.uuid, tx)).unwrap(); let connection = rx.await.unwrap(); match connection { Some(NetworkConnection::Bond(config)) => config, - _ => panic!("Not a bond connection. This is most probably a bug.") + _ => panic!("Not a bond connection. This is most probably a bug."), } } /// Updates the connection data in the NetworkSystem. /// /// * `connection`: Updated connection. - async fn update_connection<'a>( - &self, - connection: BondConnection, - ) -> zbus::fdo::Result<()> { + async fn update_connection<'a>(&self, connection: BondConnection) -> zbus::fdo::Result<()> { let actions = self.actions.lock().await; let connection = NetworkConnection::Bond(connection.clone()); actions.send(Action::UpdateConnection(connection)).unwrap(); @@ -381,16 +375,24 @@ impl Bond { /// List of bond ports. #[dbus_interface(property)] - pub async fn ports(&self) -> Vec { - let connection = self.get_bond().await; - connection.bond.ports.clone() + pub async fn ports(&self) -> zbus::fdo::Result> { + let actions = self.actions.lock().await; + let (tx, rx) = oneshot::channel(); + actions.send(Action::GetController(self.uuid, tx)).unwrap(); + + let (_, ports) = rx.await.unwrap()?; + Ok(ports) } #[dbus_interface(property)] pub async fn set_ports(&mut self, ports: Vec) -> zbus::fdo::Result<()> { - let mut connection = self.get_bond().await; - connection.set_ports(ports); - self.update_connection(connection).await + let actions = self.actions.lock().await; + let (tx, rx) = oneshot::channel(); + actions + .send(Action::SetPorts(self.uuid, ports, tx)) + .unwrap(); + let result = rx.await.unwrap(); + Ok(result?) } } diff --git a/rust/agama-dbus-server/src/network/dbus/tree.rs b/rust/agama-dbus-server/src/network/dbus/tree.rs index 975dd984b4..aaf5dc5097 100644 --- a/rust/agama-dbus-server/src/network/dbus/tree.rs +++ b/rust/agama-dbus-server/src/network/dbus/tree.rs @@ -115,11 +115,8 @@ impl Tree { .await?; if let Connection::Bond(_) = conn { - self.add_interface( - &path, - interfaces::Bond::new(self.actions.clone(), &uuid), - ) - .await?; + self.add_interface(&path, interfaces::Bond::new(self.actions.clone(), uuid)) + .await?; } if let Connection::Wireless(_) = conn { diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 3c043ffc96..efed1a8b01 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -46,8 +46,8 @@ impl NetworkState { /// Get connection by UUID /// /// * `id`: connection UUID - pub fn get_connection_by_uuid(&self, uuid: &Uuid) -> Option<&Connection> { - self.connections.iter().find(|c| c.uuid() == *uuid) + pub fn get_connection_by_uuid(&self, uuid: Uuid) -> Option<&Connection> { + self.connections.iter().find(|c| c.uuid() == uuid) } /// Get connection by ID @@ -64,6 +64,14 @@ impl NetworkState { self.connections.iter_mut().find(|c| c.id() == id) } + pub fn get_controlled_by(&mut self, uuid: Uuid) -> Vec<&Connection> { + let uuid = Some(uuid); + self.connections + .iter() + .filter(|c| c.controller() == uuid) + .collect() + } + /// Adds a new connection. /// /// It uses the `id` to decide whether the connection already exists. @@ -71,8 +79,6 @@ impl NetworkState { if self.get_connection(conn.id()).is_some() { return Err(NetworkStateError::ConnectionExists(conn.uuid())); } - - self.update_controller_ports(&conn); self.connections.push(conn); Ok(()) @@ -87,11 +93,7 @@ impl NetworkState { let Some(old_conn) = self.get_connection_mut(conn.id()) else { return Err(NetworkStateError::UnknownConnection(conn.id().to_string())); }; - - // workaround the borrow checker - let clone = conn.clone(); *old_conn = conn; - self.update_controller_ports(&clone); Ok(()) } @@ -108,20 +110,27 @@ impl NetworkState { Ok(()) } - /// It does not check whether the ports exist. + /// Sets a controller ports. + /// + /// If the connection is not a controller, do nothing (TODO: return an error). /// - /// TODO: check whether a port is missing. - fn update_controller_ports(&mut self, updated: &Connection) { - if let Connection::Bond(BondConnection { bond, .. }) = &updated { - let controller_id = updated.id().to_string(); + /// * `uuid`: controller UUID. + /// * `ports`: list of port names. TODO: should we use the ID, the UUID or the interface name? + pub fn set_ports( + &mut self, + controller: &Connection, + ports: Vec, + ) -> Result<(), NetworkStateError> { + if let Connection::Bond(_) = &controller { for conn in self.connections.iter_mut() { - if bond.ports.contains(&conn.id().to_string()) { - conn.set_controller(&controller_id); - } else if conn.controller() == Some(&controller_id) { + if ports.contains(&conn.id().to_string()) { + conn.set_controller(controller.uuid()); + } else if conn.controller() == Some(controller.uuid()) { conn.unset_controller(); } } } + Ok(()) } } @@ -239,46 +248,22 @@ mod tests { } #[test] - fn test_add_bonding() { + fn test_set_bonding_ports() { let mut state = NetworkState::default(); - let eth0 = ConnectionBuilder::new("eth0") - .with_type(DeviceType::Ethernet) - .build(); - state.add_connection(eth0).unwrap(); - + let eth0 = ConnectionBuilder::new("eth0").build(); + let eth1 = ConnectionBuilder::new("eth1").build(); let bond0 = ConnectionBuilder::new("bond0") .with_type(DeviceType::Bond) - .with_ports(vec!["eth0".to_string()]) .build(); - state.add_connection(bond0).unwrap(); - - let eth0 = state.get_connection("eth0").unwrap(); - assert_eq!(eth0.controller().unwrap(), "bond0"); - } - - #[test] - fn test_update_bonding() { - let mut state = NetworkState::default(); - let eth0 = ConnectionBuilder::new("eth0").build(); - let eth1 = ConnectionBuilder::new("eth1").build(); state.add_connection(eth0).unwrap(); state.add_connection(eth1).unwrap(); + state.add_connection(bond0.clone()).unwrap(); - let bond0 = ConnectionBuilder::new("bond0") - .with_type(DeviceType::Bond) - .with_ports(vec!["eth0".to_string()]) - .build(); - state.add_connection(bond0).unwrap(); - - let bond0 = ConnectionBuilder::new("bond0") - .with_type(DeviceType::Bond) - .with_ports(vec!["eth1".to_string()]) - .build(); - state.update_connection(bond0).unwrap(); + state.set_ports(&bond0, vec!["eth1".to_string()]).unwrap(); let eth1_found = state.get_connection("eth1").unwrap(); - assert_eq!(eth1_found.controller(), Some(&"bond0".to_string())); + assert_eq!(eth1_found.controller(), Some(bond0.uuid())); let eth0_found = state.get_connection("eth0").unwrap(); assert_eq!(eth0_found.controller(), None); } @@ -366,15 +351,15 @@ impl Connection { } /// Ports controller name, e.g.: bond0, br0 - pub fn controller(&self) -> Option<&String> { - self.base().controller.as_ref() + pub fn controller(&self) -> Option { + self.base().controller } /// Sets the ports controller. /// /// `controller`: Name of the controller (Bridge, Bond, Team), e.g.: bond0. - pub fn set_controller(&mut self, controller: &str) { - self.base_mut().controller = Some(controller.to_string()); + pub fn set_controller(&mut self, controller: Uuid) { + self.base_mut().controller = Some(controller) } pub fn unset_controller(&mut self) { @@ -435,7 +420,7 @@ pub struct BaseConnection { pub ip_config: IpConfig, pub status: Status, pub interface: String, - pub controller: Option, + pub controller: Option, pub match_config: MatchConfig, } @@ -665,10 +650,6 @@ pub struct BondConnection { } impl BondConnection { - pub fn set_ports(&mut self, ports: Vec) { - self.bond.ports = ports; - } - pub fn set_mode(&mut self, mode: BondMode) { self.bond.mode = mode; } @@ -714,7 +695,6 @@ impl fmt::Display for BondOptions { #[derive(Debug, Default, PartialEq, Clone)] pub struct BondConfig { pub mode: BondMode, - pub ports: Vec, pub options: BondOptions, } diff --git a/rust/agama-dbus-server/src/network/model/builder.rs b/rust/agama-dbus-server/src/network/model/builder.rs index 6994b2d808..fb94e1c006 100644 --- a/rust/agama-dbus-server/src/network/model/builder.rs +++ b/rust/agama-dbus-server/src/network/model/builder.rs @@ -1,4 +1,5 @@ use super::{Connection, DeviceType}; +use uuid::Uuid; // TODO: improve the implementation to allow calling methods only where it makes sense // depending on the type of the connection. @@ -7,9 +8,8 @@ use super::{Connection, DeviceType}; pub struct ConnectionBuilder { id: String, interface: Option, - controller: Option, + controller: Option, type_: Option, - ports: Vec, } impl ConnectionBuilder { @@ -25,8 +25,8 @@ impl ConnectionBuilder { self } - pub fn with_controller(mut self, controller: &str) -> Self { - self.controller = Some(controller.to_string()); + pub fn with_controller(mut self, controller: Uuid) -> Self { + self.controller = Some(controller); self } @@ -35,11 +35,6 @@ impl ConnectionBuilder { self } - pub fn with_ports(mut self, ports: Vec) -> Self { - self.ports = ports; - self - } - pub fn build(self) -> Connection { let mut conn = Connection::new(self.id, self.type_.unwrap_or(DeviceType::Ethernet)); @@ -48,14 +43,7 @@ impl ConnectionBuilder { } if let Some(controller) = self.controller { - conn.set_controller(&controller); - } - - match &mut conn { - Connection::Bond(bond) => { - bond.set_ports(self.ports); - } - _ => {} + conn.set_controller(controller); } conn diff --git a/rust/agama-dbus-server/src/network/nm/adapter.rs b/rust/agama-dbus-server/src/network/nm/adapter.rs index 82ee5dd983..ceee312947 100644 --- a/rust/agama-dbus-server/src/network/nm/adapter.rs +++ b/rust/agama-dbus-server/src/network/nm/adapter.rs @@ -41,7 +41,9 @@ impl<'a> NetworkManagerAdapter<'a> { let result = if conn.is_removed() { self.client.remove_connection(conn.uuid()).await } else { - let ctrl = conn.controller().and_then(|id| network.get_connection(&id)); + let ctrl = conn + .controller() + .and_then(|uuid| network.get_connection_by_uuid(uuid)); self.client.add_or_update_connection(&conn, ctrl).await }; @@ -72,13 +74,13 @@ impl<'a> NetworkManagerAdapter<'a> { ) { conns.push(conn); - if let Connection::Bond(BondConnection { bond, .. }) = &conn { - for port in &bond.ports { - if let Some(port_connection) = network.get_connection(port.as_str()) { - self.add_ordered_connections(port_connection, network, conns); - } - } - } + // if let Connection::Bond(BondConnection { bond, .. }) = &conn { + // for port in &bond.ports { + // if let Some(port_connection) = network.get_connection(port.as_str()) { + // self.add_ordered_connections(port_connection, network, conns); + // } + // } + // } } } diff --git a/rust/agama-dbus-server/src/network/system.rs b/rust/agama-dbus-server/src/network/system.rs index c79757d5ff..277cf2baf0 100644 --- a/rust/agama-dbus-server/src/network/system.rs +++ b/rust/agama-dbus-server/src/network/system.rs @@ -71,9 +71,27 @@ impl NetworkSystem { self.state.add_connection(conn)?; } Action::GetConnection(uuid, rx) => { - let conn = self.state.get_connection_by_uuid(&uuid); + let conn = self.state.get_connection_by_uuid(uuid); rx.send(conn.cloned()).unwrap(); } + Action::GetController(uuid, rx) => { + let conn = self.state.get_connection_by_uuid(uuid).unwrap(); + let conn = conn.clone(); + + let controlled = self.state.get_controlled_by(uuid); + let controlled = controlled + .iter() + .map(|c| c.id().to_owned()) + .collect::>(); + + let data = (conn, controlled); + rx.send(Ok(data)).unwrap() + } + Action::SetPorts(uuid, ports, rx) => { + let conn = self.state.get_connection_by_uuid(uuid).unwrap(); + let result = self.state.set_ports(&conn.clone(), ports); + rx.send(result).unwrap(); + } Action::UpdateConnection(conn) => { self.state.update_connection(conn)?; } From 48822a1ff002c0ccb57ae396d4ac1965948e2eea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 12 Dec 2023 16:03:00 +0000 Subject: [PATCH 38/58] Consider the connection interface name optional --- .../src/network/dbus/interfaces.rs | 10 +++++++--- rust/agama-dbus-server/src/network/model.rs | 8 ++++---- rust/agama-dbus-server/src/network/nm/dbus.rs | 16 +++++++++------- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index e712b91110..c266d38d43 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -234,7 +234,11 @@ impl Connection { #[dbus_interface(property)] pub async fn interface(&self) -> String { - self.get_connection().await.interface().to_string() + self.get_connection() + .await + .interface() + .map(String::to_string) + .unwrap_or(String::from("")) } #[dbus_interface(property)] @@ -299,7 +303,7 @@ impl Match { } } -/// D-Bus interface for Bond settings +/// D-Bus interface for Bond settings. pub struct Bond { actions: Arc>>, uuid: Uuid, @@ -345,7 +349,7 @@ impl Bond { #[dbus_interface(name = "org.opensuse.Agama1.Network.Connection.Bond")] impl Bond { - /// Bonding mode + /// Bonding mode. #[dbus_interface(property)] pub async fn mode(&self) -> String { let connection = self.get_bond().await; diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index efed1a8b01..6b47a7d902 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -337,12 +337,12 @@ impl Connection { self.base_mut().id = id.to_string() } - pub fn interface(&self) -> &str { - self.base().interface.as_str() + pub fn interface(&self) -> Option<&String> { + self.base().interface.as_ref() } pub fn set_interface(&mut self, interface: &str) { - self.base_mut().interface = interface.to_string() + self.base_mut().interface = Some(interface.to_string()) } /// Ports controller name, e.g.: bond0, br0 @@ -419,7 +419,7 @@ pub struct BaseConnection { pub mac_address: MacAddress, pub ip_config: IpConfig, pub status: Status, - pub interface: String, + pub interface: Option, pub controller: Option, pub match_config: MatchConfig, } diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index fd8d9abf24..c375edcb36 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -28,11 +28,13 @@ pub fn connection_to_dbus<'a>( controller: Option<&'a Connection>, ) -> NestedHash<'a> { let mut result = NestedHash::new(); - let mut connection_dbus = HashMap::from([ - ("id", conn.id().into()), - ("type", ETHERNET_KEY.into()), - ("interface-name", conn.interface().into()), - ]); + let mut connection_dbus = + HashMap::from([("id", conn.id().into()), ("type", ETHERNET_KEY.into())]); + + if let Some(interface) = &conn.interface() { + connection_dbus.insert("interface-name", interface.to_owned().into()); + } + if let Some(controller) = controller { connection_dbus.insert("slave-type", "bond".into()); // TODO: only 'bond' is supported connection_dbus.insert("master", controller.id().into()); @@ -329,7 +331,7 @@ fn base_connection_from_dbus(conn: &OwnedNestedHash) -> Option { if let Some(interface) = connection.get("interface-name") { let interface: &str = interface.downcast_ref()?; - base_connection.interface = interface.to_string(); + base_connection.interface = Some(interface.to_string()); } if let Some(match_config) = conn.get("match") { @@ -877,7 +879,7 @@ mod test { let base = BaseConnection { id: "agama".to_string(), - interface: "eth0".to_string(), + interface: Some("eth0".to_string()), ..Default::default() }; let ethernet = EthernetConnection { From aad9c0d8fb6140a2ca9cdfeb99da39b81ee0c3c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 13 Dec 2023 08:55:24 +0000 Subject: [PATCH 39/58] Imports the network connections in the right order --- rust/agama-lib/src/network/store.rs | 104 +++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 1 deletion(-) diff --git a/rust/agama-lib/src/network/store.rs b/rust/agama-lib/src/network/store.rs index b4711274a7..7ed70c332e 100644 --- a/rust/agama-lib/src/network/store.rs +++ b/rust/agama-lib/src/network/store.rs @@ -1,3 +1,4 @@ +use super::settings::NetworkConnection; use crate::error::ServiceError; use crate::network::{NetworkClient, NetworkSettings}; use zbus::Connection; @@ -22,7 +23,10 @@ impl<'a> NetworkStore<'a> { } pub async fn store(&self, settings: &NetworkSettings) -> Result<(), ServiceError> { - for conn in &settings.connections { + for id in ordered_connections(&settings.connections) { + let id = id.as_str(); + let fallback = default_connection(id); + let conn = find_connection(id, &settings.connections).unwrap_or(&fallback); self.network_client.add_or_update_connection(conn).await?; } self.network_client.apply().await?; @@ -30,3 +34,101 @@ impl<'a> NetworkStore<'a> { Ok(()) } } + +/// Returns the list of connections in the order they should be written to the D-Bus service. +/// +/// * `conns`: connections to write. +fn ordered_connections(conns: &Vec) -> Vec { + let mut ordered: Vec = Vec::with_capacity(conns.len()); + for conn in conns { + add_ordered_connection(conn, conns, &mut ordered); + } + ordered +} + +/// Adds a connections and its dependencies to the list. +/// +/// * `conn`: connection to add. +/// * `conns`: existing connections. +/// * `ordered`: ordered list of connections. +fn add_ordered_connection( + conn: &NetworkConnection, + conns: &Vec, + ordered: &mut Vec, +) { + if let Some(bond) = &conn.bond { + for port in &bond.ports { + if let Some(conn) = find_connection(port, conns) { + add_ordered_connection(conn, conns, ordered); + } else if !ordered.contains(&conn.id) { + ordered.push(port.clone()); + } + } + } + + if !ordered.contains(&conn.id) { + ordered.push(conn.id.to_owned()) + } +} + +/// Finds a connection by id in the list. +/// +/// * `id`: connection ID. +fn find_connection<'a>(id: &str, conns: &'a [NetworkConnection]) -> Option<&'a NetworkConnection> { + conns + .iter() + .find(|c| c.id == id || c.interface == Some(id.to_string())) +} + +fn default_connection(id: &str) -> NetworkConnection { + NetworkConnection { + id: id.to_string(), + interface: Some(id.to_string()), + ..Default::default() + } +} + +#[cfg(test)] +mod tests { + use super::ordered_connections; + use crate::network::settings::{BondSettings, NetworkConnection}; + + #[test] + fn test_ordered_connections() { + let bond = NetworkConnection { + id: "bond0".to_string(), + bond: Some(BondSettings { + ports: vec!["eth0".to_string(), "eth1".to_string(), "eth3".to_string()], + ..Default::default() + }), + ..Default::default() + }; + + let eth0 = NetworkConnection { + id: "eth0".to_string(), + ..Default::default() + }; + let eth1 = NetworkConnection { + id: "Wired connection".to_string(), + interface: Some("eth1".to_string()), + ..Default::default() + }; + let eth2 = NetworkConnection { + id: "eth2".to_string(), + ..Default::default() + }; + + let conns = vec![bond, eth0, eth1, eth2]; + let ordered = ordered_connections(&conns); + assert_eq!( + ordered, + vec![ + "eth0".to_string(), + "Wired connection".to_string(), + "eth3".to_string(), + "bond0".to_string(), + "eth2".to_string() + ] + ) + } +} From a4180a27e05000f47bfd5eb1b2b0abc844267be0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 13 Dec 2023 09:22:52 +0000 Subject: [PATCH 40/58] Writes the connections to NetworkManager in the right order --- .../src/network/nm/adapter.rs | 63 +++++++++---------- 1 file changed, 29 insertions(+), 34 deletions(-) diff --git a/rust/agama-dbus-server/src/network/nm/adapter.rs b/rust/agama-dbus-server/src/network/nm/adapter.rs index ceee312947..5e39da2fc9 100644 --- a/rust/agama-dbus-server/src/network/nm/adapter.rs +++ b/rust/agama-dbus-server/src/network/nm/adapter.rs @@ -28,14 +28,13 @@ impl<'a> NetworkManagerAdapter<'a> { /// Writes the connections to NetworkManager. /// - /// Internally, it creates and order list of connections before processing them. The reason is + /// Internally, it creates an ordered list of connections before processing them. The reason is /// that using async recursive functions is giving us some troubles, so we decided to go with a /// simpler approach. /// /// * `network`: network model. async fn write_connections(&self, network: &NetworkState) { - let conns = self.ordered_connections(&network); - println!("Connections to write: {:?}", &conns); + let conns = ordered_connections(network); for conn in &conns { let result = if conn.is_removed() { @@ -44,7 +43,7 @@ impl<'a> NetworkManagerAdapter<'a> { let ctrl = conn .controller() .and_then(|uuid| network.get_connection_by_uuid(uuid)); - self.client.add_or_update_connection(&conn, ctrl).await + self.client.add_or_update_connection(conn, ctrl).await }; if let Err(e) = result { @@ -52,36 +51,6 @@ impl<'a> NetworkManagerAdapter<'a> { } } } - - /// Returns the connections in the order they should be processed. - /// - /// * `network`: network model. - fn ordered_connections<'b>(&self, network: &'b NetworkState) -> Vec<&'b Connection> { - let mut conns: Vec<&Connection> = vec![]; - for conn in &network.connections { - if !conn.is_controlled() { - self.add_ordered_connections(conn, network, &mut conns); - } - } - conns - } - - fn add_ordered_connections<'b>( - &self, - conn: &'b Connection, - network: &'b NetworkState, - conns: &mut Vec<&'b Connection>, - ) { - conns.push(conn); - - // if let Connection::Bond(BondConnection { bond, .. }) = &conn { - // for port in &bond.ports { - // if let Some(port_connection) = network.get_connection(port.as_str()) { - // self.add_ordered_connections(port_connection, network, conns); - // } - // } - // } - } } impl<'a> Adapter for NetworkManagerAdapter<'a> { @@ -113,3 +82,29 @@ impl<'a> Adapter for NetworkManagerAdapter<'a> { Ok(()) } } + +/// Returns the connections in the order they should be processed. +/// +/// * `network`: network model. +fn ordered_connections(network: &NetworkState) -> Vec<&Connection> { + let mut conns: Vec<&Connection> = vec![]; + for conn in &network.connections { + add_ordered_connections(conn, network, &mut conns); + } + conns +} + +fn add_ordered_connections<'b>( + conn: &'b Connection, + network: &'b NetworkState, + conns: &mut Vec<&'b Connection>, +) { + if let Some(uuid) = conn.controller() { + let controller = network.get_connection_by_uuid(uuid).unwrap(); + add_ordered_connections(controller, network, conns); + } + + if !conns.contains(&conn) { + conns.push(conn); + } +} From 9f05659701352a6742c3c78129a126e80ade36ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 13 Dec 2023 09:59:18 +0000 Subject: [PATCH 41/58] Handle errors setting bonding ports --- .../src/network/dbus/interfaces.rs | 3 + rust/agama-dbus-server/src/network/error.rs | 4 ++ rust/agama-dbus-server/src/network/model.rs | 58 +++++++++++++++++-- 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index c266d38d43..e33a4ca50f 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -378,6 +378,9 @@ impl Bond { } /// List of bond ports. + /// + /// It uses the connection interfaces as port names. It means that you cannot + /// add a connection if it does not have a interface name. #[dbus_interface(property)] pub async fn ports(&self) -> zbus::fdo::Result> { let actions = self.actions.lock().await; diff --git a/rust/agama-dbus-server/src/network/error.rs b/rust/agama-dbus-server/src/network/error.rs index bf2b4d6242..a8f9861ffe 100644 --- a/rust/agama-dbus-server/src/network/error.rs +++ b/rust/agama-dbus-server/src/network/error.rs @@ -27,6 +27,10 @@ pub enum NetworkStateError { InvalidBondMode(String), #[error("Invalid bond options")] InvalidBondOptions, + #[error("Not a controller connection: '{0}'")] + NotControllerConnection(String), + #[error("There is no connection associated to the interface '{0}'")] + NoConnectionForInterface(String), } impl From for zbus::fdo::Error { diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 6b47a7d902..8c53152ef7 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -50,6 +50,15 @@ impl NetworkState { self.connections.iter().find(|c| c.uuid() == uuid) } + /// Get connection by interface + /// + /// * `id`: connection interface name + pub fn get_connection_by_interface(&self, name: &str) -> Option<&Connection> { + let name = name.to_string(); + let interface = Some(&name); + self.connections.iter().find(|c| c.interface() == interface) + } + /// Get connection by ID /// /// * `id`: connection ID @@ -112,25 +121,37 @@ impl NetworkState { /// Sets a controller ports. /// - /// If the connection is not a controller, do nothing (TODO: return an error). + /// If the connection is not a controller, returns an error. /// /// * `uuid`: controller UUID. - /// * `ports`: list of port names. TODO: should we use the ID, the UUID or the interface name? + /// * `ports`: list of port names (using the interface name). pub fn set_ports( &mut self, controller: &Connection, ports: Vec, ) -> Result<(), NetworkStateError> { if let Connection::Bond(_) = &controller { + let mut controlled = vec![]; + for port in ports { + let connection = self + .get_connection_by_interface(&port) + .ok_or(NetworkStateError::NoConnectionForInterface(port))?; + controlled.push(connection.uuid()); + } + for conn in self.connections.iter_mut() { - if ports.contains(&conn.id().to_string()) { + if controlled.contains(&conn.uuid()) { conn.set_controller(controller.uuid()); } else if conn.controller() == Some(controller.uuid()) { conn.unset_controller(); } } + Ok(()) + } else { + Err(NetworkStateError::NotControllerConnection( + controller.id().to_owned(), + )) } - Ok(()) } } @@ -267,6 +288,35 @@ mod tests { let eth0_found = state.get_connection("eth0").unwrap(); assert_eq!(eth0_found.controller(), None); } + + #[test] + fn test_set_bonding_missing_port() { + let mut state = NetworkState::default(); + let bond0 = ConnectionBuilder::new("bond0") + .with_type(DeviceType::Bond) + .build(); + state.add_connection(bond0.clone()).unwrap(); + + let error = state + .set_ports(&bond0, vec!["eth0".to_string()]) + .unwrap_err(); + assert!(matches!(error, NetworkStateError::UnknownConnection(_))); + } + + #[test] + fn test_set_non_controller_ports() { + let mut state = NetworkState::default(); + let eth0 = ConnectionBuilder::new("eth0").build(); + state.add_connection(eth0.clone()).unwrap(); + + let error = state + .set_ports(ð0, vec!["eth1".to_string()]) + .unwrap_err(); + assert!(matches!( + error, + NetworkStateError::NotControllerConnection(_), + )); + } } /// Network device From 779e39a9735bbf10f0f664ed7e52d66c176ae2a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 13 Dec 2023 09:59:46 +0000 Subject: [PATCH 42/58] Assign an UUID to each network connection --- rust/agama-dbus-server/src/network/model.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 8c53152ef7..524b5edcfe 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -340,6 +340,7 @@ impl Connection { pub fn new(id: String, device_type: DeviceType) -> Self { let base = BaseConnection { id, + uuid: Uuid::new_v4(), ..Default::default() }; match device_type { From 71155097c4b440d83b2ebc40dcb891c8101579d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 13 Dec 2023 10:49:53 +0000 Subject: [PATCH 43/58] Fix test_set_bonding_ports test --- rust/agama-dbus-server/src/network/model.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 524b5edcfe..fa7eaa38f1 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -271,8 +271,12 @@ mod tests { #[test] fn test_set_bonding_ports() { let mut state = NetworkState::default(); - let eth0 = ConnectionBuilder::new("eth0").build(); - let eth1 = ConnectionBuilder::new("eth1").build(); + let eth0 = ConnectionBuilder::new("eth0") + .with_interface("eth0") + .build(); + let eth1 = ConnectionBuilder::new("eth1") + .with_interface("eth1") + .build(); let bond0 = ConnectionBuilder::new("bond0") .with_type(DeviceType::Bond) .build(); From d8e0ad3adf9cfeb641979e1b1f761ea364c84539 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 13 Dec 2023 14:43:20 +0000 Subject: [PATCH 44/58] Fix writing of network connections --- .../src/network/dbus/interfaces.rs | 4 +- rust/agama-dbus-server/src/network/error.rs | 2 + rust/agama-dbus-server/src/network/model.rs | 7 ++- .../src/network/nm/adapter.rs | 50 ++++++++----------- rust/agama-dbus-server/src/network/nm/dbus.rs | 3 +- rust/agama-dbus-server/src/network/system.rs | 19 +++++-- 6 files changed, 46 insertions(+), 39 deletions(-) diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index e33a4ca50f..cbd3853803 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -237,8 +237,8 @@ impl Connection { self.get_connection() .await .interface() - .map(String::to_string) - .unwrap_or(String::from("")) + .unwrap_or("") + .to_string() } #[dbus_interface(property)] diff --git a/rust/agama-dbus-server/src/network/error.rs b/rust/agama-dbus-server/src/network/error.rs index a8f9861ffe..7a82e46f2d 100644 --- a/rust/agama-dbus-server/src/network/error.rs +++ b/rust/agama-dbus-server/src/network/error.rs @@ -31,6 +31,8 @@ pub enum NetworkStateError { NotControllerConnection(String), #[error("There is no connection associated to the interface '{0}'")] NoConnectionForInterface(String), + #[error("Missing connection with UUID '{0}'")] + MissingConnection(Uuid), } impl From for zbus::fdo::Error { diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index fa7eaa38f1..228a69d87e 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -54,8 +54,7 @@ impl NetworkState { /// /// * `id`: connection interface name pub fn get_connection_by_interface(&self, name: &str) -> Option<&Connection> { - let name = name.to_string(); - let interface = Some(&name); + let interface = Some(name); self.connections.iter().find(|c| c.interface() == interface) } @@ -392,8 +391,8 @@ impl Connection { self.base_mut().id = id.to_string() } - pub fn interface(&self) -> Option<&String> { - self.base().interface.as_ref() + pub fn interface(&self) -> Option<&str> { + self.base().interface.as_deref() } pub fn set_interface(&mut self, interface: &str) { diff --git a/rust/agama-dbus-server/src/network/nm/adapter.rs b/rust/agama-dbus-server/src/network/nm/adapter.rs index 5e39da2fc9..26a7f75b1e 100644 --- a/rust/agama-dbus-server/src/network/nm/adapter.rs +++ b/rust/agama-dbus-server/src/network/nm/adapter.rs @@ -1,5 +1,5 @@ use crate::network::{ - model::{BondConnection, Connection, NetworkState}, + model::{Connection, NetworkState}, nm::NetworkManagerClient, Adapter, }; @@ -25,32 +25,6 @@ impl<'a> NetworkManagerAdapter<'a> { fn is_writable(conn: &Connection) -> bool { !conn.is_loopback() } - - /// Writes the connections to NetworkManager. - /// - /// Internally, it creates an ordered list of connections before processing them. The reason is - /// that using async recursive functions is giving us some troubles, so we decided to go with a - /// simpler approach. - /// - /// * `network`: network model. - async fn write_connections(&self, network: &NetworkState) { - let conns = ordered_connections(network); - - for conn in &conns { - let result = if conn.is_removed() { - self.client.remove_connection(conn.uuid()).await - } else { - let ctrl = conn - .controller() - .and_then(|uuid| network.get_connection_by_uuid(uuid)); - self.client.add_or_update_connection(conn, ctrl).await - }; - - if let Err(e) = result { - log::error!("Could not process the connection {}: {}", conn.id(), e); - } - } - } } impl<'a> Adapter for NetworkManagerAdapter<'a> { @@ -65,16 +39,34 @@ impl<'a> Adapter for NetworkManagerAdapter<'a> { }) } + /// Writes the connections to NetworkManager. + /// + /// Internally, it creates an ordered list of connections before processing them. The reason is + /// that using async recursive functions is giving us some troubles, so we decided to go with a + /// simpler approach. + /// + /// * `network`: network model. fn write(&self, network: &NetworkState) -> Result<(), Box> { // By now, traits do not support async functions. Using `task::block_on` allows // to use 'await'. task::block_in_place(|| { Handle::current().block_on(async { - for conn in &network.connections { + for conn in ordered_connections(network) { if !Self::is_writable(conn) { continue; } - self.write_connections(network).await; + let result = if conn.is_removed() { + self.client.remove_connection(conn.uuid()).await + } else { + let ctrl = conn + .controller() + .and_then(|uuid| network.get_connection_by_uuid(uuid)); + self.client.add_or_update_connection(conn, ctrl).await + }; + + if let Err(e) = result { + log::error!("Could not process the connection {}: {}", conn.id(), e); + } } }) }); diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index c375edcb36..0c646a4e9a 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -37,7 +37,8 @@ pub fn connection_to_dbus<'a>( if let Some(controller) = controller { connection_dbus.insert("slave-type", "bond".into()); // TODO: only 'bond' is supported - connection_dbus.insert("master", controller.id().into()); + let master = controller.interface().unwrap_or(controller.id()); + connection_dbus.insert("master", master.into()); } result.insert("ipv4", ip_config_to_ipv4_dbus(conn.ip_config())); diff --git a/rust/agama-dbus-server/src/network/system.rs b/rust/agama-dbus-server/src/network/system.rs index 277cf2baf0..3d354957f7 100644 --- a/rust/agama-dbus-server/src/network/system.rs +++ b/rust/agama-dbus-server/src/network/system.rs @@ -1,6 +1,8 @@ +use super::error::NetworkStateError; use crate::network::{dbus::Tree, model::Connection, Action, Adapter, NetworkState}; use std::error::Error; use tokio::sync::mpsc::{self, UnboundedReceiver, UnboundedSender}; +use uuid::Uuid; /// Represents the network system using holding the state and setting up the D-Bus tree. pub struct NetworkSystem { @@ -81,15 +83,14 @@ impl NetworkSystem { let controlled = self.state.get_controlled_by(uuid); let controlled = controlled .iter() - .map(|c| c.id().to_owned()) + .filter_map(|c| c.interface().map(|c| c.to_string())) .collect::>(); let data = (conn, controlled); rx.send(Ok(data)).unwrap() } Action::SetPorts(uuid, ports, rx) => { - let conn = self.state.get_connection_by_uuid(uuid).unwrap(); - let result = self.state.set_ports(&conn.clone(), ports); + let result = self.set_ports_action(uuid, ports); rx.send(result).unwrap(); } Action::UpdateConnection(conn) => { @@ -111,4 +112,16 @@ impl NetworkSystem { Ok(()) } + + fn set_ports_action( + &mut self, + uuid: Uuid, + ports: Vec, + ) -> Result<(), NetworkStateError> { + let conn = self + .state + .get_connection_by_uuid(uuid) + .ok_or(NetworkStateError::MissingConnection(uuid))?; + self.state.set_ports(&conn.clone(), ports) + } } From 483f77128636d34807dd0e932d007af87daaddf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 13 Dec 2023 15:36:36 +0000 Subject: [PATCH 45/58] Always write the interface-name in bonds --- rust/agama-dbus-server/src/network/nm/dbus.rs | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index 0c646a4e9a..1e7f2aa48e 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -45,25 +45,29 @@ pub fn connection_to_dbus<'a>( result.insert("ipv6", ip_config_to_ipv6_dbus(conn.ip_config())); result.insert("match", match_config_to_dbus(conn.match_config())); - if conn.is_ethernet() { - let ethernet_config = - HashMap::from([("assigned-mac-address", Value::new(conn.mac_address()))]); - result.insert(ETHERNET_KEY, ethernet_config); - } else if let Connection::Wireless(wireless) = conn { - connection_dbus.insert("type", WIRELESS_KEY.into()); - let wireless_dbus = wireless_config_to_dbus(wireless); - for (k, v) in wireless_dbus { - result.insert(k, v); + match &conn { + Connection::Ethernet(_) | Connection::Loopback(_) => { + let ethernet_config = + HashMap::from([("assigned-mac-address", Value::new(conn.mac_address()))]); + result.insert(ETHERNET_KEY, ethernet_config); + } + Connection::Wireless(wireless) => { + connection_dbus.insert("type", WIRELESS_KEY.into()); + let wireless_dbus = wireless_config_to_dbus(wireless); + for (k, v) in wireless_dbus { + result.insert(k, v); + } + } + Connection::Bond(bond) => { + connection_dbus.insert("type", BOND_KEY.into()); + if !connection_dbus.contains_key("interface-name") { + connection_dbus.insert("interface-name", conn.id().into()); + } + result.insert("bond", bond_config_to_dbus(bond)); + } + Connection::Dummy(_) => { + connection_dbus.insert("type", DUMMY_KEY.into()); } - } - - if let Connection::Bond(bond) = conn { - connection_dbus.insert("type", BOND_KEY.into()); - result.insert("bond", bond_config_to_dbus(bond)); - } - - if let Connection::Dummy(_) = conn { - connection_dbus.insert("type", DUMMY_KEY.into()); } result.insert("connection", connection_dbus); From fa01ee0505e49b7e6a8514438d65530a49dfb3d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 13 Dec 2023 16:03:58 +0000 Subject: [PATCH 46/58] Improve error reporting when adding a bond --- rust/agama-dbus-server/src/network/error.rs | 2 -- rust/agama-dbus-server/src/network/model.rs | 10 +++++++--- rust/agama-dbus-server/tests/network.rs | 9 +++++++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/rust/agama-dbus-server/src/network/error.rs b/rust/agama-dbus-server/src/network/error.rs index 7a82e46f2d..c2976bf386 100644 --- a/rust/agama-dbus-server/src/network/error.rs +++ b/rust/agama-dbus-server/src/network/error.rs @@ -29,8 +29,6 @@ pub enum NetworkStateError { InvalidBondOptions, #[error("Not a controller connection: '{0}'")] NotControllerConnection(String), - #[error("There is no connection associated to the interface '{0}'")] - NoConnectionForInterface(String), #[error("Missing connection with UUID '{0}'")] MissingConnection(Uuid), } diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 228a69d87e..aa782a4ea2 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -132,9 +132,12 @@ impl NetworkState { if let Connection::Bond(_) = &controller { let mut controlled = vec![]; for port in ports { - let connection = self - .get_connection_by_interface(&port) - .ok_or(NetworkStateError::NoConnectionForInterface(port))?; + let by_interface = self.get_connection_by_interface(&port); + let by_id = self.get_connection(&port); + + let connection = by_interface + .or(by_id) + .ok_or(NetworkStateError::UnknownConnection(port))?; controlled.push(connection.uuid()); } @@ -303,6 +306,7 @@ mod tests { let error = state .set_ports(&bond0, vec!["eth0".to_string()]) .unwrap_err(); + dbg!(&error); assert!(matches!(error, NetworkStateError::UnknownConnection(_))); } diff --git a/rust/agama-dbus-server/tests/network.rs b/rust/agama-dbus-server/tests/network.rs index a99cb0c1b7..8ce55e3176 100644 --- a/rust/agama-dbus-server/tests/network.rs +++ b/rust/agama-dbus-server/tests/network.rs @@ -106,6 +106,10 @@ async fn test_add_bond_connection() -> Result<(), Box> { server.request_name().await?; let client = NetworkClient::new(server.connection().clone()).await?; + let eth0 = settings::NetworkConnection { + id: "eth0".to_string(), + ..Default::default() + }; let bond0 = settings::NetworkConnection { id: "bond0".to_string(), method4: Some("auto".to_string()), @@ -113,15 +117,16 @@ async fn test_add_bond_connection() -> Result<(), Box> { interface: Some("bond0".to_string()), bond: Some(settings::BondSettings { mode: "active-backup".to_string(), - ports: vec!["eth0".to_string(), "eth1".to_string()], + ports: vec!["eth0".to_string()], options: Some("primary=eth1".to_string()), }), ..Default::default() }; + client.add_or_update_connection(ð0).await?; client.add_or_update_connection(&bond0).await?; let conns = async_retry(|| client.connections()).await?; - assert_eq!(conns.len(), 1); + assert_eq!(conns.len(), 2); let conn = conns.iter().find(|c| c.id == "bond0".to_string()).unwrap(); assert_eq!(conn.id, "bond0"); From e7e2d634652e527de5283053cab2db86b1b2799c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 13 Dec 2023 16:04:39 +0000 Subject: [PATCH 47/58] Fix network example --- rust/agama-lib/share/examples/profile.jsonnet | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rust/agama-lib/share/examples/profile.jsonnet b/rust/agama-lib/share/examples/profile.jsonnet index e5ae5d01aa..b37e986809 100644 --- a/rust/agama-lib/share/examples/profile.jsonnet +++ b/rust/agama-lib/share/examples/profile.jsonnet @@ -47,7 +47,8 @@ local findBiggestDisk(disks) = wireless: { password: 'agama.test', security: 'wpa-psk', - ssid: 'AgamaNetwork' + ssid: 'AgamaNetwork', + mode: 'infrastructure' } }, { @@ -68,7 +69,8 @@ local findBiggestDisk(disks) = id: 'bond0', bond: { ports: ['eth0', 'eth1'], - options: "mode=active-backup primary=eth1" + mode: 'active-backup', + options: "primary=eth1" } } From c5f4b0702713e413a85fa1287093494e48495025 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 13 Dec 2023 17:53:31 +0000 Subject: [PATCH 48/58] Minor refactoring --- rust/agama-dbus-server/src/network/model.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index aa782a4ea2..e9e017f998 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -132,11 +132,9 @@ impl NetworkState { if let Connection::Bond(_) = &controller { let mut controlled = vec![]; for port in ports { - let by_interface = self.get_connection_by_interface(&port); - let by_id = self.get_connection(&port); - - let connection = by_interface - .or(by_id) + let connection = self + .get_connection_by_interface(&port) + .or_else(|| self.get_connection(&port)) .ok_or(NetworkStateError::UnknownConnection(port))?; controlled.push(connection.uuid()); } From f17fb52412b88e6b8217d32ec307f83fbc7e1033 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 13 Dec 2023 22:14:06 +0000 Subject: [PATCH 49/58] Fallback to connection IDs when exposing the ports --- rust/agama-dbus-server/src/network/action.rs | 3 +- rust/agama-dbus-server/src/network/error.rs | 4 +-- rust/agama-dbus-server/src/network/model.rs | 2 +- rust/agama-dbus-server/src/network/system.rs | 35 +++++++++++++------- 4 files changed, 27 insertions(+), 17 deletions(-) diff --git a/rust/agama-dbus-server/src/network/action.rs b/rust/agama-dbus-server/src/network/action.rs index bfc2651926..e22683452b 100644 --- a/rust/agama-dbus-server/src/network/action.rs +++ b/rust/agama-dbus-server/src/network/action.rs @@ -23,7 +23,8 @@ pub enum Action { Uuid, Responder>, ), - /// Sets a controller ports + /// Sets a controller ports. It uses the Uuid of the controller and the IDs or interface names + /// of the ports. SetPorts(Uuid, Vec, Responder>), /// Update a connection (replacing the old one). UpdateConnection(Connection), diff --git a/rust/agama-dbus-server/src/network/error.rs b/rust/agama-dbus-server/src/network/error.rs index c2976bf386..45788bb9b5 100644 --- a/rust/agama-dbus-server/src/network/error.rs +++ b/rust/agama-dbus-server/src/network/error.rs @@ -5,7 +5,7 @@ use uuid::Uuid; /// Errors that are related to the network configuration. #[derive(Error, Debug)] pub enum NetworkStateError { - #[error("Unknown connection with ID: '{0}'")] + #[error("Unknown connection '{0}'")] UnknownConnection(String), #[error("Invalid connection UUID: '{0}'")] InvalidUuid(String), @@ -29,8 +29,6 @@ pub enum NetworkStateError { InvalidBondOptions, #[error("Not a controller connection: '{0}'")] NotControllerConnection(String), - #[error("Missing connection with UUID '{0}'")] - MissingConnection(Uuid), } impl From for zbus::fdo::Error { diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index e9e017f998..550bdb4ac2 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -123,7 +123,7 @@ impl NetworkState { /// If the connection is not a controller, returns an error. /// /// * `uuid`: controller UUID. - /// * `ports`: list of port names (using the interface name). + /// * `ports`: list of port names (using the connection ID or the interface name). pub fn set_ports( &mut self, controller: &Connection, diff --git a/rust/agama-dbus-server/src/network/system.rs b/rust/agama-dbus-server/src/network/system.rs index 3d354957f7..fa17121d8b 100644 --- a/rust/agama-dbus-server/src/network/system.rs +++ b/rust/agama-dbus-server/src/network/system.rs @@ -77,17 +77,8 @@ impl NetworkSystem { rx.send(conn.cloned()).unwrap(); } Action::GetController(uuid, rx) => { - let conn = self.state.get_connection_by_uuid(uuid).unwrap(); - let conn = conn.clone(); - - let controlled = self.state.get_controlled_by(uuid); - let controlled = controlled - .iter() - .filter_map(|c| c.interface().map(|c| c.to_string())) - .collect::>(); - - let data = (conn, controlled); - rx.send(Ok(data)).unwrap() + let result = self.get_controller_action(uuid); + rx.send(result).unwrap() } Action::SetPorts(uuid, ports, rx) => { let result = self.set_ports_action(uuid, ports); @@ -121,7 +112,27 @@ impl NetworkSystem { let conn = self .state .get_connection_by_uuid(uuid) - .ok_or(NetworkStateError::MissingConnection(uuid))?; + .ok_or(NetworkStateError::UnknownConnection(uuid.to_string()))?; self.state.set_ports(&conn.clone(), ports) } + + fn get_controller_action( + &mut self, + uuid: Uuid, + ) -> Result<(Connection, Vec), NetworkStateError> { + let conn = self + .state + .get_connection_by_uuid(uuid) + .ok_or(NetworkStateError::UnknownConnection(uuid.to_string()))?; + let conn = conn.clone(); + + let controlled = self + .state + .get_controlled_by(uuid) + .iter() + .map(|c| c.interface().unwrap_or(c.id()).to_string()) + .collect::>(); + + Ok((conn, controlled)) + } } From 447f77fee713cd4b9fe899c013ec9278627c3388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 13 Dec 2023 22:16:46 +0000 Subject: [PATCH 50/58] Code and documentation clean-up --- rust/agama-dbus-server/src/network/dbus/interfaces.rs | 8 ++++---- rust/agama-dbus-server/src/network/error.rs | 2 -- rust/agama-dbus-server/src/network/model.rs | 5 ----- rust/agama-dbus-server/src/network/model/builder.rs | 3 --- 4 files changed, 4 insertions(+), 14 deletions(-) diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index cbd3853803..ae4fd329c4 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -310,7 +310,7 @@ pub struct Bond { } impl Bond { - /// Creates a Match Settings interface object. + /// Creates a Bond interface object. /// /// * `actions`: sending-half of a channel to send actions. /// * `uuid`: connection UUID. @@ -363,7 +363,7 @@ impl Bond { self.update_connection(connection).await } - /// List of bond ports. + /// List of bonding options. #[dbus_interface(property)] pub async fn options(&self) -> String { let connection = self.get_bond().await; @@ -379,8 +379,8 @@ impl Bond { /// List of bond ports. /// - /// It uses the connection interfaces as port names. It means that you cannot - /// add a connection if it does not have a interface name. + /// For the port names, it uses the interface name (preferred) or, as a fallback, + /// the connection ID of the port. #[dbus_interface(property)] pub async fn ports(&self) -> zbus::fdo::Result> { let actions = self.actions.lock().await; diff --git a/rust/agama-dbus-server/src/network/error.rs b/rust/agama-dbus-server/src/network/error.rs index 45788bb9b5..e3873eaa27 100644 --- a/rust/agama-dbus-server/src/network/error.rs +++ b/rust/agama-dbus-server/src/network/error.rs @@ -16,8 +16,6 @@ pub enum NetworkStateError { #[error("Invalid wireless mode: '{0}'")] InvalidWirelessMode(String), #[error("Connection '{0}' already exists")] - UnknownParentKind(String), - #[error("Connection '{0}' already exists")] ConnectionExists(Uuid), #[error("Invalid security wireless protocol: '{0}'")] InvalidSecurityProtocol(String), diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 550bdb4ac2..4f61998310 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -401,11 +401,6 @@ impl Connection { self.base_mut().interface = Some(interface.to_string()) } - /// Ports controller name, e.g.: bond0, br0 - pub fn is_controlled(&self) -> bool { - self.base().controller.is_some() - } - /// Ports controller name, e.g.: bond0, br0 pub fn controller(&self) -> Option { self.base().controller diff --git a/rust/agama-dbus-server/src/network/model/builder.rs b/rust/agama-dbus-server/src/network/model/builder.rs index fb94e1c006..758dd52be6 100644 --- a/rust/agama-dbus-server/src/network/model/builder.rs +++ b/rust/agama-dbus-server/src/network/model/builder.rs @@ -1,9 +1,6 @@ use super::{Connection, DeviceType}; use uuid::Uuid; -// TODO: improve the implementation to allow calling methods only where it makes sense -// depending on the type of the connection. - #[derive(Debug, Default)] pub struct ConnectionBuilder { id: String, From f118c4222463b5beff7fc12079b978f2b99d1957 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 13 Dec 2023 22:42:33 +0000 Subject: [PATCH 51/58] Update the agama-cli.changes file --- rust/package/agama-cli.changes | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rust/package/agama-cli.changes b/rust/package/agama-cli.changes index 059d13682e..9159d6b15b 100644 --- a/rust/package/agama-cli.changes +++ b/rust/package/agama-cli.changes @@ -1,3 +1,8 @@ +------------------------------------------------------------------- +Wed Dec 13 22:41:34 UTC 2023 - Knut Anderssen + +- Add support for bonding connections (gh#openSUSE/agama#885). + ------------------------------------------------------------------- Tue Dec 5 11:18:41 UTC 2023 - Jorik Cronenberg From 1a9ffd57681a10d13b0a3f2907f9c860bc448e99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 14 Dec 2023 13:16:33 +0000 Subject: [PATCH 52/58] Documentation and grammar fixes Co-authored-by: Jorik Cronenberg --- rust/agama-dbus-server/src/network/action.rs | 2 +- rust/agama-dbus-server/src/network/model.rs | 12 ++++++------ rust/agama-lib/src/network/client.rs | 4 ++-- rust/agama-lib/src/network/proxies.rs | 6 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/rust/agama-dbus-server/src/network/action.rs b/rust/agama-dbus-server/src/network/action.rs index e22683452b..3ddf6687d9 100644 --- a/rust/agama-dbus-server/src/network/action.rs +++ b/rust/agama-dbus-server/src/network/action.rs @@ -23,7 +23,7 @@ pub enum Action { Uuid, Responder>, ), - /// Sets a controller ports. It uses the Uuid of the controller and the IDs or interface names + /// Sets a controller's ports. It uses the Uuid of the controller and the IDs or interface names /// of the ports. SetPorts(Uuid, Vec, Responder>), /// Update a connection (replacing the old one). diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index 8f0eec8fc8..da1933492b 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -52,7 +52,7 @@ impl NetworkState { /// Get connection by interface /// - /// * `id`: connection interface name + /// * `name`: connection interface name pub fn get_connection_by_interface(&self, name: &str) -> Option<&Connection> { let interface = Some(name); self.connections.iter().find(|c| c.interface() == interface) @@ -118,11 +118,11 @@ impl NetworkState { Ok(()) } - /// Sets a controller ports. + /// Sets a controller's ports. /// /// If the connection is not a controller, returns an error. /// - /// * `uuid`: controller UUID. + /// * `controller`: controller to set ports on. /// * `ports`: list of port names (using the connection ID or the interface name). pub fn set_ports( &mut self, @@ -401,14 +401,14 @@ impl Connection { self.base_mut().interface = Some(interface.to_string()) } - /// Ports controller name, e.g.: bond0, br0 + /// A port's controller name, e.g.: bond0, br0 pub fn controller(&self) -> Option { self.base().controller } - /// Sets the ports controller. + /// Sets the port's controller. /// - /// `controller`: Name of the controller (Bridge, Bond, Team), e.g.: bond0. + /// `controller`: Uuid of the controller (Bridge, Bond, Team), e.g.: bond0. pub fn set_controller(&mut self, controller: Uuid) { self.base_mut().controller = Some(controller) } diff --git a/rust/agama-lib/src/network/client.rs b/rust/agama-lib/src/network/client.rs index ed5b57ca25..77707a42c6 100644 --- a/rust/agama-lib/src/network/client.rs +++ b/rust/agama-lib/src/network/client.rs @@ -141,7 +141,7 @@ impl<'a> NetworkClient<'a> { /// Returns the [bond settings][BondSettings] for the given connection /// - /// * `path`: the connections path to get the wireless config from + /// * `path`: the connection's path to get the wireless config from async fn bond_from(&self, path: &str) -> Result { let bond_proxy = BondProxy::builder(&self.connection) .path(path)? @@ -309,7 +309,7 @@ impl<'a> NetworkClient<'a> { Ok(()) } - /// Updates the bond settings for network connection. + /// Updates the bond settings for a network connection. /// /// * `path`: connection D-Bus path. /// * `bond`: bond settings of the network connection. diff --git a/rust/agama-lib/src/network/proxies.rs b/rust/agama-lib/src/network/proxies.rs index 33c7ae733b..a3b1916abf 100644 --- a/rust/agama-lib/src/network/proxies.rs +++ b/rust/agama-lib/src/network/proxies.rs @@ -9,7 +9,7 @@ use zbus::dbus_proxy; default_path = "/org/opensuse/Agama1/Network/devices" )] trait Devices { - /// GetConnections method + /// GetDevices method fn get_devices(&self) -> zbus::Result>; } @@ -196,13 +196,13 @@ trait Bond { #[dbus_proxy(property)] fn set_mode(&self, value: &str) -> zbus::Result<()>; - /// ports property + /// Ports property #[dbus_proxy(property)] fn options(&self) -> zbus::Result; #[dbus_proxy(property)] fn set_options(&self, value: &str) -> zbus::Result<()>; - /// ports property + /// Ports property #[dbus_proxy(property)] fn ports(&self) -> zbus::Result>; #[dbus_proxy(property)] From cd47d9d98ee29e1fd2f12225d2bb4136a105a1ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 14 Dec 2023 15:39:06 +0000 Subject: [PATCH 53/58] Include the ethernet section in dummy and bond connections too --- rust/agama-dbus-server/src/network/model.rs | 1 + rust/agama-dbus-server/src/network/nm/dbus.rs | 12 +++++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/rust/agama-dbus-server/src/network/model.rs b/rust/agama-dbus-server/src/network/model.rs index da1933492b..7091e746d3 100644 --- a/rust/agama-dbus-server/src/network/model.rs +++ b/rust/agama-dbus-server/src/network/model.rs @@ -454,6 +454,7 @@ impl Connection { matches!(self, Connection::Loopback(_)) || matches!(self, Connection::Ethernet(_)) || matches!(self, Connection::Dummy(_)) + || matches!(self, Connection::Bond(_)) } pub fn mac_address(&self) -> String { diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index ae8d2e842a..a9ac77170f 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -45,12 +45,13 @@ pub fn connection_to_dbus<'a>( result.insert("ipv6", ip_config_to_ipv6_dbus(conn.ip_config())); result.insert("match", match_config_to_dbus(conn.match_config())); + if conn.is_ethernet() { + let ethernet_config = + HashMap::from([("assigned-mac-address", Value::new(conn.mac_address()))]); + result.insert(ETHERNET_KEY, ethernet_config); + } + match &conn { - Connection::Ethernet(_) | Connection::Loopback(_) => { - let ethernet_config = - HashMap::from([("assigned-mac-address", Value::new(conn.mac_address()))]); - result.insert(ETHERNET_KEY, ethernet_config); - } Connection::Wireless(wireless) => { connection_dbus.insert("type", WIRELESS_KEY.into()); let wireless_dbus = wireless_config_to_dbus(wireless); @@ -68,6 +69,7 @@ pub fn connection_to_dbus<'a>( Connection::Dummy(_) => { connection_dbus.insert("type", DUMMY_KEY.into()); } + _ => {} } result.insert("connection", connection_dbus); From f335cf0629220f4e5971e56f0bbe6df9f0387a04 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Thu, 14 Dec 2023 17:32:05 +0000 Subject: [PATCH 54/58] Fix client set interface when not present --- rust/agama-lib/src/network/client.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rust/agama-lib/src/network/client.rs b/rust/agama-lib/src/network/client.rs index 93b73fd405..79d9e4466b 100644 --- a/rust/agama-lib/src/network/client.rs +++ b/rust/agama-lib/src/network/client.rs @@ -247,8 +247,9 @@ impl<'a> NetworkClient<'a> { .build() .await?; - let interface = conn.interface.as_deref().unwrap_or(""); - proxy.set_interface(interface).await?; + if let Some(ref interface) = conn.interface { + proxy.set_interface(interface).await?; + } let mac_address = conn.mac_address.as_deref().unwrap_or(""); proxy.set_mac_address(mac_address).await?; From 5f1fa3d3cd44aa01408a1b1ab4a8fb86250ba13e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 14 Dec 2023 18:43:28 +0000 Subject: [PATCH 55/58] Add support to read the bond ports from NetworkManager --- .../src/network/nm/client.rs | 32 +++++++++++++++++-- rust/agama-dbus-server/src/network/nm/dbus.rs | 10 ++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/rust/agama-dbus-server/src/network/nm/client.rs b/rust/agama-dbus-server/src/network/nm/client.rs index 23698f3e15..a37486eafa 100644 --- a/rust/agama-dbus-server/src/network/nm/client.rs +++ b/rust/agama-dbus-server/src/network/nm/client.rs @@ -1,6 +1,9 @@ //! NetworkManager client. +use std::collections::HashMap; + use super::dbus::{ - cleanup_dbus_connection, connection_from_dbus, connection_to_dbus, merge_dbus_connections, + cleanup_dbus_connection, connection_from_dbus, connection_to_dbus, controller_from_dbus, + merge_dbus_connections, }; use super::model::NmDeviceType; use super::proxies::{ConnectionProxy, DeviceProxy, NetworkManagerProxy, SettingsProxy}; @@ -70,6 +73,9 @@ impl<'a> NetworkManagerClient<'a> { /// Returns the list of network connections. pub async fn connections(&self) -> Result, ServiceError> { + let mut controlled_by: HashMap = HashMap::new(); + let mut uuids_map: HashMap = HashMap::new(); + let proxy = SettingsProxy::new(&self.connection).await?; let paths = proxy.list_connections().await?; let mut connections: Vec = Vec::with_capacity(paths.len()); @@ -79,12 +85,34 @@ impl<'a> NetworkManagerClient<'a> { .build() .await?; let settings = proxy.get_settings().await?; - // TODO: log an error if a connection is not found + if let Some(connection) = connection_from_dbus(settings.clone()) { + if let Some(controller) = controller_from_dbus(&settings) { + controlled_by.insert(connection.uuid(), controller.to_string()); + } + if let Some(iname) = connection.interface() { + uuids_map.insert(iname.to_string(), connection.uuid()); + } connections.push(connection); } } + for conn in connections.iter_mut() { + let Some(interface_name) = controlled_by.get(&conn.uuid()) else { + continue; + }; + + if let Some(uuid) = uuids_map.get(interface_name) { + conn.set_controller(*uuid); + } else { + log::warn!( + "Could not found a connection for the interface '{}' (required by connection '{}')", + interface_name, + conn.id() + ); + } + } + Ok(connections) } diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index a9ac77170f..ed74c579de 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -163,6 +163,16 @@ pub fn cleanup_dbus_connection(conn: &mut NestedHash) { } } +/// Ancillary function to get the controller for a given interface. +pub fn controller_from_dbus(conn: &OwnedNestedHash) -> Option { + let Some(connection) = conn.get("connection") else { + return None; + }; + + let master: &str = connection.get("master")?.downcast_ref()?; + Some(master.to_string()) +} + fn ip_config_to_ipv4_dbus(ip_config: &IpConfig) -> HashMap<&str, zvariant::Value> { let addresses: Vec> = ip_config .addresses From c73a8ef5d38b8881d0b06fbaf6cdc33886e1a951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 15 Dec 2023 10:51:16 +0000 Subject: [PATCH 56/58] Remove the Bond D-Bus interface when cleaning the type --- rust/agama-dbus-server/src/network/dbus/tree.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/agama-dbus-server/src/network/dbus/tree.rs b/rust/agama-dbus-server/src/network/dbus/tree.rs index aaf5dc5097..f2e9f0263e 100644 --- a/rust/agama-dbus-server/src/network/dbus/tree.rs +++ b/rust/agama-dbus-server/src/network/dbus/tree.rs @@ -192,6 +192,7 @@ impl Tree { /// * `path`: connection D-Bus path. async fn remove_connection_on(&self, path: &str) -> Result<(), ServiceError> { let object_server = self.connection.object_server(); + _ = object_server.remove::(path).await; _ = object_server.remove::(path).await; object_server.remove::(path).await?; object_server From d7fdfd3d6f667e73be2e82eb9f79bd00aad596f6 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Fri, 15 Dec 2023 11:06:43 +0000 Subject: [PATCH 57/58] Fixes for bonding support --- rust/agama-dbus-server/src/network/nm/dbus.rs | 11 +++++++++++ rust/agama-lib/src/network/settings.rs | 4 ++-- rust/agama-lib/src/network/types.rs | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/rust/agama-dbus-server/src/network/nm/dbus.rs b/rust/agama-dbus-server/src/network/nm/dbus.rs index ed74c579de..de16523628 100644 --- a/rust/agama-dbus-server/src/network/nm/dbus.rs +++ b/rust/agama-dbus-server/src/network/nm/dbus.rs @@ -39,6 +39,9 @@ pub fn connection_to_dbus<'a>( connection_dbus.insert("slave-type", "bond".into()); // TODO: only 'bond' is supported let master = controller.interface().unwrap_or(controller.id()); connection_dbus.insert("master", master.into()); + } else { + connection_dbus.insert("slave-type", "".into()); // TODO: only 'bond' is supported + connection_dbus.insert("master", "".into()); } result.insert("ipv4", ip_config_to_ipv4_dbus(conn.ip_config())); @@ -150,6 +153,14 @@ pub fn cleanup_dbus_connection(conn: &mut NestedHash) { if connection.get("interface-name").is_some_and(is_empty_value) { connection.remove("interface-name"); } + + if connection.get("master").is_some_and(is_empty_value) { + connection.remove("master"); + } + + if connection.get("slave-type").is_some_and(is_empty_value) { + connection.remove("slave-type"); + } } if let Some(ipv4) = conn.get_mut("ipv4") { diff --git a/rust/agama-lib/src/network/settings.rs b/rust/agama-lib/src/network/settings.rs index 782fea5777..db0321f74c 100644 --- a/rust/agama-lib/src/network/settings.rs +++ b/rust/agama-lib/src/network/settings.rs @@ -60,7 +60,7 @@ pub struct BondSettings { impl Default for BondSettings { fn default() -> Self { Self { - mode: "round-robin".to_string(), + mode: "balance-rr".to_string(), options: None, ports: vec![], } @@ -170,7 +170,7 @@ mod tests { fn test_bonding_defaults() { let bond = BondSettings::default(); - assert_eq!(bond.mode, "round-robin".to_string()); + assert_eq!(bond.mode, "balance-rr".to_string()); assert_eq!(bond.ports.len(), 0); assert_eq!(bond.options, None); } diff --git a/rust/agama-lib/src/network/types.rs b/rust/agama-lib/src/network/types.rs index 75b804fea6..5a14fe238e 100644 --- a/rust/agama-lib/src/network/types.rs +++ b/rust/agama-lib/src/network/types.rs @@ -91,7 +91,7 @@ impl TryFrom<&str> for BondMode { fn try_from(value: &str) -> Result { match value { - "round-robin" => Ok(BondMode::RoundRobin), + "balance-rr" => Ok(BondMode::RoundRobin), "active-backup" => Ok(BondMode::ActiveBackup), "balance-xor" => Ok(BondMode::BalanceXOR), "broadcast" => Ok(BondMode::Broadcast), From 9edb9402b92f3b9c36783c2e1ff2408745314062 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Fri, 15 Dec 2023 11:22:00 +0000 Subject: [PATCH 58/58] Publish the connection uuid and controller --- .../src/network/dbus/interfaces.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index ae4fd329c4..0467efc8a6 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -232,6 +232,24 @@ impl Connection { self.get_connection().await.id().to_string() } + /// Connection UUID. + /// + /// Unique identifier of the network connection. It may or not be the same that the used by the + /// backend. + #[dbus_interface(property)] + pub async fn uuid(&self) -> String { + self.get_connection().await.uuid().to_string() + } + + #[dbus_interface(property)] + pub async fn controller(&self) -> String { + let connection = self.get_connection().await; + match connection.controller() { + Some(uuid) => uuid.to_string(), + None => "".to_string(), + } + } + #[dbus_interface(property)] pub async fn interface(&self) -> String { self.get_connection()