netdev - Re: [RFC v8 net-next 15/16] net: dsa: felix: add phylink_get

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: <20220509173029.xkwajrngvejkyjzs@skbuf>
Date:   Mon, 9 May 2022 17:30:30 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Colin Foster <colin.foster@...advantage.com>
CC:     "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Terry Bowman <terry.bowman@....com>,
        Wolfram Sang <wsa@...nel.org>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Steen Hegelund <Steen.Hegelund@...rochip.com>,
        Lars Povlsen <lars.povlsen@...rochip.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Russell King <linux@...linux.org.uk>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        "David S. Miller" <davem@...emloft.net>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Lee Jones <lee.jones@...aro.org>
Subject: Re: [RFC v8 net-next 15/16] net: dsa: felix: add phylink_get_caps
 capability

On Mon, May 09, 2022 at 05:23:32PM -0700, Colin Foster wrote:
> > > @@ -982,15 +982,23 @@ static void felix_phylink_get_caps(struct dsa_switch *ds, int port,
> > >  				   struct phylink_config *config)
> > >  {
> > >  	struct ocelot *ocelot = ds->priv;
> > > +	struct felix *felix;
> > >  
> > > -	/* This driver does not make use of the speed, duplex, pause or the
> > > -	 * advertisement in its mac_config, so it is safe to mark this driver
> > > -	 * as non-legacy.
> > > -	 */
> > > -	config->legacy_pre_march2020 = false;
> > > +	felix = ocelot_to_felix(ocelot);
> > > +
> > > +	if (felix->info->phylink_get_caps) {
> > > +		felix->info->phylink_get_caps(ocelot, port, config);
> > > +	} else {
> > >  
> > > -	__set_bit(ocelot->ports[port]->phy_mode,
> > > -		  config->supported_interfaces);
> > > +		/* This driver does not make use of the speed, duplex, pause or
> > > +		 * the advertisement in its mac_config, so it is safe to mark
> > > +		 * this driver as non-legacy.
> > > +		 */
> > > +		config->legacy_pre_march2020 = false;
> > 
> > I don't think you mean to set legacy_pre_march2020 to true only
> > felix->info->phylink_get_caps is absent, do you?
> > 
> > Also, I'm thinking maybe we could provide an implementation of this
> > function for all switches, not just for vsc7512.
> 
> I had assumed these last two patches might spark more discussion, which
> is why I kept them separate (specifically the last patch).
> 
> With this, are you simply suggesting to take everything that is
> currently in felix_phylink_get_caps and doing it in the felix / seville
> implementations? This is because the default condition is no longer the
> "only" condition. Sounds easy enough.

No, not everything, just the way in which config->supported_interfaces
is populated. We have different PCS implementations, so it's likely that
the procedures to retrieve the valid SERDES protocols (when changing
them will be supported) are different.

But in fact I seriously doubt that the current way in which supported_interfaces
gets populated is limiting you from doing anything right now, precisely
because you don't have any code that supports changing the phy-mode.

Also, it's unlikely (I'd say impossible) for one driver to be
unconverted to the post-March-2020 requirements and the other not to be.
The simple reason is that they share the same mac_config implementation.
So it's perfectly ok to keep "config->legacy_pre_march2020 = false"
right where it is.

So I'd say it's up to you, but I'd drop this patch right now.