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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200702155652.ivokudjptoect6ch@lion.mk-sys.cz>
Date:   Thu, 2 Jul 2020 17:56:52 +0200
From:   Michal Kubecek <mkubecek@...e.cz>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 0/4] net: ethtool: Untangle PHYLIB dependency

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
>