On Tue, Dec 19, 2017 at 05:08:42AM -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 to use DNS names when specifying OpenFlow and
> OVSDB remotes.
> 
> Below are some of the features and limitations of this patch:
>     - The resolving is asynchornous, so it doesn't block the main loop;
>     - Both IPv4 and IPv6 are supported;
>     - The resolving API is thread-safe;
>     - Depends on the unbound library;
>     - When multiple ip addresses are returned, only the first one is used;
>     - /etc/nsswitch.conf isn't respected as unbound library doesn't look at 
> it;
>     - Depends on caller to retry later to use the resolved results.
> 
> v1 -> v2: refactored and improved code based on reviewer's comments.
> v2 -> v3: add commit message.
> 
> Signed-off-by: Yifeng Sun <[email protected]>

Thanks for v3!  I think that many users are looking forward to DNS
support.

Thanks for including a change log (e.g. v1->v2, ...).  However, the
change log should go after the --- line in the email; otherwise, "git
am" puts it into the commit message (and it should not be in there).
You can do that by putting --- into your commit message, or by adding
the change log manually within the editor invoked by "git send-email
--annotate".

This commit should update the documentation.  I imagine that in several
places the documentation says that only IP addresses are supported, and
that should now be fixed.

This commit should add a NEWS entry.

This commit should update the build instructions to describe the new
optional dependency.

This commit should add the proper dependencies to the Debian, RHEL, and
Fedora packaging.

The copyright notices in the new files look wrong to me.  This code is
new this year and as far as I know it is not derived from any older
code.  If so, then that means that the copyright year should be 2017 and
not any other years.

The dns-resolve code uses a strategy of making the client understand at
compile time whether the resolver is available, using an #ifdef.  This
is usually not the strategy that we use in Open vSwitch, because it
means that the knowledge whether a feature is available has to be spread
throughout the code.  Instead, we try to centralize it in a single
location to the extent practical, by writing appropriate stubs that do
nothing or return an error when the feature is not available.  You can
see a few examples of this strategy in lib/async-append*.[ch],
lib/route-table*.[ch], and lib/if-notifier*.[ch].  We usually find that
this makes code easier to understand and maintain.

Please consider the un-ratelimited log messages that this commit adds.
I think that some of them could flood the logs if they triggered.  If
so, please ratelimit them.

Thanks,

Ben.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to