From 93773febb594c455f7adc390c64ef040e670a1f0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 15:14:11 +0200 Subject: [PATCH 01/14] 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. (cherry picked from commit 42f20a4edf88cc3a705be4786dab2fe85d35f946) --- 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 ef00a4b26f..83d4c281aa 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -15087,10 +15087,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) @@ -15102,12 +15099,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), @@ -15117,6 +15108,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 1ebf60d8ff927b7d43b033b776b803a6b31e3276 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 16:30:08 +0200 Subject: [PATCH 02/14] device: define auto variables on separate lines in connection_requires_carrier() (cherry picked from commit 5b8e6c01a96c0cc29f7f933a195d75e4a5517919) --- 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 83d4c281aa..dab79457ff 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -10678,10 +10678,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 3d3dd128f9493e39a47412ff0cbcd79672c489f3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Jun 2023 14:35:46 +0200 Subject: [PATCH 03/14] 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. (cherry picked from commit cba8eb978492f3cc83cb8a854d526414851dc5e3) --- 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 6f4c60f876..5744a87e2c 100644 --- a/src/core/NetworkManagerUtils.c +++ b/src/core/NetworkManagerUtils.c @@ -918,17 +918,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 67c9cba471..676215f38c 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 dab79457ff..5d18289b52 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -17059,16 +17059,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 28d9a788d6..7bba770f84 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -1484,6 +1484,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 bc93649630..a57786479f 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 2628c11a17a57276cb50488ecbb5f2b6cf41a470 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Jun 2023 17:01:01 +0200 Subject: [PATCH 04/14] 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. (cherry picked from commit c47d6b17d580fa1466c5aa89e1c7c4717a676501) --- 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 5744a87e2c..42ed16d5f5 100644 --- a/src/core/NetworkManagerUtils.c +++ b/src/core/NetworkManagerUtils.c @@ -911,13 +911,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 5d18289b52..2413d1e508 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -17050,14 +17050,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 7bba770f84..69ae6ac604 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -1154,25 +1154,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, @@ -1240,22 +1241,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)) @@ -1265,15 +1269,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)) @@ -1281,7 +1286,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); @@ -1336,7 +1341,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; @@ -1359,10 +1364,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; @@ -1408,7 +1413,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; } @@ -1416,42 +1422,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 a57786479f..3d3b1260ff 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 887803bffe..dcea4453a9 100644 --- a/src/core/tests/test-core.c +++ b/src/core/tests/test-core.c @@ -1243,13 +1243,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; @@ -1259,9 +1255,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 cb701c6a5ddc026e5f2db799c60ae9f6031ae384 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 12:23:11 +0200 Subject: [PATCH 05/14] device: add nm_device_get_s390_subchannels() accessor (cherry picked from commit dbb45f14d325369cffc9c05fa4b57ca47c0eed09) --- 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 2413d1e508..170b9676b8 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -4952,6 +4952,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 ac843a39cb..37c2d9093d 100644 --- a/src/core/devices/nm-device.h +++ b/src/core/devices/nm-device.h @@ -450,6 +450,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 75f240704b4ddd940d2d951cb4429684b0e21d12 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 12:11:54 +0200 Subject: [PATCH 06/14] device: add nm_match_spec_device_data_init_from_device() helper Will be used later. (cherry picked from commit 798ea93c459b329f654d535eceecceab42e4709e) --- 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 42ed16d5f5..ad67671104 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" /*****************************************************************************/ @@ -896,6 +897,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 676215f38c..d7144aeb52 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 170b9676b8..0db60d2fac 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -17049,32 +17049,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 8ea3f77a3bd5330edf213c1f2bf97f5bfe497f63 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 12:32:46 +0200 Subject: [PATCH 07/14] core: add nm_match_spec_device_data_init_from_platform() helper (cherry picked from commit ccaecf7f3e7693721b85690ced808925884b689e) --- 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 ad67671104..db9f8199db 100644 --- a/src/core/NetworkManagerUtils.c +++ b/src/core/NetworkManagerUtils.c @@ -936,6 +936,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 @@ -945,23 +971,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 d7144aeb52..0f596445b9 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 da77cca27fcfa632689ef5b601cd08c95eb796f7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 12:43:03 +0200 Subject: [PATCH 08/14] 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. (cherry picked from commit 52518066515321340a15224ca478429d14767d5f) --- 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 af41e2a41b54af3b870e3571eb5a33a0b43eb5da Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 13:20:13 +0200 Subject: [PATCH 09/14] 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. (cherry picked from commit ac7b6e532f966004a6edf65ab603995db06c4184) --- 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 0db60d2fac..fa72d1def9 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -7315,14 +7315,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); } @@ -14092,14 +14093,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); } /* @@ -14647,11 +14649,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))) @@ -14718,11 +14720,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 6bb420354f..5e76197e7a 100644 --- a/src/core/devices/wifi/nm-device-iwd.c +++ b/src/core/devices/wifi/nm-device-iwd.c @@ -3104,12 +3104,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 9012a59bef..30de0eaf8c 100644 --- a/src/core/devices/wifi/nm-device-wifi.c +++ b/src/core/devices/wifi/nm-device-wifi.c @@ -1395,7 +1395,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, @@ -1428,7 +1428,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 0fca6de16f..89571e5858 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -2892,11 +2892,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 91c23e2a3d3880b5e2e5092bf6c4dc4df0fe9ba4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 13:37:08 +0200 Subject: [PATCH 10/14] core: add nm_config_data_get_device_config() helper to lookup by match-data (cherry picked from commit 97d6b7e92a781d5ffc4bc81622d37d2771094ad7) --- 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 1648ae58a32931948e489abc03489d26f6605ac8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 16:35:39 +0200 Subject: [PATCH 11/14] core: rename nm_config_data_get_ignore_carrier() to nm_config_data_get_ignore_carrier_by_device() (cherry picked from commit 13690f445aff140b44c4c7f83ae5eb78e3f547fb) --- 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 fa72d1def9..ed1f67aded 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -7339,7 +7339,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); @@ -7478,8 +7478,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, @@ -16059,7 +16060,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 f81b6a6994b6afa57e3ea5a6c809ba09a485c48b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Jun 2023 16:34:40 +0200 Subject: [PATCH 12/14] core: add nm_config_data_get_ignore_carrier_for_port() helper Will be used later. (cherry picked from commit 2b09512481d6553631d6c9f43d84e9accaa3ebe1) --- 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 8e0a61d4e22b873a8cfa420793bb2ccbbc5fb429 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 13 Jun 2023 09:31:35 +0200 Subject: [PATCH 13/14] core: various minor code cleanups around find_master() (cherry picked from commit 44076802a9093e439d16aa73f092a0bc6ed70ca5) --- 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 89571e5858..4c6f7eaf75 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -2088,9 +2088,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); @@ -2099,7 +2100,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; } @@ -2137,7 +2137,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; } @@ -2148,7 +2147,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; } @@ -2169,7 +2167,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; } @@ -2183,7 +2180,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; @@ -2199,7 +2195,6 @@ system_create_virtual_device(NMManager *self, NMConnection *connection) connection, "couldn't create the device: %s", error->message); - g_error_free(error); return NULL; } @@ -4409,8 +4404,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) @@ -4518,10 +4516,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, @@ -4531,15 +4527,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; } /** @@ -4784,8 +4780,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), @@ -5118,7 +5115,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 16263015cb711e64baccdf642836641d67b46f9f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 13 Jun 2023 11:23:15 +0200 Subject: [PATCH 14/14] 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. (cherry picked from commit 6d75b7f348e6fc62287e1f7f90650cbf094c05f3) --- 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 4c6f7eaf75..55fca5ae27 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -4516,6 +4516,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) { @@ -4527,14 +4535,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; }