Subject: Re: [PATCH net-next 0/2] ice: support FEC automatic disable

From: Jakub Kicinski <[email-masked]>
Date: Thu, 25 Aug 2022 18:01:07 -0700

On Thu, 25 Aug 2022 17:38:14 -0700 Jacob Keller wrote:
> On 8/25/2022 1:34 PM, Jakub Kicinski wrote:
> > 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.
> >   
> Ok. I think I have a basic summary of the situation and whats going on
> in the firmware. I'll try to summarize here:
> 
> Firmware has a state machine that we call the Link Establishment State
> Machine. This is the state machine which will attempt to establish link.
> This state machine only applies when auto-negotiation is not used. If
> auto-negotation is used it will perform the standard auto-negotiation
> flow and set FEC through that method.
> 
> The way this works follows this flow:
> 
> 1) driver sends a set PHY capabilities request. This includes various
> bits including whether to automatically select FEC, and which FEC modes
> to select from. When we enable ETHTOOL_FEC_AUTO, the driver always sets
> all FEC modes with the auto FEC bit.
> 
> 2) the firmware receives this request and begins the LESM. This starts
> with the firmware generating a list of configurations to attempt. Each
> configuration is a possible link mode combined with a bitwise AND of the
> FEC modes requested above in set PHY capabiltiies and the set of FEC
> modes supported by that link mode. The example I gave was if you plugged
> in a CA-L cable, it would try:
> 
>   100G-CAUI4
>   50G-LAUI2
>   25G-AUI-C2C
>   10G-SFI-DA
> 
> I'm still not 100% sure how it decides which link modes to choose for
> which cable, but I believe this is in a table stored within the firmware
> module we call the netlist.
> 
> 2a) for older firmware, the set PHY capabiltiies interface does not have
> a bit to set for requesting No FEC. Instead, each media type has a
> determination made about whether it needed FEC of not. I was told for
> example that CA-N cables would enable No FEC as an option, but CA-L
> cables would not (even though No FEC is supported for the link modes in
> question).
> 
> 2b) on newer firmware, the set PHY capabilities interface does have a
> bit to request No FEC. In this case, if we set the No FEC bit, then the
> firmware will be able to select No FEC as an option for cables that
> otherwise wouldn't have selected it in the old firmware (such as CA-L
> cables mentioned above).

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.

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?

> 3) once the firmware has generated the list of possible configurations,
> it will iterate through them in a loop. Each configuration is applied,
> and then we wait some time (the timeout is also stored in the netlist
> module). If link establishes at one of these phases, we stop and use
> that configuration. Otherwise we move to the next configuration and try
> that. Each FEC mode is tried in sequence. (Unless the automatic FEC
> selection bit is *not* set. In that case, only one of the FEC modes is
> tried instead, and it is expected that software only set one bit to try.
> That would perform forced FEC selection instead).
> 
> This process will repeat as it iterates through the configurations until
> link is established.
> 
> As a side note, the first stage is to try auto-negotiation if enabled.
> So in the case where auto-negotiation is enabled it will first try
> auto-negotiation, then the set of forced configurations, and then loop
> back to trying auto-negotiation before trying the forced configs again.
> 
> So from the software programming state, we currently translate
> ETHTOOL_FEC_AUTO by setting the automatic bit as well as setting every
> FEC mode bit, except the "No FEC" bit. This is a new bit which is only
> available on newer firmware.
> 
> With the proposed change, we would add the "No FEC" bit when user
> requested both ETHTOOL_FEC_AUTO and ETHTOOL_FEC_OFF simultaneously.
> 
> From reading your previous replies, you would prefer to just have the
> driver set the "No FEC" bit always for ETHTOOL_FEC_AUTO when its
> available/supported by firmware?