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 <[email protected]> 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).
>> -

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

Reply via email to