[net-next,v17,00/14] Introduce PHY listing and link_topology tracking

Message

Maxime Chevallier July 9, 2024, 6:30 a.m. UTC

Hello everyone,

This is V17 of the phy_link_topology series, aiming at improving support
for multiple PHYs being attached to the same MAC.

V17 is mostly a rebase of V16 on net-next, as the addition of new
features in the PSE-PD command raised a conflict on the ethtool netlink
spec, and patch 10 was updated :

	("net: ethtool: pse-pd: Target the command to the requested PHY")

The new code was updated to make use of the new helpers to retrieve the
PHY from the ethnl request, and an error message was also updated to
better reflect the fact that we don't only rely on the attached PHY for
configuration.

As a remainder, here's what the PHY listings would look like :
 - eth0 has a 88x3310 acting as media converter, and an SFP module with
   an embedded 88e1111 PHY
 - eth2 has a 88e1510 PHY

# ethtool --show-phys *

PHY for eth0:
PHY index: 1
Driver name: mv88x3310
PHY device name: f212a600.mdio-mii:00
Downstream SFP bus name: sfp-eth0
Upstream type: MAC

PHY for eth0:
PHY index: 2
Driver name: Marvell 88E1111
PHY device name: i2c:sfp-eth0:16
Upstream type: PHY
Upstream PHY index: 1
Upstream SFP name: sfp-eth0

PHY for eth2:
PHY index: 1
Driver name: Marvell 88E1510
PHY device name: f212a200.mdio-mii:00
Upstream type: MAC

Ethtool patches : https://github.com/minimaxwell/ethtool/tree/mc/topo-v16

Link to V16: https://lore.kernel.org/netdev/[e-mail]/
Link to V15: https://lore.kernel.org/netdev/[e-mail]/
Link to V14: https://lore.kernel.org/netdev/[e-mail]/
Link to V13: https://lore.kernel.org/netdev/[e-mail]/
Link to v12: https://lore.kernel.org/netdev/[e-mail]/
Link to v11: https://lore.kernel.org/netdev/[e-mail]/
Link to V10: https://lore.kernel.org/netdev/[e-mail]/
Link to V9: https://lore.kernel.org/netdev/[e-mail]/
Link to V8: https://lore.kernel.org/netdev/[e-mail]/
Link to V7: https://lore.kernel.org/netdev/[e-mail]/
Link to V6: https://lore.kernel.org/netdev/[e-mail]/
Link to V5: https://lore.kernel.org/netdev/[e-mail]/
Link to V4: https://lore.kernel.org/netdev/[e-mail]/
Link to V3: https://lore.kernel.org/netdev/[e-mail]/
Link to V2: https://lore.kernel.org/netdev/[e-mail]/
Link to V1: https://lore.kernel.org/netdev/[e-mail]/

More discussions on specific issues that happened in 6.9-rc:

https://lore.kernel.org/netdev/[e-mail]/
https://lore.kernel.org/netdev/[e-mail]/
https://lore.kernel.org/netdev/[e-mail]/

Maxime Chevallier (14):
  net: phy: Introduce ethernet link topology representation
  net: sfp: pass the phy_device when disconnecting an sfp module's PHY
  net: phy: add helpers to handle sfp phy connect/disconnect
  net: sfp: Add helper to return the SFP bus name
  net: ethtool: Allow passing a phy index for some commands
  netlink: specs: add phy-index as a header parameter
  net: ethtool: Introduce a command to list PHYs on an interface
  netlink: specs: add ethnl PHY_GET command set
  net: ethtool: plca: Target the command to the requested PHY
  net: ethtool: pse-pd: Target the command to the requested PHY
  net: ethtool: cable-test: Target the command to the requested PHY
  net: ethtool: strset: Remove unnecessary check on genl_info
  net: ethtool: strset: Allow querying phy stats by index
  Documentation: networking: document phy_link_topology

 Documentation/netlink/specs/ethtool.yaml      |  58 ++++
 Documentation/networking/ethtool-netlink.rst  |  51 +++
 Documentation/networking/index.rst            |   1 +
 .../networking/phy-link-topology.rst          | 121 +++++++
 MAINTAINERS                                   |   1 +
 drivers/net/phy/Makefile                      |   2 +-
 drivers/net/phy/marvell-88x2222.c             |   2 +
 drivers/net/phy/marvell.c                     |   2 +
 drivers/net/phy/marvell10g.c                  |   2 +
 drivers/net/phy/phy_device.c                  |  48 +++
 drivers/net/phy/phy_link_topology.c           | 105 ++++++
 drivers/net/phy/phylink.c                     |   3 +-
 drivers/net/phy/qcom/at803x.c                 |   2 +
 drivers/net/phy/qcom/qca807x.c                |   2 +
 drivers/net/phy/sfp-bus.c                     |  26 +-
 include/linux/netdevice.h                     |   4 +-
 include/linux/phy.h                           |   6 +
 include/linux/phy_link_topology.h             |  82 +++++
 include/linux/sfp.h                           |   8 +-
 include/uapi/linux/ethtool.h                  |  16 +
 include/uapi/linux/ethtool_netlink.h          |  20 ++
 net/core/dev.c                                |  15 +
 net/ethtool/Makefile                          |   3 +-
 net/ethtool/cabletest.c                       |  35 +-
 net/ethtool/netlink.c                         |  66 +++-
 net/ethtool/netlink.h                         |  33 ++
 net/ethtool/phy.c                             | 308 ++++++++++++++++++
 net/ethtool/plca.c                            |  30 +-
 net/ethtool/pse-pd.c                          |  31 +-
 net/ethtool/strset.c                          |  27 +-
 30 files changed, 1057 insertions(+), 53 deletions(-)
 create mode 100644 Documentation/networking/phy-link-topology.rst
 create mode 100644 drivers/net/phy/phy_link_topology.c
 create mode 100644 include/linux/phy_link_topology.h
 create mode 100644 net/ethtool/phy.c

Comments

Jakub Kicinski July 15, 2024, 3:31 p.m. UTC | #1

On Tue,  9 Jul 2024 08:30:23 +0200 Maxime Chevallier wrote:
> This is V17 of the phy_link_topology series, aiming at improving support
> for multiple PHYs being attached to the same MAC.
> 
> V17 is mostly a rebase of V16 on net-next, as the addition of new
> features in the PSE-PD command raised a conflict on the ethtool netlink
> spec, and patch 10 was updated :
> 
> 	("net: ethtool: pse-pd: Target the command to the requested PHY")
> 
> The new code was updated to make use of the new helpers to retrieve the
> PHY from the ethnl request, and an error message was also updated to
> better reflect the fact that we don't only rely on the attached PHY for
> configuration.

I lack the confidence to take this during the merge window, without
Russell's acks. So Deferred, sorry :(

Maxime Chevallier July 16, 2024, 8:16 a.m. UTC | #2

Hello Jakub,

On Mon, 15 Jul 2024 08:31:06 -0700
Jakub Kicinski <[e-mail]> wrote:

> On Tue,  9 Jul 2024 08:30:23 +0200 Maxime Chevallier wrote:
> > This is V17 of the phy_link_topology series, aiming at improving support
> > for multiple PHYs being attached to the same MAC.
> > 
> > V17 is mostly a rebase of V16 on net-next, as the addition of new
> > features in the PSE-PD command raised a conflict on the ethtool netlink
> > spec, and patch 10 was updated :
> > 
> > 	("net: ethtool: pse-pd: Target the command to the requested PHY")
> > 
> > The new code was updated to make use of the new helpers to retrieve the
> > PHY from the ethnl request, and an error message was also updated to
> > better reflect the fact that we don't only rely on the attached PHY for
> > configuration.  
> 
> I lack the confidence to take this during the merge window, without
> Russell's acks. So Deferred, sorry :(

Understood. Is there anything I can make next time to make that series
more digestable and easy to review ? I didn't want to split the netlink
part from the core part, as just the phy_link_topology alone doesn't
make much sense for now, but it that makes the lives of reviewers
easier I could submit these separately.

Thanks,

Maxime

Jakub Kicinski July 17, 2024, 3:26 p.m. UTC | #3

On Tue, 16 Jul 2024 10:16:26 +0200 Maxime Chevallier wrote:
> > I lack the confidence to take this during the merge window, without
> > Russell's acks. So Deferred, sorry :(  
> 
> Understood. Is there anything I can make next time to make that series
> more digestable and easy to review ? I didn't want to split the netlink
> part from the core part, as just the phy_link_topology alone doesn't
> make much sense for now, but it that makes the lives of reviewers
> easier I could submit these separately.

TBH I can only review this from coding and netlink perspective, and 
it looks solid. Folk who actually know PHYs and SFPs may have more
meaningful feedback :(

Christophe Leroy Aug. 16, 2024, 5:02 p.m. UTC | #4

Hi Jakub, Russell

Le 17/07/2024 à 17:26, Jakub Kicinski a écrit :
> On Tue, 16 Jul 2024 10:16:26 +0200 Maxime Chevallier wrote:
>>> I lack the confidence to take this during the merge window, without
>>> Russell's acks. So Deferred, sorry :(
>>
>> Understood. Is there anything I can make next time to make that series
>> more digestable and easy to review ? I didn't want to split the netlink
>> part from the core part, as just the phy_link_topology alone doesn't
>> make much sense for now, but it that makes the lives of reviewers
>> easier I could submit these separately.
> 
> TBH I can only review this from coding and netlink perspective, and
> it looks solid. Folk who actually know PHYs and SFPs may have more
> meaningful feedback :(

How can we progress on this ?

Russell, have you been able to have a look at that latest version of the 
series ? I know you reviewed earlier versions already but I understand 
Jakub is willing some feedback from you.

Jakub, as you say it looks solid. I can add to that that I have been 
using this series widely through the double Ethernet attachment on 
several boards and it works well, it is stable and more performant than 
the dirty home-made solution we had on v4.14.

So it would be great if the series could be merged for v6.12, and I 
guess the earliest it is merged into net-next the more time it spends in 
linux-next before the merge window. Any chance to get it merged anytime 
soon even without a formal feedback from Russell ? We are really looking 
forward to getting that series merged and step forward with all the work 
that depends on it and is awaiting.

Thanks
Christophe

Jakub Kicinski Aug. 16, 2024, 5:11 p.m. UTC | #5

On Fri, 16 Aug 2024 19:02:20 +0200 Christophe Leroy wrote:
> So it would be great if the series could be merged for v6.12, and I 
> guess the earliest it is merged into net-next the more time it spends in 
> linux-next before the merge window. Any chance to get it merged anytime 
> soon even without a formal feedback from Russell ? We are really looking 
> forward to getting that series merged and step forward with all the work 
> that depends on it and is awaiting.

Give Russell a few days to respond, then repost. 
Russell said his ability to review code right now may be limited.
I'm not sure whether he would like us to wait for him or just do
our best. In the absence of an opinion - we'll do the latter.

Andrew Lunn Aug. 17, 2024, 3:43 p.m. UTC | #6

> Jakub, as you say it looks solid. I can add to that that I have been using
> this series widely through the double Ethernet attachment on several boards
> and it works well, it is stable and more performant than the dirty home-made
> solution we had on v4.14.

Have you posted a Tested-by:

You can also post Reviewed-by: if you have taken a look at the
code. It won't have the same value as one from Rusell, but it does add
some degree of warm fuzzy feeling this code is O.K, and it starts
building your reputation as a reviewer.

	Andrew

Thomas Petazzoni Aug. 17, 2024, 3:47 p.m. UTC | #7

Hello Andrew,

On Sat, 17 Aug 2024 17:43:52 +0200
Andrew Lunn <[e-mail]> wrote:

> > Jakub, as you say it looks solid. I can add to that that I have been using
> > this series widely through the double Ethernet attachment on several boards
> > and it works well, it is stable and more performant than the dirty home-made
> > solution we had on v4.14.  
> 
> Have you posted a Tested-by:
> 
> You can also post Reviewed-by: if you have taken a look at the
> code. It won't have the same value as one from Rusell, but it does add
> some degree of warm fuzzy feeling this code is O.K, and it starts
> building your reputation as a reviewer.

Hmm did you check the replies from Christophe to each patch of this
series? He has already posted a Tested-by and Reviewed-by to every
single patch in the series :-)

Thanks!

Thomas