Re: [PATCH net-next 1/3] selftests: drv-net: Introduce a selftest for ethtool flow control

Hi Jakub,

On 9/23/26 04:42, Jakub Kicinski wrote:

On Sun, 20 Sep 2026 18:47:27 +0200 Maxime Chevallier (Netdev
Foundation) wrote:

quoted

Future ethtool selftests are expected to use the same, these helpers
are put in the net selftest lib.

Please resist the urge to create libraries.

If you're doing this I suspect you didn't even read the README and 
you expect people to search around for libraries?

I did read the README and fought the urge, that was something Andrew
was asking for in the previous round :( I'm ok with either TBH.

Put the code in your test, the duplication doesn't matter.
If you _absolutely have to_ you can create a local lib under
tools/testing/selftests/drivers/net/hw/ but please think really 
hard before you do.

quoted

+    :param cfg: test config

If you have nothing to say - don't say anything 🤷️

quoted

+    :returns: tuple containing :
+              - return code of the ethtool command,
+              - rx status,
+              - tx status,
+              - aneg status

Have you seen much code in netdev selftests using this style 
of comments?

Hmm that's a leftover from the previous round that was pretty much only
coments, I can trim that down.

quoted

+        defer(cmd, f"ethtool -s {cfg.ifname} autoneg on", fail=False)

Why are you putting fail=False on the defers? IIRC defer errors are
logged and ignored. Is ethtool reporting an error when there's nothing
to change?

IIRC I had the issue when running that on a USB dongle, it's likely this
is something I put there and actually forgot to remove after finding a
better solution :(

Those are just some things that immediately jump out :)
Please also fix the pylint --disable=R issues?

Sure no problem,

Thanks a lot for taking a look,

Maxime