Message-ID: <CO1PR11MB5089F11D12E8DC1500457095D6729@CO1PR11MB5089.namprd11.prod.outlook.com>
Date:   Thu, 25 Aug 2022 21:04:39 +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 1:34 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
> 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?
> 

I am not sure if that would be related

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

I believe it is trying different modes, but it is selecting from module data for what to try.

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

Right. I believe its because some combinations in their module data don't list No FEC, but still work with No FEC. The understanding I got from the firmware person I've asked was that the list of whats supported is recorded in the module somehow.

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

It's been frustratingly difficult to get real answers here.

> > 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 is at least one error in ice, and I'll update other drivers that print error messages too if I find any.

Powered by blists - more mailing lists