From 7b769e9e49b3a51e3fa96008cb766f158c7222de Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 12 Feb 2024 09:55:29 +0100 Subject: [PATCH 01/15] dispatcher: remove trailing dot from error messages The error messages are logged by the dispatcher and passed back to NetworkManager which also logs them. NetworkManager log messages usually don't end with a dot: remove it. --- src/nm-dispatcher/nm-dispatcher.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/nm-dispatcher/nm-dispatcher.c b/src/nm-dispatcher/nm-dispatcher.c index 97b85813dc..28ac51bf07 100644 --- a/src/nm-dispatcher/nm-dispatcher.c +++ b/src/nm-dispatcher/nm-dispatcher.c @@ -412,7 +412,7 @@ script_watch_cb(GPid pid, int status, gpointer user_data) script->result = DISPATCH_RESULT_SUCCESS; } else { status_desc = nm_utils_get_process_exit_status_desc(status); - script->error = g_strdup_printf("Script '%s' %s.", script->script, status_desc); + script->error = g_strdup_printf("Script '%s' %s", script->script, status_desc); } if (script->result == DISPATCH_RESULT_SUCCESS) { @@ -447,7 +447,7 @@ again: goto again; } - script->error = g_strdup_printf("Script '%s' timed out.", script->script); + script->error = g_strdup_printf("Script '%s' timed out", script->script); script->result = DISPATCH_RESULT_TIMEOUT; g_spawn_close_pid(script->pid); @@ -466,19 +466,19 @@ check_permissions(struct stat *s, const char **out_error_msg) /* Only accept files owned by root */ if (s->st_uid != 0) { - *out_error_msg = "not owned by root."; + *out_error_msg = "not owned by root"; return FALSE; } /* Only accept files not writable by group or other, and not SUID */ if (s->st_mode & (S_IWGRP | S_IWOTH | S_ISUID)) { - *out_error_msg = "writable by group or other, or set-UID."; + *out_error_msg = "writable by group or other, or set-UID"; return FALSE; } /* Only accept files executable by the owner */ if (!(s->st_mode & S_IXUSR)) { - *out_error_msg = "not executable by owner."; + *out_error_msg = "not executable by owner"; return FALSE; } From 38acb7a57d9736f65a6c2f4c7438b36f5ff4be47 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 8 Nov 2023 16:35:39 +0100 Subject: [PATCH 02/15] core: move functions for env variable name encoding to libnm-glib-aux They will be used by the dispatcher service. --- .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 2 +- .../plugins/ifcfg-rh/nms-ifcfg-rh-utils.c | 109 ----------------- .../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 2 +- src/libnm-glib-aux/nm-shared-utils.c | 111 ++++++++++++++++++ src/libnm-glib-aux/nm-shared-utils.h | 5 + 5 files changed, 118 insertions(+), 111 deletions(-) diff --git a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index 04e79725da..3bcbb71b94 100644 --- a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -1723,7 +1723,7 @@ make_user_setting(shvarFile *ifcfg) else g_string_set_size(str, 0); - if (!nms_ifcfg_rh_utils_user_key_decode(key + NM_STRLEN("NM_USER_"), str)) + if (!nm_utils_env_var_decode_name(key + NM_STRLEN("NM_USER_"), str)) continue; if (!s_user) diff --git a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c index 50e352d309..b4edefbbf9 100644 --- a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c +++ b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c @@ -398,115 +398,6 @@ utils_detect_ifcfg_path(const char *path, gboolean only_ifcfg) return utils_get_ifcfg_path(path); } -void -nms_ifcfg_rh_utils_user_key_encode(const char *key, GString *str_buffer) -{ - gsize i; - - nm_assert(key); - nm_assert(str_buffer); - - for (i = 0; key[i]; i++) { - char ch = key[i]; - - /* we encode the key in only upper case letters, digits, and underscore. - * As we expect lower-case letters to be more common, we encode lower-case - * letters as upper case, and upper-case letters with a leading underscore. */ - - if (ch >= '0' && ch <= '9') { - g_string_append_c(str_buffer, ch); - continue; - } - if (ch >= 'a' && ch <= 'z') { - g_string_append_c(str_buffer, ch - 'a' + 'A'); - continue; - } - if (ch == '.') { - g_string_append(str_buffer, "__"); - continue; - } - if (ch >= 'A' && ch <= 'Z') { - g_string_append_c(str_buffer, '_'); - g_string_append_c(str_buffer, ch); - continue; - } - g_string_append_printf(str_buffer, "_%03o", (unsigned) ch); - } -} - -gboolean -nms_ifcfg_rh_utils_user_key_decode(const char *name, GString *str_buffer) -{ - gsize i; - - nm_assert(name); - nm_assert(str_buffer); - - if (!name[0]) - return FALSE; - - for (i = 0; name[i];) { - char ch = name[i]; - - if (ch >= '0' && ch <= '9') { - g_string_append_c(str_buffer, ch); - i++; - continue; - } - if (ch >= 'A' && ch <= 'Z') { - g_string_append_c(str_buffer, ch - 'A' + 'a'); - i++; - continue; - } - - if (ch == '_') { - ch = name[i + 1]; - if (ch == '_') { - g_string_append_c(str_buffer, '.'); - i += 2; - continue; - } - if (ch >= 'A' && ch <= 'Z') { - g_string_append_c(str_buffer, ch); - i += 2; - continue; - } - if (ch >= '0' && ch <= '7') { - char ch2, ch3; - unsigned v; - - ch2 = name[i + 2]; - if (!(ch2 >= '0' && ch2 <= '7')) - return FALSE; - - ch3 = name[i + 3]; - if (!(ch3 >= '0' && ch3 <= '7')) - return FALSE; - -#define OCTAL_VALUE(ch) ((unsigned) ((ch) - '0')) - v = (OCTAL_VALUE(ch) << 6) + (OCTAL_VALUE(ch2) << 3) + OCTAL_VALUE(ch3); - if (v > 0xFF || v == 0) - return FALSE; - ch = (char) v; - if ((ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') || (ch == '.') - || (ch >= 'a' && ch <= 'z')) { - /* such characters are not expected to be encoded via - * octal representation. The encoding is invalid. */ - return FALSE; - } - g_string_append_c(str_buffer, ch); - i += 4; - continue; - } - return FALSE; - } - - return FALSE; - } - - return TRUE; -} - /*****************************************************************************/ const char *const _nm_ethtool_ifcfg_names[] = { diff --git a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c index 07e5e64d18..617c5ef60b 100644 --- a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c +++ b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c @@ -2604,7 +2604,7 @@ write_user_setting(NMConnection *connection, shvarFile *ifcfg, GError **error) g_string_set_size(str, 0); g_string_append(str, "NM_USER_"); - nms_ifcfg_rh_utils_user_key_encode(key, str); + nm_utils_env_var_encode_name(key, str); svSetValue(ifcfg, str->str, nm_setting_user_get_data(s_user, key)); } } diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c index 7d623bd9d3..421e4d1bef 100644 --- a/src/libnm-glib-aux/nm-shared-utils.c +++ b/src/libnm-glib-aux/nm-shared-utils.c @@ -7330,3 +7330,114 @@ nm_utils_poll_finish(GAsyncResult *result, gpointer *probe_user_data, GError **e return g_task_propagate_boolean(task, error); } + +/*****************************************************************************/ + +void +nm_utils_env_var_encode_name(const char *key, GString *str_buffer) +{ + gsize i; + + nm_assert(key); + nm_assert(str_buffer); + + for (i = 0; key[i]; i++) { + char ch = key[i]; + + /* we encode the key in only upper case letters, digits, and underscore. + * As we expect lower-case letters to be more common, we encode lower-case + * letters as upper case, and upper-case letters with a leading underscore. */ + + if (ch >= '0' && ch <= '9') { + g_string_append_c(str_buffer, ch); + continue; + } + if (ch >= 'a' && ch <= 'z') { + g_string_append_c(str_buffer, ch - 'a' + 'A'); + continue; + } + if (ch == '.') { + g_string_append(str_buffer, "__"); + continue; + } + if (ch >= 'A' && ch <= 'Z') { + g_string_append_c(str_buffer, '_'); + g_string_append_c(str_buffer, ch); + continue; + } + g_string_append_printf(str_buffer, "_%03o", (unsigned) ch); + } +} + +gboolean +nm_utils_env_var_decode_name(const char *name, GString *str_buffer) +{ + gsize i; + + nm_assert(name); + nm_assert(str_buffer); + + if (!name[0]) + return FALSE; + + for (i = 0; name[i];) { + char ch = name[i]; + + if (ch >= '0' && ch <= '9') { + g_string_append_c(str_buffer, ch); + i++; + continue; + } + if (ch >= 'A' && ch <= 'Z') { + g_string_append_c(str_buffer, ch - 'A' + 'a'); + i++; + continue; + } + + if (ch == '_') { + ch = name[i + 1]; + if (ch == '_') { + g_string_append_c(str_buffer, '.'); + i += 2; + continue; + } + if (ch >= 'A' && ch <= 'Z') { + g_string_append_c(str_buffer, ch); + i += 2; + continue; + } + if (ch >= '0' && ch <= '7') { + char ch2, ch3; + unsigned v; + + ch2 = name[i + 2]; + if (!(ch2 >= '0' && ch2 <= '7')) + return FALSE; + + ch3 = name[i + 3]; + if (!(ch3 >= '0' && ch3 <= '7')) + return FALSE; + +#define OCTAL_VALUE(ch) ((unsigned) ((ch) - '0')) + v = (OCTAL_VALUE(ch) << 6) + (OCTAL_VALUE(ch2) << 3) + OCTAL_VALUE(ch3); + if (v > 0xFF || v == 0) + return FALSE; + ch = (char) v; + if ((ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') || (ch == '.') + || (ch >= 'a' && ch <= 'z')) { + /* such characters are not expected to be encoded via + * octal representation. The encoding is invalid. */ + return FALSE; + } + g_string_append_c(str_buffer, ch); + i += 4; + continue; + } + return FALSE; + } + + return FALSE; + } + + return TRUE; +} diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index ea38e083cd..804034d237 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -3551,4 +3551,9 @@ void nm_utils_poll(int poll_timeout_ms, gboolean nm_utils_poll_finish(GAsyncResult *result, gpointer *probe_user_data, GError **error); +/*****************************************************************************/ + +void nm_utils_env_var_encode_name(const char *key, GString *str_buffer); +gboolean nm_utils_env_var_decode_name(const char *name, GString *str_buffer); + #endif /* __NM_SHARED_UTILS_H__ */ From d7c311eb85a6eff380013304ff13515ac5ee6f67 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 29 Sep 2023 14:14:34 +0200 Subject: [PATCH 03/15] dispatcher: pass user setting properties in the environment Properties in the "user" setting are a convenient way to associate any kind of user-provided metadata to connections. However, nmcli doesn't support the user setting at the moment and adding this feature requires a significant effort. Without nmcli support, dispatcher scripts can only access user properties by either parsing connection files or by using D-Bus (with or without libnm and GObject introspection). Since both these solutions are not very convenient, provide an alternative way: pass the properties as environment variables. --- man/NetworkManager-dispatcher.xml | 27 ++++++++++++++++++++++ src/nm-dispatcher/nm-dispatcher-utils.c | 30 +++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/man/NetworkManager-dispatcher.xml b/man/NetworkManager-dispatcher.xml index f6e1d22d76..e0c9b572d7 100644 --- a/man/NetworkManager-dispatcher.xml +++ b/man/NetworkManager-dispatcher.xml @@ -308,6 +308,33 @@ In case of VPN, VPN_IP_IFACE is set, and IP4_*, IP6_* variables with VPN prefix are exported too, like VPN_IP4_ADDRESS_0, VPN_IP4_NUM_ADDRESSES. + + The content of the user setting for the connection + being activated is also passed via environment variables. Each key is + stored in a variable with name CONNECTION_USER_ + concatenated with the encoding of the key name. The encoding works as + follows: + + + lowercase letters become uppercase + + + uppercase letters are prefixed with an underscore + + + numbers do not change + + + a dot is replaced with a double underscore + + + any other character is encoded with an underscore followed by + its 3-digit octal representation + + + For example, key test.foo-Bar2 is stored in a variable named + CONNECTION_USER_TEST__FOO_055_BAR2. + Dispatcher scripts are run one at a time, but asynchronously from the main NetworkManager process, and will be killed if they run for too long. If your script diff --git a/src/nm-dispatcher/nm-dispatcher-utils.c b/src/nm-dispatcher/nm-dispatcher-utils.c index f8a4c28000..6659936f48 100644 --- a/src/nm-dispatcher/nm-dispatcher-utils.c +++ b/src/nm-dispatcher/nm-dispatcher-utils.c @@ -540,6 +540,36 @@ nm_dispatcher_utils_construct_envp(const char *action, _items_add_key0(items, NULL, "DEVICE_IP_IFACE", ip_iface); } + { + gs_unref_variant GVariant *user_setting = NULL; + + user_setting = g_variant_lookup_value(connection_dict, + NM_SETTING_USER_SETTING_NAME, + NM_VARIANT_TYPE_SETTING); + if (user_setting) { + gs_unref_variant GVariant *data = NULL; + nm_auto_free_gstring GString *string = NULL; + GVariantIter iter; + const char *key; + const char *val; + + data = + g_variant_lookup_value(user_setting, NM_SETTING_USER_DATA, G_VARIANT_TYPE("a{ss}")); + if (data) { + g_variant_iter_init(&iter, data); + while (g_variant_iter_next(&iter, "{&s&s}", &key, &val)) { + if (key) { + if (!string) + string = g_string_sized_new(64); + g_string_assign(string, "CONNECTION_USER_"); + nm_utils_env_var_encode_name(key, string); + _items_add_key0(items, NULL, string->str, val); + } + } + } + } + } + /* Device items aren't valid if the device isn't activated */ if (iface && dev_state == NM_DEVICE_STATE_ACTIVATED) { construct_proxy_items(items, device_proxy_props, NULL); From e686ab35b3c44808e2b8788827b11e60dec3a413 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 18 Sep 2023 18:35:26 +0200 Subject: [PATCH 04/15] libnm: add generic.device-handler property Add a new "generic.device-handler" property that specifies the name of a dispatcher script to be invoked to add and delete the interface for this connection. --- man/NetworkManager-dispatcher.xml | 29 ++++ src/libnm-client-impl/libnm.ver | 1 + ...gen-metadata-nm-settings-libnm-core.xml.in | 4 + src/libnm-core-impl/nm-setting-generic.c | 128 +++++++++++++++++- src/libnm-core-public/nm-setting-generic.h | 5 + src/libnmc-setting/nm-meta-setting-desc.c | 11 +- src/libnmc-setting/settings-docs.h.in | 1 + src/nm-dispatcher/nm-dispatcher.c | 4 +- src/nmcli/connections.c | 19 +-- .../gen-metadata-nm-settings-nmcli.xml.in | 3 + 10 files changed, 190 insertions(+), 15 deletions(-) diff --git a/man/NetworkManager-dispatcher.xml b/man/NetworkManager-dispatcher.xml index e0c9b572d7..8d7f0c59ba 100644 --- a/man/NetworkManager-dispatcher.xml +++ b/man/NetworkManager-dispatcher.xml @@ -172,6 +172,35 @@ looking at file /run/NetworkManager/resolv.conf + + device-add + + + This action is called when a connection of type generic + has the generic.device-handler property set. The property + indicates the name of a dispatcher script to be executed in directory + /{etc,usr/lib}/NetworkManager/dispatcher.d/device. Note + that differently from other actions, only one script is executed. + + + The script needs to perform any action needed to create the device + for the generic connection. On successful termination, the script + returns zero. Otherwise, it returns a non-zero value to indicate an + error. + + + + + device-delete + + + This action is the counterpart of device-add and + is called to delete the device for a generic connection. All the + aspects described for device-add also apply to + this action. + + + The environment contains more information about the interface and the connection. diff --git a/src/libnm-client-impl/libnm.ver b/src/libnm-client-impl/libnm.ver index 0655d2dea4..5442377ae6 100644 --- a/src/libnm-client-impl/libnm.ver +++ b/src/libnm-client-impl/libnm.ver @@ -1959,6 +1959,7 @@ global: nm_setting_connection_get_autoconnect_ports; nm_setting_connection_get_controller; nm_setting_connection_get_port_type; + nm_setting_generic_get_device_handler; nm_setting_get_enum_property_type; nm_setting_hsr_get_multicast_spec; nm_setting_hsr_get_port1; diff --git a/src/libnm-core-impl/gen-metadata-nm-settings-libnm-core.xml.in b/src/libnm-core-impl/gen-metadata-nm-settings-libnm-core.xml.in index 57337c6069..146f92829e 100644 --- a/src/libnm-core-impl/gen-metadata-nm-settings-libnm-core.xml.in +++ b/src/libnm-core-impl/gen-metadata-nm-settings-libnm-core.xml.in @@ -1331,6 +1331,10 @@ + device_handler; +} + +static gboolean +verify(NMSetting *setting, NMConnection *connection, GError **error) +{ + NMSettingGenericPrivate *priv = NM_SETTING_GENERIC_GET_PRIVATE(setting); + + if (priv->device_handler) { + if (NM_IN_SET(priv->device_handler[0], '\0', '.') + || !NM_STRCHAR_ALL(priv->device_handler, + ch, + g_ascii_isalnum(ch) || NM_IN_SET(ch, '-', '_', '.'))) { + g_set_error_literal(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("property is invalid")); + g_prefix_error(error, + "%s.%s: ", + NM_SETTING_GENERIC_SETTING_NAME, + NM_SETTING_GENERIC_DEVICE_HANDLER); + return FALSE; + } + + if (connection) { + NMSettingConnection *s_con; + + s_con = nm_connection_get_setting_connection(connection); + if (!s_con) { + g_set_error(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_MISSING_SETTING, + _("missing setting")); + g_prefix_error(error, "%s: ", NM_SETTING_CONNECTION_SETTING_NAME); + return FALSE; + } + + if (!nm_setting_connection_get_interface_name(s_con)) { + g_set_error(error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_MISSING_PROPERTY, + _("the property is required when %s.%s is set"), + NM_SETTING_GENERIC_SETTING_NAME, + NM_SETTING_GENERIC_DEVICE_HANDLER); + g_prefix_error(error, + "%s.%s: ", + NM_SETTING_CONNECTION_SETTING_NAME, + NM_SETTING_CONNECTION_INTERFACE_NAME); + return FALSE; + } + } + } + + return TRUE; +} + /*****************************************************************************/ static void @@ -60,7 +143,46 @@ nm_setting_generic_new(void) static void nm_setting_generic_class_init(NMSettingGenericClass *klass) { - NMSettingClass *setting_class = NM_SETTING_CLASS(klass); + GObjectClass *object_class = G_OBJECT_CLASS(klass); + NMSettingClass *setting_class = NM_SETTING_CLASS(klass); + GArray *properties_override = _nm_sett_info_property_override_create_array(); - _nm_setting_class_commit(setting_class, NM_META_SETTING_TYPE_GENERIC, NULL, NULL, 0); + object_class->get_property = _nm_setting_property_get_property_direct; + object_class->set_property = _nm_setting_property_set_property_direct; + + setting_class->verify = verify; + + /** + * NMSettingGeneric:device-handler: + * + * Name of the device handler that will be invoked to add and delete + * the device for this connection. The name can only contain ASCII + * alphanumeric characters and '-', '_', '.'. It cannot start with '.'. + * + * See the NetworkManager-dispatcher(8) man page for more details + * about how to write the device handler. + * + * By setting this property the generic connection becomes "virtual", + * meaning that it can be activated without an existing device; the device + * will be created at the time the connection is started by invoking the + * device-handler. + * + * Since: 1.46 + **/ + _nm_setting_property_define_direct_string(properties_override, + obj_properties, + NM_SETTING_GENERIC_DEVICE_HANDLER, + PROP_DEVICE_HANDLER, + NM_SETTING_PARAM_FUZZY_IGNORE + | NM_SETTING_PARAM_INFERRABLE, + NMSettingGeneric, + _priv.device_handler); + + g_object_class_install_properties(object_class, _PROPERTY_ENUMS_LAST, obj_properties); + + _nm_setting_class_commit(setting_class, + NM_META_SETTING_TYPE_GENERIC, + NULL, + properties_override, + 0); } diff --git a/src/libnm-core-public/nm-setting-generic.h b/src/libnm-core-public/nm-setting-generic.h index 9bdcd11d5f..d735513ffc 100644 --- a/src/libnm-core-public/nm-setting-generic.h +++ b/src/libnm-core-public/nm-setting-generic.h @@ -27,12 +27,17 @@ G_BEGIN_DECLS #define NM_SETTING_GENERIC_SETTING_NAME "generic" +#define NM_SETTING_GENERIC_DEVICE_HANDLER "device-handler" + typedef struct _NMSettingGenericClass NMSettingGenericClass; GType nm_setting_generic_get_type(void); NMSetting *nm_setting_generic_new(void); +NM_AVAILABLE_IN_1_46 +const char *nm_setting_generic_get_device_handler(NMSettingGeneric *setting); + G_END_DECLS #endif /* __NM_SETTING_GENERIC_H__ */ diff --git a/src/libnmc-setting/nm-meta-setting-desc.c b/src/libnmc-setting/nm-meta-setting-desc.c index 16cc003a93..2871ccb6b0 100644 --- a/src/libnmc-setting/nm-meta-setting-desc.c +++ b/src/libnmc-setting/nm-meta-setting-desc.c @@ -5985,6 +5985,15 @@ static const NMMetaPropertyInfo *const property_infos_ETHTOOL[] = { NULL, }; +#undef _CURRENT_NM_META_SETTING_TYPE +#define _CURRENT_NM_META_SETTING_TYPE NM_META_SETTING_TYPE_GENERIC +static const NMMetaPropertyInfo *const property_infos_GENERIC[] = { + PROPERTY_INFO_WITH_DESC (NM_SETTING_GENERIC_DEVICE_HANDLER, + .property_type = &_pt_gobject_string, + ), + NULL +}; + #undef _CURRENT_NM_META_SETTING_TYPE #define _CURRENT_NM_META_SETTING_TYPE NM_META_SETTING_TYPE_GSM static const NMMetaPropertyInfo *const property_infos_GSM[] = { @@ -8782,7 +8791,7 @@ const NMMetaSettingInfoEditor nm_meta_setting_infos_editor[] = { NM_META_SETTING_VALID_PART_ITEM (ETHTOOL, FALSE), ), ), - SETTING_INFO_EMPTY (GENERIC, + SETTING_INFO (GENERIC, .valid_parts = NM_META_SETTING_VALID_PARTS ( NM_META_SETTING_VALID_PART_ITEM (CONNECTION, TRUE), NM_META_SETTING_VALID_PART_ITEM (GENERIC, TRUE), diff --git a/src/libnmc-setting/settings-docs.h.in b/src/libnmc-setting/settings-docs.h.in index 985e604819..af0dc05a95 100644 --- a/src/libnmc-setting/settings-docs.h.in +++ b/src/libnmc-setting/settings-docs.h.in @@ -140,6 +140,7 @@ #define DESCRIBE_DOC_NM_SETTING_DCB_PRIORITY_GROUP_ID N_("An array of 8 uint values, where the array index corresponds to the User Priority (0 - 7) and the value indicates the Priority Group ID. Allowed Priority Group ID values are 0 - 7 or 15 for the unrestricted group.") #define DESCRIBE_DOC_NM_SETTING_DCB_PRIORITY_STRICT_BANDWIDTH N_("An array of 8 boolean values, where the array index corresponds to the User Priority (0 - 7) and the value indicates whether or not the priority may use all of the bandwidth allocated to its assigned group.") #define DESCRIBE_DOC_NM_SETTING_DCB_PRIORITY_TRAFFIC_CLASS N_("An array of 8 uint values, where the array index corresponds to the User Priority (0 - 7) and the value indicates the traffic class (0 - 7) to which the priority is mapped.") +#define DESCRIBE_DOC_NM_SETTING_GENERIC_DEVICE_HANDLER N_("Name of the device handler that will be invoked to add and delete the device for this connection. The name can only contain ASCII alphanumeric characters and '-', '_', '.'. It cannot start with '.'. See the NetworkManager-dispatcher(8) man page for more details about how to write the device handler. By setting this property the generic connection becomes \"virtual\", meaning that it can be activated without an existing device; the device will be created at the time the connection is started by invoking the device-handler.") #define DESCRIBE_DOC_NM_SETTING_GSM_APN N_("The GPRS Access Point Name specifying the APN used when establishing a data session with the GSM-based network. The APN often determines how the user will be billed for their network usage and whether the user has access to the Internet or just a provider-specific walled-garden, so it is important to use the correct APN for the user's mobile broadband plan. The APN may only be composed of the characters a-z, 0-9, ., and - per GSM 03.60 Section 14.9. If the APN is unset (the default) then it may be detected based on \"auto-config\" setting. The property can be explicitly set to the empty string to prevent that and use no APN.") #define DESCRIBE_DOC_NM_SETTING_GSM_AUTO_CONFIG N_("When TRUE, the settings such as APN, username, or password will default to values that match the network the modem will register to in the Mobile Broadband Provider database.") #define DESCRIBE_DOC_NM_SETTING_GSM_DEVICE_ID N_("The device unique identifier (as given by the WWAN management service) which this connection applies to. If given, the connection will only apply to the specified device.") diff --git a/src/nm-dispatcher/nm-dispatcher.c b/src/nm-dispatcher/nm-dispatcher.c index 28ac51bf07..86277931df 100644 --- a/src/nm-dispatcher/nm-dispatcher.c +++ b/src/nm-dispatcher/nm-dispatcher.c @@ -416,10 +416,10 @@ script_watch_cb(GPid pid, int status, gpointer user_data) } if (script->result == DISPATCH_RESULT_SUCCESS) { - _LOG_S_T(script, "complete"); + _LOG_S_T(script, "complete: process succeeded"); } else { script->result = DISPATCH_RESULT_FAILED; - _LOG_S_W(script, "complete: failed with %s", script->error); + _LOG_S_W(script, "complete: process failed with %s", script->error); } g_spawn_close_pid(script->pid); diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index ac2a53c5ba..72a1fc1897 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -1066,15 +1066,16 @@ const NmcMetaGenericInfo "," NM_SETTING_TEAM_SETTING_NAME "," NM_SETTING_TEAM_PORT_SETTING_NAME \ "," NM_SETTING_OVS_BRIDGE_SETTING_NAME "," NM_SETTING_OVS_INTERFACE_SETTING_NAME \ "," NM_SETTING_OVS_PATCH_SETTING_NAME "," NM_SETTING_OVS_PORT_SETTING_NAME \ - "," NM_SETTING_DCB_SETTING_NAME "," NM_SETTING_TUN_SETTING_NAME \ - "," NM_SETTING_IP_TUNNEL_SETTING_NAME "," NM_SETTING_MACSEC_SETTING_NAME \ - "," NM_SETTING_MACVLAN_SETTING_NAME "," NM_SETTING_VXLAN_SETTING_NAME \ - "," NM_SETTING_VRF_SETTING_NAME "," NM_SETTING_WPAN_SETTING_NAME \ - "," NM_SETTING_6LOWPAN_SETTING_NAME "," NM_SETTING_WIREGUARD_SETTING_NAME \ - "," NM_SETTING_LINK_SETTING_NAME "," NM_SETTING_PROXY_SETTING_NAME \ - "," NM_SETTING_TC_CONFIG_SETTING_NAME "," NM_SETTING_SRIOV_SETTING_NAME \ - "," NM_SETTING_ETHTOOL_SETTING_NAME "," NM_SETTING_OVS_DPDK_SETTING_NAME \ - "," NM_SETTING_HOSTNAME_SETTING_NAME "," NM_SETTING_HSR_SETTING_NAME + "," NM_SETTING_GENERIC_SETTING_NAME "," NM_SETTING_DCB_SETTING_NAME \ + "," NM_SETTING_TUN_SETTING_NAME "," NM_SETTING_IP_TUNNEL_SETTING_NAME \ + "," NM_SETTING_MACSEC_SETTING_NAME "," NM_SETTING_MACVLAN_SETTING_NAME \ + "," NM_SETTING_VXLAN_SETTING_NAME "," NM_SETTING_VRF_SETTING_NAME \ + "," NM_SETTING_WPAN_SETTING_NAME "," NM_SETTING_6LOWPAN_SETTING_NAME \ + "," NM_SETTING_WIREGUARD_SETTING_NAME "," NM_SETTING_LINK_SETTING_NAME \ + "," NM_SETTING_PROXY_SETTING_NAME "," NM_SETTING_TC_CONFIG_SETTING_NAME \ + "," NM_SETTING_SRIOV_SETTING_NAME "," NM_SETTING_ETHTOOL_SETTING_NAME \ + "," NM_SETTING_OVS_DPDK_SETTING_NAME "," NM_SETTING_HOSTNAME_SETTING_NAME \ + "," NM_SETTING_HSR_SETTING_NAME /* NM_SETTING_DUMMY_SETTING_NAME NM_SETTING_WIMAX_SETTING_NAME */ const NmcMetaGenericInfo *const nmc_fields_con_active_details_groups[] = { diff --git a/src/nmcli/gen-metadata-nm-settings-nmcli.xml.in b/src/nmcli/gen-metadata-nm-settings-nmcli.xml.in index 8792f74309..4f58c8905c 100644 --- a/src/nmcli/gen-metadata-nm-settings-nmcli.xml.in +++ b/src/nmcli/gen-metadata-nm-settings-nmcli.xml.in @@ -1070,6 +1070,9 @@ values="0 - 4294967295" /> + Date: Tue, 26 Sep 2023 17:29:46 +0200 Subject: [PATCH 05/15] core/dispatcher: factorize code Move common code from nm_dispatcher_call_device() and nm_dispatcher_call_device_sync() to a new function; it will also be used in the next commits by a new variant of the function. --- src/core/nm-dispatcher.c | 88 +++++++++++++++++++--------------------- 1 file changed, 42 insertions(+), 46 deletions(-) diff --git a/src/core/nm-dispatcher.c b/src/core/nm-dispatcher.c index 9aa4194e83..3ebf1ca2ee 100644 --- a/src/core/nm-dispatcher.c +++ b/src/core/nm-dispatcher.c @@ -723,6 +723,40 @@ nm_dispatcher_call_hostname(NMDispatcherFunc callback, out_call_id); } +static gboolean +_dispatcher_call_device(NMDispatcherAction action, + NMDevice *device, + gboolean blocking, + NMActRequest *act_request, + NMDispatcherFunc callback, + gpointer user_data, + NMDispatcherCallId **out_call_id) +{ + nm_assert(NM_IS_DEVICE(device)); + if (!act_request) { + act_request = nm_device_get_act_request(device); + if (!act_request) + return FALSE; + } + nm_assert(NM_IN_SET(nm_active_connection_get_device(NM_ACTIVE_CONNECTION(act_request)), + NULL, + device)); + return _dispatcher_call( + action, + blocking, + device, + nm_act_request_get_settings_connection(act_request), + nm_act_request_get_applied_connection(act_request), + nm_active_connection_get_activation_type(NM_ACTIVE_CONNECTION(act_request)) + == NM_ACTIVATION_TYPE_EXTERNAL, + NM_CONNECTIVITY_UNKNOWN, + NULL, + NULL, + callback, + user_data, + out_call_id); +} + /** * nm_dispatcher_call_device: * @action: the %NMDispatcherAction @@ -747,29 +781,13 @@ nm_dispatcher_call_device(NMDispatcherAction action, gpointer user_data, NMDispatcherCallId **out_call_id) { - nm_assert(NM_IS_DEVICE(device)); - if (!act_request) { - act_request = nm_device_get_act_request(device); - if (!act_request) - return FALSE; - } - nm_assert(NM_IN_SET(nm_active_connection_get_device(NM_ACTIVE_CONNECTION(act_request)), - NULL, - device)); - return _dispatcher_call( - action, - FALSE, - device, - nm_act_request_get_settings_connection(act_request), - nm_act_request_get_applied_connection(act_request), - nm_active_connection_get_activation_type(NM_ACTIVE_CONNECTION(act_request)) - == NM_ACTIVATION_TYPE_EXTERNAL, - NM_CONNECTIVITY_UNKNOWN, - NULL, - NULL, - callback, - user_data, - out_call_id); + return _dispatcher_call_device(action, + device, + FALSE, + act_request, + callback, + user_data, + out_call_id); } /** @@ -789,29 +807,7 @@ nm_dispatcher_call_device_sync(NMDispatcherAction action, NMDevice *device, NMActRequest *act_request) { - nm_assert(NM_IS_DEVICE(device)); - if (!act_request) { - act_request = nm_device_get_act_request(device); - if (!act_request) - return FALSE; - } - nm_assert(NM_IN_SET(nm_active_connection_get_device(NM_ACTIVE_CONNECTION(act_request)), - NULL, - device)); - return _dispatcher_call( - action, - TRUE, - device, - nm_act_request_get_settings_connection(act_request), - nm_act_request_get_applied_connection(act_request), - nm_active_connection_get_activation_type(NM_ACTIVE_CONNECTION(act_request)) - == NM_ACTIVATION_TYPE_EXTERNAL, - NM_CONNECTIVITY_UNKNOWN, - NULL, - NULL, - NULL, - NULL, - NULL); + return _dispatcher_call_device(action, device, TRUE, act_request, NULL, NULL, NULL); } /** From 98b73e88e67e44c6f695dc87af0acab76c3e0c64 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 29 Sep 2023 13:27:26 +0200 Subject: [PATCH 06/15] core/dispatcher: refactor nm_dispatcher_need_device() Remove the "nm_" prefix that is usually reserved for non-static functions. Also, use NM_IN_SET. --- src/core/nm-dispatcher.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/core/nm-dispatcher.c b/src/core/nm-dispatcher.c index 3ebf1ca2ee..1515efeca9 100644 --- a/src/core/nm-dispatcher.c +++ b/src/core/nm-dispatcher.c @@ -50,8 +50,6 @@ } \ G_STMT_END -static gboolean nm_dispatcher_need_device(NMDispatcherAction action); - /*****************************************************************************/ struct NMDispatcherCallId { @@ -84,6 +82,20 @@ static struct { /*****************************************************************************/ +/* All actions except 'hostname', 'connectivity-change' and 'dns-change' require + * a device */ +static gboolean +action_need_device(NMDispatcherAction action) +{ + if (NM_IN_SET(action, + NM_DISPATCHER_ACTION_HOSTNAME, + NM_DISPATCHER_ACTION_CONNECTIVITY_CHANGE, + NM_DISPATCHER_ACTION_DNS_CHANGE)) { + return FALSE; + } + return TRUE; +} + static NMDispatcherCallId * dispatcher_call_id_new(guint32 request_id, gint64 start_at_msec, @@ -533,7 +545,7 @@ _dispatcher_call(NMDispatcherAction action, if (G_UNLIKELY(!request_id)) request_id = ++gl.request_id_counter; - if (!nm_dispatcher_need_device(action)) { + if (!action_need_device(action)) { _LOG2D(request_id, log_ifname, log_con_uuid, @@ -594,7 +606,7 @@ _dispatcher_call(NMDispatcherAction action, g_variant_builder_init(&vpn_ip6_props, G_VARIANT_TYPE_VARDICT); /* hostname, DNS and connectivity-change actions don't send device data */ - if (nm_dispatcher_need_device(action)) { + if (action_need_device(action)) { fill_device_props(device, &device_props, &device_proxy_props, @@ -957,16 +969,3 @@ nm_dispatcher_call_cancel(NMDispatcherCallId *call_id) _LOG3D(call_id, "cancelling dispatcher callback action"); call_id->callback = NULL; } - -/* All actions except 'hostname', 'connectivity-change' and 'dns-change' require - * a device */ -static gboolean -nm_dispatcher_need_device(NMDispatcherAction action) -{ - if (action == NM_DISPATCHER_ACTION_HOSTNAME - || action == NM_DISPATCHER_ACTION_CONNECTIVITY_CHANGE - || action == NM_DISPATCHER_ACTION_DNS_CHANGE) { - return FALSE; - } - return TRUE; -} From 703efdfbbfe926bb0f245bdc4bd8e851f103e543 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 7 Nov 2023 11:57:39 +0100 Subject: [PATCH 07/15] dispatcher: refactor building the result Introduce request_dbus_method_return() and call it whenever we need to return a result. Don't collect the list of scripts in case the parameters can't be parsed. --- src/nm-dispatcher/nm-dispatcher.c | 81 ++++++++++++++++--------------- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/src/nm-dispatcher/nm-dispatcher.c b/src/nm-dispatcher/nm-dispatcher.c index 86277931df..c16b9d7c0a 100644 --- a/src/nm-dispatcher/nm-dispatcher.c +++ b/src/nm-dispatcher/nm-dispatcher.c @@ -280,28 +280,12 @@ next_request(Request *request) return TRUE; } -/** - * complete_request: - * @request: the request - * - * Checks if all the scripts for the request have terminated and in such case - * it sends the D-Bus response and releases the request resources. - * - * It also decreases @num_requests_pending and possibly does _idle_timeout_restart(). - */ static void -complete_request(Request *request) +request_dbus_method_return(Request *request) { GVariantBuilder results; - GVariant *ret; guint i; - nm_assert(request); - - /* Are there still pending scripts? Then do nothing (for now). */ - if (request->num_scripts_done < request->scripts->len) - return; - g_variant_builder_init(&results, G_VARIANT_TYPE("a(sus)")); for (i = 0; i < request->scripts->len; i++) { ScriptInfo *script = g_ptr_array_index(request->scripts, i); @@ -313,8 +297,28 @@ complete_request(Request *request) script->error ?: ""); } - ret = g_variant_new("(a(sus))", &results); - g_dbus_method_invocation_return_value(request->context, ret); + g_dbus_method_invocation_return_value(request->context, g_variant_new("(a(sus))", &results)); +} + +/** + * complete_request: + * @request: the request + * + * Checks if all the scripts for the request have terminated and in such case + * it sends the D-Bus response and releases the request resources. + * + * It also decreases @num_requests_pending and possibly does _idle_timeout_restart(). + */ +static void +complete_request(Request *request) +{ + nm_assert(request); + + /* Are there still pending scripts? Then do nothing (for now). */ + if (request->num_scripts_done < request->scripts->len) + return; + + request_dbus_method_return(request); _LOG_R_T(request, "completed (%u scripts)", request->scripts->len); @@ -785,36 +789,35 @@ _handle_action(GDBusMethodInvocation *invocation, GVariant *parameters) &request->iface, &error_message); - request->scripts = g_ptr_array_new_full(5, script_info_free); + if (!error_message) { + request->scripts = g_ptr_array_new_full(5, script_info_free); - sorted_scripts = find_scripts(request); - for (iter = sorted_scripts; iter; iter = g_slist_next(iter)) { - ScriptInfo *s; + sorted_scripts = find_scripts(request); + for (iter = sorted_scripts; iter; iter = g_slist_next(iter)) { + ScriptInfo *s; - s = g_slice_new0(ScriptInfo); - s->request = request; - s->script = iter->data; - s->wait = script_must_wait(s->script); - g_ptr_array_add(request->scripts, s); - } - g_slist_free(sorted_scripts); + s = g_slice_new0(ScriptInfo); + s->request = request; + s->script = iter->data; + s->wait = script_must_wait(s->script); + g_ptr_array_add(request->scripts, s); + } + g_slist_free(sorted_scripts); - _LOG_R_D(request, "new request (%u scripts)", request->scripts->len); - if (_LOG_R_T_enabled(request) && request->envp) { - for (p = request->envp; *p; p++) - _LOG_R_T(request, "environment: %s", *p); + _LOG_R_D(request, "new request (%u scripts)", request->scripts->len); + if (_LOG_R_T_enabled(request) && request->envp) { + for (p = request->envp; *p; p++) + _LOG_R_T(request, "environment: %s", *p); + } } - if (error_message || request->scripts->len == 0) { - GVariant *results; - + if (request->scripts->len == 0) { if (error_message) _LOG_R_W(request, "completed: invalid request: %s", error_message); else _LOG_R_D(request, "completed: no scripts"); - results = g_variant_new_array(G_VARIANT_TYPE("(sus)"), NULL, 0); - g_dbus_method_invocation_return_value(invocation, g_variant_new("(@a(sus))", results)); + request_dbus_method_return(request); request->num_scripts_done = request->scripts->len; request_free(request); return; From abf0f03d25c810b4c7b2910b34c84e4fd29496f9 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 7 Nov 2023 13:54:54 +0100 Subject: [PATCH 08/15] dispatcher: add Action2() D-Bus method Currently, the dispatcher service implements an Action() method to dispatch events. In the next commits, we'll need to add new parameters, which is not possible with the current signature. Introduce a new Action2() method, similar to the existing one but with the following changes: - it accepts an additional "options" input parameter of type a{sv}; - for each script executed, it also returns a dictionary of type a{sv}. The new parameters will allow to easily extend functionality in the future without having to implement an Action3(). --- src/nm-dispatcher/nm-dispatcher.c | 169 ++++++++++++++++++++++-------- 1 file changed, 126 insertions(+), 43 deletions(-) diff --git a/src/nm-dispatcher/nm-dispatcher.c b/src/nm-dispatcher/nm-dispatcher.c index c16b9d7c0a..722d6e290d 100644 --- a/src/nm-dispatcher/nm-dispatcher.c +++ b/src/nm-dispatcher/nm-dispatcher.c @@ -85,6 +85,7 @@ struct Request { char *iface; char **envp; gboolean debug; + gboolean is_action2; GPtrArray *scripts; /* list of ScriptInfo */ guint idx; @@ -286,18 +287,35 @@ request_dbus_method_return(Request *request) GVariantBuilder results; guint i; - g_variant_builder_init(&results, G_VARIANT_TYPE("a(sus)")); + if (request->is_action2) { + g_variant_builder_init(&results, G_VARIANT_TYPE("a(susa{sv})")); + } else { + g_variant_builder_init(&results, G_VARIANT_TYPE("a(sus)")); + } + for (i = 0; i < request->scripts->len; i++) { ScriptInfo *script = g_ptr_array_index(request->scripts, i); - g_variant_builder_add(&results, - "(sus)", - script->script, - script->result, - script->error ?: ""); + if (request->is_action2) { + g_variant_builder_add(&results, + "(sus@a{sv})", + script->script, + script->result, + script->error ?: "", + nm_g_variant_singleton_aLsvI()); + } else { + g_variant_builder_add(&results, + "(sus)", + script->script, + script->result, + script->error ?: ""); + } } - g_dbus_method_invocation_return_value(request->context, g_variant_new("(a(sus))", &results)); + g_dbus_method_invocation_return_value(request->context, + request->is_action2 + ? g_variant_new("(a(susa{sv}))", &results) + : g_variant_new("(a(sus))", &results)); } /** @@ -708,7 +726,7 @@ script_must_wait(const char *path) } static void -_handle_action(GDBusMethodInvocation *invocation, GVariant *parameters) +_handle_action(GDBusMethodInvocation *invocation, GVariant *parameters, gboolean is_action2) { const char *action; gs_unref_variant GVariant *connection = NULL; @@ -724,6 +742,7 @@ _handle_action(GDBusMethodInvocation *invocation, GVariant *parameters) gs_unref_variant GVariant *vpn_proxy_properties = NULL; gs_unref_variant GVariant *vpn_ip4_config = NULL; gs_unref_variant GVariant *vpn_ip6_config = NULL; + gs_unref_variant GVariant *options = NULL; gboolean debug; GSList *sorted_scripts = NULL; GSList *iter; @@ -732,45 +751,84 @@ _handle_action(GDBusMethodInvocation *invocation, GVariant *parameters) guint i, num_nowait = 0; const char *error_message = NULL; - g_variant_get(parameters, - "(" - "&s" /* action */ - "@a{sa{sv}}" /* connection */ - "@a{sv}" /* connection_properties */ - "@a{sv}" /* device_properties */ - "@a{sv}" /* device_proxy_properties */ - "@a{sv}" /* device_ip4_config */ - "@a{sv}" /* device_ip6_config */ - "@a{sv}" /* device_dhcp4_config */ - "@a{sv}" /* device_dhcp6_config */ - "&s" /* connectivity_state */ - "&s" /* vpn_ip_iface */ - "@a{sv}" /* vpn_proxy_properties */ - "@a{sv}" /* vpn_ip4_config */ - "@a{sv}" /* vpn_ip6_config */ - "b" /* debug */ - ")", - &action, - &connection, - &connection_properties, - &device_properties, - &device_proxy_properties, - &device_ip4_config, - &device_ip6_config, - &device_dhcp4_config, - &device_dhcp6_config, - &connectivity_state, - &vpn_ip_iface, - &vpn_proxy_properties, - &vpn_ip4_config, - &vpn_ip6_config, - &debug); + if (is_action2) { + g_variant_get(parameters, + "(" + "&s" /* action */ + "@a{sa{sv}}" /* connection */ + "@a{sv}" /* connection_properties */ + "@a{sv}" /* device_properties */ + "@a{sv}" /* device_proxy_properties */ + "@a{sv}" /* device_ip4_config */ + "@a{sv}" /* device_ip6_config */ + "@a{sv}" /* device_dhcp4_config */ + "@a{sv}" /* device_dhcp6_config */ + "&s" /* connectivity_state */ + "&s" /* vpn_ip_iface */ + "@a{sv}" /* vpn_proxy_properties */ + "@a{sv}" /* vpn_ip4_config */ + "@a{sv}" /* vpn_ip6_config */ + "b" /* debug */ + "@a{sv}" /* options */ + ")", + &action, + &connection, + &connection_properties, + &device_properties, + &device_proxy_properties, + &device_ip4_config, + &device_ip6_config, + &device_dhcp4_config, + &device_dhcp6_config, + &connectivity_state, + &vpn_ip_iface, + &vpn_proxy_properties, + &vpn_ip4_config, + &vpn_ip6_config, + &debug, + &options); + } else { + g_variant_get(parameters, + "(" + "&s" /* action */ + "@a{sa{sv}}" /* connection */ + "@a{sv}" /* connection_properties */ + "@a{sv}" /* device_properties */ + "@a{sv}" /* device_proxy_properties */ + "@a{sv}" /* device_ip4_config */ + "@a{sv}" /* device_ip6_config */ + "@a{sv}" /* device_dhcp4_config */ + "@a{sv}" /* device_dhcp6_config */ + "&s" /* connectivity_state */ + "&s" /* vpn_ip_iface */ + "@a{sv}" /* vpn_proxy_properties */ + "@a{sv}" /* vpn_ip4_config */ + "@a{sv}" /* vpn_ip6_config */ + "b" /* debug */ + ")", + &action, + &connection, + &connection_properties, + &device_properties, + &device_proxy_properties, + &device_ip4_config, + &device_ip6_config, + &device_dhcp4_config, + &device_dhcp6_config, + &connectivity_state, + &vpn_ip_iface, + &vpn_proxy_properties, + &vpn_ip4_config, + &vpn_ip6_config, + &debug); + } request = g_slice_new0(Request); request->request_id = ++gl.request_id_counter; request->debug = debug || gl.log_verbose; request->context = invocation; request->action = g_strdup(action); + request->is_action2 = is_action2; request->envp = nm_dispatcher_utils_construct_envp(action, connection, @@ -908,8 +966,12 @@ _bus_method_call(GDBusConnection *connection, return; } if (nm_streq(interface_name, NM_DISPATCHER_DBUS_INTERFACE)) { + if (nm_streq(method_name, "Action2")) { + _handle_action(invocation, parameters, TRUE); + return; + } if (nm_streq(method_name, "Action")) { - _handle_action(invocation, parameters); + _handle_action(invocation, parameters, FALSE); return; } if (nm_streq(method_name, "Ping")) { @@ -950,7 +1012,28 @@ static GDBusInterfaceInfo *const interface_info = NM_DEFINE_GDBUS_INTERFACE_INFO NM_DEFINE_GDBUS_ARG_INFO("vpn_ip6_config", "a{sv}"), NM_DEFINE_GDBUS_ARG_INFO("debug", "b"), ), .out_args = - NM_DEFINE_GDBUS_ARG_INFOS(NM_DEFINE_GDBUS_ARG_INFO("results", "a(sus)"), ), ), ), ); + NM_DEFINE_GDBUS_ARG_INFOS(NM_DEFINE_GDBUS_ARG_INFO("results", "a(sus)"), ), ), + NM_DEFINE_GDBUS_METHOD_INFO( + "Action2", + .in_args = NM_DEFINE_GDBUS_ARG_INFOS( + NM_DEFINE_GDBUS_ARG_INFO("action", "s"), + NM_DEFINE_GDBUS_ARG_INFO("connection", "a{sa{sv}}"), + NM_DEFINE_GDBUS_ARG_INFO("connection_properties", "a{sv}"), + NM_DEFINE_GDBUS_ARG_INFO("device_properties", "a{sv}"), + NM_DEFINE_GDBUS_ARG_INFO("device_proxy_properties", "a{sv}"), + NM_DEFINE_GDBUS_ARG_INFO("device_ip4_config", "a{sv}"), + NM_DEFINE_GDBUS_ARG_INFO("device_ip6_config", "a{sv}"), + NM_DEFINE_GDBUS_ARG_INFO("device_dhcp4_config", "a{sv}"), + NM_DEFINE_GDBUS_ARG_INFO("device_dhcp6_config", "a{sv}"), + NM_DEFINE_GDBUS_ARG_INFO("connectivity_state", "s"), + NM_DEFINE_GDBUS_ARG_INFO("vpn_ip_iface", "s"), + NM_DEFINE_GDBUS_ARG_INFO("vpn_proxy_properties", "a{sv}"), + NM_DEFINE_GDBUS_ARG_INFO("vpn_ip4_config", "a{sv}"), + NM_DEFINE_GDBUS_ARG_INFO("vpn_ip6_config", "a{sv}"), + NM_DEFINE_GDBUS_ARG_INFO("debug", "b"), + NM_DEFINE_GDBUS_ARG_INFO("options", "a{sv}"), ), + .out_args = NM_DEFINE_GDBUS_ARG_INFOS( + NM_DEFINE_GDBUS_ARG_INFO("results", "a(susa{sv})"), ), ), ), ); static gboolean _bus_register_service(void) From 8fd0d39444ddb3a386808a22badb0aad9b3b0d58 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 7 Nov 2023 17:25:49 +0100 Subject: [PATCH 09/15] core/dispatcher: prefer the Action2() method and fall back to Action() Call the Action2() method first, and fall back to the old Action() if the new one is not available. This allows full interoperability between different versions of the dispatcher service and NM. --- src/core/nm-dispatcher.c | 298 ++++++++++++++++++++++++++++----------- 1 file changed, 212 insertions(+), 86 deletions(-) diff --git a/src/core/nm-dispatcher.c b/src/core/nm-dispatcher.c index 1515efeca9..f2016f14e5 100644 --- a/src/core/nm-dispatcher.c +++ b/src/core/nm-dispatcher.c @@ -57,10 +57,12 @@ struct NMDispatcherCallId { gpointer user_data; const char *log_ifname; const char *log_con_uuid; + GVariant *action_params; gint64 start_at_msec; NMDispatcherAction action; guint idle_id; guint32 request_id; + bool is_action2 : 1; char extra_strings[]; }; @@ -121,6 +123,7 @@ dispatcher_call_id_new(guint32 request_id, call_id->callback = callback; call_id->user_data = user_data; call_id->idle_id = 0; + call_id->is_action2 = TRUE; extra_strings = &call_id->extra_strings[0]; @@ -143,6 +146,7 @@ dispatcher_call_id_new(guint32 request_id, static void dispatcher_call_id_free(NMDispatcherCallId *call_id) { + nm_clear_pointer(&call_id->action_params, g_variant_unref); nm_clear_g_source(&call_id->idle_id); g_free(call_id); } @@ -390,14 +394,18 @@ dispatcher_results_process(guint32 request_id, gint64 now_msec, const char *log_ifname, const char *log_con_uuid, - GVariant *v_results) + GVariant *v_results, + gboolean is_action2) { nm_auto_free_variant_iter GVariantIter *results = NULL; const char *script, *err; guint32 result; gsize n_children; - g_variant_get(v_results, "(a(sus))", &results); + if (is_action2) + g_variant_get(v_results, "(a(susa{sv}))", &results); + else + g_variant_get(v_results, "(a(sus))", &results); n_children = g_variant_iter_n_children(results); @@ -412,7 +420,17 @@ dispatcher_results_process(guint32 request_id, if (n_children == 0) return; - while (g_variant_iter_next(results, "(&su&s)", &script, &result, &err)) { + while (TRUE) { + gs_unref_variant GVariant *options = NULL; + + if (is_action2) { + if (!g_variant_iter_next(results, "(&su&s@a{sv})", &script, &result, &err, &options)) + break; + } else { + if (!g_variant_iter_next(results, "(&su&s)", &script, &result, &err)) + break; + } + if (result == DISPATCH_RESULT_SUCCESS) { _LOG2D(request_id, log_ifname, log_con_uuid, "%s succeeded", script); } else { @@ -440,6 +458,27 @@ dispatcher_done_cb(GObject *source, GAsyncResult *result, gpointer user_data) now_msec = nm_utils_get_monotonic_timestamp_msec(); ret = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), result, &error); + + if (!ret && call_id->is_action2 + && g_error_matches(error, G_DBUS_ERROR, G_DBUS_ERROR_UNKNOWN_METHOD)) { + _LOG3D(call_id, + "dispatcher service does not implement Action2() method, falling back to Action()"); + call_id->is_action2 = FALSE; + g_dbus_connection_call(gl.dbus_connection, + NM_DISPATCHER_DBUS_SERVICE, + NM_DISPATCHER_DBUS_PATH, + NM_DISPATCHER_DBUS_INTERFACE, + "Action", + g_steal_pointer(&call_id->action_params), + G_VARIANT_TYPE("(a(sus))"), + G_DBUS_CALL_FLAGS_NONE, + CALL_TIMEOUT, + NULL, + dispatcher_done_cb, + call_id); + return; + } + if (!ret) { NMLogLevel log_level = LOGL_DEBUG; @@ -459,7 +498,8 @@ dispatcher_done_cb(GObject *source, GAsyncResult *result, gpointer user_data) now_msec, call_id->log_ifname, call_id->log_con_uuid, - ret); + ret, + call_id->is_action2); } g_hash_table_remove(gl.requests, call_id); @@ -494,75 +534,29 @@ action_to_string(NMDispatcherAction action) return action_table[(gsize) action]; } -static gboolean -_dispatcher_call(NMDispatcherAction action, - gboolean blocking, - NMDevice *device, - NMSettingsConnection *settings_connection, - NMConnection *applied_connection, - gboolean activation_type_external, - NMConnectivityState connectivity_state, - const char *vpn_iface, - const NML3ConfigData *l3cd, - NMDispatcherFunc callback, - gpointer user_data, - NMDispatcherCallId **out_call_id) +static GVariant * +build_call_parameters(NMDispatcherAction action, + NMDevice *device, + NMSettingsConnection *settings_connection, + NMConnection *applied_connection, + gboolean activation_type_external, + NMConnectivityState connectivity_state, + const char *vpn_iface, + const NML3ConfigData *l3cd, + gboolean is_action2) { + const char *connectivity_state_string = "UNKNOWN"; GVariant *connection_dict; GVariantBuilder connection_props; GVariantBuilder device_props; GVariantBuilder device_proxy_props; GVariantBuilder device_ip4_props; GVariantBuilder device_ip6_props; - gs_unref_variant GVariant *parameters_floating = NULL; - gs_unref_variant GVariant *device_dhcp4_props = NULL; - gs_unref_variant GVariant *device_dhcp6_props = NULL; + gs_unref_variant GVariant *device_dhcp4_props = NULL; + gs_unref_variant GVariant *device_dhcp6_props = NULL; GVariantBuilder vpn_proxy_props; GVariantBuilder vpn_ip4_props; GVariantBuilder vpn_ip6_props; - NMDispatcherCallId *call_id; - guint request_id; - const char *connectivity_state_string = "UNKNOWN"; - const char *log_ifname; - const char *log_con_uuid; - gint64 start_at_msec; - gint64 now_msec; - - g_return_val_if_fail(!blocking || (!callback && !user_data), FALSE); - - NM_SET_OUT(out_call_id, NULL); - - _init_dispatcher(); - - if (!gl.dbus_connection) - return FALSE; - - log_ifname = device ? nm_device_get_iface(device) : NULL; - log_con_uuid = - settings_connection ? nm_settings_connection_get_uuid(settings_connection) : NULL; - - request_id = ++gl.request_id_counter; - if (G_UNLIKELY(!request_id)) - request_id = ++gl.request_id_counter; - - if (!action_need_device(action)) { - _LOG2D(request_id, - log_ifname, - log_con_uuid, - "dispatching action '%s'%s", - action_to_string(action), - blocking ? " (blocking)" : (callback ? " (with callback)" : "")); - } else { - g_return_val_if_fail(NM_IS_DEVICE(device), FALSE); - - _LOG2D(request_id, - log_ifname, - log_con_uuid, - "(%s) dispatching action '%s'%s", - vpn_iface ?: nm_device_get_iface(device), - action_to_string(action), - blocking ? " (blocking)" : (callback ? " (with callback)" : "")); - } if (applied_connection) connection_dict = @@ -621,25 +615,114 @@ _dispatcher_call(NMDispatcherAction action, connectivity_state_string = nm_connectivity_state_to_string(connectivity_state); - parameters_floating = - g_variant_new("(s@a{sa{sv}}a{sv}a{sv}a{sv}a{sv}a{sv}@a{sv}@a{sv}ssa{sv}a{sv}a{sv}b)", - action_to_string(action), - connection_dict, - &connection_props, - &device_props, - &device_proxy_props, - &device_ip4_props, - &device_ip6_props, - device_dhcp4_props ?: nm_g_variant_singleton_aLsvI(), - device_dhcp6_props ?: nm_g_variant_singleton_aLsvI(), - connectivity_state_string, - vpn_iface ?: "", - &vpn_proxy_props, - &vpn_ip4_props, - &vpn_ip6_props, - nm_logging_enabled(LOGL_DEBUG, LOGD_DISPATCH)); + if (is_action2) { + return g_variant_new( + "(s@a{sa{sv}}a{sv}a{sv}a{sv}a{sv}a{sv}@a{sv}@a{sv}ssa{sv}a{sv}a{sv}b@a{sv})", + action_to_string(action), + connection_dict, + &connection_props, + &device_props, + &device_proxy_props, + &device_ip4_props, + &device_ip6_props, + device_dhcp4_props ?: nm_g_variant_singleton_aLsvI(), + device_dhcp6_props ?: nm_g_variant_singleton_aLsvI(), + connectivity_state_string, + vpn_iface ?: "", + &vpn_proxy_props, + &vpn_ip4_props, + &vpn_ip6_props, + nm_logging_enabled(LOGL_DEBUG, LOGD_DISPATCH), + nm_g_variant_singleton_aLsvI()); + } - start_at_msec = nm_utils_get_monotonic_timestamp_msec(); + return g_variant_new("(s@a{sa{sv}}a{sv}a{sv}a{sv}a{sv}a{sv}@a{sv}@a{sv}ssa{sv}a{sv}a{sv}b)", + action_to_string(action), + connection_dict, + &connection_props, + &device_props, + &device_proxy_props, + &device_ip4_props, + &device_ip6_props, + device_dhcp4_props ?: nm_g_variant_singleton_aLsvI(), + device_dhcp6_props ?: nm_g_variant_singleton_aLsvI(), + connectivity_state_string, + vpn_iface ?: "", + &vpn_proxy_props, + &vpn_ip4_props, + &vpn_ip6_props, + nm_logging_enabled(LOGL_DEBUG, LOGD_DISPATCH)); +} + +static gboolean +_dispatcher_call(NMDispatcherAction action, + gboolean blocking, + NMDevice *device, + NMSettingsConnection *settings_connection, + NMConnection *applied_connection, + gboolean activation_type_external, + NMConnectivityState connectivity_state, + const char *vpn_iface, + const NML3ConfigData *l3cd, + NMDispatcherFunc callback, + gpointer user_data, + NMDispatcherCallId **out_call_id) +{ + NMDispatcherCallId *call_id; + guint request_id; + const char *log_ifname; + const char *log_con_uuid; + gint64 start_at_msec; + gint64 now_msec; + gs_unref_variant GVariant *parameters_floating = NULL; + gboolean is_action2 = TRUE; + + g_return_val_if_fail(!blocking || (!callback && !user_data), FALSE); + + NM_SET_OUT(out_call_id, NULL); + + _init_dispatcher(); + + if (!gl.dbus_connection) + return FALSE; + + log_ifname = device ? nm_device_get_iface(device) : NULL; + log_con_uuid = + settings_connection ? nm_settings_connection_get_uuid(settings_connection) : NULL; + + request_id = ++gl.request_id_counter; + if (G_UNLIKELY(!request_id)) + request_id = ++gl.request_id_counter; + + if (!action_need_device(action)) { + _LOG2D(request_id, + log_ifname, + log_con_uuid, + "dispatching action '%s'%s", + action_to_string(action), + blocking ? " (blocking)" : (callback ? " (with callback)" : "")); + } else { + g_return_val_if_fail(NM_IS_DEVICE(device), FALSE); + + _LOG2D(request_id, + log_ifname, + log_con_uuid, + "(%s) dispatching action '%s'%s", + vpn_iface ?: nm_device_get_iface(device), + action_to_string(action), + blocking ? " (blocking)" : (callback ? " (with callback)" : "")); + } + + parameters_floating = build_call_parameters(action, + device, + settings_connection, + applied_connection, + activation_type_external, + connectivity_state, + vpn_iface, + l3cd, + TRUE); + start_at_msec = nm_utils_get_monotonic_timestamp_msec(); /* Send the action to the dispatcher */ if (blocking) { @@ -650,14 +733,44 @@ _dispatcher_call(NMDispatcherAction action, NM_DISPATCHER_DBUS_SERVICE, NM_DISPATCHER_DBUS_PATH, NM_DISPATCHER_DBUS_INTERFACE, - "Action", + "Action2", g_steal_pointer(¶meters_floating), - G_VARIANT_TYPE("(a(sus))"), + G_VARIANT_TYPE("(a(susa{sv}))"), G_DBUS_CALL_FLAGS_NONE, CALL_TIMEOUT, NULL, &error); + if (!ret && g_error_matches(error, G_DBUS_ERROR, G_DBUS_ERROR_UNKNOWN_METHOD)) { + _LOG2D( + request_id, + log_ifname, + log_con_uuid, + "dispatcher service does not implement Action2() method, falling back to Action()"); + g_clear_error(&error); + parameters_floating = build_call_parameters(action, + device, + settings_connection, + applied_connection, + activation_type_external, + connectivity_state, + vpn_iface, + l3cd, + FALSE); + ret = g_dbus_connection_call_sync(gl.dbus_connection, + NM_DISPATCHER_DBUS_SERVICE, + NM_DISPATCHER_DBUS_PATH, + NM_DISPATCHER_DBUS_INTERFACE, + "Action", + g_steal_pointer(¶meters_floating), + G_VARIANT_TYPE("(a(sus))"), + G_DBUS_CALL_FLAGS_NONE, + CALL_TIMEOUT, + NULL, + &error); + is_action2 = FALSE; + } + now_msec = nm_utils_get_monotonic_timestamp_msec(); if (!ret) { @@ -676,7 +789,8 @@ _dispatcher_call(NMDispatcherAction action, now_msec, log_ifname, log_con_uuid, - ret); + ret, + is_action2); return TRUE; } @@ -688,13 +802,25 @@ _dispatcher_call(NMDispatcherAction action, log_ifname, log_con_uuid); + /* Since we don't want to cache all the input parameters, already build + * and cache the argument for the Action() method in case Action2() fails. */ + call_id->action_params = build_call_parameters(action, + device, + settings_connection, + applied_connection, + activation_type_external, + connectivity_state, + vpn_iface, + l3cd, + FALSE); + g_dbus_connection_call(gl.dbus_connection, NM_DISPATCHER_DBUS_SERVICE, NM_DISPATCHER_DBUS_PATH, NM_DISPATCHER_DBUS_INTERFACE, - "Action", + "Action2", g_steal_pointer(¶meters_floating), - G_VARIANT_TYPE("(a(sus))"), + G_VARIANT_TYPE("(a(susa{sv}))"), G_DBUS_CALL_FLAGS_NONE, CALL_TIMEOUT, NULL, From ee5845063d81ba5da7ed68b4caf9d0920ebb78db Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 29 Sep 2023 14:07:53 +0200 Subject: [PATCH 10/15] dispatcher: support device-handler actions "device-add" and "device-delete" actions are called for device-handlers of generic devices. They differ from other actions in the following aspects: - only one script is invoked, the one with name specified by the device-handler property; - the script is searched in the "device" subdirectory; - since there is only one script executed, the result and error string from that script are returned by NM in the callback function. --- src/core/nm-dispatcher.c | 215 +++++++++++++----- src/core/nm-dispatcher.h | 15 ++ src/libnm-core-aux-extern/nm-dispatcher-api.h | 2 + src/nm-dispatcher/nm-dispatcher.c | 108 +++++++-- 4 files changed, 263 insertions(+), 77 deletions(-) diff --git a/src/core/nm-dispatcher.c b/src/core/nm-dispatcher.c index f2016f14e5..b6c3cd6dc4 100644 --- a/src/core/nm-dispatcher.c +++ b/src/core/nm-dispatcher.c @@ -52,18 +52,22 @@ /*****************************************************************************/ +/* Type for generic callback function; must be cast to either + * NMDispatcherFunc or NMDispatcherFuncDH before using. */ +typedef void (*NMDispatcherCallback)(void); + struct NMDispatcherCallId { - NMDispatcherFunc callback; - gpointer user_data; - const char *log_ifname; - const char *log_con_uuid; - GVariant *action_params; - gint64 start_at_msec; - NMDispatcherAction action; - guint idle_id; - guint32 request_id; - bool is_action2 : 1; - char extra_strings[]; + NMDispatcherCallback callback; + gpointer user_data; + const char *log_ifname; + const char *log_con_uuid; + GVariant *action_params; + gint64 start_at_msec; + NMDispatcherAction action; + guint idle_id; + guint32 request_id; + bool is_action2 : 1; + char extra_strings[]; }; /*****************************************************************************/ @@ -98,14 +102,20 @@ action_need_device(NMDispatcherAction action) return TRUE; } +static gboolean +action_is_device_handler(NMDispatcherAction action) +{ + return NM_IN_SET(action, NM_DISPATCHER_ACTION_DEVICE_ADD, NM_DISPATCHER_ACTION_DEVICE_DELETE); +} + static NMDispatcherCallId * -dispatcher_call_id_new(guint32 request_id, - gint64 start_at_msec, - NMDispatcherAction action, - NMDispatcherFunc callback, - gpointer user_data, - const char *log_ifname, - const char *log_con_uuid) +dispatcher_call_id_new(guint32 request_id, + gint64 start_at_msec, + NMDispatcherAction action, + NMDispatcherCallback callback, + gpointer user_data, + const char *log_ifname, + const char *log_con_uuid) { NMDispatcherCallId *call_id; gsize l_log_ifname; @@ -388,19 +398,42 @@ dispatch_result_to_string(DispatchResult result) g_assert_not_reached(); } +/* + * dispatcher_results_process: + * @action: the dispatcher action + * @request_id: request id + * @start_at_msec: the timestamp at which the dispatcher call was started + * @now_msec: the current timestamp in milliseconds + * @log_ifname: the interface name for logging + * @log_con_uuid: the connection UUID for logging + * @out_success: (out): for device-handler actions, the result of the script + * @out_error_msg: (out)(transfer full): for device-handler actions, the + * error message in case of failure + * @v_results: the GVariant containing the results to parse + * @is_action2: whether the D-Bus method is "Action2()" (or "Action()") + * + * Process the results of the dispatcher call. + * + */ static void -dispatcher_results_process(guint32 request_id, - gint64 start_at_msec, - gint64 now_msec, - const char *log_ifname, - const char *log_con_uuid, - GVariant *v_results, - gboolean is_action2) +dispatcher_results_process(NMDispatcherAction action, + guint32 request_id, + gint64 start_at_msec, + gint64 now_msec, + const char *log_ifname, + const char *log_con_uuid, + gboolean *out_success, + char **out_error_msg, + GVariant *v_results, + gboolean is_action2) { nm_auto_free_variant_iter GVariantIter *results = NULL; const char *script, *err; guint32 result; gsize n_children; + gboolean action_is_dh = action_is_device_handler(action); + + nm_assert(!action_is_dh || is_action2); if (is_action2) g_variant_get(v_results, "(a(susa{sv}))", &results); @@ -417,8 +450,13 @@ dispatcher_results_process(guint32 request_id, (int) ((now_msec - start_at_msec) % 1000), n_children); - if (n_children == 0) + if (n_children == 0) { + if (action_is_dh) { + NM_SET_OUT(out_success, FALSE); + NM_SET_OUT(out_error_msg, g_strdup("no result returned from dispatcher service")); + } return; + } while (TRUE) { gs_unref_variant GVariant *options = NULL; @@ -442,6 +480,11 @@ dispatcher_results_process(guint32 request_id, dispatch_result_to_string(result), err); } + if (action_is_dh) { + NM_SET_OUT(out_success, result == DISPATCH_RESULT_SUCCESS); + NM_SET_OUT(out_error_msg, g_strdup(err)); + break; + } } } @@ -452,6 +495,9 @@ dispatcher_done_cb(GObject *source, GAsyncResult *result, gpointer user_data) gs_free_error GError *error = NULL; NMDispatcherCallId *call_id = user_data; gint64 now_msec; + gboolean action_is_dh; + gboolean success = TRUE; + gs_free char *error_msg = NULL; nm_assert((gpointer) source == gl.dbus_connection); @@ -459,7 +505,7 @@ dispatcher_done_cb(GObject *source, GAsyncResult *result, gpointer user_data) ret = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), result, &error); - if (!ret && call_id->is_action2 + if (!ret && call_id->is_action2 && !action_is_device_handler(call_id->action) && g_error_matches(error, G_DBUS_ERROR, G_DBUS_ERROR_UNKNOWN_METHOD)) { _LOG3D(call_id, "dispatcher service does not implement Action2() method, falling back to Action()"); @@ -493,38 +539,54 @@ dispatcher_done_cb(GObject *source, GAsyncResult *result, gpointer user_data) (int) ((now_msec - call_id->start_at_msec) % 1000), error->message); } else { - dispatcher_results_process(call_id->request_id, + dispatcher_results_process(call_id->action, + call_id->request_id, call_id->start_at_msec, now_msec, call_id->log_ifname, call_id->log_con_uuid, + &success, + &error_msg, ret, call_id->is_action2); } g_hash_table_remove(gl.requests, call_id); + action_is_dh = action_is_device_handler(call_id->action); - if (call_id->callback) - call_id->callback(call_id, call_id->user_data); + if (call_id->callback) { + if (action_is_dh) { + NMDispatcherFuncDH cb = (NMDispatcherFuncDH) call_id->callback; + + cb(call_id, call_id->user_data, success, error_msg); + } else { + NMDispatcherFunc cb = (NMDispatcherFunc) call_id->callback; + + cb(call_id, call_id->user_data); + } + } dispatcher_call_id_free(call_id); } -static const char *action_table[] = {[NM_DISPATCHER_ACTION_HOSTNAME] = NMD_ACTION_HOSTNAME, - [NM_DISPATCHER_ACTION_PRE_UP] = NMD_ACTION_PRE_UP, - [NM_DISPATCHER_ACTION_UP] = NMD_ACTION_UP, - [NM_DISPATCHER_ACTION_PRE_DOWN] = NMD_ACTION_PRE_DOWN, - [NM_DISPATCHER_ACTION_DOWN] = NMD_ACTION_DOWN, - [NM_DISPATCHER_ACTION_VPN_PRE_UP] = NMD_ACTION_VPN_PRE_UP, - [NM_DISPATCHER_ACTION_VPN_UP] = NMD_ACTION_VPN_UP, - [NM_DISPATCHER_ACTION_VPN_PRE_DOWN] = NMD_ACTION_VPN_PRE_DOWN, - [NM_DISPATCHER_ACTION_VPN_DOWN] = NMD_ACTION_VPN_DOWN, - [NM_DISPATCHER_ACTION_DHCP_CHANGE_4] = NMD_ACTION_DHCP4_CHANGE, - [NM_DISPATCHER_ACTION_DHCP_CHANGE_6] = NMD_ACTION_DHCP6_CHANGE, - [NM_DISPATCHER_ACTION_CONNECTIVITY_CHANGE] = - NMD_ACTION_CONNECTIVITY_CHANGE, - [NM_DISPATCHER_ACTION_REAPPLY] = NMD_ACTION_REAPPLY, - [NM_DISPATCHER_ACTION_DNS_CHANGE] = NMD_ACTION_DNS_CHANGE}; +static const char *action_table[] = { + [NM_DISPATCHER_ACTION_HOSTNAME] = NMD_ACTION_HOSTNAME, + [NM_DISPATCHER_ACTION_PRE_UP] = NMD_ACTION_PRE_UP, + [NM_DISPATCHER_ACTION_UP] = NMD_ACTION_UP, + [NM_DISPATCHER_ACTION_PRE_DOWN] = NMD_ACTION_PRE_DOWN, + [NM_DISPATCHER_ACTION_DOWN] = NMD_ACTION_DOWN, + [NM_DISPATCHER_ACTION_VPN_PRE_UP] = NMD_ACTION_VPN_PRE_UP, + [NM_DISPATCHER_ACTION_VPN_UP] = NMD_ACTION_VPN_UP, + [NM_DISPATCHER_ACTION_VPN_PRE_DOWN] = NMD_ACTION_VPN_PRE_DOWN, + [NM_DISPATCHER_ACTION_VPN_DOWN] = NMD_ACTION_VPN_DOWN, + [NM_DISPATCHER_ACTION_DHCP_CHANGE_4] = NMD_ACTION_DHCP4_CHANGE, + [NM_DISPATCHER_ACTION_DHCP_CHANGE_6] = NMD_ACTION_DHCP6_CHANGE, + [NM_DISPATCHER_ACTION_CONNECTIVITY_CHANGE] = NMD_ACTION_CONNECTIVITY_CHANGE, + [NM_DISPATCHER_ACTION_REAPPLY] = NMD_ACTION_REAPPLY, + [NM_DISPATCHER_ACTION_DNS_CHANGE] = NMD_ACTION_DNS_CHANGE, + [NM_DISPATCHER_ACTION_DEVICE_ADD] = NMD_ACTION_DEVICE_ADD, + [NM_DISPATCHER_ACTION_DEVICE_DELETE] = NMD_ACTION_DEVICE_DELETE, +}; static const char * action_to_string(NMDispatcherAction action) @@ -664,7 +726,7 @@ _dispatcher_call(NMDispatcherAction action, NMConnectivityState connectivity_state, const char *vpn_iface, const NML3ConfigData *l3cd, - NMDispatcherFunc callback, + NMDispatcherCallback callback, gpointer user_data, NMDispatcherCallId **out_call_id) { @@ -784,11 +846,14 @@ _dispatcher_call(NMDispatcherAction action, error->message); return FALSE; } - dispatcher_results_process(request_id, + dispatcher_results_process(action, + request_id, start_at_msec, now_msec, log_ifname, log_con_uuid, + NULL, + NULL, ret, is_action2); return TRUE; @@ -856,7 +921,7 @@ nm_dispatcher_call_hostname(NMDispatcherFunc callback, NM_CONNECTIVITY_UNKNOWN, NULL, NULL, - callback, + (NMDispatcherCallback) callback, user_data, out_call_id); } @@ -866,7 +931,7 @@ _dispatcher_call_device(NMDispatcherAction action, NMDevice *device, gboolean blocking, NMActRequest *act_request, - NMDispatcherFunc callback, + NMDispatcherCallback callback, gpointer user_data, NMDispatcherCallId **out_call_id) { @@ -919,11 +984,48 @@ nm_dispatcher_call_device(NMDispatcherAction action, gpointer user_data, NMDispatcherCallId **out_call_id) { + g_return_val_if_fail(!action_is_device_handler(action), FALSE); + return _dispatcher_call_device(action, device, FALSE, act_request, - callback, + (NMDispatcherCallback) callback, + user_data, + out_call_id); +} + +/** + * nm_dispatcher_call_device_handler: + * @action: the %NMDispatcherAction, must be device-add or device-remove + * @device: the #NMDevice the action applies to + * @act_request: the #NMActRequest for the action. If %NULL, use the + * current request of the device. + * @callback: a caller-supplied device-handler callback to execute when done + * @user_data: caller-supplied pointer passed to @callback + * @out_call_id: on success, a call identifier which can be passed to + * nm_dispatcher_call_cancel() + * + * This method always invokes the device dispatcher action asynchronously. To ignore + * the result, pass %NULL to @callback. + * + * Returns: %TRUE if the action was dispatched, %FALSE on failure + */ +gboolean +nm_dispatcher_call_device_handler(NMDispatcherAction action, + NMDevice *device, + NMActRequest *act_request, + NMDispatcherFuncDH callback, + gpointer user_data, + NMDispatcherCallId **out_call_id) +{ + g_return_val_if_fail(action_is_device_handler(action), FALSE); + + return _dispatcher_call_device(action, + device, + FALSE, + act_request, + (NMDispatcherCallback) callback, user_data, out_call_id); } @@ -945,6 +1047,8 @@ nm_dispatcher_call_device_sync(NMDispatcherAction action, NMDevice *device, NMActRequest *act_request) { + g_return_val_if_fail(!action_is_device_handler(action), FALSE); + return _dispatcher_call_device(action, device, TRUE, act_request, NULL, NULL, NULL); } @@ -986,7 +1090,7 @@ nm_dispatcher_call_vpn(NMDispatcherAction action, NM_CONNECTIVITY_UNKNOWN, vpn_iface, l3cd, - callback, + (NMDispatcherCallback) callback, user_data, out_call_id); } @@ -1013,6 +1117,8 @@ nm_dispatcher_call_vpn_sync(NMDispatcherAction action, const char *vpn_iface, const NML3ConfigData *l3cd) { + g_return_val_if_fail(!action_is_device_handler(action), FALSE); + return _dispatcher_call(action, TRUE, parent_device, @@ -1054,7 +1160,7 @@ nm_dispatcher_call_connectivity(NMConnectivityState connectivity_state, connectivity_state, NULL, NULL, - callback, + (NMDispatcherCallback) callback, user_data, out_call_id); } @@ -1086,7 +1192,10 @@ nm_dispatcher_call_dns_change(void) void nm_dispatcher_call_cancel(NMDispatcherCallId *call_id) { - if (!call_id || g_hash_table_lookup(gl.requests, call_id) != call_id || !call_id->callback) + if (!call_id || g_hash_table_lookup(gl.requests, call_id) != call_id) + g_return_if_reached(); + + if (!call_id->callback) g_return_if_reached(); /* Canceling just means the callback doesn't get called, so set the diff --git a/src/core/nm-dispatcher.h b/src/core/nm-dispatcher.h index a1cb96b798..fc317ca899 100644 --- a/src/core/nm-dispatcher.h +++ b/src/core/nm-dispatcher.h @@ -24,6 +24,8 @@ typedef enum { NM_DISPATCHER_ACTION_CONNECTIVITY_CHANGE, NM_DISPATCHER_ACTION_REAPPLY, NM_DISPATCHER_ACTION_DNS_CHANGE, + NM_DISPATCHER_ACTION_DEVICE_ADD, + NM_DISPATCHER_ACTION_DEVICE_DELETE, } NMDispatcherAction; #define NM_DISPATCHER_ACTION_DHCP_CHANGE_X(IS_IPv4) \ @@ -31,7 +33,13 @@ typedef enum { typedef struct NMDispatcherCallId NMDispatcherCallId; +/* Callback function for regular dispatcher calls */ typedef void (*NMDispatcherFunc)(NMDispatcherCallId *call_id, gpointer user_data); +/* Callback function for device-handler dispatcher calls */ +typedef void (*NMDispatcherFuncDH)(NMDispatcherCallId *call_id, + gpointer user_data, + gboolean success, + const char *error_msg); gboolean nm_dispatcher_call_hostname(NMDispatcherFunc callback, gpointer user_data, @@ -44,6 +52,13 @@ gboolean nm_dispatcher_call_device(NMDispatcherAction action, gpointer user_data, NMDispatcherCallId **out_call_id); +gboolean nm_dispatcher_call_device_handler(NMDispatcherAction action, + NMDevice *device, + NMActRequest *act_request, + NMDispatcherFuncDH callback_dh, + gpointer user_data, + NMDispatcherCallId **out_call_id); + gboolean nm_dispatcher_call_device_sync(NMDispatcherAction action, NMDevice *device, NMActRequest *act_request); diff --git a/src/libnm-core-aux-extern/nm-dispatcher-api.h b/src/libnm-core-aux-extern/nm-dispatcher-api.h index 7cb370a92e..635b4fb3a8 100644 --- a/src/libnm-core-aux-extern/nm-dispatcher-api.h +++ b/src/libnm-core-aux-extern/nm-dispatcher-api.h @@ -35,6 +35,8 @@ #define NMD_ACTION_CONNECTIVITY_CHANGE "connectivity-change" #define NMD_ACTION_REAPPLY "reapply" #define NMD_ACTION_DNS_CHANGE "dns-change" +#define NMD_ACTION_DEVICE_ADD "device-add" +#define NMD_ACTION_DEVICE_DELETE "device-delete" typedef enum { DISPATCH_RESULT_UNKNOWN = 0, diff --git a/src/nm-dispatcher/nm-dispatcher.c b/src/nm-dispatcher/nm-dispatcher.c index 722d6e290d..a28b895a64 100644 --- a/src/nm-dispatcher/nm-dispatcher.c +++ b/src/nm-dispatcher/nm-dispatcher.c @@ -86,6 +86,7 @@ struct Request { char **envp; gboolean debug; gboolean is_action2; + gboolean is_device_handler; GPtrArray *scripts; /* list of ScriptInfo */ guint idx; @@ -615,6 +616,31 @@ _compare_basenames(gconstpointer a, gconstpointer b) return 0; } +static gboolean +check_file(Request *request, const char *path) +{ + gs_free char *link_target = NULL; + const char *err_msg = NULL; + struct stat st; + int err; + + link_target = g_file_read_link(path, NULL); + if (nm_streq0(link_target, "/dev/null")) + return FALSE; + + err = stat(path, &st); + if (err) { + return FALSE; + } else if (!S_ISREG(st.st_mode) || st.st_size == 0) { + /* silently skip. */ + return FALSE; + } else if (!check_permissions(&st, &err_msg)) { + _LOG_R_W(request, "find-scripts: Cannot execute '%s': %s", path, err_msg); + return FALSE; + } + return TRUE; +} + static void _find_scripts(Request *request, GHashTable *scripts, const char *base, const char *subdir) { @@ -647,7 +673,7 @@ _find_scripts(Request *request, GHashTable *scripts, const char *base, const cha } static GSList * -find_scripts(Request *request) +find_scripts(Request *request, const char *device_handler) { gs_unref_hashtable GHashTable *scripts = NULL; GSList *script_list = NULL; @@ -656,6 +682,33 @@ find_scripts(Request *request) char *path; char *filename; + if (request->is_device_handler) { + const char *const dirs[] = {NMCONFDIR, NMLIBDIR}; + guint i; + + nm_assert(device_handler); + + for (i = 0; i < G_N_ELEMENTS(dirs); i++) { + gs_free char *full_name = NULL; + + full_name = g_build_filename(dirs[i], "dispatcher.d", "device", device_handler, NULL); + if (check_file(request, full_name)) { + script_list = g_slist_prepend(script_list, g_steal_pointer(&full_name)); + return script_list; + } + } + + _LOG_R_W(request, + "find-scripts: no device-handler script found with name \"%s\"", + device_handler); + return NULL; + } + + nm_assert(!device_handler); + + /* Use a hash-table to deduplicate scripts with same name from /etc and /usr */ + scripts = g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, g_free); + if (NM_IN_STRSET(request->action, NMD_ACTION_PRE_UP, NMD_ACTION_VPN_PRE_UP)) subdir = "pre-up.d"; else if (NM_IN_STRSET(request->action, NMD_ACTION_PRE_DOWN, NMD_ACTION_VPN_PRE_DOWN)) @@ -663,33 +716,13 @@ find_scripts(Request *request) else subdir = NULL; - scripts = g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, g_free); - _find_scripts(request, scripts, NMLIBDIR, subdir); _find_scripts(request, scripts, NMCONFDIR, subdir); g_hash_table_iter_init(&iter, scripts); while (g_hash_table_iter_next(&iter, (gpointer *) &filename, (gpointer *) &path)) { - gs_free char *link_target = NULL; - const char *err_msg = NULL; - struct stat st; - int err; - - link_target = g_file_read_link(path, NULL); - if (nm_streq0(link_target, "/dev/null")) - continue; - - err = stat(path, &st); - if (err) - _LOG_R_W(request, "find-scripts: Failed to stat '%s': %d", path, err); - else if (!S_ISREG(st.st_mode) || st.st_size == 0) { - /* silently skip. */ - } else if (!check_permissions(&st, &err_msg)) - _LOG_R_W(request, "find-scripts: Cannot execute '%s': %s", path, err_msg); - else { - /* success */ + if (check_file(request, path)) { script_list = g_slist_prepend(script_list, g_strdup(path)); - continue; } } @@ -725,6 +758,27 @@ script_must_wait(const char *path) return TRUE; } +static char * +get_device_handler(GVariant *connection) +{ + gs_unref_variant GVariant *generic_setting = NULL; + const char *device_handler = NULL; + + generic_setting = g_variant_lookup_value(connection, + NM_SETTING_GENERIC_SETTING_NAME, + NM_VARIANT_TYPE_SETTING); + if (generic_setting) { + if (g_variant_lookup(generic_setting, + NM_SETTING_GENERIC_DEVICE_HANDLER, + "&s", + &device_handler)) { + return g_strdup(device_handler); + } + } + + return NULL; +} + static void _handle_action(GDBusMethodInvocation *invocation, GVariant *parameters, gboolean is_action2) { @@ -739,6 +793,7 @@ _handle_action(GDBusMethodInvocation *invocation, GVariant *parameters, gboolean gs_unref_variant GVariant *device_dhcp6_config = NULL; const char *connectivity_state; const char *vpn_ip_iface; + gs_free char *device_handler = NULL; gs_unref_variant GVariant *vpn_proxy_properties = NULL; gs_unref_variant GVariant *vpn_ip4_config = NULL; gs_unref_variant GVariant *vpn_ip6_config = NULL; @@ -829,6 +884,8 @@ _handle_action(GDBusMethodInvocation *invocation, GVariant *parameters, gboolean request->context = invocation; request->action = g_strdup(action); request->is_action2 = is_action2; + request->is_device_handler = + NM_IN_STRSET(action, NMD_ACTION_DEVICE_ADD, NMD_ACTION_DEVICE_DELETE); request->envp = nm_dispatcher_utils_construct_envp(action, connection, @@ -846,11 +903,14 @@ _handle_action(GDBusMethodInvocation *invocation, GVariant *parameters, gboolean vpn_ip6_config, &request->iface, &error_message); - if (!error_message) { + if (request->is_device_handler) { + device_handler = get_device_handler(connection); + } + request->scripts = g_ptr_array_new_full(5, script_info_free); - sorted_scripts = find_scripts(request); + sorted_scripts = find_scripts(request, device_handler); for (iter = sorted_scripts; iter; iter = g_slist_next(iter)) { ScriptInfo *s; From d72f26b87528836b273c35a7e35f96d749cc02d3 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 9 Nov 2023 23:13:55 +0100 Subject: [PATCH 11/15] dispatcher: read device-handler's stdout into a dictionary Device handlers need a way to pass data back to NetworkManager, such as the ifindex and an error message. Allow them to return a dictionary on standard output, where each line contains a "$key=$value" pair. In the daemon, the dictionary is returned via the callback function. --- man/NetworkManager-dispatcher.xml | 32 ++++- src/core/nm-dispatcher.c | 71 +++++++++-- src/core/nm-dispatcher.h | 3 +- src/nm-dispatcher/nm-dispatcher.c | 189 +++++++++++++++++++++++++----- 4 files changed, 253 insertions(+), 42 deletions(-) diff --git a/man/NetworkManager-dispatcher.xml b/man/NetworkManager-dispatcher.xml index 8d7f0c59ba..f85a495ac7 100644 --- a/man/NetworkManager-dispatcher.xml +++ b/man/NetworkManager-dispatcher.xml @@ -186,7 +186,31 @@ The script needs to perform any action needed to create the device for the generic connection. On successful termination, the script returns zero. Otherwise, it returns a non-zero value to indicate an - error. + error. The script can return values to NetworkManager by writing to + standard output; each line should contain a key name followed by the + equal sign '=' and a key value. The keys understood at the moment + are: + + + IFINDEX + Indicates the interface index of the interface + created by the script. This key is required when the script + succeeds; if it is not set, the activation will fail. The key is + ignored in case of script failure. + + + ERROR + Specifies an error message indicating the cause + of the script failure. It is ignored when the script succeeds. + + + + Since the dispatcher service captures stdout for parsing those keys, + anything written to stdout will not appear in the dispatcher service + journal log. Use stderr if you want to print messages to the journal + (for example, for debugging). Only the first 8KiB of stdout are + considered and among those, only the first 64 lines; the rest is + ignored. @@ -197,7 +221,11 @@ This action is the counterpart of device-add and is called to delete the device for a generic connection. All the aspects described for device-add also apply to - this action. + this action, with the only exception that key + IFINDEX is ignored. It is not necessary to delete + the kernel link in the handler because NetworkManager already does + that; therefore the action is useful for any additional cleanup + needed. diff --git a/src/core/nm-dispatcher.c b/src/core/nm-dispatcher.c index b6c3cd6dc4..a5b351f3e8 100644 --- a/src/core/nm-dispatcher.c +++ b/src/core/nm-dispatcher.c @@ -409,6 +409,8 @@ dispatch_result_to_string(DispatchResult result) * @out_success: (out): for device-handler actions, the result of the script * @out_error_msg: (out)(transfer full): for device-handler actions, the * error message in case of failure + * @out_dict: (out)(transfer full): for device-handler actions, the output + * dictionary in case of success * @v_results: the GVariant containing the results to parse * @is_action2: whether the D-Bus method is "Action2()" (or "Action()") * @@ -424,6 +426,7 @@ dispatcher_results_process(NMDispatcherAction action, const char *log_con_uuid, gboolean *out_success, char **out_error_msg, + GHashTable **out_dict, GVariant *v_results, gboolean is_action2) { @@ -454,6 +457,7 @@ dispatcher_results_process(NMDispatcherAction action, if (action_is_dh) { NM_SET_OUT(out_success, FALSE); NM_SET_OUT(out_error_msg, g_strdup("no result returned from dispatcher service")); + NM_SET_OUT(out_dict, NULL); } return; } @@ -480,9 +484,53 @@ dispatcher_results_process(NMDispatcherAction action, dispatch_result_to_string(result), err); } + if (action_is_dh) { - NM_SET_OUT(out_success, result == DISPATCH_RESULT_SUCCESS); - NM_SET_OUT(out_error_msg, g_strdup(err)); + if (result == DISPATCH_RESULT_SUCCESS) { + gs_unref_variant GVariant *output_dict = NULL; + gs_unref_hashtable GHashTable *hash = NULL; + GVariantIter iter; + const char *value; + const char *key; + + hash = g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, g_free); + output_dict = + g_variant_lookup_value(options, "output_dict", G_VARIANT_TYPE("a{ss}")); + if (output_dict) { + g_variant_iter_init(&iter, output_dict); + while (g_variant_iter_next(&iter, "{&s&s}", &key, &value)) { + const char *unescaped; + gpointer to_free; + gsize len; + + unescaped = nm_utils_buf_utf8safe_unescape(value, + NM_UTILS_STR_UTF8_SAFE_FLAG_NONE, + &len, + &to_free); + g_hash_table_insert(hash, + g_strdup(key), + ((char *) to_free) ?: g_strdup(unescaped)); + } + } + + NM_SET_OUT(out_success, TRUE); + NM_SET_OUT(out_dict, g_steal_pointer(&hash)); + NM_SET_OUT(out_error_msg, NULL); + } else { + gs_unref_variant GVariant *output_dict = NULL; + const char *err2 = NULL; + + output_dict = + g_variant_lookup_value(options, "output_dict", G_VARIANT_TYPE("a{ss}")); + if (output_dict) { + g_variant_lookup(output_dict, "ERROR", "&s", &err2); + } + + NM_SET_OUT(out_success, FALSE); + NM_SET_OUT(out_dict, NULL); + NM_SET_OUT(out_error_msg, + err2 ? g_strdup_printf("%s: Error: %s", err, err2) : g_strdup(err)); + } break; } } @@ -491,13 +539,14 @@ dispatcher_results_process(NMDispatcherAction action, static void dispatcher_done_cb(GObject *source, GAsyncResult *result, gpointer user_data) { - gs_unref_variant GVariant *ret = NULL; - gs_free_error GError *error = NULL; - NMDispatcherCallId *call_id = user_data; - gint64 now_msec; - gboolean action_is_dh; - gboolean success = TRUE; - gs_free char *error_msg = NULL; + gs_unref_variant GVariant *ret = NULL; + gs_free_error GError *error = NULL; + NMDispatcherCallId *call_id = user_data; + gint64 now_msec; + gboolean action_is_dh; + gboolean success = TRUE; + gs_free char *error_msg = NULL; + gs_unref_hashtable GHashTable *hash = NULL; nm_assert((gpointer) source == gl.dbus_connection); @@ -547,6 +596,7 @@ dispatcher_done_cb(GObject *source, GAsyncResult *result, gpointer user_data) call_id->log_con_uuid, &success, &error_msg, + &hash, ret, call_id->is_action2); } @@ -558,7 +608,7 @@ dispatcher_done_cb(GObject *source, GAsyncResult *result, gpointer user_data) if (action_is_dh) { NMDispatcherFuncDH cb = (NMDispatcherFuncDH) call_id->callback; - cb(call_id, call_id->user_data, success, error_msg); + cb(call_id, call_id->user_data, success, error_msg, hash); } else { NMDispatcherFunc cb = (NMDispatcherFunc) call_id->callback; @@ -854,6 +904,7 @@ _dispatcher_call(NMDispatcherAction action, log_con_uuid, NULL, NULL, + NULL, ret, is_action2); return TRUE; diff --git a/src/core/nm-dispatcher.h b/src/core/nm-dispatcher.h index fc317ca899..2882503b04 100644 --- a/src/core/nm-dispatcher.h +++ b/src/core/nm-dispatcher.h @@ -39,7 +39,8 @@ typedef void (*NMDispatcherFunc)(NMDispatcherCallId *call_id, gpointer user_data typedef void (*NMDispatcherFuncDH)(NMDispatcherCallId *call_id, gpointer user_data, gboolean success, - const char *error_msg); + const char *error_msg, + GHashTable *dict); gboolean nm_dispatcher_call_hostname(NMDispatcherFunc callback, gpointer user_data, diff --git a/src/nm-dispatcher/nm-dispatcher.c b/src/nm-dispatcher/nm-dispatcher.c index a28b895a64..efb4ec0087 100644 --- a/src/nm-dispatcher/nm-dispatcher.c +++ b/src/nm-dispatcher/nm-dispatcher.c @@ -20,6 +20,7 @@ #include "libnm-core-aux-extern/nm-dispatcher-api.h" #include "libnm-glib-aux/nm-dbus-aux.h" #include "libnm-glib-aux/nm-io-utils.h" +#include "libnm-glib-aux/nm-str-buf.h" #include "libnm-glib-aux/nm-time-utils.h" #include "nm-dispatcher-utils.h" @@ -75,6 +76,10 @@ typedef struct { gboolean dispatched; GSource *watch_source; GSource *timeout_source; + + int stdout_fd; + GSource *stdout_source; + NMStrBuf stdout_buffer; } ScriptInfo; struct Request { @@ -194,6 +199,12 @@ script_info_free(gpointer ptr) { ScriptInfo *info = ptr; + nm_assert(info->pid == -1); + nm_assert(info->stdout_fd == -1); + nm_assert(!info->stdout_source); + nm_assert(!info->timeout_source); + nm_assert(!info->watch_source); + g_free(info->script); g_free(info->error); g_slice_free(ScriptInfo, info); @@ -282,6 +293,64 @@ next_request(Request *request) return TRUE; } +static GVariant * +build_result_options(char *stdout) +{ + gs_unref_hashtable GHashTable *hash = NULL; + GHashTableIter iter; + gs_strfreev char **lines = NULL; + GVariantBuilder builder_opts; + GVariantBuilder builder_out_dict; + guint i; + char *eq; + char *key; + char *value; + + lines = g_strsplit(stdout, "\n", 65); + + for (i = 0; lines[i] && i < 64; i++) { + eq = strchr(lines[i], '='); + if (!eq) + continue; + *eq = '\0'; + + if (!NM_STRCHAR_ALL(lines[i], + ch, + (ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') || ch == '_')) + continue; + + if (!hash) { + hash = g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, g_free); + } + + g_hash_table_insert(hash, g_strdup(lines[i]), g_strdup(eq + 1)); + } + + g_variant_builder_init(&builder_out_dict, G_VARIANT_TYPE("a{ss}")); + if (hash) { + g_hash_table_iter_init(&iter, hash); + while (g_hash_table_iter_next(&iter, (gpointer *) &key, (gpointer *) &value)) { + gs_free char *to_free = NULL; + + g_variant_builder_add(&builder_out_dict, + "{ss}", + key, + nm_utils_buf_utf8safe_escape(value, + -1, + NM_UTILS_STR_UTF8_SAFE_FLAG_NONE, + &to_free)); + } + } + + g_variant_builder_init(&builder_opts, G_VARIANT_TYPE("a{sv}")); + g_variant_builder_add(&builder_opts, + "{sv}", + "output_dict", + g_variant_builder_end(&builder_out_dict)); + + return g_variant_builder_end(&builder_opts); +} + static void request_dbus_method_return(Request *request) { @@ -295,7 +364,14 @@ request_dbus_method_return(Request *request) } for (i = 0; i < request->scripts->len; i++) { - ScriptInfo *script = g_ptr_array_index(request->scripts, i); + ScriptInfo *script = g_ptr_array_index(request->scripts, i); + GVariant *options = NULL; + gs_free char *stdout = NULL; + + if (request->is_device_handler) { + stdout = nm_str_buf_finalize(&script->stdout_buffer, NULL); + options = build_result_options(stdout); + } if (request->is_action2) { g_variant_builder_add(&results, @@ -303,7 +379,7 @@ request_dbus_method_return(Request *request) script->script, script->result, script->error ?: "", - nm_g_variant_singleton_aLsvI()); + options ?: nm_g_variant_singleton_aLsvI()); } else { g_variant_builder_add(&results, "(sus)", @@ -356,10 +432,17 @@ complete_request(Request *request) static void complete_script(ScriptInfo *script) { - Request *request; - gboolean wait = script->wait; + Request *request = script->request; + gboolean wait = script->wait; - request = script->request; + if (script->pid != -1 || script->stdout_fd != -1) { + /* Wait that process has terminated and stdout is closed */ + return; + } + + script->request->num_scripts_done++; + if (!script->wait) + script->request->num_scripts_nowait--; if (wait) { /* for "wait" scripts, try to schedule the next blocking script. @@ -427,14 +510,12 @@ script_watch_cb(GPid pid, int status, gpointer user_data) nm_clear_g_source_inst(&script->watch_source); nm_clear_g_source_inst(&script->timeout_source); - script->request->num_scripts_done++; - if (!script->wait) - script->request->num_scripts_nowait--; if (WIFEXITED(status) && WEXITSTATUS(status) == 0) { script->result = DISPATCH_RESULT_SUCCESS; } else { - status_desc = nm_utils_get_process_exit_status_desc(status); + status_desc = nm_utils_get_process_exit_status_desc(status); + nm_clear_g_free(&script->error); script->error = g_strdup_printf("Script '%s' %s", script->script, status_desc); } @@ -445,8 +526,7 @@ script_watch_cb(GPid pid, int status, gpointer user_data) _LOG_S_W(script, "complete: process failed with %s", script->error); } - g_spawn_close_pid(script->pid); - + script->pid = -1; complete_script(script); } @@ -457,9 +537,8 @@ script_timeout_cb(gpointer user_data) nm_clear_g_source_inst(&script->timeout_source); nm_clear_g_source_inst(&script->watch_source); - script->request->num_scripts_done++; - if (!script->wait) - script->request->num_scripts_nowait--; + nm_clear_g_source_inst(&script->stdout_source); + nm_clear_fd(&script->stdout_fd); _LOG_S_W(script, "complete: timeout (kill script)"); @@ -473,8 +552,7 @@ again: script->error = g_strdup_printf("Script '%s' timed out", script->script); script->result = DISPATCH_RESULT_TIMEOUT; - g_spawn_close_pid(script->pid); - + script->pid = -1; complete_script(script); return G_SOURCE_CONTINUE; @@ -537,12 +615,46 @@ check_filename(const char *file_name) #define SCRIPT_TIMEOUT 600 /* 10 minutes */ +static gboolean +script_have_data(int fd, GIOCondition condition, gpointer user_data) +{ + ScriptInfo *script = user_data; + gssize n_read; + + n_read = nm_utils_fd_read(fd, &script->stdout_buffer); + + if (n_read == -EAGAIN) { + return G_SOURCE_CONTINUE; + } else if (n_read > 0) { + if (script->stdout_buffer.len < 8 * 1024) + return G_SOURCE_CONTINUE; + /* Don't allow the buffer to grow indefinitely. */ + _LOG_S_W(script, "complete: ignoring script stdout exceeding 8KiB"); + nm_str_buf_set_size(&script->stdout_buffer, 8 * 1024, FALSE, FALSE); + } else if (n_read == 0) { + _LOG_S_T(script, "complete: stdout closed"); + } else { + _LOG_S_T(script, + "complete: reading stdout failed with %d (%s)", + (int) n_read, + nm_strerror_native((int) -n_read)); + } + + nm_clear_g_source_inst(&script->stdout_source); + nm_clear_fd(&script->stdout_fd); + + complete_script(script); + + return G_SOURCE_CONTINUE; +} + static gboolean script_dispatch(ScriptInfo *script) { gs_free_error GError *error = NULL; char *argv[4]; - Request *request = script->request; + Request *request = script->request; + gboolean is_device_handler = script->request->is_device_handler; if (script->dispatched) return FALSE; @@ -559,14 +671,17 @@ script_dispatch(ScriptInfo *script) _LOG_S_T(script, "run script%s", script->wait ? "" : " (no-wait)"); - if (!g_spawn_async("/", - argv, - request->envp, - G_SPAWN_DO_NOT_REAP_CHILD, - NULL, - NULL, - &script->pid, - &error)) { + if (!g_spawn_async_with_pipes("/", + argv, + request->envp, + G_SPAWN_CLOEXEC_PIPES | G_SPAWN_DO_NOT_REAP_CHILD, + NULL, + NULL, + &script->pid, + NULL, + is_device_handler ? &script->stdout_fd : NULL, + NULL, + &error)) { _LOG_S_W(script, "complete: failed to execute script: %s", error->message); script->result = DISPATCH_RESULT_EXEC_FAILED; script->error = g_strdup(error->message); @@ -579,6 +694,19 @@ script_dispatch(ScriptInfo *script) nm_g_timeout_add_seconds_source(SCRIPT_TIMEOUT, script_timeout_cb, script); if (!script->wait) request->num_scripts_nowait++; + + if (is_device_handler) { + /* Watch process stdout */ + nm_io_fcntl_setfl_update_nonblock(script->stdout_fd); + script->stdout_source = nm_g_unix_fd_source_new(script->stdout_fd, + G_IO_IN | G_IO_ERR | G_IO_HUP, + G_PRIORITY_DEFAULT, + script_have_data, + script, + NULL); + g_source_attach(script->stdout_source, NULL); + } + return TRUE; } @@ -914,10 +1042,13 @@ _handle_action(GDBusMethodInvocation *invocation, GVariant *parameters, gboolean for (iter = sorted_scripts; iter; iter = g_slist_next(iter)) { ScriptInfo *s; - s = g_slice_new0(ScriptInfo); - s->request = request; - s->script = iter->data; - s->wait = script_must_wait(s->script); + s = g_slice_new0(ScriptInfo); + s->request = request; + s->script = iter->data; + s->wait = script_must_wait(s->script); + s->stdout_fd = -1; + s->pid = -1; + s->stdout_buffer = NM_STR_BUF_INIT(0, FALSE); g_ptr_array_add(request->scripts, s); } g_slist_free(sorted_scripts); From ae7ac3c8b73eec851da351982049eaee1540d1cb Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 28 Sep 2023 10:56:29 +0200 Subject: [PATCH 12/15] examples: add example device handler dispatcher for geneve --- examples/dispatcher/geneve | 116 +++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100755 examples/dispatcher/geneve diff --git a/examples/dispatcher/geneve b/examples/dispatcher/geneve new file mode 100755 index 0000000000..b4569902b7 --- /dev/null +++ b/examples/dispatcher/geneve @@ -0,0 +1,116 @@ +#!/bin/sh +# NetworkManager device handler for geneve interfaces. +# +# Put this script in "/etc/NetworkManager/dispatcher.d/device" and +# create a generic connection with: +# +# nmcli connection add type generic \ +# con-name geneve1 \ +# ifname geneve1 \ +# generic.device-handler geneve \ +# connection.autoconnect no +# +# Then add the following parameters at the bottom of file +# /etc/NetworkManager/system-connections/geneve1 , and do a "nmcli +# connection reload". +# +# [user] +# geneve.remote=172.25.14.15 +# geneve.vni=5555 +# geneve.dstport=6082 +# +# Now, when activating connection "geneve1", NetworkManager calls this +# script to create the device according to parameters in the user +# settings, and then performs IP configuration on it. +# +# This script will use the following properties from the [user] setting: +# +# - geneve.remote (required) +# - geneve.vni (required) +# - geneve.ttl +# - geneve.dstport +# +# See the GENEVE section of "man ip-link" for more details. + +ifname=$1 +action=$2 + +require() +{ + if ! command -v "$1" > /dev/null ; then + echo "ERROR='$1' is not installed" + exit 1 + fi +} + +get_iplink_param() +{ + ip -j -d link show "$1" | jq -r .[0].linkinfo.info_data."$2" +} + +require jq + +if [ "$action" = device-add ]; then + # Create the interface here and then write a line to stdout + # containing "IFINDEX=" followed by the ifindex of the interface + # just created, so that NetworkManager can manage it and configure + # IPs on the interface. The name of the returned ifindex must be + # the same as "$ifname". + + vni=$CONNECTION_USER_GENEVE__VNI + remote=$CONNECTION_USER_GENEVE__REMOTE + ttl=$CONNECTION_USER_GENEVE__TTL + dstport=$CONNECTION_USER_GENEVE__DSTPORT + + if [ -z "$vni" ] || [ -z "$remote" ]; then + echo "ERROR=Missing VNI or remote" + exit 2 + fi + + if [ -d /sys/class/net/"$ifname" ]; then + # If the interface already exists, reuse it after checking + # that the parameters are compatible. + # NOTE: it's not strictly necessary to handle an already + # existing interface, but if the script doesn't, it won't be + # possible to re-activate the connection when it's up. + + if [ "$vni" != "$(get_iplink_param "$ifname" id)" ]; then + echo "ERROR=The link already exists with different VNI" + exit 3 + fi + if [ "$remote" != "$(get_iplink_param "$ifname" remote)" ]; then + echo "ERROR=The link already exists with different remote" + exit 3 + fi + if [ -n "$ttl" ] && [ "$ttl" != "$(get_iplink_param "$ifname" ttl)" ]; then + echo "ERROR=The link already exists with different TTL" + exit 3 + fi + if [ -n "$dstport" ] && [ "$dstport" != "$(get_iplink_param "$ifname" port)" ]; then + echo "ERROR=The link already exists with different dstport" + exit 3 + fi + echo IFINDEX="$(cat /sys/class/net/"$ifname"/ifindex)" + exit 0 + fi + + # The interface doesn't exist, create it + + if ! err=$(ip link add "$ifname" type geneve vni "$vni" remote "$remote" \ + ${tos:+tos "$tos"} \ + ${ttl:+ttl "$ttl"} \ + ${dstport:+dstport "$dstport"} 2>&1); then + echo "ERROR=Failed creating the interface: $err" + exit 4 + fi + + echo IFINDEX="$(cat /sys/class/net/"$ifname"/ifindex)" + exit 0 +elif [ "$action" = device-delete ]; then + # NM automatically deletes the link on deactivation, + # it's not necessary to do it here. The "device-delete" + # action can be used to perform additional operations. + exit 0 +fi + +exit 5 From df6c35ec7553fd39f9ea224d0bd5779ad1a846b2 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 27 Sep 2023 09:07:13 +0200 Subject: [PATCH 13/15] device: support creating generic devices via device-handler If the device-handler of the generic connection is set, the connection is virtual and the device is created by invoking the device-handler via NetworkManager-dispatcher service. With this change, a generic device now represents two different device classes: - existing interfaces that are not natively supported or recognized by NetworkManager. Those devices have the `has_device_handler` property set to FALSE; - interfaces that are created by NM by invoking the device-handler; they have `has_device_handler` set to TRUE. --- src/core/devices/nm-device-factory.c | 1 + src/core/devices/nm-device-generic.c | 343 +++++++++++++++++++++- src/core/devices/nm-device-generic.h | 3 +- src/core/devices/nm-device-utils.c | 4 +- src/core/nm-dispatcher.c | 2 +- src/libnm-core-impl/nm-connection.c | 7 + src/libnm-core-public/nm-dbus-interface.h | 3 + src/libnmc-base/nm-client-utils.c | 4 +- 8 files changed, 351 insertions(+), 16 deletions(-) diff --git a/src/core/devices/nm-device-factory.c b/src/core/devices/nm-device-factory.c index c97fbb57d1..381e0d88e7 100644 --- a/src/core/devices/nm-device-factory.c +++ b/src/core/devices/nm-device-factory.c @@ -396,6 +396,7 @@ nm_device_factory_manager_load_factories(NMDeviceFactoryManagerFactoryFunc callb _ADD_INTERNAL(nm_bridge_device_factory_get_type); _ADD_INTERNAL(nm_dummy_device_factory_get_type); _ADD_INTERNAL(nm_ethernet_device_factory_get_type); + _ADD_INTERNAL(nm_generic_device_factory_get_type); _ADD_INTERNAL(nm_hsr_device_factory_get_type); _ADD_INTERNAL(nm_infiniband_device_factory_get_type); _ADD_INTERNAL(nm_ip_tunnel_device_factory_get_type); diff --git a/src/core/devices/nm-device-generic.c b/src/core/devices/nm-device-generic.c index ead671d4d7..85f6524695 100644 --- a/src/core/devices/nm-device-generic.c +++ b/src/core/devices/nm-device-generic.c @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ /* - * Copyright (C) 2013 Red Hat, Inc. + * Copyright (C) 2013-2023 Red Hat, Inc. */ #include "src/core/nm-default-daemon.h" @@ -10,13 +10,27 @@ #include "nm-device-private.h" #include "libnm-platform/nm-platform.h" #include "libnm-core-intern/nm-core-internal.h" +#include "nm-dispatcher.h" +#include "nm-device-factory.h" + +#define _NMLOG_DEVICE_TYPE NMDeviceGeneric +#include "devices/nm-device-logging.h" /*****************************************************************************/ -NM_GOBJECT_PROPERTIES_DEFINE_BASE(PROP_TYPE_DESCRIPTION, ); +NM_GOBJECT_PROPERTIES_DEFINE(NMDeviceGeneric, PROP_TYPE_DESCRIPTION, PROP_HAS_DEVICE_HANDLER, ); typedef struct { - const char *type_description; + const char *type_description; + bool prepare_done : 1; + bool has_device_handler : 1; + NMDispatcherCallId *dispatcher_call_id; + struct { + NMDeviceDeactivateCallback callback; + gpointer callback_data; + GCancellable *cancellable; + gulong cancellable_id; + } deactivate; } NMDeviceGenericPrivate; struct _NMDeviceGeneric { @@ -38,13 +52,151 @@ G_DEFINE_TYPE(NMDeviceGeneric, nm_device_generic, NM_TYPE_DEVICE) static NMDeviceCapabilities get_generic_capabilities(NMDevice *device) { - int ifindex = nm_device_get_ifindex(device); + NMDeviceGenericPrivate *priv = NM_DEVICE_GENERIC_GET_PRIVATE(device); + int ifindex = nm_device_get_ifindex(device); + NMDeviceCapabilities cap = NM_DEVICE_CAP_NONE; + + if (priv->has_device_handler) + cap |= NM_DEVICE_CAP_IS_SOFTWARE; if (ifindex > 0 && nm_platform_link_supports_carrier_detect(nm_device_get_platform(device), ifindex)) - return NM_DEVICE_CAP_CARRIER_DETECT; - else - return NM_DEVICE_CAP_NONE; + cap |= NM_DEVICE_CAP_CARRIER_DETECT; + + return cap; +} + +static void +device_add_dispatcher_cb(NMDispatcherCallId *call_id, + gpointer user_data, + gboolean success, + const char *error, + GHashTable *dict) +{ + nm_auto_unref_object NMDeviceGeneric *self = NM_DEVICE_GENERIC(user_data); + NMDeviceGenericPrivate *priv = NM_DEVICE_GENERIC_GET_PRIVATE(self); + NMDevice *device = NM_DEVICE(self); + NMPlatform *platform = nm_device_get_platform(device); + const NMPlatformLink *link; + int ifindex = -1; + const char *ifindex_str; + NMSettingConnection *s_con; + + nm_assert(call_id == priv->dispatcher_call_id); + priv->dispatcher_call_id = NULL; + + if (!success) { + _LOGW(LOGD_CORE, "device handler 'device-add' failed: %s", error); + nm_device_state_changed(device, + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_DEVICE_HANDLER_FAILED); + return; + } + + ifindex_str = g_hash_table_lookup(dict, "IFINDEX"); + if (!ifindex_str) { + _LOGW(LOGD_CORE, "device handler 'device-add' didn't return a IFINDEX key"); + nm_device_state_changed(device, + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_DEVICE_HANDLER_FAILED); + return; + } + + ifindex = _nm_utils_ascii_str_to_int64(ifindex_str, 10, 1, G_MAXINT32, -1); + if (ifindex < 0) { + _LOGW(LOGD_CORE, "device handler 'device-add' returned invalid ifindex '%s'", ifindex_str); + nm_device_state_changed(device, + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_DEVICE_HANDLER_FAILED); + return; + } + + _LOGD(LOGD_DEVICE, "device handler 'device-add' returned ifindex %d", ifindex); + + /* Check that the ifindex is valid and matches the interface name. */ + nm_platform_process_events(platform); + link = nm_platform_link_get(platform, ifindex); + if (!link) { + _LOGW(LOGD_DEVICE, + "device handler 'device-add' didn't create link with ifindex %d", + ifindex); + nm_device_state_changed(device, + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_DEVICE_HANDLER_FAILED); + return; + } + + s_con = nm_device_get_applied_setting(device, NM_TYPE_SETTING_CONNECTION); + nm_assert(s_con); + + if (!nm_streq(link->name, nm_setting_connection_get_interface_name(s_con))) { + _LOGW(LOGD_DEVICE, + "device handler 'device-add' created a kernel link with name '%s' instead of '%s'", + link->name, + nm_setting_connection_get_interface_name(s_con)); + nm_device_state_changed(device, + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_DEVICE_HANDLER_FAILED); + return; + } + + priv->prepare_done = TRUE; + nm_device_activate_schedule_stage1_device_prepare(device, FALSE); +} + +static NMActStageReturn +act_stage1_prepare(NMDevice *self, NMDeviceStateReason *out_failure_reason) +{ + NMDevice *device = NM_DEVICE(self); + NMDeviceGenericPrivate *priv = NM_DEVICE_GENERIC_GET_PRIVATE(device); + NMSettingGeneric *s_generic; + const char *type_desc; + int ifindex; + + s_generic = nm_device_get_applied_setting(device, NM_TYPE_SETTING_GENERIC); + g_return_val_if_fail(s_generic, NM_ACT_STAGE_RETURN_FAILURE); + + if (!nm_setting_generic_get_device_handler(s_generic)) + return NM_ACT_STAGE_RETURN_SUCCESS; + + if (priv->prepare_done) { + /* after we create a new interface via a device-handler, update the + * type description */ + ifindex = nm_device_get_ip_ifindex(NM_DEVICE(self)); + if (ifindex > 0) { + type_desc = nm_platform_link_get_type_name(nm_device_get_platform(device), ifindex); + if (!nm_streq0(priv->type_description, type_desc)) { + priv->type_description = type_desc; + _notify(NM_DEVICE_GENERIC(self), PROP_TYPE_DESCRIPTION); + } + } + return NM_ACT_STAGE_RETURN_SUCCESS; + } + + if (priv->dispatcher_call_id) { + nm_dispatcher_call_cancel(priv->dispatcher_call_id); + priv->dispatcher_call_id = NULL; + } + + _LOGD(LOGD_CORE, "calling device handler 'device-add'"); + if (!nm_dispatcher_call_device_handler(NM_DISPATCHER_ACTION_DEVICE_ADD, + device, + NULL, + device_add_dispatcher_cb, + g_object_ref(self), + &priv->dispatcher_call_id)) { + _LOGW(LOGD_DEVICE, "failed to call device handler 'device-add'"); + NM_SET_OUT(out_failure_reason, NM_DEVICE_STATE_REASON_DEVICE_HANDLER_FAILED); + return NM_ACT_STAGE_RETURN_FAILURE; + } + + return NM_ACT_STAGE_RETURN_POSTPONE; +} + +static void +act_stage3_ip_config(NMDevice *device, int addr_family) +{ + nm_device_devip_set_state(device, addr_family, NM_DEVICE_IP_STATE_READY, NULL); } static const char * @@ -110,6 +262,111 @@ update_connection(NMDevice *device, NMConnection *connection) NULL); } +static gboolean +create_and_realize(NMDevice *device, + NMConnection *connection, + NMDevice *parent, + const NMPlatformLink **out_plink, + GError **error) +{ + /* The actual interface is created during stage1 once the device + * starts activating, as we need to call the dispatcher service + * which returns asynchronously */ + return TRUE; +} + +static void +deactivate_clear_data(NMDeviceGeneric *self) +{ + NMDeviceGenericPrivate *priv = NM_DEVICE_GENERIC_GET_PRIVATE(self); + + if (priv->dispatcher_call_id) { + nm_dispatcher_call_cancel(priv->dispatcher_call_id); + priv->dispatcher_call_id = NULL; + } + + priv->deactivate.callback = NULL; + priv->deactivate.callback_data = NULL; + g_clear_object(&priv->deactivate.cancellable); +} + +static void +device_delete_dispatcher_cb(NMDispatcherCallId *call_id, + gpointer user_data, + gboolean success, + const char *error, + GHashTable *dict) +{ + NMDeviceGeneric *self = user_data; + NMDeviceGenericPrivate *priv = NM_DEVICE_GENERIC_GET_PRIVATE(self); + gs_free_error GError *local = NULL; + + nm_assert(call_id == priv->dispatcher_call_id); + priv->dispatcher_call_id = NULL; + + if (success) + _LOGT(LOGD_DEVICE, "deactivate: async callback"); + else { + local = g_error_new(NM_DEVICE_ERROR, + NM_DEVICE_ERROR_FAILED, + "device handler 'device-delete' failed with error: %s", + error); + } + + priv->deactivate.callback(NM_DEVICE(self), local, priv->deactivate.callback_data); + nm_clear_g_cancellable_disconnect(priv->deactivate.cancellable, + &priv->deactivate.cancellable_id); + deactivate_clear_data(self); +} + +static void +deactivate_cancellable_cancelled(GCancellable *cancellable, NMDeviceGeneric *self) +{ + NMDeviceGenericPrivate *priv = NM_DEVICE_GENERIC_GET_PRIVATE(self); + gs_free_error GError *error = NULL; + + error = nm_utils_error_new_cancelled(FALSE, NULL); + priv->deactivate.callback(NM_DEVICE(self), error, priv->deactivate.callback_data); + + deactivate_clear_data(self); +} + +static void +deactivate_async(NMDevice *device, + GCancellable *cancellable, + NMDeviceDeactivateCallback callback, + gpointer callback_user_data) +{ + NMDeviceGeneric *self = NM_DEVICE_GENERIC(device); + NMDeviceGenericPrivate *priv = NM_DEVICE_GENERIC_GET_PRIVATE(self); + + _LOGT(LOGD_CORE, "deactivate: start async"); + + priv->prepare_done = FALSE; + + if (priv->dispatcher_call_id) { + nm_dispatcher_call_cancel(priv->dispatcher_call_id); + priv->dispatcher_call_id = NULL; + } + + g_object_ref(self); + priv->deactivate.callback = callback; + priv->deactivate.callback_data = callback_user_data; + priv->deactivate.cancellable = g_object_ref(cancellable); + priv->deactivate.cancellable_id = + g_cancellable_connect(cancellable, + G_CALLBACK(deactivate_cancellable_cancelled), + self, + NULL); + + nm_dispatcher_call_device_handler(NM_DISPATCHER_ACTION_DEVICE_DELETE, + device, + NULL, + device_delete_dispatcher_cb, + self, + &priv->dispatcher_call_id); +} + /*****************************************************************************/ static void @@ -122,6 +379,26 @@ get_property(GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) case PROP_TYPE_DESCRIPTION: g_value_set_string(value, priv->type_description); break; + case PROP_HAS_DEVICE_HANDLER: + g_value_set_boolean(value, priv->has_device_handler); + break; + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); + break; + } +} + +static void +set_property(GObject *object, guint prop_id, const GValue *value, GParamSpec *pspec) +{ + NMDeviceGeneric *self = (NMDeviceGeneric *) object; + NMDeviceGenericPrivate *priv = NM_DEVICE_GENERIC_GET_PRIVATE(self); + + switch (prop_id) { + case PROP_HAS_DEVICE_HANDLER: + /* construct-only */ + priv->has_device_handler = g_value_get_boolean(value); + break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); break; @@ -137,16 +414,41 @@ nm_device_generic_init(NMDeviceGeneric *self) static GObject * constructor(GType type, guint n_construct_params, GObjectConstructParam *construct_params) { - GObject *object; + GObject *object; + NMDeviceGenericPrivate *priv; object = G_OBJECT_CLASS(nm_device_generic_parent_class) ->constructor(type, n_construct_params, construct_params); - nm_device_set_unmanaged_flags((NMDevice *) object, NM_UNMANAGED_BY_DEFAULT, TRUE); + priv = NM_DEVICE_GENERIC_GET_PRIVATE(object); + /* If the device is software (has a device-handler), don't set + * unmanaged-by-default so that the device can autoconnect if + * necessary. */ + if (!priv->has_device_handler) + nm_device_set_unmanaged_flags((NMDevice *) object, NM_UNMANAGED_BY_DEFAULT, TRUE); return object; } +static NMDevice * +create_device(NMDeviceFactory *factory, + const char *iface, + const NMPlatformLink *plink, + NMConnection *connection, + gboolean *out_ignore) +{ + return g_object_new(NM_TYPE_DEVICE_GENERIC, + NM_DEVICE_IFACE, + iface, + NM_DEVICE_TYPE_DESC, + "Generic", + NM_DEVICE_DEVICE_TYPE, + NM_DEVICE_TYPE_GENERIC, + NM_DEVICE_GENERIC_HAS_DEVICE_HANDLER, + TRUE, + NULL); +} + NMDevice * nm_device_generic_new(const NMPlatformLink *plink, gboolean nm_plugin_missing) { @@ -188,6 +490,7 @@ nm_device_generic_class_init(NMDeviceGenericClass *klass) object_class->constructor = constructor; object_class->get_property = get_property; + object_class->set_property = set_property; dbus_object_class->interface_infos = NM_DBUS_INTERFACE_INFOS(&interface_info_device_generic); @@ -195,10 +498,14 @@ nm_device_generic_class_init(NMDeviceGenericClass *klass) device_class->connection_type_check_compatible = NM_SETTING_GENERIC_SETTING_NAME; device_class->link_types = NM_DEVICE_DEFINE_LINK_TYPES(NM_LINK_TYPE_ANY); - device_class->realize_start_notify = realize_start_notify; + device_class->act_stage1_prepare = act_stage1_prepare; + device_class->act_stage3_ip_config = act_stage3_ip_config; + device_class->check_connection_compatible = check_connection_compatible; + device_class->create_and_realize = create_and_realize; + device_class->deactivate_async = deactivate_async; device_class->get_generic_capabilities = get_generic_capabilities; device_class->get_type_description = get_type_description; - device_class->check_connection_compatible = check_connection_compatible; + device_class->realize_start_notify = realize_start_notify; device_class->update_connection = update_connection; obj_properties[PROP_TYPE_DESCRIPTION] = @@ -207,6 +514,18 @@ nm_device_generic_class_init(NMDeviceGenericClass *klass) "", NULL, G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); - + obj_properties[PROP_HAS_DEVICE_HANDLER] = g_param_spec_boolean( + NM_DEVICE_GENERIC_HAS_DEVICE_HANDLER, + "", + "", + FALSE, + G_PARAM_READABLE | G_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); g_object_class_install_properties(object_class, _PROPERTY_ENUMS_LAST, obj_properties); } + +NM_DEVICE_FACTORY_DEFINE_INTERNAL( + GENERIC, + Generic, + generic, + NM_DEVICE_FACTORY_DECLARE_SETTING_TYPES(NM_SETTING_GENERIC_SETTING_NAME), + factory_class->create_device = create_device;); diff --git a/src/core/devices/nm-device-generic.h b/src/core/devices/nm-device-generic.h index f06a5bdc3f..07cb544754 100644 --- a/src/core/devices/nm-device-generic.h +++ b/src/core/devices/nm-device-generic.h @@ -18,7 +18,8 @@ #define NM_DEVICE_GENERIC_GET_CLASS(obj) \ (G_TYPE_INSTANCE_GET_CLASS((obj), NM_TYPE_DEVICE_GENERIC, NMDeviceGenericClass)) -#define NM_DEVICE_GENERIC_TYPE_DESCRIPTION "type-description" +#define NM_DEVICE_GENERIC_TYPE_DESCRIPTION "type-description" +#define NM_DEVICE_GENERIC_HAS_DEVICE_HANDLER "has-device-handler" typedef struct _NMDeviceGeneric NMDeviceGeneric; typedef struct _NMDeviceGenericClass NMDeviceGenericClass; diff --git a/src/core/devices/nm-device-utils.c b/src/core/devices/nm-device-utils.c index 2bf24ae6da..ed0a27382a 100644 --- a/src/core/devices/nm-device-utils.c +++ b/src/core/devices/nm-device-utils.c @@ -127,7 +127,9 @@ NM_UTILS_LOOKUP_STR_DEFINE( NM_UTILS_LOOKUP_STR_ITEM(NM_DEVICE_STATE_REASON_IP_METHOD_UNSUPPORTED, "ip-method-unsupported"), NM_UTILS_LOOKUP_STR_ITEM(NM_DEVICE_STATE_REASON_SRIOV_CONFIGURATION_FAILED, "sriov-configuration-failed"), - NM_UTILS_LOOKUP_STR_ITEM(NM_DEVICE_STATE_REASON_PEER_NOT_FOUND, "peer-not-found"), ); + NM_UTILS_LOOKUP_STR_ITEM(NM_DEVICE_STATE_REASON_PEER_NOT_FOUND, "peer-not-found"), + NM_UTILS_LOOKUP_STR_ITEM(NM_DEVICE_STATE_REASON_DEVICE_HANDLER_FAILED, + "device-handler-failed"), ); NM_UTILS_LOOKUP_STR_DEFINE(nm_device_mtu_source_to_string, NMDeviceMtuSource, diff --git a/src/core/nm-dispatcher.c b/src/core/nm-dispatcher.c index a5b351f3e8..4f442c685a 100644 --- a/src/core/nm-dispatcher.c +++ b/src/core/nm-dispatcher.c @@ -529,7 +529,7 @@ dispatcher_results_process(NMDispatcherAction action, NM_SET_OUT(out_success, FALSE); NM_SET_OUT(out_dict, NULL); NM_SET_OUT(out_error_msg, - err2 ? g_strdup_printf("%s: Error: %s", err, err2) : g_strdup(err)); + err2 ? g_strdup_printf("%s (Error: %s)", err, err2) : g_strdup(err)); } break; } diff --git a/src/libnm-core-impl/nm-connection.c b/src/libnm-core-impl/nm-connection.c index a23dc113c3..33360d04a1 100644 --- a/src/libnm-core-impl/nm-connection.c +++ b/src/libnm-core-impl/nm-connection.c @@ -3207,6 +3207,13 @@ nm_connection_is_virtual(NMConnection *connection) return !!nm_setting_pppoe_get_parent(s_pppoe); } + if (nm_streq(type, NM_SETTING_GENERIC_SETTING_NAME)) { + NMSettingGeneric *s_generic; + + s_generic = nm_connection_get_setting_generic(connection); + return !!nm_setting_generic_get_device_handler(s_generic); + } + return FALSE; } diff --git a/src/libnm-core-public/nm-dbus-interface.h b/src/libnm-core-public/nm-dbus-interface.h index cc87c74474..66cd590d6c 100644 --- a/src/libnm-core-public/nm-dbus-interface.h +++ b/src/libnm-core-public/nm-dbus-interface.h @@ -610,6 +610,8 @@ typedef enum { * @NM_DEVICE_STATE_REASON_IP_METHOD_UNSUPPORTED: The selected IP method is not supported * @NM_DEVICE_STATE_REASON_SRIOV_CONFIGURATION_FAILED: configuration of SR-IOV parameters failed * @NM_DEVICE_STATE_REASON_PEER_NOT_FOUND: The Wi-Fi P2P peer could not be found + * @NM_DEVICE_STATE_REASON_DEVICE_HANDLER_FAILED: The device handler dispatcher returned an + * error. Since: 1.46 * * Device state change reason codes */ @@ -682,6 +684,7 @@ typedef enum { NM_DEVICE_STATE_REASON_IP_METHOD_UNSUPPORTED = 65, NM_DEVICE_STATE_REASON_SRIOV_CONFIGURATION_FAILED = 66, NM_DEVICE_STATE_REASON_PEER_NOT_FOUND = 67, + NM_DEVICE_STATE_REASON_DEVICE_HANDLER_FAILED = 68, } NMDeviceStateReason; /** diff --git a/src/libnmc-base/nm-client-utils.c b/src/libnmc-base/nm-client-utils.c index b052a307fd..30213e41b5 100644 --- a/src/libnmc-base/nm-client-utils.c +++ b/src/libnmc-base/nm-client-utils.c @@ -464,7 +464,9 @@ NM_UTILS_LOOKUP_STR_DEFINE( NM_UTILS_LOOKUP_ITEM(NM_DEVICE_STATE_REASON_SRIOV_CONFIGURATION_FAILED, N_("Failed to configure SR-IOV parameters")), NM_UTILS_LOOKUP_ITEM(NM_DEVICE_STATE_REASON_PEER_NOT_FOUND, - N_("The Wi-Fi P2P peer could not be found")), ); + N_("The Wi-Fi P2P peer could not be found")), + NM_UTILS_LOOKUP_ITEM(NM_DEVICE_STATE_REASON_DEVICE_HANDLER_FAILED, + N_("The device handler dispatcher returned an error")), ); NM_UTILS_LOOKUP_STR_DEFINE( nm_active_connection_state_reason_to_string, From f2613be150ed10e651b4273ddddccd914cb866da Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 21 Nov 2023 14:57:17 +0100 Subject: [PATCH 14/15] core: persist state of software generic devices across restarts When a generic connection has a custom device-handler, it always generates a NMDeviceGeneric, even when the link that gets created is of a type natively supported by NM. On service restart, we need to keep track that the device is generic or otherwise a different device type will be instantiated. --- src/core/devices/nm-device-factory.c | 16 ++++++++++++++- src/core/devices/nm-device-factory.h | 2 ++ src/core/nm-config.c | 29 ++++++++++++++++++++++------ src/core/nm-config.h | 5 ++++- src/core/nm-manager.c | 13 ++++++++++--- 5 files changed, 54 insertions(+), 11 deletions(-) diff --git a/src/core/devices/nm-device-factory.c b/src/core/devices/nm-device-factory.c index 381e0d88e7..69c2a38f13 100644 --- a/src/core/devices/nm-device-factory.c +++ b/src/core/devices/nm-device-factory.c @@ -28,6 +28,10 @@ G_DEFINE_ABSTRACT_TYPE(NMDeviceFactory, nm_device_factory, G_TYPE_OBJECT) /*****************************************************************************/ +static NMDeviceFactory *generic_factory; + +/*****************************************************************************/ + static void nm_device_factory_get_supported_types(NMDeviceFactory *factory, const NMLinkType **out_link_types, @@ -66,7 +70,8 @@ nm_device_factory_create_device(NMDeviceFactory *factory, if (plink) { g_return_val_if_fail(!connection, NULL); g_return_val_if_fail(strcmp(iface, plink->name) == 0, NULL); - nm_assert(factory == nm_device_factory_manager_find_factory_for_link_type(plink->type)); + nm_assert(factory == nm_device_factory_manager_find_factory_for_link_type(plink->type) + || factory == generic_factory); } else if (connection) nm_assert(factory == nm_device_factory_manager_find_factory_for_connection(connection)); else @@ -184,6 +189,12 @@ static void __attribute__((destructor)) _cleanup(void) nm_clear_pointer(&factories_by_setting, g_hash_table_unref); } +NMDeviceFactory * +nm_device_factory_get_generic_factory(void) +{ + return generic_factory; +} + NMDeviceFactory * nm_device_factory_manager_find_factory_for_link_type(NMLinkType link_type) { @@ -300,9 +311,12 @@ _load_internal_factory(GType factory_gtype, gpointer user_data) { gs_unref_object NMDeviceFactory *factory = NULL; + GType nm_generic_device_factory_get_type(void); factory = g_object_new(factory_gtype, NULL); _add_factory(factory, NULL, callback, user_data); + if (factory_gtype == nm_generic_device_factory_get_type()) + generic_factory = factory; } static void diff --git a/src/core/devices/nm-device-factory.h b/src/core/devices/nm-device-factory.h index fc3d9dd43a..004ae9b1bf 100644 --- a/src/core/devices/nm-device-factory.h +++ b/src/core/devices/nm-device-factory.h @@ -234,4 +234,6 @@ NMDeviceFactory *nm_device_factory_manager_find_factory_for_connection(NMConnect void nm_device_factory_manager_for_each_factory(NMDeviceFactoryManagerFactoryFunc callback, gpointer user_data); +NMDeviceFactory *nm_device_factory_get_generic_factory(void); + #endif /* __NETWORKMANAGER_DEVICE_FACTORY_H__ */ diff --git a/src/core/nm-config.c b/src/core/nm-config.c index 5db4a92a4f..43eb364699 100644 --- a/src/core/nm-config.c +++ b/src/core/nm-config.c @@ -2354,9 +2354,10 @@ _nm_config_state_set(NMConfig *self, gboolean allow_persist, gboolean force_pers "route-metric-default-aspired" #define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_ROUTE_METRIC_DEFAULT_EFFECTIVE \ "route-metric-default-effective" -#define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_ROOT_PATH "root-path" -#define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_NEXT_SERVER "next-server" -#define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_DHCP_BOOTFILE "dhcp-bootfile" +#define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_ROOT_PATH "root-path" +#define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_NEXT_SERVER "next-server" +#define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_DHCP_BOOTFILE "dhcp-bootfile" +#define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_GENERIC_SOFTWARE "generic-software" static NM_UTILS_LOOKUP_STR_DEFINE( _device_state_managed_type_to_str, @@ -2457,6 +2458,12 @@ _config_device_state_data_new(int ifindex, GKeyFile *kf) device_state->route_metric_default_aspired = route_metric_default_aspired; device_state->route_metric_default_effective = route_metric_default_effective; + device_state->generic_sw = + nm_config_keyfile_get_boolean(kf, + DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE, + DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_GENERIC_SOFTWARE, + FALSE); + p = (char *) (&device_state[1]); if (connection_uuid) { memcpy(p, connection_uuid, connection_uuid_len); @@ -2502,7 +2509,7 @@ nm_config_device_state_load(int ifindex) ? ", nm-owned=1" : (device_state->nm_owned == NM_TERNARY_FALSE ? ", nm-owned=0" : ""); - _LOGT("device-state: %s #%d (%s); managed=%s%s%s%s%s%s%s%s, " + _LOGT("device-state: %s #%d (%s); managed=%s%s%s%s%s%s%s%s%s, " "route-metric-default=%" G_GUINT32_FORMAT "-%" G_GUINT32_FORMAT "", kf ? "read" : "miss", ifindex, @@ -2519,6 +2526,7 @@ nm_config_device_state_load(int ifindex) "", ""), nm_owned_str, + device_state->generic_sw ? ", generic-software" : "", device_state->route_metric_default_aspired, device_state->route_metric_default_effective); @@ -2577,7 +2585,8 @@ nm_config_device_state_write(int ifindex, guint32 route_metric_default_aspired, guint32 route_metric_default_effective, NMDhcpConfig *dhcp4_config, - NMDhcpConfig *dhcp6_config) + NMDhcpConfig *dhcp6_config, + gboolean generic_sw) { char path[NM_STRLEN(NM_CONFIG_DEVICE_STATE_DIR "/") + DEVICE_STATE_FILENAME_LEN_MAX + 1]; GError *local = NULL; @@ -2664,6 +2673,13 @@ nm_config_device_state_write(int ifindex, dhcp_bootfile); } + if (generic_sw) { + g_key_file_set_boolean(kf, + DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE, + DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_GENERIC_SOFTWARE, + TRUE); + } + for (IS_IPv4 = 1; IS_IPv4 >= 0; IS_IPv4--) { NMDhcpConfig *dhcp_config = IS_IPv4 ? dhcp4_config : dhcp6_config; gs_free NMUtilsNamedValue *values = NULL; @@ -2691,7 +2707,7 @@ nm_config_device_state_write(int ifindex, g_error_free(local); return FALSE; } - _LOGT("device-state: write #%d (%s); managed=%s%s%s%s%s%s%s, " + _LOGT("device-state: write #%d (%s); managed=%s%s%s%s%s%s%s%s, " "route-metric-default=%" G_GUINT32_FORMAT "-%" G_GUINT32_FORMAT "%s%s%s" "%s%s%s" "%s%s%s", @@ -2700,6 +2716,7 @@ nm_config_device_state_write(int ifindex, _device_state_managed_type_to_str(managed), NM_PRINT_FMT_QUOTED(connection_uuid, ", connection-uuid=", connection_uuid, "", ""), NM_PRINT_FMT_QUOTED(perm_hw_addr_fake, ", perm-hw-addr-fake=", perm_hw_addr_fake, "", ""), + generic_sw ? ", generic-software" : "", route_metric_default_aspired, route_metric_default_effective, NM_PRINT_FMT_QUOTED(next_server, ", next-server=", next_server, "", ""), diff --git a/src/core/nm-config.h b/src/core/nm-config.h index acec8d05cd..e65582c34a 100644 --- a/src/core/nm-config.h +++ b/src/core/nm-config.h @@ -176,6 +176,8 @@ struct _NMConfigDeviceStateData { /* whether the device was nm-owned (0/1) or -1 for * non-software devices. */ NMTernary nm_owned : 3; + /* whether the device is a generic one created by NM */ + bool generic_sw : 1; }; NMConfigDeviceStateData *nm_config_device_state_load(int ifindex); @@ -188,7 +190,8 @@ gboolean nm_config_device_state_write(int guint32 route_metric_default_aspired, guint32 route_metric_default_effective, NMDhcpConfig *dhcp4_config, - NMDhcpConfig *dhcp6_config); + NMDhcpConfig *dhcp6_config, + gboolean generic); void nm_config_device_state_prune_stale(GHashTable *preserve_ifindexes, NMPlatform *preserve_in_platform); diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index 2cf9cb1ddf..9284ae8c43 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -4213,8 +4213,12 @@ platform_link_added(NMManager *self, } add: - /* Try registered device factories */ - factory = nm_device_factory_manager_find_factory_for_link_type(plink->type); + if (dev_state && dev_state->generic_sw) { + factory = nm_device_factory_get_generic_factory(); + } else { + /* Try registered device factories */ + factory = nm_device_factory_manager_find_factory_for_link_type(plink->type); + } if (factory) { gboolean ignore = FALSE; gs_free_error GError *error = NULL; @@ -7860,7 +7864,10 @@ nm_manager_write_device_state(NMManager *self, NMDevice *device, int *out_ifinde route_metric_default_aspired, route_metric_default_effective, nm_device_get_dhcp_config(device, AF_INET), - nm_device_get_dhcp_config(device, AF_INET6))) + nm_device_get_dhcp_config(device, AF_INET6), + nm_device_is_software(device) + && nm_device_get_device_type(device) + == NM_DEVICE_TYPE_GENERIC)) return FALSE; NM_SET_OUT(out_ifindex, ifindex); From 5978fb2b27cdda8f119735bf51bee1942f6921f7 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 2 Oct 2023 15:19:48 +0200 Subject: [PATCH 15/15] manager: make generic devices compatible with all link types If a generic device is present and the name matches, it is compatible with any link type. For example, if a generic connection has a device-handler that creates a dummy interface, the link is compatible with the NMDeviceGeneric. --- src/core/nm-manager.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index 9284ae8c43..730ba4763b 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -4168,8 +4168,11 @@ platform_link_added(NMManager *self, gboolean compatible = TRUE; gs_free_error GError *error = NULL; - if (nm_device_get_link_type(candidate) != plink->type) + if (nm_device_get_device_type(candidate) == NM_DEVICE_TYPE_GENERIC) { + /* generic devices are compatible with all link types */ + } else if (nm_device_get_link_type(candidate) != plink->type) { continue; + } if (!nm_streq(nm_device_get_iface(candidate), plink->name)) continue;