netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 00/13] net: phy: Rework linkmodes handling in a dedicated file
@ 2025-02-22 14:27 Maxime Chevallier
  2025-02-22 14:27 ` [PATCH net-next 01/13] net: phy: Extract the speed/duplex to linkmode conversion from phylink Maxime Chevallier
                   ` (12 more replies)
  0 siblings, 13 replies; 23+ messages in thread
From: Maxime Chevallier @ 2025-02-22 14:27 UTC (permalink / raw)
  To: davem, Andrew Lunn, Jakub Kicinski, Eric Dumazet, Paolo Abeni,
	Russell King, Heiner Kallweit
  Cc: Maxime Chevallier, netdev, linux-kernel, thomas.petazzoni,
	linux-arm-kernel, Christophe Leroy, Herve Codina,
	Florian Fainelli, Vladimir Oltean, Köry Maincent,
	Oleksij Rempel, Simon Horman, Romain Gantois

Hello everyone,

Following the V4 of the phy_port series [1] we've discussed about attempting
to extract some of the linkmode <-> capabilities (speed/duplex) <-> interface
logic into a dedicated file, so that we can re-use that logic out of
phylink. That would be the first 3 patches of this series.

While trying to do that, I might have gotten a bit carried-away, and I'm
therefore submitting this series to rework the way we are currently
managing the linkmodes <-> capabilities handling.

We are currently defining all the possible Ethernet linkmodes in an
enum ethtool_link_mode_bit_indices value defined in uapi/linux/ethtool.h :

	ETHTOOL_LINK_MODE_10baseT_Half_BIT	= 0,
	ETHTOOL_LINK_MODE_10baseT_Full_BIT	= 1,
	...

Each of these modes represents a media-side link definition, and runs at
a given speed and duplex.

Specific attributes for each modes are stored in net/ethtool/common.c, as
an array of struct link_mode_info :

	struct link_mode_info {
		int				speed;
		u8				lanes;
		u8				duplex;
	}

The link_mode_params[] array is the canonical definition for these modes,
as (1) there are build-time checks to make sure any new linkmode
definition is also defined in this array and (2) this array is always
compiled-in, as it's part of the net stack (i.e. it is not phylib-specific).

This array is however not optimized for lookups, as it is not ordered in
any particular fashion (new modes go at the end, regardless of their speed
and duplex).

Phylib also includes a similar array, in the form of the phy_settings
array in drivers/net/phy/phy-core.c :

	struct phy_setting {
		u32 speed;
		u8 duplex;
		u8 bit; // The enum index for the linkmode
	};

The phy_settings array however is ordered by descending speeds. A variety
of helpers in phylib rely on that ordering to perform lookups, usually
to get one or any linkmode corresponding to a requested speed and duplex.

Finally, we have some helpers in phylink (phylink_caps_to_linkmodes) that
allows getting the list of linkmodes that match a set of speed and duplex
value, all at once.

While the phylink and phylib helpers allows for efficient lookups, they
have some drawbacks as well :

	(1) : It's easy to forget updating of all of these helpers and structures
		  when adding a new linkmode. New linkmodes are actually added fairly
		  often, lately either for slow BaseT1 flavours, or for crazy-fast
		  modes (800Gbps modes, but I guess people won't stop there)
		  
	(2) : Even though the phylink and phylib modes use carefull sorting
		  to speed-up the lookup process, the phylib lookups are usually
		  done in descending speed order and will therefore get slower
		  as people add even faster link speeds.
		  
This series introduces a new "link_capabilities" structure that is used
to build an array of link_caps :

	struct link_capabilities {
		int speed;                           
		unsigned int duplex;
		__ETHTOOL_DECLARE_LINK_MODE_MASK(linkmodes);
	};

We group these in an array, indexed with LINK_CAPA enums that are basically
identical to the phylink MAC_CAPS :

...
LINK_CAPA_1000HD,             
LINK_CAPA_1000FD,
LINK_CAPA_2500FD,
LINK_CAPA_5000FD,
...

We now have an associative array of <speed,duplex> <-> All compatible linkmodes

This array is initialized at phylib-init time based on the content of
the link_mode_params[] array from net/ethtool/common.c, that way it is
always up-to-date with new modes, and always properly ordered.

Patches 6 to 12 then convert all lookups from the phy_settings array into
lookups from this link_caps array, hopefully speeding-up lookups in the
meantime (we iterate over possible speeds instead of individual linkmodes)

Patch 13 simply removes the phy_settings array altogether.

This series is not meant to introduce changes in behaviour, however patches
11 and 12 do introduce functionnal changes. When configuring the advert
for speeds >= 1G in PHY devices, as well as when constructing the link
parameters for fixed links in phylink, we used to rely on phy_settings
lookups returning one, and only one, compatible linkmode. This series will
make so that the lookups will result on all matching linkmodes being
returned, and MAY cause advert/fixed-link configuring more linkmodes.

There are cons as well for this, as this is a bit more init time for phylib,
but maybe more importantly, we lose in the precision for the lookups in
phy_settings. However, given all the uses for phy_settings (most are just
to get speed/duplex), I think this is actually ok, but any comment would
be very welcome.

This series was tested with :
 - 10/100/1000M links
 - 2,5, 5, 10G BaseT links
 - 1G Fixed link

I also made sure that this compiles with the following options :

CONFIG_PHYLIB=n

CNFIG_PHYLINK=m
CONFIG_PHYLIB=m

CNFIG_PHYLINK=m
CONFIG_PHYLIB=y

CNFIG_PHYLINK=y
CONFIG_PHYLIB=y

Heiner is currently working on cleaning-up the headers and internal
helpers for phylib, this series may conflict with it.

All the new helpers that were introduced (in drivers/net/phy/phy-caps.h)
are for internal use only (only users should be core stuff, such as phylib and
phylink, and in the future, phy_port).

Thanks,

Maxime

[1]: https://lore.kernel.org/netdev/20250213101606.1154014-1-maxime.chevallier@bootlin.com/

Maxime Chevallier (13):
  net: phy: Extract the speed/duplex to linkmode conversion from phylink
  net: phy: phylink: Extract the logic to get caps from interface
  net: phy: phylink: Extract getting the max speed for a given interface
  net: ethtool: Export the link_mode_params definitions
  net: phy: Use an internal, searchable storage for the linkmodes
  net: phy: phy_caps: Move phy_speeds to phy_caps
  net: phy: phy_caps: Move __set_linkmode_max_speed to phy_caps
  net: phy: phy_caps: Introduce link_caps_valid
  net: phy: phy_caps: Implement link_capabilities lookup by linkmode
  net: phy: phy_caps: Allow looking-up link caps based on speed and
    duplex
  net: phy: phy_device: Use link_capabilities lookup for PHY aneg config
  net: phy: phylink: Use phy_caps_lookup for fixed-link configuration
  net: phy: drop phy_settings and the associated lookup helpers

 drivers/net/phy/Makefile     |   2 +-
 drivers/net/phy/phy-caps.h   |  39 +++
 drivers/net/phy/phy-core.c   | 254 ++----------------
 drivers/net/phy/phy.c        |  38 +--
 drivers/net/phy/phy_caps.c   | 496 +++++++++++++++++++++++++++++++++++
 drivers/net/phy/phy_device.c |  14 +-
 drivers/net/phy/phylink.c    | 359 +++----------------------
 include/linux/ethtool.h      |   8 +
 include/linux/phy.h          |  14 -
 net/ethtool/common.c         |   1 +
 net/ethtool/common.h         |   7 -
 11 files changed, 615 insertions(+), 617 deletions(-)
 create mode 100644 drivers/net/phy/phy-caps.h
 create mode 100644 drivers/net/phy/phy_caps.c

-- 
2.48.1


^ permalink raw reply	[flat|nested] 23+ messages in thread
end of thread, other threads:[~2025-02-25 14:17 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-22 14:27 [PATCH net-next 00/13] net: phy: Rework linkmodes handling in a dedicated file Maxime Chevallier
2025-02-22 14:27 ` [PATCH net-next 01/13] net: phy: Extract the speed/duplex to linkmode conversion from phylink Maxime Chevallier
2025-02-24 14:01   ` Russell King (Oracle)
2025-02-24 14:13     ` Maxime Chevallier
2025-02-22 14:27 ` [PATCH net-next 02/13] net: phy: phylink: Extract the logic to get caps from interface Maxime Chevallier
2025-02-22 14:27 ` [PATCH net-next 03/13] net: phy: phylink: Extract getting the max speed for a given interface Maxime Chevallier
2025-02-22 14:27 ` [PATCH net-next 04/13] net: ethtool: Export the link_mode_params definitions Maxime Chevallier
2025-02-22 14:27 ` [PATCH net-next 05/13] net: phy: Use an internal, searchable storage for the linkmodes Maxime Chevallier
2025-02-24 13:33   ` Kory Maincent
2025-02-22 14:27 ` [PATCH net-next 06/13] net: phy: phy_caps: Move phy_speeds to phy_caps Maxime Chevallier
2025-02-22 14:27 ` [PATCH net-next 07/13] net: phy: phy_caps: Move __set_linkmode_max_speed " Maxime Chevallier
2025-02-22 14:27 ` [PATCH net-next 08/13] net: phy: phy_caps: Introduce link_caps_valid Maxime Chevallier
2025-02-23  6:38   ` kernel test robot
2025-02-23 13:40     ` Maxime Chevallier
2025-02-22 14:27 ` [PATCH net-next 09/13] net: phy: phy_caps: Implement link_capabilities lookup by linkmode Maxime Chevallier
2025-02-22 14:27 ` [PATCH net-next 10/13] net: phy: phy_caps: Allow looking-up link caps based on speed and duplex Maxime Chevallier
2025-02-22 14:27 ` [PATCH net-next 11/13] net: phy: phy_device: Use link_capabilities lookup for PHY aneg config Maxime Chevallier
2025-02-22 14:27 ` [PATCH net-next 12/13] net: phy: phylink: Use phy_caps_lookup for fixed-link configuration Maxime Chevallier
2025-02-24 13:44   ` Kory Maincent
2025-02-24 13:53     ` Russell King (Oracle)
2025-02-24 14:04       ` Kory Maincent
2025-02-25 14:17         ` Maxime Chevallier
2025-02-22 14:27 ` [PATCH net-next 13/13] net: phy: drop phy_settings and the associated lookup helpers Maxime Chevallier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).