[ovs-dev] [PATCH v2] ovn-nbctl: Fix the ovn-nbctl test "LBs - daemon" which fails during rpm build

nusiddiq at redhat.com nusiddiq at redhat.com
Tue Oct 23 06:18:58 UTC 2018


From: Numan Siddique <nusiddiq at redhat.com>

The test "ovn-nbctl: LBs - daemon" fails when it runs the command
"ovn-nbctl lb-add lb0 30.0.0.1a 192.168.10.10:80,192.168.10.20:80". ovn-nbctl
extracts the vip by calling the socket util function 'inet_parse_active()',
and this function blocks when it calls dns_resolve(). It blocks because
networking is disabled with mock rpm build. Why dns_resolve() blocks, needs
to be investigated and fixed there. But to unblock this issue quickly, this
patch provides a fix in OVS itself.

This patch adds a new function - inet_parse_active_address_and_port() which
expects IP:[port] address in the 'target_' argument and disables resolving
the host.

This new function is now used in ovn-northd, ovn-nbctl and ovn-trace. It is fine
to use this function as load balancer VIP cannot be a hostname.

Reported-by: Timothy Redaelli <tredaelli at redhat.com>
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1641672
Tested-by: Timothy Redaelli <tredaelli at redhat.com>
Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
---

v1 -> v2
-------
  * Addressed review comments from Mark
     - Updated the documentation of the inet_parse_active()
     - Used the new function inet_parse_active_address_and_port()
       in ovn-trace


 lib/socket-util.c         | 50 +++++++++++++++++++++++++++++----------
 lib/socket-util.h         |  2 ++
 ovn/northd/ovn-northd.c   |  2 +-
 ovn/utilities/ovn-nbctl.c |  6 ++---
 ovn/utilities/ovn-trace.c |  2 +-
 5 files changed, 44 insertions(+), 18 deletions(-)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index df9b01a9e..63732e05e 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -512,18 +512,9 @@ exit:
     return false;
 }
 
-/* Parses 'target', which should be a string in the format "<host>[:<port>]".
- * <host>, which is required, may be an IPv4 address or an IPv6 address
- * enclosed in square brackets.  If 'default_port' is nonnegative then <port>
- * is optional and defaults to 'default_port' (use 0 to make the kernel choose
- * an available port, although this isn't usually appropriate for active
- * connections).  If 'default_port' is negative, then <port> is required.
- *
- * On success, returns true and stores the parsed remote address into '*ss'.
- * On failure, logs an error, stores zeros into '*ss', and returns false. */
-bool
-inet_parse_active(const char *target_, int default_port,
-                  struct sockaddr_storage *ss)
+static bool
+inet_parse_active__(const char *target_, int default_port,
+                  struct sockaddr_storage *ss, bool resolve_host)
 {
     char *target = xstrdup(target_);
     char *port, *host;
@@ -538,7 +529,7 @@ inet_parse_active(const char *target_, int default_port,
         ok = false;
     } else {
         ok = parse_sockaddr_components(ss, host, port, default_port,
-                                       target_, true);
+                                       target_, resolve_host);
     }
     if (!ok) {
         memset(ss, 0, sizeof *ss);
@@ -547,6 +538,39 @@ inet_parse_active(const char *target_, int default_port,
     return ok;
 }
 
+/* Parses 'target', which should be a string in the format "<host>[:<port>]".
+ * <host>, which is required, can be a host name, IPv4 address or an IPv6
+ * address and may be enclosed in square brackets.  If 'default_port' is
+ * nonnegative then <port> is optional and defaults to 'default_port' (use 0
+ * to make the kernel choose an available port, although this isn't usually
+ * appropriate for active connections).  If 'default_port' is negative,
+ * then <port> is required.
+ *
+ * On success, returns true and stores the parsed remote address into '*ss'.
+ * On failure, logs an error, stores zeros into '*ss', and returns false. */
+bool
+inet_parse_active(const char *target_, int default_port,
+                  struct sockaddr_storage *ss)
+{
+    return inet_parse_active__(target_, default_port, ss, true);
+}
+
+/* Parses 'target', which should be a string in the format "<IP>[:<port>]".
+ * <IP>, which is required, should be an IPv4 address or an IPv6 address
+ * and may be enclosed in square brackets.  If 'default_port' is nonnegative
+ * then <port> is optional and defaults to 'default_port' (use 0 to make the
+ * kernel choose an available port, although this isn't usually appropriate for
+ * active connections).  If 'default_port' is negative, then <port> is
+ * required.
+ *
+ * On success, returns true and stores the parsed remote address into '*ss'.
+ * On failure, logs an error, stores zeros into '*ss', and returns false. */
+bool
+inet_parse_active_address_and_port(const char *target_, int default_port,
+                                   struct sockaddr_storage *ss)
+{
+    return inet_parse_active__(target_, default_port, ss, false);
+}
 
 /* Opens a non-blocking IPv4 or IPv6 socket of the specified 'style' and
  * connects to 'target', which should be a string in the format
diff --git a/lib/socket-util.h b/lib/socket-util.h
index 6d386304d..447a12cfc 100644
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -50,6 +50,8 @@ void inet_parse_host_port_tokens(char *s, char **hostp, char **portp);
 void inet_parse_port_host_tokens(char *s, char **portp, char **hostp);
 bool inet_parse_active(const char *target, int default_port,
                        struct sockaddr_storage *ssp);
+bool inet_parse_active_address_and_port(const char *target, int default_port,
+                                        struct sockaddr_storage *ssp);
 int inet_open_active(int style, const char *target, int default_port,
                      struct sockaddr_storage *ssp, int *fdp, uint8_t dscp);
 
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 439651f80..ddc696499 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -3209,7 +3209,7 @@ ip_address_and_port_from_lb_key(const char *key, char **ip_address,
                                 uint16_t *port, int *addr_family)
 {
     struct sockaddr_storage ss;
-    if (!inet_parse_active(key, 0, &ss)) {
+    if (!inet_parse_active_address_and_port(key, 0, &ss)) {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
         VLOG_WARN_RL(&rl, "bad ip address or port for load balancer key %s",
                      key);
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 798c97276..a2ab646fe 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -2637,7 +2637,7 @@ nbctl_lb_add(struct ctl_context *ctx)
     }
 
     struct sockaddr_storage ss_vip;
-    if (!inet_parse_active(lb_vip, 0, &ss_vip)) {
+    if (!inet_parse_active_address_and_port(lb_vip, 0, &ss_vip)) {
         ctl_error(ctx, "%s: should be an IP address (or an IP address "
                   "and a port number with : as a separator).", lb_vip);
         return;
@@ -2667,7 +2667,7 @@ nbctl_lb_add(struct ctl_context *ctx)
         struct sockaddr_storage ss_dst;
 
         if (lb_vip_port) {
-            if (!inet_parse_active(token, -1, &ss_dst)) {
+            if (!inet_parse_active_address_and_port(token, -1, &ss_dst)) {
                 ctl_error(ctx, "%s: should be an IP address and a port "
                           "number with : as a separator.", token);
                 goto out;
@@ -2786,7 +2786,7 @@ lb_info_add_smap(const struct nbrec_load_balancer *lb,
             const struct smap_node *node = nodes[i];
 
             struct sockaddr_storage ss;
-            if (!inet_parse_active(node->key, 0, &ss)) {
+            if (!inet_parse_active_address_and_port(node->key, 0, &ss)) {
                 continue;
             }
 
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index 40a79ceea..2595d007b 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -194,7 +194,7 @@ static void
 parse_lb_option(const char *s)
 {
     struct sockaddr_storage ss;
-    if (!inet_parse_active(s, 0, &ss)) {
+    if (!inet_parse_active_address_and_port(s, 0, &ss)) {
         ovs_fatal(0, "%s: bad address", s);
     }
 
-- 
2.17.2



More information about the dev mailing list