netdev - Re: [RFC net-next] sfp/phylink: move module EEPROM ethtool access into netdev core ethtool

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: <20171217170602.GW10595@n2100.armlinux.org.uk>
Date:   Sun, 17 Dec 2017 17:06:02 +0000
From:   Russell King - ARM Linux <linux@...linux.org.uk>
To:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org
Subject: Re: [RFC net-next] sfp/phylink: move module EEPROM ethtool access
 into netdev core ethtool

> Questions:
> 1. Is it worth adding a pointer to struct net_device for these two
>    methods, rather than having multiple duplicate veneers to vector
>    the ethtool module EEPROM ioctls through to the SFP bus layer?
> 
> 2. Should this allow network/phy drivers to override the default -
>    the code is currently structured to allow phy drivers to override
>    network drivers implementations, which seems the wrong way around.

I should also mention that there's another place that having the
sfp bus pointer in the network device comes in handy - the case
where we have a SFP module connected to a PHY rather than the MAC.

In this case, phylink itself is not used to link the SFP to the
netdev, and phylib doesn't provide the necessary hooks into the PHY
driver for the PHY driver to know when the network device comes up
or goes down.  SFP needs to know that to assert/deassert the TX
DISABLE signal to disable the module laser.

Having the net_device structure contain a pointer to the SFP bus
allows phylink or network drivers to directly inform SFP of the
state of the network device, without needing intermediaries to
forward the state.

It's possible that this may not be the best approach - the only setup
I'm aware of at present that has the "mac <-> phy <-> sfp" setup is
the Macchiatobin, but if other phys are involved, it may be better
if instead of having PHY drivers having to add support for SFP, we
instead do it in phylib.  The counter argument to that is that SFP
likely needs more in-depth knowledge of the PHY than a the generic
phylib parts could know about.

The patches as they currently stand are in my "phy" branch, browsable
via:

 http://git.armlinux.org.uk/cgit/linux-arm.git/log/?h=phy

specifically:

 sfp: use netdev sfp_bus for start/stop
 net: phy: Add SFP support to Marvell 10G PHY driver

The last patch is does not (yet) take into account the RX_LOS signal
when determining the link state, which it ought to to avoid false
link assertions as can happen when there's noise pickup by the
detector.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up