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:   Thu, 25 Aug 2022 17:51:01 +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>
Subject: RE: [PATCH net-next 0/2] ice: support FEC automatic disable

> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Thursday, August 25, 2022 10:30 AM
> To: Keller, Jacob E <jacob.e.keller@...el.com>
> Cc: Gal Pressman <gal@...dia.com>; Saeed Mahameed <saeedm@...dia.com>;
> netdev@...r.kernel.org
> Subject: Re: [PATCH net-next 0/2] ice: support FEC automatic disable
> 
> On Thu, 25 Aug 2022 16:57:50 +0000 Keller, Jacob E wrote:
> > > Sorry, I misinterpreted your previous reply, somehow I thought you
> > > quoted option (3), because my fallible reading of mlx5 was that it
> > > accepts multiple flags.
> > >
> > > (First) option 2 is fine.
> >
> > Even though existing behavior doesn't do that for ice right now and
> > wouldn't be able to do that properly with old firmware?
> 
> 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.

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

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.

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

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