On Fri, Jun 13, 2025 at 12:06:23PM -0400, Sean Anderson wrote:

On 6/13/25 08:55, Daniel Golle wrote:

quoted

Hi netdev folks,

there are currently 2 competing implementations for the groundworks to
support standalone PCS drivers.

https://patchwork.kernel.org/project/netdevbpf/list/?series=970582&state=%2A&archive=both

https://patchwork.kernel.org/project/netdevbpf/list/?series=961784&state=%2A&archive=both

They both kinda stalled due to a lack of feedback in the past 2 months
since they have been published.

Merging the 2 implementation is not a viable option due to rather large
architecture differences:

				| Sean			| Ansuel
--------------------------------+-----------------------+-----------------------
Architecture			| Standalone subsystem	| Built into phylink
Need OPs wrapped		| Yes			| No
resource lifecycle		| New subsystem		| phylink
Supports hot remove		| Yes			| Yes
Supports hot add		| Yes (*)		| Yes
provides generic select_pcs	| No			| Yes
support for #pcs-cell-cells	| No			| Yes
allows migrating legacy drivers	| Yes			| Yes
comes with tested migrations	| Yes			| No

(*) requires MAC driver to also unload and subsequent re-probe for link
to work again

Obviously both architectures have pros and cons, here an incomplete and
certainly biased list (please help completing it and discussing all
details):

Standalone Subsystem (Sean)

pros
====
 * phylink code (mostly) untouched
 * doesn't burden systems which don't use dedicated PCS drivers
 * series provides tested migrations for all Ethernet drivers currently
   using dedicated PCS drivers

cons
====
 * needs wrapper for each PCS OP
 * more complex resource management (malloc/free) 
 * hot add and PCS showing up late (eg. due to deferred probe) are
   problematic
 * phylink is anyway the only user of that new subsystem

I mean, if you want I can move the whole thing to live in phylink.c, but
that just enlarges the kernel if PCSs are not being used. The reverse
criticism can be made for Ansuel's series: most phylink users do not
have "dynamic" PCSs but the code is imtimately integrated with phylink
anyway.

At the risk of stating the obvious it seems to me that a key decision
that needs to be made is weather a new subsystem is the correct direction.

If I understand things correctly it seems that not creating a new subsystem
is likely to lead to a simpler implementation, at least in the near term.
While doing so lends itself towards greater flexibility in terms of users,
I'd suggest a cleaner abstraction layer, and possibly a smaller footprint
(I assume space consumed by unused code) for cases where PCS is not used.

On the last point, I do wonder if there are other approaches to managing
the footprint. And if so, that may tip the balance towards a new subsystem.


Another way of framing this is: Say, hypothetically, Sean was to move his
implementation into phylink.c. Then we might be able to have a clearer
discussion of the merits of each implementation. Possibly driving towards
common ground. But it seems hard to do so if we're unsure if there should
be a new subsystem or not.

quoted

phylink-managed standalone PCS drivers (Ansuel)

pros
====
 * trivial resource management

Actually, I would say the resource management is much more complex and
difficult to follow due to being spread out over many different
functions.

quoted

 * no wrappers needed
 * full support for hot-add and deferred probe
 * avoids code duplication by providing generic select_pcs
   implementation
 * supports devices which provide more than one PCS port per device
   ('#pcs-cell-cells')

cons
====
 * inclusion in phylink means more (dead) code on platforms not using
   dedicated PCS
 * series does not provide migrations for existing drivers
   (but that can be done after)
 * probably a bit harder to review as one needs to know phylink very well


It would be great if more people can take a look and help deciding the
general direction to go.

I also encourage netdev maintainers to have a look; Russell does not
seem to have the time to review either system.

quoted

There are many drivers awaiting merge which require such
infrastructure (most are fine with either of the two), some for more
than a year by now.

This is the major thing. PCS drivers should have been supported from the
start of phylink, and the longer there is no solution the more legacy
code there is to migrate.

This seems to be something we can all agree on :)