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
| ||
|
Message-ID: <SA2PR11MB51005070A0E456D7DD169A1FD6769@SA2PR11MB5100.namprd11.prod.outlook.com> Date: Mon, 29 Aug 2022 07:11:18 +0000 From: "Keller, Jacob E" <jacob.e.keller@...el.com> To: Gal Pressman <gal@...dia.com>, Jakub Kicinski <kuba@...nel.org> CC: 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: Gal Pressman <gal@...dia.com> > Sent: Sunday, August 28, 2022 3:43 AM > To: Jakub Kicinski <kuba@...nel.org>; Keller, Jacob E <jacob.e.keller@...el.com> > Cc: 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 27/08/2022 02:57, Jakub Kicinski wrote: > > On Fri, 26 Aug 2022 10:51:21 -0700 Jacob Keller wrote: > >> On 8/25/2022 6:01 PM, Jakub Kicinski wrote: > >>> 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 for getting to the bottom of this :) > > > > The "override spec modes" bit sounds like a reasonable addition, > > since we possibly have different behavior between vendors letting > > the user know if the device will follow the rules can save someone > > debugging time. > > > > But it does sound orthogonal to you adding the No FEC bit to the mask > > for ice. > > > > Let me add Simon and Andy so that we have the major vendors on the CC. > > (tl;dr the question is whether your FW follows the guidance of > > 'Table 110C–1—Host and cable assembly combinations' in AUTO FEC mode). > > The other engineers I spoke to also wanted to mention that 110C-1 is only a small subset of all of the various link types. They also mentioned something about an SFF standard which describes many more types. > > If all the vendors already ignore the standard (like Intel and AFAIU > > nVidia) then we just need to document, no point adding the bit... > > I think we misunderstood each other :). > Our implementation definitely *does not* ignore the standard. > When autoneg is disabled, and auto fec is enabled, the firmware chooses > a fec mode according to the spec. If "no FEC" is not in the spec, we > will not pick it (nor do we want to). > It sounds like you're not happy with the spec, then why not change it? > This doesn't sound like an area where we want to be non-compliant. > > Regardless, changing our interface because of one device' firmware > bug/behavior change doesn't make any sense. This interface affects all > consumers, and is going to stick with us forever. Firmware will > eventually get updated and it only affects one driver. Well, the current ice behavior for every FEC mode *except* No FEC, we try modes which may be supportable even though they're outside the spec. As far as I understand, the reason we attempt these is because it allows linking in more scenarios, presumably because some combination of things is not fully spec compliant? I don't really know for sure. For future firmware, this would include No FEC as well. Thus, even with future firmware we'd still be trying some modes outside of the spec. I can try to get some more answers tomorrow about the reasoning and justification for this behavior. Thanks, Jake
Powered by blists - more mailing lists