[PATCH v3 00/11] net: phy: Add support for rate adaptation

* [PATCH v3 00/11] net: phy: Add support for rate adaptation
@ 2022-07-25 15:37 Sean Anderson
  2022-07-25 15:37 ` [PATCH v3 07/11] " Sean Anderson
  2022-08-18 15:21 ` [PATCH v3 00/11] " Sean Anderson
  0 siblings, 2 replies; 6+ messages in thread
From: Sean Anderson @ 2022-07-25 15:37 UTC (permalink / raw)
  To: netdev, Andrew Lunn, Heiner Kallweit, Russell King
  Cc: Paolo Abeni, Eric Dumazet, Alexandru Marginean, Vladimir Oltean,
	David S . Miller, Jakub Kicinski, linux-kernel, Sean Anderson,
	Bhadram Varka, Camelia Alexandra Groza, Claudiu Manoil,
	Ioana Ciornei, Jonathan Corbet, Madalin Bucur, linux-doc

This adds support for phy rate adaptation: when a phy adapts between
differing phy interface and link speeds. It was originally submitted as
part of [1], which is considered "v1" of this series.

We need support for rate adaptation for two reasons. First, the phy
consumer needs to know if the phy will perform rate adaptation in order to
program the correct advertising. An unaware consumer will only program
support for link modes at the phy interface mode's native speed. This
will cause autonegotiation to fail if the link partner only advertises
support for lower speed link modes. Second, to reduce packet loss it may
be desirable to throttle packet throughput.

There have been several past discussions [2-4] around adding rate
adaptation support. One point is that we must be certain that rate
adaptation is possible before enabling it. It is the opinion of some
developers that it is the responsibility of the system integrator or end
user to set the link settings appropriately for rate adaptation. In
particular, it was argued that (due to differing firmware) it might not
be clear if a particular phy has rate adaptation enabled. Additionally,
upper-layer protocols must already be tolerant of packet loss caused by
differing rates. Packet loss may happen anyway, such as if a faster link
is used with a slower switch or repeater. So adjusting pause settings
for rate adaptation is not strictly necessary.

I believe that our current approach is limiting, especially when
considering that rate adaptation (in two forms) has made it into IEEE
standards. In general, when we have appropriate information we should
set sensible defaults. To consider use a contrasting example, we enable
pause frames by default for link partners which autonegotiate for them.
When it's the phy itself generating these frames, we don't even have to
autonegotiate to know that we should enable pause frames.

Our current approach also encourages workarounds, such as commit
73a21fa817f0 ("dpaa_eth: support all modes with rate adapting PHYs").
These workarounds are fine for phylib drivers, but phylink drivers cannot
use this approach (since there is no direct access to the phy).

Although in earlier versions of this series, userspace could disable
rate adaptation, now it is only possible to determine the current rate
adaptation type. Disabling or otherwise configuring rate adaptation has
been left for future work. However, because currently only
RATE_ADAPT_PAUSE is implemented, it is possible to disable rate
adaptation by modifying the advertisement appropriately.

[1] https://lore.kernel.org/netdev/20220715215954.1449214-1-sean.anderson@seco.com/T/#t
[2] https://lore.kernel.org/netdev/1579701573-6609-1-git-send-email-madalin.bucur@oss.nxp.com/
[3] https://lore.kernel.org/netdev/1580137671-22081-1-git-send-email-madalin.bucur@oss.nxp.com/
[4] https://lore.kernel.org/netdev/20200116181933.32765-1-olteanv@gmail.com/

Changes in v3:
- Document MAC_(A)SYM_PAUSE
- Add some helpers for working with mac caps
- Modify link settings directly in phylink_link_up, instead of doing
  things more indirectly via link_*.
- Add phylink_cap_from_speed_duplex to look up the mac capability
  corresponding to the interface's speed.
- Include RATE_ADAPT_CRS; it's a few lines and it doesn't hurt.
- Move unused defines to next commit (where they will be used)
- Remove "Support differing link/interface speed/duplex". It has been
  rendered unnecessary due to simplification of the rate adaptation
  patches. Thanks Russell!
- Rewrite cover letter to better reflect the opinions of the developers
  involved

Changes in v2:
- Use int/defines instead of enum to allow for use in ioctls/netlink
- Add locking to phy_get_rate_adaptation
- Add (read-only) ethtool support for rate adaptation
- Move part of commit message to cover letter, as it gives a good
  overview of the whole series, and allows this patch to focus more on
  the specifics.
- Use the phy's rate adaptation setting to determine whether to use its
  link speed/duplex or the MAC's speed/duplex with MLO_AN_INBAND.
- Always use the rate adaptation setting to determine the interface
  speed/duplex (instead of sometimes using the interface mode).
- Determine the interface speed and max mac speed directly instead of
  guessing based on the caps.
- Add comments clarifying the register defines
- Reorder variables in aqr107_read_rate

Sean Anderson (11):
  net: dpaa: Fix <1G ethernet on LS1046ARDB
  net: phy: Add 1000BASE-KX interface mode
  net: phylink: Document MAC_(A)SYM_PAUSE
  net: phylink: Export phylink_caps_to_linkmodes
  net: phylink: Generate caps and convert to linkmodes separately
  net: phylink: Add some helpers for working with mac caps
  net: phy: Add support for rate adaptation
  net: phylink: Adjust link settings based on rate adaptation
  net: phylink: Adjust advertisement based on rate adaptation
  net: phy: aquantia: Add some additional phy interfaces
  net: phy: aquantia: Add support for rate adaptation

 Documentation/networking/ethtool-netlink.rst  |   2 +
 .../net/ethernet/freescale/dpaa/dpaa_eth.c    |   6 +-
 drivers/net/phy/aquantia_main.c               |  68 +++-
 drivers/net/phy/phy-core.c                    |  15 +
 drivers/net/phy/phy.c                         |  28 ++
 drivers/net/phy/phylink.c                     | 302 ++++++++++++++++--
 include/linux/phy.h                           |  26 +-
 include/linux/phylink.h                       |  29 +-
 include/uapi/linux/ethtool.h                  |  18 +-
 include/uapi/linux/ethtool_netlink.h          |   1 +
 net/ethtool/ioctl.c                           |   1 +
 net/ethtool/linkmodes.c                       |   5 +
 12 files changed, 466 insertions(+), 35 deletions(-)

-- 
2.35.1.1320.gc452695387.dirty

^ permalink raw reply	[flat|nested] 6+ messages in thread