On 7/25/22 11:37 AM, Sean Anderson wrote:
> 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@xxxxxxxx/T/#t
> [2] https://lore.kernel.org/netdev/1579701573-6609-1-git-send-email-madalin.bucur@xxxxxxxxxxx/
> [3] https://lore.kernel.org/netdev/1580137671-22081-1-git-send-email-madalin.bucur@xxxxxxxxxxx/
> [4] https://lore.kernel.org/netdev/20200116181933.32765-1-olteanv@xxxxxxxxx/
>
> 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(-)
>
ping?
Are there any comments on this series other than about the tags for patch 6?
--Sean
