merge: branch 'bg/hostname-skip-ipv6-tentative'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1536

(cherry picked from commit 34417ac3e637020f9c1f51906a23491a9e14be31)

5 files changed, 120 insertions, 50 deletions

diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c
index 040bc276ab..4a9c43eac6 100644
--- a/src/core/devices/nm-device.c
+++ b/src/core/devices/nm-device.c
@@ -312,7 +312,7 @@ typedef struct {
 
 typedef enum {
     RESOLVER_WAIT_ADDRESS = 0,
-    RESOLVER_IN_PROGRESS,
+    RESOLVER_STARTED,
     RESOLVER_DONE,
 } ResolverState;
 
@@ -17107,6 +17107,21 @@ nm_device_auth_retries_try_next(NMDevice *self)
     return TRUE;
 }
 
+static const char *
+_resolver_state_to_string(ResolverState state)
+{
+    switch (state) {
+    case RESOLVER_WAIT_ADDRESS:
+        return "WAIT-ADDRESS";
+    case RESOLVER_STARTED:
+        return "STARTED";
+    case RESOLVER_DONE:
+        return "DONE";
+    }
+    nm_assert_not_reached();
+    return "UNKNOWN";
+}
+
 static void
 hostname_dns_lookup_callback(GObject *source, GAsyncResult *result, gpointer user_data)
 {
@@ -17126,7 +17141,9 @@ hostname_dns_lookup_callback(GObject *source, GAsyncResult *result, gpointer use
 
     if (error) {
         _LOGD(LOGD_DNS,
-              "hostname-from-dns: lookup error for %s: %s",
+              "hostname-from-dns: ipv%c resolver %s: lookup error for %s: %s",
+              nm_utils_addr_family_to_char(resolver->addr_family),
+              _resolver_state_to_string(RESOLVER_DONE),
               (addr_str = g_inet_address_to_string(resolver->address)),
               error->message);
     } else {
@@ -17136,7 +17153,9 @@ hostname_dns_lookup_callback(GObject *source, GAsyncResult *result, gpointer use
         valid              = nm_utils_validate_hostname(resolver->hostname);
 
         _LOGD(LOGD_DNS,
-              "hostname-from-dns: lookup done for %s, result %s%s%s%s",
+              "hostname-from-dns: ipv%c resolver %s: lookup successful for %s, result %s%s%s%s",
+              nm_utils_addr_family_to_char(resolver->addr_family),
+              _resolver_state_to_string(RESOLVER_DONE),
               (addr_str = g_inet_address_to_string(resolver->address)),
               NM_PRINT_FMT_QUOTE_STRING(resolver->hostname),
               valid ? "" : " (invalid)");
@@ -17162,8 +17181,9 @@ hostname_dns_address_timeout(gpointer user_data)
     nm_assert(!resolver->cancellable);
 
     _LOGT(LOGD_DNS,
-          "hostname-from-dns: timed out while waiting IPv%c address",
-          nm_utils_addr_family_to_char(resolver->addr_family));
+          "hostname-from-dns: ipv%c state %s: timed out while waiting for address",
+          nm_utils_addr_family_to_char(resolver->addr_family),
+          _resolver_state_to_string(RESOLVER_DONE));
 
     resolver->timeout_id = 0;
     resolver->state      = RESOLVER_DONE;
@@ -17172,30 +17192,16 @@ hostname_dns_address_timeout(gpointer user_data)
     return G_SOURCE_REMOVE;
 }
 
-static const char *
-_resolver_state_to_string(ResolverState state)
-{
-    switch (state) {
-    case RESOLVER_WAIT_ADDRESS:
-        return "wait-address";
-    case RESOLVER_IN_PROGRESS:
-        return "in-progress";
-    case RESOLVER_DONE:
-        return "done";
-    default:
-        nm_assert_not_reached();
-        return "unknown";
-    }
-}
-
 void
-nm_device_clear_dns_lookup_data(NMDevice *self)
+nm_device_clear_dns_lookup_data(NMDevice *self, const char *reason)
 {
     NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self);
-    guint            i;
 
-    for (i = 0; i < 2; i++)
-        nm_clear_pointer(&priv->hostname_resolver_x[i], _hostname_resolver_free);
+    if (priv->hostname_resolver_4 || priv->hostname_resolver_6) {
+        _LOGT(LOGD_DNS, "hostname-from-dns: resetting (%s)", reason);
+        nm_clear_pointer(&priv->hostname_resolver_4, _hostname_resolver_free);
+        nm_clear_pointer(&priv->hostname_resolver_6, _hostname_resolver_free);
+    }
 }
 
 gboolean
@@ -17236,6 +17242,9 @@ get_address_for_hostname_dns_lookup(NMDevice *self, int addr_family)
                 return g_inet_address_new_from_bytes(addr->ax.address_ptr, G_SOCKET_FAMILY_IPV4);
             }
 
+            if (addr->ax.n_ifa_flags & IFA_F_TENTATIVE)
+                continue;
+
             /* For IPv6 prefer, in order:
              * - !link-local, !deprecated
              * - !link-local, deprecated
@@ -17329,29 +17338,35 @@ nm_device_get_hostname_from_dns_lookup(NMDevice *self, int addr_family, gboolean
     } else if (new_address != resolver->address)
         address_changed = TRUE;
 
+    if (address_changed) {
+        /* set new state before logging */
+        if (new_address)
+            resolver->state = RESOLVER_STARTED;
+        else
+            resolver->state = RESOLVER_WAIT_ADDRESS;
+    }
+
     {
         gs_free char *old_str = NULL;
         gs_free char *new_str = NULL;
 
-        _LOGT(LOGD_DNS,
-              "hostname-from-dns: ipv%c resolver state %s, old address %s, new address %s",
-              nm_utils_addr_family_to_char(resolver->addr_family),
-              _resolver_state_to_string(resolver->state),
-              resolver->address ? (old_str = g_inet_address_to_string(resolver->address))
-                                : "(null)",
-              new_address ? (new_str = g_inet_address_to_string(new_address)) : "(null)");
+        if (address_changed) {
+            _LOGT(LOGD_DNS,
+                  "hostname-from-dns: ipv%c resolver %s, address changed from %s to %s",
+                  nm_utils_addr_family_to_char(resolver->addr_family),
+                  _resolver_state_to_string(resolver->state),
+                  resolver->address ? (old_str = g_inet_address_to_string(resolver->address))
+                                    : "(null)",
+                  new_address ? (new_str = g_inet_address_to_string(new_address)) : "(null)");
+        }
     }
 
-    /* In every state, if the address changed, we restart
-     * the resolution with the new address */
     if (address_changed) {
         nm_clear_g_cancellable(&resolver->cancellable);
         g_clear_object(&resolver->address);
-        resolver->state = RESOLVER_WAIT_ADDRESS;
     }
 
     if (address_changed && new_address) {
-        resolver->state       = RESOLVER_IN_PROGRESS;
         resolver->cancellable = g_cancellable_new();
         resolver->address     = g_steal_pointer(&new_address);
 
@@ -17369,7 +17384,7 @@ nm_device_get_hostname_from_dns_lookup(NMDevice *self, int addr_family, gboolean
             resolver->timeout_id = g_timeout_add(30000, hostname_dns_address_timeout, resolver);
         NM_SET_OUT(out_wait, TRUE);
         return NULL;
-    case RESOLVER_IN_PROGRESS:
+    case RESOLVER_STARTED:
         NM_SET_OUT(out_wait, TRUE);
         return NULL;
     case RESOLVER_DONE:
diff --git a/src/core/devices/nm-device.h b/src/core/devices/nm-device.h
index f54457d12e..bcf4d7b991 100644
--- a/src/core/devices/nm-device.h
+++ b/src/core/devices/nm-device.h
@@ -817,7 +817,7 @@ gboolean nm_device_is_vpn(NMDevice *self);
 const char *
 nm_device_get_hostname_from_dns_lookup(NMDevice *self, int addr_family, gboolean *out_pending);
 
-void nm_device_clear_dns_lookup_data(NMDevice *self);
+void nm_device_clear_dns_lookup_data(NMDevice *self, const char *reason);
 
 gboolean nm_device_get_allow_autoconnect_on_external(NMDevice *self);
 
diff --git a/src/core/nm-core-utils.c b/src/core/nm-core-utils.c
index 9448ba7b03..500bffb98d 100644
--- a/src/core/nm-core-utils.c
+++ b/src/core/nm-core-utils.c
@@ -4868,11 +4868,14 @@ typedef struct {
 
     int      child_stdin;
     int      child_stdout;
+    int      child_stderr;
     GSource *input_source;
     GSource *output_source;
+    GSource *error_source;
 
     NMStrBuf in_buffer;
     NMStrBuf out_buffer;
+    NMStrBuf err_buffer;
     gsize    out_buffer_offset;
 } HelperInfo;
 
@@ -4908,13 +4911,17 @@ helper_info_free(gpointer data)
 
     nm_str_buf_destroy(&info->in_buffer);
     nm_str_buf_destroy(&info->out_buffer);
+    nm_str_buf_destroy(&info->err_buffer);
     nm_clear_g_source_inst(&info->input_source);
     nm_clear_g_source_inst(&info->output_source);
+    nm_clear_g_source_inst(&info->error_source);
 
     if (info->child_stdout != -1)
         nm_close(info->child_stdout);
     if (info->child_stdin != -1)
         nm_close(info->child_stdin);
+    if (info->child_stderr != -1)
+        nm_close(info->child_stderr);
 
     if (info->pid != -1) {
         nm_assert(info->pid > 1);
@@ -4928,6 +4935,10 @@ static void
 helper_complete(HelperInfo *info, GError *error)
 {
     if (error) {
+        if (info->err_buffer.len > 0) {
+            _LOG2T(info, "stderr: %s", nm_str_buf_get_str(&info->err_buffer));
+        }
+
         nm_clear_g_cancellable_disconnect(g_task_get_cancellable(info->task),
                                           &info->cancellable_id);
         g_task_return_error(info->task, error);
@@ -5023,6 +5034,24 @@ helper_have_data(int fd, GIOCondition condition, gpointer user_data)
     return G_SOURCE_CONTINUE;
 }
 
+static gboolean
+helper_have_err_data(int fd, GIOCondition condition, gpointer user_data)
+{
+    HelperInfo *info = user_data;
+    gssize      n_read;
+
+    n_read = nm_utils_fd_read(fd, &info->err_buffer);
+
+    if (n_read > 0)
+        return G_SOURCE_CONTINUE;
+
+    nm_clear_g_source_inst(&info->error_source);
+    nm_close(info->child_stderr);
+    info->child_stderr = -1;
+
+    return G_SOURCE_CONTINUE;
+}
+
 static void
 helper_child_terminated(GPid pid, int status, gpointer user_data)
 {
@@ -5098,10 +5127,11 @@ nm_utils_spawn_helper(const char *const  *args,
                                   &info->pid,
                                   &info->child_stdin,
                                   &info->child_stdout,
-                                  NULL,
+                                  &info->child_stderr,
                                   &error)) {
         info->child_stdin  = -1;
         info->child_stdout = -1;
+        info->child_stderr = -1;
         info->pid          = -1;
         g_task_return_error(info->task,
                             g_error_new(NM_UTILS_ERROR,
@@ -5130,9 +5160,11 @@ nm_utils_spawn_helper(const char *const  *args,
     fcntl(info->child_stdin, F_SETFL, fd_flags | O_NONBLOCK);
     fd_flags = fcntl(info->child_stdout, F_GETFD, 0);
     fcntl(info->child_stdout, F_SETFL, fd_flags | O_NONBLOCK);
+    fd_flags = fcntl(info->child_stderr, F_GETFD, 0);
+    fcntl(info->child_stderr, F_SETFL, fd_flags | O_NONBLOCK);
 
     /* Watch process stdin */
-    info->out_buffer = NM_STR_BUF_INIT(32, TRUE);
+    info->out_buffer = NM_STR_BUF_INIT(NM_UTILS_GET_NEXT_REALLOC_SIZE_40, TRUE);
     for (arg = args; *arg; arg++) {
         nm_str_buf_append(&info->out_buffer, *arg);
         nm_str_buf_append_c(&info->out_buffer, '\0');
@@ -5146,7 +5178,7 @@ nm_utils_spawn_helper(const char *const  *args,
     g_source_attach(info->output_source, g_main_context_get_thread_default());
 
     /* Watch process stdout */
-    info->in_buffer    = NM_STR_BUF_INIT(NM_UTILS_GET_NEXT_REALLOC_SIZE_1000, FALSE);
+    info->in_buffer    = NM_STR_BUF_INIT(0, FALSE);
     info->input_source = nm_g_unix_fd_source_new(info->child_stdout,
                                                  G_IO_IN | G_IO_ERR | G_IO_HUP,
                                                  G_PRIORITY_DEFAULT,
@@ -5155,6 +5187,16 @@ nm_utils_spawn_helper(const char *const  *args,
                                                  NULL);
     g_source_attach(info->input_source, g_main_context_get_thread_default());
 
+    /* Watch process stderr */
+    info->err_buffer   = NM_STR_BUF_INIT(0, FALSE);
+    info->error_source = nm_g_unix_fd_source_new(info->child_stderr,
+                                                 G_IO_IN | G_IO_ERR | G_IO_HUP,
+                                                 G_PRIORITY_DEFAULT,
+                                                 helper_have_err_data,
+                                                 info,
+                                                 NULL);
+    g_source_attach(info->error_source, g_main_context_get_thread_default());
+
     if (cancellable) {
         gulong signal_id;
 
diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c
index 4192808a7b..d7e05b7b5b 100644
--- a/src/core/nm-policy.c
+++ b/src/core/nm-policy.c
@@ -2581,7 +2581,7 @@ dns_config_changed(NMDnsManager *dns_manager, gpointer user_data)
         return;
 
     nm_manager_for_each_device (priv->manager, device, tmp_lst) {
-        nm_device_clear_dns_lookup_data(device);
+        nm_device_clear_dns_lookup_data(device, "DNS configuration changed");
     }
 
     update_system_hostname(self, "DNS configuration changed");
diff --git a/src/nm-daemon-helper/nm-daemon-helper.c b/src/nm-daemon-helper/nm-daemon-helper.c
index a101bf9b4f..a447d63cfe 100644
--- a/src/nm-daemon-helper/nm-daemon-helper.c
+++ b/src/nm-daemon-helper/nm-daemon-helper.c
@@ -11,6 +11,7 @@
 #if defined(__GLIBC__)
 #include <nss.h>
 #endif
+#include <stdarg.h>
 
 enum {
     RETURN_SUCCESS      = 0,
@@ -61,6 +62,7 @@ cmd_resolve_address(void)
     } sockaddr;
     socklen_t sockaddr_size;
     char      name[NI_MAXHOST];
+    int       ret;
 
     address = read_arg();
     if (!address)
@@ -83,15 +85,26 @@ cmd_resolve_address(void)
     } else
         return RETURN_INVALID_ARGS;
 
-    if (getnameinfo((struct sockaddr *) &sockaddr,
-                    sockaddr_size,
-                    name,
-                    sizeof(name),
-                    NULL,
-                    0,
-                    NI_NAMEREQD)
-        != 0)
+    ret = getnameinfo((struct sockaddr *) &sockaddr,
+                      sockaddr_size,
+                      name,
+                      sizeof(name),
+                      NULL,
+                      0,
+                      NI_NAMEREQD);
+    if (ret != 0) {
+        if (ret == EAI_SYSTEM) {
+            fprintf(stderr,
+                    "getnameinfo() failed: %d (%s), system error: %d (%s)\n",
+                    ret,
+                    gai_strerror(ret),
+                    errno,
+                    strerror(errno));
+        } else {
+            fprintf(stderr, "getnameinfo() failed: %d (%s)\n", ret, gai_strerror(ret));
+        }
         return RETURN_ERROR;
+    }
 
     printf("%s", name);