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.

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

Reply via email to