[ovs-dev] [PATCHv4] DNS: Add basic support for asynchronous DNS resolving

[Yifeng Sun]

Hi Ben,

Thanks a lot for your comment. I will check on the things you mentioned.

Best,
Yifeng

On Mon, Feb 26, 2018 at 2:59 PM, Ben Pfaff <blp at ovn.org> wrote:

> On Mon, Jan 22, 2018 at 09:20:36AM -0800, Yifeng Sun wrote:
> > This patch is a simple implementation for the proposal discussed in
> > https://mail.openvswitch.org/pipermail/ovs-dev/2017-August/337038.html
> and
> > https://mail.openvswitch.org/pipermail/ovs-dev/2017-October/340013.html.
> >
> > It enables ovs-vswitchd and other utilities to use DNS names when
> specifying
> > OpenFlow and OVSDB remotes.
>
> Thanks a lot for figuring this out.  I have a few minor suggestions,
> which I'm appending in the form of a patch.  The additional #includes
> are there because the BSDs require them (and building with "sparse"
> reported this); the others are just style preferences.
>
> Another suggestion: I think that the documentation should mention, in
> the places where we document that hostnames can be used, that OVS has to
> be built with "unbound" to enable that.
>
> I think that we probably need to update .travis.yml to get the unbound
> library on travis.  Can you check for that?
>
> Does this patch count the TTL from the time resolution finishes, or from
> the time that resolution starts?  It wasn't obvious to me.  If it counts
> it from the time that resolution starts, then it could be that in the
> case of slow DNS resolution that we end up resolving DNS names forever
> and never actually using them.  Can you check on that?
>
> Thanks,
>
> Ben.
>
> --8<--------------------------cut here-------------------------->8--
>
> diff --git a/lib/automake.mk b/lib/automake.mk
> index b225b168fb0d..bd16f724baae 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -455,14 +455,11 @@ else
>  lib_libopenvswitch_la_SOURCES += lib/stream-nossl.c
>  endif
>
> +lib_libopenvswitch_la_SOURCES += lib/dns-resolve.h
>  if HAVE_UNBOUND
> -lib_libopenvswitch_la_SOURCES += \
> -       lib/dns-resolve.h \
> -       lib/dns-resolve.c
> +lib_libopenvswitch_la_SOURCES += lib/dns-resolve.c
>  else
> -lib_libopenvswitch_la_SOURCES += \
> -       lib/dns-resolve.h \
> -       lib/dns-resolve-stub.c
> +lib_libopenvswitch_la_SOURCES += lib/dns-resolve-stub.c
>  endif
>
>  pkgconfig_DATA += \
> diff --git a/lib/dns-resolve.c b/lib/dns-resolve.c
> index b94a5266c33d..391a8f48872b 100644
> --- a/lib/dns-resolve.c
> +++ b/lib/dns-resolve.c
> @@ -16,6 +16,8 @@
>
>  #include <config.h>
>  #include "dns-resolve.h"
> +#include <sys/types.h>
> +#include <netinet/in.h>
>  #include <arpa/inet.h>
>  #include <arpa/nameser.h>
>  #include <errno.h>
> @@ -91,7 +93,8 @@ dns_resolve_init(bool is_daemon)
>      /* Handles '/etc/hosts' on Linux and 'WINDIR/etc/hosts' on Windows. */
>      retval = ub_ctx_hosts(ub_ctx__, NULL);
>      if (retval != 0) {
> -        VLOG_WARN_RL(&rl, "Failed to read etc/hosts: %s",
> ub_strerror(retval));
> +        VLOG_WARN_RL(&rl, "Failed to read /etc/hosts: %s",
> +                     ub_strerror(retval));
>      }
>
>      ub_ctx_async(ub_ctx__, true);
>