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

[Maxime Chevallier]

On 5/28/26 03:15, Andrew Lunn wrote:
> > +A.1 : Sanity Checks
> > +~~~~~~~~~~~~~~~~~~~
> > +
> > +Pause autoneg is set to off::
> > +
> > +  ethtool -A <iface> autoneg off
> > +
> > +The 'supported' fields retrieved using the ETHTOOL_MSG_LINKMODES_GET includes
> > +a "Pause" bit and an "Asym" bit.
> > +
> > +The ETHTOOL_MSG_PAUSE_GET command returns the currently configured pause modes
> > +in the "tx" and "rx" attributes.
>
> What you have left unspecified here is the state a link autoneg,
>
> ethtool -s <iface> autoneg on|off
>
> At minimum, it should be specified. I've not read the other tests yet,
> but we may also want to run this basic test with both possible
> setting.

True ! With a more formalized test description, it should be easier to
get non-ambiguous starting conditions here.

> One possible bug which we are trying to detect is that pause autoneg
> is off, the values are forced, but -s autoneg is on, it completes, and
> overwrites the forces values with negotiated values.

I'm trying to cover that on the latter tests indeed

> > +A.2 : Half-duplex operation
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +Pause settings as exposed with the ethtool API only concern full-duplex modes.
> > +
> > +Test scenario:
> > +--------------
> > +
> > +Set the interface under test in half-duplex mode with::
> > +
> > +  ethtool -s <iface> duplex half
> > +
> > +Expected behaviour::
> > +
> > + - ethtool <iface>
> > +
> > +shows no Pause settings advertised.
>
> Since we are talking about advertisement of pause, i think you actually want
>
> ethtool -A <iface> autoneg on
> ethtool -s <iface> duplex half autoneg on
>
> If -s autoneg is off, nothing should be advertised. So we need that
> turned on. And if -A autoneg is off, no pause values should be
> advertised, so you need that turned on as well.
>
> Also, since we expect this to trigger an autoneg, we probably want a
> sleep(2) in there before looking at the results, to ensure autoneg has
> completed.
>
> There is also the possibility that the link does not come up, because
> the link peer does not support half duplex. This is quite common for
> 1G interfaces. It could also be the local interface does not support
> 1G half. So maybe
>
> ethtool -s <iface> duplex half speed 100 autoneg on

I think that

  ethtool -s <iface> duplex half autoneg on

should be enough, the link should still establish at 100M, I've tested
that on a 1G/FULL 100MHalf+Full interface and this is the result we
get :)

That said I've tested the following on mcbin, and it seems that acually
nothing in the code currently deals with Half duplex / Pause 
interaction, and we don't get any EOPNOTSUPP.

So the broader question is, should we reflect the current behaviour or
an ideal one ?

And if we reflect the current one, the canonical behaviour is probably
the phylink one ?

> Or let it first autoneg unrestricted, look at both the local and LP
> values, and pick a half duplex link mode both support and set it to do
> that?
>
> The tests defined so far also don't cover all the possible settings of
> -A autoneg and -s autoneg. There are combinations:
>
> Link autoneg with pause autoneg
> Link autoneg with forced pause
> Force link, with forced pause
>
> It would be good to consider what can be tested for these three. Maybe
> nothing can be tested with forced link, because that it likely to
> result in no link, when only the local side can be configured.
>
> > +B : Combined devices testing
> > +============================
> > +
> > +Requirements : The interface under test must be connected to a link-partner that
> > +can be actively configured during the tests. It must at least support full-duplex
> > +modes, and optionally (but ideally) symmetric and asymmetric flow-control, as
> > +well as autonegotiation of the Pause parameters.
>
> and forced link parameters.
>
> > +
> > +B.1 : Autoneg advertising
> > +~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +Goal: Validate that the *advertised* Pause and AsymDir bits match the configured
> > +pausemarams.
>
> typo.
>
> > +
> > +The link-level autonegotiation must be enabled::
> > +
> > +  ethtool <iface> autoneg on
> > +
> > +Pause parameters are set with::
> > +
> > +  ethtool -A <iface> rx <val> tx <val> autoneg on
> > +
> > +Pause advertising is retrieved with::
> > +
> > +  ethtool <iface>
> > +
> > +Case 1
> > +------
> > +
> > +Pause parameters : rx **off** tx **off**
> > +
> > +Expected advertisement : **None** (Pause = 0, AsymDir = 0)
> > +
> > +Case 2
> > +------
> > +
> > +Pause parameters : rx **off** tx **on**
> > +
> > +Expected advertisement : **Transmit-only** (Pause = 0, AsymDir = 1)
> > +
> > +Case 3
> > +------
> > +
> > +Pause parameters : rx **on** tx **off**
> > +
> > +Expected advertisement : **Symmetric receive-only** (Pause = 1, Asymdir = 1)
> > +
> > +Case 4
> > +------
> > +
> > +Pause parameters : rx **on** tx **on**
> > +
> > +Expected advertisement : **Symmetric** (Pause = 1, Asymdir = 0)
>
> We should consider here what happens when the local side only supports
> symmetric pause. We would expect EOPNOTSUPP, or maybe EINVAL. If
> ethtool report:
>
> 	Supported pause frame use: Symmetric Receive-only
>
> not getting an error for the asymmetric settings would be a bug.

Ack, I think all test cases will need to account for the local device
support

> > +
> > +B.2 : Autoneg resolution
> > +~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +Goal: Validate that the Pause and AsymDir negotiation translates to the right
> > +TX and RX pause parameters.
> > +
> > +The following table, from the 802.3 standard, exposes the autoneg resolution
> > +result for the advertised pause parameters by each link partner.
> > +
> > ++-------------+--------------+--------------------------+
> > +|Local device | Link partner | Pause settings resolution|
> > ++------+------+-------+------+-----------+--------------+
> > +|Pause | Asym | Pause | Asym | RX        | TX	        |
>
> There is a tab vs space issue here.
>
>
> > ++======+======+=======+======+===========+==============+
> > +| 0    | 0    | Any   | Any  | No        | No           |
> > ++------+------+-------+------+-----------+--------------+
> > +| 0    | 1    | 0     | Any  | No        | No           |
> > ++------+------+-------+------+-----------+--------------+
> > +| 0    | 1    | 1     | 0    | No        | No           |
> > ++------+------+-------+------+-----------+--------------+
> > +| 0    | 1    | 1     | 1    | No        | Yes          |
> > ++------+------+-------+------+-----------+--------------+
> > +| 1    | 0    | 0     | Any  | No        | No           |
> > ++------+------+-------+------+-----------+--------------+
> > +| 1    | Any  | 1     | Any  | Yes       | Yes          |
> > ++------+------+-------+------+-----------+--------------+
> > +| 1    | 1    | 0     | 0    | No        | No           |
> > ++------+------+-------+------+-----------+--------------+
> > +| 1    | 1    | 0     | 1    | Yes       | No           |
> > ++------+------+-------+------+-----------+--------------+
> > +
> > +The mapping between the configured pause parameters and advertised modes follow
> > +the following truth table :
> > +
> > ++----+----+-------+---------+
> > +| tx | rx | Pause | AsymDir |
> > ++====+====+=======+=========+
> > +| 0  | 0  | 0     | 0       |
> > ++----+----+-------+---------+
> > +| 0  | 1  | 1     | 1       |
> > ++----+----+-------+---------+
> > +| 1  | 0  | 0     | 1       |
> > ++----+----+-------+---------+
> > +| 1  | 1  | 1     | 0       |
> > ++----+----+-------+---------+
> > +
> > +We can boil that down to the following cases to test, keeping the number small
> > +to avoid dealing with the whole combinatory::
>
> Why not do the whole set of combination? There are 16 combinations,
> autoneg takes a little over 1 second, so we are probably talking 32
> seconds in total. That is a reasonable runtime for a test.

True, I'll describe the whole set then :)

> > +Case 1
> > +------
> > +
> > +Local device : rx **off**, tx **off**
> > +Remote device : rx **on** tx **on**
> > +
> > +Expected result on local device after autonegotiation completes :
> > +        rx negotiated **off**
> > +        tx negotiated **off**
> > +
>
> ...
>
> > +Case 7
> > +------
> > +
> > +Local device : rx **on** tx **on**
> > +Remote device : rx **off** tx **off**
> > +
> > +Expected result on local device after autonegotiation completes :
> > +        rx negotiated **off**
> > +        tx negotiated **off**
> > +
> > +Case 8
> > +------
> > +
> > +Local device : rx **on** tx **on**
> > +Remote device : rx **off** tx **on**
> > +
> > +Expected result on local device after autonegotiation completes :
> > +        rx negotiated **on**
> > +        tx negotiated **off**
>
> what also needs to be considered here is:
>
> What if the local side only supports symmetric pause?
> What if the LP only supports symmetric pause?
>
> The expect results should take this into account, that the
> configuration fails, but that is not a test failure, just a hardware
> limitation.
>
> > +
> > +B.3 : Pause Autoneg
> > +~~~~~~~~~~~~~~~~~~~
> > +
> > +Goal: Validate that the Pause autonegotiation flag correctly toggles the
> > +advertised Pause and AsymDir link parameters.
> > +
> > +Test scenario:
> > +--------------
> > +
> > + - Enable pause autoneg and at least rx or tx pause::
> > +
> > +        ethtool -A <iface> rx on tx on autoneg on
> > +
> > + - Check the Advertised pause frame use::
> > +
> > +        ethtool <iface>
> > +
> > +        ...
> > +        Advertised pause frame use: Symmetric Receive-only
> > +
> > + - Disable pause autoneg::
> > +
> > +        ethtool -A <iface> autoneg off
> > +
> > + - Check the Advertised pause frame use, which must be 'No'::
> > +
> > +        ethtool <iface>
> > +
> > +        ...
> > +        Advertised pause frame use: No
>
> Please describe configuration for both sides. This is needed for all
> the tests when there are two devices involved.
>
> Also, when local pause advertisement is turned off, check what the
> link partner is reporting it received from its link partner.

Very well, I'll add this.

Thanks for the extensive feedback Andrew,

Maxime