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 <twil...@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.
>>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
