netdev - Re: [PATCH net-next v3 5/9] net: dsa: add support for phylink_pcs

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]

Date:   Mon, 22 Jun 2020 11:22:13 +0100
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Ioana Ciornei <ioana.ciornei@....com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        vladimir.oltean@....com, claudiu.manoil@....com,
        alexandru.marginean@....com, michael@...le.cc, andrew@...n.ch,
        f.fainelli@...il.com, olteanv@...il.com
Subject: Re: [PATCH net-next v3 5/9] net: dsa: add support for phylink_pcs_ops

On Mon, Jun 22, 2020 at 01:54:47AM +0300, Ioana Ciornei wrote:
> In order to support split PCS using PHYLINK properly, we need to add a
> phylink_pcs_ops structure.
> 
> Note that a DSA driver that wants to use these needs to implement all 4
> of them: the DSA core checks the presence of these 4 function pointers
> in dsa_switch_ops and only then does it add a PCS to PHYLINK. This is
> done in order to preserve compatibility with drivers that have not yet
> been converted, or don't need, a split PCS setup.
> 
> Also, when pcs_get_state() and pcs_an_restart() are present, their mac
> counterparts (mac_pcs_get_state(), mac_an_restart()) will no longer get
> called, as can be seen in phylink.c.

I don't like this at all, it means we've got all this useless layering,
and that layering will force similar layering veneers into other parts
of the kernel (such as the DPAA2 MAC driver, when we eventually come to
re-use pcs-lynx there.)

I don't think we need that - I think we can get to a position where
pcs-lynx is called requesting that it bind to phylink as the PCS, and
it calls phylink_add_pcs() directly, which means we do not end up with
veneers in DSA nor in the DPAA2 MAC driver - they just need to call
the pcs-lynx initialisation function with the phylink instance for it
to attach to.

Yes, that requires phylink_add_pcs() to change slightly, and for there
to be a PCS private pointer, as I have previously stated.  At the
moment, changing that is really easy - the phylink PCS backend has no
in-tree users at the moment.  So there is no reason not to get this
correct right now.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!