ovsdb-server: Fix handling of DNS name for listener configuration.

Commit 08e9e5337383 fixed proper initialization of the dns-resolve
module, and made DNS resolution asynchronous.

A side effect of that change revealed a long standing logic bug
which broke ovsdb-server listener configuration using DNS names.

Previously this worked because the DNS resolution would block,
now that DNS resolution is asynchronous the code before this
change would assume the error from jsonrpc_pstream_open meant
the remote was a specification for an active outgoing
connection, even when that was not the case.

To fix this a couple of changes was made to socket-util:
1) Pass optional result of dns resolution from inet_parse_passive.

When (re-)configuring listeners that use DNS names, we may need
to know whether the provided connection string is invalid or if
the provided DNS name has finished resolving.

2) Check dns resolution status in inet_open_passive.

If the connection string is valid, and contains a DNS name,
inet_open_passive will now return -EAGAIN if dns resolution
failed.  DNS resolution failure may either mean the asynchronous
resolver has not completed yet, or that the name does not resolve.

Reported-at: https://bugs.launchpad.net/bugs/1998781
Fixes: 08e9e5337383 ("ovsdb: raft: Fix inability to read the database with DNS host names.")
Fixes: 771680d96fb6 ("DNS: Add basic support for asynchronous DNS resolving")
Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

3 files changed, 39 insertions, 20 deletions

diff --git a/lib/socket-util.c b/lib/socket-util.c
index 38705cc51..3eb3a3816 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -660,7 +660,8 @@ exit:
  * zeros '*ss' and returns false. */
 bool
 inet_parse_passive(const char *target_, int default_port,
-                   struct sockaddr_storage *ss)
+                   struct sockaddr_storage *ss,
+                   bool resolve_host, bool *dns_failure)
 {
     char *target = xstrdup(target_);
     char *port, *host;
@@ -672,7 +673,7 @@ inet_parse_passive(const char *target_, int default_port,
         ok = false;
     } else {
         ok = parse_sockaddr_components(ss, host, port, default_port,
-                                       target_, true, NULL);
+                                       target_, resolve_host, dns_failure);
     }
     if (!ok) {
         memset(ss, 0, sizeof *ss);
@@ -710,8 +711,14 @@ inet_open_passive(int style, const char *target, int default_port,
     struct sockaddr_storage ss;
     int fd = 0, error;
     unsigned int yes = 1;
+    bool dns_failure;
 
-    if (!inet_parse_passive(target, default_port, &ss)) {
+    if (!inet_parse_passive(target, default_port, &ss, true, &dns_failure)) {
+        if (dns_failure) {
+            /* DNS failure means asynchronous DNS resolution is in progress,
+             * or that the name does currently not resolve. */
+            return -EAGAIN;
+        }
         return -EAFNOSUPPORT;
     }
     kernel_chooses_port = ss_get_port(&ss) == 0;
diff --git a/lib/socket-util.h b/lib/socket-util.h
index bf66393df..4eec627e3 100644
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -55,7 +55,8 @@ int inet_open_active(int style, const char *target, int default_port,
                      struct sockaddr_storage *ssp, int *fdp, uint8_t dscp);
 
 bool inet_parse_passive(const char *target, int default_port,
-                        struct sockaddr_storage *ssp);
+                        struct sockaddr_storage *ssp,
+                        bool resolve_host, bool *dns_failure);
 int inet_open_passive(int style, const char *target, int default_port,
                       struct sockaddr_storage *ssp, uint8_t dscp,
                       bool kernel_print_port);
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 916a1f414..17868f5b7 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -267,25 +267,36 @@ ovsdb_jsonrpc_server_add_remote(struct ovsdb_jsonrpc_server *svr,
     int error;
 
     error = jsonrpc_pstream_open(name, &listener, options->dscp);
-    if (error && error != EAFNOSUPPORT) {
-        VLOG_ERR_RL(&rl, "%s: listen failed: %s", name, ovs_strerror(error));
-        return NULL;
-    }
+    switch (error) {
+    case 0:
+    case EAFNOSUPPORT:
+        remote = xmalloc(sizeof *remote);
+        remote->server = svr;
+        remote->listener = listener;
+        ovs_list_init(&remote->sessions);
+        remote->dscp = options->dscp;
+        remote->read_only = options->read_only;
+        remote->role = nullable_xstrdup(options->role);
+        shash_add(&svr->remotes, name, remote);
+        if (!listener) {
+            /* Not a listener, attempt creation of active jsonrpc session. */
+            ovsdb_jsonrpc_session_create(remote,
+                                         jsonrpc_session_open(name, true),
+                                         svr->read_only || remote->read_only);
+        }
+        return remote;
 
-    remote = xmalloc(sizeof *remote);
-    remote->server = svr;
-    remote->listener = listener;
-    ovs_list_init(&remote->sessions);
-    remote->dscp = options->dscp;
-    remote->read_only = options->read_only;
-    remote->role = nullable_xstrdup(options->role);
-    shash_add(&svr->remotes, name, remote);
+    case EAGAIN:
+        VLOG_DBG_RL(&rl, "%s: listen failed: "
+                         "DNS resolution in progress or host not found", name);
+        return NULL;
 
-    if (!listener) {
-        ovsdb_jsonrpc_session_create(remote, jsonrpc_session_open(name, true),
-                                      svr->read_only || remote->read_only);
+    default:
+        VLOG_ERR_RL(&rl, "%s: listen failed: %s", name,
+                    ovs_strerror(error));
+        return NULL;
     }
-    return remote;
+    OVS_NOT_REACHED();
 }
 
 static void