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

[Andrew Lunn]

> This isn't sphynx, but I've come-up with something like this for a
> test definition :
> 
> 
> @ksft_ethtool_needs_supported_anyof([Pause, Asym_Pause])
> def test_ethtool_pause_advertising(cfg, peer) -> None:
>     """Pause advertisement
> 
>     Validate that changing pause params through the ETHTOOL_MSG_PAUSE command
>     translates to a change in the advertised pause params, and that these
>     parameters are correct w.r.t the supported pause params and requested pause
>     params.
>     
>     This exercises the .set_pauseparams() ethtool ops for MAC configuration,
>     as well as the reconfiguration of the PHY's advertising and negociation.
>     
>     On non-phylink MACs, the MAC should call phy_set_sym_pause() to update the
>     PHY's advertising, and restart a negotiation with phy_start_aneg() if
>     need be. Failure to do so will result on the wrong advertising parameters.
>     
>     Pn phylink-enabled MACs, phylink deals with the PHY reconfiguration provided

On 

>     the MAC driver calls phylink_ethtool_set_pauseparam().
>     
>     Failing this test likely means that the PHY driver is not correctly advertising
>     pause settings, either due to the MAC not triggering a PHY reconfiguration,
>     a misconficonfiguration of the advertising registers by the PHY, or by
>     mis-handling the phydev->advertising bitfield in the PHY driver directly.
>     
>     The validation is made by looking at the advertised modes locally, as well as
>     what the peer's 'lp_advertising' values report.
> 
>     cfg -- local device's interface configuration
>     peer -- peer device handle

Plain Sphinx can be made to pick up this method documentation and
include it the generated documentation. You would use something like

.. automethod:: test_ethtool_pause_advertising

in the .rst file.

I've no idea if the kernel configuration of sphinx allows this. At the
moment, i would not spend too much time on getting sphinx to generate
documentation. I would say that is nice to have. The description
itself is more important.

>     """
> 
>     # Initial conditions :
>     # - Local interface is admin UP, and reports lowlayer link UP
>     # - Remote interface is adming UP, and reports lowlayer link UP
>     #
>     # Test 1
>     # - SKIP if supported doesn't contain "Pause"
>     # - 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
>     #
>     # Test 2
>     # - SKIP uif supported doesn't contain both "Pause" and "Asym_Pause"
>     # - 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"
>     #
>     # ...
>    
> The annotation defines the pre-requisites in terms of locally supported
> linkmodes, we have a docstring containing information for developpers
> to debug their drivers, what I'm unsure about is the commented-out part
> below, so either one big function testing multiple adjacent scenarios
> or indivitual functions.

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.

However, i've no idea if the selftest code allows for classes of test
methods? It looks like ksft_run() takes a list of methods. So you can
probably instantiate the class, and then pass it methods from the
class?

I would say you are right about picking one of the simple test case,
and playing with it, define and implement it, and see what comes out
at the end. 

	Andrew