Re: [ovs-dev] [PATCH v2] ovn-nbctl: Fix the ovn-nbctl test "LBs - daemon" which fails during rpm build
On Tue, Oct 23, 2018 at 11:24:39PM +0530, Numan Siddique wrote: > On Tue, Oct 23, 2018 at 10:20 PM Ben Pfaff wrote: > > > On Tue, Oct 23, 2018 at 09:49:15AM -0700, Ben Pfaff wrote: > > > On Tue, Oct 23, 2018 at 11:48:58AM +0530, nusid...@redhat.com wrote: > > > > From: Numan Siddique > > > > > > > > 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 > > > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1641672 > > > > Tested-by: Timothy Redaelli > > > > Signed-off-by: Numan Siddique > > > > --- > > > > > > > > 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 > > > > > > I don't understand the function naming here. Why would one expect a > > > function with the name inet_parse_active_address_and_port() to not > > > accept DNS names whereas the function inet_parse_active() does? > > > > > Sorry for the bad naming. > > > > Also why can't we investigate why dns_resolve() blocks? > > > > Sorry. My bad. I didn't realize that dns_resolve() is a native ovs > function. I will investigate > why that function blocks. Thanks. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] ovn-nbctl: Fix the ovn-nbctl test "LBs - daemon" which fails during rpm build
On Tue, Oct 23, 2018 at 10:20 PM Ben Pfaff wrote: > On Tue, Oct 23, 2018 at 09:49:15AM -0700, Ben Pfaff wrote: > > On Tue, Oct 23, 2018 at 11:48:58AM +0530, nusid...@redhat.com wrote: > > > From: Numan Siddique > > > > > > 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 > > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1641672 > > > Tested-by: Timothy Redaelli > > > Signed-off-by: Numan Siddique > > > --- > > > > > > 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 > > > > I don't understand the function naming here. Why would one expect a > > function with the name inet_parse_active_address_and_port() to not > > accept DNS names whereas the function inet_parse_active() does? > > Sorry for the bad naming. > Also why can't we investigate why dns_resolve() blocks? > Sorry. My bad. I didn't realize that dns_resolve() is a native ovs function. I will investigate why that function blocks. Thanks Numan ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] ovn-nbctl: Fix the ovn-nbctl test "LBs - daemon" which fails during rpm build
On Tue, Oct 23, 2018 at 09:49:15AM -0700, Ben Pfaff wrote: > On Tue, Oct 23, 2018 at 11:48:58AM +0530, nusid...@redhat.com wrote: > > From: Numan Siddique > > > > 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 > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1641672 > > Tested-by: Timothy Redaelli > > Signed-off-by: Numan Siddique > > --- > > > > 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 > > I don't understand the function naming here. Why would one expect a > function with the name inet_parse_active_address_and_port() to not > accept DNS names whereas the function inet_parse_active() does? Also why can't we investigate why dns_resolve() blocks? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] ovn-nbctl: Fix the ovn-nbctl test "LBs - daemon" which fails during rpm build
On Tue, Oct 23, 2018 at 11:48:58AM +0530, nusid...@redhat.com wrote: > From: Numan Siddique > > 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 > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1641672 > Tested-by: Timothy Redaelli > Signed-off-by: Numan Siddique > --- > > 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 I don't understand the function naming here. Why would one expect a function with the name inet_parse_active_address_and_port() to not accept DNS names whereas the function inet_parse_active() does? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2] ovn-nbctl: Fix the ovn-nbctl test "LBs - daemon" which fails during rpm build
From: Numan Siddique
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
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1641672
Tested-by: Timothy Redaelli
Signed-off-by: Numan Siddique
---
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 "[:]".
- * , which is required, may be an IPv4 address or an IPv6 address
- * enclosed in square brackets. If 'default_port' is nonnegative then
- * 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 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 "[:]".
+ * , 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 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 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 "[:]".
+ * , 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 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 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
