netdev - Re: [PATCH net-next 0/4] net: dsa: Plug in PHYLINK support

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: <5690f51e-e303-70ad-f08e-915674efd854@gmail.com>
Date:   Sun, 18 Mar 2018 17:11:14 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     netdev@...r.kernel.org
Cc:     privat@...l-hjelmeland.no, andrew@...n.ch,
        vivien.didelot@...oirfairelinux.com, davem@...emloft.net,
        rmk+kernel@...linux.org.uk, sean.wang@...iatek.com,
        Woojung.Huh@...rochip.com, john@...ozen.org, cphealy@...il.com
Subject: Re: [PATCH net-next 0/4] net: dsa: Plug in PHYLINK support



On 03/18/2018 11:52 AM, Florian Fainelli wrote:
> Hi all,
> 
> This patch series adds PHYLINK support to DSA which is necessary to support more
> complex PHY and pluggable modules setups.
> 
> Patch series can be found here:
> 
> https://github.com/ffainelli/linux/commits/dsa-phylink
> 
> This was tested on:
> 
> - dsa-loop
> - bcm_sf2
> - mv88e6xxx
> - b53
> 
> With a variety of test cases:
> - internal & external MDIO PHYs
> - MoCA with link notification through interrupt/MMIO register
> - built-in PHYs
> - ifconfig up/down for several cycles works
> - bind/unbind of the drivers
> 
> And everything should still work as expected. Please be aware of the following:
> 
> - switch drivers (like bcm_sf2) which may have user-facing network ports using
>   fixed links would need to implement phylink_mac_ops to remain functional.
>   PHYLINK does not create a phy_device for fixed links, therefore our
>   call to adjust_link() from phylink_mac_link_{up,down} would not be calling
>   into the driver. This *should not* affect CPU/DSA ports which are configured
>   through adjust_link() but have no network devices
> 
> - support for SFP/SFF is now possible, but switch drivers will still need some
>   modifications to properly support those, including, but not limited to using
>   the correct binding information. This will be submitted on top of this series
> 
> Russell, we could theoretically eliminate patch 3 and resolve this within DSA
> entirely by keeping a per-port phy_interface_t (we did that before), this is
> not a big change if we have to, let me know if you feel like this is cleaner. I
> was initially considering passing a phylink_link_state reference to
> mac_link_{up,down} but only a couple of fields are valid during link_down and
> ended up with passing the phy_interface_t value we need instead. This is
> necessary for switch drivers which have different types of port interfaces (see
> bcm_sf2 documentation in tree).

I think I will proceed differently for v2:

- introduce DSA phylink_mac_ops in dsa_switch_ops, such that drivers can
define those as preliminary commits, those won't be used by
net/dsa/slave.c just yet though

- have all relevant drivers implement phylink_mac_ops such that the
pluming is there and functional

- switch net/dsa/slave.c to using PHYLINK

That way, we should avoid any breakage in between and have an "atomic"
switch between PHYLIB and PHYLINK.

> 
> Thank you!
> 
> Florian Fainelli (4):
>   net: dsa: Eliminate dsa_slave_get_link()
>   net: phy: phylink: Provide PHY interface to mac_link_{up,down}
>   net: dsa: Plug in PHYLINK support
>   net: dsa: bcm_sf2: Implement phylink_mac_ops
> 
>  drivers/net/dsa/bcm_sf2.c             | 190 +++++++++++++--------
>  drivers/net/ethernet/marvell/mvneta.c |   4 +-
>  drivers/net/phy/phylink.c             |   6 +-
>  include/linux/phylink.h               |  10 +-
>  include/net/dsa.h                     |  27 ++-
>  net/dsa/Kconfig                       |   2 +-
>  net/dsa/dsa_priv.h                    |   9 -
>  net/dsa/slave.c                       | 304 ++++++++++++++++++++--------------
>  8 files changed, 340 insertions(+), 212 deletions(-)
> 

-- 
Florian