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

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

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:
>> 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.
> 
>> +    :param cfg: test config
> 
> If you have nothing to say - don't say anything 🤷️
> 
>> +    :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.

> 
>> +        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