From 42f20a4edf88cc3a705be4786dab2fe85d35f946 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 15:14:11 +0200 Subject: [PATCH 01/15] device: reorder checks in check_connection_available() No change in behavior. Just reorder, so that the checks that can be reviewed in place are handled first. --- src/core/devices/nm-device.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index e5290e796d..33d7cdeb64 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -15324,10 +15324,7 @@ check_connection_available(NMDevice *self, { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); - /* Connections which require a network connection are not available when - * the device has no carrier, even with ignore-carrer=TRUE. - */ - if (priv->carrier || !connection_requires_carrier(connection)) + if (priv->carrier) return TRUE; if (NM_FLAGS_HAS(flags, _NM_DEVICE_CHECK_CON_AVAILABLE_FOR_USER_REQUEST_WAITING_CARRIER) @@ -15339,12 +15336,6 @@ check_connection_available(NMDevice *self, return TRUE; } - /* master types are always available even without carrier. - * Making connection non-available would un-enslave slaves which - * is not desired. */ - if (nm_device_is_master(self)) - return TRUE; - if (!priv->up) { /* If the device is !IFF_UP it also has no carrier. But we assume that if we * would start activating the device (and thereby set the device IFF_UP), @@ -15354,6 +15345,18 @@ check_connection_available(NMDevice *self, return TRUE; } + if (!connection_requires_carrier(connection)) { + /* Connections that don't require carrier are available. */ + return TRUE; + } + + if (nm_device_is_master(self)) { + /* master types are always available even without carrier. + * Making connection non-available would un-enslave slaves which + * is not desired. */ + return TRUE; + } + nm_utils_error_set_literal(error, NM_UTILS_ERROR_CONNECTION_AVAILABLE_TEMPORARY, "device has no carrier"); From 5b8e6c01a96c0cc29f7f933a195d75e4a5517919 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 16:30:08 +0200 Subject: [PATCH 02/15] device: define auto variables on separate lines in connection_requires_carrier() --- src/core/devices/nm-device.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 33d7cdeb64..160963d8ae 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -10902,10 +10902,13 @@ connection_ip_method_requires_carrier(NMConnection *connection, static gboolean connection_requires_carrier(NMConnection *connection) { - NMSettingIPConfig *s_ip4, *s_ip6; + NMSettingIPConfig *s_ip4; + NMSettingIPConfig *s_ip6; NMSettingConnection *s_con; - gboolean ip4_carrier_wanted, ip6_carrier_wanted; - gboolean ip4_used = FALSE, ip6_used = FALSE; + gboolean ip4_carrier_wanted; + gboolean ip6_carrier_wanted; + gboolean ip4_used = FALSE; + gboolean ip6_used = FALSE; /* We can progress to IP_CONFIG now, so that we're enslaved. * That may actually cause carrier to go up and thus continue activation. */ From cba8eb978492f3cc83cb8a854d526414851dc5e3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Jun 2023 14:35:46 +0200 Subject: [PATCH 03/15] core: add nm_match_spec_match_type_to_bool() helper to convert enum to boolean All callers eventually want a boolean instead of a NMMatchSpecMatchType. I think the NMMatchSpecMatchType enum still has value at the lower layers, where the enum values are clearer (when reading the code). So don't drop NMMatchSpecMatchType entirely. However, let's add nm_match_spec_match_type_to_bool() to convert the match-type to a boolean to avoid duplicating the code. --- src/core/NetworkManagerUtils.c | 12 +----------- src/core/NetworkManagerUtils.h | 2 ++ src/core/devices/nm-device.c | 11 +---------- src/core/nm-core-utils.c | 14 ++++++++++++++ src/core/nm-core-utils.h | 2 ++ 5 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/core/NetworkManagerUtils.c b/src/core/NetworkManagerUtils.c index 972bcf0a19..9dac44502e 100644 --- a/src/core/NetworkManagerUtils.c +++ b/src/core/NetworkManagerUtils.c @@ -922,17 +922,7 @@ nm_match_spec_device_by_pllink(const NMPlatformLink *pllink, NULL, NULL, match_dhcp_plugin); - - switch (m) { - case NM_MATCH_SPEC_MATCH: - return TRUE; - case NM_MATCH_SPEC_NEG_MATCH: - return FALSE; - case NM_MATCH_SPEC_NO_MATCH: - return no_match_value; - } - nm_assert_not_reached(); - return no_match_value; + return nm_match_spec_match_type_to_bool(m, no_match_value); } /*****************************************************************************/ diff --git a/src/core/NetworkManagerUtils.h b/src/core/NetworkManagerUtils.h index c55e6255bc..514d1cc8da 100644 --- a/src/core/NetworkManagerUtils.h +++ b/src/core/NetworkManagerUtils.h @@ -89,6 +89,8 @@ NMConnection *nm_utils_match_connection(NMConnection *const *connections, NMUtilsMatchFilterFunc match_filter_func, gpointer match_filter_data); +/*****************************************************************************/ + int nm_match_spec_device_by_pllink(const NMPlatformLink *pllink, const char *match_device_type, const char *match_dhcp_plugin, diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 160963d8ae..352bd26e1c 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -17303,16 +17303,7 @@ nm_device_spec_match_list_full(NMDevice *self, const GSList *specs, int no_match klass->get_s390_subchannels ? klass->get_s390_subchannels(self) : NULL, nm_dhcp_manager_get_config(nm_dhcp_manager_get())); - switch (m) { - case NM_MATCH_SPEC_MATCH: - return TRUE; - case NM_MATCH_SPEC_NEG_MATCH: - return FALSE; - case NM_MATCH_SPEC_NO_MATCH: - return no_match_value; - } - nm_assert_not_reached(); - return no_match_value; + return nm_match_spec_match_type_to_bool(m, no_match_value); } guint diff --git a/src/core/nm-core-utils.c b/src/core/nm-core-utils.c index f12a731694..e6c4c8cbd6 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -1478,6 +1478,20 @@ nm_match_spec_device(const GSList *specs, return _match_result(has_except, has_not_except, has_match, has_match_except); } +int +nm_match_spec_match_type_to_bool(NMMatchSpecMatchType m, int no_match_value) +{ + switch (m) { + case NM_MATCH_SPEC_MATCH: + return TRUE; + case NM_MATCH_SPEC_NEG_MATCH: + return FALSE; + case NM_MATCH_SPEC_NO_MATCH: + return no_match_value; + } + return nm_assert_unreachable_val(no_match_value); +} + typedef struct { const char *uuid; const char *id; diff --git a/src/core/nm-core-utils.h b/src/core/nm-core-utils.h index 87d3559e19..bdcaa9b6d2 100644 --- a/src/core/nm-core-utils.h +++ b/src/core/nm-core-utils.h @@ -193,6 +193,8 @@ typedef enum { NM_MATCH_SPEC_NEG_MATCH = 2, } NMMatchSpecMatchType; +int nm_match_spec_match_type_to_bool(NMMatchSpecMatchType m, int no_match_value); + NMMatchSpecMatchType nm_match_spec_device(const GSList *specs, const char *interface_name, const char *device_type, From c47d6b17d580fa1466c5aa89e1c7c4717a676501 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Jun 2023 17:01:01 +0200 Subject: [PATCH 04/15] core: replace multiple arguments of nm_match_spec_device() with struct Struct allow named arguments, which seems easier to maintain instead of a function with many arguments. Also, adding a new parameter does not require changes to most of the callers. The real advantage of this is that we encode all the search parameters in one argument. And we can add that argument to _match_section_infos_lookup(), alongside lookup by NMDevice or NMPlatformLink. --- src/core/NetworkManagerUtils.c | 16 +++-- src/core/devices/nm-device.c | 20 +++--- src/core/nm-core-utils.c | 124 +++++++++++++++++---------------- src/core/nm-core-utils.h | 20 +++--- src/core/tests/test-core.c | 21 +++--- 5 files changed, 109 insertions(+), 92 deletions(-) diff --git a/src/core/NetworkManagerUtils.c b/src/core/NetworkManagerUtils.c index 9dac44502e..94e14852af 100644 --- a/src/core/NetworkManagerUtils.c +++ b/src/core/NetworkManagerUtils.c @@ -915,13 +915,15 @@ nm_match_spec_device_by_pllink(const NMPlatformLink *pllink, * It's still useful because of specs like "*" and "except:interface-name:eth0", * which match even in that case. */ m = nm_match_spec_device(specs, - pllink ? pllink->name : NULL, - match_device_type, - pllink ? pllink->driver : NULL, - NULL, - NULL, - NULL, - match_dhcp_plugin); + &((const NMMatchSpecDeviceData){ + .interface_name = pllink ? pllink->name : NULL, + .device_type = match_device_type, + .driver = pllink ? pllink->driver : NULL, + .driver_version = NULL, + .hwaddr = NULL, + .s390_subchannels = NULL, + .dhcp_plugin = match_dhcp_plugin, + })); return nm_match_spec_match_type_to_bool(m, no_match_value); } diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 352bd26e1c..aae3356913 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -17294,14 +17294,18 @@ nm_device_spec_match_list_full(NMDevice *self, const GSList *specs, int no_match !nm_device_get_unmanaged_flags(self, NM_UNMANAGED_PLATFORM_INIT), &is_fake); - m = nm_match_spec_device(specs, - nm_device_get_iface(self), - nm_device_get_type_description(self), - nm_device_get_driver(self), - nm_device_get_driver_version(self), - is_fake ? NULL : hw_address, - klass->get_s390_subchannels ? klass->get_s390_subchannels(self) : NULL, - nm_dhcp_manager_get_config(nm_dhcp_manager_get())); + m = nm_match_spec_device( + specs, + &((const NMMatchSpecDeviceData){ + .interface_name = nm_device_get_iface(self), + .device_type = nm_device_get_type_description(self), + .driver = nm_device_get_driver(self), + .driver_version = nm_device_get_driver_version(self), + .hwaddr = is_fake ? NULL : hw_address, + .s390_subchannels = + klass->get_s390_subchannels ? klass->get_s390_subchannels(self) : NULL, + .dhcp_plugin = nm_dhcp_manager_get_config(nm_dhcp_manager_get()), + })); return nm_match_spec_match_type_to_bool(m, no_match_value); } diff --git a/src/core/nm-core-utils.c b/src/core/nm-core-utils.c index e6c4c8cbd6..5442efbf88 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -1148,25 +1148,26 @@ nm_utils_read_link_absolute(const char *link_file, GError **error) #define MATCH_TAG_CONFIG_ENV "env:" typedef struct { - const char *interface_name; - const char *device_type; - const char *driver; - const char *driver_version; - const char *dhcp_plugin; + /* This struct contains pre-processed data from NMMatchSpecDeviceData so + * we only need to parse it once. */ + const NMMatchSpecDeviceData *data; + const char *device_type; + const char *driver; + const char *driver_version; + const char *dhcp_plugin; struct { - const char *value; - gboolean is_parsed; - guint len; - guint8 bin[_NM_UTILS_HWADDR_LEN_MAX]; + gboolean is_parsed; + guint len; + guint8 bin[_NM_UTILS_HWADDR_LEN_MAX]; } hwaddr; struct { - const char *value; - gboolean is_parsed; - guint32 a; - guint32 b; - guint32 c; + gboolean is_parsed; + gboolean is_good; + guint32 a; + guint32 b; + guint32 c; } s390_subchannels; -} MatchDeviceData; +} MatchSpecDeviceData; static gboolean match_device_s390_subchannels_parse(const char *s390_subchannels, @@ -1234,22 +1235,25 @@ match_device_s390_subchannels_parse(const char *s390_subchannels, } static gboolean -match_data_s390_subchannels_eval(const char *spec_str, MatchDeviceData *match_data) +match_data_s390_subchannels_eval(const char *spec_str, MatchSpecDeviceData *match_data) { - guint32 a, b, c; + guint32 a; + guint32 b; + guint32 c; if (G_UNLIKELY(!match_data->s390_subchannels.is_parsed)) { + nm_assert(!match_data->s390_subchannels.is_good); match_data->s390_subchannels.is_parsed = TRUE; - if (!match_data->s390_subchannels.value - || !match_device_s390_subchannels_parse(match_data->s390_subchannels.value, + if (!match_data->data->s390_subchannels + || !match_device_s390_subchannels_parse(match_data->data->s390_subchannels, &match_data->s390_subchannels.a, &match_data->s390_subchannels.b, &match_data->s390_subchannels.c)) { - match_data->s390_subchannels.value = NULL; return FALSE; } - } else if (!match_data->s390_subchannels.value) + match_data->s390_subchannels.is_good = TRUE; + } else if (!match_data->s390_subchannels.is_good) return FALSE; if (!match_device_s390_subchannels_parse(spec_str, &a, &b, &c)) @@ -1259,15 +1263,16 @@ match_data_s390_subchannels_eval(const char *spec_str, MatchDeviceData *match_da } static gboolean -match_device_hwaddr_eval(const char *spec_str, MatchDeviceData *match_data) +match_device_hwaddr_eval(const char *spec_str, MatchSpecDeviceData *match_data) { if (G_UNLIKELY(!match_data->hwaddr.is_parsed)) { match_data->hwaddr.is_parsed = TRUE; + nm_assert(match_data->hwaddr.len == 0); - if (match_data->hwaddr.value) { + if (match_data->data->hwaddr) { gsize l; - if (!_nm_utils_hwaddr_aton(match_data->hwaddr.value, + if (!_nm_utils_hwaddr_aton(match_data->data->hwaddr, match_data->hwaddr.bin, sizeof(match_data->hwaddr.bin), &l)) @@ -1275,7 +1280,7 @@ match_device_hwaddr_eval(const char *spec_str, MatchDeviceData *match_data) match_data->hwaddr.len = l; } else return FALSE; - } else if (!match_data->hwaddr.len) + } else if (match_data->hwaddr.len == 0) return FALSE; return nm_utils_hwaddr_matches(spec_str, -1, match_data->hwaddr.bin, match_data->hwaddr.len); @@ -1330,7 +1335,7 @@ match_except(const char *spec_str, gboolean *out_except) } static gboolean -match_device_eval(const char *spec_str, gboolean allow_fuzzy, MatchDeviceData *match_data) +match_device_eval(const char *spec_str, gboolean allow_fuzzy, MatchSpecDeviceData *match_data) { if (spec_str[0] == '*' && spec_str[1] == '\0') return TRUE; @@ -1353,10 +1358,10 @@ match_device_eval(const char *spec_str, gboolean allow_fuzzy, MatchDeviceData *m use_pattern = TRUE; } - if (match_data->interface_name) { - if (nm_streq(spec_str, match_data->interface_name)) + if (match_data->data->interface_name) { + if (nm_streq(spec_str, match_data->data->interface_name)) return TRUE; - if (use_pattern && g_pattern_match_simple(spec_str, match_data->interface_name)) + if (use_pattern && g_pattern_match_simple(spec_str, match_data->data->interface_name)) return TRUE; } return FALSE; @@ -1402,7 +1407,8 @@ match_device_eval(const char *spec_str, gboolean allow_fuzzy, MatchDeviceData *m if (allow_fuzzy) { if (match_device_hwaddr_eval(spec_str, match_data)) return TRUE; - if (match_data->interface_name && nm_streq(spec_str, match_data->interface_name)) + if (match_data->data->interface_name + && nm_streq(spec_str, match_data->data->interface_name)) return TRUE; } @@ -1410,42 +1416,40 @@ match_device_eval(const char *spec_str, gboolean allow_fuzzy, MatchDeviceData *m } NMMatchSpecMatchType -nm_match_spec_device(const GSList *specs, - const char *interface_name, - const char *device_type, - const char *driver, - const char *driver_version, - const char *hwaddr, - const char *s390_subchannels, - const char *dhcp_plugin) +nm_match_spec_device(const GSList *specs, const NMMatchSpecDeviceData *data) { - const GSList *iter; - gboolean has_match = FALSE; - gboolean has_match_except = FALSE; - gboolean has_except = FALSE; - gboolean has_not_except = FALSE; - const char *spec_str; - MatchDeviceData match_data = { - .interface_name = interface_name, - .device_type = nm_str_not_empty(device_type), - .driver = nm_str_not_empty(driver), - .driver_version = nm_str_not_empty(driver_version), - .dhcp_plugin = nm_str_not_empty(dhcp_plugin), - .hwaddr = - { - .value = hwaddr, - }, - .s390_subchannels = - { - .value = s390_subchannels, - }, - }; + const GSList *iter; + gboolean has_match = FALSE; + gboolean has_match_except = FALSE; + gboolean has_except = FALSE; + gboolean has_not_except = FALSE; + const char *spec_str; + MatchSpecDeviceData match_data; - nm_assert(!hwaddr || nm_utils_hwaddr_valid(hwaddr, -1)); + nm_assert(data); + nm_assert(!data->hwaddr || nm_utils_hwaddr_valid(data->hwaddr, -1)); if (!specs) return NM_MATCH_SPEC_NO_MATCH; + match_data = (MatchSpecDeviceData){ + .data = data, + .device_type = nm_str_not_empty(data->device_type), + .driver = nm_str_not_empty(data->driver), + .driver_version = nm_str_not_empty(data->driver_version), + .dhcp_plugin = nm_str_not_empty(data->dhcp_plugin), + .hwaddr = + { + .is_parsed = FALSE, + .len = 0, + }, + .s390_subchannels = + { + .is_parsed = FALSE, + .is_good = FALSE, + }, + }; + for (iter = specs; iter; iter = iter->next) { gboolean except; diff --git a/src/core/nm-core-utils.h b/src/core/nm-core-utils.h index bdcaa9b6d2..5511250422 100644 --- a/src/core/nm-core-utils.h +++ b/src/core/nm-core-utils.h @@ -195,14 +195,18 @@ typedef enum { int nm_match_spec_match_type_to_bool(NMMatchSpecMatchType m, int no_match_value); -NMMatchSpecMatchType nm_match_spec_device(const GSList *specs, - const char *interface_name, - const char *device_type, - const char *driver, - const char *driver_version, - const char *hwaddr, - const char *s390_subchannels, - const char *dhcp_plugin); +typedef struct _NMMatchSpecDeviceData { + const char *interface_name; + const char *device_type; + const char *driver; + const char *driver_version; + const char *dhcp_plugin; + const char *hwaddr; + const char *s390_subchannels; +} NMMatchSpecDeviceData; + +NMMatchSpecMatchType nm_match_spec_device(const GSList *specs, const NMMatchSpecDeviceData *data); + NMMatchSpecMatchType nm_match_spec_config(const GSList *specs, guint nm_version, const char *env); GSList *nm_match_spec_split(const char *value); char *nm_match_spec_join(GSList *specs); diff --git a/src/core/tests/test-core.c b/src/core/tests/test-core.c index a30cc3babc..8296d74579 100644 --- a/src/core/tests/test-core.c +++ b/src/core/tests/test-core.c @@ -1244,13 +1244,9 @@ _test_match_spec_device(const GSList *specs, const char *match_str) { if (match_str && g_str_has_prefix(match_str, MATCH_S390)) return nm_match_spec_device(specs, - NULL, - NULL, - NULL, - NULL, - NULL, - &match_str[NM_STRLEN(MATCH_S390)], - NULL); + &((const NMMatchSpecDeviceData){ + .s390_subchannels = &match_str[NM_STRLEN(MATCH_S390)], + })); if (match_str && g_str_has_prefix(match_str, MATCH_DRIVER)) { gs_free char *s = g_strdup(&match_str[NM_STRLEN(MATCH_DRIVER)]); char *t; @@ -1260,9 +1256,16 @@ _test_match_spec_device(const GSList *specs, const char *match_str) t[0] = '\0'; t++; } - return nm_match_spec_device(specs, NULL, NULL, s, t, NULL, NULL, NULL); + return nm_match_spec_device(specs, + &((const NMMatchSpecDeviceData){ + .driver = s, + .driver_version = t, + })); } - return nm_match_spec_device(specs, match_str, NULL, NULL, NULL, NULL, NULL, NULL); + return nm_match_spec_device(specs, + &((const NMMatchSpecDeviceData){ + .interface_name = match_str, + })); } static void From dbb45f14d325369cffc9c05fa4b57ca47c0eed09 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 12:23:11 +0200 Subject: [PATCH 05/15] device: add nm_device_get_s390_subchannels() accessor --- src/core/devices/nm-device.c | 12 ++++++++++++ src/core/devices/nm-device.h | 1 + 2 files changed, 13 insertions(+) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index aae3356913..e84317c2f5 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -5115,6 +5115,18 @@ nm_device_get_ip_iface_identifier(NMDevice *self, return NM_DEVICE_GET_CLASS(self)->get_ip_iface_identifier(self, iid); } +const char * +nm_device_get_s390_subchannels(NMDevice *self) +{ + NMDeviceClass *klass; + + g_return_val_if_fail(NM_IS_DEVICE(self), NULL); + + klass = NM_DEVICE_GET_CLASS(self); + + return klass->get_s390_subchannels ? klass->get_s390_subchannels(self) : NULL; +} + const char * nm_device_get_driver(NMDevice *self) { diff --git a/src/core/devices/nm-device.h b/src/core/devices/nm-device.h index 35a8d1cd49..b096d23ac1 100644 --- a/src/core/devices/nm-device.h +++ b/src/core/devices/nm-device.h @@ -460,6 +460,7 @@ gboolean nm_device_is_real(NMDevice *dev); const char *nm_device_get_ip_iface(NMDevice *dev); const char *nm_device_get_ip_iface_from_platform(NMDevice *dev); int nm_device_get_ip_ifindex(const NMDevice *dev); +const char *nm_device_get_s390_subchannels(NMDevice *self); const char *nm_device_get_driver(NMDevice *dev); const char *nm_device_get_driver_version(NMDevice *dev); const char *nm_device_get_type_desc(NMDevice *dev); From 798ea93c459b329f654d535eceecceab42e4709e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 12:11:54 +0200 Subject: [PATCH 06/15] device: add nm_match_spec_device_data_init_from_device() helper Will be used later. --- src/core/NetworkManagerUtils.c | 42 ++++++++++++++++++++++++++++++++++ src/core/NetworkManagerUtils.h | 6 +++++ src/core/devices/nm-device.c | 28 +++-------------------- 3 files changed, 51 insertions(+), 25 deletions(-) diff --git a/src/core/NetworkManagerUtils.c b/src/core/NetworkManagerUtils.c index 94e14852af..33a0c0a466 100644 --- a/src/core/NetworkManagerUtils.c +++ b/src/core/NetworkManagerUtils.c @@ -30,6 +30,7 @@ #include "libnm-platform/nm-linux-platform.h" #include "libnm-platform/nm-platform-utils.h" #include "nm-auth-utils.h" +#include "devices/nm-device.h" /*****************************************************************************/ @@ -900,6 +901,47 @@ nm_utils_match_connection(NMConnection *const *connections, /*****************************************************************************/ +const struct _NMMatchSpecDeviceData * +nm_match_spec_device_data_init_from_device(struct _NMMatchSpecDeviceData *out_data, + NMDevice *device) +{ + const char *hw_address; + gboolean is_fake; + + nm_assert(out_data); + + if (!device) { + *out_data = (NMMatchSpecDeviceData){}; + return out_data; + } + + nm_assert(NM_IS_DEVICE(device)); + + hw_address = nm_device_get_permanent_hw_address_full( + device, + !nm_device_get_unmanaged_flags(device, NM_UNMANAGED_PLATFORM_INIT), + &is_fake); + + /* Note that here we access various getters on @device, without cloning + * or taking ownership and return it to the caller. + * + * The returned data is only valid, until NMDevice gets modified again. */ + + *out_data = (NMMatchSpecDeviceData){ + .interface_name = nm_device_get_iface(device), + .device_type = nm_device_get_type_description(device), + .driver = nm_device_get_driver(device), + .driver_version = nm_device_get_driver_version(device), + .hwaddr = is_fake ? NULL : hw_address, + .s390_subchannels = nm_device_get_s390_subchannels(device), + .dhcp_plugin = nm_dhcp_manager_get_config(nm_dhcp_manager_get()), + }; + + return out_data; +} + +/*****************************************************************************/ + int nm_match_spec_device_by_pllink(const NMPlatformLink *pllink, const char *match_device_type, diff --git a/src/core/NetworkManagerUtils.h b/src/core/NetworkManagerUtils.h index 514d1cc8da..c6d6e274ba 100644 --- a/src/core/NetworkManagerUtils.h +++ b/src/core/NetworkManagerUtils.h @@ -91,6 +91,12 @@ NMConnection *nm_utils_match_connection(NMConnection *const *connections, /*****************************************************************************/ +struct _NMMatchSpecDeviceData; + +const struct _NMMatchSpecDeviceData * +nm_match_spec_device_data_init_from_device(struct _NMMatchSpecDeviceData *out_data, + NMDevice *device); + int nm_match_spec_device_by_pllink(const NMPlatformLink *pllink, const char *match_device_type, const char *match_dhcp_plugin, diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index e84317c2f5..67babb055f 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -17293,32 +17293,10 @@ nm_device_spec_match_list(NMDevice *self, const GSList *specs) int nm_device_spec_match_list_full(NMDevice *self, const GSList *specs, int no_match_value) { - NMDeviceClass *klass; - NMMatchSpecMatchType m; - const char *hw_address = NULL; - gboolean is_fake; - - g_return_val_if_fail(NM_IS_DEVICE(self), FALSE); - - klass = NM_DEVICE_GET_CLASS(self); - hw_address = nm_device_get_permanent_hw_address_full( - self, - !nm_device_get_unmanaged_flags(self, NM_UNMANAGED_PLATFORM_INIT), - &is_fake); - - m = nm_match_spec_device( - specs, - &((const NMMatchSpecDeviceData){ - .interface_name = nm_device_get_iface(self), - .device_type = nm_device_get_type_description(self), - .driver = nm_device_get_driver(self), - .driver_version = nm_device_get_driver_version(self), - .hwaddr = is_fake ? NULL : hw_address, - .s390_subchannels = - klass->get_s390_subchannels ? klass->get_s390_subchannels(self) : NULL, - .dhcp_plugin = nm_dhcp_manager_get_config(nm_dhcp_manager_get()), - })); + NMMatchSpecDeviceData data; + NMMatchSpecMatchType m; + m = nm_match_spec_device(specs, nm_match_spec_device_data_init_from_device(&data, self)); return nm_match_spec_match_type_to_bool(m, no_match_value); } From ccaecf7f3e7693721b85690ced808925884b689e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 12:32:46 +0200 Subject: [PATCH 07/15] core: add nm_match_spec_device_data_init_from_platform() helper --- src/core/NetworkManagerUtils.c | 47 +++++++++++++++++++++++----------- src/core/NetworkManagerUtils.h | 6 +++++ 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/core/NetworkManagerUtils.c b/src/core/NetworkManagerUtils.c index 33a0c0a466..94b2351310 100644 --- a/src/core/NetworkManagerUtils.c +++ b/src/core/NetworkManagerUtils.c @@ -940,6 +940,32 @@ nm_match_spec_device_data_init_from_device(struct _NMMatchSpecDeviceData *out_da return out_data; } +const NMMatchSpecDeviceData * +nm_match_spec_device_data_init_from_platform(NMMatchSpecDeviceData *out_data, + const NMPlatformLink *pllink, + const char *match_device_type, + const char *match_dhcp_plugin) +{ + nm_assert(out_data); + + /* we can only match by certain properties that are available on the + * platform link (and even @pllink might be missing. + * + * It's still useful because of specs like "*" and "except:interface-name:eth0", + * which match even in that case. */ + + *out_data = (NMMatchSpecDeviceData){ + .interface_name = pllink ? pllink->name : NULL, + .device_type = match_device_type, + .driver = pllink ? pllink->driver : NULL, + .driver_version = NULL, + .hwaddr = NULL, + .s390_subchannels = NULL, + .dhcp_plugin = match_dhcp_plugin, + }; + return out_data; +} + /*****************************************************************************/ int @@ -949,23 +975,14 @@ nm_match_spec_device_by_pllink(const NMPlatformLink *pllink, const GSList *specs, int no_match_value) { - NMMatchSpecMatchType m; + NMMatchSpecMatchType m; + NMMatchSpecDeviceData data; - /* we can only match by certain properties that are available on the - * platform link (and even @pllink might be missing. - * - * It's still useful because of specs like "*" and "except:interface-name:eth0", - * which match even in that case. */ m = nm_match_spec_device(specs, - &((const NMMatchSpecDeviceData){ - .interface_name = pllink ? pllink->name : NULL, - .device_type = match_device_type, - .driver = pllink ? pllink->driver : NULL, - .driver_version = NULL, - .hwaddr = NULL, - .s390_subchannels = NULL, - .dhcp_plugin = match_dhcp_plugin, - })); + nm_match_spec_device_data_init_from_platform(&data, + pllink, + match_device_type, + match_dhcp_plugin)); return nm_match_spec_match_type_to_bool(m, no_match_value); } diff --git a/src/core/NetworkManagerUtils.h b/src/core/NetworkManagerUtils.h index c6d6e274ba..7d8afe5a2b 100644 --- a/src/core/NetworkManagerUtils.h +++ b/src/core/NetworkManagerUtils.h @@ -97,6 +97,12 @@ const struct _NMMatchSpecDeviceData * nm_match_spec_device_data_init_from_device(struct _NMMatchSpecDeviceData *out_data, NMDevice *device); +const struct _NMMatchSpecDeviceData * +nm_match_spec_device_data_init_from_platform(struct _NMMatchSpecDeviceData *out_data, + const NMPlatformLink *pllink, + const char *match_device_type, + const char *match_dhcp_plugin); + int nm_match_spec_device_by_pllink(const NMPlatformLink *pllink, const char *match_device_type, const char *match_dhcp_plugin, From 52518066515321340a15224ca478429d14767d5f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 12:43:03 +0200 Subject: [PATCH 08/15] core: refactor _match_section_infos_lookup() to accept match_data argument This makes the code more generic, where _match_section_infos_lookup() accepts a match_data argument. Previously, it supported two hard-coded approaches (from-device, from-platform). Note that _match_section_infos_lookup() still accepts either a "match_data" or a "device" argument. It might be nicer, if _match_section_infos_lookup() would only accept "match_data". If a caller wants lookup by "device", they would need to call nm_match_spec_device_data_init_from_device() first. However, it's done this way with a separate "device" argument, because often we don't have any configuration to search for, and the initialization of the match-data can be saved. So for the common case where we want to lookup by "device", we initialize the "match-data" lazy and on demand. --- src/core/nm-config-data.c | 61 +++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/src/core/nm-config-data.c b/src/core/nm-config-data.c index ff44bc46b3..62949ccbfb 100644 --- a/src/core/nm-config-data.c +++ b/src/core/nm-config-data.c @@ -1488,21 +1488,23 @@ global_dns_equal(NMGlobalDnsConfig *old, NMGlobalDnsConfig *new) /*****************************************************************************/ static const MatchSectionInfo * -_match_section_infos_lookup(const MatchSectionInfo *match_section_infos, - GKeyFile *keyfile, - const char *property, - NMDevice *device, - const NMPlatformLink *pllink, - const char *match_device_type, - const char **out_value) +_match_section_infos_lookup(const MatchSectionInfo *match_section_infos, + GKeyFile *keyfile, + const char *property, + const NMMatchSpecDeviceData *match_data, + NMDevice *device, + const char **out_value) { - const char *match_dhcp_plugin; + NMMatchSpecDeviceData match_data_local; + + /* Caller must either provide a "match_data" or a "device" (actually, + * neither is also fine, albeit unusual). */ + nm_assert(!match_data || !device); + nm_assert(!device || NM_IS_DEVICE(device)); if (!match_section_infos) goto out; - match_dhcp_plugin = nm_dhcp_manager_get_config(nm_dhcp_manager_get()); - for (; match_section_infos->group_name; match_section_infos++) { const char *value; gboolean match; @@ -1519,16 +1521,17 @@ _match_section_infos_lookup(const MatchSectionInfo *match_section_infos, continue; if (match_section_infos->match_device.has) { - if (device) - match = nm_device_spec_match_list(device, match_section_infos->match_device.spec); - else if (pllink) - match = nm_match_spec_device_by_pllink(pllink, - match_device_type, - match_dhcp_plugin, - match_section_infos->match_device.spec, - FALSE); - else - match = FALSE; + NMMatchSpecMatchType m; + + if (G_UNLIKELY(!match_data)) { + /* In most cases, we don't actually have any matches. So we "optimize" + * here by allowing the user to specify a NMDEvice directly, and only + * initialize the match-data when needed. */ + match_data = nm_match_spec_device_data_init_from_device(&match_data_local, device); + } + + m = nm_match_spec_device(match_section_infos->match_device.spec, match_data); + match = nm_match_spec_match_type_to_bool(m, FALSE); } else match = TRUE; @@ -1563,9 +1566,8 @@ nm_config_data_get_device_config(const NMConfigData *self, connection_info = _match_section_infos_lookup(&priv->device_infos[0], priv->keyfile, property, + NULL, device, - NULL, - NULL, &value); NM_SET_OUT(has_match, !!connection_info); return value; @@ -1581,18 +1583,23 @@ nm_config_data_get_device_config_by_pllink(const NMConfigData *self, const NMConfigDataPrivate *priv; const MatchSectionInfo *connection_info; const char *value; + NMMatchSpecDeviceData match_data; g_return_val_if_fail(self, NULL); g_return_val_if_fail(property && *property, NULL); priv = NM_CONFIG_DATA_GET_PRIVATE(self); + nm_match_spec_device_data_init_from_platform(&match_data, + pllink, + match_device_type, + nm_dhcp_manager_get_config(nm_dhcp_manager_get())); + connection_info = _match_section_infos_lookup(&priv->device_infos[0], priv->keyfile, property, + &match_data, NULL, - pllink, - match_device_type, &value); NM_SET_OUT(has_match, !!connection_info); return value; @@ -1651,9 +1658,8 @@ nm_config_data_get_device_allowed_connections_specs(const NMConfigData *self, connection_info = _match_section_infos_lookup(&priv->device_infos[0], priv->keyfile, NM_CONFIG_KEYFILE_KEY_DEVICE_ALLOWED_CONNECTIONS, + NULL, device, - NULL, - NULL, NULL); if (connection_info) { @@ -1696,9 +1702,8 @@ nm_config_data_get_connection_default(const NMConfigData *self, _match_section_infos_lookup(&priv->connection_infos[0], priv->keyfile, property, + NULL, device, - NULL, - NULL, &value); return value; } From ac7b6e532f966004a6edf65ab603995db06c4184 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 13:20:13 +0200 Subject: [PATCH 09/15] core: rename nm_config_data_get_device_config_*() variants The fully generic way to lookup a device config is using the NMMatchSpecDeviceData. Rename the accessors that operate on a NMDevice instead. --- src/core/devices/nm-device.c | 54 +++++++++++++------------- src/core/devices/wifi/nm-device-iwd.c | 12 +++--- src/core/devices/wifi/nm-device-wifi.c | 4 +- src/core/nm-config-data.c | 46 +++++++++++----------- src/core/nm-config-data.h | 34 ++++++++-------- src/core/nm-manager.c | 11 +++--- 6 files changed, 82 insertions(+), 79 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 67babb055f..b202638610 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -7520,14 +7520,15 @@ device_init_static_sriov_num_vfs(NMDevice *self) if (priv->ifindex > 0 && nm_device_has_capability(self, NM_DEVICE_CAP_SRIOV)) { int num_vfs; - num_vfs = nm_config_data_get_device_config_int64(NM_CONFIG_GET_DATA, - NM_CONFIG_KEYFILE_KEY_DEVICE_SRIOV_NUM_VFS, - self, - 10, - 0, - G_MAXINT32, - -1, - -1); + num_vfs = nm_config_data_get_device_config_int64_by_device( + NM_CONFIG_GET_DATA, + NM_CONFIG_KEYFILE_KEY_DEVICE_SRIOV_NUM_VFS, + self, + 10, + 0, + G_MAXINT32, + -1, + -1); if (num_vfs >= 0) sriov_op_queue(self, num_vfs, NM_OPTION_BOOL_DEFAULT, NULL, NULL); } @@ -14317,14 +14318,15 @@ nm_device_is_up(NMDevice *self) static gint64 _get_carrier_wait_ms(NMDevice *self) { - return nm_config_data_get_device_config_int64(NM_CONFIG_GET_DATA, - NM_CONFIG_KEYFILE_KEY_DEVICE_CARRIER_WAIT_TIMEOUT, - self, - 10, - 0, - G_MAXINT32, - CARRIER_WAIT_TIME_MS, - CARRIER_WAIT_TIME_MS); + return nm_config_data_get_device_config_int64_by_device( + NM_CONFIG_GET_DATA, + NM_CONFIG_KEYFILE_KEY_DEVICE_CARRIER_WAIT_TIMEOUT, + self, + 10, + 0, + G_MAXINT32, + CARRIER_WAIT_TIME_MS, + CARRIER_WAIT_TIME_MS); } /* @@ -14881,11 +14883,11 @@ nm_device_check_unrealized_device_managed(NMDevice *self) nm_assert(!nm_device_is_real(self)); - if (!nm_config_data_get_device_config_boolean(NM_CONFIG_GET_DATA, - NM_CONFIG_KEYFILE_KEY_DEVICE_MANAGED, - self, - TRUE, - TRUE)) + if (!nm_config_data_get_device_config_boolean_by_device(NM_CONFIG_GET_DATA, + NM_CONFIG_KEYFILE_KEY_DEVICE_MANAGED, + self, + TRUE, + TRUE)) return FALSE; if (nm_device_spec_match_list(self, nm_settings_get_unmanaged_specs(priv->settings))) @@ -14952,11 +14954,11 @@ nm_device_set_unmanaged_by_user_conf(NMDevice *self) gboolean value; NMUnmanFlagOp set_op; - value = nm_config_data_get_device_config_boolean(NM_CONFIG_GET_DATA, - NM_CONFIG_KEYFILE_KEY_DEVICE_MANAGED, - self, - -1, - TRUE); + value = nm_config_data_get_device_config_boolean_by_device(NM_CONFIG_GET_DATA, + NM_CONFIG_KEYFILE_KEY_DEVICE_MANAGED, + self, + -1, + TRUE); switch (value) { case TRUE: set_op = NM_UNMAN_FLAG_OP_SET_MANAGED; diff --git a/src/core/devices/wifi/nm-device-iwd.c b/src/core/devices/wifi/nm-device-iwd.c index 12a827fde3..47407a1e05 100644 --- a/src/core/devices/wifi/nm-device-iwd.c +++ b/src/core/devices/wifi/nm-device-iwd.c @@ -3108,12 +3108,12 @@ config_changed(NMConfig *config, NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE(self); gboolean old_iwd_ac = priv->iwd_autoconnect; - priv->iwd_autoconnect = - nm_config_data_get_device_config_boolean(config_data, - NM_CONFIG_KEYFILE_KEY_DEVICE_WIFI_IWD_AUTOCONNECT, - NM_DEVICE(self), - TRUE, - TRUE); + priv->iwd_autoconnect = nm_config_data_get_device_config_boolean_by_device( + config_data, + NM_CONFIG_KEYFILE_KEY_DEVICE_WIFI_IWD_AUTOCONNECT, + NM_DEVICE(self), + TRUE, + TRUE); if (old_iwd_ac != priv->iwd_autoconnect && priv->dbus_station_proxy && !priv->current_ap) { gs_unref_variant GVariant *value = NULL; diff --git a/src/core/devices/wifi/nm-device-wifi.c b/src/core/devices/wifi/nm-device-wifi.c index 40fdfa9041..4377283458 100644 --- a/src/core/devices/wifi/nm-device-wifi.c +++ b/src/core/devices/wifi/nm-device-wifi.c @@ -1398,7 +1398,7 @@ _hw_addr_set_scanning(NMDeviceWifi *self, gboolean do_reset) priv = NM_DEVICE_WIFI_GET_PRIVATE(self); - randomize = nm_config_data_get_device_config_boolean( + randomize = nm_config_data_get_device_config_boolean_by_device( NM_CONFIG_GET_DATA, NM_CONFIG_KEYFILE_KEY_DEVICE_WIFI_SCAN_RAND_MAC_ADDRESS, device, @@ -1431,7 +1431,7 @@ _hw_addr_set_scanning(NMDeviceWifi *self, gboolean do_reset) * a new one.*/ priv->hw_addr_scan_expire = now + SCAN_RAND_MAC_ADDRESS_EXPIRE_SEC; - generate_mac_address_mask = nm_config_data_get_device_config( + generate_mac_address_mask = nm_config_data_get_device_config_by_device( NM_CONFIG_GET_DATA, NM_CONFIG_KEYFILE_KEY_DEVICE_WIFI_SCAN_GENERATE_MAC_ADDRESS_MASK, device, diff --git a/src/core/nm-config-data.c b/src/core/nm-config-data.c index 62949ccbfb..c59a3331a2 100644 --- a/src/core/nm-config-data.c +++ b/src/core/nm-config-data.c @@ -375,10 +375,10 @@ nm_config_data_get_ignore_carrier(const NMConfigData *self, NMDevice *device) g_return_val_if_fail(NM_IS_CONFIG_DATA(self), FALSE); g_return_val_if_fail(NM_IS_DEVICE(device), FALSE); - value = nm_config_data_get_device_config(self, - NM_CONFIG_KEYFILE_KEY_DEVICE_IGNORE_CARRIER, - device, - &has_match); + value = nm_config_data_get_device_config_by_device(self, + NM_CONFIG_KEYFILE_KEY_DEVICE_IGNORE_CARRIER, + device, + &has_match); if (has_match) m = nm_config_parse_boolean(value, -1); else @@ -1547,10 +1547,10 @@ out: } const char * -nm_config_data_get_device_config(const NMConfigData *self, - const char *property, - NMDevice *device, - gboolean *has_match) +nm_config_data_get_device_config_by_device(const NMConfigData *self, + const char *property, + NMDevice *device, + gboolean *has_match) { const NMConfigDataPrivate *priv; const MatchSectionInfo *connection_info; @@ -1606,35 +1606,35 @@ nm_config_data_get_device_config_by_pllink(const NMConfigData *self, } gboolean -nm_config_data_get_device_config_boolean(const NMConfigData *self, - const char *property, - NMDevice *device, - int val_no_match, - int val_invalid) +nm_config_data_get_device_config_boolean_by_device(const NMConfigData *self, + const char *property, + NMDevice *device, + int val_no_match, + int val_invalid) { const char *value; gboolean has_match; - value = nm_config_data_get_device_config(self, property, device, &has_match); + value = nm_config_data_get_device_config_by_device(self, property, device, &has_match); if (!has_match) return val_no_match; return nm_config_parse_boolean(value, val_invalid); } gint64 -nm_config_data_get_device_config_int64(const NMConfigData *self, - const char *property, - NMDevice *device, - int base, - gint64 min, - gint64 max, - gint64 val_no_match, - gint64 val_invalid) +nm_config_data_get_device_config_int64_by_device(const NMConfigData *self, + const char *property, + NMDevice *device, + int base, + gint64 min, + gint64 max, + gint64 val_no_match, + gint64 val_invalid) { const char *value; gboolean has_match; - value = nm_config_data_get_device_config(self, property, device, &has_match); + value = nm_config_data_get_device_config_by_device(self, property, device, &has_match); if (!has_match) { errno = ENOENT; return val_no_match; diff --git a/src/core/nm-config-data.h b/src/core/nm-config-data.h index e3dc90dd4b..28d9b5c9cb 100644 --- a/src/core/nm-config-data.h +++ b/src/core/nm-config-data.h @@ -219,10 +219,10 @@ gint64 nm_config_data_get_connection_default_int64(const NMConfigData *self, gint64 max, gint64 fallback); -const char *nm_config_data_get_device_config(const NMConfigData *self, - const char *property, - NMDevice *device, - gboolean *has_match); +const char *nm_config_data_get_device_config_by_device(const NMConfigData *self, + const char *property, + NMDevice *device, + gboolean *has_match); const char *nm_config_data_get_device_config_by_pllink(const NMConfigData *self, const char *property, @@ -230,19 +230,19 @@ const char *nm_config_data_get_device_config_by_pllink(const NMConfigData *sel const char *match_device_type, gboolean *has_match); -gboolean nm_config_data_get_device_config_boolean(const NMConfigData *self, - const char *property, - NMDevice *device, - int val_no_match, - int val_invalid); -gint64 nm_config_data_get_device_config_int64(const NMConfigData *self, - const char *property, - NMDevice *device, - int base, - gint64 min, - gint64 max, - gint64 val_no_match, - gint64 val_invalid); +gboolean nm_config_data_get_device_config_boolean_by_device(const NMConfigData *self, + const char *property, + NMDevice *device, + int val_no_match, + int val_invalid); +gint64 nm_config_data_get_device_config_int64_by_device(const NMConfigData *self, + const char *property, + NMDevice *device, + int base, + gint64 min, + gint64 max, + gint64 val_no_match, + gint64 val_invalid); const GSList *nm_config_data_get_device_allowed_connections_specs(const NMConfigData *self, NMDevice *device, diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index 323d7476e3..d49191e5db 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -3379,11 +3379,12 @@ get_existing_connection(NMManager *self, NMDevice *device, gboolean *out_generat } } - if (nm_config_data_get_device_config_boolean(NM_CONFIG_GET_DATA, - NM_CONFIG_KEYFILE_KEY_DEVICE_KEEP_CONFIGURATION, - device, - TRUE, - TRUE)) { + if (nm_config_data_get_device_config_boolean_by_device( + NM_CONFIG_GET_DATA, + NM_CONFIG_KEYFILE_KEY_DEVICE_KEEP_CONFIGURATION, + device, + TRUE, + TRUE)) { /* The core of the API is nm_device_generate_connection() function, based on * update_connection() virtual method and the @connection_type_supported * class attribute. Devices that support assuming existing connections must From 97d6b7e92a781d5ffc4bc81622d37d2771094ad7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 13:37:08 +0200 Subject: [PATCH 10/15] core: add nm_config_data_get_device_config() helper to lookup by match-data --- src/core/nm-config-data.c | 34 ++++++++++++++++++++++++++++------ src/core/nm-config-data.h | 7 +++++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/core/nm-config-data.c b/src/core/nm-config-data.c index c59a3331a2..7b778cc753 100644 --- a/src/core/nm-config-data.c +++ b/src/core/nm-config-data.c @@ -1546,11 +1546,12 @@ out: return NULL; } -const char * -nm_config_data_get_device_config_by_device(const NMConfigData *self, - const char *property, - NMDevice *device, - gboolean *has_match) +static const char * +_config_data_get_device_config(const NMConfigData *self, + const char *property, + const NMMatchSpecDeviceData *match_data, + NMDevice *device, + gboolean *has_match) { const NMConfigDataPrivate *priv; const MatchSectionInfo *connection_info; @@ -1561,18 +1562,39 @@ nm_config_data_get_device_config_by_device(const NMConfigData *self, g_return_val_if_fail(self, NULL); g_return_val_if_fail(property && *property, NULL); + nm_assert(!match_data || !device); + nm_assert(!device || NM_IS_DEVICE(device)); + priv = NM_CONFIG_DATA_GET_PRIVATE(self); connection_info = _match_section_infos_lookup(&priv->device_infos[0], priv->keyfile, property, - NULL, + match_data, device, &value); NM_SET_OUT(has_match, !!connection_info); return value; } +const char * +nm_config_data_get_device_config(const NMConfigData *self, + const char *property, + const NMMatchSpecDeviceData *match_data, + gboolean *has_match) +{ + return _config_data_get_device_config(self, property, match_data, NULL, has_match); +} + +const char * +nm_config_data_get_device_config_by_device(const NMConfigData *self, + const char *property, + NMDevice *device, + gboolean *has_match) +{ + return _config_data_get_device_config(self, property, NULL, device, has_match); +} + const char * nm_config_data_get_device_config_by_pllink(const NMConfigData *self, const char *property, diff --git a/src/core/nm-config-data.h b/src/core/nm-config-data.h index 28d9b5c9cb..8ecf926058 100644 --- a/src/core/nm-config-data.h +++ b/src/core/nm-config-data.h @@ -219,6 +219,13 @@ gint64 nm_config_data_get_connection_default_int64(const NMConfigData *self, gint64 max, gint64 fallback); +struct _NMMatchSpecDeviceData; + +const char *nm_config_data_get_device_config(const NMConfigData *self, + const char *property, + const struct _NMMatchSpecDeviceData *match_data, + gboolean *has_match); + const char *nm_config_data_get_device_config_by_device(const NMConfigData *self, const char *property, NMDevice *device, From 13690f445aff140b44c4c7f83ae5eb78e3f547fb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 16:35:39 +0200 Subject: [PATCH 11/15] core: rename nm_config_data_get_ignore_carrier() to nm_config_data_get_ignore_carrier_by_device() --- src/core/devices/nm-device.c | 10 ++++++---- src/core/nm-config-data.c | 2 +- src/core/nm-config-data.h | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index b202638610..a03de14bca 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -7544,7 +7544,7 @@ config_changed(NMConfig *config, NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); if (priv->state <= NM_DEVICE_STATE_DISCONNECTED || priv->state >= NM_DEVICE_STATE_ACTIVATED) { - priv->ignore_carrier = nm_config_data_get_ignore_carrier(config_data, self); + priv->ignore_carrier = nm_config_data_get_ignore_carrier_by_device(config_data, self); if (NM_FLAGS_HAS(changes, NM_CONFIG_CHANGE_VALUES) && !nm_device_get_applied_setting(self, NM_TYPE_SETTING_SRIOV)) device_init_static_sriov_num_vfs(self); @@ -7683,8 +7683,9 @@ realize_start_setup(NMDevice *self, nm_device_update_permanent_hw_address(self, FALSE); /* Note: initial hardware address must be read before calling get_ignore_carrier() */ - config = nm_config_get(); - priv->ignore_carrier = nm_config_data_get_ignore_carrier(nm_config_get_data(config), self); + config = nm_config_get(); + priv->ignore_carrier = + nm_config_data_get_ignore_carrier_by_device(nm_config_get_data(config), self); if (!priv->config_changed_id) { priv->config_changed_id = g_signal_connect(config, NM_CONFIG_SIGNAL_CONFIG_CHANGED, @@ -16303,7 +16304,8 @@ _set_state_full(NMDevice *self, NMDeviceState state, NMDeviceStateReason reason, /* We cache the ignore_carrier state to not react on config-reloads while the connection * is active. But on deactivating, reset the ignore-carrier flag to the current state. */ - priv->ignore_carrier = nm_config_data_get_ignore_carrier(NM_CONFIG_GET_DATA, self); + priv->ignore_carrier = + nm_config_data_get_ignore_carrier_by_device(NM_CONFIG_GET_DATA, self); if (quitting) { nm_dispatcher_call_device_sync(NM_DISPATCHER_ACTION_PRE_DOWN, self, req); diff --git a/src/core/nm-config-data.c b/src/core/nm-config-data.c index 7b778cc753..2fa49ae7aa 100644 --- a/src/core/nm-config-data.c +++ b/src/core/nm-config-data.c @@ -366,7 +366,7 @@ nm_config_data_get_iwd_config_path(const NMConfigData *self) } gboolean -nm_config_data_get_ignore_carrier(const NMConfigData *self, NMDevice *device) +nm_config_data_get_ignore_carrier_by_device(const NMConfigData *self, NMDevice *device) { const char *value; gboolean has_match; diff --git a/src/core/nm-config-data.h b/src/core/nm-config-data.h index 8ecf926058..acc968d69a 100644 --- a/src/core/nm-config-data.h +++ b/src/core/nm-config-data.h @@ -185,7 +185,7 @@ const char *nm_config_data_get_dns_mode(const NMConfigData *self); const char *nm_config_data_get_rc_manager(const NMConfigData *self); gboolean nm_config_data_get_systemd_resolved(const NMConfigData *self); -gboolean nm_config_data_get_ignore_carrier(const NMConfigData *self, NMDevice *device); +gboolean nm_config_data_get_ignore_carrier_by_device(const NMConfigData *self, NMDevice *device); gboolean nm_config_data_get_assume_ipv6ll_only(const NMConfigData *self, NMDevice *device); int nm_config_data_get_sriov_num_vfs(const NMConfigData *self, NMDevice *device); From 2b09512481d6553631d6c9f43d84e9accaa3ebe1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 16:34:40 +0200 Subject: [PATCH 12/15] core: add nm_config_data_get_ignore_carrier_for_port() helper Will be used later. --- src/core/nm-config-data.c | 54 +++++++++++++++++++++++++++++++++++++++ src/core/nm-config-data.h | 4 +++ 2 files changed, 58 insertions(+) diff --git a/src/core/nm-config-data.c b/src/core/nm-config-data.c index 2fa49ae7aa..fe21308005 100644 --- a/src/core/nm-config-data.c +++ b/src/core/nm-config-data.c @@ -130,6 +130,12 @@ G_DEFINE_TYPE(NMConfigData, nm_config_data, G_TYPE_OBJECT) static const char * _match_section_info_get_str(const MatchSectionInfo *m, GKeyFile *keyfile, const char *property); +static const char *_config_data_get_device_config(const NMConfigData *self, + const char *property, + const NMMatchSpecDeviceData *match_data, + NMDevice *device, + gboolean *has_match); + /*****************************************************************************/ const char * @@ -365,6 +371,54 @@ nm_config_data_get_iwd_config_path(const NMConfigData *self) return NM_CONFIG_DATA_GET_PRIVATE(self)->iwd_config_path; } +gboolean +nm_config_data_get_ignore_carrier_for_port(const NMConfigData *self, + const char *master, + const char *slave_type) +{ + const char *value; + gboolean has_match; + int m; + NMMatchSpecDeviceData match_data; + + g_return_val_if_fail(NM_IS_CONFIG_DATA(self), FALSE); + + if (!master || !slave_type) + goto out_default; + + if (!nm_utils_ifname_valid_kernel(master, NULL)) + goto out_default; + + match_data = (NMMatchSpecDeviceData){ + .interface_name = master, + .device_type = slave_type, + }; + + value = _config_data_get_device_config(self, + NM_CONFIG_KEYFILE_KEY_DEVICE_IGNORE_CARRIER, + &match_data, + NULL, + &has_match); + if (has_match) + m = nm_config_parse_boolean(value, -1); + else { + NMMatchSpecMatchType x; + + x = nm_match_spec_device(NM_CONFIG_DATA_GET_PRIVATE(self)->ignore_carrier, &match_data); + m = nm_match_spec_match_type_to_bool(x, -1); + } + + if (NM_IN_SET(m, TRUE, FALSE)) + return m; + +out_default: + /* if ignore-carrier is not explicitly or detected for the master, then we assume it's + * enabled. This is in line with nm_config_data_get_ignore_carrier_by_device(), where + * ignore-carrier is enabled based on nm_device_ignore_carrier_by_default(). + */ + return TRUE; +} + gboolean nm_config_data_get_ignore_carrier_by_device(const NMConfigData *self, NMDevice *device) { diff --git a/src/core/nm-config-data.h b/src/core/nm-config-data.h index acc968d69a..9e7a50fc24 100644 --- a/src/core/nm-config-data.h +++ b/src/core/nm-config-data.h @@ -185,6 +185,10 @@ const char *nm_config_data_get_dns_mode(const NMConfigData *self); const char *nm_config_data_get_rc_manager(const NMConfigData *self); gboolean nm_config_data_get_systemd_resolved(const NMConfigData *self); +gboolean nm_config_data_get_ignore_carrier_for_port(const NMConfigData *self, + const char *master, + const char *slave_type); + gboolean nm_config_data_get_ignore_carrier_by_device(const NMConfigData *self, NMDevice *device); gboolean nm_config_data_get_assume_ipv6ll_only(const NMConfigData *self, NMDevice *device); int nm_config_data_get_sriov_num_vfs(const NMConfigData *self, NMDevice *device); From 44076802a9093e439d16aa73f092a0bc6ed70ca5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 13 Jun 2023 09:31:35 +0200 Subject: [PATCH 13/15] core: various minor code cleanups around find_master() --- src/core/nm-manager.c | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index d49191e5db..63645227ee 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -2575,9 +2575,10 @@ system_create_virtual_device(NMManager *self, NMConnection *connection) guint i; gs_free char *iface = NULL; const char *parent_spec; - NMDevice *device = NULL, *parent = NULL; + NMDevice *device = NULL; + NMDevice *parent = NULL; NMDevice *dev_candidate; - GError *error = NULL; + gs_free_error GError *error = NULL; NMLogLevel log_level; g_return_val_if_fail(NM_IS_MANAGER(self), NULL); @@ -2586,7 +2587,6 @@ system_create_virtual_device(NMManager *self, NMConnection *connection) iface = nm_manager_get_connection_iface(self, connection, &parent, &parent_spec, &error); if (!iface) { _LOG3D(LOGD_DEVICE, connection, "can't get a name of a virtual device: %s", error->message); - g_error_free(error); return NULL; } @@ -2624,7 +2624,6 @@ system_create_virtual_device(NMManager *self, NMConnection *connection) device = nm_device_factory_create_device(factory, iface, NULL, connection, NULL, &error); if (!device) { _LOG3W(LOGD_DEVICE, connection, "factory can't create the device: %s", error->message); - g_error_free(error); return NULL; } @@ -2635,7 +2634,6 @@ system_create_virtual_device(NMManager *self, NMConnection *connection) connection, "can't register the device with manager: %s", error->message); - g_error_free(error); g_object_unref(device); return NULL; } @@ -2656,7 +2654,6 @@ system_create_virtual_device(NMManager *self, NMConnection *connection) if (!find_master(self, connection, device, NULL, NULL, NULL, &error)) { _LOG3D(LOGD_DEVICE, connection, "skip activation: %s", error->message); - g_error_free(error); return device; } @@ -2670,7 +2667,6 @@ system_create_virtual_device(NMManager *self, NMConnection *connection) continue; s_con = nm_connection_get_setting_connection(candidate); - g_assert(s_con); if (!nm_setting_connection_get_autoconnect(s_con) || nm_settings_connection_autoconnect_is_blocked(connections[i])) continue; @@ -2686,7 +2682,6 @@ system_create_virtual_device(NMManager *self, NMConnection *connection) connection, "couldn't create the device: %s", error->message); - g_error_free(error); return NULL; } @@ -4898,8 +4893,11 @@ find_master(NMManager *self, NMSettingsConnection *const *connections; guint i; - s_con = nm_connection_get_setting_connection(connection); - g_assert(s_con); + nm_assert(!out_master_connection || !*out_master_connection); + nm_assert(!out_master_device || !*out_master_device); + nm_assert(!out_master_ac || !*out_master_ac); + + s_con = nm_connection_get_setting_connection(connection); master = nm_setting_connection_get_master(s_con); if (master == NULL) @@ -5007,10 +5005,8 @@ find_master(NMManager *self, nm_device_get_iface(master_device)); } - if (out_master_connection) - *out_master_connection = master_connection; - if (out_master_device) - *out_master_device = master_device; + NM_SET_OUT(out_master_connection, master_connection); + NM_SET_OUT(out_master_device, master_device); if (out_master_ac && master_connection) { *out_master_ac = active_connection_find(self, master_connection, @@ -5020,15 +5016,15 @@ find_master(NMManager *self, NULL); } - if (master_device || master_connection) - return TRUE; - else { + if (!master_device && !master_connection) { g_set_error_literal(error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_UNKNOWN_DEVICE, "Master connection not found or invalid"); return FALSE; } + + return TRUE; } /** @@ -5273,8 +5269,9 @@ find_slaves(NMManager *manager, &n_all_connections); for (i = 0; i < n_all_connections; i++) { NMSettingsConnection *master_connection = NULL; - NMDevice *master_device = NULL, *slave_device; - NMSettingsConnection *candidate = all_connections[i]; + NMDevice *master_device = NULL; + NMDevice *slave_device; + NMSettingsConnection *candidate = all_connections[i]; find_master(manager, nm_settings_connection_get_connection(candidate), @@ -5607,7 +5604,8 @@ active_connection_parent_active(NMActiveConnection *active, static gboolean _internal_activate_device(NMManager *self, NMActiveConnection *active, GError **error) { - NMDevice *device, *master_device = NULL; + NMDevice *device; + NMDevice *master_device = NULL; NMConnection *applied; NMSettingsConnection *sett_conn; NMSettingsConnection *master_connection = NULL; From 6d75b7f348e6fc62287e1f7f90650cbf094c05f3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 13 Jun 2023 11:23:15 +0200 Subject: [PATCH 14/15] core: reorder return in find_master() It feels ugly to set the out arguments, in case we are failing the function. Note that there is no change in behavior here. This is purely cosmetic. --- src/core/nm-manager.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index 63645227ee..d724f84eff 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -5005,6 +5005,14 @@ find_master(NMManager *self, nm_device_get_iface(master_device)); } + if (!master_device && !master_connection) { + g_set_error_literal(error, + NM_MANAGER_ERROR, + NM_MANAGER_ERROR_UNKNOWN_DEVICE, + "Master connection not found or invalid"); + return FALSE; + } + NM_SET_OUT(out_master_connection, master_connection); NM_SET_OUT(out_master_device, master_device); if (out_master_ac && master_connection) { @@ -5016,14 +5024,6 @@ find_master(NMManager *self, NULL); } - if (!master_device && !master_connection) { - g_set_error_literal(error, - NM_MANAGER_ERROR, - NM_MANAGER_ERROR_UNKNOWN_DEVICE, - "Master connection not found or invalid"); - return FALSE; - } - return TRUE; } From bd3162ad891547188589fd3019b18a9e7f3a9c71 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 13 Jun 2023 17:08:36 +0200 Subject: [PATCH 15/15] core: allow resetting blocked reason for a device in nm_manager_devcon_autoconnect_blocked_reason_set() --- src/core/nm-manager.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index d724f84eff..3eee169cee 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -1523,10 +1523,31 @@ nm_manager_devcon_autoconnect_blocked_reason_set(NMManager gboolean changed = FALSE; char buf[100]; - nm_assert(NM_IS_SETTINGS_CONNECTION(sett_conn)); + nm_assert(!sett_conn || NM_IS_SETTINGS_CONNECTION(sett_conn)); + nm_assert(!device || NM_IS_DEVICE(device)); nm_assert(value != NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NONE); nm_assert(!NM_FLAGS_ANY(value, ~(NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_FAILED))); + if (!sett_conn) { + if (!device) + g_return_val_if_reached(FALSE); + c_list_for_each_entry (data, &device->devcon_dev_lst_head, dev_lst) { + v = data->autoconnect.blocked_reason; + v = NM_FLAGS_ASSIGN(v, value, set); + + if (data->autoconnect.blocked_reason == v) + continue; + + _LOGT(LOGD_SETTINGS, + "block-autoconnect: " DEV_CON_DATA_LOG_FMT ": set blocked reason %s", + DEV_CON_DATA_LOG_ARGS_DATA(data), + nm_settings_autoconnect_blocked_reason_to_string(v, buf, sizeof(buf))); + data->autoconnect.blocked_reason = v; + changed = TRUE; + } + return changed; + } + if (device) { data = _devcon_lookup_data(self, device, sett_conn, TRUE, TRUE); v = data->autoconnect.blocked_reason;