Re: [PATCH net-next] Documentation: networking: Add a test plan for ethtool pause validation

[Maxime Chevallier]

Hi,

On 6/26/26 14:39, Andrew Lunn wrote:
> On Fri, Jun 26, 2026 at 10:33:50AM +0200, Maxime Chevallier wrote:
>>
>>> Sphinx follows pythons object orientate structure. So you could have a
>>> class test_ethtool_pause_advertising, with class documentation. And
>>> then methods within the class which are individual tests.  The
>>> commented out section would then be method documentation.
>>
>> Good point, so maybe something along these lines :
>>
>>  - A class for the test
>>  - methods for indivitual tests
>>  - For readability, I've written what the internal test helper would look
>>    like (_adv_test), and how a test would look like without the helper in
>>    adv_rx_on_tx_on().
>>
>> I'm already diving into coding, but it helps me a bit in the definition of the
>> "description" format :)
>>
>> this is what the class would look like :
> 
> I like this :-)

Great :)

> 
>>
>>
>>     @ksft_ethtool_needs_supported_allof([Pause])
>>     def adv_rx_on_tx_on(cfg, peer) -> None:
> 
> Using decorators is a nice idea. Since it is not a C concept, please
> give the decorator a good comment explaining what it does. We should
> not assume driver developers know python.

No problem, I'll add that

> 
>>         """Advertising test with rx on tx on
>>
>>         - run 'ethtool -A ethX rx on tx on autoneg on'
>>         - FAIL if the return isn't 0
>>         - FAIL if ETHTOOL_A_LINKMODES_OURS's advertised values does not contain
>>           "Pause" or contains "Asym_Pause"
>>         - FAIL if peer's lp_advertising doesn't contain "Pause" or contains
>>           "Asym_Pause"
>>         - Succeed otherwise
>>         """
>>         ret = cfg.run('ethtool -A ethX rx on tx on autoneg on')
>>         ksft_eq(ret, 0)
>>
>>         linkmodes = cfg.get_advertising()
>>         ksft_in('Pause', linkmodes, "rx on tx on must advertise Pause")
>>         ksft_not_in('Asym_Pause', linkmodes, "rx on tx on must not advertise Asym_Pause")
>>
>>         remote_linkmodes = peer.get_lp_advertising()
>>         ksft_in('Pause', linkmodes, "PHY does not advertise Pause")
>>         ksft_not_in('Asym_Pause', linkmodes, "PHY incorrectly advertises Asym_Pause")
> 
> There should be a sleep in here somewhere, to allow the autoneg to
> complete.

Indeed, I think in the end this will be wrapped by some ksft_ethtool_* helper we'll add,
that will also deal with the case where autoneg doesn't succeed and the link stays down.

That's both for error detections, but I also expect there might be cases we'll want to test
that autoneg does not actually succeed.

Good to see we're closing in on a definition, I'll spin V2 based on that format :)

Maxime