[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e933e67f-66f2-422b-b00e-09ae788ed51d@lunn.ch>
Date: Fri, 20 Dec 2024 10:09:06 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
pabeni@...hat.com, shuah@...nel.org, willemb@...gle.com,
petrm@...dia.com, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next] selftests: drv-net: assume stats refresh is 0
if no ethtool -c support
On Thu, Dec 19, 2024 at 04:31:16PM -0800, Jakub Kicinski wrote:
> Tests using HW stats wait for them to stabilize, using data from
> ethtool -c as the delay. Not all drivers implement ethtool -c
> so handle the errors gracefully.
>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
> CC: shuah@...nel.org
> CC: willemb@...gle.com
> CC: petrm@...dia.com
> CC: linux-kselftest@...r.kernel.org
> ---
> tools/testing/selftests/drivers/net/lib/py/env.py | 9 +++++++--
> tools/testing/selftests/net/lib/py/utils.py | 6 ++++--
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py
> index 1ea9bb695e94..fea343f209ea 100644
> --- a/tools/testing/selftests/drivers/net/lib/py/env.py
> +++ b/tools/testing/selftests/drivers/net/lib/py/env.py
> @@ -5,7 +5,7 @@ import time
> from pathlib import Path
> from lib.py import KsftSkipEx, KsftXfailEx
> from lib.py import ksft_setup
> -from lib.py import cmd, ethtool, ip
> +from lib.py import cmd, ethtool, ip, CmdExitFailure
> from lib.py import NetNS, NetdevSimDev
> from .remote import Remote
>
> @@ -234,7 +234,12 @@ from .remote import Remote
> Good drivers will tell us via ethtool what their sync period is.
> """
> if self._stats_settle_time is None:
> - data = ethtool("-c " + self.ifname, json=True)[0]
> + data = {}
> + try:
> + data = ethtool("-c " + self.ifname, json=True)[0]
> + except CmdExitFailure as e:
> + if "Operation not supported" not in e.cmd.stderr:
> + raise
How important is this time to the test itself? If it is not available,
can the test just default to 50ms and keep going? I would of thought
we find more issues by running the test too slowly, than not running
it at all, unless having the wrong timer makes it more flaky.
Andrew
Powered by blists - more mailing lists