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

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

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


class test_ethtool_pause_advertising:
    """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
    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 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
    """

    def _adv_test(cfg, peer, rx, tx, adv, not_adv):
        ret = cfg.run(f"ethtool -A ethX rx {rx} tx {tx} autoneg on")
        ksft_eq(ret, 0)

        linkmodes = cfg.get_advertising()
        if adv:
            ksft_in(adv, linkmodes, f"rx {rx} tx {tx} must advertise {adv}")

        if not_adv:
            ksft_not_in(not_adv, linkmodes, f"rx {rx} tx {tx} must not advertise {not_adv}")

        remote_linkmodes = peer.get_lp_advertising()

        if adv:
            ksft_in(adv, linkmodes, f"PHY does not advertise {adv}")

        if not_adv:
            ksft_not_in(not_adv, linkmodes, f"PHY incorrectly advertises {not_adv}")


    @ksft_ethtool_needs_supported_allof([Pause])
    def adv_rx_on_tx_on(cfg, peer) -> None:
        """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")


    @ksft_ethtool_needs_supported_allof([Pause, Asym_Pause])
    def adv_rx_on_tx_off(cfg, peer) -> None:
        """Advertising test with rx on tx off

        - run 'ethtool -A ethX rx on tx off autoneg on'
        - FAIL if the return isn't 0
        - FAIL if ETHTOOL_A_LINKMODES_OURS's advertised values does not contain
          "Pause" and "Asym_Pause"
        - FAIL if peer's lp_advertising doesn't contain "Pause" and "Asym_Pause"
        - Succeed otherwise
        """

        _adv_test(cfg, peer, 'on', 'off', ["Pause", "Asym_Pause"], [])

    @ksft_ethtool_needs_supported_allof([Asym_Pause])
    def adv_rx_off_tx_on(cfg, peer) -> None:
        """Advertising test with rx off tx on

        - run 'ethtool -A ethX rx off tx on autoneg on'
        - FAIL if the return isn't 0
        - FAIL if ETHTOOL_A_LINKMODES_OURS's advertised values does not contain
          "Asym_Pause" or contains "Pause"
        - FAIL if peer's lp_advertising doesn't contain "Pause" and "Asym_Pause"
        - Succeed otherwise
        """

        _adv_test(cfg, peer, 'off', 'on', ["Asym_Pause"], ["Pause"])


Maxime