| Message ID | 20230614210737.1629660-1-twilson@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v3] python: Add async DNS support | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/intel-ovs-compilation | success | test: success |
Just checking in on this. I see a single test failure https://github.com/ovsrobot/ovs/actions/runs/5272193494 'linux clang test asan' but it looks like something related to bfd, so definitely not the python patch here. Anything I need to do? On Wed, Jun 14, 2023 at 4:07 PM Terry Wilson <twilson@redhat.com> 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 <twilson@redhat.com> > --- > .github/workflows/build-and-test.yml | 4 +- > Documentation/intro/install/general.rst | 4 +- > Documentation/intro/install/rhel.rst | 2 +- > Documentation/intro/install/windows.rst | 2 +- > NEWS | 4 +- > debian/control.in | 1 + > m4/openvswitch.m4 | 8 +- > python/TODO.rst | 7 + > python/automake.mk | 2 + > python/ovs/dns_resolve.py | 272 +++++++++++++++++++++++ > 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 + > 16 files changed, 601 insertions(+), 18 deletions(-) > create mode 100644 python/ovs/dns_resolve.py > create mode 100644 python/ovs/tests/test_dns_resolve.py > > diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml > index f66ab43b0..47d239f10 100644 > --- a/.github/workflows/build-and-test.yml > +++ b/.github/workflows/build-and-test.yml > @@ -183,10 +183,10 @@ jobs: > run: sudo apt update || true > - name: install common dependencies > run: sudo apt install -y ${{ env.dependencies }} > - - name: install libunbound libunwind > + - name: install libunbound libunwind python3-unbound > # GitHub Actions doesn't have 32-bit versions of these libraries. > if: matrix.m32 == '' > - run: sudo apt install -y libunbound-dev libunwind-dev > + run: sudo apt install -y libunbound-dev libunwind-dev python3-unbound > - name: install 32-bit libraries > if: matrix.m32 != '' > run: sudo apt install -y gcc-multilib > diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst > index 42b5682fd..19e360d47 100644 > --- a/Documentation/intro/install/general.rst > +++ b/Documentation/intro/install/general.rst > @@ -90,7 +90,7 @@ need the following software: > If libcap-ng is installed, then Open vSwitch will automatically build with > support for it. > > -- Python 3.4 or later. > +- Python 3.6 or later. > > - Unbound library, from http://www.unbound.net, is optional but recommended if > you want to enable ovs-vswitchd and other utilities to use DNS names when > @@ -208,7 +208,7 @@ simply install and run Open vSwitch you require the following software: > from iproute2 (part of all major distributions and available at > https://wiki.linuxfoundation.org/networking/iproute2). > > -- Python 3.4 or later. > +- Python 3.6 or later. > > On Linux you should ensure that ``/dev/urandom`` exists. To support TAP > devices, you must also ensure that ``/dev/net/tun`` exists. > diff --git a/Documentation/intro/install/rhel.rst b/Documentation/intro/install/rhel.rst > index d1fc42021..f2151d890 100644 > --- a/Documentation/intro/install/rhel.rst > +++ b/Documentation/intro/install/rhel.rst > @@ -92,7 +92,7 @@ Once that is completed, remove the file ``/tmp/ovs.spec``. > If python3-sphinx package is not available in your version of RHEL, you can > install it via pip with 'pip install sphinx'. > > -Open vSwitch requires python 3.4 or newer which is not available in older > +Open vSwitch requires python 3.6 or newer which is not available in older > distributions. In the case of RHEL 6.x and its derivatives, one option is > to install python34 from `EPEL`_. > > diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst > index 78f60f35a..fce099d5d 100644 > --- a/Documentation/intro/install/windows.rst > +++ b/Documentation/intro/install/windows.rst > @@ -56,7 +56,7 @@ The following explains the steps in some detail. > > 'C:/MinGW /mingw'. > > -- Python 3.4 or later. > +- Python 3.6 or later. > > Install the latest Python 3.x from python.org and verify that its path is > part of Windows' PATH environment variable. > diff --git a/NEWS b/NEWS > index cfd466663..24c694b8f 100644 > --- a/NEWS > +++ b/NEWS > @@ -36,7 +36,9 @@ Post-v3.1.0 > process extra privileges when mapping physical interconnect memory. > - SRv6 Tunnel Protocol > * Added support for userspace datapath (only). > - > + - Python > + * Added async DNS support > + * Dropped support for Python < 3.6 > > v3.1.0 - 16 Feb 2023 > -------------------- > diff --git a/debian/control.in b/debian/control.in > index 19f590d06..64b0a4ce0 100644 > --- a/debian/control.in > +++ b/debian/control.in > @@ -287,6 +287,7 @@ Depends: > Suggests: > python3-netaddr, > python3-pyparsing, > + python3-unbound, > Description: Python 3 bindings for Open vSwitch > Open vSwitch is a production quality, multilayer, software-based, > Ethernet virtual switch. It is designed to enable massive network > diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 > index 14d9249b8..373a7e413 100644 > --- a/m4/openvswitch.m4 > +++ b/m4/openvswitch.m4 > @@ -371,16 +371,16 @@ dnl Checks for valgrind/valgrind.h. > AC_DEFUN([OVS_CHECK_VALGRIND], > [AC_CHECK_HEADERS([valgrind/valgrind.h])]) > > -dnl Checks for Python 3.4 or later. > +dnl Checks for Python 3.6 or later. > AC_DEFUN([OVS_CHECK_PYTHON3], > [AC_CACHE_CHECK( > - [for Python 3 (version 3.4 or later)], > + [for Python 3 (version 3.6 or later)], > [ovs_cv_python3], > [if test -n "$PYTHON3"; then > ovs_cv_python3=$PYTHON3 > else > ovs_cv_python3=no > - for binary in python3 python3.4 python3.5 python3.6 python3.7; do > + for binary in python3{,.{6..12}}; do > ovs_save_IFS=$IFS; IFS=$PATH_SEPARATOR > for dir in $PATH; do > IFS=$ovs_save_IFS > @@ -397,7 +397,7 @@ else: > done > fi]) > if test "$ovs_cv_python3" = no; then > - AC_MSG_ERROR([Python 3.4 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) > + AC_MSG_ERROR([Python 3.6 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) > fi > AC_ARG_VAR([PYTHON3]) > PYTHON3=$ovs_cv_python3]) > diff --git a/python/TODO.rst b/python/TODO.rst > index 3a53489f1..acc5461e2 100644 > --- a/python/TODO.rst > +++ b/python/TODO.rst > @@ -32,3 +32,10 @@ Python Bindings To-do List > > * Support write-only-changed monitor mode (equivalent of > OVSDB_IDL_WRITE_CHANGED_ONLY). > + > +* socket_util: > + > + * Add equivalent fuctions to inet_parse_passive, parse_sockaddr_components, > + et al. to better support using async dns. The reconnect code will > + currently log a warning when inet_parse_active() returns w/o yet having > + resolved an address, but will continue to connect and eventually succeed. > diff --git a/python/automake.mk b/python/automake.mk > index d00911828..82a508787 100644 > --- a/python/automake.mk > +++ b/python/automake.mk > @@ -16,6 +16,7 @@ ovs_pyfiles = \ > python/ovs/compat/sortedcontainers/sorteddict.py \ > python/ovs/compat/sortedcontainers/sortedset.py \ > python/ovs/daemon.py \ > + python/ovs/dns_resolve.py \ > python/ovs/db/__init__.py \ > python/ovs/db/custom_index.py \ > python/ovs/db/data.py \ > @@ -55,6 +56,7 @@ ovs_pyfiles = \ > > ovs_pytests = \ > python/ovs/tests/test_decoders.py \ > + python/ovs/tests/test_dns_resolve.py \ > python/ovs/tests/test_filter.py \ > python/ovs/tests/test_kv.py \ > python/ovs/tests/test_list.py \ > diff --git a/python/ovs/dns_resolve.py b/python/ovs/dns_resolve.py > new file mode 100644 > index 000000000..7b0f6c266 > --- /dev/null > +++ b/python/ovs/dns_resolve.py > @@ -0,0 +1,272 @@ > +# Copyright (c) 2023 Red Hat, Inc. > +# > +# Licensed under the Apache License, Version 2.0 (the "License"); > +# you may not use this file except in compliance with the License. > +# You may obtain a copy of the License at: > +# > +# http://www.apache.org/licenses/LICENSE-2.0 > +# > +# Unless required by applicable law or agreed to in writing, software > +# distributed under the License is distributed on an "AS IS" BASIS, > +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > +# See the License for the specific language governing permissions and > +# limitations under the License. > + > +import collections > +import enum > +import functools > +import ipaddress > +import os > +import time > +import typing > + > +try: > + import unbound # type: ignore > +except ImportError: > + pass > + > +import ovs.vlog > + > +vlog = ovs.vlog.Vlog("dns_resolve") > + > + > +class ReqState(enum.Enum): > + INVALID = 0 > + PENDING = 1 > + GOOD = 2 > + ERROR = 3 > + > + > +class DNSRequest: > + > + def __init__(self, name: str): > + self.name: str = name > + self.state: ReqState = ReqState.INVALID > + self.time: typing.Optional[float] = None > + # set by DNSResolver._callback > + self.result: typing.Optional[str] = None > + self.ttl: typing.Optional[float] = None > + > + @property > + def expired(self): > + return time.time() > self.time + self.ttl > + > + @property > + def is_valid(self): > + return self.state == ReqState.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_RESOLV_CONF: A filename to override the system default resolv.conf > + OVS_HOSTS_FILE: A filename to override the system default hosts file > + > + In the event that the unbound library is missing or fails to initialize > + DNS lookup support will be disabled and the resolve() method will > + return None. > + """ > + self._is_daemon = is_daemon > + try: > + self._ctx = unbound.ub_ctx() > + self.dns_enabled = True > + except Exception: > + # The unbound docs mention that this could thrown an exception > + # but do not specify what exception that is. This can also > + # happen with a missing unbound library. > + self.dns_enabled = False > + vlog.err("Failed to initialize the unbound library") > + return > + > + # NOTE(twilson) This cache, like the C version, can grow without bound > + # and has no cleanup or aging mechanism. Given our usage patterns, this > + # should not be a problem. But this should not be used to resolve an > + # unbounded list of addresses in a long-running daemon. > + self._requests = DefaultReqDict() > + > + self._ub_call(self._set_unbound_conf) > + > + # NOTE(twilson) The C version disables DNS in this case. I didn't do > + # that here since it could still be useful to resolve addresses from > + # /etc/hosts even w/o resolv.conf > + self._ub_call(self._set_resolv_conf) > + self._ub_call(self._set_hosts_file) > + > + self._ctx.set_async(True) # Sets threaded behavior for resolve_async() > + > + def _ub_call(self, fn, *args, **kwargs): > + """Convert UnboundExceptions into vlog warnings""" > + try: > + return fn(*args, **kwargs) > + except UnboundException as e: > + vlog.warn(e) > + > + @dns_enabled > + def _set_unbound_conf(self): > + ub_cfg = os.getenv("OVS_UNBOUND_CONF") > + if ub_cfg: > + retval = self._ctx.config(ub_cfg) > + if retval != 0: > + raise UnboundException( > + "Failed to set libunbound context config", retval) > + > + @dns_enabled > + def _set_resolv_conf(self): > + filename = os.getenv("OVS_RESOLV_CONF") > + # The C lib checks that the file exists and also sets filename to > + # /etc/resolv.conf on non-Windows, but resolvconf already does this. > + retval = self._ctx.resolvconf(filename) > + if retval != 0: > + location = filename or "system default nameserver" > + raise UnboundException(location, retval) > + > + @dns_enabled > + def _set_hosts_file(self): > + # The C lib doesn't have the ability to set a hosts file, but it is > + # useful to have, especially for writing tests that don't rely on > + # network connectivity. hosts(None) uses /etc/hosts. > + filename = os.getenv("OVS_HOSTS_FILE") > + retval = self._ctx.hosts(filename) > + if retval != 0: > + location = filename or "system default hosts file" > + raise UnboundException(location, retval) > + > + @dns_enabled > + def _callback(self, req: DNSRequest, err: int, result): > + if err != 0 or (result.qtype == unbound.RR_TYPE_AAAA > + and not result.havedata): > + req.state = ReqState.ERROR > + vlog.warn(f"{req.name}: failed to resolve") > + return > + if result.qtype == unbound.RR_TYPE_A and not result.havedata: > + self._resolve_async(req, unbound.RR_TYPE_AAAA) > + return > + try: > + ip_str = next(iter(result.data.as_raw_data())) > + ip = ipaddress.ip_address(ip_str) # test if IP is valid > + # NOTE (twilson) For some reason, accessing result data outside of > + # _callback causes a segfault. So just grab and store what we need. > + req.result = str(ip) > + req.ttl = result.ttl > + req.state = ReqState.GOOD > + req.time = time.time() > + except (ValueError, StopIteration): > + req.state = ReqState.ERROR > + vlog.err(f"{req.name}: failed to resolve") > + > + @dns_enabled > + def _resolve_sync(self, name: str) -> typing.Optional[str]: > + for qtype in (unbound.RR_TYPE_A, unbound.RR_TYPE_AAAA): > + err, result = self._ctx.resolve(name, qtype) > + if err != 0: > + return None > + if not result.havedata: > + continue > + try: > + ip = ipaddress.ip_address( > + next(iter(result.data.as_raw_data()))) > + except (ValueError, StopIteration): > + return None > + return str(ip) > + > + return None > + > + @dns_enabled > + def _resolve_async(self, req: DNSRequest, qtype) -> None: > + err, _ = self._ctx.resolve_async(req.name, req, self._callback, > + qtype) > + if err != 0: > + req.state = ReqState.ERROR > + return None > + > + req.state = ReqState.PENDING > + return None > + > + @dns_enabled > + def resolve(self, name: str) -> typing.Optional[str]: > + """Resolve a host name to an IP address > + > + If the resolver is set to handle requests asynchronously, resolve() > + should be recalled until it returns a non-None result. Errors will be > + logged. > + > + :param name: The host name to resolve > + :returns: The IP address or None on error or not (yet) found > + """ > + if not self._is_daemon: > + return self._resolve_sync(name) > + retval = self._ctx.process() > + if retval != 0: > + vlog.err(f"dns-resolve error: {unbound.ub_strerror(retval)}") > + return None > + req = self._requests[name] # Creates a DNSRequest if not found > + if req.is_valid: > + return req.result > + elif req.state != ReqState.PENDING: > + self._resolve_async(req, unbound.RR_TYPE_A) > + return None > + > + > +def resolve(name: str) -> typing.Optional[str]: > + """Resolve a host name to an IP address > + > + If a DNSResolver instance has not been instantiated, or if it has been > + created with is_daemon=False, resolve() will synchronously resolve the > + hostname. If DNSResolver has been initialized with is_daemon=True, it > + will instead resolve asynchornously and resolve() will return None until > + the hostname has been resolved. > + > + :param name: The host name to resolve > + :returns: The IP address or None on error or not (yet) found > + """ > + > + return DNSResolver().resolve(name) > diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py > index 7b41dc44b..a26298b75 100644 > --- a/python/ovs/socket_util.py > +++ b/python/ovs/socket_util.py > @@ -13,12 +13,14 @@ > # limitations under the License. > > import errno > +import ipaddress > import os > import os.path > import random > import socket > import sys > > +from ovs import dns_resolve > import ovs.fatal_signal > import ovs.poller > import ovs.vlog > @@ -216,7 +218,7 @@ def is_valid_ipv4_address(address): > return True > > > -def inet_parse_active(target, default_port): > +def _inet_parse_active(target, default_port): > address = target.split(":") > if len(address) >= 2: > host_name = ":".join(address[0:-1]).lstrip('[').rstrip(']') > @@ -229,9 +231,24 @@ def inet_parse_active(target, default_port): > host_name = address[0] > if not host_name: > raise ValueError("%s: bad peer name format" % target) > + try: > + host_name = str(ipaddress.ip_address(host_name)) > + except ValueError: > + host_name = dns_resolve.resolve(host_name) > + if not host_name: > + raise ValueError("%s: bad peer name format" % target) > return (host_name, port) > > > +def inet_parse_active(target, default_port, raises=True): > + try: > + return _inet_parse_active(target, default_port) > + except ValueError: > + if raises: > + raise > + return ("", default_port) > + > + > def inet_create_socket_active(style, address): > try: > is_addr_inet = is_valid_ipv4_address(address[0]) > @@ -262,7 +279,7 @@ def inet_connect_active(sock, address, family, dscp): > > > def inet_open_active(style, target, default_port, dscp): > - address = inet_parse_active(target, default_port) > + address = inet_parse_active(target, default_port, raises=False) > family, sock = inet_create_socket_active(style, address) > if sock is None: > return family, sock > diff --git a/python/ovs/stream.py b/python/ovs/stream.py > index b32341076..82fbb0d68 100644 > --- a/python/ovs/stream.py > +++ b/python/ovs/stream.py > @@ -784,7 +784,7 @@ class SSLStream(Stream): > > @staticmethod > def _open(suffix, dscp): > - address = ovs.socket_util.inet_parse_active(suffix, 0) > + address = ovs.socket_util.inet_parse_active(suffix, 0, raises=False) > family, sock = ovs.socket_util.inet_create_socket_active( > socket.SOCK_STREAM, address) > if sock is None: > diff --git a/python/ovs/tests/test_dns_resolve.py b/python/ovs/tests/test_dns_resolve.py > new file mode 100644 > index 000000000..fca8049b6 > --- /dev/null > +++ b/python/ovs/tests/test_dns_resolve.py > @@ -0,0 +1,280 @@ > +import contextlib > +import ipaddress > +import sys > +import time > +from unittest import mock > + > +import pytest > + > +from ovs import dns_resolve > +from ovs import socket_util > + > + > +skip_no_unbound = pytest.mark.skipif("unbound" not in dns_resolve.__dict__, > + reason="Unbound not installed") > + > +HOSTS = [("192.0.2.1", "fake.ip4.domain", "192.0.2.1"), > + ("2001:db8:2::1", "fake.ip6.domain", "2001:db8:2::1"), > + ("192.0.2.2", "fake.both.domain", "192.0.2.2"), > + ("2001:db8:2::2", "fake.both.domain", "192.0.2.2")] > + > + > +def _tmp_file(path, content): > + path.write_text(content) > + assert content == path.read_text() > + return path > + > + > +@pytest.fixture(params=[False, True], ids=["not_daemon", "daemon"]) > +def resolver_factory(monkeypatch, tmp_path, request): > + # Allow delaying the instantiation of the DNSResolver > + def resolver_factory(hosts=HOSTS): > + path = tmp_path / "hosts" > + content = "\n".join(f"{ip}\t{host}" for ip, host, _ in hosts) > + _tmp_file(path, content) > + > + with monkeypatch.context() as m: > + m.setenv("OVS_HOSTS_FILE", str(path)) > + # Test with both is_daemon False and True > + resolver = dns_resolve.DNSResolver(request.param) > + assert resolver._is_daemon == request.param > + return dns_resolve.DNSResolver(request.param) > + > + yield resolver_factory > + dns_resolve.DNSResolver._instance = None > + > + > +@contextlib.contextmanager > +def DNSResolver(*args, **kwargs): > + """Clean up after returning a dns_resolver.DNSResolver > + > + Since it is a singleton, and pytest runs all tests in the same process, > + we can't use dns_resolver.DNSResolver directly in these tests. This > + context manager will reset the singleton at the end of the with block. > + """ > + resolver = dns_resolve.DNSResolver(*args, **kwargs) > + try: > + yield resolver > + finally: > + dns_resolve.DNSResolver._instance = None > + > + > +@pytest.fixture > +def unbound_conf(tmp_path): > + path = tmp_path / "unbound.conf" > + content = """ > + server: > + verbosity: 1 > + """ > + return _tmp_file(path, content) > + > + > +@pytest.fixture > +def resolv_conf(tmp_path): > + path = tmp_path / "resolv.conf" > + content = "nameserver 127.0.0.1" > + return _tmp_file(path, content) > + > + > +@pytest.fixture > +def hosts_file(tmp_path): > + path = tmp_path / "hosts" > + content = "127.0.0.1\tfakelocalhost.localdomain" > + return _tmp_file(path, content) > + > + > +@pytest.fixture > +def missing_file(tmp_path): > + f = tmp_path / "missing_file" > + assert not f.exists() > + return f > + > + > +@pytest.fixture(params=[False, True], ids=["with unbound", "without unbound"]) > +def missing_unbound(monkeypatch, request): > + if request.param: > + if "unbound" in dns_resolve.__dict__: > + monkeypatch.setitem(sys.modules, 'unbound', None) > + monkeypatch.delitem(dns_resolve.__dict__, "unbound") > + elif "unbound" not in dns_resolve.__dict__: > + pytest.skip("Unbound not installed") > + return request.param > + > + > +def test_missing_unbound(missing_unbound, resolver_factory): > + resolver = resolver_factory() # Dont fail even w/o unbound > + assert resolver.dns_enabled == (not missing_unbound) > + > + > +def test_DNSRequest_defaults(): > + req = dns_resolve.DNSRequest(HOSTS[0][1]) > + assert HOSTS[0][1] == req.name > + assert req.state == dns_resolve.ReqState.INVALID > + assert req.time == req.result == req.ttl is None > + assert str(req) > + > + > +def test_DNSResolver_singleton(): > + with DNSResolver(True) as r1: > + assert r1._is_daemon > + r2 = dns_resolve.DNSResolver(False) > + assert r1 == r2 > + assert r1._is_daemon > + > + > +def _resolve(resolver, host, fn=dns_resolve.resolve): > + """Handle sync/async lookups, giving up if more than 1 second has passed""" > + > + timeout = 1 > + start = time.time() > + name = fn(host) > + if resolver._is_daemon: > + while name is None: > + name = fn(host) > + if name: > + break > + time.sleep(0.01) > + end = time.time() > + if end - start > timeout: > + break > + if name: > + return name > + raise LookupError(f"{host} not found") > + > + > +@pytest.mark.parametrize("ip,host,expected", HOSTS) > +def test_resolve_addresses(missing_unbound, resolver_factory, ip, host, > + expected): > + resolver = resolver_factory() > + if missing_unbound: > + with pytest.raises(LookupError): > + _resolve(resolver, host) > + else: > + result = _resolve(resolver, host) > + assert ipaddress.ip_address(expected) == ipaddress.ip_address(result) > + > + > +def test_resolve_unknown_host(missing_unbound, resolver_factory): > + resolver = resolver_factory() > + with pytest.raises(LookupError): > + _resolve(resolver, "fake.notadomain") > + > + > +@skip_no_unbound > +def test_resolve_process_error(): > + with DNSResolver(True) as resolver: > + with mock.patch.object(resolver._ctx, "process", return_value=-1): > + assert resolver.resolve("fake.domain") is None > + > + > +@skip_no_unbound > +def test_resolve_resolve_error(): > + with DNSResolver(False) as resolver: > + with mock.patch.object(resolver._ctx, "resolve", > + return_value=(-1, None)): > + assert resolver.resolve("fake.domain") is None > + > + > +@skip_no_unbound > +def test_resolve_resolve_async_error(): > + with DNSResolver(True) as resolver: > + with mock.patch.object(resolver._ctx, "resolve_async", > + return_value=(-1, None)): > + with pytest.raises(LookupError): > + _resolve(resolver, "fake.domain") > + > + > +@pytest.mark.parametrize("file,raises", > + [(None, False), > + ("missing_file", dns_resolve.UnboundException), > + ("unbound_conf", False)]) > +def test_set_unbound_conf(monkeypatch, missing_unbound, resolver_factory, > + request, file, raises): > + if file: > + file = str(request.getfixturevalue(file)) > + monkeypatch.setenv("OVS_UNBOUND_CONF", file) > + resolver = resolver_factory() # Doesn't raise > + if missing_unbound: > + assert resolver._set_unbound_conf() is None > + return > + with mock.patch.object(resolver._ctx, "config", > + side_effect=resolver._ctx.config) as c: > + if raises: > + with pytest.raises(raises): > + resolver._set_unbound_conf() > + else: > + resolver._set_unbound_conf() > + if file: > + c.assert_called_once_with(file) > + else: > + c.assert_not_called() > + > + > +@pytest.mark.parametrize("file,raises", > + [(None, False), > + ("missing_file", dns_resolve.UnboundException), > + ("resolv_conf", False)]) > +def test_resolv_conf(monkeypatch, missing_unbound, resolver_factory, request, > + file, raises): > + if file: > + file = str(request.getfixturevalue(file)) > + monkeypatch.setenv("OVS_RESOLV_CONF", file) > + resolver = resolver_factory() # Doesn't raise > + if missing_unbound: > + assert resolver._set_resolv_conf() is None > + return > + with mock.patch.object(resolver._ctx, "resolvconf", > + side_effect=resolver._ctx.resolvconf) as c: > + if raises: > + with pytest.raises(raises): > + resolver._set_resolv_conf() > + else: > + resolver._set_resolv_conf() > + c.assert_called_once_with(file) > + > + > +@pytest.mark.parametrize("file,raises", > + [(None, False), > + ("missing_file", dns_resolve.UnboundException), > + ("hosts_file", False)]) > +def test_hosts(monkeypatch, missing_unbound, resolver_factory, request, file, > + raises): > + if file: > + file = str(request.getfixturevalue(file)) > + monkeypatch.setenv("OVS_HOSTS_FILE", file) > + resolver = resolver_factory() # Doesn't raise > + if missing_unbound: > + assert resolver._set_hosts_file() is None > + return > + with mock.patch.object(resolver._ctx, "hosts", > + side_effect=resolver._ctx.hosts) as c: > + if raises: > + with pytest.raises(raises): > + resolver._set_hosts_file() > + else: > + resolver._set_hosts_file() > + c.assert_called_once_with(file) > + > + > +def test_UnboundException(missing_unbound): > + with pytest.raises(dns_resolve.UnboundException): > + raise dns_resolve.UnboundException("Fake exception", -1) > + > + > +@skip_no_unbound > +@pytest.mark.parametrize("ip,host,expected", HOSTS) > +def test_inet_parse_active(resolver_factory, ip, host, expected): > + resolver = resolver_factory() > + > + def fn(name): > + # Return the same thing _resolve() would so we can call > + # this multiple times for the is_daemon=True case > + return socket_util.inet_parse_active(f"{name}:6640", 6640, > + raises=False)[0] or None > + > + # parsing IPs still works > + IP = _resolve(resolver, ip, fn) > + assert ipaddress.ip_address(ip) == ipaddress.ip_address(IP) > + # parsing hosts works > + IP = _resolve(resolver, host, fn) > + assert ipaddress.ip_address(IP) == ipaddress.ip_address(expected) > diff --git a/python/setup.py b/python/setup.py > index 27684c404..bcf832ce9 100644 > --- a/python/setup.py > +++ b/python/setup.py > @@ -99,8 +99,7 @@ setup_args = dict( > 'Topic :: System :: Networking', > 'License :: OSI Approved :: Apache Software License', > 'Programming Language :: Python :: 3', > - 'Programming Language :: Python :: 3.4', > - 'Programming Language :: Python :: 3.5', > + 'Programming Language :: Python :: 3.6', > ], > ext_modules=[setuptools.Extension("ovs._json", > sources=["ovs/_json.c"], > @@ -110,7 +109,8 @@ setup_args = dict( > cmdclass={'build_ext': try_build_ext}, > install_requires=['sortedcontainers'], > extras_require={':sys_platform == "win32"': ['pywin32 >= 1.0'], > - 'flow': ['netaddr', 'pyparsing']}, > + 'flow': ['netaddr', 'pyparsing'], > + 'dns': ['unbound']}, > ) > > try: > diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in > index 44899c1ca..343a5716d 100644 > --- a/rhel/openvswitch-fedora.spec.in > +++ b/rhel/openvswitch-fedora.spec.in > @@ -113,7 +113,7 @@ Summary: Open vSwitch python3 bindings > License: ASL 2.0 > BuildArch: noarch > Requires: python3 > -Suggests: python3-netaddr python3-pyparsing > +Suggests: python3-netaddr python3-pyparsing python3-unbound > %{?python_provide:%python_provide python3-openvswitch = %{version}-%{release}} > > %description -n python3-openvswitch > diff --git a/tests/vlog.at b/tests/vlog.at > index 3e92e70a9..785014956 100644 > --- a/tests/vlog.at > +++ b/tests/vlog.at > @@ -385,6 +385,7 @@ AT_CHECK([APPCTL -t test-unixctl.py vlog/list], [0], [dnl > console syslog file > ------- ------ ------ > daemon info info info > +dns_resolve info info info > fatal-signal info info info > jsonrpc info info info > poller info info info > @@ -404,6 +405,7 @@ unixctl_server info info info > console syslog file > ------- ------ ------ > daemon info err dbg > +dns_resolve info info dbg > fatal-signal info info dbg > jsonrpc info info dbg > poller info info dbg > -- > 2.34.3 >
On 6/23/23 17:15, Terry Wilson wrote: > Just checking in on this. I see a single test failure > https://github.com/ovsrobot/ovs/actions/runs/5272193494 'linux clang > test asan' but it looks like something related to bfd, so definitely > not the python patch here. This one is unrelated, yes. > Anything I need to do? I think, Adrian wanted to take another look at this version. See a couple of comments inline, but if there will be no other comments I can fix these while applying the change. > > On Wed, Jun 14, 2023 at 4:07 PM Terry Wilson <twilson@redhat.com> 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 <twilson@redhat.com> >> --- >> .github/workflows/build-and-test.yml | 4 +- >> Documentation/intro/install/general.rst | 4 +- >> Documentation/intro/install/rhel.rst | 2 +- >> Documentation/intro/install/windows.rst | 2 +- >> NEWS | 4 +- >> debian/control.in | 1 + >> m4/openvswitch.m4 | 8 +- >> python/TODO.rst | 7 + >> python/automake.mk | 2 + >> python/ovs/dns_resolve.py | 272 +++++++++++++++++++++++ >> 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 + >> 16 files changed, 601 insertions(+), 18 deletions(-) >> create mode 100644 python/ovs/dns_resolve.py >> create mode 100644 python/ovs/tests/test_dns_resolve.py >> >> diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml >> index f66ab43b0..47d239f10 100644 >> --- a/.github/workflows/build-and-test.yml >> +++ b/.github/workflows/build-and-test.yml >> @@ -183,10 +183,10 @@ jobs: >> run: sudo apt update || true >> - name: install common dependencies >> run: sudo apt install -y ${{ env.dependencies }} >> - - name: install libunbound libunwind >> + - name: install libunbound libunwind python3-unbound >> # GitHub Actions doesn't have 32-bit versions of these libraries. >> if: matrix.m32 == '' >> - run: sudo apt install -y libunbound-dev libunwind-dev >> + run: sudo apt install -y libunbound-dev libunwind-dev python3-unbound >> - name: install 32-bit libraries >> if: matrix.m32 != '' >> run: sudo apt install -y gcc-multilib >> diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst >> index 42b5682fd..19e360d47 100644 >> --- a/Documentation/intro/install/general.rst >> +++ b/Documentation/intro/install/general.rst >> @@ -90,7 +90,7 @@ need the following software: >> If libcap-ng is installed, then Open vSwitch will automatically build with >> support for it. >> >> -- Python 3.4 or later. >> +- Python 3.6 or later. >> >> - Unbound library, from http://www.unbound.net, is optional but recommended if >> you want to enable ovs-vswitchd and other utilities to use DNS names when >> @@ -208,7 +208,7 @@ simply install and run Open vSwitch you require the following software: >> from iproute2 (part of all major distributions and available at >> https://wiki.linuxfoundation.org/networking/iproute2). >> >> -- Python 3.4 or later. >> +- Python 3.6 or later. >> >> On Linux you should ensure that ``/dev/urandom`` exists. To support TAP >> devices, you must also ensure that ``/dev/net/tun`` exists. >> diff --git a/Documentation/intro/install/rhel.rst b/Documentation/intro/install/rhel.rst >> index d1fc42021..f2151d890 100644 >> --- a/Documentation/intro/install/rhel.rst >> +++ b/Documentation/intro/install/rhel.rst >> @@ -92,7 +92,7 @@ Once that is completed, remove the file ``/tmp/ovs.spec``. >> If python3-sphinx package is not available in your version of RHEL, you can >> install it via pip with 'pip install sphinx'. >> >> -Open vSwitch requires python 3.4 or newer which is not available in older >> +Open vSwitch requires python 3.6 or newer which is not available in older >> distributions. In the case of RHEL 6.x and its derivatives, one option is >> to install python34 from `EPEL`_. >> >> diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst >> index 78f60f35a..fce099d5d 100644 >> --- a/Documentation/intro/install/windows.rst >> +++ b/Documentation/intro/install/windows.rst >> @@ -56,7 +56,7 @@ The following explains the steps in some detail. >> >> 'C:/MinGW /mingw'. >> >> -- Python 3.4 or later. >> +- Python 3.6 or later. >> >> Install the latest Python 3.x from python.org and verify that its path is >> part of Windows' PATH environment variable. >> diff --git a/NEWS b/NEWS >> index cfd466663..24c694b8f 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -36,7 +36,9 @@ Post-v3.1.0 >> process extra privileges when mapping physical interconnect memory. >> - SRv6 Tunnel Protocol >> * Added support for userspace datapath (only). >> - Nit: We should keep 2 empty lines between sections for consistency. >> + - Python >> + * Added async DNS support >> + * Dropped support for Python < 3.6 Nit: Some periods at the end of sentences would be nice. >> >> v3.1.0 - 16 Feb 2023 >> -------------------- >> diff --git a/debian/control.in b/debian/control.in >> index 19f590d06..64b0a4ce0 100644 >> --- a/debian/control.in >> +++ b/debian/control.in >> @@ -287,6 +287,7 @@ Depends: >> Suggests: >> python3-netaddr, >> python3-pyparsing, >> + python3-unbound, >> Description: Python 3 bindings for Open vSwitch >> Open vSwitch is a production quality, multilayer, software-based, >> Ethernet virtual switch. It is designed to enable massive network >> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 >> index 14d9249b8..373a7e413 100644 >> --- a/m4/openvswitch.m4 >> +++ b/m4/openvswitch.m4 >> @@ -371,16 +371,16 @@ dnl Checks for valgrind/valgrind.h. >> AC_DEFUN([OVS_CHECK_VALGRIND], >> [AC_CHECK_HEADERS([valgrind/valgrind.h])]) >> >> -dnl Checks for Python 3.4 or later. >> +dnl Checks for Python 3.6 or later. >> AC_DEFUN([OVS_CHECK_PYTHON3], >> [AC_CACHE_CHECK( >> - [for Python 3 (version 3.4 or later)], >> + [for Python 3 (version 3.6 or later)], >> [ovs_cv_python3], >> [if test -n "$PYTHON3"; then >> ovs_cv_python3=$PYTHON3 >> else >> ovs_cv_python3=no >> - for binary in python3 python3.4 python3.5 python3.6 python3.7; do >> + for binary in python3{,.{6..12}}; do This is not a portable syntax. It fails a FreeBSD build for example. We need to unwind it back, or use some other portable way of iteration. Best regards, Ilya Maximets.
On Fri, Jun 23, 2023 at 11:20 AM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 6/23/23 17:15, Terry Wilson wrote: > > Just checking in on this. I see a single test failure > > https://github.com/ovsrobot/ovs/actions/runs/5272193494 'linux clang > > test asan' but it looks like something related to bfd, so definitely > > not the python patch here. > > This one is unrelated, yes. > > > Anything I need to do? > > I think, Adrian wanted to take another look at this version. > > See a couple of comments inline, but if there will be no other > comments I can fix these while applying the change. > > > > > On Wed, Jun 14, 2023 at 4:07 PM Terry Wilson <twilson@redhat.com> 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 <twilson@redhat.com> > >> --- > >> .github/workflows/build-and-test.yml | 4 +- > >> Documentation/intro/install/general.rst | 4 +- > >> Documentation/intro/install/rhel.rst | 2 +- > >> Documentation/intro/install/windows.rst | 2 +- > >> NEWS | 4 +- > >> debian/control.in | 1 + > >> m4/openvswitch.m4 | 8 +- > >> python/TODO.rst | 7 + > >> python/automake.mk | 2 + > >> python/ovs/dns_resolve.py | 272 +++++++++++++++++++++++ > >> 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 + > >> 16 files changed, 601 insertions(+), 18 deletions(-) > >> create mode 100644 python/ovs/dns_resolve.py > >> create mode 100644 python/ovs/tests/test_dns_resolve.py > >> > >> diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml > >> index f66ab43b0..47d239f10 100644 > >> --- a/.github/workflows/build-and-test.yml > >> +++ b/.github/workflows/build-and-test.yml > >> @@ -183,10 +183,10 @@ jobs: > >> run: sudo apt update || true > >> - name: install common dependencies > >> run: sudo apt install -y ${{ env.dependencies }} > >> - - name: install libunbound libunwind > >> + - name: install libunbound libunwind python3-unbound > >> # GitHub Actions doesn't have 32-bit versions of these libraries. > >> if: matrix.m32 == '' > >> - run: sudo apt install -y libunbound-dev libunwind-dev > >> + run: sudo apt install -y libunbound-dev libunwind-dev python3-unbound > >> - name: install 32-bit libraries > >> if: matrix.m32 != '' > >> run: sudo apt install -y gcc-multilib > >> diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst > >> index 42b5682fd..19e360d47 100644 > >> --- a/Documentation/intro/install/general.rst > >> +++ b/Documentation/intro/install/general.rst > >> @@ -90,7 +90,7 @@ need the following software: > >> If libcap-ng is installed, then Open vSwitch will automatically build with > >> support for it. > >> > >> -- Python 3.4 or later. > >> +- Python 3.6 or later. > >> > >> - Unbound library, from http://www.unbound.net, is optional but recommended if > >> you want to enable ovs-vswitchd and other utilities to use DNS names when > >> @@ -208,7 +208,7 @@ simply install and run Open vSwitch you require the following software: > >> from iproute2 (part of all major distributions and available at > >> https://wiki.linuxfoundation.org/networking/iproute2). > >> > >> -- Python 3.4 or later. > >> +- Python 3.6 or later. > >> > >> On Linux you should ensure that ``/dev/urandom`` exists. To support TAP > >> devices, you must also ensure that ``/dev/net/tun`` exists. > >> diff --git a/Documentation/intro/install/rhel.rst b/Documentation/intro/install/rhel.rst > >> index d1fc42021..f2151d890 100644 > >> --- a/Documentation/intro/install/rhel.rst > >> +++ b/Documentation/intro/install/rhel.rst > >> @@ -92,7 +92,7 @@ Once that is completed, remove the file ``/tmp/ovs.spec``. > >> If python3-sphinx package is not available in your version of RHEL, you can > >> install it via pip with 'pip install sphinx'. > >> > >> -Open vSwitch requires python 3.4 or newer which is not available in older > >> +Open vSwitch requires python 3.6 or newer which is not available in older > >> distributions. In the case of RHEL 6.x and its derivatives, one option is > >> to install python34 from `EPEL`_. > >> > >> diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst > >> index 78f60f35a..fce099d5d 100644 > >> --- a/Documentation/intro/install/windows.rst > >> +++ b/Documentation/intro/install/windows.rst > >> @@ -56,7 +56,7 @@ The following explains the steps in some detail. > >> > >> 'C:/MinGW /mingw'. > >> > >> -- Python 3.4 or later. > >> +- Python 3.6 or later. > >> > >> Install the latest Python 3.x from python.org and verify that its path is > >> part of Windows' PATH environment variable. > >> diff --git a/NEWS b/NEWS > >> index cfd466663..24c694b8f 100644 > >> --- a/NEWS > >> +++ b/NEWS > >> @@ -36,7 +36,9 @@ Post-v3.1.0 > >> process extra privileges when mapping physical interconnect memory. > >> - SRv6 Tunnel Protocol > >> * Added support for userspace datapath (only). > >> - > > Nit: We should keep 2 empty lines between sections for consistency. > > >> + - Python > >> + * Added async DNS support > >> + * Dropped support for Python < 3.6 > > Nit: Some periods at the end of sentences would be nice. > > >> > >> v3.1.0 - 16 Feb 2023 > >> -------------------- > >> diff --git a/debian/control.in b/debian/control.in > >> index 19f590d06..64b0a4ce0 100644 > >> --- a/debian/control.in > >> +++ b/debian/control.in > >> @@ -287,6 +287,7 @@ Depends: > >> Suggests: > >> python3-netaddr, > >> python3-pyparsing, > >> + python3-unbound, > >> Description: Python 3 bindings for Open vSwitch > >> Open vSwitch is a production quality, multilayer, software-based, > >> Ethernet virtual switch. It is designed to enable massive network > >> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 > >> index 14d9249b8..373a7e413 100644 > >> --- a/m4/openvswitch.m4 > >> +++ b/m4/openvswitch.m4 > >> @@ -371,16 +371,16 @@ dnl Checks for valgrind/valgrind.h. > >> AC_DEFUN([OVS_CHECK_VALGRIND], > >> [AC_CHECK_HEADERS([valgrind/valgrind.h])]) > >> > >> -dnl Checks for Python 3.4 or later. > >> +dnl Checks for Python 3.6 or later. > >> AC_DEFUN([OVS_CHECK_PYTHON3], > >> [AC_CACHE_CHECK( > >> - [for Python 3 (version 3.4 or later)], > >> + [for Python 3 (version 3.6 or later)], > >> [ovs_cv_python3], > >> [if test -n "$PYTHON3"; then > >> ovs_cv_python3=$PYTHON3 > >> else > >> ovs_cv_python3=no > >> - for binary in python3 python3.4 python3.5 python3.6 python3.7; do > >> + for binary in python3{,.{6..12}}; do > > This is not a portable syntax. It fails a FreeBSD build for example. > We need to unwind it back, or use some other portable way of iteration. Ah, thanks. I tested it with bash, bash running in compatibility mode, and zsh. /me shakes his fist at tcsh. > Best regards, Ilya Maximets. >
On 6/14/23 23:07, 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 <twilson@redhat.com> > --- > .github/workflows/build-and-test.yml | 4 +- > Documentation/intro/install/general.rst | 4 +- > Documentation/intro/install/rhel.rst | 2 +- > Documentation/intro/install/windows.rst | 2 +- > NEWS | 4 +- > debian/control.in | 1 + > m4/openvswitch.m4 | 8 +- > python/TODO.rst | 7 + > python/automake.mk | 2 + > python/ovs/dns_resolve.py | 272 +++++++++++++++++++++++ > 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 + > 16 files changed, 601 insertions(+), 18 deletions(-) > create mode 100644 python/ovs/dns_resolve.py > create mode 100644 python/ovs/tests/test_dns_resolve.py > > diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml > index f66ab43b0..47d239f10 100644 > --- a/.github/workflows/build-and-test.yml > +++ b/.github/workflows/build-and-test.yml > @@ -183,10 +183,10 @@ jobs: > run: sudo apt update || true > - name: install common dependencies > run: sudo apt install -y ${{ env.dependencies }} > - - name: install libunbound libunwind > + - name: install libunbound libunwind python3-unbound > # GitHub Actions doesn't have 32-bit versions of these libraries. > if: matrix.m32 == '' > - run: sudo apt install -y libunbound-dev libunwind-dev > + run: sudo apt install -y libunbound-dev libunwind-dev python3-unbound > - name: install 32-bit libraries > if: matrix.m32 != '' > run: sudo apt install -y gcc-multilib > diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst > index 42b5682fd..19e360d47 100644 > --- a/Documentation/intro/install/general.rst > +++ b/Documentation/intro/install/general.rst > @@ -90,7 +90,7 @@ need the following software: > If libcap-ng is installed, then Open vSwitch will automatically build with > support for it. > > -- Python 3.4 or later. > +- Python 3.6 or later. > > - Unbound library, from http://www.unbound.net, is optional but recommended if > you want to enable ovs-vswitchd and other utilities to use DNS names when > @@ -208,7 +208,7 @@ simply install and run Open vSwitch you require the following software: > from iproute2 (part of all major distributions and available at > https://wiki.linuxfoundation.org/networking/iproute2). > > -- Python 3.4 or later. > +- Python 3.6 or later. > > On Linux you should ensure that ``/dev/urandom`` exists. To support TAP > devices, you must also ensure that ``/dev/net/tun`` exists. > diff --git a/Documentation/intro/install/rhel.rst b/Documentation/intro/install/rhel.rst > index d1fc42021..f2151d890 100644 > --- a/Documentation/intro/install/rhel.rst > +++ b/Documentation/intro/install/rhel.rst > @@ -92,7 +92,7 @@ Once that is completed, remove the file ``/tmp/ovs.spec``. > If python3-sphinx package is not available in your version of RHEL, you can > install it via pip with 'pip install sphinx'. > > -Open vSwitch requires python 3.4 or newer which is not available in older > +Open vSwitch requires python 3.6 or newer which is not available in older > distributions. In the case of RHEL 6.x and its derivatives, one option is > to install python34 from `EPEL`_. > > diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst > index 78f60f35a..fce099d5d 100644 > --- a/Documentation/intro/install/windows.rst > +++ b/Documentation/intro/install/windows.rst > @@ -56,7 +56,7 @@ The following explains the steps in some detail. > > 'C:/MinGW /mingw'. > > -- Python 3.4 or later. > +- Python 3.6 or later. > > Install the latest Python 3.x from python.org and verify that its path is > part of Windows' PATH environment variable. > diff --git a/NEWS b/NEWS > index cfd466663..24c694b8f 100644 > --- a/NEWS > +++ b/NEWS > @@ -36,7 +36,9 @@ Post-v3.1.0 > process extra privileges when mapping physical interconnect memory. > - SRv6 Tunnel Protocol > * Added support for userspace datapath (only). > - > + - Python > + * Added async DNS support > + * Dropped support for Python < 3.6 > > v3.1.0 - 16 Feb 2023 > -------------------- > diff --git a/debian/control.in b/debian/control.in > index 19f590d06..64b0a4ce0 100644 > --- a/debian/control.in > +++ b/debian/control.in > @@ -287,6 +287,7 @@ Depends: > Suggests: > python3-netaddr, > python3-pyparsing, > + python3-unbound, > Description: Python 3 bindings for Open vSwitch > Open vSwitch is a production quality, multilayer, software-based, > Ethernet virtual switch. It is designed to enable massive network > diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 > index 14d9249b8..373a7e413 100644 > --- a/m4/openvswitch.m4 > +++ b/m4/openvswitch.m4 > @@ -371,16 +371,16 @@ dnl Checks for valgrind/valgrind.h. > AC_DEFUN([OVS_CHECK_VALGRIND], > [AC_CHECK_HEADERS([valgrind/valgrind.h])]) > > -dnl Checks for Python 3.4 or later. > +dnl Checks for Python 3.6 or later. > AC_DEFUN([OVS_CHECK_PYTHON3], > [AC_CACHE_CHECK( > - [for Python 3 (version 3.4 or later)], > + [for Python 3 (version 3.6 or later)], > [ovs_cv_python3], > [if test -n "$PYTHON3"; then > ovs_cv_python3=$PYTHON3 > else > ovs_cv_python3=no > - for binary in python3 python3.4 python3.5 python3.6 python3.7; do > + for binary in python3{,.{6..12}}; do > ovs_save_IFS=$IFS; IFS=$PATH_SEPARATOR > for dir in $PATH; do > IFS=$ovs_save_IFS > @@ -397,7 +397,7 @@ else: > done > fi]) > if test "$ovs_cv_python3" = no; then > - AC_MSG_ERROR([Python 3.4 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) > + AC_MSG_ERROR([Python 3.6 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) > fi > AC_ARG_VAR([PYTHON3]) > PYTHON3=$ovs_cv_python3]) > diff --git a/python/TODO.rst b/python/TODO.rst > index 3a53489f1..acc5461e2 100644 > --- a/python/TODO.rst > +++ b/python/TODO.rst > @@ -32,3 +32,10 @@ Python Bindings To-do List > > * Support write-only-changed monitor mode (equivalent of > OVSDB_IDL_WRITE_CHANGED_ONLY). > + > +* socket_util: > + > + * Add equivalent fuctions to inet_parse_passive, parse_sockaddr_components, > + et al. to better support using async dns. The reconnect code will > + currently log a warning when inet_parse_active() returns w/o yet having > + resolved an address, but will continue to connect and eventually succeed. > diff --git a/python/automake.mk b/python/automake.mk > index d00911828..82a508787 100644 > --- a/python/automake.mk > +++ b/python/automake.mk > @@ -16,6 +16,7 @@ ovs_pyfiles = \ > python/ovs/compat/sortedcontainers/sorteddict.py \ > python/ovs/compat/sortedcontainers/sortedset.py \ > python/ovs/daemon.py \ > + python/ovs/dns_resolve.py \ > python/ovs/db/__init__.py \ > python/ovs/db/custom_index.py \ > python/ovs/db/data.py \ > @@ -55,6 +56,7 @@ ovs_pyfiles = \ > > ovs_pytests = \ > python/ovs/tests/test_decoders.py \ > + python/ovs/tests/test_dns_resolve.py \ > python/ovs/tests/test_filter.py \ > python/ovs/tests/test_kv.py \ > python/ovs/tests/test_list.py \ > diff --git a/python/ovs/dns_resolve.py b/python/ovs/dns_resolve.py > new file mode 100644 > index 000000000..7b0f6c266 > --- /dev/null > +++ b/python/ovs/dns_resolve.py > @@ -0,0 +1,272 @@ > +# Copyright (c) 2023 Red Hat, Inc. > +# > +# Licensed under the Apache License, Version 2.0 (the "License"); > +# you may not use this file except in compliance with the License. > +# You may obtain a copy of the License at: > +# > +# http://www.apache.org/licenses/LICENSE-2.0 > +# > +# Unless required by applicable law or agreed to in writing, software > +# distributed under the License is distributed on an "AS IS" BASIS, > +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > +# See the License for the specific language governing permissions and > +# limitations under the License. > + > +import collections > +import enum > +import functools > +import ipaddress > +import os > +import time > +import typing > + > +try: > + import unbound # type: ignore > +except ImportError: > + pass > + > +import ovs.vlog > + > +vlog = ovs.vlog.Vlog("dns_resolve") > + > + > +class ReqState(enum.Enum): > + INVALID = 0 > + PENDING = 1 > + GOOD = 2 > + ERROR = 3 > + > + > +class DNSRequest: > + nit: extra space > + def __init__(self, name: str): > + self.name: str = name > + self.state: ReqState = ReqState.INVALID > + self.time: typing.Optional[float] = None > + # set by DNSResolver._callback > + self.result: typing.Optional[str] = None > + self.ttl: typing.Optional[float] = None > + > + @property > + def expired(self): > + return time.time() > self.time + self.ttl > + > + @property > + def is_valid(self): > + return self.state == ReqState.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 > + Having a singleton class that accepts a parameter in the constructor could be a bit confusing. For instance: "d = DNSResolver(is_daemon=True)" might yield a resolver where "is_daemon" is actually "False" if some other part of the code created it before. Woudn't it be cleaner if it just exposed "resove_sync" and "resolve_async" and let users choose how to resolve? > + > +@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_RESOLV_CONF: A filename to override the system default resolv.conf > + OVS_HOSTS_FILE: A filename to override the system default hosts file > + > + In the event that the unbound library is missing or fails to initialize > + DNS lookup support will be disabled and the resolve() method will > + return None. > + """ > + self._is_daemon = is_daemon > + try: > + self._ctx = unbound.ub_ctx() > + self.dns_enabled = True > + except Exception: > + # The unbound docs mention that this could thrown an exception > + # but do not specify what exception that is. This can also > + # happen with a missing unbound library. > + self.dns_enabled = False > + vlog.err("Failed to initialize the unbound library") > + return > + > + # NOTE(twilson) This cache, like the C version, can grow without bound > + # and has no cleanup or aging mechanism. Given our usage patterns, this > + # should not be a problem. But this should not be used to resolve an > + # unbounded list of addresses in a long-running daemon. > + self._requests = DefaultReqDict() > + > + self._ub_call(self._set_unbound_conf) > + > + # NOTE(twilson) The C version disables DNS in this case. I didn't do > + # that here since it could still be useful to resolve addresses from > + # /etc/hosts even w/o resolv.conf > + self._ub_call(self._set_resolv_conf) > + self._ub_call(self._set_hosts_file) > + > + self._ctx.set_async(True) # Sets threaded behavior for resolve_async() > + > + def _ub_call(self, fn, *args, **kwargs): > + """Convert UnboundExceptions into vlog warnings""" > + try: > + return fn(*args, **kwargs) > + except UnboundException as e: > + vlog.warn(e) > + > + @dns_enabled > + def _set_unbound_conf(self): > + ub_cfg = os.getenv("OVS_UNBOUND_CONF") > + if ub_cfg: > + retval = self._ctx.config(ub_cfg) > + if retval != 0: > + raise UnboundException( > + "Failed to set libunbound context config", retval) > + > + @dns_enabled > + def _set_resolv_conf(self): > + filename = os.getenv("OVS_RESOLV_CONF") > + # The C lib checks that the file exists and also sets filename to > + # /etc/resolv.conf on non-Windows, but resolvconf already does this. > + retval = self._ctx.resolvconf(filename) > + if retval != 0: > + location = filename or "system default nameserver" > + raise UnboundException(location, retval) > + > + @dns_enabled > + def _set_hosts_file(self): > + # The C lib doesn't have the ability to set a hosts file, but it is > + # useful to have, especially for writing tests that don't rely on > + # network connectivity. hosts(None) uses /etc/hosts. > + filename = os.getenv("OVS_HOSTS_FILE") > + retval = self._ctx.hosts(filename) > + if retval != 0: > + location = filename or "system default hosts file" > + raise UnboundException(location, retval) > + > + @dns_enabled > + def _callback(self, req: DNSRequest, err: int, result): > + if err != 0 or (result.qtype == unbound.RR_TYPE_AAAA > + and not result.havedata): > + req.state = ReqState.ERROR > + vlog.warn(f"{req.name}: failed to resolve") > + return > + if result.qtype == unbound.RR_TYPE_A and not result.havedata: > + self._resolve_async(req, unbound.RR_TYPE_AAAA) > + return > + try: > + ip_str = next(iter(result.data.as_raw_data())) > + ip = ipaddress.ip_address(ip_str) # test if IP is valid > + # NOTE (twilson) For some reason, accessing result data outside of > + # _callback causes a segfault. So just grab and store what we need. > + req.result = str(ip) > + req.ttl = result.ttl > + req.state = ReqState.GOOD > + req.time = time.time() > + except (ValueError, StopIteration): > + req.state = ReqState.ERROR > + vlog.err(f"{req.name}: failed to resolve") > + > + @dns_enabled > + def _resolve_sync(self, name: str) -> typing.Optional[str]: > + for qtype in (unbound.RR_TYPE_A, unbound.RR_TYPE_AAAA): > + err, result = self._ctx.resolve(name, qtype) > + if err != 0: > + return None > + if not result.havedata: > + continue > + try: > + ip = ipaddress.ip_address( > + next(iter(result.data.as_raw_data()))) > + except (ValueError, StopIteration): > + return None > + return str(ip) > + > + return None > + > + @dns_enabled > + def _resolve_async(self, req: DNSRequest, qtype) -> None: > + err, _ = self._ctx.resolve_async(req.name, req, self._callback, > + qtype) > + if err != 0: > + req.state = ReqState.ERROR > + return None > + > + req.state = ReqState.PENDING > + return None > + > + @dns_enabled > + def resolve(self, name: str) -> typing.Optional[str]: > + """Resolve a host name to an IP address > + > + If the resolver is set to handle requests asynchronously, resolve() > + should be recalled until it returns a non-None result. Errors will be > + logged. > + > + :param name: The host name to resolve > + :returns: The IP address or None on error or not (yet) found > + """ > + if not self._is_daemon: > + return self._resolve_sync(name) > + retval = self._ctx.process() > + if retval != 0: > + vlog.err(f"dns-resolve error: {unbound.ub_strerror(retval)}") > + return None > + req = self._requests[name] # Creates a DNSRequest if not found > + if req.is_valid: > + return req.result > + elif req.state != ReqState.PENDING: > + self._resolve_async(req, unbound.RR_TYPE_A) > + return None > + > + > +def resolve(name: str) -> typing.Optional[str]: > + """Resolve a host name to an IP address > + > + If a DNSResolver instance has not been instantiated, or if it has been > + created with is_daemon=False, resolve() will synchronously resolve the > + hostname. If DNSResolver has been initialized with is_daemon=True, it > + will instead resolve asynchornously and resolve() will return None until > + the hostname has been resolved. > + > + :param name: The host name to resolve > + :returns: The IP address or None on error or not (yet) found > + """ > + > + return DNSResolver().resolve(name) > diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py > index 7b41dc44b..a26298b75 100644 > --- a/python/ovs/socket_util.py > +++ b/python/ovs/socket_util.py > @@ -13,12 +13,14 @@ > # limitations under the License. > > import errno > +import ipaddress > import os > import os.path > import random > import socket > import sys > > +from ovs import dns_resolve > import ovs.fatal_signal > import ovs.poller > import ovs.vlog > @@ -216,7 +218,7 @@ def is_valid_ipv4_address(address): > return True > > > -def inet_parse_active(target, default_port): > +def _inet_parse_active(target, default_port): > address = target.split(":") > if len(address) >= 2: > host_name = ":".join(address[0:-1]).lstrip('[').rstrip(']') > @@ -229,9 +231,24 @@ def inet_parse_active(target, default_port): > host_name = address[0] > if not host_name: > raise ValueError("%s: bad peer name format" % target) > + try: > + host_name = str(ipaddress.ip_address(host_name)) > + except ValueError: > + host_name = dns_resolve.resolve(host_name) > + if not host_name: > + raise ValueError("%s: bad peer name format" % target) > return (host_name, port) > > > +def inet_parse_active(target, default_port, raises=True): > + try: > + return _inet_parse_active(target, default_port) > + except ValueError: > + if raises: > + raise > + return ("", default_port) > + > + > def inet_create_socket_active(style, address): > try: > is_addr_inet = is_valid_ipv4_address(address[0]) > @@ -262,7 +279,7 @@ def inet_connect_active(sock, address, family, dscp): > > > def inet_open_active(style, target, default_port, dscp): > - address = inet_parse_active(target, default_port) > + address = inet_parse_active(target, default_port, raises=False) > family, sock = inet_create_socket_active(style, address) > if sock is None: > return family, sock > diff --git a/python/ovs/stream.py b/python/ovs/stream.py > index b32341076..82fbb0d68 100644 > --- a/python/ovs/stream.py > +++ b/python/ovs/stream.py > @@ -784,7 +784,7 @@ class SSLStream(Stream): > > @staticmethod > def _open(suffix, dscp): > - address = ovs.socket_util.inet_parse_active(suffix, 0) > + address = ovs.socket_util.inet_parse_active(suffix, 0, raises=False) > family, sock = ovs.socket_util.inet_create_socket_active( > socket.SOCK_STREAM, address) > if sock is None: > diff --git a/python/ovs/tests/test_dns_resolve.py b/python/ovs/tests/test_dns_resolve.py > new file mode 100644 > index 000000000..fca8049b6 > --- /dev/null > +++ b/python/ovs/tests/test_dns_resolve.py > @@ -0,0 +1,280 @@ > +import contextlib > +import ipaddress > +import sys > +import time > +from unittest import mock > + > +import pytest > + > +from ovs import dns_resolve > +from ovs import socket_util > + > + > +skip_no_unbound = pytest.mark.skipif("unbound" not in dns_resolve.__dict__, > + reason="Unbound not installed") > + > +HOSTS = [("192.0.2.1", "fake.ip4.domain", "192.0.2.1"), > + ("2001:db8:2::1", "fake.ip6.domain", "2001:db8:2::1"), > + ("192.0.2.2", "fake.both.domain", "192.0.2.2"), > + ("2001:db8:2::2", "fake.both.domain", "192.0.2.2")] > + > + > +def _tmp_file(path, content): > + path.write_text(content) > + assert content == path.read_text() > + return path > + > + > +@pytest.fixture(params=[False, True], ids=["not_daemon", "daemon"]) > +def resolver_factory(monkeypatch, tmp_path, request): > + # Allow delaying the instantiation of the DNSResolver > + def resolver_factory(hosts=HOSTS): > + path = tmp_path / "hosts" > + content = "\n".join(f"{ip}\t{host}" for ip, host, _ in hosts) > + _tmp_file(path, content) > + > + with monkeypatch.context() as m: > + m.setenv("OVS_HOSTS_FILE", str(path)) > + # Test with both is_daemon False and True > + resolver = dns_resolve.DNSResolver(request.param) > + assert resolver._is_daemon == request.param > + return dns_resolve.DNSResolver(request.param) > + > + yield resolver_factory > + dns_resolve.DNSResolver._instance = None > + > + > +@contextlib.contextmanager > +def DNSResolver(*args, **kwargs): > + """Clean up after returning a dns_resolver.DNSResolver > + > + Since it is a singleton, and pytest runs all tests in the same process, > + we can't use dns_resolver.DNSResolver directly in these tests. This > + context manager will reset the singleton at the end of the with block. > + """ > + resolver = dns_resolve.DNSResolver(*args, **kwargs) > + try: > + yield resolver > + finally: > + dns_resolve.DNSResolver._instance = None > + > + > +@pytest.fixture > +def unbound_conf(tmp_path): > + path = tmp_path / "unbound.conf" > + content = """ > + server: > + verbosity: 1 > + """ > + return _tmp_file(path, content) > +k/ > + > +@pytest.fixture > +def resolv_conf(tmp_path): > + path = tmp_path / "resolv.conf" > + content = "nameserver 127.0.0.1" > + return _tmp_file(path, content) > + > + > +@pytest.fixture > +def hosts_file(tmp_path): > + path = tmp_path / "hosts" > + content = "127.0.0.1\tfakelocalhost.localdomain" > + return _tmp_file(path, content) > + > + > +@pytest.fixture > +def missing_file(tmp_path): > + f = tmp_path / "missing_file" > + assert not f.exists() > + return f > + > + > +@pytest.fixture(params=[False, True], ids=["with unbound", "without unbound"]) > +def missing_unbound(monkeypatch, request): > + if request.param: > + if "unbound" in dns_resolve.__dict__: > + monkeypatch.setitem(sys.modules, 'unbound', None) > + monkeypatch.delitem(dns_resolve.__dict__, "unbound") > + elif "unbound" not in dns_resolve.__dict__: > + pytest.skip("Unbound not installed") > + return request.param > + > + > +def test_missing_unbound(missing_unbound, resolver_factory): > + resolver = resolver_factory() # Dont fail even w/o unbound > + assert resolver.dns_enabled == (not missing_unbound) > + > + > +def test_DNSRequest_defaults(): > + req = dns_resolve.DNSRequest(HOSTS[0][1]) > + assert HOSTS[0][1] == req.name > + assert req.state == dns_resolve.ReqState.INVALID > + assert req.time == req.result == req.ttl is None > + assert str(req) > + > + > +def test_DNSResolver_singleton(): > + with DNSResolver(True) as r1: > + assert r1._is_daemon > + r2 = dns_resolve.DNSResolver(False) > + assert r1 == r2 > + assert r1._is_daemon > + > + > +def _resolve(resolver, host, fn=dns_resolve.resolve): > + """Handle sync/async lookups, giving up if more than 1 second has passed""" > + > + timeout = 1 > + start = time.time() > + name = fn(host) > + if resolver._is_daemon: > + while name is None: > + name = fn(host) > + if name: > + break > + time.sleep(0.01) > + end = time.time() > + if end - start > timeout: > + break > + if name: > + return name > + raise LookupError(f"{host} not found") > + > + > +@pytest.mark.parametrize("ip,host,expected", HOSTS) > +def test_resolve_addresses(missing_unbound, resolver_factory, ip, host, > + expected): > + resolver = resolver_factory() > + if missing_unbound: > + with pytest.raises(LookupError): > + _resolve(resolver, host) > + else: > + result = _resolve(resolver, host) > + assert ipaddress.ip_address(expected) == ipaddress.ip_address(result) > + > + > +def test_resolve_unknown_host(missing_unbound, resolver_factory): > + resolver = resolver_factory() > + with pytest.raises(LookupError): > + _resolve(resolver, "fake.notadomain") > + > + > +@skip_no_unbound > +def test_resolve_process_error(): > + with DNSResolver(True) as resolver: > + with mock.patch.object(resolver._ctx, "process", return_value=-1): > + assert resolver.resolve("fake.domain") is None > + > + > +@skip_no_unbound > +def test_resolve_resolve_error(): > + with DNSResolver(False) as resolver: > + with mock.patch.object(resolver._ctx, "resolve", > + return_value=(-1, None)): > + assert resolver.resolve("fake.domain") is None > + > + > +@skip_no_unbound > +def test_resolve_resolve_async_error(): > + with DNSResolver(True) as resolver: > + with mock.patch.object(resolver._ctx, "resolve_async", > + return_value=(-1, None)): > + with pytest.raises(LookupError): > + _resolve(resolver, "fake.domain") > + > + > +@pytest.mark.parametrize("file,raises", > + [(None, False), > + ("missing_file", dns_resolve.UnboundException), > + ("unbound_conf", False)]) > +def test_set_unbound_conf(monkeypatch, missing_unbound, resolver_factory, > + request, file, raises): > + if file: > + file = str(request.getfixturevalue(file)) > + monkeypatch.setenv("OVS_UNBOUND_CONF", file) > + resolver = resolver_factory() # Doesn't raise > + if missing_unbound: > + assert resolver._set_unbound_conf() is None > + return > + with mock.patch.object(resolver._ctx, "config", > + side_effect=resolver._ctx.config) as c: > + if raises: > + with pytest.raises(raises): > + resolver._set_unbound_conf() > + else: > + resolver._set_unbound_conf() > + if file: > + c.assert_called_once_with(file) > + else: > + c.assert_not_called() > + > + > +@pytest.mark.parametrize("file,raises", > + [(None, False), > + ("missing_file", dns_resolve.UnboundException), > + ("resolv_conf", False)]) > +def test_resolv_conf(monkeypatch, missing_unbound, resolver_factory, request, > + file, raises): > + if file: > + file = str(request.getfixturevalue(file)) > + monkeypatch.setenv("OVS_RESOLV_CONF", file) > + resolver = resolver_factory() # Doesn't raise > + if missing_unbound: > + assert resolver._set_resolv_conf() is None > + return > + with mock.patch.object(resolver._ctx, "resolvconf", > + side_effect=resolver._ctx.resolvconf) as c: > + if raises: > + with pytest.raises(raises): > + resolver._set_resolv_conf() > + else: > + resolver._set_resolv_conf() > + c.assert_called_once_with(file) > + > + > +@pytest.mark.parametrize("file,raises", > + [(None, False), > + ("missing_file", dns_resolve.UnboundException), > + ("hosts_file", False)]) > +def test_hosts(monkeypatch, missing_unbound, resolver_factory, request, file, > + raises): > + if file: > + file = str(request.getfixturevalue(file)) > + monkeypatch.setenv("OVS_HOSTS_FILE", file) > + resolver = resolver_factory() # Doesn't raise > + if missing_unbound: > + assert resolver._set_hosts_file() is None > + return > + with mock.patch.object(resolver._ctx, "hosts", > + side_effect=resolver._ctx.hosts) as c: > + if raises: > + with pytest.raises(raises): > + resolver._set_hosts_file() > + else: > + resolver._set_hosts_file() > + c.assert_called_once_with(file) > + > + > +def test_UnboundException(missing_unbound): > + with pytest.raises(dns_resolve.UnboundException): > + raise dns_resolve.UnboundException("Fake exception", -1) > + > + > +@skip_no_unbound > +@pytest.mark.parametrize("ip,host,expected", HOSTS) > +def test_inet_parse_active(resolver_factory, ip, host, expected): > + resolver = resolver_factory() > + > + def fn(name): > + # Return the same thing _resolve() would so we can call > + # this multiple times for the is_daemon=True case > + return socket_util.inet_parse_active(f"{name}:6640", 6640, > + raises=False)[0] or None > + > + # parsing IPs still works > + IP = _resolve(resolver, ip, fn) > + assert ipaddress.ip_address(ip) == ipaddress.ip_address(IP) > + # parsing hosts works > + IP = _resolve(resolver, host, fn) > + assert ipaddress.ip_address(IP) == ipaddress.ip_address(expected) > diff --git a/python/setup.py b/python/setup.py > index 27684c404..bcf832ce9 100644 > --- a/python/setup.py > +++ b/python/setup.py > @@ -99,8 +99,7 @@ setup_args = dict( > 'Topic :: System :: Networking', > 'License :: OSI Approved :: Apache Software License', > 'Programming Language :: Python :: 3', > - 'Programming Language :: Python :: 3.4', > - 'Programming Language :: Python :: 3.5', > + 'Programming Language :: Python :: 3.6', > ], > ext_modules=[setuptools.Extension("ovs._json", > / sources=["ovs/_json.c"], > @@ -110,7 +109,8 @@ setup_args = dict( > cmdclass={'build_ext': try_build_ext}, > install_requires=['sortedcontainers'], > extras_require={':sys_platform == "win32"': ['pywin32 >= 1.0'], > - 'flow': ['netaddr', 'pyparsing']}, > + 'flow': ['netaddr', 'pyparsing'], > + 'dns': ['unbound']}, > ) > > try: > diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in > index 44899c1ca..343a5716d 100644 > --- a/rhel/openvswitch-fedora.spec.in > +++ b/rhel/openvswitch-fedora.spec.in > @@ -113,7 +113,7 @@ Summary: Open vSwitch python3 bindings > License: ASL 2.0 > BuildArch: noarch > Requires: python3 > -Suggests: python3-netaddr python3-pyparsing > +Suggests: python3-netaddr python3-pyparsing python3-unbound > %{?python_provide:%python_provide python3-openvswitch = %{version}-%{release}} > > %description -n python3-openvswitch > diff --git a/tests/vlog.at b/tests/vlog.at > index 3e92e70a9..785014956 100644 > --- a/tests/vlog.at > +++ b/tests/vlog.at > @@ -385,6 +385,7 @@ AT_CHECK([APPCTL -t test-unixctl.py vlog/list], [0], [dnl > console syslog file > ------- ------ ------ > daemon info info info > +dns_resolve info info info > fatal-signal info info info > jsonrpc info info info > poller info info info > @@ -404,6 +405,7 @@ unixctl_server info info info > console syslog file > ------- ------ ------ > daemon info err dbg > +dns_resolve info info dbg > fatal-signal info info dbg > jsonrpc info info dbg > poller info info dbg
On 6/30/23 12:39, Adrian Moreno wrote: > > > On 6/14/23 23:07, 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 <twilson@redhat.com> >> --- >> .github/workflows/build-and-test.yml | 4 +- >> Documentation/intro/install/general.rst | 4 +- >> Documentation/intro/install/rhel.rst | 2 +- >> Documentation/intro/install/windows.rst | 2 +- >> NEWS | 4 +- >> debian/control.in | 1 + >> m4/openvswitch.m4 | 8 +- >> python/TODO.rst | 7 + >> python/automake.mk | 2 + >> python/ovs/dns_resolve.py | 272 +++++++++++++++++++++++ >> 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 + >> 16 files changed, 601 insertions(+), 18 deletions(-) >> create mode 100644 python/ovs/dns_resolve.py >> create mode 100644 python/ovs/tests/test_dns_resolve.py >> >> diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml >> index f66ab43b0..47d239f10 100644 >> --- a/.github/workflows/build-and-test.yml >> +++ b/.github/workflows/build-and-test.yml >> @@ -183,10 +183,10 @@ jobs: >> run: sudo apt update || true >> - name: install common dependencies >> run: sudo apt install -y ${{ env.dependencies }} >> - - name: install libunbound libunwind >> + - name: install libunbound libunwind python3-unbound >> # GitHub Actions doesn't have 32-bit versions of these libraries. >> if: matrix.m32 == '' >> - run: sudo apt install -y libunbound-dev libunwind-dev >> + run: sudo apt install -y libunbound-dev libunwind-dev python3-unbound >> - name: install 32-bit libraries >> if: matrix.m32 != '' >> run: sudo apt install -y gcc-multilib >> diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst >> index 42b5682fd..19e360d47 100644 >> --- a/Documentation/intro/install/general.rst >> +++ b/Documentation/intro/install/general.rst >> @@ -90,7 +90,7 @@ need the following software: >> If libcap-ng is installed, then Open vSwitch will automatically build with >> support for it. >> >> -- Python 3.4 or later. >> +- Python 3.6 or later. >> >> - Unbound library, from http://www.unbound.net, is optional but recommended if >> you want to enable ovs-vswitchd and other utilities to use DNS names when >> @@ -208,7 +208,7 @@ simply install and run Open vSwitch you require the following software: >> from iproute2 (part of all major distributions and available at >> https://wiki.linuxfoundation.org/networking/iproute2). >> >> -- Python 3.4 or later. >> +- Python 3.6 or later. >> >> On Linux you should ensure that ``/dev/urandom`` exists. To support TAP >> devices, you must also ensure that ``/dev/net/tun`` exists. >> diff --git a/Documentation/intro/install/rhel.rst b/Documentation/intro/install/rhel.rst >> index d1fc42021..f2151d890 100644 >> --- a/Documentation/intro/install/rhel.rst >> +++ b/Documentation/intro/install/rhel.rst >> @@ -92,7 +92,7 @@ Once that is completed, remove the file ``/tmp/ovs.spec``. >> If python3-sphinx package is not available in your version of RHEL, you can >> install it via pip with 'pip install sphinx'. >> >> -Open vSwitch requires python 3.4 or newer which is not available in older >> +Open vSwitch requires python 3.6 or newer which is not available in older >> distributions. In the case of RHEL 6.x and its derivatives, one option is >> to install python34 from `EPEL`_. >> >> diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst >> index 78f60f35a..fce099d5d 100644 >> --- a/Documentation/intro/install/windows.rst >> +++ b/Documentation/intro/install/windows.rst >> @@ -56,7 +56,7 @@ The following explains the steps in some detail. >> >> 'C:/MinGW /mingw'. >> >> -- Python 3.4 or later. >> +- Python 3.6 or later. >> >> Install the latest Python 3.x from python.org and verify that its path is >> part of Windows' PATH environment variable. >> diff --git a/NEWS b/NEWS >> index cfd466663..24c694b8f 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -36,7 +36,9 @@ Post-v3.1.0 >> process extra privileges when mapping physical interconnect memory. >> - SRv6 Tunnel Protocol >> * Added support for userspace datapath (only). >> - >> + - Python >> + * Added async DNS support >> + * Dropped support for Python < 3.6 >> >> v3.1.0 - 16 Feb 2023 >> -------------------- >> diff --git a/debian/control.in b/debian/control.in >> index 19f590d06..64b0a4ce0 100644 >> --- a/debian/control.in >> +++ b/debian/control.in >> @@ -287,6 +287,7 @@ Depends: >> Suggests: >> python3-netaddr, >> python3-pyparsing, >> + python3-unbound, >> Description: Python 3 bindings for Open vSwitch >> Open vSwitch is a production quality, multilayer, software-based, >> Ethernet virtual switch. It is designed to enable massive network >> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 >> index 14d9249b8..373a7e413 100644 >> --- a/m4/openvswitch.m4 >> +++ b/m4/openvswitch.m4 >> @@ -371,16 +371,16 @@ dnl Checks for valgrind/valgrind.h. >> AC_DEFUN([OVS_CHECK_VALGRIND], >> [AC_CHECK_HEADERS([valgrind/valgrind.h])]) >> >> -dnl Checks for Python 3.4 or later. >> +dnl Checks for Python 3.6 or later. >> AC_DEFUN([OVS_CHECK_PYTHON3], >> [AC_CACHE_CHECK( >> - [for Python 3 (version 3.4 or later)], >> + [for Python 3 (version 3.6 or later)], >> [ovs_cv_python3], >> [if test -n "$PYTHON3"; then >> ovs_cv_python3=$PYTHON3 >> else >> ovs_cv_python3=no >> - for binary in python3 python3.4 python3.5 python3.6 python3.7; do >> + for binary in python3{,.{6..12}}; do >> ovs_save_IFS=$IFS; IFS=$PATH_SEPARATOR >> for dir in $PATH; do >> IFS=$ovs_save_IFS >> @@ -397,7 +397,7 @@ else: >> done >> fi]) >> if test "$ovs_cv_python3" = no; then >> - AC_MSG_ERROR([Python 3.4 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) >> + AC_MSG_ERROR([Python 3.6 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) >> fi >> AC_ARG_VAR([PYTHON3]) >> PYTHON3=$ovs_cv_python3]) >> diff --git a/python/TODO.rst b/python/TODO.rst >> index 3a53489f1..acc5461e2 100644 >> --- a/python/TODO.rst >> +++ b/python/TODO.rst >> @@ -32,3 +32,10 @@ Python Bindings To-do List >> >> * Support write-only-changed monitor mode (equivalent of >> OVSDB_IDL_WRITE_CHANGED_ONLY). >> + >> +* socket_util: >> + >> + * Add equivalent fuctions to inet_parse_passive, parse_sockaddr_components, >> + et al. to better support using async dns. The reconnect code will >> + currently log a warning when inet_parse_active() returns w/o yet having >> + resolved an address, but will continue to connect and eventually succeed. >> diff --git a/python/automake.mk b/python/automake.mk >> index d00911828..82a508787 100644 >> --- a/python/automake.mk >> +++ b/python/automake.mk >> @@ -16,6 +16,7 @@ ovs_pyfiles = \ >> python/ovs/compat/sortedcontainers/sorteddict.py \ >> python/ovs/compat/sortedcontainers/sortedset.py \ >> python/ovs/daemon.py \ >> + python/ovs/dns_resolve.py \ >> python/ovs/db/__init__.py \ >> python/ovs/db/custom_index.py \ >> python/ovs/db/data.py \ >> @@ -55,6 +56,7 @@ ovs_pyfiles = \ >> >> ovs_pytests = \ >> python/ovs/tests/test_decoders.py \ >> + python/ovs/tests/test_dns_resolve.py \ >> python/ovs/tests/test_filter.py \ >> python/ovs/tests/test_kv.py \ >> python/ovs/tests/test_list.py \ >> diff --git a/python/ovs/dns_resolve.py b/python/ovs/dns_resolve.py >> new file mode 100644 >> index 000000000..7b0f6c266 >> --- /dev/null >> +++ b/python/ovs/dns_resolve.py >> @@ -0,0 +1,272 @@ >> +# Copyright (c) 2023 Red Hat, Inc. >> +# >> +# Licensed under the Apache License, Version 2.0 (the "License"); >> +# you may not use this file except in compliance with the License. >> +# You may obtain a copy of the License at: >> +# >> +# http://www.apache.org/licenses/LICENSE-2.0 >> +# >> +# Unless required by applicable law or agreed to in writing, software >> +# distributed under the License is distributed on an "AS IS" BASIS, >> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. >> +# See the License for the specific language governing permissions and >> +# limitations under the License. >> + >> +import collections >> +import enum >> +import functools >> +import ipaddress >> +import os >> +import time >> +import typing >> + >> +try: >> + import unbound # type: ignore >> +except ImportError: >> + pass >> + >> +import ovs.vlog >> + >> +vlog = ovs.vlog.Vlog("dns_resolve") >> + >> + >> +class ReqState(enum.Enum): >> + INVALID = 0 >> + PENDING = 1 >> + GOOD = 2 >> + ERROR = 3 >> + >> + >> +class DNSRequest: >> + > > nit: extra space > >> + def __init__(self, name: str): >> + self.name: str = name >> + self.state: ReqState = ReqState.INVALID >> + self.time: typing.Optional[float] = None >> + # set by DNSResolver._callback >> + self.result: typing.Optional[str] = None >> + self.ttl: typing.Optional[float] = None >> + >> + @property >> + def expired(self): >> + return time.time() > self.time + self.ttl >> + >> + @property >> + def is_valid(self): >> + return self.state == ReqState.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 >> + > > Having a singleton class that accepts a parameter in the constructor could be a > bit confusing. For instance: "d = DNSResolver(is_daemon=True)" > might yield a resolver where "is_daemon" is actually "False" if some other part > of the code created it before. > > Woudn't it be cleaner if it just exposed "resove_sync" and "resolve_async" and > let users choose how to resolve? I suppose, the problem is that most of the resolving is happening from the inside of OVS libraries. So, it will not be a choice of an application. Maybe a warning that the parameter is different will be enough? Best regards, Ilya Maximets.
On 6/30/23 13:40, Ilya Maximets wrote: > On 6/30/23 12:39, Adrian Moreno wrote: >> >> >> On 6/14/23 23:07, 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 <twilson@redhat.com> >>> --- >>> .github/workflows/build-and-test.yml | 4 +- >>> Documentation/intro/install/general.rst | 4 +- >>> Documentation/intro/install/rhel.rst | 2 +- >>> Documentation/intro/install/windows.rst | 2 +- >>> NEWS | 4 +- >>> debian/control.in | 1 + >>> m4/openvswitch.m4 | 8 +- >>> python/TODO.rst | 7 + >>> python/automake.mk | 2 + >>> python/ovs/dns_resolve.py | 272 +++++++++++++++++++++++ >>> 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 + >>> 16 files changed, 601 insertions(+), 18 deletions(-) >>> create mode 100644 python/ovs/dns_resolve.py >>> create mode 100644 python/ovs/tests/test_dns_resolve.py >>> >>> diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml >>> index f66ab43b0..47d239f10 100644 >>> --- a/.github/workflows/build-and-test.yml >>> +++ b/.github/workflows/build-and-test.yml >>> @@ -183,10 +183,10 @@ jobs: >>> run: sudo apt update || true >>> - name: install common dependencies >>> run: sudo apt install -y ${{ env.dependencies }} >>> - - name: install libunbound libunwind >>> + - name: install libunbound libunwind python3-unbound >>> # GitHub Actions doesn't have 32-bit versions of these libraries. >>> if: matrix.m32 == '' >>> - run: sudo apt install -y libunbound-dev libunwind-dev >>> + run: sudo apt install -y libunbound-dev libunwind-dev python3-unbound >>> - name: install 32-bit libraries >>> if: matrix.m32 != '' >>> run: sudo apt install -y gcc-multilib >>> diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst >>> index 42b5682fd..19e360d47 100644 >>> --- a/Documentation/intro/install/general.rst >>> +++ b/Documentation/intro/install/general.rst >>> @@ -90,7 +90,7 @@ need the following software: >>> If libcap-ng is installed, then Open vSwitch will automatically build with >>> support for it. >>> >>> -- Python 3.4 or later. >>> +- Python 3.6 or later. >>> >>> - Unbound library, from http://www.unbound.net, is optional but recommended if >>> you want to enable ovs-vswitchd and other utilities to use DNS names when >>> @@ -208,7 +208,7 @@ simply install and run Open vSwitch you require the following software: >>> from iproute2 (part of all major distributions and available at >>> https://wiki.linuxfoundation.org/networking/iproute2). >>> >>> -- Python 3.4 or later. >>> +- Python 3.6 or later. >>> >>> On Linux you should ensure that ``/dev/urandom`` exists. To support TAP >>> devices, you must also ensure that ``/dev/net/tun`` exists. >>> diff --git a/Documentation/intro/install/rhel.rst b/Documentation/intro/install/rhel.rst >>> index d1fc42021..f2151d890 100644 >>> --- a/Documentation/intro/install/rhel.rst >>> +++ b/Documentation/intro/install/rhel.rst >>> @@ -92,7 +92,7 @@ Once that is completed, remove the file ``/tmp/ovs.spec``. >>> If python3-sphinx package is not available in your version of RHEL, you can >>> install it via pip with 'pip install sphinx'. >>> >>> -Open vSwitch requires python 3.4 or newer which is not available in older >>> +Open vSwitch requires python 3.6 or newer which is not available in older >>> distributions. In the case of RHEL 6.x and its derivatives, one option is >>> to install python34 from `EPEL`_. >>> >>> diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst >>> index 78f60f35a..fce099d5d 100644 >>> --- a/Documentation/intro/install/windows.rst >>> +++ b/Documentation/intro/install/windows.rst >>> @@ -56,7 +56,7 @@ The following explains the steps in some detail. >>> >>> 'C:/MinGW /mingw'. >>> >>> -- Python 3.4 or later. >>> +- Python 3.6 or later. >>> >>> Install the latest Python 3.x from python.org and verify that its path is >>> part of Windows' PATH environment variable. >>> diff --git a/NEWS b/NEWS >>> index cfd466663..24c694b8f 100644 >>> --- a/NEWS >>> +++ b/NEWS >>> @@ -36,7 +36,9 @@ Post-v3.1.0 >>> process extra privileges when mapping physical interconnect memory. >>> - SRv6 Tunnel Protocol >>> * Added support for userspace datapath (only). >>> - >>> + - Python >>> + * Added async DNS support >>> + * Dropped support for Python < 3.6 >>> >>> v3.1.0 - 16 Feb 2023 >>> -------------------- >>> diff --git a/debian/control.in b/debian/control.in >>> index 19f590d06..64b0a4ce0 100644 >>> --- a/debian/control.in >>> +++ b/debian/control.in >>> @@ -287,6 +287,7 @@ Depends: >>> Suggests: >>> python3-netaddr, >>> python3-pyparsing, >>> + python3-unbound, >>> Description: Python 3 bindings for Open vSwitch >>> Open vSwitch is a production quality, multilayer, software-based, >>> Ethernet virtual switch. It is designed to enable massive network >>> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 >>> index 14d9249b8..373a7e413 100644 >>> --- a/m4/openvswitch.m4 >>> +++ b/m4/openvswitch.m4 >>> @@ -371,16 +371,16 @@ dnl Checks for valgrind/valgrind.h. >>> AC_DEFUN([OVS_CHECK_VALGRIND], >>> [AC_CHECK_HEADERS([valgrind/valgrind.h])]) >>> >>> -dnl Checks for Python 3.4 or later. >>> +dnl Checks for Python 3.6 or later. >>> AC_DEFUN([OVS_CHECK_PYTHON3], >>> [AC_CACHE_CHECK( >>> - [for Python 3 (version 3.4 or later)], >>> + [for Python 3 (version 3.6 or later)], >>> [ovs_cv_python3], >>> [if test -n "$PYTHON3"; then >>> ovs_cv_python3=$PYTHON3 >>> else >>> ovs_cv_python3=no >>> - for binary in python3 python3.4 python3.5 python3.6 python3.7; do >>> + for binary in python3{,.{6..12}}; do >>> ovs_save_IFS=$IFS; IFS=$PATH_SEPARATOR >>> for dir in $PATH; do >>> IFS=$ovs_save_IFS >>> @@ -397,7 +397,7 @@ else: >>> done >>> fi]) >>> if test "$ovs_cv_python3" = no; then >>> - AC_MSG_ERROR([Python 3.4 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) >>> + AC_MSG_ERROR([Python 3.6 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) >>> fi >>> AC_ARG_VAR([PYTHON3]) >>> PYTHON3=$ovs_cv_python3]) >>> diff --git a/python/TODO.rst b/python/TODO.rst >>> index 3a53489f1..acc5461e2 100644 >>> --- a/python/TODO.rst >>> +++ b/python/TODO.rst >>> @@ -32,3 +32,10 @@ Python Bindings To-do List >>> >>> * Support write-only-changed monitor mode (equivalent of >>> OVSDB_IDL_WRITE_CHANGED_ONLY). >>> + >>> +* socket_util: >>> + >>> + * Add equivalent fuctions to inet_parse_passive, parse_sockaddr_components, >>> + et al. to better support using async dns. The reconnect code will >>> + currently log a warning when inet_parse_active() returns w/o yet having >>> + resolved an address, but will continue to connect and eventually succeed. >>> diff --git a/python/automake.mk b/python/automake.mk >>> index d00911828..82a508787 100644 >>> --- a/python/automake.mk >>> +++ b/python/automake.mk >>> @@ -16,6 +16,7 @@ ovs_pyfiles = \ >>> python/ovs/compat/sortedcontainers/sorteddict.py \ >>> python/ovs/compat/sortedcontainers/sortedset.py \ >>> python/ovs/daemon.py \ >>> + python/ovs/dns_resolve.py \ >>> python/ovs/db/__init__.py \ >>> python/ovs/db/custom_index.py \ >>> python/ovs/db/data.py \ >>> @@ -55,6 +56,7 @@ ovs_pyfiles = \ >>> >>> ovs_pytests = \ >>> python/ovs/tests/test_decoders.py \ >>> + python/ovs/tests/test_dns_resolve.py \ >>> python/ovs/tests/test_filter.py \ >>> python/ovs/tests/test_kv.py \ >>> python/ovs/tests/test_list.py \ >>> diff --git a/python/ovs/dns_resolve.py b/python/ovs/dns_resolve.py >>> new file mode 100644 >>> index 000000000..7b0f6c266 >>> --- /dev/null >>> +++ b/python/ovs/dns_resolve.py >>> @@ -0,0 +1,272 @@ >>> +# Copyright (c) 2023 Red Hat, Inc. >>> +# >>> +# Licensed under the Apache License, Version 2.0 (the "License"); >>> +# you may not use this file except in compliance with the License. >>> +# You may obtain a copy of the License at: >>> +# >>> +# http://www.apache.org/licenses/LICENSE-2.0 >>> +# >>> +# Unless required by applicable law or agreed to in writing, software >>> +# distributed under the License is distributed on an "AS IS" BASIS, >>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. >>> +# See the License for the specific language governing permissions and >>> +# limitations under the License. >>> + >>> +import collections >>> +import enum >>> +import functools >>> +import ipaddress >>> +import os >>> +import time >>> +import typing >>> + >>> +try: >>> + import unbound # type: ignore >>> +except ImportError: >>> + pass >>> + >>> +import ovs.vlog >>> + >>> +vlog = ovs.vlog.Vlog("dns_resolve") >>> + >>> + >>> +class ReqState(enum.Enum): >>> + INVALID = 0 >>> + PENDING = 1 >>> + GOOD = 2 >>> + ERROR = 3 >>> + >>> + >>> +class DNSRequest: >>> + >> >> nit: extra space >> >>> + def __init__(self, name: str): >>> + self.name: str = name >>> + self.state: ReqState = ReqState.INVALID >>> + self.time: typing.Optional[float] = None >>> + # set by DNSResolver._callback >>> + self.result: typing.Optional[str] = None >>> + self.ttl: typing.Optional[float] = None >>> + >>> + @property >>> + def expired(self): >>> + return time.time() > self.time + self.ttl >>> + >>> + @property >>> + def is_valid(self): >>> + return self.state == ReqState.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 >>> + >> >> Having a singleton class that accepts a parameter in the constructor could be a >> bit confusing. For instance: "d = DNSResolver(is_daemon=True)" >> might yield a resolver where "is_daemon" is actually "False" if some other part >> of the code created it before. >> >> Woudn't it be cleaner if it just exposed "resove_sync" and "resolve_async" and >> let users choose how to resolve? > > I suppose, the problem is that most of the resolving is happening > from the inside of OVS libraries. So, it will not be a choice of > an application. > Not sure I understand. Currently if a user does: 'DNSResolver(is_daemon=False).resolve("openvswitch.org")', it'll resolve synchronously and so will all the rest of the resolutions of that program. The internal variable "_is_daemon" is only used to select the way to resolve. The same behavior would be achieved if, for instance, "is_daemon" is a default argument to "resolve()" with the extra benefit of the first resolution not influencing all the rest. > Maybe a warning that the parameter is different will be enough? > > Best regards, Ilya Maximets. >
On 6/30/23 14:23, Adrian Moreno wrote: > > > On 6/30/23 13:40, Ilya Maximets wrote: >> On 6/30/23 12:39, Adrian Moreno wrote: >>> >>> >>> On 6/14/23 23:07, 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 <twilson@redhat.com> >>>> --- >>>> .github/workflows/build-and-test.yml | 4 +- >>>> Documentation/intro/install/general.rst | 4 +- >>>> Documentation/intro/install/rhel.rst | 2 +- >>>> Documentation/intro/install/windows.rst | 2 +- >>>> NEWS | 4 +- >>>> debian/control.in | 1 + >>>> m4/openvswitch.m4 | 8 +- >>>> python/TODO.rst | 7 + >>>> python/automake.mk | 2 + >>>> python/ovs/dns_resolve.py | 272 +++++++++++++++++++++++ >>>> 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 + >>>> 16 files changed, 601 insertions(+), 18 deletions(-) >>>> create mode 100644 python/ovs/dns_resolve.py >>>> create mode 100644 python/ovs/tests/test_dns_resolve.py >>>> >>>> diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml >>>> index f66ab43b0..47d239f10 100644 >>>> --- a/.github/workflows/build-and-test.yml >>>> +++ b/.github/workflows/build-and-test.yml >>>> @@ -183,10 +183,10 @@ jobs: >>>> run: sudo apt update || true >>>> - name: install common dependencies >>>> run: sudo apt install -y ${{ env.dependencies }} >>>> - - name: install libunbound libunwind >>>> + - name: install libunbound libunwind python3-unbound >>>> # GitHub Actions doesn't have 32-bit versions of these libraries. >>>> if: matrix.m32 == '' >>>> - run: sudo apt install -y libunbound-dev libunwind-dev >>>> + run: sudo apt install -y libunbound-dev libunwind-dev python3-unbound >>>> - name: install 32-bit libraries >>>> if: matrix.m32 != '' >>>> run: sudo apt install -y gcc-multilib >>>> diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst >>>> index 42b5682fd..19e360d47 100644 >>>> --- a/Documentation/intro/install/general.rst >>>> +++ b/Documentation/intro/install/general.rst >>>> @@ -90,7 +90,7 @@ need the following software: >>>> If libcap-ng is installed, then Open vSwitch will automatically build with >>>> support for it. >>>> >>>> -- Python 3.4 or later. >>>> +- Python 3.6 or later. >>>> >>>> - Unbound library, from http://www.unbound.net, is optional but recommended if >>>> you want to enable ovs-vswitchd and other utilities to use DNS names when >>>> @@ -208,7 +208,7 @@ simply install and run Open vSwitch you require the following software: >>>> from iproute2 (part of all major distributions and available at >>>> https://wiki.linuxfoundation.org/networking/iproute2). >>>> >>>> -- Python 3.4 or later. >>>> +- Python 3.6 or later. >>>> >>>> On Linux you should ensure that ``/dev/urandom`` exists. To support TAP >>>> devices, you must also ensure that ``/dev/net/tun`` exists. >>>> diff --git a/Documentation/intro/install/rhel.rst b/Documentation/intro/install/rhel.rst >>>> index d1fc42021..f2151d890 100644 >>>> --- a/Documentation/intro/install/rhel.rst >>>> +++ b/Documentation/intro/install/rhel.rst >>>> @@ -92,7 +92,7 @@ Once that is completed, remove the file ``/tmp/ovs.spec``. >>>> If python3-sphinx package is not available in your version of RHEL, you can >>>> install it via pip with 'pip install sphinx'. >>>> >>>> -Open vSwitch requires python 3.4 or newer which is not available in older >>>> +Open vSwitch requires python 3.6 or newer which is not available in older >>>> distributions. In the case of RHEL 6.x and its derivatives, one option is >>>> to install python34 from `EPEL`_. >>>> >>>> diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst >>>> index 78f60f35a..fce099d5d 100644 >>>> --- a/Documentation/intro/install/windows.rst >>>> +++ b/Documentation/intro/install/windows.rst >>>> @@ -56,7 +56,7 @@ The following explains the steps in some detail. >>>> >>>> 'C:/MinGW /mingw'. >>>> >>>> -- Python 3.4 or later. >>>> +- Python 3.6 or later. >>>> >>>> Install the latest Python 3.x from python.org and verify that its path is >>>> part of Windows' PATH environment variable. >>>> diff --git a/NEWS b/NEWS >>>> index cfd466663..24c694b8f 100644 >>>> --- a/NEWS >>>> +++ b/NEWS >>>> @@ -36,7 +36,9 @@ Post-v3.1.0 >>>> process extra privileges when mapping physical interconnect memory. >>>> - SRv6 Tunnel Protocol >>>> * Added support for userspace datapath (only). >>>> - >>>> + - Python >>>> + * Added async DNS support >>>> + * Dropped support for Python < 3.6 >>>> >>>> v3.1.0 - 16 Feb 2023 >>>> -------------------- >>>> diff --git a/debian/control.in b/debian/control.in >>>> index 19f590d06..64b0a4ce0 100644 >>>> --- a/debian/control.in >>>> +++ b/debian/control.in >>>> @@ -287,6 +287,7 @@ Depends: >>>> Suggests: >>>> python3-netaddr, >>>> python3-pyparsing, >>>> + python3-unbound, >>>> Description: Python 3 bindings for Open vSwitch >>>> Open vSwitch is a production quality, multilayer, software-based, >>>> Ethernet virtual switch. It is designed to enable massive network >>>> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 >>>> index 14d9249b8..373a7e413 100644 >>>> --- a/m4/openvswitch.m4 >>>> +++ b/m4/openvswitch.m4 >>>> @@ -371,16 +371,16 @@ dnl Checks for valgrind/valgrind.h. >>>> AC_DEFUN([OVS_CHECK_VALGRIND], >>>> [AC_CHECK_HEADERS([valgrind/valgrind.h])]) >>>> >>>> -dnl Checks for Python 3.4 or later. >>>> +dnl Checks for Python 3.6 or later. >>>> AC_DEFUN([OVS_CHECK_PYTHON3], >>>> [AC_CACHE_CHECK( >>>> - [for Python 3 (version 3.4 or later)], >>>> + [for Python 3 (version 3.6 or later)], >>>> [ovs_cv_python3], >>>> [if test -n "$PYTHON3"; then >>>> ovs_cv_python3=$PYTHON3 >>>> else >>>> ovs_cv_python3=no >>>> - for binary in python3 python3.4 python3.5 python3.6 python3.7; do >>>> + for binary in python3{,.{6..12}}; do >>>> ovs_save_IFS=$IFS; IFS=$PATH_SEPARATOR >>>> for dir in $PATH; do >>>> IFS=$ovs_save_IFS >>>> @@ -397,7 +397,7 @@ else: >>>> done >>>> fi]) >>>> if test "$ovs_cv_python3" = no; then >>>> - AC_MSG_ERROR([Python 3.4 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) >>>> + AC_MSG_ERROR([Python 3.6 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) >>>> fi >>>> AC_ARG_VAR([PYTHON3]) >>>> PYTHON3=$ovs_cv_python3]) >>>> diff --git a/python/TODO.rst b/python/TODO.rst >>>> index 3a53489f1..acc5461e2 100644 >>>> --- a/python/TODO.rst >>>> +++ b/python/TODO.rst >>>> @@ -32,3 +32,10 @@ Python Bindings To-do List >>>> >>>> * Support write-only-changed monitor mode (equivalent of >>>> OVSDB_IDL_WRITE_CHANGED_ONLY). >>>> + >>>> +* socket_util: >>>> + >>>> + * Add equivalent fuctions to inet_parse_passive, parse_sockaddr_components, >>>> + et al. to better support using async dns. The reconnect code will >>>> + currently log a warning when inet_parse_active() returns w/o yet having >>>> + resolved an address, but will continue to connect and eventually succeed. >>>> diff --git a/python/automake.mk b/python/automake.mk >>>> index d00911828..82a508787 100644 >>>> --- a/python/automake.mk >>>> +++ b/python/automake.mk >>>> @@ -16,6 +16,7 @@ ovs_pyfiles = \ >>>> python/ovs/compat/sortedcontainers/sorteddict.py \ >>>> python/ovs/compat/sortedcontainers/sortedset.py \ >>>> python/ovs/daemon.py \ >>>> + python/ovs/dns_resolve.py \ >>>> python/ovs/db/__init__.py \ >>>> python/ovs/db/custom_index.py \ >>>> python/ovs/db/data.py \ >>>> @@ -55,6 +56,7 @@ ovs_pyfiles = \ >>>> >>>> ovs_pytests = \ >>>> python/ovs/tests/test_decoders.py \ >>>> + python/ovs/tests/test_dns_resolve.py \ >>>> python/ovs/tests/test_filter.py \ >>>> python/ovs/tests/test_kv.py \ >>>> python/ovs/tests/test_list.py \ >>>> diff --git a/python/ovs/dns_resolve.py b/python/ovs/dns_resolve.py >>>> new file mode 100644 >>>> index 000000000..7b0f6c266 >>>> --- /dev/null >>>> +++ b/python/ovs/dns_resolve.py >>>> @@ -0,0 +1,272 @@ >>>> +# Copyright (c) 2023 Red Hat, Inc. >>>> +# >>>> +# Licensed under the Apache License, Version 2.0 (the "License"); >>>> +# you may not use this file except in compliance with the License. >>>> +# You may obtain a copy of the License at: >>>> +# >>>> +# http://www.apache.org/licenses/LICENSE-2.0 >>>> +# >>>> +# Unless required by applicable law or agreed to in writing, software >>>> +# distributed under the License is distributed on an "AS IS" BASIS, >>>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. >>>> +# See the License for the specific language governing permissions and >>>> +# limitations under the License. >>>> + >>>> +import collections >>>> +import enum >>>> +import functools >>>> +import ipaddress >>>> +import os >>>> +import time >>>> +import typing >>>> + >>>> +try: >>>> + import unbound # type: ignore >>>> +except ImportError: >>>> + pass >>>> + >>>> +import ovs.vlog >>>> + >>>> +vlog = ovs.vlog.Vlog("dns_resolve") >>>> + >>>> + >>>> +class ReqState(enum.Enum): >>>> + INVALID = 0 >>>> + PENDING = 1 >>>> + GOOD = 2 >>>> + ERROR = 3 >>>> + >>>> + >>>> +class DNSRequest: >>>> + >>> >>> nit: extra space >>> >>>> + def __init__(self, name: str): >>>> + self.name: str = name >>>> + self.state: ReqState = ReqState.INVALID >>>> + self.time: typing.Optional[float] = None >>>> + # set by DNSResolver._callback >>>> + self.result: typing.Optional[str] = None >>>> + self.ttl: typing.Optional[float] = None >>>> + >>>> + @property >>>> + def expired(self): >>>> + return time.time() > self.time + self.ttl >>>> + >>>> + @property >>>> + def is_valid(self): >>>> + return self.state == ReqState.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 >>>> + >>> >>> Having a singleton class that accepts a parameter in the constructor could be a >>> bit confusing. For instance: "d = DNSResolver(is_daemon=True)" >>> might yield a resolver where "is_daemon" is actually "False" if some other part >>> of the code created it before. >>> >>> Woudn't it be cleaner if it just exposed "resove_sync" and "resolve_async" and >>> let users choose how to resolve? >> >> I suppose, the problem is that most of the resolving is happening >> from the inside of OVS libraries. So, it will not be a choice of >> an application. >> > > Not sure I understand. Currently if a user does: > 'DNSResolver(is_daemon=False).resolve("openvswitch.org")', it'll resolve > synchronously and so will all the rest of the resolutions of that program. > > The internal variable "_is_daemon" is only used to select the way to resolve. > The same behavior would be achieved if, for instance, "is_daemon" is a default > argument to "resolve()" with the extra benefit of the first resolution not > influencing all the rest. My point is that inet_parse_active() is calling: host_name = dns_resolve.resolve(host_name) And we will have to add an argument to this call in order to give user a choice to perform sync or async resolution. As well as add this argument to all the functions that may call inet_parse_active() and all the functions that may call these functions. To avoid that we have a global "is_daemon" state. Does that make sense? Maybe we could remove the class paramater and add a function like dns_resolve.enable_async() instead, so the global state change will be explicit? > >> Maybe a warning that the parameter is different will be enough? >>> Best regards, Ilya Maximets. >> >
On 6/30/23 14:35, Ilya Maximets wrote: > On 6/30/23 14:23, Adrian Moreno wrote: >> >> >> On 6/30/23 13:40, Ilya Maximets wrote: >>> On 6/30/23 12:39, Adrian Moreno wrote: >>>> >>>> >>>> On 6/14/23 23:07, 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 <twilson@redhat.com> >>>>> --- >>>>> .github/workflows/build-and-test.yml | 4 +- >>>>> Documentation/intro/install/general.rst | 4 +- >>>>> Documentation/intro/install/rhel.rst | 2 +- >>>>> Documentation/intro/install/windows.rst | 2 +- >>>>> NEWS | 4 +- >>>>> debian/control.in | 1 + >>>>> m4/openvswitch.m4 | 8 +- >>>>> python/TODO.rst | 7 + >>>>> python/automake.mk | 2 + >>>>> python/ovs/dns_resolve.py | 272 +++++++++++++++++++++++ >>>>> 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 + >>>>> 16 files changed, 601 insertions(+), 18 deletions(-) >>>>> create mode 100644 python/ovs/dns_resolve.py >>>>> create mode 100644 python/ovs/tests/test_dns_resolve.py >>>>> >>>>> diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml >>>>> index f66ab43b0..47d239f10 100644 >>>>> --- a/.github/workflows/build-and-test.yml >>>>> +++ b/.github/workflows/build-and-test.yml >>>>> @@ -183,10 +183,10 @@ jobs: >>>>> run: sudo apt update || true >>>>> - name: install common dependencies >>>>> run: sudo apt install -y ${{ env.dependencies }} >>>>> - - name: install libunbound libunwind >>>>> + - name: install libunbound libunwind python3-unbound >>>>> # GitHub Actions doesn't have 32-bit versions of these libraries. >>>>> if: matrix.m32 == '' >>>>> - run: sudo apt install -y libunbound-dev libunwind-dev >>>>> + run: sudo apt install -y libunbound-dev libunwind-dev python3-unbound >>>>> - name: install 32-bit libraries >>>>> if: matrix.m32 != '' >>>>> run: sudo apt install -y gcc-multilib >>>>> diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst >>>>> index 42b5682fd..19e360d47 100644 >>>>> --- a/Documentation/intro/install/general.rst >>>>> +++ b/Documentation/intro/install/general.rst >>>>> @@ -90,7 +90,7 @@ need the following software: >>>>> If libcap-ng is installed, then Open vSwitch will automatically build with >>>>> support for it. >>>>> >>>>> -- Python 3.4 or later. >>>>> +- Python 3.6 or later. >>>>> >>>>> - Unbound library, from http://www.unbound.net, is optional but recommended if >>>>> you want to enable ovs-vswitchd and other utilities to use DNS names when >>>>> @@ -208,7 +208,7 @@ simply install and run Open vSwitch you require the following software: >>>>> from iproute2 (part of all major distributions and available at >>>>> https://wiki.linuxfoundation.org/networking/iproute2). >>>>> >>>>> -- Python 3.4 or later. >>>>> +- Python 3.6 or later. >>>>> >>>>> On Linux you should ensure that ``/dev/urandom`` exists. To support TAP >>>>> devices, you must also ensure that ``/dev/net/tun`` exists. >>>>> diff --git a/Documentation/intro/install/rhel.rst b/Documentation/intro/install/rhel.rst >>>>> index d1fc42021..f2151d890 100644 >>>>> --- a/Documentation/intro/install/rhel.rst >>>>> +++ b/Documentation/intro/install/rhel.rst >>>>> @@ -92,7 +92,7 @@ Once that is completed, remove the file ``/tmp/ovs.spec``. >>>>> If python3-sphinx package is not available in your version of RHEL, you can >>>>> install it via pip with 'pip install sphinx'. >>>>> >>>>> -Open vSwitch requires python 3.4 or newer which is not available in older >>>>> +Open vSwitch requires python 3.6 or newer which is not available in older >>>>> distributions. In the case of RHEL 6.x and its derivatives, one option is >>>>> to install python34 from `EPEL`_. >>>>> >>>>> diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst >>>>> index 78f60f35a..fce099d5d 100644 >>>>> --- a/Documentation/intro/install/windows.rst >>>>> +++ b/Documentation/intro/install/windows.rst >>>>> @@ -56,7 +56,7 @@ The following explains the steps in some detail. >>>>> >>>>> 'C:/MinGW /mingw'. >>>>> >>>>> -- Python 3.4 or later. >>>>> +- Python 3.6 or later. >>>>> >>>>> Install the latest Python 3.x from python.org and verify that its path is >>>>> part of Windows' PATH environment variable. >>>>> diff --git a/NEWS b/NEWS >>>>> index cfd466663..24c694b8f 100644 >>>>> --- a/NEWS >>>>> +++ b/NEWS >>>>> @@ -36,7 +36,9 @@ Post-v3.1.0 >>>>> process extra privileges when mapping physical interconnect memory. >>>>> - SRv6 Tunnel Protocol >>>>> * Added support for userspace datapath (only). >>>>> - >>>>> + - Python >>>>> + * Added async DNS support >>>>> + * Dropped support for Python < 3.6 >>>>> >>>>> v3.1.0 - 16 Feb 2023 >>>>> -------------------- >>>>> diff --git a/debian/control.in b/debian/control.in >>>>> index 19f590d06..64b0a4ce0 100644 >>>>> --- a/debian/control.in >>>>> +++ b/debian/control.in >>>>> @@ -287,6 +287,7 @@ Depends: >>>>> Suggests: >>>>> python3-netaddr, >>>>> python3-pyparsing, >>>>> + python3-unbound, >>>>> Description: Python 3 bindings for Open vSwitch >>>>> Open vSwitch is a production quality, multilayer, software-based, >>>>> Ethernet virtual switch. It is designed to enable massive network >>>>> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 >>>>> index 14d9249b8..373a7e413 100644 >>>>> --- a/m4/openvswitch.m4 >>>>> +++ b/m4/openvswitch.m4 >>>>> @@ -371,16 +371,16 @@ dnl Checks for valgrind/valgrind.h. >>>>> AC_DEFUN([OVS_CHECK_VALGRIND], >>>>> [AC_CHECK_HEADERS([valgrind/valgrind.h])]) >>>>> >>>>> -dnl Checks for Python 3.4 or later. >>>>> +dnl Checks for Python 3.6 or later. >>>>> AC_DEFUN([OVS_CHECK_PYTHON3], >>>>> [AC_CACHE_CHECK( >>>>> - [for Python 3 (version 3.4 or later)], >>>>> + [for Python 3 (version 3.6 or later)], >>>>> [ovs_cv_python3], >>>>> [if test -n "$PYTHON3"; then >>>>> ovs_cv_python3=$PYTHON3 >>>>> else >>>>> ovs_cv_python3=no >>>>> - for binary in python3 python3.4 python3.5 python3.6 python3.7; do >>>>> + for binary in python3{,.{6..12}}; do >>>>> ovs_save_IFS=$IFS; IFS=$PATH_SEPARATOR >>>>> for dir in $PATH; do >>>>> IFS=$ovs_save_IFS >>>>> @@ -397,7 +397,7 @@ else: >>>>> done >>>>> fi]) >>>>> if test "$ovs_cv_python3" = no; then >>>>> - AC_MSG_ERROR([Python 3.4 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) >>>>> + AC_MSG_ERROR([Python 3.6 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) >>>>> fi >>>>> AC_ARG_VAR([PYTHON3]) >>>>> PYTHON3=$ovs_cv_python3]) >>>>> diff --git a/python/TODO.rst b/python/TODO.rst >>>>> index 3a53489f1..acc5461e2 100644 >>>>> --- a/python/TODO.rst >>>>> +++ b/python/TODO.rst >>>>> @@ -32,3 +32,10 @@ Python Bindings To-do List >>>>> >>>>> * Support write-only-changed monitor mode (equivalent of >>>>> OVSDB_IDL_WRITE_CHANGED_ONLY). >>>>> + >>>>> +* socket_util: >>>>> + >>>>> + * Add equivalent fuctions to inet_parse_passive, parse_sockaddr_components, >>>>> + et al. to better support using async dns. The reconnect code will >>>>> + currently log a warning when inet_parse_active() returns w/o yet having >>>>> + resolved an address, but will continue to connect and eventually succeed. >>>>> diff --git a/python/automake.mk b/python/automake.mk >>>>> index d00911828..82a508787 100644 >>>>> --- a/python/automake.mk >>>>> +++ b/python/automake.mk >>>>> @@ -16,6 +16,7 @@ ovs_pyfiles = \ >>>>> python/ovs/compat/sortedcontainers/sorteddict.py \ >>>>> python/ovs/compat/sortedcontainers/sortedset.py \ >>>>> python/ovs/daemon.py \ >>>>> + python/ovs/dns_resolve.py \ >>>>> python/ovs/db/__init__.py \ >>>>> python/ovs/db/custom_index.py \ >>>>> python/ovs/db/data.py \ >>>>> @@ -55,6 +56,7 @@ ovs_pyfiles = \ >>>>> >>>>> ovs_pytests = \ >>>>> python/ovs/tests/test_decoders.py \ >>>>> + python/ovs/tests/test_dns_resolve.py \ >>>>> python/ovs/tests/test_filter.py \ >>>>> python/ovs/tests/test_kv.py \ >>>>> python/ovs/tests/test_list.py \ >>>>> diff --git a/python/ovs/dns_resolve.py b/python/ovs/dns_resolve.py >>>>> new file mode 100644 >>>>> index 000000000..7b0f6c266 >>>>> --- /dev/null >>>>> +++ b/python/ovs/dns_resolve.py >>>>> @@ -0,0 +1,272 @@ >>>>> +# Copyright (c) 2023 Red Hat, Inc. >>>>> +# >>>>> +# Licensed under the Apache License, Version 2.0 (the "License"); >>>>> +# you may not use this file except in compliance with the License. >>>>> +# You may obtain a copy of the License at: >>>>> +# >>>>> +# http://www.apache.org/licenses/LICENSE-2.0 >>>>> +# >>>>> +# Unless required by applicable law or agreed to in writing, software >>>>> +# distributed under the License is distributed on an "AS IS" BASIS, >>>>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. >>>>> +# See the License for the specific language governing permissions and >>>>> +# limitations under the License. >>>>> + >>>>> +import collections >>>>> +import enum >>>>> +import functools >>>>> +import ipaddress >>>>> +import os >>>>> +import time >>>>> +import typing >>>>> + >>>>> +try: >>>>> + import unbound # type: ignore >>>>> +except ImportError: >>>>> + pass >>>>> + >>>>> +import ovs.vlog >>>>> + >>>>> +vlog = ovs.vlog.Vlog("dns_resolve") >>>>> + >>>>> + >>>>> +class ReqState(enum.Enum): >>>>> + INVALID = 0 >>>>> + PENDING = 1 >>>>> + GOOD = 2 >>>>> + ERROR = 3 >>>>> + >>>>> + >>>>> +class DNSRequest: >>>>> + >>>> >>>> nit: extra space >>>> >>>>> + def __init__(self, name: str): >>>>> + self.name: str = name >>>>> + self.state: ReqState = ReqState.INVALID >>>>> + self.time: typing.Optional[float] = None >>>>> + # set by DNSResolver._callback >>>>> + self.result: typing.Optional[str] = None >>>>> + self.ttl: typing.Optional[float] = None >>>>> + >>>>> + @property >>>>> + def expired(self): >>>>> + return time.time() > self.time + self.ttl >>>>> + >>>>> + @property >>>>> + def is_valid(self): >>>>> + return self.state == ReqState.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 >>>>> + >>>> >>>> Having a singleton class that accepts a parameter in the constructor could be a >>>> bit confusing. For instance: "d = DNSResolver(is_daemon=True)" >>>> might yield a resolver where "is_daemon" is actually "False" if some other part >>>> of the code created it before. >>>> >>>> Woudn't it be cleaner if it just exposed "resove_sync" and "resolve_async" and >>>> let users choose how to resolve? >>> >>> I suppose, the problem is that most of the resolving is happening >>> from the inside of OVS libraries. So, it will not be a choice of >>> an application. >>> >> >> Not sure I understand. Currently if a user does: >> 'DNSResolver(is_daemon=False).resolve("openvswitch.org")', it'll resolve >> synchronously and so will all the rest of the resolutions of that program. >> >> The internal variable "_is_daemon" is only used to select the way to resolve. >> The same behavior would be achieved if, for instance, "is_daemon" is a default >> argument to "resolve()" with the extra benefit of the first resolution not >> influencing all the rest. > > My point is that inet_parse_active() is calling: > > host_name = dns_resolve.resolve(host_name) > > And we will have to add an argument to this call in order to give user > a choice to perform sync or async resolution. As well as add this > argument to all the functions that may call inet_parse_active() and all > the functions that may call these functions. > If it has the right default value it woudn't impact this particular caller. > To avoid that we have a global "is_daemon" state. > > Does that make sense? Yes. I guess I was thinking in the possibility of the dns resolver being used outside of inet_parse_active > > Maybe we could remove the class paramater and add a function like > dns_resolve.enable_async() instead, so the global state change will > be explicit? > That would make it clearer, yes. >> >>> Maybe a warning that the parameter is different will be enough? >>>> Best regards, Ilya Maximets. >>> >> >
On 6/30/23 16:54, Adrian Moreno wrote: > > > On 6/30/23 14:35, Ilya Maximets wrote: >> On 6/30/23 14:23, Adrian Moreno wrote: >>> >>> >>> On 6/30/23 13:40, Ilya Maximets wrote: >>>> On 6/30/23 12:39, Adrian Moreno wrote: >>>>> >>>>> >>>>> On 6/14/23 23:07, 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 <twilson@redhat.com> >>>>>> --- >>>>>> .github/workflows/build-and-test.yml | 4 +- >>>>>> Documentation/intro/install/general.rst | 4 +- >>>>>> Documentation/intro/install/rhel.rst | 2 +- >>>>>> Documentation/intro/install/windows.rst | 2 +- >>>>>> NEWS | 4 +- >>>>>> debian/control.in | 1 + >>>>>> m4/openvswitch.m4 | 8 +- >>>>>> python/TODO.rst | 7 + >>>>>> python/automake.mk | 2 + >>>>>> python/ovs/dns_resolve.py | 272 +++++++++++++++++++++++ >>>>>> 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 + >>>>>> 16 files changed, 601 insertions(+), 18 deletions(-) >>>>>> create mode 100644 python/ovs/dns_resolve.py >>>>>> create mode 100644 python/ovs/tests/test_dns_resolve.py >>>>>> >>>>>> diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml >>>>>> index f66ab43b0..47d239f10 100644 >>>>>> --- a/.github/workflows/build-and-test.yml >>>>>> +++ b/.github/workflows/build-and-test.yml >>>>>> @@ -183,10 +183,10 @@ jobs: >>>>>> run: sudo apt update || true >>>>>> - name: install common dependencies >>>>>> run: sudo apt install -y ${{ env.dependencies }} >>>>>> - - name: install libunbound libunwind >>>>>> + - name: install libunbound libunwind python3-unbound >>>>>> # GitHub Actions doesn't have 32-bit versions of these libraries. >>>>>> if: matrix.m32 == '' >>>>>> - run: sudo apt install -y libunbound-dev libunwind-dev >>>>>> + run: sudo apt install -y libunbound-dev libunwind-dev python3-unbound >>>>>> - name: install 32-bit libraries >>>>>> if: matrix.m32 != '' >>>>>> run: sudo apt install -y gcc-multilib >>>>>> diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst >>>>>> index 42b5682fd..19e360d47 100644 >>>>>> --- a/Documentation/intro/install/general.rst >>>>>> +++ b/Documentation/intro/install/general.rst >>>>>> @@ -90,7 +90,7 @@ need the following software: >>>>>> If libcap-ng is installed, then Open vSwitch will automatically build with >>>>>> support for it. >>>>>> >>>>>> -- Python 3.4 or later. >>>>>> +- Python 3.6 or later. >>>>>> >>>>>> - Unbound library, from http://www.unbound.net, is optional but recommended if >>>>>> you want to enable ovs-vswitchd and other utilities to use DNS names when >>>>>> @@ -208,7 +208,7 @@ simply install and run Open vSwitch you require the following software: >>>>>> from iproute2 (part of all major distributions and available at >>>>>> https://wiki.linuxfoundation.org/networking/iproute2). >>>>>> >>>>>> -- Python 3.4 or later. >>>>>> +- Python 3.6 or later. >>>>>> >>>>>> On Linux you should ensure that ``/dev/urandom`` exists. To support TAP >>>>>> devices, you must also ensure that ``/dev/net/tun`` exists. >>>>>> diff --git a/Documentation/intro/install/rhel.rst b/Documentation/intro/install/rhel.rst >>>>>> index d1fc42021..f2151d890 100644 >>>>>> --- a/Documentation/intro/install/rhel.rst >>>>>> +++ b/Documentation/intro/install/rhel.rst >>>>>> @@ -92,7 +92,7 @@ Once that is completed, remove the file ``/tmp/ovs.spec``. >>>>>> If python3-sphinx package is not available in your version of RHEL, you can >>>>>> install it via pip with 'pip install sphinx'. >>>>>> >>>>>> -Open vSwitch requires python 3.4 or newer which is not available in older >>>>>> +Open vSwitch requires python 3.6 or newer which is not available in older >>>>>> distributions. In the case of RHEL 6.x and its derivatives, one option is >>>>>> to install python34 from `EPEL`_. >>>>>> >>>>>> diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst >>>>>> index 78f60f35a..fce099d5d 100644 >>>>>> --- a/Documentation/intro/install/windows.rst >>>>>> +++ b/Documentation/intro/install/windows.rst >>>>>> @@ -56,7 +56,7 @@ The following explains the steps in some detail. >>>>>> >>>>>> 'C:/MinGW /mingw'. >>>>>> >>>>>> -- Python 3.4 or later. >>>>>> +- Python 3.6 or later. >>>>>> >>>>>> Install the latest Python 3.x from python.org and verify that its path is >>>>>> part of Windows' PATH environment variable. >>>>>> diff --git a/NEWS b/NEWS >>>>>> index cfd466663..24c694b8f 100644 >>>>>> --- a/NEWS >>>>>> +++ b/NEWS >>>>>> @@ -36,7 +36,9 @@ Post-v3.1.0 >>>>>> process extra privileges when mapping physical interconnect memory. >>>>>> - SRv6 Tunnel Protocol >>>>>> * Added support for userspace datapath (only). >>>>>> - >>>>>> + - Python >>>>>> + * Added async DNS support >>>>>> + * Dropped support for Python < 3.6 >>>>>> >>>>>> v3.1.0 - 16 Feb 2023 >>>>>> -------------------- >>>>>> diff --git a/debian/control.in b/debian/control.in >>>>>> index 19f590d06..64b0a4ce0 100644 >>>>>> --- a/debian/control.in >>>>>> +++ b/debian/control.in >>>>>> @@ -287,6 +287,7 @@ Depends: >>>>>> Suggests: >>>>>> python3-netaddr, >>>>>> python3-pyparsing, >>>>>> + python3-unbound, >>>>>> Description: Python 3 bindings for Open vSwitch >>>>>> Open vSwitch is a production quality, multilayer, software-based, >>>>>> Ethernet virtual switch. It is designed to enable massive network >>>>>> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 >>>>>> index 14d9249b8..373a7e413 100644 >>>>>> --- a/m4/openvswitch.m4 >>>>>> +++ b/m4/openvswitch.m4 >>>>>> @@ -371,16 +371,16 @@ dnl Checks for valgrind/valgrind.h. >>>>>> AC_DEFUN([OVS_CHECK_VALGRIND], >>>>>> [AC_CHECK_HEADERS([valgrind/valgrind.h])]) >>>>>> >>>>>> -dnl Checks for Python 3.4 or later. >>>>>> +dnl Checks for Python 3.6 or later. >>>>>> AC_DEFUN([OVS_CHECK_PYTHON3], >>>>>> [AC_CACHE_CHECK( >>>>>> - [for Python 3 (version 3.4 or later)], >>>>>> + [for Python 3 (version 3.6 or later)], >>>>>> [ovs_cv_python3], >>>>>> [if test -n "$PYTHON3"; then >>>>>> ovs_cv_python3=$PYTHON3 >>>>>> else >>>>>> ovs_cv_python3=no >>>>>> - for binary in python3 python3.4 python3.5 python3.6 python3.7; do >>>>>> + for binary in python3{,.{6..12}}; do >>>>>> ovs_save_IFS=$IFS; IFS=$PATH_SEPARATOR >>>>>> for dir in $PATH; do >>>>>> IFS=$ovs_save_IFS >>>>>> @@ -397,7 +397,7 @@ else: >>>>>> done >>>>>> fi]) >>>>>> if test "$ovs_cv_python3" = no; then >>>>>> - AC_MSG_ERROR([Python 3.4 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) >>>>>> + AC_MSG_ERROR([Python 3.6 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) >>>>>> fi >>>>>> AC_ARG_VAR([PYTHON3]) >>>>>> PYTHON3=$ovs_cv_python3]) >>>>>> diff --git a/python/TODO.rst b/python/TODO.rst >>>>>> index 3a53489f1..acc5461e2 100644 >>>>>> --- a/python/TODO.rst >>>>>> +++ b/python/TODO.rst >>>>>> @@ -32,3 +32,10 @@ Python Bindings To-do List >>>>>> >>>>>> * Support write-only-changed monitor mode (equivalent of >>>>>> OVSDB_IDL_WRITE_CHANGED_ONLY). >>>>>> + >>>>>> +* socket_util: >>>>>> + >>>>>> + * Add equivalent fuctions to inet_parse_passive, parse_sockaddr_components, >>>>>> + et al. to better support using async dns. The reconnect code will >>>>>> + currently log a warning when inet_parse_active() returns w/o yet having >>>>>> + resolved an address, but will continue to connect and eventually succeed. >>>>>> diff --git a/python/automake.mk b/python/automake.mk >>>>>> index d00911828..82a508787 100644 >>>>>> --- a/python/automake.mk >>>>>> +++ b/python/automake.mk >>>>>> @@ -16,6 +16,7 @@ ovs_pyfiles = \ >>>>>> python/ovs/compat/sortedcontainers/sorteddict.py \ >>>>>> python/ovs/compat/sortedcontainers/sortedset.py \ >>>>>> python/ovs/daemon.py \ >>>>>> + python/ovs/dns_resolve.py \ >>>>>> python/ovs/db/__init__.py \ >>>>>> python/ovs/db/custom_index.py \ >>>>>> python/ovs/db/data.py \ >>>>>> @@ -55,6 +56,7 @@ ovs_pyfiles = \ >>>>>> >>>>>> ovs_pytests = \ >>>>>> python/ovs/tests/test_decoders.py \ >>>>>> + python/ovs/tests/test_dns_resolve.py \ >>>>>> python/ovs/tests/test_filter.py \ >>>>>> python/ovs/tests/test_kv.py \ >>>>>> python/ovs/tests/test_list.py \ >>>>>> diff --git a/python/ovs/dns_resolve.py b/python/ovs/dns_resolve.py >>>>>> new file mode 100644 >>>>>> index 000000000..7b0f6c266 >>>>>> --- /dev/null >>>>>> +++ b/python/ovs/dns_resolve.py >>>>>> @@ -0,0 +1,272 @@ >>>>>> +# Copyright (c) 2023 Red Hat, Inc. >>>>>> +# >>>>>> +# Licensed under the Apache License, Version 2.0 (the "License"); >>>>>> +# you may not use this file except in compliance with the License. >>>>>> +# You may obtain a copy of the License at: >>>>>> +# >>>>>> +# http://www.apache.org/licenses/LICENSE-2.0 >>>>>> +# >>>>>> +# Unless required by applicable law or agreed to in writing, software >>>>>> +# distributed under the License is distributed on an "AS IS" BASIS, >>>>>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. >>>>>> +# See the License for the specific language governing permissions and >>>>>> +# limitations under the License. >>>>>> + >>>>>> +import collections >>>>>> +import enum >>>>>> +import functools >>>>>> +import ipaddress >>>>>> +import os >>>>>> +import time >>>>>> +import typing >>>>>> + >>>>>> +try: >>>>>> + import unbound # type: ignore >>>>>> +except ImportError: >>>>>> + pass >>>>>> + >>>>>> +import ovs.vlog >>>>>> + >>>>>> +vlog = ovs.vlog.Vlog("dns_resolve") >>>>>> + >>>>>> + >>>>>> +class ReqState(enum.Enum): >>>>>> + INVALID = 0 >>>>>> + PENDING = 1 >>>>>> + GOOD = 2 >>>>>> + ERROR = 3 >>>>>> + >>>>>> + >>>>>> +class DNSRequest: >>>>>> + >>>>> >>>>> nit: extra space >>>>> >>>>>> + def __init__(self, name: str): >>>>>> + self.name: str = name >>>>>> + self.state: ReqState = ReqState.INVALID >>>>>> + self.time: typing.Optional[float] = None >>>>>> + # set by DNSResolver._callback >>>>>> + self.result: typing.Optional[str] = None >>>>>> + self.ttl: typing.Optional[float] = None >>>>>> + >>>>>> + @property >>>>>> + def expired(self): >>>>>> + return time.time() > self.time + self.ttl >>>>>> + >>>>>> + @property >>>>>> + def is_valid(self): >>>>>> + return self.state == ReqState.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 >>>>>> + >>>>> >>>>> Having a singleton class that accepts a parameter in the constructor could be a >>>>> bit confusing. For instance: "d = DNSResolver(is_daemon=True)" >>>>> might yield a resolver where "is_daemon" is actually "False" if some other part >>>>> of the code created it before. >>>>> >>>>> Woudn't it be cleaner if it just exposed "resove_sync" and "resolve_async" and >>>>> let users choose how to resolve? >>>> >>>> I suppose, the problem is that most of the resolving is happening >>>> from the inside of OVS libraries. So, it will not be a choice of >>>> an application. >>>> >>> >>> Not sure I understand. Currently if a user does: >>> 'DNSResolver(is_daemon=False).resolve("openvswitch.org")', it'll resolve >>> synchronously and so will all the rest of the resolutions of that program. >>> >>> The internal variable "_is_daemon" is only used to select the way to resolve. >>> The same behavior would be achieved if, for instance, "is_daemon" is a default >>> argument to "resolve()" with the extra benefit of the first resolution not >>> influencing all the rest. >> >> My point is that inet_parse_active() is calling: >> >> host_name = dns_resolve.resolve(host_name) >> >> And we will have to add an argument to this call in order to give user >> a choice to perform sync or async resolution. As well as add this >> argument to all the functions that may call inet_parse_active() and all >> the functions that may call these functions. >> > > If it has the right default value it woudn't impact this particular caller. > >> To avoid that we have a global "is_daemon" state. >> >> Does that make sense? > > Yes. I guess I was thinking in the possibility of the dns resolver being used > outside of inet_parse_active > >> >> Maybe we could remove the class paramater and add a function like >> dns_resolve.enable_async() instead, so the global state change will >> be explicit? >> > > That would make it clearer, yes. OK. Terry, what do you think? > >>> >>>> Maybe a warning that the parameter is different will be enough? >>>>> Best regards, Ilya Maximets. >>>> >>> >> >
I accidentally forgot to click reply-to-all. On Fri, Jun 30, 2023 at 10:27 AM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 6/30/23 16:54, Adrian Moreno wrote: > > > > > > On 6/30/23 14:35, Ilya Maximets wrote: > >> On 6/30/23 14:23, Adrian Moreno wrote: > >>> > >>> > >>> On 6/30/23 13:40, Ilya Maximets wrote: > >>>> On 6/30/23 12:39, Adrian Moreno wrote: > >>>>> > >>>>> > >>>>> On 6/14/23 23:07, 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 <twilson@redhat.com> > >>>>>> --- > >>>>>> .github/workflows/build-and-test.yml | 4 +- > >>>>>> Documentation/intro/install/general.rst | 4 +- > >>>>>> Documentation/intro/install/rhel.rst | 2 +- > >>>>>> Documentation/intro/install/windows.rst | 2 +- > >>>>>> NEWS | 4 +- > >>>>>> debian/control.in | 1 + > >>>>>> m4/openvswitch.m4 | 8 +- > >>>>>> python/TODO.rst | 7 + > >>>>>> python/automake.mk | 2 + > >>>>>> python/ovs/dns_resolve.py | 272 +++++++++++++++++++++++ > >>>>>> 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 + > >>>>>> 16 files changed, 601 insertions(+), 18 deletions(-) > >>>>>> create mode 100644 python/ovs/dns_resolve.py > >>>>>> create mode 100644 python/ovs/tests/test_dns_resolve.py > >>>>>> > >>>>>> diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml > >>>>>> index f66ab43b0..47d239f10 100644 > >>>>>> --- a/.github/workflows/build-and-test.yml > >>>>>> +++ b/.github/workflows/build-and-test.yml > >>>>>> @@ -183,10 +183,10 @@ jobs: > >>>>>> run: sudo apt update || true > >>>>>> - name: install common dependencies > >>>>>> run: sudo apt install -y ${{ env.dependencies }} > >>>>>> - - name: install libunbound libunwind > >>>>>> + - name: install libunbound libunwind python3-unbound > >>>>>> # GitHub Actions doesn't have 32-bit versions of these libraries. > >>>>>> if: matrix.m32 == '' > >>>>>> - run: sudo apt install -y libunbound-dev libunwind-dev > >>>>>> + run: sudo apt install -y libunbound-dev libunwind-dev python3-unbound > >>>>>> - name: install 32-bit libraries > >>>>>> if: matrix.m32 != '' > >>>>>> run: sudo apt install -y gcc-multilib > >>>>>> diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst > >>>>>> index 42b5682fd..19e360d47 100644 > >>>>>> --- a/Documentation/intro/install/general.rst > >>>>>> +++ b/Documentation/intro/install/general.rst > >>>>>> @@ -90,7 +90,7 @@ need the following software: > >>>>>> If libcap-ng is installed, then Open vSwitch will automatically build with > >>>>>> support for it. > >>>>>> > >>>>>> -- Python 3.4 or later. > >>>>>> +- Python 3.6 or later. > >>>>>> > >>>>>> - Unbound library, from http://www.unbound.net, is optional but recommended if > >>>>>> you want to enable ovs-vswitchd and other utilities to use DNS names when > >>>>>> @@ -208,7 +208,7 @@ simply install and run Open vSwitch you require the following software: > >>>>>> from iproute2 (part of all major distributions and available at > >>>>>> https://wiki.linuxfoundation.org/networking/iproute2). > >>>>>> > >>>>>> -- Python 3.4 or later. > >>>>>> +- Python 3.6 or later. > >>>>>> > >>>>>> On Linux you should ensure that ``/dev/urandom`` exists. To support TAP > >>>>>> devices, you must also ensure that ``/dev/net/tun`` exists. > >>>>>> diff --git a/Documentation/intro/install/rhel.rst b/Documentation/intro/install/rhel.rst > >>>>>> index d1fc42021..f2151d890 100644 > >>>>>> --- a/Documentation/intro/install/rhel.rst > >>>>>> +++ b/Documentation/intro/install/rhel.rst > >>>>>> @@ -92,7 +92,7 @@ Once that is completed, remove the file ``/tmp/ovs.spec``. > >>>>>> If python3-sphinx package is not available in your version of RHEL, you can > >>>>>> install it via pip with 'pip install sphinx'. > >>>>>> > >>>>>> -Open vSwitch requires python 3.4 or newer which is not available in older > >>>>>> +Open vSwitch requires python 3.6 or newer which is not available in older > >>>>>> distributions. In the case of RHEL 6.x and its derivatives, one option is > >>>>>> to install python34 from `EPEL`_. > >>>>>> > >>>>>> diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst > >>>>>> index 78f60f35a..fce099d5d 100644 > >>>>>> --- a/Documentation/intro/install/windows.rst > >>>>>> +++ b/Documentation/intro/install/windows.rst > >>>>>> @@ -56,7 +56,7 @@ The following explains the steps in some detail. > >>>>>> > >>>>>> 'C:/MinGW /mingw'. > >>>>>> > >>>>>> -- Python 3.4 or later. > >>>>>> +- Python 3.6 or later. > >>>>>> > >>>>>> Install the latest Python 3.x from python.org and verify that its path is > >>>>>> part of Windows' PATH environment variable. > >>>>>> diff --git a/NEWS b/NEWS > >>>>>> index cfd466663..24c694b8f 100644 > >>>>>> --- a/NEWS > >>>>>> +++ b/NEWS > >>>>>> @@ -36,7 +36,9 @@ Post-v3.1.0 > >>>>>> process extra privileges when mapping physical interconnect memory. > >>>>>> - SRv6 Tunnel Protocol > >>>>>> * Added support for userspace datapath (only). > >>>>>> - > >>>>>> + - Python > >>>>>> + * Added async DNS support > >>>>>> + * Dropped support for Python < 3.6 > >>>>>> > >>>>>> v3.1.0 - 16 Feb 2023 > >>>>>> -------------------- > >>>>>> diff --git a/debian/control.in b/debian/control.in > >>>>>> index 19f590d06..64b0a4ce0 100644 > >>>>>> --- a/debian/control.in > >>>>>> +++ b/debian/control.in > >>>>>> @@ -287,6 +287,7 @@ Depends: > >>>>>> Suggests: > >>>>>> python3-netaddr, > >>>>>> python3-pyparsing, > >>>>>> + python3-unbound, > >>>>>> Description: Python 3 bindings for Open vSwitch > >>>>>> Open vSwitch is a production quality, multilayer, software-based, > >>>>>> Ethernet virtual switch. It is designed to enable massive network > >>>>>> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 > >>>>>> index 14d9249b8..373a7e413 100644 > >>>>>> --- a/m4/openvswitch.m4 > >>>>>> +++ b/m4/openvswitch.m4 > >>>>>> @@ -371,16 +371,16 @@ dnl Checks for valgrind/valgrind.h. > >>>>>> AC_DEFUN([OVS_CHECK_VALGRIND], > >>>>>> [AC_CHECK_HEADERS([valgrind/valgrind.h])]) > >>>>>> > >>>>>> -dnl Checks for Python 3.4 or later. > >>>>>> +dnl Checks for Python 3.6 or later. > >>>>>> AC_DEFUN([OVS_CHECK_PYTHON3], > >>>>>> [AC_CACHE_CHECK( > >>>>>> - [for Python 3 (version 3.4 or later)], > >>>>>> + [for Python 3 (version 3.6 or later)], > >>>>>> [ovs_cv_python3], > >>>>>> [if test -n "$PYTHON3"; then > >>>>>> ovs_cv_python3=$PYTHON3 > >>>>>> else > >>>>>> ovs_cv_python3=no > >>>>>> - for binary in python3 python3.4 python3.5 python3.6 python3.7; do > >>>>>> + for binary in python3{,.{6..12}}; do > >>>>>> ovs_save_IFS=$IFS; IFS=$PATH_SEPARATOR > >>>>>> for dir in $PATH; do > >>>>>> IFS=$ovs_save_IFS > >>>>>> @@ -397,7 +397,7 @@ else: > >>>>>> done > >>>>>> fi]) > >>>>>> if test "$ovs_cv_python3" = no; then > >>>>>> - AC_MSG_ERROR([Python 3.4 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) > >>>>>> + AC_MSG_ERROR([Python 3.6 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) > >>>>>> fi > >>>>>> AC_ARG_VAR([PYTHON3]) > >>>>>> PYTHON3=$ovs_cv_python3]) > >>>>>> diff --git a/python/TODO.rst b/python/TODO.rst > >>>>>> index 3a53489f1..acc5461e2 100644 > >>>>>> --- a/python/TODO.rst > >>>>>> +++ b/python/TODO.rst > >>>>>> @@ -32,3 +32,10 @@ Python Bindings To-do List > >>>>>> > >>>>>> * Support write-only-changed monitor mode (equivalent of > >>>>>> OVSDB_IDL_WRITE_CHANGED_ONLY). > >>>>>> + > >>>>>> +* socket_util: > >>>>>> + > >>>>>> + * Add equivalent fuctions to inet_parse_passive, parse_sockaddr_components, > >>>>>> + et al. to better support using async dns. The reconnect code will > >>>>>> + currently log a warning when inet_parse_active() returns w/o yet having > >>>>>> + resolved an address, but will continue to connect and eventually succeed. > >>>>>> diff --git a/python/automake.mk b/python/automake.mk > >>>>>> index d00911828..82a508787 100644 > >>>>>> --- a/python/automake.mk > >>>>>> +++ b/python/automake.mk > >>>>>> @@ -16,6 +16,7 @@ ovs_pyfiles = \ > >>>>>> python/ovs/compat/sortedcontainers/sorteddict.py \ > >>>>>> python/ovs/compat/sortedcontainers/sortedset.py \ > >>>>>> python/ovs/daemon.py \ > >>>>>> + python/ovs/dns_resolve.py \ > >>>>>> python/ovs/db/__init__.py \ > >>>>>> python/ovs/db/custom_index.py \ > >>>>>> python/ovs/db/data.py \ > >>>>>> @@ -55,6 +56,7 @@ ovs_pyfiles = \ > >>>>>> > >>>>>> ovs_pytests = \ > >>>>>> python/ovs/tests/test_decoders.py \ > >>>>>> + python/ovs/tests/test_dns_resolve.py \ > >>>>>> python/ovs/tests/test_filter.py \ > >>>>>> python/ovs/tests/test_kv.py \ > >>>>>> python/ovs/tests/test_list.py \ > >>>>>> diff --git a/python/ovs/dns_resolve.py b/python/ovs/dns_resolve.py > >>>>>> new file mode 100644 > >>>>>> index 000000000..7b0f6c266 > >>>>>> --- /dev/null > >>>>>> +++ b/python/ovs/dns_resolve.py > >>>>>> @@ -0,0 +1,272 @@ > >>>>>> +# Copyright (c) 2023 Red Hat, Inc. > >>>>>> +# > >>>>>> +# Licensed under the Apache License, Version 2.0 (the "License"); > >>>>>> +# you may not use this file except in compliance with the License. > >>>>>> +# You may obtain a copy of the License at: > >>>>>> +# > >>>>>> +# http://www.apache.org/licenses/LICENSE-2.0 > >>>>>> +# > >>>>>> +# Unless required by applicable law or agreed to in writing, software > >>>>>> +# distributed under the License is distributed on an "AS IS" BASIS, > >>>>>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > >>>>>> +# See the License for the specific language governing permissions and > >>>>>> +# limitations under the License. > >>>>>> + > >>>>>> +import collections > >>>>>> +import enum > >>>>>> +import functools > >>>>>> +import ipaddress > >>>>>> +import os > >>>>>> +import time > >>>>>> +import typing > >>>>>> + > >>>>>> +try: > >>>>>> + import unbound # type: ignore > >>>>>> +except ImportError: > >>>>>> + pass > >>>>>> + > >>>>>> +import ovs.vlog > >>>>>> + > >>>>>> +vlog = ovs.vlog.Vlog("dns_resolve") > >>>>>> + > >>>>>> + > >>>>>> +class ReqState(enum.Enum): > >>>>>> + INVALID = 0 > >>>>>> + PENDING = 1 > >>>>>> + GOOD = 2 > >>>>>> + ERROR = 3 > >>>>>> + > >>>>>> + > >>>>>> +class DNSRequest: > >>>>>> + > >>>>> > >>>>> nit: extra space > >>>>> > >>>>>> + def __init__(self, name: str): > >>>>>> + self.name: str = name > >>>>>> + self.state: ReqState = ReqState.INVALID > >>>>>> + self.time: typing.Optional[float] = None > >>>>>> + # set by DNSResolver._callback > >>>>>> + self.result: typing.Optional[str] = None > >>>>>> + self.ttl: typing.Optional[float] = None > >>>>>> + > >>>>>> + @property > >>>>>> + def expired(self): > >>>>>> + return time.time() > self.time + self.ttl > >>>>>> + > >>>>>> + @property > >>>>>> + def is_valid(self): > >>>>>> + return self.state == ReqState.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 > >>>>>> + > >>>>> > >>>>> Having a singleton class that accepts a parameter in the constructor could be a > >>>>> bit confusing. For instance: "d = DNSResolver(is_daemon=True)" > >>>>> might yield a resolver where "is_daemon" is actually "False" if some other part > >>>>> of the code created it before. > >>>>> > >>>>> Woudn't it be cleaner if it just exposed "resove_sync" and "resolve_async" and > >>>>> let users choose how to resolve? Singletons aren't really my favorite thing to use in the first place, but I also didn't really like the idea of it just being a bunch of global variables either. We're following the C lib here as far as dns_resolve_init()/dns_resolve() being the interface, though. It does allow you to re-call dns_resolve_init() with a new is_daemon value and just overwrites all of the global state, which seems kind of weird to me. I can't think of a situation where anyone would ever call dns_resolve_init() or DNSResolver() multiple times in an application (at least with the current implementations). It seems like if we were going to really do that, both the C and Python versions should just stop using global state altogether. > >>>> I suppose, the problem is that most of the resolving is happening > >>>> from the inside of OVS libraries. So, it will not be a choice of > >>>> an application. > >>>> > >>> > >>> Not sure I understand. Currently if a user does: > >>> 'DNSResolver(is_daemon=False).resolve("openvswitch.org")', it'll resolve > >>> synchronously and so will all the rest of the resolutions of that program. > >>> > >>> The internal variable "_is_daemon" is only used to select the way to resolve. > >>> The same behavior would be achieved if, for instance, "is_daemon" is a default > >>> argument to "resolve()" with the extra benefit of the first resolution not > >>> influencing all the rest. > >> > >> My point is that inet_parse_active() is calling: > >> > >> host_name = dns_resolve.resolve(host_name) > >> > >> And we will have to add an argument to this call in order to give user > >> a choice to perform sync or async resolution. As well as add this > >> argument to all the functions that may call inet_parse_active() and all > >> the functions that may call these functions. > >> > > > > If it has the right default value it woudn't impact this particular caller. > > > >> To avoid that we have a global "is_daemon" state. > >> > >> Does that make sense? > > > > Yes. I guess I was thinking in the possibility of the dns resolver being used > > outside of inet_parse_active > > > >> > >> Maybe we could remove the class paramater and add a function like > >> dns_resolve.enable_async() instead, so the global state change will > >> be explicit? > >> > > > > That would make it clearer, yes. > > OK. > > Terry, what do you think? > > > > >>> > >>>> Maybe a warning that the parameter is different will be enough? > >>>>> Best regards, Ilya Maximets. > >>>> > >>> > >> > > >
On Mon, Jul 10, 2023 at 10:32 AM Terry Wilson <twilson@redhat.com> wrote: > > I accidentally forgot to click reply-to-all. > > On Fri, Jun 30, 2023 at 10:27 AM Ilya Maximets <i.maximets@ovn.org> wrote: > > > > On 6/30/23 16:54, Adrian Moreno wrote: > > > > > > > > > On 6/30/23 14:35, Ilya Maximets wrote: > > >> On 6/30/23 14:23, Adrian Moreno wrote: > > >>> > > >>> > > >>> On 6/30/23 13:40, Ilya Maximets wrote: > > >>>> On 6/30/23 12:39, Adrian Moreno wrote: > > >>>>> > > >>>>> > > >>>>> On 6/14/23 23:07, 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 <twilson@redhat.com> > > >>>>>> --- > > >>>>>> .github/workflows/build-and-test.yml | 4 +- > > >>>>>> Documentation/intro/install/general.rst | 4 +- > > >>>>>> Documentation/intro/install/rhel.rst | 2 +- > > >>>>>> Documentation/intro/install/windows.rst | 2 +- > > >>>>>> NEWS | 4 +- > > >>>>>> debian/control.in | 1 + > > >>>>>> m4/openvswitch.m4 | 8 +- > > >>>>>> python/TODO.rst | 7 + > > >>>>>> python/automake.mk | 2 + > > >>>>>> python/ovs/dns_resolve.py | 272 +++++++++++++++++++++++ > > >>>>>> 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 + > > >>>>>> 16 files changed, 601 insertions(+), 18 deletions(-) > > >>>>>> create mode 100644 python/ovs/dns_resolve.py > > >>>>>> create mode 100644 python/ovs/tests/test_dns_resolve.py > > >>>>>> > > >>>>>> diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml > > >>>>>> index f66ab43b0..47d239f10 100644 > > >>>>>> --- a/.github/workflows/build-and-test.yml > > >>>>>> +++ b/.github/workflows/build-and-test.yml > > >>>>>> @@ -183,10 +183,10 @@ jobs: > > >>>>>> run: sudo apt update || true > > >>>>>> - name: install common dependencies > > >>>>>> run: sudo apt install -y ${{ env.dependencies }} > > >>>>>> - - name: install libunbound libunwind > > >>>>>> + - name: install libunbound libunwind python3-unbound > > >>>>>> # GitHub Actions doesn't have 32-bit versions of these libraries. > > >>>>>> if: matrix.m32 == '' > > >>>>>> - run: sudo apt install -y libunbound-dev libunwind-dev > > >>>>>> + run: sudo apt install -y libunbound-dev libunwind-dev python3-unbound > > >>>>>> - name: install 32-bit libraries > > >>>>>> if: matrix.m32 != '' > > >>>>>> run: sudo apt install -y gcc-multilib > > >>>>>> diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst > > >>>>>> index 42b5682fd..19e360d47 100644 > > >>>>>> --- a/Documentation/intro/install/general.rst > > >>>>>> +++ b/Documentation/intro/install/general.rst > > >>>>>> @@ -90,7 +90,7 @@ need the following software: > > >>>>>> If libcap-ng is installed, then Open vSwitch will automatically build with > > >>>>>> support for it. > > >>>>>> > > >>>>>> -- Python 3.4 or later. > > >>>>>> +- Python 3.6 or later. > > >>>>>> > > >>>>>> - Unbound library, from http://www.unbound.net, is optional but recommended if > > >>>>>> you want to enable ovs-vswitchd and other utilities to use DNS names when > > >>>>>> @@ -208,7 +208,7 @@ simply install and run Open vSwitch you require the following software: > > >>>>>> from iproute2 (part of all major distributions and available at > > >>>>>> https://wiki.linuxfoundation.org/networking/iproute2). > > >>>>>> > > >>>>>> -- Python 3.4 or later. > > >>>>>> +- Python 3.6 or later. > > >>>>>> > > >>>>>> On Linux you should ensure that ``/dev/urandom`` exists. To support TAP > > >>>>>> devices, you must also ensure that ``/dev/net/tun`` exists. > > >>>>>> diff --git a/Documentation/intro/install/rhel.rst b/Documentation/intro/install/rhel.rst > > >>>>>> index d1fc42021..f2151d890 100644 > > >>>>>> --- a/Documentation/intro/install/rhel.rst > > >>>>>> +++ b/Documentation/intro/install/rhel.rst > > >>>>>> @@ -92,7 +92,7 @@ Once that is completed, remove the file ``/tmp/ovs.spec``. > > >>>>>> If python3-sphinx package is not available in your version of RHEL, you can > > >>>>>> install it via pip with 'pip install sphinx'. > > >>>>>> > > >>>>>> -Open vSwitch requires python 3.4 or newer which is not available in older > > >>>>>> +Open vSwitch requires python 3.6 or newer which is not available in older > > >>>>>> distributions. In the case of RHEL 6.x and its derivatives, one option is > > >>>>>> to install python34 from `EPEL`_. > > >>>>>> > > >>>>>> diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst > > >>>>>> index 78f60f35a..fce099d5d 100644 > > >>>>>> --- a/Documentation/intro/install/windows.rst > > >>>>>> +++ b/Documentation/intro/install/windows.rst > > >>>>>> @@ -56,7 +56,7 @@ The following explains the steps in some detail. > > >>>>>> > > >>>>>> 'C:/MinGW /mingw'. > > >>>>>> > > >>>>>> -- Python 3.4 or later. > > >>>>>> +- Python 3.6 or later. > > >>>>>> > > >>>>>> Install the latest Python 3.x from python.org and verify that its path is > > >>>>>> part of Windows' PATH environment variable. > > >>>>>> diff --git a/NEWS b/NEWS > > >>>>>> index cfd466663..24c694b8f 100644 > > >>>>>> --- a/NEWS > > >>>>>> +++ b/NEWS > > >>>>>> @@ -36,7 +36,9 @@ Post-v3.1.0 > > >>>>>> process extra privileges when mapping physical interconnect memory. > > >>>>>> - SRv6 Tunnel Protocol > > >>>>>> * Added support for userspace datapath (only). > > >>>>>> - > > >>>>>> + - Python > > >>>>>> + * Added async DNS support > > >>>>>> + * Dropped support for Python < 3.6 > > >>>>>> > > >>>>>> v3.1.0 - 16 Feb 2023 > > >>>>>> -------------------- > > >>>>>> diff --git a/debian/control.in b/debian/control.in > > >>>>>> index 19f590d06..64b0a4ce0 100644 > > >>>>>> --- a/debian/control.in > > >>>>>> +++ b/debian/control.in > > >>>>>> @@ -287,6 +287,7 @@ Depends: > > >>>>>> Suggests: > > >>>>>> python3-netaddr, > > >>>>>> python3-pyparsing, > > >>>>>> + python3-unbound, > > >>>>>> Description: Python 3 bindings for Open vSwitch > > >>>>>> Open vSwitch is a production quality, multilayer, software-based, > > >>>>>> Ethernet virtual switch. It is designed to enable massive network > > >>>>>> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 > > >>>>>> index 14d9249b8..373a7e413 100644 > > >>>>>> --- a/m4/openvswitch.m4 > > >>>>>> +++ b/m4/openvswitch.m4 > > >>>>>> @@ -371,16 +371,16 @@ dnl Checks for valgrind/valgrind.h. > > >>>>>> AC_DEFUN([OVS_CHECK_VALGRIND], > > >>>>>> [AC_CHECK_HEADERS([valgrind/valgrind.h])]) > > >>>>>> > > >>>>>> -dnl Checks for Python 3.4 or later. > > >>>>>> +dnl Checks for Python 3.6 or later. > > >>>>>> AC_DEFUN([OVS_CHECK_PYTHON3], > > >>>>>> [AC_CACHE_CHECK( > > >>>>>> - [for Python 3 (version 3.4 or later)], > > >>>>>> + [for Python 3 (version 3.6 or later)], > > >>>>>> [ovs_cv_python3], > > >>>>>> [if test -n "$PYTHON3"; then > > >>>>>> ovs_cv_python3=$PYTHON3 > > >>>>>> else > > >>>>>> ovs_cv_python3=no > > >>>>>> - for binary in python3 python3.4 python3.5 python3.6 python3.7; do > > >>>>>> + for binary in python3{,.{6..12}}; do > > >>>>>> ovs_save_IFS=$IFS; IFS=$PATH_SEPARATOR > > >>>>>> for dir in $PATH; do > > >>>>>> IFS=$ovs_save_IFS > > >>>>>> @@ -397,7 +397,7 @@ else: > > >>>>>> done > > >>>>>> fi]) > > >>>>>> if test "$ovs_cv_python3" = no; then > > >>>>>> - AC_MSG_ERROR([Python 3.4 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) > > >>>>>> + AC_MSG_ERROR([Python 3.6 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) > > >>>>>> fi > > >>>>>> AC_ARG_VAR([PYTHON3]) > > >>>>>> PYTHON3=$ovs_cv_python3]) > > >>>>>> diff --git a/python/TODO.rst b/python/TODO.rst > > >>>>>> index 3a53489f1..acc5461e2 100644 > > >>>>>> --- a/python/TODO.rst > > >>>>>> +++ b/python/TODO.rst > > >>>>>> @@ -32,3 +32,10 @@ Python Bindings To-do List > > >>>>>> > > >>>>>> * Support write-only-changed monitor mode (equivalent of > > >>>>>> OVSDB_IDL_WRITE_CHANGED_ONLY). > > >>>>>> + > > >>>>>> +* socket_util: > > >>>>>> + > > >>>>>> + * Add equivalent fuctions to inet_parse_passive, parse_sockaddr_components, > > >>>>>> + et al. to better support using async dns. The reconnect code will > > >>>>>> + currently log a warning when inet_parse_active() returns w/o yet having > > >>>>>> + resolved an address, but will continue to connect and eventually succeed. > > >>>>>> diff --git a/python/automake.mk b/python/automake.mk > > >>>>>> index d00911828..82a508787 100644 > > >>>>>> --- a/python/automake.mk > > >>>>>> +++ b/python/automake.mk > > >>>>>> @@ -16,6 +16,7 @@ ovs_pyfiles = \ > > >>>>>> python/ovs/compat/sortedcontainers/sorteddict.py \ > > >>>>>> python/ovs/compat/sortedcontainers/sortedset.py \ > > >>>>>> python/ovs/daemon.py \ > > >>>>>> + python/ovs/dns_resolve.py \ > > >>>>>> python/ovs/db/__init__.py \ > > >>>>>> python/ovs/db/custom_index.py \ > > >>>>>> python/ovs/db/data.py \ > > >>>>>> @@ -55,6 +56,7 @@ ovs_pyfiles = \ > > >>>>>> > > >>>>>> ovs_pytests = \ > > >>>>>> python/ovs/tests/test_decoders.py \ > > >>>>>> + python/ovs/tests/test_dns_resolve.py \ > > >>>>>> python/ovs/tests/test_filter.py \ > > >>>>>> python/ovs/tests/test_kv.py \ > > >>>>>> python/ovs/tests/test_list.py \ > > >>>>>> diff --git a/python/ovs/dns_resolve.py b/python/ovs/dns_resolve.py > > >>>>>> new file mode 100644 > > >>>>>> index 000000000..7b0f6c266 > > >>>>>> --- /dev/null > > >>>>>> +++ b/python/ovs/dns_resolve.py > > >>>>>> @@ -0,0 +1,272 @@ > > >>>>>> +# Copyright (c) 2023 Red Hat, Inc. > > >>>>>> +# > > >>>>>> +# Licensed under the Apache License, Version 2.0 (the "License"); > > >>>>>> +# you may not use this file except in compliance with the License. > > >>>>>> +# You may obtain a copy of the License at: > > >>>>>> +# > > >>>>>> +# http://www.apache.org/licenses/LICENSE-2.0 > > >>>>>> +# > > >>>>>> +# Unless required by applicable law or agreed to in writing, software > > >>>>>> +# distributed under the License is distributed on an "AS IS" BASIS, > > >>>>>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > > >>>>>> +# See the License for the specific language governing permissions and > > >>>>>> +# limitations under the License. > > >>>>>> + > > >>>>>> +import collections > > >>>>>> +import enum > > >>>>>> +import functools > > >>>>>> +import ipaddress > > >>>>>> +import os > > >>>>>> +import time > > >>>>>> +import typing > > >>>>>> + > > >>>>>> +try: > > >>>>>> + import unbound # type: ignore > > >>>>>> +except ImportError: > > >>>>>> + pass > > >>>>>> + > > >>>>>> +import ovs.vlog > > >>>>>> + > > >>>>>> +vlog = ovs.vlog.Vlog("dns_resolve") > > >>>>>> + > > >>>>>> + > > >>>>>> +class ReqState(enum.Enum): > > >>>>>> + INVALID = 0 > > >>>>>> + PENDING = 1 > > >>>>>> + GOOD = 2 > > >>>>>> + ERROR = 3 > > >>>>>> + > > >>>>>> + > > >>>>>> +class DNSRequest: > > >>>>>> + > > >>>>> > > >>>>> nit: extra space > > >>>>> > > >>>>>> + def __init__(self, name: str): > > >>>>>> + self.name: str = name > > >>>>>> + self.state: ReqState = ReqState.INVALID > > >>>>>> + self.time: typing.Optional[float] = None > > >>>>>> + # set by DNSResolver._callback > > >>>>>> + self.result: typing.Optional[str] = None > > >>>>>> + self.ttl: typing.Optional[float] = None > > >>>>>> + > > >>>>>> + @property > > >>>>>> + def expired(self): > > >>>>>> + return time.time() > self.time + self.ttl > > >>>>>> + > > >>>>>> + @property > > >>>>>> + def is_valid(self): > > >>>>>> + return self.state == ReqState.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 > > >>>>>> + > > >>>>> > > >>>>> Having a singleton class that accepts a parameter in the constructor could be a > > >>>>> bit confusing. For instance: "d = DNSResolver(is_daemon=True)" > > >>>>> might yield a resolver where "is_daemon" is actually "False" if some other part > > >>>>> of the code created it before. > > >>>>> > > >>>>> Woudn't it be cleaner if it just exposed "resove_sync" and "resolve_async" and > > >>>>> let users choose how to resolve? > > Singletons aren't really my favorite thing to use in the first place, > but I also didn't really like the idea of it just being a bunch of > global variables either. We're following the C lib here as far as > dns_resolve_init()/dns_resolve() being the interface, though. It does > allow you to re-call dns_resolve_init() with a new is_daemon value and > just overwrites all of the global state, which seems kind of weird to > me. > > I can't think of a situation where anyone would ever call > dns_resolve_init() or DNSResolver() multiple times in an application > (at least with the current implementations). It seems like if we were > going to really do that, both the C and Python versions should just > stop using global state altogether. I'm currently working on a solution that removes the singleton and keeps the class. Then top-level init/resolve/destroy methods that work on a global DNSResolver, but would also allow users to just make multiple resolvers if they wanted. It's really rewriting the tests that is the pain for this, though. Ultimately, it's most an aesthetic issue since I don't imagine anyone will ever write apps that destroy/re-init a resolver. But maybe that's just a failure of my imagination. With that said, it looks like we set up the ub_ctx to be able to handle async regardless of is_daemon value, so even in the C lib I'm not sure why we don't just make resolve_sync/resolve_async both available to end users. Terry > > > >>>> I suppose, the problem is that most of the resolving is happening > > >>>> from the inside of OVS libraries. So, it will not be a choice of > > >>>> an application. > > >>>> > > >>> > > >>> Not sure I understand. Currently if a user does: > > >>> 'DNSResolver(is_daemon=False).resolve("openvswitch.org")', it'll resolve > > >>> synchronously and so will all the rest of the resolutions of that program. > > >>> > > >>> The internal variable "_is_daemon" is only used to select the way to resolve. > > >>> The same behavior would be achieved if, for instance, "is_daemon" is a default > > >>> argument to "resolve()" with the extra benefit of the first resolution not > > >>> influencing all the rest. > > >> > > >> My point is that inet_parse_active() is calling: > > >> > > >> host_name = dns_resolve.resolve(host_name) > > >> > > >> And we will have to add an argument to this call in order to give user > > >> a choice to perform sync or async resolution. As well as add this > > >> argument to all the functions that may call inet_parse_active() and all > > >> the functions that may call these functions. > > >> > > > > > > If it has the right default value it woudn't impact this particular caller. > > > > > >> To avoid that we have a global "is_daemon" state. > > >> > > >> Does that make sense? > > > > > > Yes. I guess I was thinking in the possibility of the dns resolver being used > > > outside of inet_parse_active > > > > > >> > > >> Maybe we could remove the class paramater and add a function like > > >> dns_resolve.enable_async() instead, so the global state change will > > >> be explicit? > > >> > > > > > > That would make it clearer, yes. > > > > OK. > > > > Terry, what do you think? > > > > > > > >>> > > >>>> Maybe a warning that the parameter is different will be enough? > > >>>>> Best regards, Ilya Maximets. > > >>>> > > >>> > > >> > > > > >
On 7/10/23 17:36, Terry Wilson wrote: > On Mon, Jul 10, 2023 at 10:32 AM Terry Wilson <twilson@redhat.com> wrote: >> >> I accidentally forgot to click reply-to-all. >> >> On Fri, Jun 30, 2023 at 10:27 AM Ilya Maximets <i.maximets@ovn.org> wrote: >>> >>> On 6/30/23 16:54, Adrian Moreno wrote: >>>> >>>> >>>> On 6/30/23 14:35, Ilya Maximets wrote: >>>>> On 6/30/23 14:23, Adrian Moreno wrote: >>>>>> >>>>>> >>>>>> On 6/30/23 13:40, Ilya Maximets wrote: >>>>>>> On 6/30/23 12:39, Adrian Moreno wrote: >>>>>>>> >>>>>>>> >>>>>>>> On 6/14/23 23:07, 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 <twilson@redhat.com> >>>>>>>>> --- >>>>>>>>> .github/workflows/build-and-test.yml | 4 +- >>>>>>>>> Documentation/intro/install/general.rst | 4 +- >>>>>>>>> Documentation/intro/install/rhel.rst | 2 +- >>>>>>>>> Documentation/intro/install/windows.rst | 2 +- >>>>>>>>> NEWS | 4 +- >>>>>>>>> debian/control.in | 1 + >>>>>>>>> m4/openvswitch.m4 | 8 +- >>>>>>>>> python/TODO.rst | 7 + >>>>>>>>> python/automake.mk | 2 + >>>>>>>>> python/ovs/dns_resolve.py | 272 +++++++++++++++++++++++ >>>>>>>>> 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 + >>>>>>>>> 16 files changed, 601 insertions(+), 18 deletions(-) >>>>>>>>> create mode 100644 python/ovs/dns_resolve.py >>>>>>>>> create mode 100644 python/ovs/tests/test_dns_resolve.py >>>>>>>>> >>>>>>>>> diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml >>>>>>>>> index f66ab43b0..47d239f10 100644 >>>>>>>>> --- a/.github/workflows/build-and-test.yml >>>>>>>>> +++ b/.github/workflows/build-and-test.yml >>>>>>>>> @@ -183,10 +183,10 @@ jobs: >>>>>>>>> run: sudo apt update || true >>>>>>>>> - name: install common dependencies >>>>>>>>> run: sudo apt install -y ${{ env.dependencies }} >>>>>>>>> - - name: install libunbound libunwind >>>>>>>>> + - name: install libunbound libunwind python3-unbound >>>>>>>>> # GitHub Actions doesn't have 32-bit versions of these libraries. >>>>>>>>> if: matrix.m32 == '' >>>>>>>>> - run: sudo apt install -y libunbound-dev libunwind-dev >>>>>>>>> + run: sudo apt install -y libunbound-dev libunwind-dev python3-unbound >>>>>>>>> - name: install 32-bit libraries >>>>>>>>> if: matrix.m32 != '' >>>>>>>>> run: sudo apt install -y gcc-multilib >>>>>>>>> diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst >>>>>>>>> index 42b5682fd..19e360d47 100644 >>>>>>>>> --- a/Documentation/intro/install/general.rst >>>>>>>>> +++ b/Documentation/intro/install/general.rst >>>>>>>>> @@ -90,7 +90,7 @@ need the following software: >>>>>>>>> If libcap-ng is installed, then Open vSwitch will automatically build with >>>>>>>>> support for it. >>>>>>>>> >>>>>>>>> -- Python 3.4 or later. >>>>>>>>> +- Python 3.6 or later. >>>>>>>>> >>>>>>>>> - Unbound library, from http://www.unbound.net, is optional but recommended if >>>>>>>>> you want to enable ovs-vswitchd and other utilities to use DNS names when >>>>>>>>> @@ -208,7 +208,7 @@ simply install and run Open vSwitch you require the following software: >>>>>>>>> from iproute2 (part of all major distributions and available at >>>>>>>>> https://wiki.linuxfoundation.org/networking/iproute2). >>>>>>>>> >>>>>>>>> -- Python 3.4 or later. >>>>>>>>> +- Python 3.6 or later. >>>>>>>>> >>>>>>>>> On Linux you should ensure that ``/dev/urandom`` exists. To support TAP >>>>>>>>> devices, you must also ensure that ``/dev/net/tun`` exists. >>>>>>>>> diff --git a/Documentation/intro/install/rhel.rst b/Documentation/intro/install/rhel.rst >>>>>>>>> index d1fc42021..f2151d890 100644 >>>>>>>>> --- a/Documentation/intro/install/rhel.rst >>>>>>>>> +++ b/Documentation/intro/install/rhel.rst >>>>>>>>> @@ -92,7 +92,7 @@ Once that is completed, remove the file ``/tmp/ovs.spec``. >>>>>>>>> If python3-sphinx package is not available in your version of RHEL, you can >>>>>>>>> install it via pip with 'pip install sphinx'. >>>>>>>>> >>>>>>>>> -Open vSwitch requires python 3.4 or newer which is not available in older >>>>>>>>> +Open vSwitch requires python 3.6 or newer which is not available in older >>>>>>>>> distributions. In the case of RHEL 6.x and its derivatives, one option is >>>>>>>>> to install python34 from `EPEL`_. >>>>>>>>> >>>>>>>>> diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst >>>>>>>>> index 78f60f35a..fce099d5d 100644 >>>>>>>>> --- a/Documentation/intro/install/windows.rst >>>>>>>>> +++ b/Documentation/intro/install/windows.rst >>>>>>>>> @@ -56,7 +56,7 @@ The following explains the steps in some detail. >>>>>>>>> >>>>>>>>> 'C:/MinGW /mingw'. >>>>>>>>> >>>>>>>>> -- Python 3.4 or later. >>>>>>>>> +- Python 3.6 or later. >>>>>>>>> >>>>>>>>> Install the latest Python 3.x from python.org and verify that its path is >>>>>>>>> part of Windows' PATH environment variable. >>>>>>>>> diff --git a/NEWS b/NEWS >>>>>>>>> index cfd466663..24c694b8f 100644 >>>>>>>>> --- a/NEWS >>>>>>>>> +++ b/NEWS >>>>>>>>> @@ -36,7 +36,9 @@ Post-v3.1.0 >>>>>>>>> process extra privileges when mapping physical interconnect memory. >>>>>>>>> - SRv6 Tunnel Protocol >>>>>>>>> * Added support for userspace datapath (only). >>>>>>>>> - >>>>>>>>> + - Python >>>>>>>>> + * Added async DNS support >>>>>>>>> + * Dropped support for Python < 3.6 >>>>>>>>> >>>>>>>>> v3.1.0 - 16 Feb 2023 >>>>>>>>> -------------------- >>>>>>>>> diff --git a/debian/control.in b/debian/control.in >>>>>>>>> index 19f590d06..64b0a4ce0 100644 >>>>>>>>> --- a/debian/control.in >>>>>>>>> +++ b/debian/control.in >>>>>>>>> @@ -287,6 +287,7 @@ Depends: >>>>>>>>> Suggests: >>>>>>>>> python3-netaddr, >>>>>>>>> python3-pyparsing, >>>>>>>>> + python3-unbound, >>>>>>>>> Description: Python 3 bindings for Open vSwitch >>>>>>>>> Open vSwitch is a production quality, multilayer, software-based, >>>>>>>>> Ethernet virtual switch. It is designed to enable massive network >>>>>>>>> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 >>>>>>>>> index 14d9249b8..373a7e413 100644 >>>>>>>>> --- a/m4/openvswitch.m4 >>>>>>>>> +++ b/m4/openvswitch.m4 >>>>>>>>> @@ -371,16 +371,16 @@ dnl Checks for valgrind/valgrind.h. >>>>>>>>> AC_DEFUN([OVS_CHECK_VALGRIND], >>>>>>>>> [AC_CHECK_HEADERS([valgrind/valgrind.h])]) >>>>>>>>> >>>>>>>>> -dnl Checks for Python 3.4 or later. >>>>>>>>> +dnl Checks for Python 3.6 or later. >>>>>>>>> AC_DEFUN([OVS_CHECK_PYTHON3], >>>>>>>>> [AC_CACHE_CHECK( >>>>>>>>> - [for Python 3 (version 3.4 or later)], >>>>>>>>> + [for Python 3 (version 3.6 or later)], >>>>>>>>> [ovs_cv_python3], >>>>>>>>> [if test -n "$PYTHON3"; then >>>>>>>>> ovs_cv_python3=$PYTHON3 >>>>>>>>> else >>>>>>>>> ovs_cv_python3=no >>>>>>>>> - for binary in python3 python3.4 python3.5 python3.6 python3.7; do >>>>>>>>> + for binary in python3{,.{6..12}}; do >>>>>>>>> ovs_save_IFS=$IFS; IFS=$PATH_SEPARATOR >>>>>>>>> for dir in $PATH; do >>>>>>>>> IFS=$ovs_save_IFS >>>>>>>>> @@ -397,7 +397,7 @@ else: >>>>>>>>> done >>>>>>>>> fi]) >>>>>>>>> if test "$ovs_cv_python3" = no; then >>>>>>>>> - AC_MSG_ERROR([Python 3.4 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) >>>>>>>>> + AC_MSG_ERROR([Python 3.6 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) >>>>>>>>> fi >>>>>>>>> AC_ARG_VAR([PYTHON3]) >>>>>>>>> PYTHON3=$ovs_cv_python3]) >>>>>>>>> diff --git a/python/TODO.rst b/python/TODO.rst >>>>>>>>> index 3a53489f1..acc5461e2 100644 >>>>>>>>> --- a/python/TODO.rst >>>>>>>>> +++ b/python/TODO.rst >>>>>>>>> @@ -32,3 +32,10 @@ Python Bindings To-do List >>>>>>>>> >>>>>>>>> * Support write-only-changed monitor mode (equivalent of >>>>>>>>> OVSDB_IDL_WRITE_CHANGED_ONLY). >>>>>>>>> + >>>>>>>>> +* socket_util: >>>>>>>>> + >>>>>>>>> + * Add equivalent fuctions to inet_parse_passive, parse_sockaddr_components, >>>>>>>>> + et al. to better support using async dns. The reconnect code will >>>>>>>>> + currently log a warning when inet_parse_active() returns w/o yet having >>>>>>>>> + resolved an address, but will continue to connect and eventually succeed. >>>>>>>>> diff --git a/python/automake.mk b/python/automake.mk >>>>>>>>> index d00911828..82a508787 100644 >>>>>>>>> --- a/python/automake.mk >>>>>>>>> +++ b/python/automake.mk >>>>>>>>> @@ -16,6 +16,7 @@ ovs_pyfiles = \ >>>>>>>>> python/ovs/compat/sortedcontainers/sorteddict.py \ >>>>>>>>> python/ovs/compat/sortedcontainers/sortedset.py \ >>>>>>>>> python/ovs/daemon.py \ >>>>>>>>> + python/ovs/dns_resolve.py \ >>>>>>>>> python/ovs/db/__init__.py \ >>>>>>>>> python/ovs/db/custom_index.py \ >>>>>>>>> python/ovs/db/data.py \ >>>>>>>>> @@ -55,6 +56,7 @@ ovs_pyfiles = \ >>>>>>>>> >>>>>>>>> ovs_pytests = \ >>>>>>>>> python/ovs/tests/test_decoders.py \ >>>>>>>>> + python/ovs/tests/test_dns_resolve.py \ >>>>>>>>> python/ovs/tests/test_filter.py \ >>>>>>>>> python/ovs/tests/test_kv.py \ >>>>>>>>> python/ovs/tests/test_list.py \ >>>>>>>>> diff --git a/python/ovs/dns_resolve.py b/python/ovs/dns_resolve.py >>>>>>>>> new file mode 100644 >>>>>>>>> index 000000000..7b0f6c266 >>>>>>>>> --- /dev/null >>>>>>>>> +++ b/python/ovs/dns_resolve.py >>>>>>>>> @@ -0,0 +1,272 @@ >>>>>>>>> +# Copyright (c) 2023 Red Hat, Inc. >>>>>>>>> +# >>>>>>>>> +# Licensed under the Apache License, Version 2.0 (the "License"); >>>>>>>>> +# you may not use this file except in compliance with the License. >>>>>>>>> +# You may obtain a copy of the License at: >>>>>>>>> +# >>>>>>>>> +# http://www.apache.org/licenses/LICENSE-2.0 >>>>>>>>> +# >>>>>>>>> +# Unless required by applicable law or agreed to in writing, software >>>>>>>>> +# distributed under the License is distributed on an "AS IS" BASIS, >>>>>>>>> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. >>>>>>>>> +# See the License for the specific language governing permissions and >>>>>>>>> +# limitations under the License. >>>>>>>>> + >>>>>>>>> +import collections >>>>>>>>> +import enum >>>>>>>>> +import functools >>>>>>>>> +import ipaddress >>>>>>>>> +import os >>>>>>>>> +import time >>>>>>>>> +import typing >>>>>>>>> + >>>>>>>>> +try: >>>>>>>>> + import unbound # type: ignore >>>>>>>>> +except ImportError: >>>>>>>>> + pass >>>>>>>>> + >>>>>>>>> +import ovs.vlog >>>>>>>>> + >>>>>>>>> +vlog = ovs.vlog.Vlog("dns_resolve") >>>>>>>>> + >>>>>>>>> + >>>>>>>>> +class ReqState(enum.Enum): >>>>>>>>> + INVALID = 0 >>>>>>>>> + PENDING = 1 >>>>>>>>> + GOOD = 2 >>>>>>>>> + ERROR = 3 >>>>>>>>> + >>>>>>>>> + >>>>>>>>> +class DNSRequest: >>>>>>>>> + >>>>>>>> >>>>>>>> nit: extra space >>>>>>>> >>>>>>>>> + def __init__(self, name: str): >>>>>>>>> + self.name: str = name >>>>>>>>> + self.state: ReqState = ReqState.INVALID >>>>>>>>> + self.time: typing.Optional[float] = None >>>>>>>>> + # set by DNSResolver._callback >>>>>>>>> + self.result: typing.Optional[str] = None >>>>>>>>> + self.ttl: typing.Optional[float] = None >>>>>>>>> + >>>>>>>>> + @property >>>>>>>>> + def expired(self): >>>>>>>>> + return time.time() > self.time + self.ttl >>>>>>>>> + >>>>>>>>> + @property >>>>>>>>> + def is_valid(self): >>>>>>>>> + return self.state == ReqState.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 >>>>>>>>> + >>>>>>>> >>>>>>>> Having a singleton class that accepts a parameter in the constructor could be a >>>>>>>> bit confusing. For instance: "d = DNSResolver(is_daemon=True)" >>>>>>>> might yield a resolver where "is_daemon" is actually "False" if some other part >>>>>>>> of the code created it before. >>>>>>>> >>>>>>>> Woudn't it be cleaner if it just exposed "resove_sync" and "resolve_async" and >>>>>>>> let users choose how to resolve? >> >> Singletons aren't really my favorite thing to use in the first place, >> but I also didn't really like the idea of it just being a bunch of >> global variables either. We're following the C lib here as far as >> dns_resolve_init()/dns_resolve() being the interface, though. It does >> allow you to re-call dns_resolve_init() with a new is_daemon value and >> just overwrites all of the global state, which seems kind of weird to >> me. >> >> I can't think of a situation where anyone would ever call >> dns_resolve_init() or DNSResolver() multiple times in an application >> (at least with the current implementations). It seems like if we were >> going to really do that, both the C and Python versions should just >> stop using global state altogether. > > I'm currently working on a solution that removes the singleton and > keeps the class. Then top-level init/resolve/destroy methods that work > on a global DNSResolver, but would also allow users to just make > multiple resolvers if they wanted. It's really rewriting the tests > that is the pain for this, though. Ultimately, it's most an aesthetic > issue since I don't imagine anyone will ever write apps that > destroy/re-init a resolver. But maybe that's just a failure of my > imagination. > > With that said, it looks like we set up the ub_ctx to be able to > handle async regardless of is_daemon value, so even in the C lib I'm > not sure why we don't just make resolve_sync/resolve_async both > available to end users. The reason is the same as I described below in reply to Adrian's first suggestion - most of the calls are made from the deep of OVS libraries and different processes may want different behavior. E.g. ovsdb-server will want async resolution while ovn-nbctl will need a synchronous one, because it doesn't have any work to do menwhile anyway. So, we either have a global switch in the init()-style function, or we pass is_async argument through the half of library functions. Whoever implemented the C version desided to not touch half of of the libraries and it kind of makes sense. > > Terry >> >>>>>>> I suppose, the problem is that most of the resolving is happening >>>>>>> from the inside of OVS libraries. So, it will not be a choice of >>>>>>> an application. >>>>>>> >>>>>> >>>>>> Not sure I understand. Currently if a user does: >>>>>> 'DNSResolver(is_daemon=False).resolve("openvswitch.org")', it'll resolve >>>>>> synchronously and so will all the rest of the resolutions of that program. >>>>>> >>>>>> The internal variable "_is_daemon" is only used to select the way to resolve. >>>>>> The same behavior would be achieved if, for instance, "is_daemon" is a default >>>>>> argument to "resolve()" with the extra benefit of the first resolution not >>>>>> influencing all the rest. >>>>> >>>>> My point is that inet_parse_active() is calling: >>>>> >>>>> host_name = dns_resolve.resolve(host_name) >>>>> >>>>> And we will have to add an argument to this call in order to give user >>>>> a choice to perform sync or async resolution. As well as add this >>>>> argument to all the functions that may call inet_parse_active() and all >>>>> the functions that may call these functions. >>>>> >>>> >>>> If it has the right default value it woudn't impact this particular caller. >>>> >>>>> To avoid that we have a global "is_daemon" state. >>>>> >>>>> Does that make sense? >>>> >>>> Yes. I guess I was thinking in the possibility of the dns resolver being used >>>> outside of inet_parse_active >>>> >>>>> >>>>> Maybe we could remove the class paramater and add a function like >>>>> dns_resolve.enable_async() instead, so the global state change will >>>>> be explicit? >>>>> >>>> >>>> That would make it clearer, yes. >>> >>> OK. >>> >>> Terry, what do you think? >>> >>>> >>>>>> >>>>>>> Maybe a warning that the parameter is different will be enough? >>>>>>>> Best regards, Ilya Maximets. >>>>>>> >>>>>> >>>>> >>>> >>> >
diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index f66ab43b0..47d239f10 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -183,10 +183,10 @@ jobs: run: sudo apt update || true - name: install common dependencies run: sudo apt install -y ${{ env.dependencies }} - - name: install libunbound libunwind + - name: install libunbound libunwind python3-unbound # GitHub Actions doesn't have 32-bit versions of these libraries. if: matrix.m32 == '' - run: sudo apt install -y libunbound-dev libunwind-dev + run: sudo apt install -y libunbound-dev libunwind-dev python3-unbound - name: install 32-bit libraries if: matrix.m32 != '' run: sudo apt install -y gcc-multilib diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst index 42b5682fd..19e360d47 100644 --- a/Documentation/intro/install/general.rst +++ b/Documentation/intro/install/general.rst @@ -90,7 +90,7 @@ need the following software: If libcap-ng is installed, then Open vSwitch will automatically build with support for it. -- Python 3.4 or later. +- Python 3.6 or later. - Unbound library, from http://www.unbound.net, is optional but recommended if you want to enable ovs-vswitchd and other utilities to use DNS names when @@ -208,7 +208,7 @@ simply install and run Open vSwitch you require the following software: from iproute2 (part of all major distributions and available at https://wiki.linuxfoundation.org/networking/iproute2). -- Python 3.4 or later. +- Python 3.6 or later. On Linux you should ensure that ``/dev/urandom`` exists. To support TAP devices, you must also ensure that ``/dev/net/tun`` exists. diff --git a/Documentation/intro/install/rhel.rst b/Documentation/intro/install/rhel.rst index d1fc42021..f2151d890 100644 --- a/Documentation/intro/install/rhel.rst +++ b/Documentation/intro/install/rhel.rst @@ -92,7 +92,7 @@ Once that is completed, remove the file ``/tmp/ovs.spec``. If python3-sphinx package is not available in your version of RHEL, you can install it via pip with 'pip install sphinx'. -Open vSwitch requires python 3.4 or newer which is not available in older +Open vSwitch requires python 3.6 or newer which is not available in older distributions. In the case of RHEL 6.x and its derivatives, one option is to install python34 from `EPEL`_. diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst index 78f60f35a..fce099d5d 100644 --- a/Documentation/intro/install/windows.rst +++ b/Documentation/intro/install/windows.rst @@ -56,7 +56,7 @@ The following explains the steps in some detail. 'C:/MinGW /mingw'. -- Python 3.4 or later. +- Python 3.6 or later. Install the latest Python 3.x from python.org and verify that its path is part of Windows' PATH environment variable. diff --git a/NEWS b/NEWS index cfd466663..24c694b8f 100644 --- a/NEWS +++ b/NEWS @@ -36,7 +36,9 @@ Post-v3.1.0 process extra privileges when mapping physical interconnect memory. - SRv6 Tunnel Protocol * Added support for userspace datapath (only). - + - Python + * Added async DNS support + * Dropped support for Python < 3.6 v3.1.0 - 16 Feb 2023 -------------------- diff --git a/debian/control.in b/debian/control.in index 19f590d06..64b0a4ce0 100644 --- a/debian/control.in +++ b/debian/control.in @@ -287,6 +287,7 @@ Depends: Suggests: python3-netaddr, python3-pyparsing, + python3-unbound, Description: Python 3 bindings for Open vSwitch Open vSwitch is a production quality, multilayer, software-based, Ethernet virtual switch. It is designed to enable massive network diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 index 14d9249b8..373a7e413 100644 --- a/m4/openvswitch.m4 +++ b/m4/openvswitch.m4 @@ -371,16 +371,16 @@ dnl Checks for valgrind/valgrind.h. AC_DEFUN([OVS_CHECK_VALGRIND], [AC_CHECK_HEADERS([valgrind/valgrind.h])]) -dnl Checks for Python 3.4 or later. +dnl Checks for Python 3.6 or later. AC_DEFUN([OVS_CHECK_PYTHON3], [AC_CACHE_CHECK( - [for Python 3 (version 3.4 or later)], + [for Python 3 (version 3.6 or later)], [ovs_cv_python3], [if test -n "$PYTHON3"; then ovs_cv_python3=$PYTHON3 else ovs_cv_python3=no - for binary in python3 python3.4 python3.5 python3.6 python3.7; do + for binary in python3{,.{6..12}}; do ovs_save_IFS=$IFS; IFS=$PATH_SEPARATOR for dir in $PATH; do IFS=$ovs_save_IFS @@ -397,7 +397,7 @@ else: done fi]) if test "$ovs_cv_python3" = no; then - AC_MSG_ERROR([Python 3.4 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) + AC_MSG_ERROR([Python 3.6 or later is required but not found in $PATH, please install it or set $PYTHON3 to point to it]) fi AC_ARG_VAR([PYTHON3]) PYTHON3=$ovs_cv_python3]) diff --git a/python/TODO.rst b/python/TODO.rst index 3a53489f1..acc5461e2 100644 --- a/python/TODO.rst +++ b/python/TODO.rst @@ -32,3 +32,10 @@ Python Bindings To-do List * Support write-only-changed monitor mode (equivalent of OVSDB_IDL_WRITE_CHANGED_ONLY). + +* socket_util: + + * Add equivalent fuctions to inet_parse_passive, parse_sockaddr_components, + et al. to better support using async dns. The reconnect code will + currently log a warning when inet_parse_active() returns w/o yet having + resolved an address, but will continue to connect and eventually succeed. diff --git a/python/automake.mk b/python/automake.mk index d00911828..82a508787 100644 --- a/python/automake.mk +++ b/python/automake.mk @@ -16,6 +16,7 @@ ovs_pyfiles = \ python/ovs/compat/sortedcontainers/sorteddict.py \ python/ovs/compat/sortedcontainers/sortedset.py \ python/ovs/daemon.py \ + python/ovs/dns_resolve.py \ python/ovs/db/__init__.py \ python/ovs/db/custom_index.py \ python/ovs/db/data.py \ @@ -55,6 +56,7 @@ ovs_pyfiles = \ ovs_pytests = \ python/ovs/tests/test_decoders.py \ + python/ovs/tests/test_dns_resolve.py \ python/ovs/tests/test_filter.py \ python/ovs/tests/test_kv.py \ python/ovs/tests/test_list.py \ diff --git a/python/ovs/dns_resolve.py b/python/ovs/dns_resolve.py new file mode 100644 index 000000000..7b0f6c266 --- /dev/null +++ b/python/ovs/dns_resolve.py @@ -0,0 +1,272 @@ +# Copyright (c) 2023 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at: +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import collections +import enum +import functools +import ipaddress +import os +import time +import typing + +try: + import unbound # type: ignore +except ImportError: + pass + +import ovs.vlog + +vlog = ovs.vlog.Vlog("dns_resolve") + + +class ReqState(enum.Enum): + INVALID = 0 + PENDING = 1 + GOOD = 2 + ERROR = 3 + + +class DNSRequest: + + def __init__(self, name: str): + self.name: str = name + self.state: ReqState = ReqState.INVALID + self.time: typing.Optional[float] = None + # set by DNSResolver._callback + self.result: typing.Optional[str] = None + self.ttl: typing.Optional[float] = None + + @property + def expired(self): + return time.time() > self.time + self.ttl + + @property + def is_valid(self): + return self.state == ReqState.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_RESOLV_CONF: A filename to override the system default resolv.conf + OVS_HOSTS_FILE: A filename to override the system default hosts file + + In the event that the unbound library is missing or fails to initialize + DNS lookup support will be disabled and the resolve() method will + return None. + """ + self._is_daemon = is_daemon + try: + self._ctx = unbound.ub_ctx() + self.dns_enabled = True + except Exception: + # The unbound docs mention that this could thrown an exception + # but do not specify what exception that is. This can also + # happen with a missing unbound library. + self.dns_enabled = False + vlog.err("Failed to initialize the unbound library") + return + + # NOTE(twilson) This cache, like the C version, can grow without bound + # and has no cleanup or aging mechanism. Given our usage patterns, this + # should not be a problem. But this should not be used to resolve an + # unbounded list of addresses in a long-running daemon. + self._requests = DefaultReqDict() + + self._ub_call(self._set_unbound_conf) + + # NOTE(twilson) The C version disables DNS in this case. I didn't do + # that here since it could still be useful to resolve addresses from + # /etc/hosts even w/o resolv.conf + self._ub_call(self._set_resolv_conf) + self._ub_call(self._set_hosts_file) + + self._ctx.set_async(True) # Sets threaded behavior for resolve_async() + + def _ub_call(self, fn, *args, **kwargs): + """Convert UnboundExceptions into vlog warnings""" + try: + return fn(*args, **kwargs) + except UnboundException as e: + vlog.warn(e) + + @dns_enabled + def _set_unbound_conf(self): + ub_cfg = os.getenv("OVS_UNBOUND_CONF") + if ub_cfg: + retval = self._ctx.config(ub_cfg) + if retval != 0: + raise UnboundException( + "Failed to set libunbound context config", retval) + + @dns_enabled + def _set_resolv_conf(self): + filename = os.getenv("OVS_RESOLV_CONF") + # The C lib checks that the file exists and also sets filename to + # /etc/resolv.conf on non-Windows, but resolvconf already does this. + retval = self._ctx.resolvconf(filename) + if retval != 0: + location = filename or "system default nameserver" + raise UnboundException(location, retval) + + @dns_enabled + def _set_hosts_file(self): + # The C lib doesn't have the ability to set a hosts file, but it is + # useful to have, especially for writing tests that don't rely on + # network connectivity. hosts(None) uses /etc/hosts. + filename = os.getenv("OVS_HOSTS_FILE") + retval = self._ctx.hosts(filename) + if retval != 0: + location = filename or "system default hosts file" + raise UnboundException(location, retval) + + @dns_enabled + def _callback(self, req: DNSRequest, err: int, result): + if err != 0 or (result.qtype == unbound.RR_TYPE_AAAA + and not result.havedata): + req.state = ReqState.ERROR + vlog.warn(f"{req.name}: failed to resolve") + return + if result.qtype == unbound.RR_TYPE_A and not result.havedata: + self._resolve_async(req, unbound.RR_TYPE_AAAA) + return + try: + ip_str = next(iter(result.data.as_raw_data())) + ip = ipaddress.ip_address(ip_str) # test if IP is valid + # NOTE (twilson) For some reason, accessing result data outside of + # _callback causes a segfault. So just grab and store what we need. + req.result = str(ip) + req.ttl = result.ttl + req.state = ReqState.GOOD + req.time = time.time() + except (ValueError, StopIteration): + req.state = ReqState.ERROR + vlog.err(f"{req.name}: failed to resolve") + + @dns_enabled + def _resolve_sync(self, name: str) -> typing.Optional[str]: + for qtype in (unbound.RR_TYPE_A, unbound.RR_TYPE_AAAA): + err, result = self._ctx.resolve(name, qtype) + if err != 0: + return None + if not result.havedata: + continue + try: + ip = ipaddress.ip_address( + next(iter(result.data.as_raw_data()))) + except (ValueError, StopIteration): + return None + return str(ip) + + return None + + @dns_enabled + def _resolve_async(self, req: DNSRequest, qtype) -> None: + err, _ = self._ctx.resolve_async(req.name, req, self._callback, + qtype) + if err != 0: + req.state = ReqState.ERROR + return None + + req.state = ReqState.PENDING + return None + + @dns_enabled + def resolve(self, name: str) -> typing.Optional[str]: + """Resolve a host name to an IP address + + If the resolver is set to handle requests asynchronously, resolve() + should be recalled until it returns a non-None result. Errors will be + logged. + + :param name: The host name to resolve + :returns: The IP address or None on error or not (yet) found + """ + if not self._is_daemon: + return self._resolve_sync(name) + retval = self._ctx.process() + if retval != 0: + vlog.err(f"dns-resolve error: {unbound.ub_strerror(retval)}") + return None + req = self._requests[name] # Creates a DNSRequest if not found + if req.is_valid: + return req.result + elif req.state != ReqState.PENDING: + self._resolve_async(req, unbound.RR_TYPE_A) + return None + + +def resolve(name: str) -> typing.Optional[str]: + """Resolve a host name to an IP address + + If a DNSResolver instance has not been instantiated, or if it has been + created with is_daemon=False, resolve() will synchronously resolve the + hostname. If DNSResolver has been initialized with is_daemon=True, it + will instead resolve asynchornously and resolve() will return None until + the hostname has been resolved. + + :param name: The host name to resolve + :returns: The IP address or None on error or not (yet) found + """ + + return DNSResolver().resolve(name) diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py index 7b41dc44b..a26298b75 100644 --- a/python/ovs/socket_util.py +++ b/python/ovs/socket_util.py @@ -13,12 +13,14 @@ # limitations under the License. import errno +import ipaddress import os import os.path import random import socket import sys +from ovs import dns_resolve import ovs.fatal_signal import ovs.poller import ovs.vlog @@ -216,7 +218,7 @@ def is_valid_ipv4_address(address): return True -def inet_parse_active(target, default_port): +def _inet_parse_active(target, default_port): address = target.split(":") if len(address) >= 2: host_name = ":".join(address[0:-1]).lstrip('[').rstrip(']') @@ -229,9 +231,24 @@ def inet_parse_active(target, default_port): host_name = address[0] if not host_name: raise ValueError("%s: bad peer name format" % target) + try: + host_name = str(ipaddress.ip_address(host_name)) + except ValueError: + host_name = dns_resolve.resolve(host_name) + if not host_name: + raise ValueError("%s: bad peer name format" % target) return (host_name, port) +def inet_parse_active(target, default_port, raises=True): + try: + return _inet_parse_active(target, default_port) + except ValueError: + if raises: + raise + return ("", default_port) + + def inet_create_socket_active(style, address): try: is_addr_inet = is_valid_ipv4_address(address[0]) @@ -262,7 +279,7 @@ def inet_connect_active(sock, address, family, dscp): def inet_open_active(style, target, default_port, dscp): - address = inet_parse_active(target, default_port) + address = inet_parse_active(target, default_port, raises=False) family, sock = inet_create_socket_active(style, address) if sock is None: return family, sock diff --git a/python/ovs/stream.py b/python/ovs/stream.py index b32341076..82fbb0d68 100644 --- a/python/ovs/stream.py +++ b/python/ovs/stream.py @@ -784,7 +784,7 @@ class SSLStream(Stream): @staticmethod def _open(suffix, dscp): - address = ovs.socket_util.inet_parse_active(suffix, 0) + address = ovs.socket_util.inet_parse_active(suffix, 0, raises=False) family, sock = ovs.socket_util.inet_create_socket_active( socket.SOCK_STREAM, address) if sock is None: diff --git a/python/ovs/tests/test_dns_resolve.py b/python/ovs/tests/test_dns_resolve.py new file mode 100644 index 000000000..fca8049b6 --- /dev/null +++ b/python/ovs/tests/test_dns_resolve.py @@ -0,0 +1,280 @@ +import contextlib +import ipaddress +import sys +import time +from unittest import mock + +import pytest + +from ovs import dns_resolve +from ovs import socket_util + + +skip_no_unbound = pytest.mark.skipif("unbound" not in dns_resolve.__dict__, + reason="Unbound not installed") + +HOSTS = [("192.0.2.1", "fake.ip4.domain", "192.0.2.1"), + ("2001:db8:2::1", "fake.ip6.domain", "2001:db8:2::1"), + ("192.0.2.2", "fake.both.domain", "192.0.2.2"), + ("2001:db8:2::2", "fake.both.domain", "192.0.2.2")] + + +def _tmp_file(path, content): + path.write_text(content) + assert content == path.read_text() + return path + + +@pytest.fixture(params=[False, True], ids=["not_daemon", "daemon"]) +def resolver_factory(monkeypatch, tmp_path, request): + # Allow delaying the instantiation of the DNSResolver + def resolver_factory(hosts=HOSTS): + path = tmp_path / "hosts" + content = "\n".join(f"{ip}\t{host}" for ip, host, _ in hosts) + _tmp_file(path, content) + + with monkeypatch.context() as m: + m.setenv("OVS_HOSTS_FILE", str(path)) + # Test with both is_daemon False and True + resolver = dns_resolve.DNSResolver(request.param) + assert resolver._is_daemon == request.param + return dns_resolve.DNSResolver(request.param) + + yield resolver_factory + dns_resolve.DNSResolver._instance = None + + +@contextlib.contextmanager +def DNSResolver(*args, **kwargs): + """Clean up after returning a dns_resolver.DNSResolver + + Since it is a singleton, and pytest runs all tests in the same process, + we can't use dns_resolver.DNSResolver directly in these tests. This + context manager will reset the singleton at the end of the with block. + """ + resolver = dns_resolve.DNSResolver(*args, **kwargs) + try: + yield resolver + finally: + dns_resolve.DNSResolver._instance = None + + +@pytest.fixture +def unbound_conf(tmp_path): + path = tmp_path / "unbound.conf" + content = """ + server: + verbosity: 1 + """ + return _tmp_file(path, content) + + +@pytest.fixture +def resolv_conf(tmp_path): + path = tmp_path / "resolv.conf" + content = "nameserver 127.0.0.1" + return _tmp_file(path, content) + + +@pytest.fixture +def hosts_file(tmp_path): + path = tmp_path / "hosts" + content = "127.0.0.1\tfakelocalhost.localdomain" + return _tmp_file(path, content) + + +@pytest.fixture +def missing_file(tmp_path): + f = tmp_path / "missing_file" + assert not f.exists() + return f + + +@pytest.fixture(params=[False, True], ids=["with unbound", "without unbound"]) +def missing_unbound(monkeypatch, request): + if request.param: + if "unbound" in dns_resolve.__dict__: + monkeypatch.setitem(sys.modules, 'unbound', None) + monkeypatch.delitem(dns_resolve.__dict__, "unbound") + elif "unbound" not in dns_resolve.__dict__: + pytest.skip("Unbound not installed") + return request.param + + +def test_missing_unbound(missing_unbound, resolver_factory): + resolver = resolver_factory() # Dont fail even w/o unbound + assert resolver.dns_enabled == (not missing_unbound) + + +def test_DNSRequest_defaults(): + req = dns_resolve.DNSRequest(HOSTS[0][1]) + assert HOSTS[0][1] == req.name + assert req.state == dns_resolve.ReqState.INVALID + assert req.time == req.result == req.ttl is None + assert str(req) + + +def test_DNSResolver_singleton(): + with DNSResolver(True) as r1: + assert r1._is_daemon + r2 = dns_resolve.DNSResolver(False) + assert r1 == r2 + assert r1._is_daemon + + +def _resolve(resolver, host, fn=dns_resolve.resolve): + """Handle sync/async lookups, giving up if more than 1 second has passed""" + + timeout = 1 + start = time.time() + name = fn(host) + if resolver._is_daemon: + while name is None: + name = fn(host) + if name: + break + time.sleep(0.01) + end = time.time() + if end - start > timeout: + break + if name: + return name + raise LookupError(f"{host} not found") + + +@pytest.mark.parametrize("ip,host,expected", HOSTS) +def test_resolve_addresses(missing_unbound, resolver_factory, ip, host, + expected): + resolver = resolver_factory() + if missing_unbound: + with pytest.raises(LookupError): + _resolve(resolver, host) + else: + result = _resolve(resolver, host) + assert ipaddress.ip_address(expected) == ipaddress.ip_address(result) + + +def test_resolve_unknown_host(missing_unbound, resolver_factory): + resolver = resolver_factory() + with pytest.raises(LookupError): + _resolve(resolver, "fake.notadomain") + + +@skip_no_unbound +def test_resolve_process_error(): + with DNSResolver(True) as resolver: + with mock.patch.object(resolver._ctx, "process", return_value=-1): + assert resolver.resolve("fake.domain") is None + + +@skip_no_unbound +def test_resolve_resolve_error(): + with DNSResolver(False) as resolver: + with mock.patch.object(resolver._ctx, "resolve", + return_value=(-1, None)): + assert resolver.resolve("fake.domain") is None + + +@skip_no_unbound +def test_resolve_resolve_async_error(): + with DNSResolver(True) as resolver: + with mock.patch.object(resolver._ctx, "resolve_async", + return_value=(-1, None)): + with pytest.raises(LookupError): + _resolve(resolver, "fake.domain") + + +@pytest.mark.parametrize("file,raises", + [(None, False), + ("missing_file", dns_resolve.UnboundException), + ("unbound_conf", False)]) +def test_set_unbound_conf(monkeypatch, missing_unbound, resolver_factory, + request, file, raises): + if file: + file = str(request.getfixturevalue(file)) + monkeypatch.setenv("OVS_UNBOUND_CONF", file) + resolver = resolver_factory() # Doesn't raise + if missing_unbound: + assert resolver._set_unbound_conf() is None + return + with mock.patch.object(resolver._ctx, "config", + side_effect=resolver._ctx.config) as c: + if raises: + with pytest.raises(raises): + resolver._set_unbound_conf() + else: + resolver._set_unbound_conf() + if file: + c.assert_called_once_with(file) + else: + c.assert_not_called() + + +@pytest.mark.parametrize("file,raises", + [(None, False), + ("missing_file", dns_resolve.UnboundException), + ("resolv_conf", False)]) +def test_resolv_conf(monkeypatch, missing_unbound, resolver_factory, request, + file, raises): + if file: + file = str(request.getfixturevalue(file)) + monkeypatch.setenv("OVS_RESOLV_CONF", file) + resolver = resolver_factory() # Doesn't raise + if missing_unbound: + assert resolver._set_resolv_conf() is None + return + with mock.patch.object(resolver._ctx, "resolvconf", + side_effect=resolver._ctx.resolvconf) as c: + if raises: + with pytest.raises(raises): + resolver._set_resolv_conf() + else: + resolver._set_resolv_conf() + c.assert_called_once_with(file) + + +@pytest.mark.parametrize("file,raises", + [(None, False), + ("missing_file", dns_resolve.UnboundException), + ("hosts_file", False)]) +def test_hosts(monkeypatch, missing_unbound, resolver_factory, request, file, + raises): + if file: + file = str(request.getfixturevalue(file)) + monkeypatch.setenv("OVS_HOSTS_FILE", file) + resolver = resolver_factory() # Doesn't raise + if missing_unbound: + assert resolver._set_hosts_file() is None + return + with mock.patch.object(resolver._ctx, "hosts", + side_effect=resolver._ctx.hosts) as c: + if raises: + with pytest.raises(raises): + resolver._set_hosts_file() + else: + resolver._set_hosts_file() + c.assert_called_once_with(file) + + +def test_UnboundException(missing_unbound): + with pytest.raises(dns_resolve.UnboundException): + raise dns_resolve.UnboundException("Fake exception", -1) + + +@skip_no_unbound +@pytest.mark.parametrize("ip,host,expected", HOSTS) +def test_inet_parse_active(resolver_factory, ip, host, expected): + resolver = resolver_factory() + + def fn(name): + # Return the same thing _resolve() would so we can call + # this multiple times for the is_daemon=True case + return socket_util.inet_parse_active(f"{name}:6640", 6640, + raises=False)[0] or None + + # parsing IPs still works + IP = _resolve(resolver, ip, fn) + assert ipaddress.ip_address(ip) == ipaddress.ip_address(IP) + # parsing hosts works + IP = _resolve(resolver, host, fn) + assert ipaddress.ip_address(IP) == ipaddress.ip_address(expected) diff --git a/python/setup.py b/python/setup.py index 27684c404..bcf832ce9 100644 --- a/python/setup.py +++ b/python/setup.py @@ -99,8 +99,7 @@ setup_args = dict( 'Topic :: System :: Networking', 'License :: OSI Approved :: Apache Software License', 'Programming Language :: Python :: 3', - 'Programming Language :: Python :: 3.4', - 'Programming Language :: Python :: 3.5', + 'Programming Language :: Python :: 3.6', ], ext_modules=[setuptools.Extension("ovs._json", sources=["ovs/_json.c"], @@ -110,7 +109,8 @@ setup_args = dict( cmdclass={'build_ext': try_build_ext}, install_requires=['sortedcontainers'], extras_require={':sys_platform == "win32"': ['pywin32 >= 1.0'], - 'flow': ['netaddr', 'pyparsing']}, + 'flow': ['netaddr', 'pyparsing'], + 'dns': ['unbound']}, ) try: diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in index 44899c1ca..343a5716d 100644 --- a/rhel/openvswitch-fedora.spec.in +++ b/rhel/openvswitch-fedora.spec.in @@ -113,7 +113,7 @@ Summary: Open vSwitch python3 bindings License: ASL 2.0 BuildArch: noarch Requires: python3 -Suggests: python3-netaddr python3-pyparsing +Suggests: python3-netaddr python3-pyparsing python3-unbound %{?python_provide:%python_provide python3-openvswitch = %{version}-%{release}} %description -n python3-openvswitch diff --git a/tests/vlog.at b/tests/vlog.at index 3e92e70a9..785014956 100644 --- a/tests/vlog.at +++ b/tests/vlog.at @@ -385,6 +385,7 @@ AT_CHECK([APPCTL -t test-unixctl.py vlog/list], [0], [dnl console syslog file ------- ------ ------ daemon info info info +dns_resolve info info info fatal-signal info info info jsonrpc info info info poller info info info @@ -404,6 +405,7 @@ unixctl_server info info info console syslog file ------- ------ ------ daemon info err dbg +dns_resolve info info dbg fatal-signal info info dbg jsonrpc info info dbg poller info info dbg
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 <twilson@redhat.com> --- .github/workflows/build-and-test.yml | 4 +- Documentation/intro/install/general.rst | 4 +- Documentation/intro/install/rhel.rst | 2 +- Documentation/intro/install/windows.rst | 2 +- NEWS | 4 +- debian/control.in | 1 + m4/openvswitch.m4 | 8 +- python/TODO.rst | 7 + python/automake.mk | 2 + python/ovs/dns_resolve.py | 272 +++++++++++++++++++++++ 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 + 16 files changed, 601 insertions(+), 18 deletions(-) create mode 100644 python/ovs/dns_resolve.py create mode 100644 python/ovs/tests/test_dns_resolve.py