From 8ff1cbf38b53950a6f12239326a1f41eca573915 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 12 Feb 2025 20:54:51 +0100 Subject: [PATCH 1/3] dnsconfd: fix handling of the update-pending flag After every state change of the plugin there should be an invocation of _nm_dns_plugin_update_pending_maybe_changed() to re-evaluate whether we are waiting for an update. send_dnsconfd_update() doesn't change the state and so there is need to check again afterwards. --- src/core/dns/nm-dns-dnsconfd.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/core/dns/nm-dns-dnsconfd.c b/src/core/dns/nm-dns-dnsconfd.c index b356c2f9c..38b7558a1 100644 --- a/src/core/dns/nm-dns-dnsconfd.c +++ b/src/core/dns/nm-dns-dnsconfd.c @@ -61,7 +61,11 @@ G_DEFINE_TYPE(NMDnsDnsconfd, nm_dns_dnsconfd, NM_TYPE_DNS_PLUGIN) #define DNSCONFD_DBUS_SERVICE "com.redhat.dnsconfd" -typedef enum { CONNECTION_FAIL, CONNECTION_SUCCESS, CONNECTION_WAIT } ConnectionState; +typedef enum { + CONNECTION_FAIL, + CONNECTION_SUCCESS, + CONNECTION_WAIT, +} ConnectionState; /*****************************************************************************/ @@ -704,6 +708,8 @@ update(NMDnsPlugin *plugin, _LOGT("connected, waiting for update to finish"); } + _nm_dns_plugin_update_pending_maybe_changed(plugin); + if (all_connected == CONNECTION_FAIL) { nm_utils_error_set(error, NM_UTILS_ERROR_UNKNOWN, @@ -717,8 +723,6 @@ update(NMDnsPlugin *plugin, send_dnsconfd_update(self); - _nm_dns_plugin_update_pending_maybe_changed(NM_DNS_PLUGIN(self)); - return TRUE; } From 2bfd27f74d869a529a14e478019399075ca37d03 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 12 Feb 2025 20:58:27 +0100 Subject: [PATCH 2/3] dnsconfd: set the state to idle when connection fails If the plugin can't connect to D-Bus, it is not waiting for an update; set the state to idle. --- src/core/dns/nm-dns-dnsconfd.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/dns/nm-dns-dnsconfd.c b/src/core/dns/nm-dns-dnsconfd.c index 38b7558a1..b4c935db6 100644 --- a/src/core/dns/nm-dns-dnsconfd.c +++ b/src/core/dns/nm-dns-dnsconfd.c @@ -700,7 +700,10 @@ update(NMDnsPlugin *plugin, /* We need to consider only whether we are connected, because newer update call * overrides the old one */ - if (all_connected != CONNECTION_SUCCESS) { + if (all_connected == CONNECTION_FAIL) { + priv->plugin_state = DNSCONFD_PLUGIN_IDLE; + _LOGT("failed to connect"); + } else if (all_connected == CONNECTION_WAIT) { priv->plugin_state = DNSCONFD_PLUGIN_WAIT_CONNECT; _LOGT("not connected, waiting to connect"); } else { From 4d84e6cddf38754e04ca112f79b2e8937f60b01e Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 13 Feb 2025 09:54:21 +0100 Subject: [PATCH 3/3] dnsconfd: drop "connection-*" entries from the update method Stop passing "connection-*" entries in the update method to dnsconfd. The plugin tries to determine the connection from the ifindex, but it's not possible to do it right at the moment because the same ifindex can be used at the same time e.g. by a policy-based VPN like ipsec and a normal device. Instead, it should be NM that explicitly passes the information about the connection to the DNS plugin. Anyway, these variables are not used at the moment by dnsconfd. Fixes: c6e1925decd8 ('dns: Add dnsconfd DNS plugin') --- src/core/dns/nm-dns-dnsconfd.c | 65 +++++----------------------------- 1 file changed, 8 insertions(+), 57 deletions(-) diff --git a/src/core/dns/nm-dns-dnsconfd.c b/src/core/dns/nm-dns-dnsconfd.c index b4c935db6..ea2376063 100644 --- a/src/core/dns/nm-dns-dnsconfd.c +++ b/src/core/dns/nm-dns-dnsconfd.c @@ -333,29 +333,8 @@ get_networks(NMDnsConfigIPData *ip_data, char ***networks) static void server_builder_append_interface_info(GVariantBuilder *argument_builder, const char *interface, - char **networks, - const char *connection_id, - const char *connection_uuid, - const char *dbus_path) + char **networks) { - if (connection_id) { - g_variant_builder_add(argument_builder, - "{sv}", - "connection-id", - g_variant_new("s", connection_id)); - } - if (connection_uuid) { - g_variant_builder_add(argument_builder, - "{sv}", - "connection-uuid", - g_variant_new("s", connection_uuid)); - } - if (dbus_path) { - g_variant_builder_add(argument_builder, - "{sv}", - "connection-object", - g_variant_new("s", dbus_path)); - } if (interface) { g_variant_builder_add(argument_builder, "{sv}", "interface", g_variant_new("s", interface)); } @@ -593,18 +572,11 @@ parse_all_interface_config(GVariantBuilder *argument_builder, const CList *ip_data_lst_head, const char *ca) { - NMDnsConfigIPData *ip_data; - const char *const *dns_server_strings; - guint nameserver_count; - const char *ifname; - NMDevice *device; - NMActiveConnection *active_connection; - NMSettingsConnection *settings_connection; - NMActRequest *act_request; - const char *connection_id; - const char *connection_uuid; - const char *dbus_path; - gboolean explicit_default = is_default_interface_explicit(ip_data_lst_head); + NMDnsConfigIPData *ip_data; + const char *const *dns_server_strings; + guint nameserver_count; + const char *ifname; + gboolean explicit_default = is_default_interface_explicit(ip_data_lst_head); c_list_for_each_entry (ip_data, ip_data_lst_head, ip_data_lst) { /* No need to free insides of routing and search domains, as they point to data @@ -619,23 +591,7 @@ parse_all_interface_config(GVariantBuilder *argument_builder, &nameserver_count); if (!nameserver_count) continue; - ifname = nm_platform_link_get_name(NM_PLATFORM_GET, ip_data->data->ifindex); - device = nm_manager_get_device_by_ifindex(NM_MANAGER_GET, ip_data->data->ifindex); - act_request = nm_device_get_act_request(device); - active_connection = NM_ACTIVE_CONNECTION(act_request); - - /* Presume that when we have server of this interface then the interface has to have - * an active connection */ - nm_assert(active_connection); - - settings_connection = nm_active_connection_get_settings_connection(active_connection); - connection_id = nm_settings_connection_get_id(settings_connection); - connection_uuid = nm_settings_connection_get_uuid(settings_connection); - dbus_path = nm_dbus_object_get_path_still_exported(NM_DBUS_OBJECT(act_request)); - - /* dbus_path also should be set, because if we are parsing this connection then we - * expect it to be active and exported on dbus */ - nm_assert(dbus_path && dbus_path[0] != 0); + ifname = nm_platform_link_get_name(NM_PLATFORM_GET, ip_data->data->ifindex); gather_interface_domains(ip_data, explicit_default, &routing_domains, &search_domains); get_networks(ip_data, &networks); @@ -647,12 +603,7 @@ parse_all_interface_config(GVariantBuilder *argument_builder, routing_domains, search_domains, ca)) { - server_builder_append_interface_info(argument_builder, - ifname, - networks, - connection_id, - connection_uuid, - dbus_path); + server_builder_append_interface_info(argument_builder, ifname, networks); } } }