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 }