From 435f479984042c7547d58d254f53cacf76b8337f Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 19 Oct 2021 16:38:52 +0200 Subject: [PATCH] D-Bus NetworkManager improvements (#3243) * Introduce enum for network connectivity * Differentiate between Bus Name and Interface consts The Bus names and interfaces look quite similar in D-Bus: Both use dots to separate words. Usually all interfaces available below a certan Bus name start with the Bus name. Quite often the Bus name itself is also available as an interface. However, those are different things. To avoid confusion, add the type of const to the const name. * Remove unused const * Disconnect D-Bus when not used Make sure Python disconnects from D-Bus when objects get destroyed. This avoids exhausting D-Bus connection limit which causes the following error message: [system] The maximum number of active connections for UID 0 has been reached (max_connections_per_user=256) * Filter signals by object as well Make sure we only listen to signals on that particular object. Also support filtering messages via message filter callback. * Explicitly wait until Connection is activated Wait for activated or raise an error. This avoids too early/errornous updates when state of the connection changes to "activating" or similar intermediate signal states. Fixes: #2639 * Fix VLAN configuration * Add link to D-Bus object documentation * Fix network settings update test * Make MessageBus object optional --- supervisor/const.py | 1 - supervisor/dbus/agent/__init__.py | 5 +- supervisor/dbus/agent/apparmor.py | 4 +- supervisor/dbus/agent/datadisk.py | 4 +- supervisor/dbus/const.py | 54 ++++++++++++------- supervisor/dbus/hostname.py | 3 +- supervisor/dbus/network/__init__.py | 35 ++++++++---- supervisor/dbus/network/accesspoint.py | 9 ++-- supervisor/dbus/network/connection.py | 20 ++++--- supervisor/dbus/network/dns.py | 9 ++-- supervisor/dbus/network/interface.py | 11 ++-- supervisor/dbus/network/setting/__init__.py | 7 ++- supervisor/dbus/network/setting/generate.py | 10 ++-- supervisor/dbus/network/settings.py | 5 +- supervisor/dbus/network/wireless.py | 9 ++-- supervisor/dbus/rauc.py | 8 +-- supervisor/dbus/systemd.py | 4 +- supervisor/dbus/timedate.py | 3 +- supervisor/host/network.py | 49 ++++++++++++----- supervisor/utils/dbus.py | 47 ++++++++++++---- tests/conftest.py | 6 ++- ...tworkManager-AddAndActivateConnection.json | 2 +- 22 files changed, 210 insertions(+), 95 deletions(-) diff --git a/supervisor/const.py b/supervisor/const.py index 7704b3d4e..43f0be7f5 100644 --- a/supervisor/const.py +++ b/supervisor/const.py @@ -280,7 +280,6 @@ ATTR_SIZE = "size" ATTR_SLUG = "slug" ATTR_SOURCE = "source" ATTR_SQUASH = "squash" -ATTR_SSD = "ssid" ATTR_SSID = "ssid" ATTR_SSL = "ssl" ATTR_STAGE = "stage" diff --git a/supervisor/dbus/agent/__init__.py b/supervisor/dbus/agent/__init__.py index 81558922f..fc5c4e653 100644 --- a/supervisor/dbus/agent/__init__.py +++ b/supervisor/dbus/agent/__init__.py @@ -10,6 +10,7 @@ from ...utils.dbus import DBus from ..const import ( DBUS_ATTR_DIAGNOSTICS, DBUS_ATTR_VERSION, + DBUS_IFACE_HAOS, DBUS_NAME_HAOS, DBUS_OBJECT_HAOS, ) @@ -74,7 +75,7 @@ class OSAgent(DBusInterface): def diagnostics(self, value: bool) -> None: """Enable or disable OS-Agent diagnostics.""" asyncio.create_task( - self.dbus.set_property(DBUS_NAME_HAOS, DBUS_ATTR_DIAGNOSTICS, value) + self.dbus.set_property(DBUS_IFACE_HAOS, DBUS_ATTR_DIAGNOSTICS, value) ) async def connect(self) -> None: @@ -95,6 +96,6 @@ class OSAgent(DBusInterface): @dbus_connected async def update(self): """Update Properties.""" - self.properties = await self.dbus.get_properties(DBUS_NAME_HAOS) + self.properties = await self.dbus.get_properties(DBUS_IFACE_HAOS) await self.apparmor.update() await self.datadisk.update() diff --git a/supervisor/dbus/agent/apparmor.py b/supervisor/dbus/agent/apparmor.py index 90f862020..65a861dc5 100644 --- a/supervisor/dbus/agent/apparmor.py +++ b/supervisor/dbus/agent/apparmor.py @@ -7,8 +7,8 @@ from awesomeversion import AwesomeVersion from ...utils.dbus import DBus from ..const import ( DBUS_ATTR_PARSER_VERSION, + DBUS_IFACE_HAOS_APPARMOR, DBUS_NAME_HAOS, - DBUS_NAME_HAOS_APPARMOR, DBUS_OBJECT_HAOS_APPARMOR, ) from ..interface import DBusInterface, dbus_property @@ -35,7 +35,7 @@ class AppArmor(DBusInterface): @dbus_connected async def update(self): """Update Properties.""" - self.properties = await self.dbus.get_properties(DBUS_NAME_HAOS_APPARMOR) + self.properties = await self.dbus.get_properties(DBUS_IFACE_HAOS_APPARMOR) @dbus_connected async def load_profile(self, profile: Path, cache: Path) -> None: diff --git a/supervisor/dbus/agent/datadisk.py b/supervisor/dbus/agent/datadisk.py index 5991920b1..0e1ae646a 100644 --- a/supervisor/dbus/agent/datadisk.py +++ b/supervisor/dbus/agent/datadisk.py @@ -5,8 +5,8 @@ from typing import Any from ...utils.dbus import DBus from ..const import ( DBUS_ATTR_CURRENT_DEVICE, + DBUS_IFACE_HAOS_DATADISK, DBUS_NAME_HAOS, - DBUS_NAME_HAOS_DATADISK, DBUS_OBJECT_HAOS_DATADISK, ) from ..interface import DBusInterface, dbus_property @@ -33,7 +33,7 @@ class DataDisk(DBusInterface): @dbus_connected async def update(self): """Update Properties.""" - self.properties = await self.dbus.get_properties(DBUS_NAME_HAOS_DATADISK) + self.properties = await self.dbus.get_properties(DBUS_IFACE_HAOS_DATADISK) @dbus_connected async def change_device(self, device: Path) -> None: diff --git a/supervisor/dbus/const.py b/supervisor/dbus/const.py index 51ea28eb9..413e929fe 100644 --- a/supervisor/dbus/const.py +++ b/supervisor/dbus/const.py @@ -1,32 +1,37 @@ """Constants for DBUS.""" from enum import Enum -DBUS_NAME_ACCESSPOINT = "org.freedesktop.NetworkManager.AccessPoint" -DBUS_NAME_CONNECTION_ACTIVE = "org.freedesktop.NetworkManager.Connection.Active" -DBUS_NAME_DEVICE = "org.freedesktop.NetworkManager.Device" -DBUS_NAME_DEVICE_WIRELESS = "org.freedesktop.NetworkManager.Device.Wireless" -DBUS_NAME_DNS = "org.freedesktop.NetworkManager.DnsManager" DBUS_NAME_HAOS = "io.hass.os" -DBUS_NAME_HAOS_APPARMOR = "io.hass.os.AppArmor" -DBUS_NAME_HAOS_CGROUP = "io.hass.os.CGroup" -DBUS_NAME_HAOS_DATADISK = "io.hass.os.DataDisk" -DBUS_NAME_HAOS_SYSTEM = "io.hass.os.System" DBUS_NAME_HOSTNAME = "org.freedesktop.hostname1" -DBUS_NAME_IP4CONFIG = "org.freedesktop.NetworkManager.IP4Config" -DBUS_NAME_IP6CONFIG = "org.freedesktop.NetworkManager.IP6Config" DBUS_NAME_LOGIND = "org.freedesktop.login1" DBUS_NAME_NM = "org.freedesktop.NetworkManager" -DBUS_NAME_NM_CONNECTION_ACTIVE_CHANGED = ( - "org.freedesktop.NetworkManager.Connection.Active.StateChanged" -) DBUS_NAME_RAUC = "de.pengutronix.rauc" -DBUS_NAME_RAUC_INSTALLER = "de.pengutronix.rauc.Installer" -DBUS_NAME_RAUC_INSTALLER_COMPLETED = "de.pengutronix.rauc.Installer.Completed" -DBUS_NAME_SETTINGS_CONNECTION = "org.freedesktop.NetworkManager.Settings.Connection" DBUS_NAME_SYSTEMD = "org.freedesktop.systemd1" -DBUS_NAME_SYSTEMD_MANAGER = "org.freedesktop.systemd1.Manager" DBUS_NAME_TIMEDATE = "org.freedesktop.timedate1" +DBUS_IFACE_ACCESSPOINT = "org.freedesktop.NetworkManager.AccessPoint" +DBUS_IFACE_CONNECTION_ACTIVE = "org.freedesktop.NetworkManager.Connection.Active" +DBUS_IFACE_DEVICE = "org.freedesktop.NetworkManager.Device" +DBUS_IFACE_DEVICE_WIRELESS = "org.freedesktop.NetworkManager.Device.Wireless" +DBUS_IFACE_DNS = "org.freedesktop.NetworkManager.DnsManager" +DBUS_IFACE_HAOS = "io.hass.os" +DBUS_IFACE_HAOS_APPARMOR = "io.hass.os.AppArmor" +DBUS_IFACE_HAOS_CGROUP = "io.hass.os.CGroup" +DBUS_IFACE_HAOS_DATADISK = "io.hass.os.DataDisk" +DBUS_IFACE_HAOS_SYSTEM = "io.hass.os.System" +DBUS_IFACE_HOSTNAME = "org.freedesktop.hostname1" +DBUS_IFACE_IP4CONFIG = "org.freedesktop.NetworkManager.IP4Config" +DBUS_IFACE_IP6CONFIG = "org.freedesktop.NetworkManager.IP6Config" +DBUS_IFACE_NM = "org.freedesktop.NetworkManager" +DBUS_IFACE_RAUC_INSTALLER = "de.pengutronix.rauc.Installer" +DBUS_IFACE_SETTINGS_CONNECTION = "org.freedesktop.NetworkManager.Settings.Connection" +DBUS_IFACE_SYSTEMD_MANAGER = "org.freedesktop.systemd1.Manager" +DBUS_IFACE_TIMEDATE = "org.freedesktop.timedate1" + +DBUS_SIGNAL_NM_CONNECTION_ACTIVE_CHANGED = ( + "org.freedesktop.NetworkManager.Connection.Active.StateChanged" +) +DBUS_SIGNAL_RAUC_INSTALLER_COMPLETED = "de.pengutronix.rauc.Installer.Completed" DBUS_OBJECT_BASE = "/" DBUS_OBJECT_DNS = "/org/freedesktop/NetworkManager/DnsManager" @@ -135,6 +140,19 @@ class ConnectionStateType(int, Enum): DEACTIVATED = 4 +class ConnectivityState(int, Enum): + """Network connectvity. + + https://developer.gnome.org/NetworkManager/unstable/nm-dbus-types.html#NMConnectivityState + """ + + CONNECTIVITY_UNKNOWN = 0 + CONNECTIVITY_NONE = 1 + CONNECTIVITY_PORTAL = 2 + CONNECTIVITY_LIMITED = 3 + CONNECTIVITY_FULL = 4 + + class DeviceType(int, Enum): """Device types. diff --git a/supervisor/dbus/hostname.py b/supervisor/dbus/hostname.py index 0952db33c..c978ebba1 100644 --- a/supervisor/dbus/hostname.py +++ b/supervisor/dbus/hostname.py @@ -11,6 +11,7 @@ from .const import ( DBUS_ATTR_OPERATING_SYSTEM_PRETTY_NAME, DBUS_ATTR_STATIC_HOSTNAME, DBUS_ATTR_STATIC_OPERATING_SYSTEM_CPE_NAME, + DBUS_IFACE_HOSTNAME, DBUS_NAME_HOSTNAME, DBUS_OBJECT_HOSTNAME, ) @@ -87,4 +88,4 @@ class Hostname(DBusInterface): @dbus_connected async def update(self): """Update Properties.""" - self.properties = await self.dbus.get_properties(DBUS_NAME_HOSTNAME) + self.properties = await self.dbus.get_properties(DBUS_IFACE_HOSTNAME) diff --git a/supervisor/dbus/network/__init__.py b/supervisor/dbus/network/__init__.py index dbfc3367c..f4f44f0a5 100644 --- a/supervisor/dbus/network/__init__.py +++ b/supervisor/dbus/network/__init__.py @@ -1,10 +1,14 @@ """Network Manager implementation for DBUS.""" +import asyncio import logging from typing import Any, Awaitable from awesomeversion import AwesomeVersion, AwesomeVersionException import sentry_sdk +from supervisor.dbus.network.connection import NetworkConnection +from supervisor.dbus.network.setting import NetworkSetting + from ...exceptions import ( DBusError, DBusFatalError, @@ -17,6 +21,7 @@ from ..const import ( DBUS_ATTR_DEVICES, DBUS_ATTR_PRIMARY_CONNECTION, DBUS_ATTR_VERSION, + DBUS_IFACE_NM, DBUS_NAME_NM, DBUS_OBJECT_BASE, DBUS_OBJECT_NM, @@ -34,7 +39,10 @@ MINIMAL_VERSION = AwesomeVersion("1.14.6") class NetworkManager(DBusInterface): - """Handle D-Bus interface for Network Manager.""" + """Handle D-Bus interface for Network Manager. + + https://developer.gnome.org/NetworkManager/stable/gdbus-org.freedesktop.NetworkManager.html + """ name = DBUS_NAME_NM @@ -72,23 +80,32 @@ class NetworkManager(DBusInterface): return AwesomeVersion(self.properties[DBUS_ATTR_VERSION]) @dbus_connected - def activate_connection( + async def activate_connection( self, connection_object: str, device_object: str - ) -> Awaitable[Any]: + ) -> NetworkConnection: """Activate a connction on a device.""" - return self.dbus.ActivateConnection( + result = await self.dbus.ActivateConnection( ("o", connection_object), ("o", device_object), ("o", DBUS_OBJECT_BASE) ) + obj_active_con = result[0] + active_con = NetworkConnection(obj_active_con) + await active_con.connect() + return active_con @dbus_connected - def add_and_activate_connection( + async def add_and_activate_connection( self, settings: Any, device_object: str - ) -> Awaitable[Any]: + ) -> tuple[NetworkSetting, NetworkConnection]: """Activate a connction on a device.""" - return self.dbus.AddAndActivateConnection( + obj_con_setting, obj_active_con = await self.dbus.AddAndActivateConnection( ("a{sa{sv}}", settings), ("o", device_object), ("o", DBUS_OBJECT_BASE) ) + con_setting = NetworkSetting(obj_con_setting) + active_con = NetworkConnection(obj_active_con) + await asyncio.gather(con_setting.connect(), active_con.connect()) + return con_setting, active_con + @dbus_connected async def check_connectivity(self) -> Awaitable[Any]: """Check the connectivity of the host.""" @@ -118,7 +135,7 @@ class NetworkManager(DBusInterface): async def _validate_version(self) -> None: """Validate Version of NetworkManager.""" - self.properties = await self.dbus.get_properties(DBUS_NAME_NM) + self.properties = await self.dbus.get_properties(DBUS_IFACE_NM) try: if self.version >= MINIMAL_VERSION: @@ -134,7 +151,7 @@ class NetworkManager(DBusInterface): @dbus_connected async def update(self): """Update Properties.""" - self.properties = await self.dbus.get_properties(DBUS_NAME_NM) + self.properties = await self.dbus.get_properties(DBUS_IFACE_NM) await self.dns.update() diff --git a/supervisor/dbus/network/accesspoint.py b/supervisor/dbus/network/accesspoint.py index 80fca7e91..d3a843ad7 100644 --- a/supervisor/dbus/network/accesspoint.py +++ b/supervisor/dbus/network/accesspoint.py @@ -7,14 +7,17 @@ from ..const import ( DBUS_ATTR_MODE, DBUS_ATTR_SSID, DBUS_ATTR_STRENGTH, - DBUS_NAME_ACCESSPOINT, + DBUS_IFACE_ACCESSPOINT, DBUS_NAME_NM, ) from ..interface import DBusInterfaceProxy class NetworkWirelessAP(DBusInterfaceProxy): - """NetworkWireless AP object for Network Manager.""" + """NetworkWireless AP object for Network Manager. + + https://developer.gnome.org/NetworkManager/stable/gdbus-org.freedesktop.NetworkManager.AccessPoint.html + """ def __init__(self, object_path: str) -> None: """Initialize NetworkWireless AP object.""" @@ -49,4 +52,4 @@ class NetworkWirelessAP(DBusInterfaceProxy): async def connect(self) -> None: """Get connection information.""" self.dbus = await DBus.connect(DBUS_NAME_NM, self.object_path) - self.properties = await self.dbus.get_properties(DBUS_NAME_ACCESSPOINT) + self.properties = await self.dbus.get_properties(DBUS_IFACE_ACCESSPOINT) diff --git a/supervisor/dbus/network/connection.py b/supervisor/dbus/network/connection.py index 2a273cb31..a9fce0584 100644 --- a/supervisor/dbus/network/connection.py +++ b/supervisor/dbus/network/connection.py @@ -16,18 +16,22 @@ from ..const import ( DBUS_ATTR_STATE, DBUS_ATTR_TYPE, DBUS_ATTR_UUID, - DBUS_NAME_CONNECTION_ACTIVE, - DBUS_NAME_IP4CONFIG, - DBUS_NAME_IP6CONFIG, + DBUS_IFACE_CONNECTION_ACTIVE, + DBUS_IFACE_IP4CONFIG, + DBUS_IFACE_IP6CONFIG, DBUS_NAME_NM, DBUS_OBJECT_BASE, + ConnectionStateType, ) from ..interface import DBusInterfaceProxy from .configuration import IpConfiguration class NetworkConnection(DBusInterfaceProxy): - """NetworkConnection object for Network Manager.""" + """Active network connection object for Network Manager. + + https://developer.gnome.org/NetworkManager/stable/gdbus-org.freedesktop.NetworkManager.Connection.Active.html + """ def __init__(self, object_path: str) -> None: """Initialize NetworkConnection object.""" @@ -53,7 +57,7 @@ class NetworkConnection(DBusInterfaceProxy): return self.properties[DBUS_ATTR_UUID] @property - def state(self) -> int: + def state(self) -> ConnectionStateType: """Return the state of the connection.""" return self.properties[DBUS_ATTR_STATE] @@ -75,12 +79,12 @@ class NetworkConnection(DBusInterfaceProxy): async def connect(self) -> None: """Get connection information.""" self.dbus = await DBus.connect(DBUS_NAME_NM, self.object_path) - self.properties = await self.dbus.get_properties(DBUS_NAME_CONNECTION_ACTIVE) + self.properties = await self.dbus.get_properties(DBUS_IFACE_CONNECTION_ACTIVE) # IPv4 if self.properties[DBUS_ATTR_IP4CONFIG] != DBUS_OBJECT_BASE: ip4 = await DBus.connect(DBUS_NAME_NM, self.properties[DBUS_ATTR_IP4CONFIG]) - ip4_data = await ip4.get_properties(DBUS_NAME_IP4CONFIG) + ip4_data = await ip4.get_properties(DBUS_IFACE_IP4CONFIG) self._ipv4 = IpConfiguration( ip_address(ip4_data[DBUS_ATTR_GATEWAY]) @@ -99,7 +103,7 @@ class NetworkConnection(DBusInterfaceProxy): # IPv6 if self.properties[DBUS_ATTR_IP6CONFIG] != DBUS_OBJECT_BASE: ip6 = await DBus.connect(DBUS_NAME_NM, self.properties[DBUS_ATTR_IP6CONFIG]) - ip6_data = await ip6.get_properties(DBUS_NAME_IP6CONFIG) + ip6_data = await ip6.get_properties(DBUS_IFACE_IP6CONFIG) self._ipv6 = IpConfiguration( ip_address(ip6_data[DBUS_ATTR_GATEWAY]) diff --git a/supervisor/dbus/network/dns.py b/supervisor/dbus/network/dns.py index 843b2992e..46d4aad44 100644 --- a/supervisor/dbus/network/dns.py +++ b/supervisor/dbus/network/dns.py @@ -16,7 +16,7 @@ from ..const import ( DBUS_ATTR_CONFIGURATION, DBUS_ATTR_MODE, DBUS_ATTR_RCMANAGER, - DBUS_NAME_DNS, + DBUS_IFACE_DNS, DBUS_NAME_NM, DBUS_OBJECT_DNS, ) @@ -28,7 +28,10 @@ _LOGGER: logging.Logger = logging.getLogger(__name__) class NetworkManagerDNS(DBusInterface): - """Handle D-Bus interface for NMI DnsManager.""" + """Handle D-Bus interface for NM DnsManager. + + https://developer.gnome.org/NetworkManager/stable/gdbus-org.freedesktop.NetworkManager.DnsManager.html + """ def __init__(self) -> None: """Initialize Properties.""" @@ -65,7 +68,7 @@ class NetworkManagerDNS(DBusInterface): @dbus_connected async def update(self): """Update Properties.""" - data = await self.dbus.get_properties(DBUS_NAME_DNS) + data = await self.dbus.get_properties(DBUS_IFACE_DNS) if not data: _LOGGER.warning("Can't get properties for DnsManager") return diff --git a/supervisor/dbus/network/interface.py b/supervisor/dbus/network/interface.py index 2aaff5411..e08d419d0 100644 --- a/supervisor/dbus/network/interface.py +++ b/supervisor/dbus/network/interface.py @@ -8,7 +8,7 @@ from ..const import ( DBUS_ATTR_DEVICE_TYPE, DBUS_ATTR_DRIVER, DBUS_ATTR_MANAGED, - DBUS_NAME_DEVICE, + DBUS_IFACE_DEVICE, DBUS_NAME_NM, DBUS_OBJECT_BASE, DeviceType, @@ -20,7 +20,10 @@ from .wireless import NetworkWireless class NetworkInterface(DBusInterfaceProxy): - """NetworkInterface object for Network Manager.""" + """NetworkInterface object represents Network Manager Device objects. + + https://developer.gnome.org/NetworkManager/stable/gdbus-org.freedesktop.NetworkManager.Device.html + """ def __init__(self, nm_dbus: DBus, object_path: str) -> None: """Initialize NetworkConnection object.""" @@ -72,13 +75,13 @@ class NetworkInterface(DBusInterfaceProxy): async def connect(self) -> None: """Get device information.""" self.dbus = await DBus.connect(DBUS_NAME_NM, self.object_path) - self.properties = await self.dbus.get_properties(DBUS_NAME_DEVICE) + self.properties = await self.dbus.get_properties(DBUS_IFACE_DEVICE) # Abort if device is not managed if not self.managed: return - # If connection exists + # If active connection exists if self.properties[DBUS_ATTR_ACTIVE_CONNECTION] != DBUS_OBJECT_BASE: self._connection = NetworkConnection( self.properties[DBUS_ATTR_ACTIVE_CONNECTION] diff --git a/supervisor/dbus/network/setting/__init__.py b/supervisor/dbus/network/setting/__init__.py index 0457c6c04..87f795737 100644 --- a/supervisor/dbus/network/setting/__init__.py +++ b/supervisor/dbus/network/setting/__init__.py @@ -38,7 +38,10 @@ _LOGGER: logging.Logger = logging.getLogger(__name__) class NetworkSetting(DBusInterfaceProxy): - """NetworkConnection object for Network Manager.""" + """Network connection setting object for Network Manager. + + https://developer.gnome.org/NetworkManager/stable/gdbus-org.freedesktop.NetworkManager.Settings.Connection.html + """ def __init__(self, object_path: str) -> None: """Initialize NetworkConnection object.""" @@ -108,6 +111,8 @@ class NetworkSetting(DBusInterfaceProxy): self.dbus = await DBus.connect(DBUS_NAME_NM, self.object_path) data = (await self.get_settings())[0] + # Get configuration settings we care about + # See: https://developer-old.gnome.org/NetworkManager/stable/ch01.html if CONF_ATTR_CONNECTION in data: self._connection = ConnectionProperties( data[CONF_ATTR_CONNECTION].get(ATTR_ID), diff --git a/supervisor/dbus/network/setting/generate.py b/supervisor/dbus/network/setting/generate.py index 2e62fd372..9dc9f771a 100644 --- a/supervisor/dbus/network/setting/generate.py +++ b/supervisor/dbus/network/setting/generate.py @@ -47,18 +47,20 @@ def get_connection_from_interface( connection = { "id": Variant("s", name), - "interface-name": Variant("s", interface.name), "type": Variant("s", iftype), "uuid": Variant("s", uuid), "llmnr": Variant("i", 2), "mdns": Variant("i", 2), } + if interface.type != InterfaceType.VLAN: + connection["interface-name"] = Variant("s", interface.name) + conn = {} conn[CONF_ATTR_CONNECTION] = connection ipv4 = {} - if interface.ipv4.method == InterfaceMethod.AUTO: + if not interface.ipv4 or interface.ipv4.method == InterfaceMethod.AUTO: ipv4["method"] = Variant("s", "auto") elif interface.ipv4.method == InterfaceMethod.DISABLED: ipv4["method"] = Variant("s", "disabled") @@ -87,7 +89,7 @@ def get_connection_from_interface( conn[CONF_ATTR_IPV4] = ipv4 ipv6 = {} - if interface.ipv6.method == InterfaceMethod.AUTO: + if not interface.ipv6 or interface.ipv6.method == InterfaceMethod.AUTO: ipv6["method"] = Variant("s", "auto") elif interface.ipv6.method == InterfaceMethod.DISABLED: ipv6["method"] = Variant("s", "disabled") @@ -109,7 +111,7 @@ def get_connection_from_interface( ) ipv6["address-data"] = Variant("(a{sv})", adressdata) - ipv4["gateway"] = Variant("s", str(interface.ipv6.gateway)) + ipv6["gateway"] = Variant("s", str(interface.ipv6.gateway)) conn[CONF_ATTR_IPV6] = ipv6 diff --git a/supervisor/dbus/network/settings.py b/supervisor/dbus/network/settings.py index 758f32a0b..dd94cc3cb 100644 --- a/supervisor/dbus/network/settings.py +++ b/supervisor/dbus/network/settings.py @@ -12,7 +12,10 @@ _LOGGER: logging.Logger = logging.getLogger(__name__) class NetworkManagerSettings(DBusInterface): - """Handle D-Bus interface for Network Manager.""" + """Handle D-Bus interface for Network Manager Connection Settings Profile Manager. + + https://developer.gnome.org/NetworkManager/stable/gdbus-org.freedesktop.NetworkManager.Settings.html + """ async def connect(self) -> None: """Connect to system's D-Bus.""" diff --git a/supervisor/dbus/network/wireless.py b/supervisor/dbus/network/wireless.py index cf60ed93b..b15a14791 100644 --- a/supervisor/dbus/network/wireless.py +++ b/supervisor/dbus/network/wireless.py @@ -4,7 +4,7 @@ from typing import Any, Awaitable, Optional from ...utils.dbus import DBus from ..const import ( DBUS_ATTR_ACTIVE_ACCESSPOINT, - DBUS_NAME_DEVICE_WIRELESS, + DBUS_IFACE_DEVICE_WIRELESS, DBUS_NAME_NM, DBUS_OBJECT_BASE, ) @@ -14,7 +14,10 @@ from .accesspoint import NetworkWirelessAP class NetworkWireless(DBusInterfaceProxy): - """NetworkWireless object for Network Manager.""" + """Wireless object for Network Manager. + + https://developer.gnome.org/NetworkManager/stable/gdbus-org.freedesktop.NetworkManager.Device.Wireless.html + """ def __init__(self, object_path: str) -> None: """Initialize NetworkConnection object.""" @@ -41,7 +44,7 @@ class NetworkWireless(DBusInterfaceProxy): async def connect(self) -> None: """Get connection information.""" self.dbus = await DBus.connect(DBUS_NAME_NM, self.object_path) - self.properties = await self.dbus.get_properties(DBUS_NAME_DEVICE_WIRELESS) + self.properties = await self.dbus.get_properties(DBUS_IFACE_DEVICE_WIRELESS) # Get details from current active if self.properties[DBUS_ATTR_ACTIVE_ACCESSPOINT] != DBUS_OBJECT_BASE: diff --git a/supervisor/dbus/rauc.py b/supervisor/dbus/rauc.py index d29beed39..6de419709 100644 --- a/supervisor/dbus/rauc.py +++ b/supervisor/dbus/rauc.py @@ -10,10 +10,10 @@ from .const import ( DBUS_ATTR_LAST_ERROR, DBUS_ATTR_OPERATION, DBUS_ATTR_VARIANT, + DBUS_IFACE_RAUC_INSTALLER, DBUS_NAME_RAUC, - DBUS_NAME_RAUC_INSTALLER, - DBUS_NAME_RAUC_INSTALLER_COMPLETED, DBUS_OBJECT_BASE, + DBUS_SIGNAL_RAUC_INSTALLER_COMPLETED, RaucState, ) from .interface import DBusInterface @@ -91,7 +91,7 @@ class Rauc(DBusInterface): Return a coroutine. """ - return self.dbus.wait_signal(DBUS_NAME_RAUC_INSTALLER_COMPLETED) + return self.dbus.wait_signal(DBUS_SIGNAL_RAUC_INSTALLER_COMPLETED) @dbus_connected def mark(self, state: RaucState, slot_identifier: str): @@ -104,7 +104,7 @@ class Rauc(DBusInterface): @dbus_connected async def update(self): """Update Properties.""" - data = await self.dbus.get_properties(DBUS_NAME_RAUC_INSTALLER) + data = await self.dbus.get_properties(DBUS_IFACE_RAUC_INSTALLER) if not data: _LOGGER.warning("Can't get properties for rauc") return diff --git a/supervisor/dbus/systemd.py b/supervisor/dbus/systemd.py index 1ec3ca44f..a17e4a52a 100644 --- a/supervisor/dbus/systemd.py +++ b/supervisor/dbus/systemd.py @@ -10,8 +10,8 @@ from .const import ( DBUS_ATTR_KERNEL_TIMESTAMP_MONOTONIC, DBUS_ATTR_LOADER_TIMESTAMP_MONOTONIC, DBUS_ATTR_USERSPACE_TIMESTAMP_MONOTONIC, + DBUS_IFACE_SYSTEMD_MANAGER, DBUS_NAME_SYSTEMD, - DBUS_NAME_SYSTEMD_MANAGER, DBUS_OBJECT_SYSTEMD, ) from .interface import DBusInterface, dbus_property @@ -116,4 +116,4 @@ class Systemd(DBusInterface): @dbus_connected async def update(self): """Update Properties.""" - self.properties = await self.dbus.get_properties(DBUS_NAME_SYSTEMD_MANAGER) + self.properties = await self.dbus.get_properties(DBUS_IFACE_SYSTEMD_MANAGER) diff --git a/supervisor/dbus/timedate.py b/supervisor/dbus/timedate.py index a857de8cf..40e919477 100644 --- a/supervisor/dbus/timedate.py +++ b/supervisor/dbus/timedate.py @@ -12,6 +12,7 @@ from .const import ( DBUS_ATTR_NTPSYNCHRONIZED, DBUS_ATTR_TIMEUSEC, DBUS_ATTR_TIMEZONE, + DBUS_IFACE_TIMEDATE, DBUS_NAME_TIMEDATE, DBUS_OBJECT_TIMEDATE, ) @@ -90,4 +91,4 @@ class TimeDate(DBusInterface): @dbus_connected async def update(self): """Update Properties.""" - self.properties = await self.dbus.get_properties(DBUS_NAME_TIMEDATE) + self.properties = await self.dbus.get_properties(DBUS_IFACE_TIMEDATE) diff --git a/supervisor/host/network.py b/supervisor/host/network.py index 9151346d8..89075d10f 100644 --- a/supervisor/host/network.py +++ b/supervisor/host/network.py @@ -10,8 +10,9 @@ import attr from ..const import ATTR_HOST_INTERNET from ..coresys import CoreSys, CoreSysAttributes from ..dbus.const import ( - DBUS_NAME_NM_CONNECTION_ACTIVE_CHANGED, + DBUS_SIGNAL_NM_CONNECTION_ACTIVE_CHANGED, ConnectionStateType, + ConnectivityState, DeviceType, InterfaceMethod as NMInterfaceMethod, WirelessMethodType, @@ -77,18 +78,15 @@ class NetworkManager(CoreSysAttributes): return list(dict.fromkeys(servers)) async def check_connectivity(self): - """Check the internet connection. + """Check the internet connection.""" - ConnectionState 4 == FULL (has internet) - https://developer.gnome.org/NetworkManager/stable/nm-dbus-types.html#NMConnectivityState - """ if not self.sys_dbus.network.connectivity_enabled: return # Check connectivity try: state = await self.sys_dbus.network.check_connectivity() - self.connectivity = state[0] == 4 + self.connectivity = state[0] == ConnectivityState.CONNECTIVITY_FULL except DBusError as err: _LOGGER.warning("Can't update connectivity information: %s", err) self.connectivity = False @@ -119,6 +117,7 @@ class NetworkManager(CoreSysAttributes): async def apply_changes(self, interface: Interface) -> None: """Apply Interface changes to host.""" inet = self.sys_dbus.network.interfaces.get(interface.name) + con: NetworkConnection = None # Update exist configuration if ( @@ -136,9 +135,13 @@ class NetworkManager(CoreSysAttributes): try: await inet.settings.update(settings) - await self.sys_dbus.network.activate_connection( + con = await self.sys_dbus.network.activate_connection( inet.settings.object_path, inet.object_path ) + _LOGGER.debug( + "activate_connection returns %s", + con.object_path, + ) except DBusError as err: raise HostNetworkError( f"Can't update config on {interface.name}: {err}", _LOGGER.error @@ -150,10 +153,13 @@ class NetworkManager(CoreSysAttributes): settings = get_connection_from_interface(interface) try: - new_con = await self.sys_dbus.network.add_and_activate_connection( + settings, con = await self.sys_dbus.network.add_and_activate_connection( settings, inet.object_path ) - _LOGGER.debug("add_and_activate_connection returns %s", new_con) + _LOGGER.debug( + "add_and_activate_connection returns %s", + con.object_path, + ) except DBusError as err: raise HostNetworkError( f"Can't create config and activate {interface.name}: {err}", @@ -184,11 +190,26 @@ class NetworkManager(CoreSysAttributes): "Requested Network interface update is not possible", _LOGGER.warning ) - # This signal is fired twice: Activating -> Activated. It seems we miss the first - # "usually"... We should filter by state and explicitly wait for the second. - await self.sys_dbus.network.dbus.wait_signal( - DBUS_NAME_NM_CONNECTION_ACTIVE_CHANGED - ) + if con: + # Only consider activated or deactivated signals, continue waiting on others + def message_filter(msg_body): + state: ConnectionStateType = msg_body[0] + if state == ConnectionStateType.DEACTIVATED: + return True + elif state == ConnectionStateType.ACTIVATED: + return True + return False + + result = await con.dbus.wait_signal( + DBUS_SIGNAL_NM_CONNECTION_ACTIVE_CHANGED, message_filter + ) + + _LOGGER.debug("StateChanged signal received, result: %s", str(result)) + state: ConnectionStateType = result[0] + if state != ConnectionStateType.ACTIVATED: + raise HostNetworkError( + "Activating connection failed, check connection settings." + ) await self.update() diff --git a/supervisor/utils/dbus.py b/supervisor/utils/dbus.py index fab5950d7..736166c3e 100644 --- a/supervisor/utils/dbus.py +++ b/supervisor/utils/dbus.py @@ -49,7 +49,12 @@ class DBus: self.object_path: str = object_path self.methods: set[str] = set() self.signals: set[str] = set() - self._bus: MessageBus = None + self._bus: MessageBus | None = None + + def __del__(self): + """Delete dbus object.""" + if self._bus: + self._bus.disconnect() @staticmethod async def connect(bus_name: str, object_path: str) -> DBus: @@ -171,13 +176,14 @@ class DBus: _LOGGER.error("No Set attribute %s for %s", name, interface) raise DBusFatalError() from err - async def wait_signal(self, signal): - """Wait for single event.""" - signal_parts = signal.split(".") + async def wait_signal(self, signal_member, message_filter=None) -> Any: + """Wait for signal on this object.""" + signal_parts = signal_member.split(".") interface = ".".join(signal_parts[:-1]) member = signal_parts[-1] + match = f"type='signal',interface={interface},member={member},path={self.object_path}" - _LOGGER.debug("Wait for signal %s", signal) + _LOGGER.debug("Install match for signal %s", signal_member) await self._bus.call( Message( destination="org.freedesktop.DBus", @@ -185,7 +191,7 @@ class DBus: path="/org/freedesktop/DBus", member="AddMatch", signature="s", - body=[f"type='signal',interface={interface},member={member}"], + body=[match], ) ) @@ -197,21 +203,44 @@ class DBus: return _LOGGER.debug( - "Signal message received %s, %s %s", msg, msg.interface, msg.member + "Signal message received %s, %s.%s object %s", + msg.body, + msg.interface, + msg.member, + msg.path, ) - if msg.interface != interface or msg.member != member: + if ( + msg.interface != interface + or msg.member != member + or msg.path != self.object_path + ): return # Avoid race condition: We already received signal but handler not yet removed. if future.done(): return - future.set_result(_remove_dbus_signature(msg.body)) + msg_body = _remove_dbus_signature(msg.body) + if message_filter and not message_filter(msg_body): + return + + future.set_result(msg_body) self._bus.add_message_handler(message_handler) result = await future self._bus.remove_message_handler(message_handler) + await self._bus.call( + Message( + destination="org.freedesktop.DBus", + interface="org.freedesktop.DBus", + path="/org/freedesktop/DBus", + member="RemoveMatch", + signature="s", + body=[match], + ) + ) + return result def __getattr__(self, name: str) -> DBusCallWrapper: diff --git a/tests/conftest.py b/tests/conftest.py index d6705410c..b6894e9a2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,6 +18,7 @@ from supervisor.api import RestAPI from supervisor.bootstrap import initialize_coresys from supervisor.const import REQUEST_FROM from supervisor.coresys import CoreSys +from supervisor.dbus.const import DBUS_SIGNAL_NM_CONNECTION_ACTIVE_CHANGED from supervisor.dbus.network import NetworkManager from supervisor.docker import DockerAPI from supervisor.store.addon import AddonStore @@ -79,8 +80,9 @@ def dbus() -> DBus: return load_json_fixture(f"{fixture}.json") - async def mock_wait_signal(_, __): - pass + async def mock_wait_signal(_, signal_method, ___): + if signal_method == DBUS_SIGNAL_NM_CONNECTION_ACTIVE_CHANGED: + return [2, 0] async def mock_init_proxy(self): diff --git a/tests/fixtures/org_freedesktop_NetworkManager-AddAndActivateConnection.json b/tests/fixtures/org_freedesktop_NetworkManager-AddAndActivateConnection.json index 0637a088a..131b2bab8 100644 --- a/tests/fixtures/org_freedesktop_NetworkManager-AddAndActivateConnection.json +++ b/tests/fixtures/org_freedesktop_NetworkManager-AddAndActivateConnection.json @@ -1 +1 @@ -[] \ No newline at end of file +[ "/org/freedesktop/NetworkManager/Settings/1", "/org/freedesktop/NetworkManager/ActiveConnection/1" ]