netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sabrina Dubroca <sd@queasysnail.net>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Jakub Kicinski <kuba@kernel.org>,
	Wang Liang <wangliang74@huawei.com>,
	davem@davemloft.net, edumazet@google.com, pabeni@redhat.com,
	shuah@kernel.org, horms@kernel.org, netdev@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
	yuehaibing@huawei.com, zhangchangzhong@huawei.com
Subject: Re: [PATCH net] selftests: netdevsim: Fix ethtool-features.sh fail
Date: Mon, 3 Nov 2025 16:01:00 +0100	[thread overview]
Message-ID: <aQjDrMH34QVz6e1E@krikkit> (raw)
In-Reply-To: <e014c4c5-105a-43cb-9411-ec139af2b2a1@lunn.ch>

2025-11-03, 14:36:00 +0100, Andrew Lunn wrote:
> On Mon, Nov 03, 2025 at 11:13:08AM +0100, Sabrina Dubroca wrote:
> > 2025-10-30, 17:02:17 -0700, Jakub Kicinski wrote:
> > > On Fri, 31 Oct 2025 00:13:59 +0100 Sabrina Dubroca wrote:
> > > > >  set -o pipefail
> > > > >  
> > > > > +if ! ethtool --json -k $NSIM_NETDEV > /dev/null 2>&1; then  
> > > > 
> > > > I guess it's improving the situation, but I've got a system with an
> > > > ethtool that accepts the --json argument, but silently ignores it for
> > > >  -k (ie `ethtool --json -k $DEV` succeeds but doesn't produce a json
> > > > output), which will still cause the test to fail later.
> > > 
> > > And --json was added to -k in Jan 2022, that's pretty long ago.
> > > I'm not sure we need this aspect of the patch at all..
> > 
> > Ok.  Then maybe a silly idea: for the tests that currently have some
> > form of "$TOOL is too old" check, do we want to remove those after a
> > while? If so, how long after the feature was introduced in $TOOL?
> 
> Another option is to turn them into a hard fail, after X years.

If the "skip if too old" check is removed, the test will fail when run
with old tools (because whatever feature is needed will not be
supported, so somewhere in the middle of test execution there will be
a failure - but the developer will have to figure out "tool too old"
from some random command failing).

"check version + hard fail" makes it clear, but the (minor) benefit of
simply dropping the check is removing a few unneeded lines.

> My
> guess is, tests which get skipped because the test tools are too old
> frequently get ignored. Tests which fail are more likely to be looked
> at, and the tools updated.
> 
> Another idea is have a dedicated test which simply tests the versions
> of all the tools. And it should only pass if the installed tools are
> sufficiently new that all test can pass. If you have tools which are
> in the grey zone between too old to cause skips, but not old enough to
> cause fails, you then just have one failing test you need to turn a
> blind eye to.

That's assumming people run all the tests every time. Is that really
the case, or do people often run the 2-5 tests that cover the area
they care about? For example it doesn't make much sense to run nexthop
and TC tests for a macsec patch (and the other way around). If my
iproute is too old to run some nexthop or TC tests, I can still run
the tests I really need for my patch.

But maybe if the tests are run as "run everything" (rather than
manually running a few of them), ensuring all the needed tools are
recent enough makes sense.

-- 
Sabrina

  reply	other threads:[~2025-11-03 15:01 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-30  3:22 [PATCH net] selftests: netdevsim: Fix ethtool-features.sh fail Wang Liang
2025-10-30 23:13 ` Sabrina Dubroca
2025-10-31  0:02   ` Jakub Kicinski
2025-11-03 10:13     ` Sabrina Dubroca
2025-11-03 13:36       ` Andrew Lunn
2025-11-03 15:01         ` Sabrina Dubroca [this message]
2025-11-03 15:31           ` Andrew Lunn
2025-11-04  0:01       ` Jakub Kicinski
2025-11-04 11:04         ` Sabrina Dubroca
2025-11-05  0:51           ` Jakub Kicinski
2025-11-03  8:58   ` Wang Liang
2025-11-03  9:59     ` Sabrina Dubroca

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aQjDrMH34QVz6e1E@krikkit \
    --to=sd@queasysnail.net \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=shuah@kernel.org \
    --cc=wangliang74@huawei.com \
    --cc=yuehaibing@huawei.com \
    --cc=zhangchangzhong@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).