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: <b20f0964-42b7-53af-fe24-540d6cd011de@nvidia.com> Date: Mon, 29 Aug 2022 14:21:35 +0300 From: Gal Pressman <gal@...dia.com> To: "Keller, Jacob E" <jacob.e.keller@...el.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 On 29/08/2022 10:11, Keller, Jacob E wrote: > >> -----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. The firmware folks here claim there's also a difference between TX and RX. > >>> 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. > Yea, understood, but respectfully, I don't understand why we should go along with your requirement to support this non-spec behavior.
Powered by blists - more mailing lists