mbox series

[net-next,v13,00/13] Introduce PHY listing and link_topology tracking

Message ID 20240607071836.911403-1-maxime.chevallier@bootlin.com (mailing list archive)
Headers show
Series Introduce PHY listing and link_topology tracking | expand

Message

Maxime Chevallier June 7, 2024, 7:18 a.m. UTC
Hello everyone,

This is V13 for the link topology addition, allowing to track all PHYs
that are linked to netdevices.

This version is based on the V12, and addresses the missing
documentation for the return code of some helpersn, and gathers the
review from Köry.

Discussions on the patch 01/13 updates can be found here :

https://lore.kernel.org/netdev/20240412104615.3779632-1-maxime.chevallier@bootlin.com/
https://lore.kernel.org/netdev/20240429131008.439231-1-maxime.chevallier@bootlin.com/
https://lore.kernel.org/netdev/20240507102822.2023826-1-maxime.chevallier@bootlin.com/

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
PHY id: 0
Upstream type: MAC

PHY for eth0:
PHY index: 2
Driver name: Marvell 88E1111
PHY device name: i2c:sfp-eth0:16
PHY id: 21040322
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
PHY id: 21040593
Upstream type: MAC

Ethtool patches : https://github.com/minimaxwell/ethtool/tree/mc/main

Link to v12: https://lore.kernel.org/netdev/20240605124920.720690-1-maxime.chevallier@bootlin.com/
Link to v11: https://lore.kernel.org/netdev/20240404093004.2552221-1-maxime.chevallier@bootlin.com/
Link to V10: https://lore.kernel.org/netdev/20240304151011.1610175-1-maxime.chevallier@bootlin.com/
Link to V9: https://lore.kernel.org/netdev/20240228114728.51861-1-maxime.chevallier@bootlin.com/
Link to V8: https://lore.kernel.org/netdev/20240220184217.3689988-1-maxime.chevallier@bootlin.com/
Link to V7: https://lore.kernel.org/netdev/20240213150431.1796171-1-maxime.chevallier@bootlin.com/
Link to V6: https://lore.kernel.org/netdev/20240126183851.2081418-1-maxime.chevallier@bootlin.com/
Link to V5: https://lore.kernel.org/netdev/20231221180047.1924733-1-maxime.chevallier@bootlin.com/
Link to V4: https://lore.kernel.org/netdev/20231215171237.1152563-1-maxime.chevallier@bootlin.com/
Link to V3: https://lore.kernel.org/netdev/20231201163704.1306431-1-maxime.chevallier@bootlin.com/
Link to V2: https://lore.kernel.org/netdev/20231117162323.626979-1-maxime.chevallier@bootlin.com/
Link to V1: https://lore.kernel.org/netdev/20230907092407.647139-1-maxime.chevallier@bootlin.com/



Maxime Chevallier (13):
  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: Allow querying phy stats by index
  Documentation: networking: document phy_link_topology

 Documentation/netlink/specs/ethtool.yaml      |  62 ++++
 Documentation/networking/ethtool-netlink.rst  |  52 +++
 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                     |  15 +-
 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          |  21 ++
 net/core/dev.c                                |  15 +
 net/ethtool/Makefile                          |   2 +-
 net/ethtool/cabletest.c                       |  16 +-
 net/ethtool/netlink.c                         |  57 +++-
 net/ethtool/netlink.h                         |  10 +
 net/ethtool/phy.c                             | 306 ++++++++++++++++++
 net/ethtool/plca.c                            |  19 +-
 net/ethtool/pse-pd.c                          |  16 +-
 net/ethtool/strset.c                          |  17 +-
 30 files changed, 970 insertions(+), 45 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

Marc Kleine-Budde June 26, 2024, 9:47 a.m. UTC | #1
On 07.06.2024 09:18:13, Maxime Chevallier wrote:
> Hello everyone,
> 
> This is V13 for the link topology addition, allowing to track all PHYs
> that are linked to netdevices.
> 
> This version is based on the V12, and addresses the missing
> documentation for the return code of some helpersn, and gathers the
> review from Köry.
> 
> Discussions on the patch 01/13 updates can be found here :
> 
> https://lore.kernel.org/netdev/20240412104615.3779632-1-maxime.chevallier@bootlin.com/
> https://lore.kernel.org/netdev/20240429131008.439231-1-maxime.chevallier@bootlin.com/
> https://lore.kernel.org/netdev/20240507102822.2023826-1-maxime.chevallier@bootlin.com/
> 
> 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 *

This creates the following warning for me:

[   51.877429] ------------[ cut here ]------------
[   51.882094] WARNING: CPU: 0 PID: 333 at lib/refcount.c:31 ref_tracker_free+0x1ac/0x254
[   51.890222] refcount_t: decrement hit 0; leaking memory.
[   51.895611] Modules linked in: mcp251xfd flexcan imx_sdma can_dev spi_imx
[   51.902493] CPU: 0 PID: 333 Comm: ethtool Not tainted 6.10.0-rc4+ #327
[   51.909056] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[   51.915603] Call trace: 
[   51.915623] [<c0d2cbd0>] (unwind_backtrace) from [<c0109bcc>] (show_stack+0x10/0x14)
[   51.925979] [<c0109bcc>] (show_stack) from [<c0d4744c>] (dump_stack_lvl+0x50/0x64)
[   51.933605] [<c0d4744c>] (dump_stack_lvl) from [<c0d2d2ec>] (__warn+0x88/0xc0)
[   51.940877] [<c0d2d2ec>] (__warn) from [<c0120ba0>] (warn_slowpath_fmt+0x1b4/0x1c4)
[   51.948590] [<c0120ba0>] (warn_slowpath_fmt) from [<c0697f74>] (ref_tracker_free+0x1ac/0x254)
[   51.957176] [<c0697f74>] (ref_tracker_free) from [<c0ae4b7c>] (ethnl_phy_done+0x24/0x54)
[   51.965318] [<c0ae4b7c>] (ethnl_phy_done) from [<c0acda68>] (genl_done+0x3c/0x88)
[   51.972845] [<c0acda68>] (genl_done) from [<c0ac9b6c>] (netlink_dump+0x2d8/0x3d0)
[   51.980387] [<c0ac9b6c>] (netlink_dump) from [<c0aca2fc>] (__netlink_dump_start+0x1f4/0x2c4)
[   51.988889] [<c0aca2fc>] (__netlink_dump_start) from [<c0acd7bc>] (genl_family_rcv_msg+0x140/0x328)
[   51.997989] [<c0acd7bc>] (genl_family_rcv_msg) from [<c0acd9e8>] (genl_rcv_msg+0x44/0x88)
[   52.006204] [<c0acd9e8>] (genl_rcv_msg) from [<c0acc554>] (netlink_rcv_skb+0xb8/0x118)
[   52.014157] [<c0acc554>] (netlink_rcv_skb) from [<c0acd038>] (genl_rcv+0x20/0x34)
[   52.021673] [<c0acd038>] (genl_rcv) from [<c0acbd24>] (netlink_unicast+0x23c/0x3d0)
[   52.029367] [<c0acbd24>] (netlink_unicast) from [<c0acc044>] (netlink_sendmsg+0x18c/0x3d4)
[   52.037667] [<c0acc044>] (netlink_sendmsg) from [<c0a4b30c>] (__sys_sendto+0xd4/0x128)
[   52.045626] [<c0a4b30c>] (__sys_sendto) from [<c0100080>] (ret_fast_syscall+0x0/0x54)
[   52.053496] Exception stack(0xc3967fa8 to 0xc3967ff0)
[   52.058576] 7fa0:                   b6f1130c 0000000c 00000003 015dd238 00000018 00000000
[   52.066780] 7fc0: b6f1130c 0000000c b6fb6700 00000122 00571000 00000001 0052a2f8 015dd190
[   52.074978] 7fe0: 00000122 bec7cf38 b6ea847d b6e1fe86
[   52.080184] ---[ end trace 0000000000000000 ]---

While a "ethtool --show-phys lan0" works w/o problems.

| $ ip a s
| 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
|     link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
|     inet 127.0.0.1/8 scope host lo
|        valid_lft forever preferred_lft forever
|     inet6 ::1/128 scope host 
|        valid_lft forever preferred_lft forever
| 2: sit0@NONE: <NOARP> mtu 1480 qdisc noop state DOWN group default qlen 1000
|     link/sit 0.0.0.0 brd 0.0.0.0
| 3: lan0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP group default qlen 1000
|     link/ether xx:xx:xx:xx:xx:xx brd ff:ff:ff:ff:ff:ff
|     inet 192.168.178.140/24 metric 1024 brd 192.168.178.255 scope global dynamic lan0
|        valid_lft 863858sec preferred_lft 863858sec
|     inet6 2003:xx:xxxx:xxxx:xxx:xxxx:xxxx:xxxx/64 scope global temporary dynamic 
|        valid_lft 7057sec preferred_lft 982sec
|     inet6 2003:xx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx/64 scope global dynamic mngtmpaddr noprefixroute 
|        valid_lft 7057sec preferred_lft 982sec
|     inet6 fe80::xxxx:xxxx:xxxx:xxxx/64 scope link 
|        valid_lft forever preferred_lft forever
| 4: flexcan0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UP group default qlen 10
|     link/can 
| 5: mcp251xfd0: <NOARP,UP,LOWER_UP,ECHO> mtu 72 qdisc pfifo_fast state UP group default qlen 10
|     link/can 

regards,
Marc
Maxime Chevallier June 26, 2024, 10:01 a.m. UTC | #2
Hello Marc,

Thanks for giving this a test.

On Wed, 26 Jun 2024 11:47:52 +0200
Marc Kleine-Budde <mkl@pengutronix.de> wrote:

> > # ethtool --show-phys *  
> 
> This creates the following warning for me:
> 
> [   51.877429] ------------[ cut here ]------------
> [   51.882094] WARNING: CPU: 0 PID: 333 at lib/refcount.c:31 ref_tracker_free+0x1ac/0x254
> [   51.890222] refcount_t: decrement hit 0; leaking memory.
> [   51.895611] Modules linked in: mcp251xfd flexcan imx_sdma can_dev spi_imx
> [   51.902493] CPU: 0 PID: 333 Comm: ethtool Not tainted 6.10.0-rc4+ #327
> [   51.909056] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> [   51.915603] Call trace: 
> [   51.915623] [<c0d2cbd0>] (unwind_backtrace) from [<c0109bcc>] (show_stack+0x10/0x14)
> [   51.925979] [<c0109bcc>] (show_stack) from [<c0d4744c>] (dump_stack_lvl+0x50/0x64)
> [   51.933605] [<c0d4744c>] (dump_stack_lvl) from [<c0d2d2ec>] (__warn+0x88/0xc0)
> [   51.940877] [<c0d2d2ec>] (__warn) from [<c0120ba0>] (warn_slowpath_fmt+0x1b4/0x1c4)
> [   51.948590] [<c0120ba0>] (warn_slowpath_fmt) from [<c0697f74>] (ref_tracker_free+0x1ac/0x254)
> [   51.957176] [<c0697f74>] (ref_tracker_free) from [<c0ae4b7c>] (ethnl_phy_done+0x24/0x54)
> [   51.965318] [<c0ae4b7c>] (ethnl_phy_done) from [<c0acda68>] (genl_done+0x3c/0x88)
> [   51.972845] [<c0acda68>] (genl_done) from [<c0ac9b6c>] (netlink_dump+0x2d8/0x3d0)
> [   51.980387] [<c0ac9b6c>] (netlink_dump) from [<c0aca2fc>] (__netlink_dump_start+0x1f4/0x2c4)
> [   51.988889] [<c0aca2fc>] (__netlink_dump_start) from [<c0acd7bc>] (genl_family_rcv_msg+0x140/0x328)
> [   51.997989] [<c0acd7bc>] (genl_family_rcv_msg) from [<c0acd9e8>] (genl_rcv_msg+0x44/0x88)
> [   52.006204] [<c0acd9e8>] (genl_rcv_msg) from [<c0acc554>] (netlink_rcv_skb+0xb8/0x118)
> [   52.014157] [<c0acc554>] (netlink_rcv_skb) from [<c0acd038>] (genl_rcv+0x20/0x34)
> [   52.021673] [<c0acd038>] (genl_rcv) from [<c0acbd24>] (netlink_unicast+0x23c/0x3d0)
> [   52.029367] [<c0acbd24>] (netlink_unicast) from [<c0acc044>] (netlink_sendmsg+0x18c/0x3d4)
> [   52.037667] [<c0acc044>] (netlink_sendmsg) from [<c0a4b30c>] (__sys_sendto+0xd4/0x128)
> [   52.045626] [<c0a4b30c>] (__sys_sendto) from [<c0100080>] (ret_fast_syscall+0x0/0x54)
> [   52.053496] Exception stack(0xc3967fa8 to 0xc3967ff0)
> [   52.058576] 7fa0:                   b6f1130c 0000000c 00000003 015dd238 00000018 00000000
> [   52.066780] 7fc0: b6f1130c 0000000c b6fb6700 00000122 00571000 00000001 0052a2f8 015dd190
> [   52.074978] 7fe0: 00000122 bec7cf38 b6ea847d b6e1fe86
> [   52.080184] ---[ end trace 0000000000000000 ]---

Hmm I've never seen this one before, but I could be missing some debug
options. Looks like my ethnl_parse_header_dev_put() doesn't belong in
the ethnl_phy_done() callback. I'll see if I can reproduce this error
on some setups, and address that in the next iteration.

> While a "ethtool --show-phys lan0" works w/o problems.

Thanks a lot for the test,

Maxime
Marc Kleine-Budde June 26, 2024, 10:15 a.m. UTC | #3
On 26.06.2024 12:01:37, Maxime Chevallier wrote:
> Thanks for giving this a test.

I was looking at this to figure out if it's possible to use/reuse/abuse
this for CAN transceiver switching. Although Oleksij coded a CAN
transceiver struct phy_device POC integration, we're using the "other"
PHY framework from drivers/phy, i.e. struct phy now.

regards,
Marc