Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Message-ID: <96bdb031129cdebfa6e0bdd4342439d9d864518b.camel@redhat.com>
Subject: Re: [PATCH net 1/4] selftests: openvswitch: Add version check for
 pyroute2
From:   Paolo Abeni <pabeni@redhat.com>
To:     Aaron Conole <aconole@redhat.com>, netdev@vger.kernel.org
Cc:     dev@openvswitch.org, linux-kselftest@vger.kernel.org,
        linux-kernel@vger.kernel.org, Pravin B Shelar <pshelar@ovn.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Adrian Moreno <amorenoz@redhat.com>,
        Eelco Chaudron <echaudro@redhat.com>
Date:   Tue, 10 Oct 2023 12:25:51 +0200
In-Reply-To: <20231006151258.983906-2-aconole@redhat.com>
References: <20231006151258.983906-1-aconole@redhat.com>
         <20231006151258.983906-2-aconole@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

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.
>=20
> 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.
>=20
> Fixes: 25f16c873fb1 ("selftests: add openvswitch selftest suite")
> Reported-by: Paolo Abeni <pabeni@redhat.com>
> Closes: https://lore.kernel.org/lkml/8470c431e0930d2ea204a9363a60937289b7=
fdbe.camel@redhat.com/
> Signed-off-by: Aaron Conole <aconole@redhat.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(-)
>=20
> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/too=
ls/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
> =20
>  	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; th=
en
>  		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 =3D ovskey
>      nlmsg_atoms.ovsactions =3D ovsactions
> =20
> +    # version check for pyroute2
> +    prverscheck =3D pyroute2.__version__.split(".")
> +    if int(prverscheck[0]) =3D=3D 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.

Cheers,

Paolo