netdev - Re: [PATCH net 1/4] selftests: openvswitch: Add version check for pyroute2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f7tedi1cm5i.fsf@redhat.com>
Date: Wed, 11 Oct 2023 09:40:41 -0400
From: Aaron Conole <aconole@...hat.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org,  dev@...nvswitch.org,
  linux-kselftest@...r.kernel.org,  linux-kernel@...r.kernel.org,  Pravin B
 Shelar <pshelar@....org>,  "David S. Miller" <davem@...emloft.net>,  Eric
 Dumazet <edumazet@...gle.com>,  Jakub Kicinski <kuba@...nel.org>,  Adrian
 Moreno <amorenoz@...hat.com>,  Eelco Chaudron <echaudro@...hat.com>
Subject: Re: [PATCH net 1/4] selftests: openvswitch: Add version check for
 pyroute2

Paolo Abeni <pabeni@...hat.com> writes:

> On Fri, 2023-10-06 at 11:12 -0400, Aaron Conole wrote:
>> Paolo Abeni reports that on some systems the pyroute2 version isn't
>> new enough to run the test suite.  Ensure that we support a minimum
>> version of 0.6 for all cases (which does include the existing ones).
>> The 0.6.1 version was released in May of 2021, so should be
>> propagated to most installations at this point.
>> 
>> The alternative that Paolo proposed was to only skip when the
>> add-flow is being run.  This would be okay for most cases, except
>> if a future test case is added that needs to do flow dump without
>> an associated add (just guessing).  In that case, it could also be
>> broken and we would need additional skip logic anyway.  Just draw
>> a line in the sand now.
>> 
>> Fixes: 25f16c873fb1 ("selftests: add openvswitch selftest suite")
>> Reported-by: Paolo Abeni <pabeni@...hat.com>
>> Closes: https://lore.kernel.org/lkml/8470c431e0930d2ea204a9363a60937289b7fdbe.camel@redhat.com/
>> Signed-off-by: Aaron Conole <aconole@...hat.com>
>> ---
>>  tools/testing/selftests/net/openvswitch/openvswitch.sh | 2 +-
>>  tools/testing/selftests/net/openvswitch/ovs-dpctl.py   | 8 ++++++++
>>  2 files changed, 9 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
>> index 9c2012d70b08e..220c3356901ef 100755
>> --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
>> +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
>> @@ -525,7 +525,7 @@ run_test() {
>>  	fi
>>  
>>  	if python3 ovs-dpctl.py -h 2>&1 | \
>> -	     grep "Need to install the python" >/dev/null 2>&1; then
>> +	     grep -E "Need to (install|upgrade) the python" >/dev/null 2>&1; then
>>  		stdbuf -o0 printf "TEST: %-60s  [PYLIB]\n" "${tdesc}"
>>  		return $ksft_skip
>>  	fi
>> diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
>> index 912dc8c490858..9686ca30d516d 100644
>> --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
>> +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
>> @@ -28,6 +28,8 @@ try:
>>      from pyroute2.netlink import nlmsg_atoms
>>      from pyroute2.netlink.exceptions import NetlinkError
>>      from pyroute2.netlink.generic import GenericNetlinkSocket
>> +    import pyroute2
>> +
>>  except ModuleNotFoundError:
>>      print("Need to install the python pyroute2 package.")
>>      sys.exit(0)
>> @@ -1998,6 +2000,12 @@ def main(argv):
>>      nlmsg_atoms.ovskey = ovskey
>>      nlmsg_atoms.ovsactions = ovsactions
>>  
>> +    # version check for pyroute2
>> +    prverscheck = pyroute2.__version__.split(".")
>> +    if int(prverscheck[0]) == 0 and int(prverscheck[1]) < 6:
>> +        print("Need to upgrade the python pyroute2 package.")
>
> I think it would be better to propagate/print also the minimum version
> required, so that the user should not have to resort looking at the
> self-test sources to learn the required minimum version.

ACK - makes sense to me.

> Cheers,
>
> Paolo