netdev - RE: [PATCH net-next 0/2] ice: support FEC automatic disable

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:   Tue, 30 Aug 2022 23:09:40 +0000
From:   "Keller, Jacob E" <jacob.e.keller@...el.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     Gal Pressman <gal@...dia.com>, Saeed Mahameed <saeedm@...dia.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Simon Horman <horms@...ge.net.au>,
        Andy Gospodarek <andy@...yhouse.net>
Subject: RE: [PATCH net-next 0/2] ice: support FEC automatic disable



> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Tuesday, August 30, 2022 2:45 PM
> To: Keller, Jacob E <jacob.e.keller@...el.com>
> Cc: Gal Pressman <gal@...dia.com>; Saeed Mahameed <saeedm@...dia.com>;
> netdev@...r.kernel.org; Simon Horman <horms@...ge.net.au>; Andy
> Gospodarek <andy@...yhouse.net>
> Subject: Re: [PATCH net-next 0/2] ice: support FEC automatic disable
> 
> On Tue, 30 Aug 2022 13:09:20 -0700 Jacob Keller wrote:
> > I'm trying to figure out what my next steps are here.
> >
> > Jakub, from earlier discussion it sounded like you are ok with accepting
> > patch to include "No FEC" into our auto override behavior, with no uAPI
> > changes. Is that still ok given the recent dicussion regarding going
> > beyond the spec?
> 
> Yes, I reserve the right to change my mind :) but AFAIU it doesn't make
> things worse, so fine by me.
>

Ok.
 
> > I'm also happy to rename the flag in ice so that its not misnamed and
> > clearly indicates its behavior.
> 
> Which flag? A new ethtool priv flag?
> 

No the flag I am referring to here is for the bit we pass to firmware from the driver. This is confusingly named "EN_AUTO_FEC" but it really means something like "override spec and try all FEC modes".

> > Gal seems against extending uAPI to indicate or support "ignore spec".
> > To be properly correct that would mean changing ice to stop setting the
> > AUTO_FEC flag. As explained above, I believe this will lead to breakage
> > in situations where we used to link and function properly.
> 
> Stop setting the AUTO_FEC flag or start using a new standard compliant
> AUTO flag?

There is only "EN_AUTO_FEC" which means both "try multiple FEC modes, including ones outside the spec". If we disable this flag, then I believe it will only try the highest priority FEC mode for each link type. This is the flag which I think is poorly named and led me to misunderstand the whole behavior.

> 
> Gal, within the spec do you iterate over modes or pick one mode somehow
> (the spec gives a set, AFAICT)?
> 
> > I have no way to verify whether other vendors actually follow this or
> > not, as it essentially requires checking with modules that wouldn't link
> > otherwise and likely requires a lot of trial and error.
> 
> Getting some input from Broadcom or Netronome would be useful, yes :(