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:   Wed, 31 Aug 2022 13:15:01 -0700
From:   Jacob Keller <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

On 8/31/2022 4:01 AM, Gal Pressman wrote:
> On 31/08/2022 00:44, Jakub Kicinski wrote:
>> On Tue, 30 Aug 2022 13:09:20 -0700 Jacob Keller wrote:
>>> Gal seems against extending uAPI to indicate or support "ignore spec".
>>> To be properly correct that would mean changing ice to stop setting the
>>> AUTO_FEC flag. As explained above, I believe this will lead to breakage
>>> in situations where we used to link and function properly.
>> Stop setting the AUTO_FEC flag or start using a new standard compliant
>> AUTO flag?
>>
>> Gal, within the spec do you iterate over modes or pick one mode somehow
>> (the spec gives a set, AFAICT)?
>> When autoneg is enabled (and auto fec enabled), auto negotiation is done
> from the link modes according to spec.

This is the same for ice, except that the ETHTOOL_FEC_* configuration is
 ignored. The documentation indicates its only intended for when
autonegotiation is disabled.

> When autoneg is disabled (and auto fec enabled), the firmware chooses
> one of the supported modes according to the spec. As far as I
> understand, it doesn't try anything, just picks a supported mode.
> 

This is how ice works if we don't set the ICE_AQC_PHY_EN_FEC_AUTO flag
when configuring our firmware.

> This whole thing revolves around customers who don't use auto
> negotiation, but it sounds like ice is still trying to do auto
> negotiation for fec under the hood.

It's not really auto negotiation as it is more like auto-retry, its a
simple state machine that iterates through a series of possible
configurations. The goal being to reduce cognitive burden on users and
just try to establish link.

I believe that sigh ICE_AQC_PHY_EN_FEC_AUTO set, we still try different
media type settings but only one FEC mode. The exact way it picks such a
FEC mode is unclear to me in this case. With ICE_AQC_PHY_EN_FEC_AUTO,
each media type is re-tried with each FEC mode that is theoretically
possible even if its not in spec.

> Looking at the reasons Jacob listed here (unspec switches/modules), it
> seems like all this can be resolved by simply setting the fec mode
> explicitly, and to me it sounds like a reasonable solution for these
> special cases.