From 16b01233fa7defb66e9edb96b87ff287936c2f42 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 11 Jun 2021 15:28:42 +0200 Subject: [PATCH 01/22] libnm: add nm_meta_setting_info helpers --- src/libnm-core-impl/nm-connection.c | 15 +++++++ src/libnm-core-impl/nm-setting-private.h | 54 ++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c index 35a14a80b8..9672dcc252 100644 --- a/src/libnm-core-impl/nm-connection.c +++ b/src/libnm-core-impl/nm-connection.c @@ -2426,6 +2426,21 @@ _get_settings_sort(gconstpointer p_a, gconstpointer p_b, gpointer unused) return 0; } +int +_nmtst_nm_setting_sort(NMSetting *a, NMSetting *b) +{ + g_assert(NM_IS_SETTING(a)); + g_assert(NM_IS_SETTING(b)); + g_assert(a != b); + g_assert(G_OBJECT_TYPE(a) != G_OBJECT_TYPE(b)); + + NM_CMP_RETURN(_nm_setting_compare_priority(a, b)); + NM_CMP_DIRECT_STRCMP(nm_setting_get_name(a), nm_setting_get_name(b)); + + g_assert_not_reached(); + return 0; +} + /** * nm_connection_get_settings: * @connection: the #NMConnection instance diff --git a/src/libnm-core-impl/nm-setting-private.h b/src/libnm-core-impl/nm-setting-private.h index b0abb28df5..339921e65e 100644 --- a/src/libnm-core-impl/nm-setting-private.h +++ b/src/libnm-core-impl/nm-setting-private.h @@ -132,6 +132,60 @@ struct _NMSettingIPConfigClass { NMSettingPriority _nm_setting_get_base_type_priority(NMSetting *setting); int _nm_setting_compare_priority(gconstpointer a, gconstpointer b); +int _nmtst_nm_setting_sort(NMSetting *a, NMSetting *b); + +/*****************************************************************************/ + +#define _nm_assert_setting_info(setting_info, gtype) \ + G_STMT_START \ + { \ + const NMMetaSettingInfo *_setting_info = (setting_info); \ + \ + if (NM_MORE_ASSERTS > 0) { \ + GType _gtype = (gtype); \ + \ + nm_assert(_setting_info); \ + nm_assert(_NM_INT_NOT_NEGATIVE(_setting_info->meta_type)); \ + nm_assert(_setting_info->meta_type < _NM_META_SETTING_TYPE_NUM); \ + nm_assert(_setting_info->get_setting_gtype); \ + if (_gtype != 0) \ + nm_assert(_setting_info->get_setting_gtype() == _gtype); \ + else \ + _gtype = _setting_info->get_setting_gtype(); \ + nm_assert(g_type_is_a(_gtype, NM_TYPE_SETTING)); \ + } \ + } \ + G_STMT_END + +static inline const NMMetaSettingInfo * +_nm_meta_setting_info_from_class(NMSettingClass *klass) +{ + const NMMetaSettingInfo *setting_info; + + if (!NM_IS_SETTING_CLASS(klass)) + return NULL; + + setting_info = klass->setting_info; + if (!setting_info) + return NULL; + + _nm_assert_setting_info(setting_info, G_OBJECT_CLASS_TYPE(klass)); + return setting_info; +} + +static inline const NMMetaSettingInfo * +_nm_meta_setting_info_from_gtype(GType gtype) +{ + const NMMetaSettingInfo *setting_info; + + setting_info = nm_meta_setting_infos_by_gtype(gtype); + if (!setting_info) + return NULL; + + _nm_assert_setting_info(setting_info, gtype); + return setting_info; +} + /*****************************************************************************/ void _nm_setting_emit_property_changed(NMSetting *setting); From 1a5a4838f1bbfac62876c8d0e40470a87bd1a775 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 11 Jun 2021 15:26:19 +0200 Subject: [PATCH 02/22] libnm: pack NMMetaSettingType enum We keep the enum around in memory, so let's make it smaller/packed. --- src/libnm-core-intern/nm-meta-setting-base-impl.h | 2 +- src/libnmc-setting/nm-meta-setting-base-impl.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libnm-core-intern/nm-meta-setting-base-impl.h b/src/libnm-core-intern/nm-meta-setting-base-impl.h index 94b14e844f..5e05ff31e2 100644 --- a/src/libnm-core-intern/nm-meta-setting-base-impl.h +++ b/src/libnm-core-intern/nm-meta-setting-base-impl.h @@ -94,7 +94,7 @@ extern const NMSetting8021xSchemeVtable /*****************************************************************************/ -typedef enum { +typedef enum _nm_packed { /* the enum (and their numeric values) are internal API. Do not assign * any meaning the numeric values, because they already have one: * diff --git a/src/libnmc-setting/nm-meta-setting-base-impl.h b/src/libnmc-setting/nm-meta-setting-base-impl.h index 94b14e844f..5e05ff31e2 100644 --- a/src/libnmc-setting/nm-meta-setting-base-impl.h +++ b/src/libnmc-setting/nm-meta-setting-base-impl.h @@ -94,7 +94,7 @@ extern const NMSetting8021xSchemeVtable /*****************************************************************************/ -typedef enum { +typedef enum _nm_packed { /* the enum (and their numeric values) are internal API. Do not assign * any meaning the numeric values, because they already have one: * From b7a7cc1b13d4a455a81995fe9b1f17f4da99d3e3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 11 Jun 2021 15:28:09 +0200 Subject: [PATCH 03/22] libnm: add nm_meta_setting_types_by_priority array for sorting settings nm_meta_setting_infos is a list of all NMMetaSettingInfo, sorted by name. Add nm_meta_setting_types_by_priority which provides a mapping with a different sort order (first by priority). We need that sometimes. --- .../nm-meta-setting-base-impl.c | 68 +++++++++++++++++++ .../nm-meta-setting-base-impl.h | 2 + .../nm-meta-setting-base-impl.c | 68 +++++++++++++++++++ .../nm-meta-setting-base-impl.h | 2 + 4 files changed, 140 insertions(+) diff --git a/src/libnm-core-impl/nm-meta-setting-base-impl.c b/src/libnm-core-impl/nm-meta-setting-base-impl.c index e049e2339d..a952ced9aa 100644 --- a/src/libnm-core-impl/nm-meta-setting-base-impl.c +++ b/src/libnm-core-impl/nm-meta-setting-base-impl.c @@ -519,6 +519,74 @@ const NMMetaSettingInfo nm_meta_setting_infos[] = { }, }; +const NMMetaSettingType nm_meta_setting_types_by_priority[] = { + + /* NM_SETTING_PRIORITY_CONNECTION */ + NM_META_SETTING_TYPE_CONNECTION, + + /* NM_SETTING_PRIORITY_HW_BASE */ + NM_META_SETTING_TYPE_6LOWPAN, + NM_META_SETTING_TYPE_OLPC_MESH, + NM_META_SETTING_TYPE_WIRELESS, + NM_META_SETTING_TYPE_WIRED, + NM_META_SETTING_TYPE_ADSL, + NM_META_SETTING_TYPE_BOND, + NM_META_SETTING_TYPE_BRIDGE, + NM_META_SETTING_TYPE_CDMA, + NM_META_SETTING_TYPE_DUMMY, + NM_META_SETTING_TYPE_GENERIC, + NM_META_SETTING_TYPE_GSM, + NM_META_SETTING_TYPE_INFINIBAND, + NM_META_SETTING_TYPE_IP_TUNNEL, + NM_META_SETTING_TYPE_MACSEC, + NM_META_SETTING_TYPE_MACVLAN, + NM_META_SETTING_TYPE_OVS_BRIDGE, + NM_META_SETTING_TYPE_OVS_DPDK, + NM_META_SETTING_TYPE_OVS_INTERFACE, + NM_META_SETTING_TYPE_OVS_PATCH, + NM_META_SETTING_TYPE_OVS_PORT, + NM_META_SETTING_TYPE_TEAM, + NM_META_SETTING_TYPE_TUN, + NM_META_SETTING_TYPE_VETH, + NM_META_SETTING_TYPE_VLAN, + NM_META_SETTING_TYPE_VPN, + NM_META_SETTING_TYPE_VRF, + NM_META_SETTING_TYPE_VXLAN, + NM_META_SETTING_TYPE_WIFI_P2P, + NM_META_SETTING_TYPE_WIMAX, + NM_META_SETTING_TYPE_WIREGUARD, + NM_META_SETTING_TYPE_WPAN, + + /* NM_SETTING_PRIORITY_HW_NON_BASE */ + NM_META_SETTING_TYPE_BLUETOOTH, + + /* NM_SETTING_PRIORITY_HW_AUX */ + NM_META_SETTING_TYPE_WIRELESS_SECURITY, + NM_META_SETTING_TYPE_802_1X, + NM_META_SETTING_TYPE_DCB, + NM_META_SETTING_TYPE_SERIAL, + NM_META_SETTING_TYPE_SRIOV, + + /* NM_SETTING_PRIORITY_AUX */ + NM_META_SETTING_TYPE_BRIDGE_PORT, + NM_META_SETTING_TYPE_ETHTOOL, + NM_META_SETTING_TYPE_MATCH, + NM_META_SETTING_TYPE_OVS_EXTERNAL_IDS, + NM_META_SETTING_TYPE_PPP, + NM_META_SETTING_TYPE_PPPOE, + NM_META_SETTING_TYPE_TEAM_PORT, + + /* NM_SETTING_PRIORITY_IP */ + NM_META_SETTING_TYPE_HOSTNAME, + NM_META_SETTING_TYPE_IP4_CONFIG, + NM_META_SETTING_TYPE_IP6_CONFIG, + NM_META_SETTING_TYPE_PROXY, + NM_META_SETTING_TYPE_TC_CONFIG, + + /* NM_SETTING_PRIORITY_USER */ + NM_META_SETTING_TYPE_USER, +}; + const NMMetaSettingInfo * nm_meta_setting_infos_by_name(const char *name) { diff --git a/src/libnm-core-intern/nm-meta-setting-base-impl.h b/src/libnm-core-intern/nm-meta-setting-base-impl.h index 5e05ff31e2..44bd9024c2 100644 --- a/src/libnm-core-intern/nm-meta-setting-base-impl.h +++ b/src/libnm-core-intern/nm-meta-setting-base-impl.h @@ -175,6 +175,8 @@ typedef struct _NMMetaSettingInfo_Alias NMMetaSettingInfo; extern const NMMetaSettingInfo nm_meta_setting_infos[_NM_META_SETTING_TYPE_NUM + 1]; +extern const NMMetaSettingType nm_meta_setting_types_by_priority[_NM_META_SETTING_TYPE_NUM]; + const NMMetaSettingInfo *nm_meta_setting_infos_by_name(const char *name); const NMMetaSettingInfo *nm_meta_setting_infos_by_gtype(GType gtype); diff --git a/src/libnmc-setting/nm-meta-setting-base-impl.c b/src/libnmc-setting/nm-meta-setting-base-impl.c index e049e2339d..a952ced9aa 100644 --- a/src/libnmc-setting/nm-meta-setting-base-impl.c +++ b/src/libnmc-setting/nm-meta-setting-base-impl.c @@ -519,6 +519,74 @@ const NMMetaSettingInfo nm_meta_setting_infos[] = { }, }; +const NMMetaSettingType nm_meta_setting_types_by_priority[] = { + + /* NM_SETTING_PRIORITY_CONNECTION */ + NM_META_SETTING_TYPE_CONNECTION, + + /* NM_SETTING_PRIORITY_HW_BASE */ + NM_META_SETTING_TYPE_6LOWPAN, + NM_META_SETTING_TYPE_OLPC_MESH, + NM_META_SETTING_TYPE_WIRELESS, + NM_META_SETTING_TYPE_WIRED, + NM_META_SETTING_TYPE_ADSL, + NM_META_SETTING_TYPE_BOND, + NM_META_SETTING_TYPE_BRIDGE, + NM_META_SETTING_TYPE_CDMA, + NM_META_SETTING_TYPE_DUMMY, + NM_META_SETTING_TYPE_GENERIC, + NM_META_SETTING_TYPE_GSM, + NM_META_SETTING_TYPE_INFINIBAND, + NM_META_SETTING_TYPE_IP_TUNNEL, + NM_META_SETTING_TYPE_MACSEC, + NM_META_SETTING_TYPE_MACVLAN, + NM_META_SETTING_TYPE_OVS_BRIDGE, + NM_META_SETTING_TYPE_OVS_DPDK, + NM_META_SETTING_TYPE_OVS_INTERFACE, + NM_META_SETTING_TYPE_OVS_PATCH, + NM_META_SETTING_TYPE_OVS_PORT, + NM_META_SETTING_TYPE_TEAM, + NM_META_SETTING_TYPE_TUN, + NM_META_SETTING_TYPE_VETH, + NM_META_SETTING_TYPE_VLAN, + NM_META_SETTING_TYPE_VPN, + NM_META_SETTING_TYPE_VRF, + NM_META_SETTING_TYPE_VXLAN, + NM_META_SETTING_TYPE_WIFI_P2P, + NM_META_SETTING_TYPE_WIMAX, + NM_META_SETTING_TYPE_WIREGUARD, + NM_META_SETTING_TYPE_WPAN, + + /* NM_SETTING_PRIORITY_HW_NON_BASE */ + NM_META_SETTING_TYPE_BLUETOOTH, + + /* NM_SETTING_PRIORITY_HW_AUX */ + NM_META_SETTING_TYPE_WIRELESS_SECURITY, + NM_META_SETTING_TYPE_802_1X, + NM_META_SETTING_TYPE_DCB, + NM_META_SETTING_TYPE_SERIAL, + NM_META_SETTING_TYPE_SRIOV, + + /* NM_SETTING_PRIORITY_AUX */ + NM_META_SETTING_TYPE_BRIDGE_PORT, + NM_META_SETTING_TYPE_ETHTOOL, + NM_META_SETTING_TYPE_MATCH, + NM_META_SETTING_TYPE_OVS_EXTERNAL_IDS, + NM_META_SETTING_TYPE_PPP, + NM_META_SETTING_TYPE_PPPOE, + NM_META_SETTING_TYPE_TEAM_PORT, + + /* NM_SETTING_PRIORITY_IP */ + NM_META_SETTING_TYPE_HOSTNAME, + NM_META_SETTING_TYPE_IP4_CONFIG, + NM_META_SETTING_TYPE_IP6_CONFIG, + NM_META_SETTING_TYPE_PROXY, + NM_META_SETTING_TYPE_TC_CONFIG, + + /* NM_SETTING_PRIORITY_USER */ + NM_META_SETTING_TYPE_USER, +}; + const NMMetaSettingInfo * nm_meta_setting_infos_by_name(const char *name) { diff --git a/src/libnmc-setting/nm-meta-setting-base-impl.h b/src/libnmc-setting/nm-meta-setting-base-impl.h index 5e05ff31e2..44bd9024c2 100644 --- a/src/libnmc-setting/nm-meta-setting-base-impl.h +++ b/src/libnmc-setting/nm-meta-setting-base-impl.h @@ -175,6 +175,8 @@ typedef struct _NMMetaSettingInfo_Alias NMMetaSettingInfo; extern const NMMetaSettingInfo nm_meta_setting_infos[_NM_META_SETTING_TYPE_NUM + 1]; +extern const NMMetaSettingType nm_meta_setting_types_by_priority[_NM_META_SETTING_TYPE_NUM]; + const NMMetaSettingInfo *nm_meta_setting_infos_by_name(const char *name); const NMMetaSettingInfo *nm_meta_setting_infos_by_gtype(GType gtype); From 042cd99049087d1f2a6a9e8883fb687da70d51ab Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 11 Jun 2021 15:28:09 +0200 Subject: [PATCH 04/22] libnm/tests: test consistency for nm_meta_setting_types_by_priority --- src/libnm-core-impl/tests/test-setting.c | 58 ++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/libnm-core-impl/tests/test-setting.c b/src/libnm-core-impl/tests/test-setting.c index 8709283d78..50821bb16c 100644 --- a/src/libnm-core-impl/tests/test-setting.c +++ b/src/libnm-core-impl/tests/test-setting.c @@ -109,6 +109,61 @@ _connection_new_from_dbus_strict(GVariant *dict, gboolean normalize) /*****************************************************************************/ +static void +test_nm_meta_setting_types_by_priority(void) +{ + gs_unref_ptrarray GPtrArray *arr = NULL; + int i; + int j; + + G_STATIC_ASSERT_EXPR(_NM_META_SETTING_TYPE_NUM + == G_N_ELEMENTS(nm_meta_setting_types_by_priority)); + + G_STATIC_ASSERT_EXPR(_NM_META_SETTING_TYPE_NUM == 51); + + arr = g_ptr_array_new_with_free_func(g_object_unref); + + for (i = 0; i < _NM_META_SETTING_TYPE_NUM; i++) { + const NMMetaSettingType meta_type = nm_meta_setting_types_by_priority[i]; + const NMMetaSettingInfo *setting_info; + NMSetting * setting; + + g_assert(_NM_INT_NOT_NEGATIVE(meta_type)); + g_assert(meta_type < _NM_META_SETTING_TYPE_NUM); + + setting_info = &nm_meta_setting_infos[meta_type]; + g_assert(setting_info); + _nm_assert_setting_info(setting_info, 0); + + for (j = 0; j < i; j++) + g_assert_cmpint(nm_meta_setting_types_by_priority[j], !=, meta_type); + + setting = g_object_new(setting_info->get_setting_gtype(), NULL); + g_assert(NM_IS_SETTING(setting)); + + g_ptr_array_add(arr, setting); + } + + for (i = 1; i < _NM_META_SETTING_TYPE_NUM; i++) { + NMSetting *setting = arr->pdata[i]; + + for (j = 0; j < i; j++) { + NMSetting *other = arr->pdata[j]; + + if (_nmtst_nm_setting_sort(other, setting) >= 0) { + g_error("sort order for nm_meta_setting_types_by_priority[%d vs %d] is wrong: %s " + "should be before %s", + j, + i, + nm_setting_get_name(setting), + nm_setting_get_name(other)); + } + } + } +} + +/*****************************************************************************/ + static char * _create_random_ipaddr(int addr_family, gboolean as_service) { @@ -4593,6 +4648,9 @@ main(int argc, char **argv) g_test_add_func("/libnm/test_connection_uuid", test_connection_uuid); + g_test_add_func("/libnm/settings/test_nm_meta_setting_types_by_priority", + test_nm_meta_setting_types_by_priority); + g_test_add_data_func("/libnm/setting-8021x/key-and-cert", "test_key_and_cert.pem, test", test_8021x); From 91aacbef416f449e724250b2c7970516a8cbb077 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 11 Jun 2021 15:29:15 +0200 Subject: [PATCH 05/22] libnm: refactor tracking of NMSetting in NMConnection A NMConnection tracks a list of NMSetting instances. For each setting type, it only can track one instance, as is clear by the API nm_connection_get_setting(). The number of different setting types is known at compile time, currently it is 52. Also, we have an NMMetaSettingType enum, which assigns each type a number. Previously, we were tracking the settings in a GHashTable. Rework that, to instead use a fixed size array. Now every NMConnection instance consumes 52 * sizeof(pointer) for the settings array. Previously, the GHashTable required to malloc the "struct _GHashTable" (on 64bit that is about the size of 12 pointers) and for N settings it allocated two buffers (for the key and the values) plus one buffer for the hash values. So, it may or may not consume a bit more memory now, but also can lookup settings directly without hashing. When looking at all settings, we iterate the entire array. Most entries will be NULL, so it's a question whether this could be done better. But as the array is of a fixed, small size, naive iteration is probably still faster and simpler than anything else. --- Test: compiled with -O2, x86_64: $ T=src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh; \ make -j 8 "$T" && \ "$T" 1>/dev/null && \ perf stat -r 200 -B "$T" 1>/dev/null Before: Performance counter stats for 'src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh' (200 runs): 338.39 msec task-clock:u # 0.962 CPUs utilized ( +- 0.68% ) 0 context-switches:u # 0.000 K/sec 0 cpu-migrations:u # 0.000 K/sec 1,121 page-faults:u # 0.003 M/sec ( +- 0.03% ) 1,060,001,815 cycles:u # 3.132 GHz ( +- 0.50% ) 1,877,905,122 instructions:u # 1.77 insn per cycle ( +- 0.01% ) 374,065,113 branches:u # 1105.429 M/sec ( +- 0.01% ) 6,862,991 branch-misses:u # 1.83% of all branches ( +- 0.36% ) 0.35185 +- 0.00247 seconds time elapsed ( +- 0.70% ) After: Performance counter stats for 'src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh' (200 runs): 328.07 msec task-clock:u # 0.959 CPUs utilized ( +- 0.39% ) 0 context-switches:u # 0.000 K/sec 0 cpu-migrations:u # 0.000 K/sec 1,130 page-faults:u # 0.003 M/sec ( +- 0.03% ) 1,034,858,368 cycles:u # 3.154 GHz ( +- 0.33% ) 1,846,714,951 instructions:u # 1.78 insn per cycle ( +- 0.00% ) 369,754,267 branches:u # 1127.052 M/sec ( +- 0.01% ) 6,594,396 branch-misses:u # 1.78% of all branches ( +- 0.23% ) 0.34193 +- 0.00145 seconds time elapsed ( +- 0.42% ) --- src/libnm-core-impl/nm-connection.c | 560 ++++++++++++++++------------ 1 file changed, 324 insertions(+), 236 deletions(-) diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c index 9672dcc252..2b3a3efafa 100644 --- a/src/libnm-core-impl/nm-connection.c +++ b/src/libnm-core-impl/nm-connection.c @@ -37,14 +37,19 @@ /*****************************************************************************/ -enum { SECRETS_UPDATED, SECRETS_CLEARED, CHANGED, LAST_SIGNAL }; +enum { + SECRETS_UPDATED, + SECRETS_CLEARED, + CHANGED, + LAST_SIGNAL, +}; static guint signals[LAST_SIGNAL] = {0}; typedef struct { NMConnection *self; - GHashTable *settings; + NMSetting *settings[_NM_META_SETTING_TYPE_NUM]; /* D-Bus path of the connection, if any */ char *path; @@ -57,60 +62,86 @@ static NMConnectionPrivate *nm_connection_get_private(NMConnection *connection); /*****************************************************************************/ -static gpointer -_gtype_to_hash_key(GType gtype) -{ -#if NM_MORE_ASSERTS - _nm_unused const gsize *const test_gtype_typedef = >ype; - - nm_assert((GType) (GPOINTER_TO_SIZE(GSIZE_TO_POINTER(gtype))) == gtype); - G_STATIC_ASSERT_EXPR(sizeof(gpointer) >= sizeof(gsize)); - G_STATIC_ASSERT_EXPR(sizeof(gsize) == sizeof(GType)); -#endif - - return GSIZE_TO_POINTER(gtype); -} - -/*****************************************************************************/ - static void -setting_changed_cb(NMSetting *setting, GParamSpec *pspec, NMConnection *self) +_signal_emit_changed(NMConnection *self) { g_signal_emit(self, signals[CHANGED], 0); } static void -_setting_release(NMConnection *connection, NMSetting *setting) +setting_changed_cb(NMSetting *setting, GParamSpec *pspec, NMConnection *self) +{ + _signal_emit_changed(self); +} + +static void +_setting_notify_connect(NMConnection *connection, NMSetting *setting) +{ + g_signal_connect(setting, "notify", (GCallback) setting_changed_cb, connection); +} + +static void +_setting_notify_disconnect(NMConnection *connection, NMSetting *setting) { g_signal_handlers_disconnect_by_func(setting, setting_changed_cb, connection); } -static gboolean -_setting_release_hfr(gpointer key, gpointer value, gpointer user_data) +static void +_setting_notify_block(NMConnection *connection, NMSetting *setting) { - _setting_release(user_data, value); - return TRUE; + g_signal_handlers_block_by_func(setting, (GCallback) setting_changed_cb, connection); +} + +static void +_setting_notify_unblock(NMConnection *connection, NMSetting *setting) +{ + g_signal_handlers_unblock_by_func(setting, (GCallback) setting_changed_cb, connection); +} + +static gboolean +_nm_connection_clear_settings(NMConnection *connection, NMConnectionPrivate *priv) +{ + gboolean changed = FALSE; + int i; + + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + if (priv->settings[i]) { + _setting_notify_disconnect(connection, priv->settings[i]); + g_clear_object(&priv->settings[i]); + changed = TRUE; + } + } + return changed; } static void _nm_connection_add_setting(NMConnection *connection, NMSetting *setting) { - NMConnectionPrivate *priv; - GType setting_type; - NMSetting * s_old; + const NMMetaSettingInfo *setting_info; + NMConnectionPrivate * priv; + NMSetting * s_old; nm_assert(NM_IS_CONNECTION(connection)); nm_assert(NM_IS_SETTING(setting)); - priv = NM_CONNECTION_GET_PRIVATE(connection); - setting_type = G_OBJECT_TYPE(setting); + setting_info = _nm_meta_setting_info_from_class(NM_SETTING_GET_CLASS(setting)); + if (!setting_info) + g_return_if_reached(); - if ((s_old = g_hash_table_lookup(priv->settings, _gtype_to_hash_key(setting_type)))) - _setting_release(connection, s_old); + priv = NM_CONNECTION_GET_PRIVATE(connection); - g_hash_table_insert(priv->settings, _gtype_to_hash_key(setting_type), setting); + s_old = priv->settings[setting_info->meta_type]; + if (s_old == setting) + return; - g_signal_connect(setting, "notify", (GCallback) setting_changed_cb, connection); + priv->settings[setting_info->meta_type] = setting; + + _setting_notify_connect(connection, setting); + + if (s_old) { + _setting_notify_disconnect(connection, s_old); + g_object_unref(s_old); + } } /** @@ -130,27 +161,31 @@ nm_connection_add_setting(NMConnection *connection, NMSetting *setting) g_return_if_fail(NM_IS_SETTING(setting)); _nm_connection_add_setting(connection, setting); - g_signal_emit(connection, signals[CHANGED], 0); + _signal_emit_changed(connection); } gboolean _nm_connection_remove_setting(NMConnection *connection, GType setting_type) { - NMConnectionPrivate *priv; - NMSetting * setting; + NMConnectionPrivate * priv; + NMSetting * setting; + const NMMetaSettingInfo *setting_info; g_return_val_if_fail(NM_IS_CONNECTION(connection), FALSE); - g_return_val_if_fail(g_type_is_a(setting_type, NM_TYPE_SETTING), FALSE); + + setting_info = _nm_meta_setting_info_from_gtype(setting_type); + if (!setting_info) + g_return_val_if_reached(FALSE); priv = NM_CONNECTION_GET_PRIVATE(connection); - setting = g_hash_table_lookup(priv->settings, _gtype_to_hash_key(setting_type)); - if (setting) { - g_signal_handlers_disconnect_by_func(setting, setting_changed_cb, connection); - g_hash_table_remove(priv->settings, _gtype_to_hash_key(setting_type)); - g_signal_emit(connection, signals[CHANGED], 0); - return TRUE; - } - return FALSE; + setting = g_steal_pointer(&priv->settings[setting_info->meta_type]); + if (!setting) + return FALSE; + + _setting_notify_disconnect(connection, setting); + _signal_emit_changed(connection); + g_object_unref(setting); + return TRUE; } /** @@ -170,13 +205,18 @@ nm_connection_remove_setting(NMConnection *connection, GType setting_type) static gpointer _connection_get_setting(NMConnection *connection, GType setting_type) { - NMSetting *setting; + NMSetting * setting; + const NMMetaSettingInfo *setting_info; nm_assert(NM_IS_CONNECTION(connection)); nm_assert(g_type_is_a(setting_type, NM_TYPE_SETTING)); - setting = g_hash_table_lookup(NM_CONNECTION_GET_PRIVATE(connection)->settings, - _gtype_to_hash_key(setting_type)); + setting_info = _nm_meta_setting_info_from_gtype(setting_type); + if (!setting_info) + g_return_val_if_reached(NULL); + + setting = NM_CONNECTION_GET_PRIVATE(connection)->settings[setting_info->meta_type]; + nm_assert(!setting || G_TYPE_CHECK_INSTANCE_TYPE(setting, setting_type)); return setting; } @@ -311,6 +351,20 @@ validate_permissions_type(GVariant *variant, GError **error) return valid; } +static void +_auto_settings(NMSetting ***p_settings) +{ + NMSetting **settings = *p_settings; + int i; + + if (settings) { + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + if (settings[i]) + g_object_unref(settings[i]); + } + } +} + /** * _nm_connection_replace_settings: * @connection: a #NMConnection @@ -334,12 +388,15 @@ _nm_connection_replace_settings(NMConnection * connection, NMSettingParseFlags parse_flags, GError ** error) { - NMConnectionPrivate *priv; - GVariantIter iter; - const char * setting_name; - GVariant * setting_dict; - GSList * settings = NULL, *s; - gboolean changed, success; + NMSetting * settings[_NM_META_SETTING_TYPE_NUM] = {}; + nm_auto(_auto_settings) NMSetting **settings_cleanup = settings; + GVariantIter iter; + const char * setting_name; + GVariant * setting_dict; + gboolean changed; + gboolean success; + guint n_settings = 0; + int i; g_return_val_if_fail(NM_IS_CONNECTION(connection), FALSE); g_return_val_if_fail(g_variant_is_of_type(new_settings, NM_VARIANT_TYPE_CONNECTION), FALSE); @@ -349,8 +406,6 @@ _nm_connection_replace_settings(NMConnection * connection, nm_assert(!NM_FLAGS_ALL(parse_flags, NM_SETTING_PARSE_FLAGS_STRICT | NM_SETTING_PARSE_FLAGS_BEST_EFFORT)); - priv = NM_CONNECTION_GET_PRIVATE(connection); - if (!NM_FLAGS_HAS(parse_flags, NM_SETTING_PARSE_FLAGS_BEST_EFFORT) && !validate_permissions_type(new_settings, error)) return FALSE; @@ -360,12 +415,12 @@ _nm_connection_replace_settings(NMConnection * connection, gs_unref_variant GVariant *setting_dict_free = NULL; GError * local = NULL; NMSetting * setting; - GType type; + const NMMetaSettingInfo * setting_info; setting_dict_free = setting_dict; - type = nm_setting_lookup_type(setting_name); - if (type == G_TYPE_INVALID) { + setting_info = nm_meta_setting_infos_by_name(setting_name); + if (!setting_info) { if (NM_FLAGS_HAS(parse_flags, NM_SETTING_PARSE_FLAGS_BEST_EFFORT)) continue; g_set_error_literal(error, @@ -373,54 +428,58 @@ _nm_connection_replace_settings(NMConnection * connection, NM_CONNECTION_ERROR_INVALID_SETTING, _("unknown setting name")); g_prefix_error(error, "%s: ", setting_name); - g_slist_free_full(settings, g_object_unref); return FALSE; } - for (s = settings; s; s = s->next) { - if (G_OBJECT_TYPE(s->data) == type) { - if (NM_FLAGS_HAS(parse_flags, NM_SETTING_PARSE_FLAGS_STRICT)) { - g_set_error_literal(error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_SETTING, - _("duplicate setting name")); - g_prefix_error(error, "%s: ", setting_name); - g_slist_free_full(settings, g_object_unref); - return FALSE; - } - /* last wins. */ - g_object_unref(s->data); - settings = g_slist_delete_link(settings, s); - break; + _nm_assert_setting_info(setting_info, 0); + + if (settings[setting_info->meta_type]) { + if (NM_FLAGS_HAS(parse_flags, NM_SETTING_PARSE_FLAGS_STRICT)) { + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_SETTING, + _("duplicate setting name")); + g_prefix_error(error, "%s: ", setting_name); + return FALSE; } + /* last wins. We remove the setting of this type, and will + * add the new one afterwards. */ + g_clear_object(&settings[setting_info->meta_type]); + break; } - setting = _nm_setting_new_from_dbus(type, setting_dict, new_settings, parse_flags, &local); + setting = _nm_setting_new_from_dbus(setting_info->get_setting_gtype(), + setting_dict, + new_settings, + parse_flags, + &local); if (!setting) { if (NM_FLAGS_HAS(parse_flags, NM_SETTING_PARSE_FLAGS_BEST_EFFORT)) continue; g_propagate_error(error, local); - g_slist_free_full(settings, g_object_unref); return FALSE; } - settings = g_slist_prepend(settings, setting); + settings[setting_info->meta_type] = setting; + n_settings++; } - if (g_hash_table_size(priv->settings) > 0) { - g_hash_table_foreach_remove(priv->settings, _setting_release_hfr, connection); + if (_nm_connection_clear_settings(connection, NM_CONNECTION_GET_PRIVATE(connection))) changed = TRUE; - } else - changed = (settings != NULL); + else + changed = (n_settings > 0); /* Note: @settings might be empty in which case the connection * has no NMSetting instances... which is fine, just something * to be aware of. */ - for (s = settings; s; s = s->next) - _nm_connection_add_setting(connection, s->data); - - g_slist_free(settings); + if (n_settings > 0) { + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + if (settings[i]) + _nm_connection_add_setting(connection, g_steal_pointer(&settings[i])); + } + settings_cleanup = NULL; + } /* If verification/normalization fails, the original connection * is already lost. From an API point of view, it would be nicer @@ -435,7 +494,7 @@ _nm_connection_replace_settings(NMConnection * connection, success = TRUE; if (changed) - g_signal_emit(connection, signals[CHANGED], 0); + _signal_emit_changed(connection); return success; } @@ -473,10 +532,10 @@ void nm_connection_replace_settings_from_connection(NMConnection *connection, NMConnection *new_connection) { - NMConnectionPrivate *priv, *new_priv; - GHashTableIter iter; - NMSetting * setting; + NMConnectionPrivate *priv; + NMConnectionPrivate *new_priv; gboolean changed; + int i; g_return_if_fail(NM_IS_CONNECTION(connection)); g_return_if_fail(NM_IS_CONNECTION(new_connection)); @@ -494,18 +553,28 @@ nm_connection_replace_settings_from_connection(NMConnection *connection, priv = NM_CONNECTION_GET_PRIVATE(connection); new_priv = NM_CONNECTION_GET_PRIVATE(new_connection); - if ((changed = g_hash_table_size(priv->settings) > 0)) - g_hash_table_foreach_remove(priv->settings, _setting_release_hfr, connection); + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + NMSetting *s_old; + + if (new_priv->settings[i] == priv->settings[i]) + continue; - if (g_hash_table_size(new_priv->settings)) { - g_hash_table_iter_init(&iter, new_priv->settings); - while (g_hash_table_iter_next(&iter, NULL, (gpointer) &setting)) - _nm_connection_add_setting(connection, nm_setting_duplicate(setting)); changed = TRUE; + + s_old = g_steal_pointer(&priv->settings[i]); + + if (new_priv->settings[i]) { + priv->settings[i] = nm_setting_duplicate(new_priv->settings[i]); + _setting_notify_connect(connection, priv->settings[i]); + } + if (s_old) { + _setting_notify_disconnect(connection, s_old); + g_object_unref(s_old); + } } if (changed) - g_signal_emit(connection, signals[CHANGED], 0); + _signal_emit_changed(connection); } /** @@ -517,16 +586,10 @@ nm_connection_replace_settings_from_connection(NMConnection *connection, void nm_connection_clear_settings(NMConnection *connection) { - NMConnectionPrivate *priv; - g_return_if_fail(NM_IS_CONNECTION(connection)); - priv = NM_CONNECTION_GET_PRIVATE(connection); - - if (g_hash_table_size(priv->settings) > 0) { - g_hash_table_foreach_remove(priv->settings, _setting_release_hfr, connection); - g_signal_emit(connection, signals[CHANGED], 0); - } + if (_nm_connection_clear_settings(connection, NM_CONNECTION_GET_PRIVATE(connection))) + _signal_emit_changed(connection); } /** @@ -544,28 +607,28 @@ nm_connection_clear_settings(NMConnection *connection) gboolean nm_connection_compare(NMConnection *a, NMConnection *b, NMSettingCompareFlags flags) { - GHashTableIter iter; - NMSetting * src; + NMConnectionPrivate *a_priv; + NMConnectionPrivate *b_priv; + int i; if (a == b) return TRUE; if (!a || !b) return FALSE; - /* B / A: ensure settings in B that are not in A make the comparison fail */ - if (g_hash_table_size(NM_CONNECTION_GET_PRIVATE(a)->settings) - != g_hash_table_size(NM_CONNECTION_GET_PRIVATE(b)->settings)) - return FALSE; + a_priv = NM_CONNECTION_GET_PRIVATE(a); + b_priv = NM_CONNECTION_GET_PRIVATE(b); - /* A / B: ensure all settings in A match corresponding ones in B */ - g_hash_table_iter_init(&iter, NM_CONNECTION_GET_PRIVATE(a)->settings); - while (g_hash_table_iter_next(&iter, NULL, (gpointer) &src)) { - NMSetting *cmp = nm_connection_get_setting(b, G_OBJECT_TYPE(src)); + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + if (a_priv->settings[i] == b_priv->settings[i]) + continue; - if (!cmp || !_nm_setting_compare(a, src, b, cmp, flags)) + if (!a_priv->settings[i] || !b_priv->settings[i]) + return FALSE; + + if (!_nm_setting_compare(a, a_priv->settings[i], b, b_priv->settings[i], flags)) return FALSE; } - return TRUE; } @@ -576,30 +639,30 @@ diff_one_connection(NMConnection * a, gboolean invert_results, GHashTable * diffs) { - NMConnectionPrivate *priv = NM_CONNECTION_GET_PRIVATE(a); - GHashTableIter iter; - NMSetting * a_setting = NULL; + NMConnectionPrivate *a_priv = NM_CONNECTION_GET_PRIVATE(a); + NMConnectionPrivate *b_priv = b ? NM_CONNECTION_GET_PRIVATE(b) : NULL; gboolean diff_found = FALSE; + int i; - g_hash_table_iter_init(&iter, priv->settings); - while (g_hash_table_iter_next(&iter, NULL, (gpointer) &a_setting)) { - NMSetting * b_setting = NULL; - const char *setting_name = nm_setting_get_name(a_setting); - GHashTable *results; - gboolean new_results = TRUE; + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + NMSetting *a_setting = a_priv->settings[i]; - if (b) - b_setting = nm_connection_get_setting(b, G_OBJECT_TYPE(a_setting)); + if (a_setting) { + NMSetting * b_setting = b ? b_priv->settings[i] : NULL; + const char *setting_name = nm_setting_get_name(a_setting); + GHashTable *results; + gboolean new_results = TRUE; - results = g_hash_table_lookup(diffs, setting_name); - if (results) - new_results = FALSE; + results = g_hash_table_lookup(diffs, setting_name); + if (results) + new_results = FALSE; - if (!_nm_setting_diff(a, a_setting, b, b_setting, flags, invert_results, &results)) - diff_found = TRUE; + if (!_nm_setting_diff(a, a_setting, b, b_setting, flags, invert_results, &results)) + diff_found = TRUE; - if (new_results && results) - g_hash_table_insert(diffs, g_strdup(setting_name), results); + if (new_results && results) + g_hash_table_insert(diffs, g_strdup(setting_name), results); + } } return diff_found; @@ -666,15 +729,18 @@ nm_connection_diff(NMConnection * a, NMSetting * _nm_connection_find_base_type_setting(NMConnection *connection) { - NMConnectionPrivate *priv = NM_CONNECTION_GET_PRIVATE(connection); - GHashTableIter iter; - NMSetting * setting = NULL; - NMSetting * s_iter; + NMConnectionPrivate *priv = NM_CONNECTION_GET_PRIVATE(connection); + NMSetting * setting = NULL; NMSettingPriority setting_prio = NM_SETTING_PRIORITY_USER; - NMSettingPriority s_iter_prio; + int i; + + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + NMSetting * s_iter = priv->settings[i]; + NMSettingPriority s_iter_prio; + + if (!s_iter) + continue; - g_hash_table_iter_init(&iter, priv->settings); - while (g_hash_table_iter_next(&iter, NULL, (gpointer *) &s_iter)) { s_iter_prio = _nm_setting_get_base_type_priority(s_iter); if (s_iter_prio == NM_SETTING_PRIORITY_INVALID) continue; @@ -884,25 +950,24 @@ _nm_connection_detect_bluetooth_type(NMConnection *self) const char * _nm_connection_detect_slave_type(NMConnection *connection, NMSetting **out_s_port) { - NMConnectionPrivate *priv = NM_CONNECTION_GET_PRIVATE(connection); - GHashTableIter iter; + NMConnectionPrivate *priv = NM_CONNECTION_GET_PRIVATE(connection); const char * slave_type = NULL; - NMSetting * s_port = NULL, *s_iter; + NMSetting * s_port = NULL; + int i; + static const struct { + NMMetaSettingType meta_type; + const char * controller_type_name; + } infos[] = { + {NM_META_SETTING_TYPE_BRIDGE_PORT, NM_SETTING_BRIDGE_SETTING_NAME}, + {NM_META_SETTING_TYPE_TEAM_PORT, NM_SETTING_TEAM_SETTING_NAME}, + {NM_META_SETTING_TYPE_OVS_PORT, NM_SETTING_OVS_BRIDGE_SETTING_NAME}, + {NM_META_SETTING_TYPE_OVS_INTERFACE, NM_SETTING_OVS_PORT_SETTING_NAME}, + }; - g_hash_table_iter_init(&iter, priv->settings); - while (g_hash_table_iter_next(&iter, NULL, (gpointer *) &s_iter)) { - const char *name = nm_setting_get_name(s_iter); - const char *i_slave_type = NULL; + for (i = 0; i < (int) G_N_ELEMENTS(infos); i++) { + NMSetting *setting = priv->settings[infos[i].meta_type]; - if (!strcmp(name, NM_SETTING_BRIDGE_PORT_SETTING_NAME)) - i_slave_type = NM_SETTING_BRIDGE_SETTING_NAME; - else if (!strcmp(name, NM_SETTING_TEAM_PORT_SETTING_NAME)) - i_slave_type = NM_SETTING_TEAM_SETTING_NAME; - else if (!strcmp(name, NM_SETTING_OVS_PORT_SETTING_NAME)) - i_slave_type = NM_SETTING_OVS_BRIDGE_SETTING_NAME; - else if (!strcmp(name, NM_SETTING_OVS_INTERFACE_SETTING_NAME)) - i_slave_type = NM_SETTING_OVS_PORT_SETTING_NAME; - else + if (!setting) continue; if (slave_type) { @@ -911,8 +976,8 @@ _nm_connection_detect_slave_type(NMConnection *connection, NMSetting **out_s_por s_port = NULL; break; } - slave_type = i_slave_type; - s_port = s_iter; + slave_type = infos[i].controller_type_name; + s_port = setting; } if (out_s_port) @@ -1684,15 +1749,18 @@ _nm_connection_verify(NMConnection *connection, GError **error) gboolean nm_connection_verify_secrets(NMConnection *connection, GError **error) { - GHashTableIter iter; - NMSetting * setting; + NMConnectionPrivate *priv; + int i; g_return_val_if_fail(NM_IS_CONNECTION(connection), FALSE); g_return_val_if_fail(!error || !*error, FALSE); - g_hash_table_iter_init(&iter, NM_CONNECTION_GET_PRIVATE(connection)->settings); - while (g_hash_table_iter_next(&iter, NULL, (gpointer) &setting)) { - if (!nm_setting_verify_secrets(setting, connection, error)) + priv = NM_CONNECTION_GET_PRIVATE(connection); + + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + if (!priv->settings[i]) + continue; + if (!nm_setting_verify_secrets(priv->settings[i], connection, error)) return FALSE; } return TRUE; @@ -2025,9 +2093,9 @@ nm_connection_update_secrets(NMConnection *connection, } } - g_signal_handlers_block_by_func(setting, (GCallback) setting_changed_cb, connection); + _setting_notify_block(connection, setting); success_detail = _nm_setting_update_secrets(setting, setting_dict ?: secrets, error); - g_signal_handlers_unblock_by_func(setting, (GCallback) setting_changed_cb, connection); + _setting_notify_unblock(connection, setting); nm_clear_pointer(&setting_dict, g_variant_unref); @@ -2059,10 +2127,10 @@ nm_connection_update_secrets(NMConnection *connection, /* Update the secrets for this setting */ setting = nm_connection_get_setting_by_name(connection, key); - g_signal_handlers_block_by_func(setting, (GCallback) setting_changed_cb, connection); + _setting_notify_block(connection, setting); success_detail = _nm_setting_update_secrets(setting, setting_dict, error ? &local : NULL); - g_signal_handlers_unblock_by_func(setting, (GCallback) setting_changed_cb, connection); + _setting_notify_unblock(connection, setting); g_variant_unref(setting_dict); @@ -2111,12 +2179,9 @@ nm_connection_update_secrets(NMConnection *connection, const char * nm_connection_need_secrets(NMConnection *connection, GPtrArray **hints) { + NMSetting * setting_before = NULL; NMConnectionPrivate *priv; - GHashTableIter hiter; - GSList * settings = NULL; - GSList * iter; - const char * name = NULL; - NMSetting * setting; + int i; g_return_val_if_fail(NM_IS_CONNECTION(connection), NULL); if (hints) @@ -2125,28 +2190,30 @@ nm_connection_need_secrets(NMConnection *connection, GPtrArray **hints) priv = NM_CONNECTION_GET_PRIVATE(connection); /* Get list of settings in priority order */ - g_hash_table_iter_init(&hiter, priv->settings); - while (g_hash_table_iter_next(&hiter, NULL, (gpointer) &setting)) - settings = g_slist_insert_sorted(settings, setting, _nm_setting_compare_priority); - - for (iter = settings; iter; iter = g_slist_next(iter)) { + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + NMSetting *setting = priv->settings[nm_meta_setting_types_by_priority[i]]; GPtrArray *secrets; - setting = NM_SETTING(iter->data); - secrets = _nm_setting_need_secrets(setting); - if (secrets) { - if (hints) - *hints = secrets; - else - g_ptr_array_free(secrets, TRUE); + if (!setting) + continue; - name = nm_setting_get_name(setting); - break; - } + nm_assert(!setting_before || _nmtst_nm_setting_sort(setting_before, setting) < 0); + nm_assert(!setting_before || _nm_setting_compare_priority(setting_before, setting) <= 0); + setting_before = setting; + + secrets = _nm_setting_need_secrets(setting); + if (!secrets) + continue; + + if (hints) + *hints = secrets; + else + g_ptr_array_free(secrets, TRUE); + + return nm_setting_get_name(setting); } - g_slist_free(settings); - return name; + return NULL; } /** @@ -2176,16 +2243,22 @@ nm_connection_clear_secrets_with_flags(NMConnection * connecti NMSettingClearSecretsWithFlagsFn func, gpointer user_data) { - GHashTableIter iter; - NMSetting * setting; + NMConnectionPrivate *priv; + int i; g_return_if_fail(NM_IS_CONNECTION(connection)); - g_hash_table_iter_init(&iter, NM_CONNECTION_GET_PRIVATE(connection)->settings); - while (g_hash_table_iter_next(&iter, NULL, (gpointer) &setting)) { - g_signal_handlers_block_by_func(setting, (GCallback) setting_changed_cb, connection); + priv = NM_CONNECTION_GET_PRIVATE(connection); + + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + NMSetting *setting = priv->settings[i]; + + if (!setting) + continue; + + _setting_notify_block(connection, setting); _nm_setting_clear_secrets(setting, func, user_data); - g_signal_handlers_unblock_by_func(setting, (GCallback) setting_changed_cb, connection); + _setting_notify_unblock(connection, setting); } g_signal_emit(connection, signals[SECRETS_CLEARED], 0); @@ -2408,24 +2481,6 @@ nm_connection_is_type(NMConnection *connection, const char *type) return nm_streq0(type, nm_connection_get_connection_type(connection)); } -static int -_get_settings_sort(gconstpointer p_a, gconstpointer p_b, gpointer unused) -{ - NMSetting *a = *((NMSetting **) p_a); - NMSetting *b = *((NMSetting **) p_b); - - nm_assert(NM_IS_SETTING(a)); - nm_assert(NM_IS_SETTING(b)); - nm_assert(a != b); - nm_assert(G_OBJECT_TYPE(a) != G_OBJECT_TYPE(b)); - - NM_CMP_RETURN(_nm_setting_compare_priority(a, b)); - NM_CMP_DIRECT_STRCMP(nm_setting_get_name(a), nm_setting_get_name(b)); - - nm_assert_not_reached(); - return 0; -} - int _nmtst_nm_setting_sort(NMSetting *a, NMSetting *b) { @@ -2460,13 +2515,39 @@ _nmtst_nm_setting_sort(NMSetting *a, NMSetting *b) NMSetting ** nm_connection_get_settings(NMConnection *connection, guint *out_length) { + NMConnectionPrivate *priv; + NMSetting ** arr; + int len; + int i; + int j; + g_return_val_if_fail(NM_IS_CONNECTION(connection), NULL); - return (NMSetting **) nm_utils_hash_values_to_array( - NM_CONNECTION_GET_PRIVATE(connection)->settings, - _get_settings_sort, - NULL, - out_length); + priv = NM_CONNECTION_GET_PRIVATE(connection); + + len = 0; + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + if (priv->settings[i]) + len++; + } + + NM_SET_OUT(out_length, len); + + if (len == 0) + return NULL; + + arr = g_new(NMSetting *, len + 1); + for (i = 0, j = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + NMSetting *setting = priv->settings[nm_meta_setting_types_by_priority[i]]; + + if (setting) { + nm_assert(j == 0 || _nmtst_nm_setting_sort(arr[j - 1], setting) < 0); + arr[j++] = setting; + } + } + arr[len] = NULL; + + return arr; } /** @@ -2514,11 +2595,10 @@ gboolean _nm_connection_aggregate(NMConnection *connection, NMConnectionAggregateType type, gpointer arg) { NMConnectionPrivate *priv; - GHashTableIter iter; - NMSetting * setting; gboolean arg_boolean; gboolean completed_early; gpointer my_arg; + int i; g_return_val_if_fail(NM_IS_CONNECTION(connection), FALSE); @@ -2538,8 +2618,11 @@ good: priv = NM_CONNECTION_GET_PRIVATE(connection); completed_early = FALSE; - g_hash_table_iter_init(&iter, priv->settings); - while (g_hash_table_iter_next(&iter, NULL, (gpointer) &setting)) { + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + NMSetting *setting = priv->settings[i]; + + if (!setting) + continue; if (_nm_setting_aggregate(setting, type, my_arg)) { completed_early = TRUE; break; @@ -2570,18 +2653,25 @@ good: void nm_connection_dump(NMConnection *connection) { - GHashTableIter iter; - NMSetting * setting; - char * str; + NMConnectionPrivate *priv; + int i; if (!connection) return; - g_hash_table_iter_init(&iter, NM_CONNECTION_GET_PRIVATE(connection)->settings); - while (g_hash_table_iter_next(&iter, NULL, (gpointer) &setting)) { - str = nm_setting_to_string(setting); - g_print("%s\n", str); - g_free(str); + g_return_if_fail(NM_IS_CONNECTION(connection)); + + priv = NM_CONNECTION_GET_PRIVATE(connection); + + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + NMSetting *setting = priv->settings[nm_meta_setting_types_by_priority[i]]; + + if (setting) { + gs_free char *str = NULL; + + str = nm_setting_to_string(setting); + g_print("%s\n", str); + } } } @@ -3433,8 +3523,7 @@ nm_connection_private_free(NMConnectionPrivate *priv) { NMConnection *self = priv->self; - g_hash_table_foreach_remove(priv->settings, _setting_release_hfr, self); - g_hash_table_destroy(priv->settings); + _nm_connection_clear_settings(self, priv); g_free(priv->path); g_slice_free(NMConnectionPrivate, priv); @@ -3458,8 +3547,7 @@ nm_connection_get_private(NMConnection *connection) priv, (GDestroyNotify) nm_connection_private_free); - priv->self = connection; - priv->settings = g_hash_table_new_full(nm_direct_hash, NULL, NULL, g_object_unref); + priv->self = connection; } return priv; From c8c606b32391ff6b86a34bb2876b42b047acba5e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 11 Jun 2021 17:45:42 +0200 Subject: [PATCH 06/22] libnm: avoid cloning list of settings in _nm_connection_verify() --- src/libnm-core-impl/nm-connection.c | 39 +++++++++++++++++++---------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c index 2b3a3efafa..9b3514ed10 100644 --- a/src/libnm-core-impl/nm-connection.c +++ b/src/libnm-core-impl/nm-connection.c @@ -221,6 +221,16 @@ _connection_get_setting(NMConnection *connection, GType setting_type) return setting; } +static gpointer +_connection_get_setting_by_meta_type(NMConnectionPrivate *priv, NMMetaSettingType meta_type) +{ + nm_assert(priv); + nm_assert(_NM_INT_NOT_NEGATIVE(meta_type)); + nm_assert(meta_type < _NM_META_SETTING_TYPE_NUM); + + return priv->settings[meta_type]; +} + static gpointer _connection_get_setting_check(NMConnection *connection, GType setting_type) { @@ -1587,18 +1597,20 @@ nm_connection_verify(NMConnection *connection, GError **error) NMSettingVerifyResult _nm_connection_verify(NMConnection *connection, GError **error) { - NMSettingIPConfig *s_ip4, *s_ip6; - NMSettingProxy * s_proxy; - gs_free NMSetting **settings = NULL; + NMConnectionPrivate *priv; + NMSettingIPConfig * s_ip4; + NMSettingIPConfig * s_ip6; + NMSettingProxy * s_proxy; gs_free_error GError *normalizable_error = NULL; NMSettingVerifyResult normalizable_error_type = NM_SETTING_VERIFY_SUCCESS; - guint i; + int i; g_return_val_if_fail(NM_IS_CONNECTION(connection), NM_SETTING_VERIFY_ERROR); g_return_val_if_fail(!error || !*error, NM_SETTING_VERIFY_ERROR); - settings = nm_connection_get_settings(connection, NULL); - if (!settings || !NM_IS_SETTING_CONNECTION(settings[0])) { + priv = NM_CONNECTION_GET_PRIVATE(connection); + + if (!_connection_get_setting_by_meta_type(priv, NM_META_SETTING_TYPE_CONNECTION)) { g_set_error_literal(error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_MISSING_SETTING, @@ -1607,12 +1619,13 @@ _nm_connection_verify(NMConnection *connection, GError **error) return NM_SETTING_VERIFY_ERROR; } - for (i = 0; settings[i]; i++) { + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + NMSetting * setting = priv->settings[nm_meta_setting_types_by_priority[i]]; GError * verify_error = NULL; NMSettingVerifyResult verify_result; - nm_assert(NM_IS_SETTING(settings[i])); - nm_assert(NM_IS_SETTING_CONNECTION(settings[i]) == (i == 0)); + if (!setting) + continue; /* verify all settings. We stop if we find the first non-normalizable * @NM_SETTING_VERIFY_ERROR. If we find normalizable errors we continue @@ -1621,7 +1634,7 @@ _nm_connection_verify(NMConnection *connection, GError **error) * @NM_SETTING_VERIFY_NORMALIZABLE, so, if we encounter such an error type, * we remember it instead (to return it as output). **/ - verify_result = _nm_setting_verify(settings[i], connection, &verify_error); + verify_result = _nm_setting_verify(setting, connection, &verify_error); if (verify_result == NM_SETTING_VERIFY_NORMALIZABLE || verify_result == NM_SETTING_VERIFY_NORMALIZABLE_ERROR) { if (verify_result == NM_SETTING_VERIFY_NORMALIZABLE_ERROR @@ -1642,9 +1655,9 @@ _nm_connection_verify(NMConnection *connection, GError **error) g_clear_error(&verify_error); } - s_ip4 = nm_connection_get_setting_ip4_config(connection); - s_ip6 = nm_connection_get_setting_ip6_config(connection); - s_proxy = nm_connection_get_setting_proxy(connection); + s_ip4 = _connection_get_setting_by_meta_type(priv, NM_META_SETTING_TYPE_IP4_CONFIG); + s_ip6 = _connection_get_setting_by_meta_type(priv, NM_META_SETTING_TYPE_IP6_CONFIG); + s_proxy = _connection_get_setting_by_meta_type(priv, NM_META_SETTING_TYPE_PROXY); nm_assert(normalizable_error_type != NM_SETTING_VERIFY_ERROR); if (NM_IN_SET(normalizable_error_type, From 97eef2bf6d3af4c9246f026470010c8fff232384 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 11 Jun 2021 17:50:51 +0200 Subject: [PATCH 07/22] libnm: implement nm_connection_get_setting*() via NMMetaSettingType The NM_TYPE_SETTING_* macros are really function calls (to a GType/gsize which is guarded by an atomic operation for thread safe initialization). Also, finding the setting_info based on the GType requires additional lookups. It's no longer necessary. We can directly find the setting using the well known index. --- src/libnm-core-impl/nm-connection.c | 86 ++++++++++++++++------------- 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c index 9b3514ed10..5145003554 100644 --- a/src/libnm-core-impl/nm-connection.c +++ b/src/libnm-core-impl/nm-connection.c @@ -239,6 +239,14 @@ _connection_get_setting_check(NMConnection *connection, GType setting_type) return _connection_get_setting(connection, setting_type); } +static gpointer +_connection_get_setting_by_meta_type_check(NMConnection *connection, NMMetaSettingType meta_type) +{ + g_return_val_if_fail(NM_IS_CONNECTION(connection), NULL); + + return _connection_get_setting_by_meta_type(NM_CONNECTION_GET_PRIVATE(connection), meta_type); +} + /** * nm_connection_get_setting: * @connection: a #NMConnection @@ -2963,7 +2971,7 @@ nm_connection_get_virtual_device_description(NMConnection *connection) NMSetting8021x * nm_connection_get_setting_802_1x(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_802_1X); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_802_1X); } /** @@ -2977,7 +2985,7 @@ nm_connection_get_setting_802_1x(NMConnection *connection) NMSettingBluetooth * nm_connection_get_setting_bluetooth(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_BLUETOOTH); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_BLUETOOTH); } /** @@ -2991,7 +2999,7 @@ nm_connection_get_setting_bluetooth(NMConnection *connection) NMSettingBond * nm_connection_get_setting_bond(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_BOND); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_BOND); } /** @@ -3005,7 +3013,7 @@ nm_connection_get_setting_bond(NMConnection *connection) NMSettingTeam * nm_connection_get_setting_team(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_TEAM); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_TEAM); } /** @@ -3019,7 +3027,7 @@ nm_connection_get_setting_team(NMConnection *connection) NMSettingTeamPort * nm_connection_get_setting_team_port(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_TEAM_PORT); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_TEAM_PORT); } /** @@ -3033,7 +3041,7 @@ nm_connection_get_setting_team_port(NMConnection *connection) NMSettingBridge * nm_connection_get_setting_bridge(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_BRIDGE); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_BRIDGE); } /** @@ -3047,7 +3055,7 @@ nm_connection_get_setting_bridge(NMConnection *connection) NMSettingCdma * nm_connection_get_setting_cdma(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_CDMA); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_CDMA); } /** @@ -3061,7 +3069,7 @@ nm_connection_get_setting_cdma(NMConnection *connection) NMSettingConnection * nm_connection_get_setting_connection(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_CONNECTION); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_CONNECTION); } /** @@ -3075,7 +3083,7 @@ nm_connection_get_setting_connection(NMConnection *connection) NMSettingDcb * nm_connection_get_setting_dcb(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_DCB); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_DCB); } /** @@ -3091,7 +3099,7 @@ nm_connection_get_setting_dcb(NMConnection *connection) NMSettingDummy * nm_connection_get_setting_dummy(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_DUMMY); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_DUMMY); } /** @@ -3105,7 +3113,7 @@ nm_connection_get_setting_dummy(NMConnection *connection) NMSettingGeneric * nm_connection_get_setting_generic(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_GENERIC); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_GENERIC); } /** @@ -3119,7 +3127,7 @@ nm_connection_get_setting_generic(NMConnection *connection) NMSettingGsm * nm_connection_get_setting_gsm(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_GSM); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_GSM); } /** @@ -3133,7 +3141,7 @@ nm_connection_get_setting_gsm(NMConnection *connection) NMSettingInfiniband * nm_connection_get_setting_infiniband(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_INFINIBAND); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_INFINIBAND); } /** @@ -3152,7 +3160,7 @@ nm_connection_get_setting_infiniband(NMConnection *connection) NMSettingIPConfig * nm_connection_get_setting_ip4_config(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_IP4_CONFIG); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_IP4_CONFIG); } /** @@ -3168,7 +3176,7 @@ nm_connection_get_setting_ip4_config(NMConnection *connection) NMSettingIPTunnel * nm_connection_get_setting_ip_tunnel(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_IP_TUNNEL); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_IP_TUNNEL); } /** @@ -3187,7 +3195,7 @@ nm_connection_get_setting_ip_tunnel(NMConnection *connection) NMSettingIPConfig * nm_connection_get_setting_ip6_config(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_IP6_CONFIG); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_IP6_CONFIG); } /** @@ -3203,7 +3211,7 @@ nm_connection_get_setting_ip6_config(NMConnection *connection) NMSettingMacsec * nm_connection_get_setting_macsec(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_MACSEC); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_MACSEC); } /** @@ -3219,7 +3227,7 @@ nm_connection_get_setting_macsec(NMConnection *connection) NMSettingMacvlan * nm_connection_get_setting_macvlan(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_MACVLAN); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_MACVLAN); } /** @@ -3233,7 +3241,7 @@ nm_connection_get_setting_macvlan(NMConnection *connection) NMSettingOlpcMesh * nm_connection_get_setting_olpc_mesh(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_OLPC_MESH); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_OLPC_MESH); } /** @@ -3249,7 +3257,7 @@ nm_connection_get_setting_olpc_mesh(NMConnection *connection) NMSettingOvsBridge * nm_connection_get_setting_ovs_bridge(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_OVS_BRIDGE); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_OVS_BRIDGE); } /** @@ -3265,7 +3273,8 @@ nm_connection_get_setting_ovs_bridge(NMConnection *connection) NMSettingOvsInterface * nm_connection_get_setting_ovs_interface(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_OVS_INTERFACE); + return _connection_get_setting_by_meta_type_check(connection, + NM_META_SETTING_TYPE_OVS_INTERFACE); } /** @@ -3281,7 +3290,7 @@ nm_connection_get_setting_ovs_interface(NMConnection *connection) NMSettingOvsPatch * nm_connection_get_setting_ovs_patch(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_OVS_PATCH); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_OVS_PATCH); } /** @@ -3297,7 +3306,7 @@ nm_connection_get_setting_ovs_patch(NMConnection *connection) NMSettingOvsPort * nm_connection_get_setting_ovs_port(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_OVS_PORT); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_OVS_PORT); } /** @@ -3311,7 +3320,7 @@ nm_connection_get_setting_ovs_port(NMConnection *connection) NMSettingPpp * nm_connection_get_setting_ppp(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_PPP); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_PPP); } /** @@ -3325,7 +3334,7 @@ nm_connection_get_setting_ppp(NMConnection *connection) NMSettingPppoe * nm_connection_get_setting_pppoe(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_PPPOE); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_PPPOE); } /** @@ -3341,7 +3350,7 @@ nm_connection_get_setting_pppoe(NMConnection *connection) NMSettingProxy * nm_connection_get_setting_proxy(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_PROXY); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_PROXY); } /** @@ -3355,7 +3364,7 @@ nm_connection_get_setting_proxy(NMConnection *connection) NMSettingSerial * nm_connection_get_setting_serial(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_SERIAL); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_SERIAL); } /** @@ -3371,7 +3380,7 @@ nm_connection_get_setting_serial(NMConnection *connection) NMSettingTCConfig * nm_connection_get_setting_tc_config(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_TC_CONFIG); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_TC_CONFIG); } /** @@ -3387,7 +3396,7 @@ nm_connection_get_setting_tc_config(NMConnection *connection) NMSettingTun * nm_connection_get_setting_tun(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_TUN); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_TUN); } /** @@ -3401,7 +3410,7 @@ nm_connection_get_setting_tun(NMConnection *connection) NMSettingVpn * nm_connection_get_setting_vpn(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_VPN); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_VPN); } /** @@ -3417,7 +3426,7 @@ nm_connection_get_setting_vpn(NMConnection *connection) NMSettingVxlan * nm_connection_get_setting_vxlan(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_VXLAN); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_VXLAN); } /** @@ -3431,7 +3440,7 @@ nm_connection_get_setting_vxlan(NMConnection *connection) NMSettingWimax * nm_connection_get_setting_wimax(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_WIMAX); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_WIMAX); } /** @@ -3445,7 +3454,7 @@ nm_connection_get_setting_wimax(NMConnection *connection) NMSettingWired * nm_connection_get_setting_wired(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_WIRED); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_WIRED); } /** @@ -3459,7 +3468,7 @@ nm_connection_get_setting_wired(NMConnection *connection) NMSettingAdsl * nm_connection_get_setting_adsl(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_ADSL); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_ADSL); } /** @@ -3473,7 +3482,7 @@ nm_connection_get_setting_adsl(NMConnection *connection) NMSettingWireless * nm_connection_get_setting_wireless(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_WIRELESS); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_WIRELESS); } /** @@ -3487,7 +3496,8 @@ nm_connection_get_setting_wireless(NMConnection *connection) NMSettingWirelessSecurity * nm_connection_get_setting_wireless_security(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_WIRELESS_SECURITY); + return _connection_get_setting_by_meta_type_check(connection, + NM_META_SETTING_TYPE_WIRELESS_SECURITY); } /** @@ -3501,7 +3511,7 @@ nm_connection_get_setting_wireless_security(NMConnection *connection) NMSettingBridgePort * nm_connection_get_setting_bridge_port(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_BRIDGE_PORT); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_BRIDGE_PORT); } /** @@ -3515,7 +3525,7 @@ nm_connection_get_setting_bridge_port(NMConnection *connection) NMSettingVlan * nm_connection_get_setting_vlan(NMConnection *connection) { - return _connection_get_setting_check(connection, NM_TYPE_SETTING_VLAN); + return _connection_get_setting_by_meta_type_check(connection, NM_META_SETTING_TYPE_VLAN); } NMSettingBluetooth * From d829849a7b0bac21f6903f2f3946ff62084be761 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 11 Jun 2021 17:45:42 +0200 Subject: [PATCH 08/22] libnm: avoid cloning list of settings in nm_connection_to_dbus_full() --- src/libnm-core-impl/nm-connection.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c index 5145003554..851c710016 100644 --- a/src/libnm-core-impl/nm-connection.c +++ b/src/libnm-core-impl/nm-connection.c @@ -2453,19 +2453,22 @@ nm_connection_to_dbus_full(NMConnection * connection, NMConnectionSerializationFlags flags, const NMConnectionSerializationOptions *options) { - GVariantBuilder builder; - gboolean any = FALSE; - gs_free NMSetting **settings = NULL; - guint settings_len = 0; - guint i; + NMConnectionPrivate *priv; + GVariantBuilder builder; + gboolean any = FALSE; + int i; g_return_val_if_fail(NM_IS_CONNECTION(connection), NULL); - settings = nm_connection_get_settings(connection, &settings_len); - for (i = 0; i < settings_len; i++) { - NMSetting *setting = settings[i]; + priv = NM_CONNECTION_GET_PRIVATE(connection); + + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + NMSetting *setting = priv->settings[nm_meta_setting_types_by_priority[i]]; GVariant * setting_dict; + if (!setting) + continue; + setting_dict = _nm_setting_to_dbus(setting, connection, flags, options); if (!setting_dict) continue; From 207b101238f3f4ace9536961b537da51438d4973 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 11 Jun 2021 19:09:23 +0200 Subject: [PATCH 09/22] libnm: take reference to settings in nm_connection_for_each_setting_value() As we iterate over the settings, let's ensure that they stay alive while we call back to the user data. --- src/libnm-core-impl/nm-connection.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c index 851c710016..1e4300e6cb 100644 --- a/src/libnm-core-impl/nm-connection.c +++ b/src/libnm-core-impl/nm-connection.c @@ -2589,14 +2589,19 @@ nm_connection_for_each_setting_value(NMConnection * connection, gpointer user_data) { gs_free NMSetting **settings = NULL; - guint i, length = 0; + guint length = 0; + guint i; g_return_if_fail(NM_IS_CONNECTION(connection)); g_return_if_fail(func); settings = nm_connection_get_settings(connection, &length); + for (i = 1; i < length; i++) + g_object_ref(settings[i]); for (i = 0; i < length; i++) nm_setting_enumerate_values(settings[i], func, user_data); + for (i = 1; i < length; i++) + g_object_unref(settings[i]); } /** From 5aef93355fe8edeb55fb3ba1b457650888e37d37 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 14 Jun 2021 09:06:15 +0200 Subject: [PATCH 10/22] libnm: add _nm_connection_get_settings_arr() helper --- src/libnm-core-impl/nm-connection.c | 8 ++++++++ src/libnm-core-intern/nm-core-internal.h | 2 ++ 2 files changed, 10 insertions(+) diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c index 1e4300e6cb..3ab2cafcb2 100644 --- a/src/libnm-core-impl/nm-connection.c +++ b/src/libnm-core-impl/nm-connection.c @@ -2520,6 +2520,14 @@ _nmtst_nm_setting_sort(NMSetting *a, NMSetting *b) return 0; } +NMSetting ** +_nm_connection_get_settings_arr(NMConnection *connection) +{ + nm_assert(NM_IS_CONNECTION(connection)); + + return NM_CONNECTION_GET_PRIVATE(connection)->settings; +} + /** * nm_connection_get_settings: * @connection: the #NMConnection instance diff --git a/src/libnm-core-intern/nm-core-internal.h b/src/libnm-core-intern/nm-core-internal.h index d01e432c68..9f2eadf4ab 100644 --- a/src/libnm-core-intern/nm-core-internal.h +++ b/src/libnm-core-intern/nm-core-internal.h @@ -180,6 +180,8 @@ NM_TERNARY_TO_OPTION_BOOL(NMTernary v) /*****************************************************************************/ +NMSetting **_nm_connection_get_settings_arr(NMConnection *connection); + typedef enum { /*< skip >*/ NM_SETTING_PARSE_FLAGS_NONE = 0, NM_SETTING_PARSE_FLAGS_STRICT = 1LL << 0, From b0f4bb84bff0bfb90050e856d573ec0f6bef3897 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 14 Jun 2021 09:09:07 +0200 Subject: [PATCH 11/22] libnm: avoid cloning buffer for nm_connection_get_settings() in nm_keyfile_write() --- src/libnm-core-impl/nm-keyfile.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libnm-core-impl/nm-keyfile.c b/src/libnm-core-impl/nm-keyfile.c index 91fd1f3ec7..88aa0d46cd 100644 --- a/src/libnm-core-impl/nm-keyfile.c +++ b/src/libnm-core-impl/nm-keyfile.c @@ -4083,10 +4083,9 @@ nm_keyfile_write(NMConnection * connection, nm_auto_unref_keyfile GKeyFile *keyfile = NULL; GError * local = NULL; KeyfileWriterInfo info; - gs_free NMSetting **settings = NULL; - guint n_settings = 0; - guint i; - guint j; + NMSetting ** settings; + int i; + guint j; g_return_val_if_fail(NM_IS_CONNECTION(connection), NULL); g_return_val_if_fail(!error || !*error, NULL); @@ -4124,13 +4123,16 @@ nm_keyfile_write(NMConnection * connection, .user_data = user_data, }; - settings = nm_connection_get_settings(connection, &n_settings); - for (i = 0; i < n_settings; i++) { + settings = _nm_connection_get_settings_arr(connection); + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { + NMSetting * setting = settings[nm_meta_setting_types_by_priority[i]]; const NMSettInfoSetting *sett_info; - NMSetting * setting = settings[i]; const char * setting_name; const char * setting_alias; + if (!setting) + continue; + sett_info = _nm_setting_class_get_sett_info(NM_SETTING_GET_CLASS(setting)); setting_name = sett_info->setting_class->setting_info->setting_name; From f3abf2491ad491f6a4beaeae45aa1034cbc9adc8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 14 Jun 2021 09:26:25 +0200 Subject: [PATCH 12/22] libnm: add code comment about preserving ABI for libnm GObject structs --- src/libnm-core-impl/nm-setting-8021x.c | 2 +- src/libnm-core-impl/nm-setting-adsl.c | 2 +- src/libnm-core-impl/nm-setting-bluetooth.c | 2 +- src/libnm-core-impl/nm-setting-bond.c | 2 +- src/libnm-core-impl/nm-setting-bridge-port.c | 2 +- src/libnm-core-impl/nm-setting-cdma.c | 2 +- src/libnm-core-impl/nm-setting-connection.c | 2 +- src/libnm-core-impl/nm-setting-dcb.c | 2 +- src/libnm-core-impl/nm-setting-dummy.c | 2 +- src/libnm-core-impl/nm-setting-generic.c | 2 +- src/libnm-core-impl/nm-setting-gsm.c | 2 +- src/libnm-core-impl/nm-setting-infiniband.c | 2 +- src/libnm-core-impl/nm-setting-ip-tunnel.c | 2 +- src/libnm-core-impl/nm-setting-ip4-config.c | 2 +- src/libnm-core-impl/nm-setting-ip6-config.c | 2 +- src/libnm-core-impl/nm-setting-macsec.c | 2 +- src/libnm-core-impl/nm-setting-macvlan.c | 2 +- src/libnm-core-impl/nm-setting-olpc-mesh.c | 2 +- src/libnm-core-impl/nm-setting-ppp.c | 2 +- src/libnm-core-impl/nm-setting-pppoe.c | 2 +- src/libnm-core-impl/nm-setting-private.h | 13 +++---------- src/libnm-core-impl/nm-setting-serial.c | 2 +- src/libnm-core-impl/nm-setting-team-port.c | 2 +- src/libnm-core-impl/nm-setting-team.c | 2 +- src/libnm-core-impl/nm-setting-tun.c | 2 +- src/libnm-core-impl/nm-setting-vlan.c | 2 +- src/libnm-core-impl/nm-setting-vpn.c | 2 +- src/libnm-core-impl/nm-setting-vxlan.c | 2 +- src/libnm-core-impl/nm-setting-wimax.c | 2 +- src/libnm-core-impl/nm-setting-wired.c | 2 +- src/libnm-core-impl/nm-setting-wireless-security.c | 2 +- src/libnm-core-impl/nm-setting-wireless.c | 2 +- src/libnm-core-impl/nm-simple-connection.c | 2 +- 33 files changed, 35 insertions(+), 42 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-8021x.c b/src/libnm-core-impl/nm-setting-8021x.c index de330a92ef..78a31e9c39 100644 --- a/src/libnm-core-impl/nm-setting-8021x.c +++ b/src/libnm-core-impl/nm-setting-8021x.c @@ -187,11 +187,11 @@ typedef struct { */ struct _NMSetting8021x { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSetting8021xClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-adsl.c b/src/libnm-core-impl/nm-setting-adsl.c index 4223e37d8d..8a82fb724c 100644 --- a/src/libnm-core-impl/nm-setting-adsl.c +++ b/src/libnm-core-impl/nm-setting-adsl.c @@ -47,11 +47,11 @@ typedef struct { */ struct _NMSettingAdsl { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingAdslClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-bluetooth.c b/src/libnm-core-impl/nm-setting-bluetooth.c index b0fe71e4db..4bd91a691a 100644 --- a/src/libnm-core-impl/nm-setting-bluetooth.c +++ b/src/libnm-core-impl/nm-setting-bluetooth.c @@ -43,11 +43,11 @@ typedef struct { */ struct _NMSettingBluetooth { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingBluetoothClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-bond.c b/src/libnm-core-impl/nm-setting-bond.c index eaf1712fcc..93baa9d689 100644 --- a/src/libnm-core-impl/nm-setting-bond.c +++ b/src/libnm-core-impl/nm-setting-bond.c @@ -45,11 +45,11 @@ typedef struct { */ struct _NMSettingBond { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingBondClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-bridge-port.c b/src/libnm-core-impl/nm-setting-bridge-port.c index a23ab68656..9efb1563c3 100644 --- a/src/libnm-core-impl/nm-setting-bridge-port.c +++ b/src/libnm-core-impl/nm-setting-bridge-port.c @@ -46,11 +46,11 @@ typedef struct { */ struct _NMSettingBridgePort { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingBridgePortClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-cdma.c b/src/libnm-core-impl/nm-setting-cdma.c index caec308287..6dc61e8385 100644 --- a/src/libnm-core-impl/nm-setting-cdma.c +++ b/src/libnm-core-impl/nm-setting-cdma.c @@ -43,11 +43,11 @@ typedef struct { */ struct _NMSettingCdma { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingCdmaClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-connection.c b/src/libnm-core-impl/nm-setting-connection.c index 3288916701..19573db478 100644 --- a/src/libnm-core-impl/nm-setting-connection.c +++ b/src/libnm-core-impl/nm-setting-connection.c @@ -104,11 +104,11 @@ typedef struct { */ struct _NMSettingConnection { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingConnectionClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-dcb.c b/src/libnm-core-impl/nm-setting-dcb.c index 0b176708c2..89df5835e8 100644 --- a/src/libnm-core-impl/nm-setting-dcb.c +++ b/src/libnm-core-impl/nm-setting-dcb.c @@ -72,11 +72,11 @@ typedef struct { */ struct _NMSettingDcb { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingDcbClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-dummy.c b/src/libnm-core-impl/nm-setting-dummy.c index 486dea9d43..a998a27b14 100644 --- a/src/libnm-core-impl/nm-setting-dummy.c +++ b/src/libnm-core-impl/nm-setting-dummy.c @@ -28,11 +28,11 @@ */ struct _NMSettingDummy { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingDummyClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-generic.c b/src/libnm-core-impl/nm-setting-generic.c index 379428675a..0eff1ddf49 100644 --- a/src/libnm-core-impl/nm-setting-generic.c +++ b/src/libnm-core-impl/nm-setting-generic.c @@ -34,11 +34,11 @@ typedef struct { */ struct _NMSettingGeneric { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingGenericClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-gsm.c b/src/libnm-core-impl/nm-setting-gsm.c index 400dc3a82a..a1f0500e4d 100644 --- a/src/libnm-core-impl/nm-setting-gsm.c +++ b/src/libnm-core-impl/nm-setting-gsm.c @@ -62,11 +62,11 @@ typedef struct { */ struct _NMSettingGsm { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingGsmClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-infiniband.c b/src/libnm-core-impl/nm-setting-infiniband.c index 5563f0b52a..95dc7b6696 100644 --- a/src/libnm-core-impl/nm-setting-infiniband.c +++ b/src/libnm-core-impl/nm-setting-infiniband.c @@ -47,11 +47,11 @@ typedef struct { */ struct _NMSettingInfiniband { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingInfinibandClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-ip-tunnel.c b/src/libnm-core-impl/nm-setting-ip-tunnel.c index 4a7276f9c1..68135eea81 100644 --- a/src/libnm-core-impl/nm-setting-ip-tunnel.c +++ b/src/libnm-core-impl/nm-setting-ip-tunnel.c @@ -54,11 +54,11 @@ typedef struct { */ struct _NMSettingIPTunnel { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingIPTunnelClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-ip4-config.c b/src/libnm-core-impl/nm-setting-ip4-config.c index 7b71ffe33e..003e8dc131 100644 --- a/src/libnm-core-impl/nm-setting-ip4-config.c +++ b/src/libnm-core-impl/nm-setting-ip4-config.c @@ -52,11 +52,11 @@ typedef struct { */ struct _NMSettingIP4Config { NMSettingIPConfig parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingIP4ConfigClass { NMSettingIPConfigClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-ip6-config.c b/src/libnm-core-impl/nm-setting-ip6-config.c index d2229e1a90..d2a016fe25 100644 --- a/src/libnm-core-impl/nm-setting-ip6-config.c +++ b/src/libnm-core-impl/nm-setting-ip6-config.c @@ -59,11 +59,11 @@ typedef struct { */ struct _NMSettingIP6Config { NMSettingIPConfig parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingIP6ConfigClass { NMSettingIPConfigClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-macsec.c b/src/libnm-core-impl/nm-setting-macsec.c index 2385917307..9593419304 100644 --- a/src/libnm-core-impl/nm-setting-macsec.c +++ b/src/libnm-core-impl/nm-setting-macsec.c @@ -56,11 +56,11 @@ typedef struct { */ struct _NMSettingMacsec { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingMacsecClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-macvlan.c b/src/libnm-core-impl/nm-setting-macvlan.c index 5ae0f8497d..98f943e9cd 100644 --- a/src/libnm-core-impl/nm-setting-macvlan.c +++ b/src/libnm-core-impl/nm-setting-macvlan.c @@ -41,11 +41,11 @@ typedef struct { */ struct _NMSettingMacvlan { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingMacvlanClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-olpc-mesh.c b/src/libnm-core-impl/nm-setting-olpc-mesh.c index 2121ee795b..103f619519 100644 --- a/src/libnm-core-impl/nm-setting-olpc-mesh.c +++ b/src/libnm-core-impl/nm-setting-olpc-mesh.c @@ -40,11 +40,11 @@ typedef struct { */ struct _NMSettingOlpcMesh { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingOlpcMeshClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-ppp.c b/src/libnm-core-impl/nm-setting-ppp.c index ce111ccb59..73ffad647e 100644 --- a/src/libnm-core-impl/nm-setting-ppp.c +++ b/src/libnm-core-impl/nm-setting-ppp.c @@ -69,11 +69,11 @@ typedef struct { */ struct _NMSettingPpp { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingPppClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-pppoe.c b/src/libnm-core-impl/nm-setting-pppoe.c index 1839305033..12beb51a5b 100644 --- a/src/libnm-core-impl/nm-setting-pppoe.c +++ b/src/libnm-core-impl/nm-setting-pppoe.c @@ -44,11 +44,11 @@ typedef struct { */ struct _NMSettingPppoe { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingPppoeClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-private.h b/src/libnm-core-impl/nm-setting-private.h index 339921e65e..4185629e22 100644 --- a/src/libnm-core-impl/nm-setting-private.h +++ b/src/libnm-core-impl/nm-setting-private.h @@ -27,12 +27,14 @@ */ struct _NMSetting { GObject parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingClass { GObjectClass parent; - /* Virtual functions */ + /* In the past, this struct was public API. Preserve ABI! */ + int (*verify)(NMSetting *setting, NMConnection *connection, GError **error); gboolean (*verify_secrets)(NMSetting *setting, NMConnection *connection, GError **error); @@ -51,7 +53,6 @@ struct _NMSettingClass { NMSettingSecretFlags flags, GError ** error); - /* In the past, this struct was public API. Preserve ABI! */ gboolean (*clear_secrets)(const struct _NMSettInfoSetting *sett_info, guint property_idx, NMSetting * setting, @@ -64,7 +65,6 @@ struct _NMSettingClass { * * @other may be %NULL, in which case the function only determines whether * the setting should be compared (TRUE) or not (DEFAULT). */ - /* In the past, this struct was public API. Preserve ABI! */ NMTernary (*compare_property)(const struct _NMSettInfoSetting *sett_info, guint property_idx, NMConnection * con_a, @@ -73,21 +73,17 @@ struct _NMSettingClass { NMSetting * set_b, NMSettingCompareFlags flags); - /* In the past, this struct was public API. Preserve ABI! */ void (*duplicate_copy_properties)(const struct _NMSettInfoSetting *sett_info, NMSetting * src, NMSetting * dst); - /* In the past, this struct was public API. Preserve ABI! */ void (*enumerate_values)(const struct _NMSettInfoProperty *property_info, NMSetting * setting, NMSettingValueIterFn func, gpointer user_data); - /* In the past, this struct was public API. Preserve ABI! */ gboolean (*aggregate)(NMSetting *setting, int type_i, gpointer arg); - /* In the past, this struct was public API. Preserve ABI! */ void (*for_each_secret)(NMSetting * setting, const char * secret_name, GVariant * val, @@ -96,7 +92,6 @@ struct _NMSettingClass { gpointer callback_data, GVariantBuilder * setting_builder); - /* In the past, this struct was public API. Preserve ABI! */ gboolean (*init_from_dbus)(NMSetting * setting, GHashTable * keys, GVariant * setting_dict, @@ -104,10 +99,8 @@ struct _NMSettingClass { guint /* NMSettingParseFlags */ parse_flags, GError ** error); - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[1]; - /* In the past, this struct was public API. Preserve ABI! */ const struct _NMMetaSettingInfo *setting_info; }; diff --git a/src/libnm-core-impl/nm-setting-serial.c b/src/libnm-core-impl/nm-setting-serial.c index 44dbd43c64..5a3802a3e7 100644 --- a/src/libnm-core-impl/nm-setting-serial.c +++ b/src/libnm-core-impl/nm-setting-serial.c @@ -43,11 +43,11 @@ typedef struct { */ struct _NMSettingSerial { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingSerialClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-team-port.c b/src/libnm-core-impl/nm-setting-team-port.c index 1e45d731b3..316fc67c96 100644 --- a/src/libnm-core-impl/nm-setting-team-port.c +++ b/src/libnm-core-impl/nm-setting-team-port.c @@ -42,11 +42,11 @@ typedef struct { */ struct _NMSettingTeamPort { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingTeamPortClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-team.c b/src/libnm-core-impl/nm-setting-team.c index d90d940826..b9a368c51d 100644 --- a/src/libnm-core-impl/nm-setting-team.c +++ b/src/libnm-core-impl/nm-setting-team.c @@ -742,11 +742,11 @@ typedef struct { */ struct _NMSettingTeam { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingTeamClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-tun.c b/src/libnm-core-impl/nm-setting-tun.c index e989e0be8f..be953e695e 100644 --- a/src/libnm-core-impl/nm-setting-tun.c +++ b/src/libnm-core-impl/nm-setting-tun.c @@ -47,11 +47,11 @@ typedef struct { */ struct _NMSettingTun { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingTunClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-vlan.c b/src/libnm-core-impl/nm-setting-vlan.c index 3ac0d64763..541186ecf3 100644 --- a/src/libnm-core-impl/nm-setting-vlan.c +++ b/src/libnm-core-impl/nm-setting-vlan.c @@ -48,11 +48,11 @@ typedef struct { */ struct _NMSettingVlan { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingVlanClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-vpn.c b/src/libnm-core-impl/nm-setting-vpn.c index 139860974b..d4a99d4da0 100644 --- a/src/libnm-core-impl/nm-setting-vpn.c +++ b/src/libnm-core-impl/nm-setting-vpn.c @@ -81,11 +81,11 @@ typedef struct { */ struct _NMSettingVpn { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingVpnClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-vxlan.c b/src/libnm-core-impl/nm-setting-vxlan.c index b65d2a2a4e..2a89b0c32e 100644 --- a/src/libnm-core-impl/nm-setting-vxlan.c +++ b/src/libnm-core-impl/nm-setting-vxlan.c @@ -67,11 +67,11 @@ typedef struct { */ struct _NMSettingVxlan { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingVxlanClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-wimax.c b/src/libnm-core-impl/nm-setting-wimax.c index e16e25279b..6d9f3284fa 100644 --- a/src/libnm-core-impl/nm-setting-wimax.c +++ b/src/libnm-core-impl/nm-setting-wimax.c @@ -42,11 +42,11 @@ typedef struct { */ struct _NMSettingWimax { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingWimaxClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-wired.c b/src/libnm-core-impl/nm-setting-wired.c index f00a272009..36d254b3a5 100644 --- a/src/libnm-core-impl/nm-setting-wired.c +++ b/src/libnm-core-impl/nm-setting-wired.c @@ -76,11 +76,11 @@ typedef struct { */ struct _NMSettingWired { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingWiredClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-wireless-security.c b/src/libnm-core-impl/nm-setting-wireless-security.c index 069309f8c8..df618e36d9 100644 --- a/src/libnm-core-impl/nm-setting-wireless-security.c +++ b/src/libnm-core-impl/nm-setting-wireless-security.c @@ -91,11 +91,11 @@ typedef struct { */ struct _NMSettingWirelessSecurity { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingWirelessSecurityClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-setting-wireless.c b/src/libnm-core-impl/nm-setting-wireless.c index fe8eaa6427..92209aec7f 100644 --- a/src/libnm-core-impl/nm-setting-wireless.c +++ b/src/libnm-core-impl/nm-setting-wireless.c @@ -73,11 +73,11 @@ typedef struct { */ struct _NMSettingWireless { NMSetting parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSettingWirelessClass { NMSettingClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; diff --git a/src/libnm-core-impl/nm-simple-connection.c b/src/libnm-core-impl/nm-simple-connection.c index 7b133e4551..185d47fc4f 100644 --- a/src/libnm-core-impl/nm-simple-connection.c +++ b/src/libnm-core-impl/nm-simple-connection.c @@ -25,11 +25,11 @@ */ struct _NMSimpleConnection { GObject parent; + /* In the past, this struct was public API. Preserve ABI! */ }; struct _NMSimpleConnectionClass { GObjectClass parent; - /* In the past, this struct was public API. Preserve ABI! */ gpointer padding[4]; }; From 7a71aedf46e59e66bba4cbb304617ee85200f4c3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 14 Jun 2021 19:29:18 +0200 Subject: [PATCH 13/22] libnm: optimize NM_CONNECTION_GET_PRIVATE() for NMSimpleConnection NMConnection is a glib interface, implemented only by NMSimpleConnection and NMRemoteConnection. Inside the daemon, every NMConnection instance is always a NMSimpleConnection. Using glib interfaces has an overhead, for example NM_IS_CONNECTION() needs to search the implemented types for the pointer. And NM_CONNECTION_GET_PRIVATE() is implemented by attaching user data to the GObject instance. Both have measurable overhead. Special case them for NMSimpleConnection. This optimizes primarily the call to nm_connection_get_setting_connection(), which easily gets called millions of times. This is easily measurable. --- src/libnm-core-impl/nm-connection.c | 136 ++++++++++++++------- src/libnm-core-impl/nm-setting-private.h | 16 +++ src/libnm-core-impl/nm-simple-connection.c | 31 ++++- 3 files changed, 133 insertions(+), 50 deletions(-) diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c index 3ab2cafcb2..5555a20a51 100644 --- a/src/libnm-core-impl/nm-connection.c +++ b/src/libnm-core-impl/nm-connection.c @@ -46,19 +46,98 @@ enum { static guint signals[LAST_SIGNAL] = {0}; -typedef struct { - NMConnection *self; - - NMSetting *settings[_NM_META_SETTING_TYPE_NUM]; - - /* D-Bus path of the connection, if any */ - char *path; -} NMConnectionPrivate; - G_DEFINE_INTERFACE(NMConnection, nm_connection, G_TYPE_OBJECT) -static NMConnectionPrivate *nm_connection_get_private(NMConnection *connection); -#define NM_CONNECTION_GET_PRIVATE(o) (nm_connection_get_private((NMConnection *) o)) +/*****************************************************************************/ + +static gboolean _nm_connection_clear_settings(NMConnection *connection, NMConnectionPrivate *priv); + +/*****************************************************************************/ + +#undef NM_IS_SIMPLE_CONNECTION +#define NM_IS_SIMPLE_CONNECTION(self) \ + ({ \ + gconstpointer _self1 = (self); \ + gboolean _result; \ + \ + _result = \ + (_self1 \ + && (((GTypeInstance *) _self1)->g_class == _nm_simple_connection_class_instance)); \ + \ + nm_assert(_result == G_TYPE_CHECK_INSTANCE_TYPE(_self1, NM_TYPE_SIMPLE_CONNECTION)); \ + \ + _result; \ + }) + +#undef NM_IS_CONNECTION +#define NM_IS_CONNECTION(self) \ + ({ \ + gconstpointer _self0 = (self); \ + \ + (_self0 \ + && (NM_IS_SIMPLE_CONNECTION(_self0) \ + || G_TYPE_CHECK_INSTANCE_TYPE(_self0, NM_TYPE_CONNECTION))); \ + }) + +/*****************************************************************************/ + +void +_nm_connection_private_clear(NMConnectionPrivate *priv) +{ + if (priv->self) { + _nm_connection_clear_settings(priv->self, priv); + nm_clear_g_free(&priv->path); + priv->self = NULL; + } +} + +static void +_nm_connection_private_free(gpointer data) +{ + NMConnectionPrivate *priv = data; + + _nm_connection_private_clear(priv); + + nm_g_slice_free(priv); +} + +static NMConnectionPrivate * +_nm_connection_get_private_from_qdata(NMConnection *connection) +{ + GQuark key; + NMConnectionPrivate *priv; + + nm_assert(NM_IS_CONNECTION(connection)); + nm_assert(!NM_IS_SIMPLE_CONNECTION(connection)); + + key = NM_CACHED_QUARK("NMConnectionPrivate"); + + priv = g_object_get_qdata((GObject *) connection, key); + if (G_UNLIKELY(!priv)) { + priv = g_slice_new(NMConnectionPrivate); + *priv = (NMConnectionPrivate){ + .self = connection, + }; + g_object_set_qdata_full((GObject *) connection, key, priv, _nm_connection_private_free); + } + + return priv; +} + +#define NM_CONNECTION_GET_PRIVATE(connection) \ + ({ \ + NMConnection * _connection = (connection); \ + NMConnectionPrivate *_priv; \ + \ + if (G_LIKELY(NM_IS_SIMPLE_CONNECTION(_connection))) \ + _priv = (gpointer) (&(((char *) _connection)[_nm_simple_connection_private_offset])); \ + else \ + _priv = _nm_connection_get_private_from_qdata(_connection); \ + \ + nm_assert(_priv && _priv->self == _connection); \ + \ + _priv; \ + }) /*****************************************************************************/ @@ -3557,41 +3636,6 @@ _nm_connection_get_setting_bluetooth_for_nap(NMConnection *connection) /*****************************************************************************/ -static void -nm_connection_private_free(NMConnectionPrivate *priv) -{ - NMConnection *self = priv->self; - - _nm_connection_clear_settings(self, priv); - g_free(priv->path); - - g_slice_free(NMConnectionPrivate, priv); -} - -static NMConnectionPrivate * -nm_connection_get_private(NMConnection *connection) -{ - GQuark key; - NMConnectionPrivate *priv; - - nm_assert(NM_IS_CONNECTION(connection)); - - key = NM_CACHED_QUARK("NMConnectionPrivate"); - - priv = g_object_get_qdata((GObject *) connection, key); - if (G_UNLIKELY(!priv)) { - priv = g_slice_new0(NMConnectionPrivate); - g_object_set_qdata_full((GObject *) connection, - key, - priv, - (GDestroyNotify) nm_connection_private_free); - - priv->self = connection; - } - - return priv; -} - static void nm_connection_default_init(NMConnectionInterface *iface) { diff --git a/src/libnm-core-impl/nm-setting-private.h b/src/libnm-core-impl/nm-setting-private.h index 4185629e22..e8373f8e63 100644 --- a/src/libnm-core-impl/nm-setting-private.h +++ b/src/libnm-core-impl/nm-setting-private.h @@ -19,6 +19,22 @@ /*****************************************************************************/ +typedef struct { + NMConnection *self; + + NMSetting *settings[_NM_META_SETTING_TYPE_NUM]; + + /* D-Bus path of the connection, if any */ + char *path; +} NMConnectionPrivate; + +extern GTypeClass *_nm_simple_connection_class_instance; +extern int _nm_simple_connection_private_offset; + +void _nm_connection_private_clear(NMConnectionPrivate *priv); + +/*****************************************************************************/ + /** * NMSetting: * diff --git a/src/libnm-core-impl/nm-simple-connection.c b/src/libnm-core-impl/nm-simple-connection.c index 185d47fc4f..6b5b118413 100644 --- a/src/libnm-core-impl/nm-simple-connection.c +++ b/src/libnm-core-impl/nm-simple-connection.c @@ -20,6 +20,11 @@ /*****************************************************************************/ +GTypeClass *_nm_simple_connection_class_instance = NULL; +int _nm_simple_connection_private_offset; + +/*****************************************************************************/ + /** * NMSimpleConnection: */ @@ -42,11 +47,20 @@ G_DEFINE_TYPE_WITH_CODE(NMSimpleConnection, G_IMPLEMENT_INTERFACE(NM_TYPE_CONNECTION, nm_simple_connection_interface_init);) +#define _GET_PRIVATE(self) \ + G_TYPE_INSTANCE_GET_PRIVATE(self, NM_TYPE_SIMPLE_CONNECTION, NMConnectionPrivate) + /*****************************************************************************/ static void nm_simple_connection_init(NMSimpleConnection *self) -{} +{ + NMConnectionPrivate *priv; + + priv = _GET_PRIVATE(self); + + priv->self = (NMConnection *) self; +} /** * nm_simple_connection_new: @@ -143,22 +157,31 @@ nm_simple_connection_new_clone(NMConnection *connection) static void dispose(GObject *object) { + NMConnection *connection = NM_CONNECTION(object); + #if NM_MORE_ASSERTS g_signal_handlers_disconnect_by_data(object, (gpointer) &_nmtst_connection_unchanging_user_data); #endif - nm_connection_clear_secrets(NM_CONNECTION(object)); + nm_connection_clear_secrets(connection); + + _nm_connection_private_clear(_GET_PRIVATE(connection)); G_OBJECT_CLASS(nm_simple_connection_parent_class)->dispose(object); } static void -nm_simple_connection_class_init(NMSimpleConnectionClass *simple_class) +nm_simple_connection_class_init(NMSimpleConnectionClass *klass) { - GObjectClass *object_class = G_OBJECT_CLASS(simple_class); + GObjectClass *object_class = G_OBJECT_CLASS(klass); + + g_type_class_add_private(klass, sizeof(NMConnectionPrivate)); object_class->dispose = dispose; + + _nm_simple_connection_private_offset = g_type_class_get_instance_private_offset(klass); + _nm_simple_connection_class_instance = (GTypeClass *) klass; } static void From 85df025e93673d2aabfbaa639100a8ffed591f57 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 15 Jun 2021 00:47:59 +0200 Subject: [PATCH 14/22] core: avoid undefined behavior comparing plain pointer values in _cmp_last_resort() --- src/core/settings/nm-settings-connection.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/core/settings/nm-settings-connection.c b/src/core/settings/nm-settings-connection.c index e05fb0751f..6d3666b8ca 100644 --- a/src/core/settings/nm-settings-connection.c +++ b/src/core/settings/nm-settings-connection.c @@ -2084,7 +2084,9 @@ _cmp_last_resort(NMSettingsConnection *a, NMSettingsConnection *b) /* hm, same UUID. Use their pointer value to give them a stable * order. */ - return (a > b) ? -1 : 1; + NM_CMP_DIRECT_PTR(a, b); + + return nm_assert_unreachable_val(0); } /* sorting for "best" connections. From f51d3862f95c5b67d8f8cf2d0d093182a2cd94ee Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 15 Jun 2021 08:37:56 +0200 Subject: [PATCH 15/22] glib-aux: add nm_utils_ptrarray_is_sorted() helper --- src/libnm-glib-aux/nm-shared-utils.c | 22 ++++++++++++++++++++++ src/libnm-glib-aux/nm-shared-utils.h | 6 ++++++ 2 files changed, 28 insertions(+) diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c index 7a49298ec3..ff1b705006 100644 --- a/src/libnm-glib-aux/nm-shared-utils.c +++ b/src/libnm-glib-aux/nm-shared-utils.c @@ -3948,6 +3948,28 @@ nm_utils_ptrarray_find_first(gconstpointer *list, gssize len, gconstpointer need /*****************************************************************************/ +gboolean +nm_utils_ptrarray_is_sorted(gconstpointer * list, + gsize len, + gboolean require_strict, + GCompareDataFunc cmpfcn, + gpointer user_data) +{ + gsize i; + + for (i = 1; i < len; i++) { + int c; + + c = cmpfcn(list[i - 1], list[i], user_data); + if (G_LIKELY(c < 0)) + continue; + + if (c > 0 || require_strict) + return FALSE; + } + return TRUE; +} + gssize nm_utils_ptrarray_find_binary_search(gconstpointer * list, gsize len, diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index 2e4215fdf7..d33cc3d177 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -2143,6 +2143,12 @@ nm_g_hash_table_remove(GHashTable *hash, gconstpointer key) /*****************************************************************************/ +gboolean nm_utils_ptrarray_is_sorted(gconstpointer * list, + gsize len, + gboolean require_strict, + GCompareDataFunc cmpfcn, + gpointer user_data); + gssize nm_utils_ptrarray_find_binary_search(gconstpointer * list, gsize len, gconstpointer needle, From 1f09e13f43af23cc3eb8a83ee01e5fd63a815ac4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 15 Jun 2021 09:44:28 +0200 Subject: [PATCH 16/22] core: add nm_settings_connection_cmp_autoconnect_priority_with_data() helper --- src/core/settings/nm-settings-connection.c | 9 +++++++++ src/core/settings/nm-settings-connection.h | 3 +++ 2 files changed, 12 insertions(+) diff --git a/src/core/settings/nm-settings-connection.c b/src/core/settings/nm-settings-connection.c index 6d3666b8ca..520dcc24a0 100644 --- a/src/core/settings/nm-settings-connection.c +++ b/src/core/settings/nm-settings-connection.c @@ -2127,6 +2127,15 @@ nm_settings_connection_cmp_autoconnect_priority(NMSettingsConnection *a, NMSetti return _cmp_last_resort(a, b); } +int +nm_settings_connection_cmp_autoconnect_priority_with_data(gconstpointer pa, + gconstpointer pb, + gpointer user_data) +{ + return nm_settings_connection_cmp_autoconnect_priority((NMSettingsConnection *) pa, + (NMSettingsConnection *) pb); +} + int nm_settings_connection_cmp_autoconnect_priority_p_with_data(gconstpointer pa, gconstpointer pb, diff --git a/src/core/settings/nm-settings-connection.h b/src/core/settings/nm-settings-connection.h index aed4ff614d..83a6a7f62b 100644 --- a/src/core/settings/nm-settings-connection.h +++ b/src/core/settings/nm-settings-connection.h @@ -323,6 +323,9 @@ int nm_settings_connection_cmp_timestamp_p_with_data(gconstpointer pa, gpointer user_data); int nm_settings_connection_cmp_autoconnect_priority(NMSettingsConnection *a, NMSettingsConnection *b); +int nm_settings_connection_cmp_autoconnect_priority_with_data(gconstpointer pa, + gconstpointer pb, + gpointer user_data); int nm_settings_connection_cmp_autoconnect_priority_p_with_data(gconstpointer pa, gconstpointer pb, gpointer user_data); From e7b5650eff4eb257cd4544fa89eb53c0554c3025 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 15 Jun 2021 00:41:45 +0200 Subject: [PATCH 17/22] core: add nm_settings_get_connection_sorted_by_autoconnect_priority() Turns out, we call nm_settings_get_connection_clone() *a lot* with sort order nm_settings_connection_cmp_autoconnect_priority_p_with_data(). As we cache the (differently sorted) list of connections, also cache the presorted list. The only complication is that every time we still need to check whether the list is still sorted, because it would be more complicated to invalidate the list when an entry changes which affects the sort order. Still, such a check is usually successful and requires "only" N-1 comparisons. --- src/core/nm-manager.c | 80 ++++++++++----------------- src/core/settings/nm-settings.c | 97 ++++++++++++++++++++++++++++----- src/core/settings/nm-settings.h | 3 + 3 files changed, 115 insertions(+), 65 deletions(-) diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index 419ce88dfe..b66f7fbb37 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -1999,16 +1999,16 @@ nm_manager_remove_device(NMManager *self, const char *ifname, NMDeviceType devic static NMDevice * system_create_virtual_device(NMManager *self, NMConnection *connection) { - NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE(self); - NMDeviceFactory * factory; - gs_free NMSettingsConnection **connections = NULL; - guint i; - gs_free char * iface = NULL; - const char * parent_spec; - NMDevice * device = NULL, *parent = NULL; - NMDevice * dev_candidate; - GError * error = NULL; - NMLogLevel log_level; + NMManagerPrivate * priv = NM_MANAGER_GET_PRIVATE(self); + NMDeviceFactory * factory; + NMSettingsConnection *const *connections; + guint i; + gs_free char * iface = NULL; + const char * parent_spec; + NMDevice * device = NULL, *parent = NULL; + NMDevice * dev_candidate; + GError * error = NULL; + NMLogLevel log_level; g_return_val_if_fail(NM_IS_MANAGER(self), NULL); g_return_val_if_fail(NM_IS_CONNECTION(connection), NULL); @@ -2091,13 +2091,7 @@ system_create_virtual_device(NMManager *self, NMConnection *connection) } /* Create backing resources if the device has any autoconnect connections */ - connections = nm_settings_get_connections_clone( - priv->settings, - NULL, - NULL, - NULL, - nm_settings_connection_cmp_autoconnect_priority_p_with_data, - NULL); + connections = nm_settings_get_connections_sorted_by_autoconnect_priority(priv->settings, NULL); for (i = 0; connections[i]; i++) { NMConnection * candidate = nm_settings_connection_get_connection(connections[i]); NMSettingConnection *s_con; @@ -2136,19 +2130,13 @@ system_create_virtual_device(NMManager *self, NMConnection *connection) static void retry_connections_for_parent_device(NMManager *self, NMDevice *device) { - NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE(self); - gs_free NMSettingsConnection **connections = NULL; - guint i; + NMManagerPrivate * priv = NM_MANAGER_GET_PRIVATE(self); + NMSettingsConnection *const *connections; + guint i; g_return_if_fail(device); - connections = nm_settings_get_connections_clone( - priv->settings, - NULL, - NULL, - NULL, - nm_settings_connection_cmp_autoconnect_priority_p_with_data, - NULL); + connections = nm_settings_get_connections_sorted_by_autoconnect_priority(priv->settings, NULL); for (i = 0; connections[i]; i++) { NMSettingsConnection *sett_conn = connections[i]; NMConnection * connection = nm_settings_connection_get_connection(sett_conn); @@ -4425,13 +4413,13 @@ find_slaves(NMManager * manager, guint * out_n_slaves, gboolean for_user_request) { - NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE(manager); - gs_free NMSettingsConnection **all_connections = NULL; - guint n_all_connections; - guint i; - SlaveConnectionInfo * slaves = NULL; - guint n_slaves = 0; - NMSettingConnection * s_con; + NMManagerPrivate * priv = NM_MANAGER_GET_PRIVATE(manager); + NMSettingsConnection *const *all_connections = NULL; + guint n_all_connections; + guint i; + SlaveConnectionInfo * slaves = NULL; + guint n_slaves = 0; + NMSettingConnection * s_con; gs_unref_hashtable GHashTable *devices = NULL; nm_assert(out_n_slaves); @@ -4445,13 +4433,9 @@ find_slaves(NMManager * manager, * even if a slave was already active, it might be deactivated during * master reactivation. */ - all_connections = nm_settings_get_connections_clone( - priv->settings, - &n_all_connections, - NULL, - NULL, - nm_settings_connection_cmp_autoconnect_priority_p_with_data, - NULL); + all_connections = + nm_settings_get_connections_sorted_by_autoconnect_priority(priv->settings, + &n_all_connections); for (i = 0; i < n_all_connections; i++) { NMSettingsConnection *master_connection = NULL; NMDevice * master_device = NULL, *slave_device; @@ -6918,9 +6902,9 @@ devices_inited_cb(gpointer user_data) gboolean nm_manager_start(NMManager *self, GError **error) { - NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE(self); - gs_free NMSettingsConnection **connections = NULL; - guint i; + NMManagerPrivate * priv = NM_MANAGER_GET_PRIVATE(self); + NMSettingsConnection *const *connections; + guint i; nm_device_factory_manager_load_factories(_register_device_factory, self); @@ -6978,13 +6962,7 @@ nm_manager_start(NMManager *self, GError **error) NM_SETTINGS_SIGNAL_CONNECTION_UPDATED, G_CALLBACK(connection_updated_cb), self); - connections = nm_settings_get_connections_clone( - priv->settings, - NULL, - NULL, - NULL, - nm_settings_connection_cmp_autoconnect_priority_p_with_data, - NULL); + connections = nm_settings_get_connections_sorted_by_autoconnect_priority(priv->settings, NULL); for (i = 0; connections[i]; i++) connection_changed(self, connections[i]); diff --git a/src/core/settings/nm-settings.c b/src/core/settings/nm-settings.c index 36a357748e..bb58af9899 100644 --- a/src/core/settings/nm-settings.c +++ b/src/core/settings/nm-settings.c @@ -371,6 +371,7 @@ typedef struct { CList connections_lst_head; NMSettingsConnection **connections_cached_list; + NMSettingsConnection **connections_cached_list_sorted_by_autoconnect_priority; GSList *unmanaged_specs; GSList *unrecognized_specs; @@ -2873,22 +2874,37 @@ impl_settings_reload_connections(NMDBusObject * obj, static void _clear_connections_cached_list(NMSettingsPrivate *priv) { - if (!priv->connections_cached_list) - return; - - nm_assert(priv->connections_len == NM_PTRARRAY_LEN(priv->connections_cached_list)); + if (priv->connections_cached_list) { + nm_assert(priv->connections_len == NM_PTRARRAY_LEN(priv->connections_cached_list)); #if NM_MORE_ASSERTS - /* set the pointer to a bogus value. This makes it more apparent - * if somebody has a reference to the cached list and still uses - * it. That is a bug, this code just tries to make it blow up - * more eagerly. */ - memset(priv->connections_cached_list, - 0x43, - sizeof(NMSettingsConnection *) * (priv->connections_len + 1)); + /* set the pointer to a bogus value. This makes it more apparent + * if somebody has a reference to the cached list and still uses + * it. That is a bug, this code just tries to make it blow up + * more eagerly. */ + memset(priv->connections_cached_list, + 0x43, + sizeof(NMSettingsConnection *) * (priv->connections_len + 1)); #endif - nm_clear_g_free(&priv->connections_cached_list); + nm_clear_g_free(&priv->connections_cached_list); + } + if (priv->connections_cached_list_sorted_by_autoconnect_priority) { + nm_assert(priv->connections_len + == NM_PTRARRAY_LEN(priv->connections_cached_list_sorted_by_autoconnect_priority)); + +#if NM_MORE_ASSERTS + /* set the pointer to a bogus value. This makes it more apparent + * if somebody has a reference to the cached list and still uses + * it. That is a bug, this code just tries to make it blow up + * more eagerly. */ + memset(priv->connections_cached_list_sorted_by_autoconnect_priority, + 0x42, + sizeof(NMSettingsConnection *) * (priv->connections_len + 1)); +#endif + + nm_clear_g_free(&priv->connections_cached_list_sorted_by_autoconnect_priority); + } } static void @@ -3027,6 +3043,55 @@ nm_settings_get_connections(NMSettings *self, guint *out_len) return priv->connections_cached_list; } +NMSettingsConnection *const * +nm_settings_get_connections_sorted_by_autoconnect_priority(NMSettings *self, guint *out_len) +{ + NMSettingsPrivate *priv; + gboolean needs_sort = FALSE; + + g_return_val_if_fail(NM_IS_SETTINGS(self), NULL); + + priv = NM_SETTINGS_GET_PRIVATE(self); + + nm_assert(priv->connections_len == c_list_length(&priv->connections_lst_head)); + nm_assert( + !priv->connections_cached_list_sorted_by_autoconnect_priority + || (priv->connections_len + == NM_PTRARRAY_LEN(priv->connections_cached_list_sorted_by_autoconnect_priority))); + + if (!priv->connections_cached_list_sorted_by_autoconnect_priority) { + NMSettingsConnection *const *list_cached; + guint len; + + list_cached = nm_settings_get_connections(self, &len); + priv->connections_cached_list_sorted_by_autoconnect_priority = + nm_memdup(list_cached, sizeof(NMSettingsConnection *) * (len + 1)); + needs_sort = (len > 1); + } else if (!nm_utils_ptrarray_is_sorted( + (gconstpointer *) priv->connections_cached_list_sorted_by_autoconnect_priority, + priv->connections_len, + FALSE, + nm_settings_connection_cmp_autoconnect_priority_with_data, + NULL)) { + /* We cache the sorted list, but we don't monitor all entries whether they + * get modified to invalidate the sort order. So every time we have to check + * whether the sort order is still correct. The vast majority of the time it + * is, and this check is faster than sorting anew. */ + needs_sort = TRUE; + } + + if (needs_sort) { + g_qsort_with_data(priv->connections_cached_list_sorted_by_autoconnect_priority, + priv->connections_len, + sizeof(NMSettingsConnection *), + nm_settings_connection_cmp_autoconnect_priority_p_with_data, + NULL); + } + + NM_SET_OUT(out_len, priv->connections_len); + return priv->connections_cached_list_sorted_by_autoconnect_priority; +} + /** * nm_settings_get_connections_clone: * @self: the #NMSetting @@ -3058,9 +3123,13 @@ nm_settings_get_connections_clone(NMSettings * self, g_return_val_if_fail(NM_IS_SETTINGS(self), NULL); - list_cached = nm_settings_get_connections(self, &len); + if (sort_compare_func == nm_settings_connection_cmp_autoconnect_priority_p_with_data) { + list_cached = nm_settings_get_connections_sorted_by_autoconnect_priority(self, &len); + sort_compare_func = NULL; + } else + list_cached = nm_settings_get_connections(self, &len); -#if NM_MORE_ASSERTS +#if NM_MORE_ASSERTS > 10 nm_assert(list_cached); for (i = 0; i < len; i++) nm_assert(NM_IS_SETTINGS_CONNECTION(list_cached[i])); diff --git a/src/core/settings/nm-settings.h b/src/core/settings/nm-settings.h index 09a0af57f6..fbbc957477 100644 --- a/src/core/settings/nm-settings.h +++ b/src/core/settings/nm-settings.h @@ -79,6 +79,9 @@ void nm_settings_add_connection_dbus(NMSettings * self, NMSettingsConnection *const *nm_settings_get_connections(NMSettings *settings, guint *out_len); +NMSettingsConnection *const * +nm_settings_get_connections_sorted_by_autoconnect_priority(NMSettings *self, guint *out_len); + NMSettingsConnection **nm_settings_get_connections_clone(NMSettings * self, guint * out_len, NMSettingsConnectionFilterFunc func, From 60957a4c8afdbfce0ba6bfb10f61c231578daee7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 15 Jun 2021 10:27:29 +0200 Subject: [PATCH 18/22] libnm: add helper functions for emitting signals in NMConnection Not very useful, but it seems nicer to read. They anyway can be inlined. After all, naming and structure is important and the places where we emit signals are important. By having well-named helper functions, these places are easier to find and reason about. --- src/libnm-core-impl/nm-connection.c | 32 ++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c index 5555a20a51..e212894785 100644 --- a/src/libnm-core-impl/nm-connection.c +++ b/src/libnm-core-impl/nm-connection.c @@ -148,7 +148,21 @@ _signal_emit_changed(NMConnection *self) } static void -setting_changed_cb(NMSetting *setting, GParamSpec *pspec, NMConnection *self) +_signal_emit_secrets_updated(NMConnection *self, const char *setting_name) +{ + g_signal_emit(self, signals[SECRETS_UPDATED], 0, setting_name); +} + +static void +_signal_emit_secrets_cleared(NMConnection *self) +{ + g_signal_emit(self, signals[SECRETS_CLEARED], 0); +} + +/*****************************************************************************/ + +static void +_setting_notify_changed_cb(NMSetting *setting, GParamSpec *pspec, NMConnection *self) { _signal_emit_changed(self); } @@ -156,27 +170,31 @@ setting_changed_cb(NMSetting *setting, GParamSpec *pspec, NMConnection *self) static void _setting_notify_connect(NMConnection *connection, NMSetting *setting) { - g_signal_connect(setting, "notify", (GCallback) setting_changed_cb, connection); + g_signal_connect(setting, "notify", G_CALLBACK(_setting_notify_changed_cb), connection); } static void _setting_notify_disconnect(NMConnection *connection, NMSetting *setting) { - g_signal_handlers_disconnect_by_func(setting, setting_changed_cb, connection); + g_signal_handlers_disconnect_by_func(setting, + G_CALLBACK(_setting_notify_changed_cb), + connection); } static void _setting_notify_block(NMConnection *connection, NMSetting *setting) { - g_signal_handlers_block_by_func(setting, (GCallback) setting_changed_cb, connection); + g_signal_handlers_block_by_func(setting, G_CALLBACK(_setting_notify_changed_cb), connection); } static void _setting_notify_unblock(NMConnection *connection, NMSetting *setting) { - g_signal_handlers_unblock_by_func(setting, (GCallback) setting_changed_cb, connection); + g_signal_handlers_unblock_by_func(setting, G_CALLBACK(_setting_notify_changed_cb), connection); } +/*****************************************************************************/ + static gboolean _nm_connection_clear_settings(NMConnection *connection, NMConnectionPrivate *priv) { @@ -2252,7 +2270,7 @@ nm_connection_update_secrets(NMConnection *connection, } if (updated) - g_signal_emit(connection, signals[SECRETS_UPDATED], 0, setting_name); + _signal_emit_secrets_updated(connection, setting_name); return success; } @@ -2361,7 +2379,7 @@ nm_connection_clear_secrets_with_flags(NMConnection * connecti _setting_notify_unblock(connection, setting); } - g_signal_emit(connection, signals[SECRETS_CLEARED], 0); + _signal_emit_secrets_cleared(connection); } static gboolean From 92daaff7d1ee0655a5f62d5d0d57aee44121bbab Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 15 Jun 2021 12:21:00 +0200 Subject: [PATCH 19/22] glib-aux: add NM_STRV_EMPTY_CC() helper macro --- src/libnm-glib-aux/nm-shared-utils.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index d33cc3d177..aced71f985 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -556,6 +556,7 @@ extern const void *const _NM_PTRARRAY_EMPTY[1]; #define NM_PTRARRAY_EMPTY(type) ((type const *) _NM_PTRARRAY_EMPTY) #define NM_STRV_EMPTY() ((char **) _NM_PTRARRAY_EMPTY) +#define NM_STRV_EMPTY_CC() NM_PTRARRAY_EMPTY(const char *) static inline void _nm_utils_strbuf_init(char *buf, gsize len, char **p_buf_ptr, gsize *p_buf_len) From cea52c7cbdf4c71311e9a680f6ccf8c1c6ddce9d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 15 Jun 2021 12:36:58 +0200 Subject: [PATCH 20/22] libnm: add nm_connection_serialization_options_equal() helper --- src/libnm-core-impl/nm-connection.c | 35 ++++++++++++++++++++++++ src/libnm-core-intern/nm-core-internal.h | 5 +++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c index e212894785..ed864db8dc 100644 --- a/src/libnm-core-impl/nm-connection.c +++ b/src/libnm-core-impl/nm-connection.c @@ -2527,6 +2527,38 @@ _nm_connection_find_secret(NMConnection * self, /*****************************************************************************/ +static const NMConnectionSerializationOptions _connection_serialization_options_empty = { + .timestamp = + { + .has = FALSE, + .val = 0, + }, + .seen_bssids = NULL, +}; + +gboolean +nm_connection_serialization_options_equal(const NMConnectionSerializationOptions *a, + const NMConnectionSerializationOptions *b) +{ + if (!a) + a = &_connection_serialization_options_empty; + if (!b) + b = &_connection_serialization_options_empty; + + if (a == b) + return TRUE; + + if (a->timestamp.has != b->timestamp.has) + return FALSE; + if (a->timestamp.has && a->timestamp.val != b->timestamp.val) + return FALSE; + if (!nm_utils_strv_equal(a->seen_bssids ?: NM_STRV_EMPTY_CC(), + b->seen_bssids ?: NM_STRV_EMPTY_CC())) + return FALSE; + + return TRUE; +} + /** * nm_connection_to_dbus: * @connection: the #NMConnection @@ -2559,6 +2591,9 @@ nm_connection_to_dbus_full(NMConnection * connection, priv = NM_CONNECTION_GET_PRIVATE(connection); + if (!options) + options = &_connection_serialization_options_empty; + for (i = 0; i < (int) _NM_META_SETTING_TYPE_NUM; i++) { NMSetting *setting = priv->settings[nm_meta_setting_types_by_priority[i]]; GVariant * setting_dict; diff --git a/src/libnm-core-intern/nm-core-internal.h b/src/libnm-core-intern/nm-core-internal.h index 9f2eadf4ab..5ea68ea3bd 100644 --- a/src/libnm-core-intern/nm-core-internal.h +++ b/src/libnm-core-intern/nm-core-internal.h @@ -207,10 +207,13 @@ typedef struct { bool has; } timestamp; - const char **seen_bssids; + const char *const *seen_bssids; } NMConnectionSerializationOptions; +gboolean nm_connection_serialization_options_equal(const NMConnectionSerializationOptions *a, + const NMConnectionSerializationOptions *b); + GVariant *nm_connection_to_dbus_full(NMConnection * connection, NMConnectionSerializationFlags flags, const NMConnectionSerializationOptions *options); From 252e4a676b80da3a4ea383517bd90cf7824554ea Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 15 Jun 2021 12:45:35 +0200 Subject: [PATCH 21/22] core: cache GVariant for result of GetSettings() The GetSettings() call is not the only place where we convert a NMConnection to D-Bus. However it is one of the most prominent ones with a measurable performance overhead. The connection seldom changes, so it makes sense to cache it. Note that GetSettings() is the only caller that specifies an option, thus it's the only caller that converts a NMConnection to variant in this particular way. That means, other callers don't benefit from this caching and we could not cache the variant in the NMConnection instance itself, because those callers use different parameters. --- src/core/settings/nm-settings-connection.c | 67 ++++++++++++++++++++-- 1 file changed, 62 insertions(+), 5 deletions(-) diff --git a/src/core/settings/nm-settings-connection.c b/src/core/settings/nm-settings-connection.c index 520dcc24a0..71431f29d9 100644 --- a/src/core/settings/nm-settings-connection.c +++ b/src/core/settings/nm-settings-connection.c @@ -80,6 +80,11 @@ typedef struct _NMSettingsConnectionPrivate { NMConnection *connection; + struct { + NMConnectionSerializationOptions options; + GVariant * variant; + } getsettings_cached; + NMSettingsStorage *storage; char *filename; @@ -249,6 +254,57 @@ _seen_bssids_hash_new(void) /*****************************************************************************/ +static void +_getsettings_cached_clear(NMSettingsConnectionPrivate *priv) +{ + if (nm_clear_pointer(&priv->getsettings_cached.variant, g_variant_unref)) { + priv->getsettings_cached.options.timestamp.has = FALSE; + priv->getsettings_cached.options.timestamp.val = 0; + nm_clear_g_free((gpointer *) &priv->getsettings_cached.options.seen_bssids); + } +} + +static GVariant * +_getsettings_cached_get(NMSettingsConnection *self, const NMConnectionSerializationOptions *options) +{ + NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE(self); + GVariant * variant; + + if (priv->getsettings_cached.variant) { + if (nm_connection_serialization_options_equal(&priv->getsettings_cached.options, options)) { +#if NM_MORE_ASSERTS > 10 + gs_unref_variant GVariant *variant2 = NULL; + + variant = nm_connection_to_dbus_full(priv->connection, + NM_CONNECTION_SERIALIZE_WITH_NON_SECRET, + options); + nm_assert(variant); + variant2 = g_variant_new("(@a{sa{sv}})", variant); + nm_assert(g_variant_equal(priv->getsettings_cached.variant, variant2)); +#endif + return priv->getsettings_cached.variant; + } + _getsettings_cached_clear(priv); + } + + nm_assert(!priv->getsettings_cached.options.seen_bssids); + + variant = nm_connection_to_dbus_full(priv->connection, + NM_CONNECTION_SERIALIZE_WITH_NON_SECRET, + options); + nm_assert(variant); + + priv->getsettings_cached.variant = g_variant_ref_sink(g_variant_new("(@a{sa{sv}})", variant)); + + priv->getsettings_cached.options = *options; + priv->getsettings_cached.options.seen_bssids = + nm_utils_strv_dup_packed(priv->getsettings_cached.options.seen_bssids, -1); + + return priv->getsettings_cached.variant; +} + +/*****************************************************************************/ + NMConnection * nm_settings_connection_get_connection(NMSettingsConnection *self) { @@ -280,6 +336,8 @@ _nm_settings_connection_set_connection(NMSettingsConnection * self, priv->connection = g_object_ref(new_connection); nmtst_connection_assert_unchanging(priv->connection); + _getsettings_cached_clear(priv); + /* note that we only return @connection_old if the new connection actually differs from * before. * @@ -1243,7 +1301,6 @@ get_settings_auth_cb(NMSettingsConnection * self, { gs_free const char ** seen_bssids = NULL; NMConnectionSerializationOptions options = {}; - GVariant * settings; if (error) { g_dbus_method_invocation_return_gerror(context, error); @@ -1270,10 +1327,8 @@ get_settings_auth_cb(NMSettingsConnection * self, * get returned by the GetSecrets method which can be better * protected against leakage of secrets to unprivileged callers. */ - settings = nm_connection_to_dbus_full(nm_settings_connection_get_connection(self), - NM_CONNECTION_SERIALIZE_WITH_NON_SECRET, - &options); - g_dbus_method_invocation_return_value(context, g_variant_new("(@a{sa{sv}})", settings)); + + g_dbus_method_invocation_return_value(context, _getsettings_cached_get(self, &options)); } static void @@ -2620,6 +2675,8 @@ dispose(GObject *object) g_clear_object(&priv->connection); + _getsettings_cached_clear(priv); + nm_clear_pointer(&priv->kf_db_timestamps, nm_key_file_db_unref); nm_clear_pointer(&priv->kf_db_seen_bssids, nm_key_file_db_unref); From 877d2b236f8f5ffd0d09c0955bfa75cbf05fc15f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 17 Jun 2021 19:29:33 +0200 Subject: [PATCH 22/22] core: avoid checking sort order for cached settings list We now have a cached list of NMSettingsConnection instances, sorted by their autoconnect priority. However, the sort order nm_settings_connection_cmp_autoconnect_priority() depends on various properties of the connection: - "connection.autoconnect" and "connection.autoconnect-priority" - the timestamp - "connection.uuid" These properties almost never change, so it's a waste that every call to nm_settings_get_connections_sorted_by_autoconnect_priority() needs to check whether the sort order is still satisfied. We can do better by tracking when the sort order might have been destroyed and only check in those (much fewer) cases. Note that we end up calling nm_settings_get_connections_sorted_by_autoconnect_priority() a lot, so this makes a difference. --- src/core/settings/nm-settings-connection.c | 3 ++ src/core/settings/nm-settings.c | 46 ++++++++++++++++------ src/core/settings/nm-settings.h | 2 + 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/src/core/settings/nm-settings-connection.c b/src/core/settings/nm-settings-connection.c index 71431f29d9..641f3297b8 100644 --- a/src/core/settings/nm-settings-connection.c +++ b/src/core/settings/nm-settings-connection.c @@ -337,6 +337,7 @@ _nm_settings_connection_set_connection(NMSettingsConnection * self, nmtst_connection_assert_unchanging(priv->connection); _getsettings_cached_clear(priv); + _nm_settings_notify_sorted_by_autoconnect_priority_maybe_changed(priv->settings); /* note that we only return @connection_old if the new connection actually differs from * before. @@ -2251,6 +2252,8 @@ nm_settings_connection_update_timestamp(NMSettingsConnection *self, guint64 time _LOGT("timestamp: set timestamp %" G_GUINT64_FORMAT, timestamp); + _nm_settings_notify_sorted_by_autoconnect_priority_maybe_changed(priv->settings); + if (!priv->kf_db_timestamps) return; diff --git a/src/core/settings/nm-settings.c b/src/core/settings/nm-settings.c index bb58af9899..c876ea148c 100644 --- a/src/core/settings/nm-settings.c +++ b/src/core/settings/nm-settings.c @@ -390,6 +390,12 @@ typedef struct { bool started : 1; + /* Whether NMSettingsConnections changed in a way that affects the comparison + * with nm_settings_connection_cmp_autoconnect_priority_with_data(). In that case, + * we may need to re-sort the connections_cached_list_sorted_by_autoconnect_priority + * list. */ + bool sorted_by_autoconnect_priority_maybe_changed : 1; + } NMSettingsPrivate; struct _NMSettings { @@ -2871,6 +2877,14 @@ impl_settings_reload_connections(NMDBusObject * obj, /*****************************************************************************/ +void +_nm_settings_notify_sorted_by_autoconnect_priority_maybe_changed(NMSettings *self) +{ + NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE(self); + + priv->sorted_by_autoconnect_priority_maybe_changed = TRUE; +} + static void _clear_connections_cached_list(NMSettingsPrivate *priv) { @@ -3067,19 +3081,29 @@ nm_settings_get_connections_sorted_by_autoconnect_priority(NMSettings *self, gui priv->connections_cached_list_sorted_by_autoconnect_priority = nm_memdup(list_cached, sizeof(NMSettingsConnection *) * (len + 1)); needs_sort = (len > 1); - } else if (!nm_utils_ptrarray_is_sorted( - (gconstpointer *) priv->connections_cached_list_sorted_by_autoconnect_priority, - priv->connections_len, - FALSE, - nm_settings_connection_cmp_autoconnect_priority_with_data, - NULL)) { - /* We cache the sorted list, but we don't monitor all entries whether they - * get modified to invalidate the sort order. So every time we have to check - * whether the sort order is still correct. The vast majority of the time it - * is, and this check is faster than sorting anew. */ - needs_sort = TRUE; + } else if (priv->sorted_by_autoconnect_priority_maybe_changed) { + if (!nm_utils_ptrarray_is_sorted( + (gconstpointer *) priv->connections_cached_list_sorted_by_autoconnect_priority, + priv->connections_len, + FALSE, + nm_settings_connection_cmp_autoconnect_priority_with_data, + NULL)) { + /* We cache the sorted list, but we don't monitor all entries whether they + * get modified to invalidate the sort order. So every time we have to check + * whether the sort order is still correct. The vast majority of the time it + * is, and this check is faster than sorting anew. */ + needs_sort = TRUE; + } + } else { + nm_assert(nm_utils_ptrarray_is_sorted( + (gconstpointer *) priv->connections_cached_list_sorted_by_autoconnect_priority, + priv->connections_len, + TRUE, + nm_settings_connection_cmp_autoconnect_priority_with_data, + NULL)); } + priv->sorted_by_autoconnect_priority_maybe_changed = FALSE; if (needs_sort) { g_qsort_with_data(priv->connections_cached_list_sorted_by_autoconnect_priority, priv->connections_len, diff --git a/src/core/settings/nm-settings.h b/src/core/settings/nm-settings.h index fbbc957477..ce18fecccf 100644 --- a/src/core/settings/nm-settings.h +++ b/src/core/settings/nm-settings.h @@ -130,4 +130,6 @@ const char *nm_settings_get_startup_complete_blocked_reason(NMSettings *self, void nm_settings_kf_db_write(NMSettings *settings); +void _nm_settings_notify_sorted_by_autoconnect_priority_maybe_changed(NMSettings *self); + #endif /* __NM_SETTINGS_H__ */