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]

Message-ID: <20220825133425.7bfb34e9@kernel.org>
Date:   Thu, 25 Aug 2022 13:34:25 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     "Keller, Jacob E" <jacob.e.keller@...el.com>
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 Thu, 25 Aug 2022 17:51:01 +0000 Keller, Jacob E wrote:
> > Update your FW seems like a reasonable thing to ask customers to do.
> > Are there cables already qualified for the old FW which would switch
> > to No FEC under new rules?  
> 
> Not sure I follow what you're asking here.

IIRC your NICs only work with qualified cables. I was asking if any of
the qualified cables would actually negotiate to No FEC under new rules.
Maybe such cables are very rare and there's no need to be super careful?

> > Can you share how your FW picks the mode exactly?
> 
> I'm not entirely sure how it selects, other than it selects from the
> table of supported modes. It uses some state machine to go through
> options until a suitable link is made, but the details of how exactly
> it does this I'm not quite sure.

State machine? So you're trying different FEC modes or reading module
data and picking one?

Various NICs do either or both, but I believe AUTO means the former.

> The old firmware never picks "No FEC" for some media types, because
> the standard was that those types would always require FEC of some
> kind (R or RS). However in practice the modules can work with no FEC
> anyways, and according to customer reports enabling this has helped
> with linking issues. That's the sum of my understanding thus far.

Well, according to the IEEE standard there sure are cables which don't
need FEC. Maybe your customers had problems linking up because switch
had a different selection algo?

> I would prefer this option of just "auto means also possibly
> disable", but I wasn't sure how other devices worked and we had some
> concerns about the change in behavior. Going with this option would
> significantly simplify things since I could bury the "set the auto
> disable flag if necessary" into a lower level of the code and only
> expose a single ICE_FEC_AUTO instead of ICE_FEC_AUTO_DIS...
> 
> Thus, I'm happy to respin this, as the new behavior when supported by
> firmware if we have some consensus there.

Hard to get consensus if we still don't know what the FW does...
But if there's no new uAPI, just always enabling OFF with AUTO
then I guess I'd have nothing to complain about as I don't know
what other drivers do either.

> I am happy to drop or
> respin the extack changes if you think thats still valuable as well
> in the event we need to extend that API in the future.

Your call. May be useful to do it sooner rather than later, but 
we should find at least one use for the extack as a justification.

> > There must be _some_ standardization here, because we're talking
> > about <5m cables, so we can safely assume it's linking to a ToR
> > switch.  
> 
> I believe so.