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 <[email protected]> >>>>>> --- >>>>>> .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. >>>> >>> >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
