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: <20220510005537.GH895@COLIN-DESKTOP1.localdomain>
Date:   Mon, 9 May 2022 17:55:37 -0700
From:   Colin Foster <colin.foster@...advantage.com>
To:     Vladimir Oltean <vladimir.oltean@....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:30:30PM +0000, Vladimir Oltean wrote:
> 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.

Hmm... So the main reason I needed get_caps was because
phylink_generic_validate looks at mac_capabilities. I know I'll have to
deal with supported_interfaces once I finally get the other four ports
working, but for now the main purpose of this patch is to allow me to
populate the mac_capabilities entry for phylink_generic_validate.

Aside from ensuring legacy_pre_march2020 = false, I feel like the rest
of the patch makes sense.

> 
> 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.