Re: [PATCH net-next 0/4] net: ethtool: Untangle PHYLIB dependency

[Michal Kubecek]

On Wed, Jul 01, 2020 at 09:29:38PM -0700, Florian Fainelli wrote:
> Hi all,
> 
> This patch series untangles the ethtool netlink dependency with PHYLIB
> which exists because the cable test feature calls directly into PHY
> library functions. The approach taken here is to utilize a new set of
> net_device_ops function pointers which are automatically set to the PHY
> library variants when a network device driver attaches to a PHY device.

I'm not sure about the idea of creating a copy of netdev_ops for each
device using phylib. First, there would be some overhead (just checked
my 5.8-rc3 kernel, struct netdev_ops is 632 bytes). Second, there is
quite frequent pattern of comparing dev->netdev_ops against known
constants to check if a network device is of certain type; I can't say
for sure if it is also used with devices using phylib in existing code
but it feels risky.

As the two pointers are universal for all devices, couldn't we simply
use one global structure with them like we do for IPv6 (ipv6_stub) or
some netfilter modules (e.g. nf_ct_hook)?

Michal

> Florian Fainelli (4):
>   net: Add cable test netdevice operations
>   net: phy: Change cable test arguments to net_device
>   net: phy: Automatically set-up cable test netdev_ops
>   net: ethtool: Remove PHYLIB dependency
> 
>  drivers/net/phy/phy.c        | 18 ++++++++++++++----
>  drivers/net/phy/phy_device.c | 32 ++++++++++++++++++++++++++++++++
>  include/linux/netdevice.h    | 14 ++++++++++++++
>  include/linux/phy.h          | 10 ++++++----
>  net/Kconfig                  |  1 -
>  net/ethtool/cabletest.c      | 12 ++++++++----
>  6 files changed, 74 insertions(+), 13 deletions(-)
> 
> -- 
> 2.25.1
>