rpms/NetworkManager
5
0
Fork 0
Code Issues Pull Requests Releases Activity

Fix state handling in the dnsconfd DNS plugin (RHEL-79693)

Browse Source 
Resolves: RHEL-79693
This commit is contained in:
Beniamino Galvani 2025-02-17 15:20:22 +01:00
parent 6880bc7ae3
commit 1a67b223a3
3 changed files with 217 additions and 216 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

213
1001-checkpoint-fix-port-reactivation-when-controller-is-.patch
View File

@ -1,213 +0,0 @@
From 4726822fb05d846ee402ce5e2c139dfb6c5a8ca3 Mon Sep 17 00:00:00 2001
From: Gris Ge <fge@redhat.com>
Date: Tue, 14 May 2024 11:39:21 +0800
Subject: [PATCH] checkpoint: fix port reactivation when controller is
deactivating
Problem:
Given a OVS port with `autoconnect-ports` set to default or false,
when reactivation required for checkpoint rollback,
previous activated OVS interface will be in deactivate state after
checkpoint rollback.
The root cause:
The `activate_stage1_device_prepare()` will mark the device as
failed when controller is deactivating or deactivated.
In `activate_stage1_device_prepare()`, the controller device is
retrieved from NMActiveConnection, it will be NULL when NMActiveConnection
is in deactivated state. This will cause device been set to
`NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED` which prevent all follow
up `autoconnect` actions.
Fix:
When noticing controller is deactivating or deactivated with reason
`NM_DEVICE_STATE_REASON_NEW_ACTIVATION`, use new function
`nm_active_connection_set_controller_dev()` to wait on controller
device state between NM_DEVICE_STATE_PREPARE and
NM_DEVICE_STATE_ACTIVATED. After that, use existing
`nm_active_connection_set_controller()` to use new
NMActiveConnection of controller to move on.
Resolves: https://issues.redhat.com/browse/RHEL-31972
Signed-off-by: Gris Ge <fge@redhat.com>
(cherry picked from commit a68d2fd7807b8c6f976c45ffa29835b7f8f470ca)
---
src/core/devices/nm-device.c | 8 +++++
src/core/devices/nm-device.h | 3 +-
src/core/nm-active-connection.c | 62 +++++++++++++++++++++++++++++++++
src/core/nm-active-connection.h | 1 +
src/core/nm-manager.c | 15 +++++++-
5 files changed, 87 insertions(+), 2 deletions(-)
diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c
index b96adefbd0..f3441508ab 100644
--- a/src/core/devices/nm-device.c
+++ b/src/core/devices/nm-device.c
@@ -17205,6 +17205,14 @@ nm_device_get_state(NMDevice *self)
return NM_DEVICE_GET_PRIVATE(self)->state;
}
+NMDeviceStateReason
+nm_device_get_state_reason(NMDevice *self)
+{
+ g_return_val_if_fail(NM_IS_DEVICE(self), NM_DEVICE_STATE_REASON_NONE);
+
+ return NM_DEVICE_GET_PRIVATE(self)->state_reason;
+}
+
/*****************************************************************************/
/**
diff --git a/src/core/devices/nm-device.h b/src/core/devices/nm-device.h
index ffe6b1af99..ba45497ce2 100644
--- a/src/core/devices/nm-device.h
+++ b/src/core/devices/nm-device.h
@@ -561,7 +561,8 @@ int nm_device_spec_match_list_full(NMDevice *self, const GSList *specs, int
gboolean nm_device_is_activating(NMDevice *dev);
gboolean nm_device_autoconnect_allowed(NMDevice *self);
-NMDeviceState nm_device_get_state(NMDevice *device);
+NMDeviceState nm_device_get_state(NMDevice *device);
+NMDeviceStateReason nm_device_get_state_reason(NMDevice *device);
gboolean nm_device_get_enabled(NMDevice *device);
diff --git a/src/core/nm-active-connection.c b/src/core/nm-active-connection.c
index b08d26c28b..7d89251caa 100644
--- a/src/core/nm-active-connection.c
+++ b/src/core/nm-active-connection.c
@@ -50,6 +50,7 @@ typedef struct _NMActiveConnectionPrivate {
NMAuthSubject *subject;
NMActiveConnection *controller;
+ NMDevice *controller_dev;
NMActiveConnection *parent;
@@ -826,6 +827,31 @@ master_state_cb(NMActiveConnection *master, GParamSpec *pspec, gpointer user_dat
}
}
+static void
+controller_dev_state_cb(NMDevice *controller_dev,
+ NMDeviceState new_state,
+ NMDeviceState old_state,
+ NMDeviceStateReason reason,
+ gpointer user_data)
+{
+ NMActiveConnection *self = NM_ACTIVE_CONNECTION(user_data);
+ NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE(self);
+ NMActRequest *controller_act_request;
+ NMActiveConnection *controller_ac;
+
+ if (new_state >= NM_DEVICE_STATE_PREPARE && new_state <= NM_DEVICE_STATE_ACTIVATED) {
+ controller_act_request = nm_device_get_act_request(controller_dev);
+ if (controller_act_request) {
+ controller_ac = NM_ACTIVE_CONNECTION(controller_act_request);
+ g_signal_handlers_disconnect_by_func(controller_dev,
+ G_CALLBACK(controller_dev_state_cb),
+ self);
+ g_clear_object(&priv->controller_dev);
+ nm_active_connection_set_controller(self, controller_ac);
+ }
+ }
+}
+
/**
* nm_active_connection_set_controller:
* @self: the #NMActiveConnection
@@ -867,6 +893,36 @@ nm_active_connection_set_controller(NMActiveConnection *self, NMActiveConnection
check_controller_ready(self);
}
+void
+nm_active_connection_set_controller_dev(NMActiveConnection *self, NMDevice *controller_dev)
+{
+ NMActiveConnectionPrivate *priv;
+
+ g_return_if_fail(NM_IS_ACTIVE_CONNECTION(self));
+ g_return_if_fail(NM_IS_DEVICE(controller_dev));
+
+ priv = NM_ACTIVE_CONNECTION_GET_PRIVATE(self);
+
+ /* Controller device is write-once, and must be set before exporting the object */
+ g_return_if_fail(priv->controller_dev == NULL);
+ g_return_if_fail(!nm_dbus_object_is_exported(NM_DBUS_OBJECT(self)));
+ if (priv->device) {
+ g_return_if_fail(priv->device != controller_dev);
+ }
+
+ _LOGD("set controller device %p, %s(%s), state %s",
+ controller_dev,
+ nm_device_get_iface(controller_dev),
+ nm_device_get_type_desc(controller_dev),
+ nm_device_state_to_string(nm_device_get_state(controller_dev)));
+
+ priv->controller_dev = g_object_ref(controller_dev);
+ g_signal_connect(priv->controller_dev,
+ NM_DEVICE_STATE_CHANGED,
+ G_CALLBACK(controller_dev_state_cb),
+ self);
+}
+
NMActivationType
nm_active_connection_get_activation_type(NMActiveConnection *self)
{
@@ -1533,7 +1589,13 @@ dispose(GObject *object)
if (priv->controller) {
g_signal_handlers_disconnect_by_func(priv->controller, G_CALLBACK(master_state_cb), self);
}
+ if (priv->controller_dev) {
+ g_signal_handlers_disconnect_by_func(priv->controller_dev,
+ G_CALLBACK(controller_dev_state_cb),
+ self);
+ }
g_clear_object(&priv->controller);
+ g_clear_object(&priv->controller_dev);
if (priv->parent)
unwatch_parent(self, TRUE);
diff --git a/src/core/nm-active-connection.h b/src/core/nm-active-connection.h
index 12cb311c97..ba32830257 100644
--- a/src/core/nm-active-connection.h
+++ b/src/core/nm-active-connection.h
@@ -175,6 +175,7 @@ NMActiveConnection *nm_active_connection_get_controller(NMActiveConnection *self
gboolean nm_active_connection_get_controller_ready(NMActiveConnection *self);
void nm_active_connection_set_controller(NMActiveConnection *self, NMActiveConnection *controller);
+void nm_active_connection_set_controller_dev(NMActiveConnection *self, NMDevice *controller_dev);
void nm_active_connection_set_parent(NMActiveConnection *self, NMActiveConnection *parent);
diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c
index b2a827e38b..2c2834e9fc 100644
--- a/src/core/nm-manager.c
+++ b/src/core/nm-manager.c
@@ -5943,7 +5943,20 @@ _internal_activate_device(NMManager *self, NMActiveConnection *active, GError **
NM_DEVICE_STATE_REASON_USER_REQUESTED);
}
- nm_active_connection_set_controller(active, master_ac);
+ /* If controller NMActiveConnection is deactivating, we should wait on
+ * controller's NMDevice to have new NMActiveConnection after
+ * controller device state change to between NM_DEVICE_STATE_PREPARE and
+ * NM_DEVICE_STATE_ACTIVATED.
+ */
+ if ((nm_active_connection_get_state(master_ac) >= NM_ACTIVE_CONNECTION_STATE_DEACTIVATING)
+ && master_device
+ && (nm_device_get_state_reason(master_device)
+ == NM_DEVICE_STATE_REASON_NEW_ACTIVATION)) {
+ nm_active_connection_set_controller_dev(active, master_device);
+ } else {
+ nm_active_connection_set_controller(active, master_ac);
+ }
+
_LOGD(LOGD_CORE,
"Activation of '%s' depends on active connection %p %s",
nm_settings_connection_get_id(sett_conn),
--
2.45.1

210
1001-dnsconfd-fixes-rhel-79693.patch Normal file
View File

@ -0,0 +1,210 @@
From 736c6c7930c74c653c61379d7258cc1d93973e0a Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
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.
(cherry picked from commit 8ff1cbf38b53950a6f12239326a1f41eca573915)
(cherry picked from commit dc0ff10efb2b9ea370fbe98d0a5720e9e9f15173)
---
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 b356c2f9ca..38b7558a12 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;
}
--
2.48.1
From 3784576e6f967b1c70c7364fe55ee0b73a06dd8b Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
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.
(cherry picked from commit 2bfd27f74d869a529a14e478019399075ca37d03)
(cherry picked from commit e20794989b0bea80a120888a34d4d9915cbd8529)
---
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 38b7558a12..b4c935db62 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 {
--
2.48.1
From cc90aa81383d816ff9bb2dcd2080868787d3e776 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
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')
(cherry picked from commit 4d84e6cddf38754e04ca112f79b2e8937f60b01e)
(cherry picked from commit 5e18da31a45971a11fa0c12978f9685cca429862)
---
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 b4c935db62..ea23760634 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);
}
}
}
--
2.48.1

10
NetworkManager.spec
View File

@ -7,7 +7,7 @@
%global real_version 1.51.90
%global git_tag_version_suffix -dev
%global rpm_version %{real_version}
%global release_version 1
%global release_version 2
%global snapshot %{nil}
%global git_sha %{nil}
%global bcond_default_debug 0
@ -189,6 +189,7 @@ Source9: readme-ifcfg-rh-migrated.txt
# Bugfixes that are only relevant until next rebase of the package.
# Patch1001: 1001-some.patch
Patch1001: 1001-dnsconfd-fixes-rhel-79693.patch
Requires(post): systemd
Requires(post): systemd-udev
@ -1080,10 +1081,13 @@ fi
%changelog
* Wed Feb 12 2025 Filip Pokryvka <fpokryvk@redhat.com> - 1:1:51.90-1
* Mon Feb 17 2025 Beniamino Galvani <bgalvani@redhat.com> - 1:1.51.90-2
- Fix state handling in the dnsconfd DNS plugin (RHEL-79693)
* Wed Feb 12 2025 Filip Pokryvka <fpokryvk@redhat.com> - 1:1.51.90-1
- Fix balance-slb ports rarely not being active after reboot (RHEL-77167)
* Mon Feb 10 2025 Íñigo Huguet <ihuguet@redhat.com> - 1:1:51.7-2
* Mon Feb 10 2025 Íñigo Huguet <ihuguet@redhat.com> - 1:1.51.7-2
- Remove the build argument -flto-partition to avoid compilation crashes due
to high memory usage.