All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sabrina Dubroca <sd@queasysnail.net>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Wang Liang <wangliang74@huawei.com>,
	andrew@lunn.ch, 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: Tue, 4 Nov 2025 12:04:52 +0100	[thread overview]
Message-ID: <aQnd1HQ-b7wFI2WP@krikkit> (raw)
In-Reply-To: <20251103160133.31c856a4@kernel.org>

2025-11-03, 16:01:33 -0800, Jakub Kicinski wrote:
> On Mon, 3 Nov 2025 11:13:08 +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:  
> > > > 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?
> > 
> > Or should we leave them, but not accept new checks to exclude
> > really-old versions of tools?  Do we need to document the cut-off ("we
> > don't support tool versions older than 2 years for networking
> > selftests" [or similar]) somewhere in Documentation/ ?
> 
> FWIW my current thinking is to prioritize test development and kernel
> needs over the ability to run ksft on random old set of tools and have
> clean skips. IOW avoid complicating writing tests by making the author
> also responsible for testing versions of all tools.

I see. I liked Andrew's idea ("embed the date the requirement was
added into the test"), but it goes completely in the opposite
direction.

Figuring out why exactly a test failed in case of an old tool
(unexpected output passed to some pipe/parsing, exit with a non-zero
code, maybe other issues) is not always obvious. So without version
checks on the tools, I think we have to assume that the test requires
the latest version of all tools it calls (or at least a very recent
one). Which I guess is reasonable for upstream kernel development.

> The list of tools which need to be updated or installed for all
> networking tests to pass is rather long. My uneducated guess
> is all these one off SKIP patches don't amount to much. Here for
> example author is fixing one test, I'm pretty sure that far more
> tests depend on -k --json.

A quick grep found only a few more (in python scripts under
drivers/net) for -k. But (also from a quick grep) many tests seem to
use jq without checking that the command is present.

So I guess you would lean toward not accepting any such patch, not
requiring new tests to have SKIP checks, but leaving any existing
checks in? (and I suspect removing all the existing ones wouldn't
actually reduce the flow of "add check for too old $tool" patches, so
it probably doesn't make sense to do that)

-- 
Sabrina

  reply	other threads:[~2025-11-04 11:04 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
2025-11-03 15:31           ` Andrew Lunn
2025-11-04  0:01       ` Jakub Kicinski
2025-11-04 11:04         ` Sabrina Dubroca [this message]
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=aQnd1HQ-b7wFI2WP@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.