On 5/18/23 05:50, Terry Wilson wrote:
> This adds a Python version of the async DNS support added in:
>
> 771680d96 DNS: Add basic support for asynchronous DNS resolving
>
> The above version uses the unbound C library, and this
> implimentation uses the SWIG-wrapped Python version of that.
>
> In the event that the Python unbound library is not available,
> a warning will be logged and the resolve() method will just
> return None. For the case where inet_parse_active() is passed
> an IP address, it will not try to resolve it, so existing
> behavior should be preserved in the case that the unbound
> library is unavailable.
>
> Intentional differences from the C version are as follows:
>
> OVS_HOSTS_FILE environment variable can bet set to override
> the system 'hosts' file. This is primarily to allow testing to
> be done without requiring network connectivity.
>
> Since resolution can still be done via hosts file lookup, DNS
> lookups are not disabled when resolv.conf cannot be loaded.
>
> The Python socket_util module has fallen behind its C equivalent.
> The bare minimum change was done to inet_parse_active() to support
> sync/async dns, as there is no equivalent to
> parse_sockaddr_components(), inet_parse_passive(), etc. A TODO
> was added to bring socket_util.py up to equivalency to the C
> version.
>
> Signed-off-by: Terry Wilson <twil...@redhat.com>
> ---
Thanks, Terry! The patch looks good in general, except for a
few things that Adrian pointed out in his review.
See a couple additional comments below.
> .github/workflows/build-and-test.yml | 4 +-
> debian/control.in | 1 +
> python/TODO.rst | 7 +
> python/automake.mk | 2 +
> python/ovs/dns_resolve.py | 257 ++++++++++++++++++++++++
> python/ovs/socket_util.py | 21 +-
> python/ovs/stream.py | 2 +-
> python/ovs/tests/test_dns_resolve.py | 280 +++++++++++++++++++++++++++
> python/setup.py | 6 +-
> rhel/openvswitch-fedora.spec.in | 2 +-
> tests/vlog.at | 2 +
> 11 files changed, 575 insertions(+), 9 deletions(-)
> create mode 100644 python/ovs/dns_resolve.py
> create mode 100644 python/ovs/tests/test_dns_resolve.py
>
<snip>
> diff --git a/python/ovs/dns_resolve.py b/python/ovs/dns_resolve.py
> new file mode 100644
> index 000000000..463a39277
> --- /dev/null
> +++ b/python/ovs/dns_resolve.py
> @@ -0,0 +1,257 @@
Generally, each file with a code should have a copyright header
with a license boilerplate.
It's probably less important for test files, but that is what
our current coding style document is asking to do.
> +import collections
> +import functools
> +import ipaddress
> +import os
> +import threading
> +import time
> +import typing
> +
> +try:
> + import unbound # type: ignore
> +except ImportError:
> + pass
> +
> +import ovs.vlog
> +
> +vlog = ovs.vlog.Vlog("dns_resolve")
> +
> +
> +class DNSRequest:
> + INVALID = 0
> + PENDING = 1
> + GOOD = 2
> + ERROR = 3
> +
> + def __init__(self, name):
> + self.name = name
> + self.state = self.INVALID
> + self.time = None
> + self.result = None # set by DNSResolver._callback
> + self.ttl = None
> +
> + @property
> + def expired(self):
> + return time.time() > self.time + self.ttl
> +
> + @property
> + def is_valid(self):
> + return self.state == self.GOOD and not self.expired
> +
> + def __str__(self):
> + return (f"DNSRequest(name={self.name}, state={self.state}, "
> + f"time={self.time}, result={self.result})")
> +
> +
> +class DefaultReqDict(collections.defaultdict):
> + def __init__(self):
> + super().__init__(DNSRequest)
> +
> + def __missing__(self, key):
> + ret = self.default_factory(key)
> + self[key] = ret
> + return ret
> +
> +
> +class UnboundException(Exception):
> +
> + def __init__(self, message, errno):
> + try:
> + msg = f"{message}: {unbound.ub_strerror(errno)}"
> + except NameError:
> + msg = message
> + super().__init__(msg)
> +
> +
> +def dns_enabled(func):
> + @functools.wraps(func)
> + def wrapper(self, *args, **kwargs):
> + if self.dns_enabled:
> + return func(self, *args, **kwargs)
> + vlog.err("DNS support requires the python unbound library")
> + return wrapper
> +
> +
> +class singleton:
> + def __init__(self, klass):
> + self._klass = klass
> + self._instance = None
> +
> + def __call__(self, *args, **kwargs):
> + if self._instance is None:
> + self._instance = self._klass(*args, **kwargs)
> + return self._instance
> +
> +
> +@singleton
> +class DNSResolver:
> + def __init__(self, is_daemon: bool = False):
> + """Create a resolver instance
> +
> + If is_daemon is true, set the resolver to handle requests
> + asynchronously. The following environment variables are processed:
> +
> + OVS_UNBOUND_CONF: The filename for an unbound.conf file
> + OVS_RESOLVE_CONF: A filename to override the system default
> resolv.conf
s/OVS_RESOLVE_CONF/OVS_RESOLV_CONF/
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
