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

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

[Maxime Chevallier (Netdev Foundation)]

Flow control configuration through Ethernet Pause frames is not
straightforward to get right when implementing a MAC or a PHY driver.

This is often a feature that's poorly tested during development, and
getting it right requires correct configuration both in MAC drivers,
that implement the flow-control, and the PHY drivers, which deal with
the negotiation of the pause parameters.

As part of the Netdev Foundation effort to improve driver testing, and to ease
the development and review effort, The netdev mailing list has been surveyed
to identify the common pitfalls. This testing plan has been written to try and
catch most of the common problems.

This document is aimed as being a human readable basis we can adjust and
discuss before diving into the actual tests implementation, it may not
necessarily have to live in the kernel doc forever, especially once
tests are implemented.

Note that Oleksij Rempel made some attempts at documenting Ethernet Flow-control
before [1], this current work isn't meant to superseed it but rather
complement it with a testing protocol.

[1] : https://lore.kernel.org/netdev/20260304094811.2779953-1-o.rempel@pengutronix.de/

Signed-off-by: Maxime Chevallier (Netdev Foundation) <maxime.chevallier@bootlin.com>
---

Temptative approach to send that as part of the kdoc, this may not be
the best path, as this could simply be some email thread.

 Documentation/networking/index.rst           |   1 +
 Documentation/networking/pause_test_plan.rst | 556 +++++++++++++++++++
 2 files changed, 557 insertions(+)
 create mode 100644 Documentation/networking/pause_test_plan.rst

diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
index 44a422ad3b05..fef54999267d 100644
--- a/Documentation/networking/index.rst
+++ b/Documentation/networking/index.rst
@@ -91,6 +91,7 @@ Contents:
    openvswitch
    operstates
    packet_mmap
+   pause_test_plan
    phonet
    phy-link-topology
    phy-port
diff --git a/Documentation/networking/pause_test_plan.rst b/Documentation/networking/pause_test_plan.rst
new file mode 100644
index 000000000000..5ff9860efbe5
--- /dev/null
+++ b/Documentation/networking/pause_test_plan.rst
@@ -0,0 +1,556 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Ethtool Pause testing
+=====================
+
+This document describes the test plan to be used for the ethtool selftest, aimed
+at providing a validation framework for Ethernet drivers that implement flow-control.
+
+To ease readability, the document uses the "ethtool" command to describe test cases,
+actual tests may use the netlink API directly.
+
+This document was written after surveying the common error patterns for pause-related
+patches on the netdev mailing list. The common mistakes are usually found in either
+MAC drivers that don't make use of phylink, or PHY drivers that interfere with
+pause settings.
+
+The main classes of problems are :
+
+ - MAC drivers incorrectly configuring the Pause/Asym capabilities in the attached
+   PHY device through calling phy_support_asym_pause() and phy_support_sym_pause().
+
+   These are used to allow the PHY do know what Pause settings the MAC supports, in
+   order to advertise it to the LP.
+
+   They aren't to be used to configure the Pause advertisement, and the MAC driver
+   needs to actually configure Pause on its side after negotiation finishes.
+
+   Using phylink is the easiest fix for this kind of problem.
+
+ - PHY drivers overriding the Pause/AsymPause bits in the supported/advertising
+   fields. Pause is a MAC feature, but the PHY negotiates the settings with the
+   partner. The PHY driver shouldn't have to tweak Pause settings, only advertise
+   them and report the negotiation results.
+
+ - MAC drivers confusing phydev->autoneg and pause autoneg. Pause autoneg is about
+   whether or not we want to negotiate the Pause settings with the partner, or use
+   locally enforced settings. phydev->autoneg, also referred to as link-wide autoneg
+   is about whether or not we want to negotiate anything with the link partner,
+   including speed, duplex, pause, etc. Both phydev->autoneg and pause autoneg can
+   be configured independently.
+
+ - MAC drivers mishandling the pause parameters in the ethtool get/set_pauseparam
+   callbacks, usually by ignoring the pause autoneg settings.
+
+The hardware capabilities regarding Pause support are reported through the
+linkmodes bits 'ETHTOOL_LINK_MODE_Pause' and 'ETHTOOL_LINK_MODE_Asym_Pause', the
+four different combinations are accepted and together describe the ability for
+the hardware to send TX pause frames, and process the received ones :
+
+ - **Pause** bit : the interface has the ability to Receive and process Pause frames
+ - **Asym** bit (Or AsymDir) : The TX and RX pause setting can be different.
+
+This translates to :
+
+ - **Pause** & **Asym** : We can support **ALL** combinations of rx/tx
+
+ - **Pause only** : only **rx == tx** configurations are valid
+
+ - **Asym** only : **rx** must be **off**, **tx** can be **on or off**
+
+Similarly, the link partner can advertise its capabilities through the same
+bits. These parameters are exchanged through the negotiation process, but can
+also be enforced locally by disabling **pause autoneg**, thus ignoring the
+link partner's capabilities.
+
+The local resolution of the pause configuration after receiving the link-partner
+abilities is done according to the following table, from 802.3 Annex 28B.3 :
+
++-------------+--------------+--------------------------+
+|Local device | Link partner | Pause settings resolution|
++------+------+-------+------+-----------+--------------+
+|Pause | Asym | Pause | Asym | RX        | TX           |
++======+======+=======+======+===========+==============+
+| 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 currently configured and advertised settings can be queried with ::
+
+  ethtool <iface>
+
+  Settings for eth0:
+        ...
+	Supported pause frame use: Symmetric Receive-only
+        ...
+	Advertised pause frame use: Symmetric Receive-only
+        ...
+	Link partner advertised pause frame use: Symmetric Receive-only
+
+While testing can be done directly with the Netlink API, as a reminder here's
+what the ethtool output translates to :
+
++------------------------+-------+---------+
+|     Ethtool output     | Pause | AsymDir |
++========================+=======+=========+
+|          No            |   0   |    0    |
++------------------------+-------+---------+
+|     Transmit-only      |   0   |    1    |
++------------------------+-------+---------+
+|       Symmetric        |   1   |    0    |
++------------------------+-------+---------+
+| Symmetric Receive-only |   1   |    1    |
++------------------------+-------+---------+
+
+Mac drivers's current pause behaviour is reported through the
+ethtool -a/--show-pause command, e.g.::
+
+        ethtool -a <iface>
+
+        Autonegotiate:	on
+        RX:		off
+        TX:		off
+        RX negotiated: on
+        TX negotiated: on
+
+When **pause autoneg** is on, the currently used pause parameters are the
+*negotiated* ones, that may differ from the user-specified rx and tx values.
+
+When **pause autoneg** is off, the rx and tx values are the ones used by the
+hardware.
+
+A : Standalone driver testing
+=============================
+
+Requirements : The interface under test must be connected to a link-partner whose
+interface is admin-up. We don't require the link-partner to be configured in any
+other specific manner for these tests.
+
+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.
+
+These parameters must validate against the following truth table :
+
+   +--------------+-----------------+
+   | linkmodes    | pauseparam      |
+   +-------+------+--------+--------+
+   | Pause | Asym | rx     | tx     |
+   +=======+======+========+========+
+   | 0     | 0    | 0      | 0      |
+   +-------+------+--------+--------+
+   | 0     | 1    | 0      | 0 or 1 |
+   +-------+------+--------+--------+
+   | 1     | 0    |     rx == tx    |
+   +-------+------+--------+--------+
+   | 1     | 1    | 0 or 1 | 0 or 1 |
+   +-------+------+--------+--------+
+
+Test scenario
+-------------
+
+Following the reported value from::
+
+        ethtool <iface>
+        Settings for <iface>:
+                ...
+	        Supported pause frame use: <value>
+
+Iterate over all the 4 combinations of rx and tx pause parameters::
+
+        ethtool -A <iface> autoneg off rx <rx_val> tx <tx_val>
+
+The settings must be accepted or rejected, according to the above truth table.
+
+Failing this test likely means the MAC driver is incorrectly setting the PHY's
+Pause and AsymDir bits, or that the PHY driver overwrites the Pause and AsymDir
+supported bits.
+
+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.
+
+Should this test fail, it likely means that either the MAC driver incorrectly
+calls phydev_set_asym_pause(), or that the PHY driver overrides the settings.
+
+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.
+
+B.1 : Autoneg advertising
+~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Goal: Validate that the *advertised* Pause and AsymDir bits match the configured
+pausemarams.
+
+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)
+
+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	        |
++======+======+=======+======+===========+==============+
+| 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::
+
+        ethtool -A <iface> autoneg on rx <rx_param> tx <tx_param>
+
+Failing tests here would likely indicate that the MAC driver doesn't correctly
+accounts for the negotiated Pause parameters in its .adjust_link() callback. A
+MAC driver that uses phylink should pass these tests.
+
+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 2
+------
+
+Local device : rx **off** tx **on**
+Remote device : rx **off** tx **on**
+
+Expected result on local device after autonegotiation completes :
+        rx negotiated **off**
+        tx negotiated **off**
+
+Case 3
+------
+
+Local device : rx **off** tx **on**
+Remote device : rx **on** tx **off**
+
+Expected result on local device after autonegotiation completes :
+        rx negotiated **off**
+        tx negotiated **on**
+
+Case 4
+------
+
+Local device : rx **off** tx **on**
+Remote device : rx **on** tx **on**
+
+Expected result on local device after autonegotiation completes :
+        rx negotiated **off**
+        tx negotiated **off**
+
+This case looks surprising but boils down to the fact that we advertise Asym only,
+whereas the LP is advertising Pause (and not Pause + Asym).
+
+As per 802.3 this resolves to all "Off".
+
+Case 5
+------
+
+Local device : rx **on** tx **off**
+Remote device : rx **off** tx **on**
+
+Expected result on local device after autonegotiation completes :
+        rx negotiated **on**
+        tx negotiated **off**
+
+Case 6
+------
+
+Local device : rx **on** tx **on**
+Remote device : rx **on** tx **off**
+
+Expected result on local device after autonegotiation completes :
+        rx negotiated **on**
+        tx negotiated **on**
+
+This is also not an obvious result. We advertise Pause, the Link partner advertises
+Pause + Asym, so it resolves to all "On".
+
+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**
+
+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
+
+B.4 : Pause autoneg transition
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Goal: Validate the advertising and pause parameters upon toggling pause
+      autonegotiation. When pause autoneg is disabled, the local pause settings
+      are dictated by the user configuration, without relying on negotiated
+      parameters.
+
+This validation focuses on cases where the pause autoneg result differs from the
+user-specified rx and tx pauseparams, e.g. :
+
+Local device : rx **on** tx **off**
+Remote device : rx **on** tx **on**
+
+This will negotiate into rx and tx being **on**, contrary to the user intent.
+
+Test scenario:
+--------------
+
+ - Enable link-wide autonegotiation::
+
+        ethtool -s <iface> autoneg on
+
+ - Enable Pause autonegotiation, setting RX on and TX off::
+
+        ethtool -A <iface> rx on tx off autoneg on
+
+ - Expected result::
+
+        ethtool -a <iface>
+
+        Pause parameters for <iface>:
+        Autonegotiate:	on
+        RX:		on
+        TX:		off
+        RX negotiated: on
+        TX negotiated: on
+
+Note that the currently applied configuration here is expressed by the "negotiated"
+parameters, so the link is currently using RX and TX Pauses.
+
+ - Disable pause autonegotiation::
+
+        ethtool -A <iface> autoneg off
+
+ - Expected result::
+
+        ethtool -a <iface>
+
+        Pause parameters for <iface>:
+        Autonegotiate:	off
+        RX:		on
+        TX:		off
+
+Note that now, the currently applied configuration really is RX on TX off. It is
+expected that upon running the above command, the link flips down and up again
+as flow-control parameters are updated.
+
+ - Re-enable pause autonegotiation::
+
+        ethtool -A <iface> autoneg on
+
+ - Expected result::
+
+        ethtool -a <iface>
+
+        Pause parameters for <iface>:
+        Autonegotiate:	on
+        RX:		on
+        TX:		off
+        RX negotiated: on
+        TX negotiated: on
+
+The user-defined pause parameters needs to be again converted to advertised Pause
+and Asym parameters, negotiated and the final outcome must be the same as the original
+conditions.
+
+B.5 : Pause autoneg only to Pause and Link autoneg transition
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Goal: Validate that drivers store the user intent in the pause rx/tx attributes
+when disabling link autoneg, but *keeping the pause autoneg enabled*.
+
+When pause autoneg is enabled, the rx and tx pause parameters are the user intent,
+but can be different to the actual pause parameters actively in use.
+
+Test scenario:
+--------------
+
+ - Configure rx and tx pause parameters to **on** and pause autoneg to **on**::
+
+        ethtool -A <iface> autoneg on rx on tx on
+
+ - Disable link autonegotiation::
+
+        ethtool -s <iface> autoneg off
+
+ - Configure rx and tx pause to **rx off**, **tx on** and autoneg to **on**::
+
+        ethtool -A <iface> autoneg on rx off tx on
+
+ - No change in the pause parameters should occur
+ - Re-enable link negotiation::
+
+        ethtool -s <iface> autoneg on
+
+ - Link must now re-negotiate pause to **rx off** and **tx on**::
+
+        ethtool -a <iface>
+
+        Pause parameters for <iface>:
+        Autonegotiate:	on
+        RX:		off
+        TX:		on
+        RX negotiated: off
+        TX negotiated: on
+
-- 
2.54.0

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

[Jakub Kicinski]

On Fri, 22 May 2026 19:51:06 +0200 Maxime Chevallier (Netdev
Foundation) wrote:
>  Documentation/networking/pause_test_plan.rst | 556 +++++++++++++++++++

It'd be great to hear from others but IMHO in the current form this is
not suitable for Documentation/networking/ We can commit the "knowledge"
part but enumerating the test cases seems odd for Documentation/.

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

[Andrew Lunn]

On Tue, May 26, 2026 at 05:24:47PM -0700, Jakub Kicinski wrote:
> On Fri, 22 May 2026 19:51:06 +0200 Maxime Chevallier (Netdev
> Foundation) wrote:
> >  Documentation/networking/pause_test_plan.rst | 556 +++++++++++++++++++
> 
> It'd be great to hear from others but IMHO in the current form this is
> not suitable for Documentation/networking/ We can commit the "knowledge"
> part but enumerating the test cases seems odd for Documentation/.

Sorry, not looked too deeply at the actual content yet.

What i was thinking was a python file, which sphinx can ingest to
produce documentation, and place holders were code would be added to
implement the actual test during the next phase.

This is how i've done testing in the past. I would be the evil one who
thought up the tests and described them in detail using sphinx markup
in a python test template file. After some review they got passed off
to a python developer for implementation. And when they got run and
failed, sometimes the feature developer, the test developer and myself
got together to figure who made the error.

I'm not sure we even need sphinx. What i find important is that the
test is documented. What kAPI calls should be made with what
parameters. What results we are expected and why? So that when a test
fails, a developer has the information they need to fix their
code. The Why? is important, and often missing from the kernel tests.

	Andrew

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

[Andrew Lunn]

> +   These are used to allow the PHY do know what Pause settings the MAC supports, in

s/do/to/

> +Similarly, the link partner can advertise its capabilities through the same
> +bits. These parameters are exchanged through the negotiation process, but can
> +also be enforced locally by disabling **pause autoneg**, thus ignoring the
> +link partner's capabilities.
> +
> +The local resolution of the pause configuration after receiving the link-partner
> +abilities is done according to the following table, from 802.3 Annex 28B.3 :
> +
> ++-------------+--------------+--------------------------+
> +|Local device | Link partner | Pause settings resolution|
> ++------+------+-------+------+-----------+--------------+
> +|Pause | Asym | Pause | Asym | RX        | TX           |
> ++======+======+=======+======+===========+==============+
> +| 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 currently configured and advertised settings can be queried with ::
> +
> +  ethtool <iface>
> +
> +  Settings for eth0:
> +        ...
> +	Supported pause frame use: Symmetric Receive-only
> +        ...
> +	Advertised pause frame use: Symmetric Receive-only
> +        ...
> +	Link partner advertised pause frame use: Symmetric Receive-only

> +A : Standalone driver testing
> +=============================
> +
> +Requirements : The interface under test must be connected to a link-partner whose
> +interface is admin-up. We don't require the link-partner to be configured in any
> +other specific manner for these tests.
> +
> +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.
> +
> +These parameters must validate against the following truth table :
> +
> +   +--------------+-----------------+
> +   | linkmodes    | pauseparam      |
> +   +-------+------+--------+--------+
> +   | Pause | Asym | rx     | tx     |
> +   +=======+======+========+========+
> +   | 0     | 0    | 0      | 0      |
> +   +-------+------+--------+--------+
> +   | 0     | 1    | 0      | 0 or 1 |
> +   +-------+------+--------+--------+
> +   | 1     | 0    |     rx == tx    |
> +   +-------+------+--------+--------+
> +   | 1     | 1    | 0 or 1 | 0 or 1 |
> +   +-------+------+--------+--------+

I think we need more sanity checks here, in order to know if the tests
that follow can be run.

If ETHTOOL_MSG_PAUSE_GET returns -EOPNOTSUPP, it is not a test error,
but all the following tests using forced pause should be skipped,
since it indicates forced pause is not supported.

When pause autoneg is on, and LP values are not reported, we probably
want a warning. I don't think we can say it is an error to report
local values but not LP values. There is probably firmware
implementations which don't make it available to user space. But we
should discourage such behaviour with a warning. And some of the tests
which follow will need to be skipped.

I would suggest looking through the tests and making a list of things
which must be implemented in order to actually perform the test. If
something is missing and returns -EOPNOTSUPP, we need to skip the
test.


> +
> +Test scenario
> +-------------
> +
> +Following the reported value from::
> +
> +        ethtool <iface>
> +        Settings for <iface>:
> +                ...
> +	        Supported pause frame use: <value>
> +
> +Iterate over all the 4 combinations of rx and tx pause parameters::
> +
> +        ethtool -A <iface> autoneg off rx <rx_val> tx <tx_val>
> +
> +The settings must be accepted or rejected, according to the above truth table.

-EOPNOTSUPP should also be consider a pass. Other codes should be a
fail.

Sorry, out of time now, i will continue later.

       Andrew

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

[Maxime Chevallier]

On 5/27/26 02:24, Jakub Kicinski wrote:
> On Fri, 22 May 2026 19:51:06 +0200 Maxime Chevallier (Netdev
> Foundation) wrote:
>>   Documentation/networking/pause_test_plan.rst | 556 +++++++++++++++++++
> 
> It'd be great to hear from others but IMHO in the current form this is
> not suitable for Documentation/networking/ We can commit the "knowledge"
> part but enumerating the test cases seems odd for Documentation/.

I think the same, I wasn't sure exactly how/where to send that to, so at 
least in this form you can generate that as a nicer to read html form, 
but this will likely change given Andrew's feedback

Maxime

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

[Maxime Chevallier]

On 5/27/26 04:47, Andrew Lunn wrote:
> On Tue, May 26, 2026 at 05:24:47PM -0700, Jakub Kicinski wrote:
>> On Fri, 22 May 2026 19:51:06 +0200 Maxime Chevallier (Netdev
>> Foundation) wrote:
>>>   Documentation/networking/pause_test_plan.rst | 556 +++++++++++++++++++
>>
>> It'd be great to hear from others but IMHO in the current form this is
>> not suitable for Documentation/networking/ We can commit the "knowledge"
>> part but enumerating the test cases seems odd for Documentation/.
> 
> Sorry, not looked too deeply at the actual content yet.
> 
> What i was thinking was a python file, which sphinx can ingest to
> produce documentation, and place holders were code would be added to
> implement the actual test during the next phase.
> 
> This is how i've done testing in the past. I would be the evil one who
> thought up the tests and described them in detail using sphinx markup
> in a python test template file. After some review they got passed off
> to a python developer for implementation. And when they got run and
> failed, sometimes the feature developer, the test developer and myself
> got together to figure who made the error.
> 
> I'm not sure we even need sphinx. What i find important is that the
> test is documented. What kAPI calls should be made with what
> parameters. What results we are expected and why? So that when a test
> fails, a developer has the information they need to fix their
> code. The Why? is important, and often missing from the kernel tests.

I see, so we'd get an actual test plan as code (likely python), with for
now placeholder tests, that would work :)

As for the kAPI testing, I agree that the end goal is to get driver
authors to get their flow control implementation right running this
suite.

But I don't really see how we can validate kAPI itself, as we're down at
the ethnl level.

This is made more complex by the fact that there are 2 drivers involved
here, the MAC and the PHY one. On top of that, either the MAC driver
uses phylink, or it doesn't. The common issues are different depending
on the case.

Raw phylib usage is by far the trickiest, but even phylink-based MAC
drivers can get it wrong, by ignoring the pause params in their 
mac_link_up() phylink callback, or getting their capabilities wrong.

The problem is we get an aggregated view of that in userspace. The
pauseparams come from the MAC driver, but the supported and advertised
modes are an aggregation of PHY + MAC stuff, which makes it very hard
to pinpoint where people are getting it wrong. Maybe people are writing
a MAC driver, but their tests fail because the PHY on their board has
a mis-behaving PHY driver, etc.

We're missing some information about what the MAC alone does, and what
the PHY driver alone also does, so that we can look at it from the
userspace perspective, and says for example "OK your MAC says that
it can only do Sym Pause, but I see that the interface reports
supporting Asym as well, and you're not using phylink, so you are
not calling phy_set_asymp_pause() correctly".

One possible path for that could be debugfs ? to let MAC/PHY expose
precise info, so we know who is wrong ?

Or, we go lower level and use the .self_test ethtool callback. We're no
longer validating the user-visible behaviour, but we have more control
on kAPI testing. We can poke into net_device, net_device->phydev, we may
even instrument phylib to validate what the MAC is doing ? But that
means we're drifting away from the netlink-level testing.

Maxime
> 
> 	Andrew

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

[Andrew Lunn]

> As for the kAPI testing, I agree that the end goal is to get driver
> authors to get their flow control implementation right running this
> suite.
> 
> But I don't really see how we can validate kAPI itself, as we're down at
> the ethnl level.

All we can do is invoke the kAPI in different ways, and test we get
the expected results. When it fails, it is down to the developer to
figure out why, which layer. But they have a description of what the
test is doing, and why? In most reviews, all i need to explain is the
expected behaviour, and the second version is correct. So a test with
explanation text should sort cut that process. I don't think we need
any more.

To some extent, we have an iterative process here. We have never done
testing of this, we don't know exactly what we need. If we get
feedback that a test is failing, but they cannot figure out why, we
might need to help out, and then extend either the text, or add finer
grain testing to narrow down the problem space. If we get a submission
which passes all the tests but review turns up problems, we might want
to ask the developers to extend the tests to catch the failure.

    Andrew

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

[Jakub Kicinski]

On Wed, 27 May 2026 04:47:47 +0200 Andrew Lunn wrote:
> > It'd be great to hear from others but IMHO in the current form this is
> > not suitable for Documentation/networking/ We can commit the "knowledge"
> > part but enumerating the test cases seems odd for Documentation/.  
> 
> Sorry, not looked too deeply at the actual content yet.
> 
> What i was thinking was a python file, which sphinx can ingest to
> produce documentation, and place holders were code would be added to
> implement the actual test during the next phase.
> 
> This is how i've done testing in the past. I would be the evil one who
> thought up the tests and described them in detail using sphinx markup
> in a python test template file. After some review they got passed off
> to a python developer for implementation. And when they got run and
> failed, sometimes the feature developer, the test developer and myself
> got together to figure who made the error.
> 
> I'm not sure we even need sphinx. What i find important is that the
> test is documented. What kAPI calls should be made with what
> parameters. What results we are expected and why? So that when a test
> fails, a developer has the information they need to fix their
> code. The Why? is important, and often missing from the kernel tests.

All makes sense. The question is primarily how we fit that into 
the existing project layout we have in the kernel :(

The python tests can be hacked up to print the test case docstring
before the failure.

But I think for human and AI reviewer consumption it may be nice
to keep the condensed knowledge / common mistakes in Documentation/
If we had the ability to exercise the submissions it'd be a different
story test output would be a sufficient signal and/or could be fed into
the review. But for AI making a guess at whether the submitted driver is
correct purely from the driver source - knowledge is useful.

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

[Xuan Zhuo]

On Wed, 27 May 2026 14:08:30 +0200, Andrew Lunn <andrew@lunn.ch> wrote:
> > As for the kAPI testing, I agree that the end goal is to get driver
> > authors to get their flow control implementation right running this
> > suite.
> >
> > But I don't really see how we can validate kAPI itself, as we're down at
> > the ethnl level.
>
> All we can do is invoke the kAPI in different ways, and test we get
> the expected results. When it fails, it is down to the developer to
> figure out why, which layer. But they have a description of what the
> test is doing, and why? In most reviews, all i need to explain is the
> expected behaviour, and the second version is correct. So a test with
> explanation text should sort cut that process. I don't think we need
> any more.
>
> To some extent, we have an iterative process here. We have never done
> testing of this, we don't know exactly what we need. If we get
> feedback that a test is failing, but they cannot figure out why, we
> might need to help out, and then extend either the text, or add finer
> grain testing to narrow down the problem space. If we get a submission
> which passes all the tests but review turns up problems, we might want
> to ask the developers to extend the tests to catch the failure.
>

So I've been thinking lately: should we let AI generate and maintain these tests,
including kselftest? This would give us a much richer and more comprehensive
set of tests. Plus, each test could come with a complete explanation of its
purpose and methodology. In short, much of the work we used to do manually
can be offloaded to AI. This way, we can build a massive test suite and
achieve much broader coverage.

I'm actually trying this out right now.

Thanks.


>     Andrew
>

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

[Andrew Lunn]

> So I've been thinking lately: should we let AI generate and maintain these tests,
> including kselftest? This would give us a much richer and more comprehensive
> set of tests. Plus, each test could come with a complete explanation of its
> purpose and methodology. In short, much of the work we used to do manually
> can be offloaded to AI. This way, we can build a massive test suite and
> achieve much broader coverage.

A test suite only has value if it is correct. It actually has negative
value if it is wrong, because you are forcing developers to make their
implementations wrong in order to pass the tests.

Do you have the knowledge to validate whatever you generate via IA?
Can you argue it is correct? Do you have a range of hardware to test
it on?

It could be an interesting experiment, but i'm sceptical, until proven
otherwise.

	Andrew

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

[Maxime Chevallier]

Hi,

On 5/28/26 01:25, Jakub Kicinski wrote:
> On Wed, 27 May 2026 04:47:47 +0200 Andrew Lunn wrote:
>>> It'd be great to hear from others but IMHO in the current form this is
>>> not suitable for Documentation/networking/ We can commit the "knowledge"
>>> part but enumerating the test cases seems odd for Documentation/.
>>
>> Sorry, not looked too deeply at the actual content yet.
>>
>> What i was thinking was a python file, which sphinx can ingest to
>> produce documentation, and place holders were code would be added to
>> implement the actual test during the next phase.
>>
>> This is how i've done testing in the past. I would be the evil one who
>> thought up the tests and described them in detail using sphinx markup
>> in a python test template file. After some review they got passed off
>> to a python developer for implementation. And when they got run and
>> failed, sometimes the feature developer, the test developer and myself
>> got together to figure who made the error.
>>
>> I'm not sure we even need sphinx. What i find important is that the
>> test is documented. What kAPI calls should be made with what
>> parameters. What results we are expected and why? So that when a test
>> fails, a developer has the information they need to fix their
>> code. The Why? is important, and often missing from the kernel tests.
> 
> All makes sense. The question is primarily how we fit that into
> the existing project layout we have in the kernel :(
> 
> The python tests can be hacked up to print the test case docstring
> before the failure.
> 
> But I think for human and AI reviewer consumption it may be nice
> to keep the condensed knowledge / common mistakes in Documentation/
> If we had the ability to exercise the submissions it'd be a different
> story test output would be a sufficient signal and/or could be fed into
> the review. But for AI making a guess at whether the submitted driver is
> correct purely from the driver source - knowledge is useful.

Ok so let's split this then, on one hand the knowledge part, and on the
other hand the test definitions.

 From what I get from Andrew and you, the test definitions is lacking in 
details w.r.t the start conditions, the return code handling as well.

Having that in the doc is going to be too verbose, but in some python 
test that could really be great.

I'll use that as a basis for V2

Maxime

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

[Maxime Chevallier]

Hi

On 5/28/26 01:25, Jakub Kicinski wrote:
> On Wed, 27 May 2026 04:47:47 +0200 Andrew Lunn wrote:
>>> It'd be great to hear from others but IMHO in the current form this is
>>> not suitable for Documentation/networking/ We can commit the "knowledge"
>>> part but enumerating the test cases seems odd for Documentation/.
>>
>> Sorry, not looked too deeply at the actual content yet.
>>
>> What i was thinking was a python file, which sphinx can ingest to
>> produce documentation, and place holders were code would be added to
>> implement the actual test during the next phase.
>>
>> This is how i've done testing in the past. I would be the evil one who
>> thought up the tests and described them in detail using sphinx markup
>> in a python test template file. After some review they got passed off
>> to a python developer for implementation. And when they got run and
>> failed, sometimes the feature developer, the test developer and myself
>> got together to figure who made the error.
>>
>> I'm not sure we even need sphinx. What i find important is that the
>> test is documented. What kAPI calls should be made with what
>> parameters. What results we are expected and why? So that when a test
>> fails, a developer has the information they need to fix their
>> code. The Why? is important, and often missing from the kernel tests.
> 
> All makes sense. The question is primarily how we fit that into
> the existing project layout we have in the kernel :(
> 
> The python tests can be hacked up to print the test case docstring
> before the failure.
> 
> But I think for human and AI reviewer consumption it may be nice
> to keep the condensed knowledge / common mistakes in Documentation/
> If we had the ability to exercise the submissions it'd be a different
> story test output would be a sufficient signal and/or could be fed into
> the review. But for AI making a guess at whether the submitted driver is
> correct purely from the driver source - knowledge is useful.

Also for the knowledge part, we had Oleksij's series that documented
more aspects of flow control (not just the pause part) :

https://lore.kernel.org/netdev/20260304094811.2779953-1-o.rempel@pengutronix.de/

Oleksij, maybe we can merge some of the information here with your
doc for the Docuentation/networking part ?

Maxime

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

[Oleksij Rempel]

Hi,

On Fri, May 29, 2026 at 09:42:14AM +0200, Maxime Chevallier wrote:
> Hi
> 
> On 5/28/26 01:25, Jakub Kicinski wrote:
> > On Wed, 27 May 2026 04:47:47 +0200 Andrew Lunn wrote:
> > > > It'd be great to hear from others but IMHO in the current form this is
> > > > not suitable for Documentation/networking/ We can commit the "knowledge"
> > > > part but enumerating the test cases seems odd for Documentation/.
> > > 
> > > Sorry, not looked too deeply at the actual content yet.
> > > 
> > > What i was thinking was a python file, which sphinx can ingest to
> > > produce documentation, and place holders were code would be added to
> > > implement the actual test during the next phase.
> > > 
> > > This is how i've done testing in the past. I would be the evil one who
> > > thought up the tests and described them in detail using sphinx markup
> > > in a python test template file. After some review they got passed off
> > > to a python developer for implementation. And when they got run and
> > > failed, sometimes the feature developer, the test developer and myself
> > > got together to figure who made the error.
> > > 
> > > I'm not sure we even need sphinx. What i find important is that the
> > > test is documented. What kAPI calls should be made with what
> > > parameters. What results we are expected and why? So that when a test
> > > fails, a developer has the information they need to fix their
> > > code. The Why? is important, and often missing from the kernel tests.
> > 
> > All makes sense. The question is primarily how we fit that into
> > the existing project layout we have in the kernel :(
> > 
> > The python tests can be hacked up to print the test case docstring
> > before the failure.
> > 
> > But I think for human and AI reviewer consumption it may be nice
> > to keep the condensed knowledge / common mistakes in Documentation/
> > If we had the ability to exercise the submissions it'd be a different
> > story test output would be a sufficient signal and/or could be fed into
> > the review. But for AI making a guess at whether the submitted driver is
> > correct purely from the driver source - knowledge is useful.
> 
> Also for the knowledge part, we had Oleksij's series that documented
> more aspects of flow control (not just the pause part) :
> 
> https://lore.kernel.org/netdev/20260304094811.2779953-1-o.rempel@pengutronix.de/
> 
> Oleksij, maybe we can merge some of the information here with your
> doc for the Docuentation/networking part ?

I have nothing against it. We need a documentation for proper testing
and implementations too :)

Best Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

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

[Andrew Lunn]

> From what I get from Andrew and you, the test definitions is lacking in
> details w.r.t the start conditions, the return code handling as well.
> 
> Having that in the doc is going to be too verbose, but in some python test
> that could really be great.

My experience with such documents is that they are verbose, very much
cut/paste repeated text, with just one parameter changed between test
variants, and a slight different expected result. And i personally
tend to make a lot of cut/paste errors :-(

Return code handling is also important, so i think it should be
stated. It is also potentially the most complex part of it. It happens
in all the corner cases which are going to get the least testing when
implementing the tests. They only really get exercised when the tests
are thrown at oddball hardware by developers trying to validate their
hardware.

There are also cases where the hardware reports it can do something,
and then returns EOPNOTSUPP when asked to do it, which is a bug. So
the tests becomes a sort of decision tree. The hardware reports it can
only do symmetric pause. We go down the branch that makes sure
symmetric pause passes, but asymmetric reports EOPNOTSUPP, etc.

There are a lot of combinations/permutations, and limited resources,
so it could be we cannot actually test everything. We might need to
accept EOPNOTSUPP at any stage as a pass, simply because we don't have
the resources to test all the corner cases.

I would normally actually concentrate on the corner cases, because
developers should have tested the happy path cases themselves.
However, for pause, we often see the happy path being very wrong, so
we should spend more time testing the happy path, and less on the
corner cases.

And that is probably true for all the testing we are doing at the
moment, because we are concentrating on subjects where developers
often get the basics wrong.

     Andrew

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

[Andrew Lunn]

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

Nice.

But i still think the test should check the autoneg result and do
something sensible if the link does not come up. This probably applies
to all cases where we trigger auto neg.

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

What 802.3 says. If we come across cases where phylib/phylink is
broken, let me know, and i will fix it. But we will leave driver bugs
to individual driver developers.

But we also need to consider that for some APIs, we have decided that
a configuration can be set now, which does not actually apply in our
current conditions, but it will be stored away for when conditions
change and it is applicable. The half duplex case could fit that. When
the link is currently half duplex, you can configure pause, but you
don't expect it to actually change the current behaviour. It only
kicks in when the link renegotiates to full duplex sometime in the
future. We have to also consider this the other way around. The link
is full duplex and pause is configured by the user. Something happens
with the LP and the link renegotiates to half duplex. The local end
should not throw away the configuration, it simply cannot apply it
given the current situation.

	Andrew

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

[Maxime Chevallier]

On 5/29/26 14:59, Andrew Lunn wrote:
>> 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 :)
> 
> Nice.
> 
> But i still think the test should check the autoneg result and do
> something sensible if the link does not come up. This probably applies
> to all cases where we trigger auto neg.

Yes indeed, we need some sets of helpers to deal with link management, 
especially for this kind of ethtool tests, with ways to recover a known
working configuration.

> 
>> 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 ?
> 
> What 802.3 says. 

Ok, this is what I've been using to define the documented cases so far:)

> If we come across cases where phylib/phylink is
> broken, let me know, and i will fix it. But we will leave driver bugs
> to individual driver developers.
> 
> But we also need to consider that for some APIs, we have decided that
> a configuration can be set now, which does not actually apply in our
> current conditions, but it will be stored away for when conditions
> change and it is applicable. The half duplex case could fit that. When
> the link is currently half duplex, you can configure pause, but you
> don't expect it to actually change the current behaviour. It only
> kicks in when the link renegotiates to full duplex sometime in the
> future. We have to also consider this the other way around. The link
> is full duplex and pause is configured by the user. Something happens
> with the LP and the link renegotiates to half duplex. The local end
> should not throw away the configuration, it simply cannot apply it
> given the current situation.
> 

True, so the end result is that we must not error out when changing the
pause params while the link is in half duplex, but store the user intent.

Maybe an exception is if we only advertise HD modes ? i.e. HW can't do
FD, or user forced HD modes ? even then, ethtool will show that we
aren't advertising any Pause modes, so maybe that's enough.

OK maybe in the end the best course of action is to leave this 
discussion here and I followup with :

  - Well defined test cases in a machine-readable (or human readable but
    with much more details) format, including start conditions, a
    decision tree based on the "supported" fields

   - A list of identified corner cases that may be ill-defined (such as
    this HD topic) that we can openly discuss and potentially followup
    with code fixes

  - Human readable doc for developpers and AI to ingest

Maxime