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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9d962e38-1aa9-d0ed-261e-eb77c82b186b@intel.com>
Date:   Fri, 26 Aug 2022 10:51:21 -0700
From:   Jacob Keller <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>
Subject: Re: [PATCH net-next 0/2] ice: support FEC automatic disable

On 8/25/2022 6:01 PM, Jakub Kicinski wrote:
> On Thu, 25 Aug 2022 17:38:14 -0700 Jacob Keller wrote:
>> 2b) on newer firmware, the set PHY capabilities interface does have a
>> bit to request No FEC. In this case, if we set the No FEC bit, then the
>> firmware will be able to select No FEC as an option for cables that
>> otherwise wouldn't have selected it in the old firmware (such as CA-L
>> cables mentioned above).
> 
> Oh, but per the IEEE standard No FEC is _not_ an option for CA-L.
> From the initial reading of your series I thought that Intel NICs 
> would _never_ pick No FEC.
> 

That was my original interpretation when I was first introduced to this
problem but I was mistaken, hence why the commit message wasn't clear :(

This is rather more complicated than I originally understood and the
names for various bits have not been named very well so their behavior
isn't exactly obvious...

> Sounds like we need a bit for "ignore the standard and try everything".
> 
> What about BASE-R FEC? Is the FW going to try it on the CA-L cable?
> 

Ok I got further clarification on this. We have a bit, "Auto FEC
enable", as well as a bitmask for which FEC modes to try.

If "Auto FEC En" is set, then the Link Establishment State Machine will
try all of the FEC options we list in that bitmask, as long as we can
theoretically support them even if they aren't spec compliant.

For old firmware the bitmask didn't include a bit for "No FEC", where as
the new firmware has a bit for "No FEC".

We were always setting "Auto FEC En" so currently we try all FEC modes
we could theoretically support.

If "Auto FEC En" is disabled, then we only try FEC modes which are spec
compliant. Additionally, only a single FEC mode is tried based on a
priority and the bitmask.

Currently and historically the driver has always set "Auto FEC En", so
we were enabling non-spec compliant FEC modes, but "No FEC" was only
based on spec compliance with the media type.

>From this, I think I agree the correct behavior is to add a bit for
"override the spec and try everything", and then on new firmware we'd
set the "No FEC" while on old firmware we'd be limited to only trying
FEC modes.

Does that make sense?

So yea I think we do probably need a "ignore the standard" bit.. but
currently that appears to already be what ice does (excepting No FEC
which didn't previously have a bit to set for it)

Thanks,
Jake