Re: [RFC PATCH net-next] net: ethtool: add support for forward error correction modes

[Casey Leedom]

  I'm attempting to start the work necessary to implement Vidya's proposed new 
ethtool interface to manage Forward Error Correction settings on a link.  I'm 
confused by the ethtool FEC API and the degree/type of control it offers.  At 
the top of the patch we have:

Encoding: Types of encoding
Off:  Turning off any encoding
RS :  enforcing RS-FEC encoding on supported speeds
BaseR  :  enforcing Base R encoding on supported speeds
Auto   :  Default FEC settings  for  divers , and would represent
  asking the hardware to essentially go into a best effort mode.

but then later on we have:

+struct ethtool_fecparam {
+ __u32   cmd;
+ __u32   autoneg;
+ /* bitmask of FEC modes */
+ __u32   fec;
+ __u32   reserved;
+};

...

+enum ethtool_fec_config_bits {
+   ETHTOOL_FEC_NONE_BIT,
+   ETHTOOL_FEC_AUTO_BIT,
+   ETHTOOL_FEC_OFF_BIT,
+   ETHTOOL_FEC_RS_BIT,
+   ETHTOOL_FEC_BASER_BIT,
+};

...

+   ETHTOOL_LINK_MODE_FEC_NONE_BIT  = 47,
+   ETHTOOL_LINK_MODE_FEC_RS_BIT= 48,
+   ETHTOOL_LINK_MODE_FEC_BASER_BIT   = 49,

The last ethtool Link Mode bits seem to imply a separable FEC on/off with 
individual control for RS and BASER.  How would the "Auto" from the top be 
encoded within these Link Mode bits?  And I don't see any reference to the 
ethtool_fec_config_bits in the kernel or ethtool patches so I'm not sure what 
they're supposed to reference.  Can you clarify the above?  I.e. can you offer 
a small template example of what a driver implementation might look like 
interpreting the incoming Link Mode Bits?

  And do we expect that there will be new FECs in the future?

Casey

Re: [RFC PATCH net-next] net: ethtool: add support for forward error correction modes

[Casey Leedom]

  And by the way, we currently have two ethtool APIs which pump in an 
Auto-Negotiation indication -- set_link_ksettings() and set_pauseparam().  Now 
we're talking about adding a third, set_fecparam().  Are all of the calls to 
these three APIs supposed to agree on the concept of Auto-Negotiations?  I.e. 
what's it mean if set_link_ksettings() gets called with 
link_ksettings->base.autoneg == AUTONEG_ENABLE but set_pauseparam() gets called 
with epause->autoneg == AUTONEG_DISABLE?  And now adding set_fecparam() into 
the system with a similar ability to specify the state of Auto-Negotiation is 
even more confusing.

Casey

[RFC PATCH net-next] net: ethtool: add support for forward error correction modes

[Casey Leedom]

N.B.  Sorry I'm not able to respond to the original message since I
wasn't subscribed to netdev when it was sent a couple of weeks ago.

This feature is something that Chelsio's cxgb4 driver needs.
As we've tested our adapters against a number of switches,
we've discovered a few which use varying defaults for FEC.
And when Auto-Negotiation isn't used (or even possible with
Optical Links), we need to be able to control turning FEC on/off.

For our part, we default FEC Off for Optical Transceivers.
For Copper, we read the Cable's EEPROM to determine
how to default FEC.  For some switches this works, but
for at least one where that switch enables FEC for Optical
Transceivers, it doesn't.  For that switch we had to hard
wire FEC on.  Obviously that's not a good solution and we
need an administrative interface so the system
administrator can configure our adapter to use the
appropriate FEC setting to match that of the switch.

So this is basically a long-winded ACK of Vidya's patch
and we would immediately implement this new ethtool API
as soon as it's available.

Casey

Re: [RFC PATCH net-next] net: ethtool: add support for forward error correction modes

[Vidya Sagar Ravipati]

On Thu, Nov 3, 2016 at 6:24 AM, Gal Pressman  wrote:
>
>
> On 25/10/2016 05:50, Vidya Sagar Ravipati wrote:
>> SET FEC option:
>> root@tor: ethtool --set-fec  swp1 encoding [off | RS | BaseR | auto] autoneg 
>> [off | on]
>>
>> Encoding: Types of encoding
>> Off:  Turning off any encoding
>> RS :  enforcing RS-FEC encoding on supported speeds
>> BaseR  :  enforcing Base R encoding on supported speeds
>> Auto   :  Default FEC settings  for  divers , and would represent
>
> divers? :)
Drivers :)
>
>>   asking the hardware to essentially go into a best effort mode.
>>
>> Here are a few examples of what we would expect if encoding=auto:
>> - if autoneg is on, we are  expecting FEC to be negotiated as on or off
>>   as long as protocol supports it
>> - if the hardware is capable of detecting the FEC encoding on it's
>>   receiver it will reconfigure its encoder to match
>> - in absence of the above, the configuration would be set to IEEE
>>   defaults.
>
> Not sure I follow, why do we need an autoneg option if encoding type can be 
> set to auto?
Auto is one of the FEC configuration modes  which indicates  the
drivers to set the IEEE defaults based on speed/duplex combination of
the port. i.e. RS FEC mode will be set in case of 100G/full

Auto negotiation is the configuration for the link  to negotiate the
FEC capabilities and different FEC modes with other endpoint  using
encoded bits D44:47 in base link code word.

Re: [RFC PATCH net-next] net: ethtool: add support for forward error correction modes

[Gal Pressman]

On 25/10/2016 05:50, Vidya Sagar Ravipati wrote:
> SET FEC option:
> root@tor: ethtool --set-fec  swp1 encoding [off | RS | BaseR | auto] autoneg 
> [off | on]
> 
> Encoding: Types of encoding
> Off:  Turning off any encoding
> RS :  enforcing RS-FEC encoding on supported speeds
> BaseR  :  enforcing Base R encoding on supported speeds
> Auto   :  Default FEC settings  for  divers , and would represent

divers? :)

>   asking the hardware to essentially go into a best effort mode.
> 
> Here are a few examples of what we would expect if encoding=auto:
> - if autoneg is on, we are  expecting FEC to be negotiated as on or off
>   as long as protocol supports it
> - if the hardware is capable of detecting the FEC encoding on it's
>   receiver it will reconfigure its encoder to match
> - in absence of the above, the configuration would be set to IEEE
>   defaults.

Not sure I follow, why do we need an autoneg option if encoding type can be set 
to auto?

[RFC PATCH net-next] net: ethtool: add support for forward error correction modes

[Vidya Sagar Ravipati]

From: Vidya Sagar Ravipati 

Forward Error Correction (FEC) modes i.e Base-R
and Reed-Solomon modes are introduced in 25G/40G/100G standards
for providing good BER at high speeds.
Various networking devices which support 25G/40G/100G provides ability
to manage supported FEC modes and the lack of FEC encoding control and
reporting today is a source for itneroperability issues for many vendors.
FEC capability as well as specific FEC mode i.e. Base-R
or RS modes can be requested or advertised through bits D44:47 of base link
codeword.

This patch set intends to provide option under ethtool to manage and report
FEC encoding settings for networking devices as per IEEE 802.3 bj, bm and by
specs.

set-fec/show-fec option(s) are  designed to provide  control and report
the FEC encoding on the link.

SET FEC option:
root@tor: ethtool --set-fec  swp1 encoding [off | RS | BaseR | auto] autoneg 
[off | on]

Encoding: Types of encoding
Off:  Turning off any encoding
RS :  enforcing RS-FEC encoding on supported speeds
BaseR  :  enforcing Base R encoding on supported speeds
Auto   :  Default FEC settings  for  divers , and would represent
  asking the hardware to essentially go into a best effort mode.

Here are a few examples of what we would expect if encoding=auto:
- if autoneg is on, we are  expecting FEC to be negotiated as on or off
  as long as protocol supports it
- if the hardware is capable of detecting the FEC encoding on it's
  receiver it will reconfigure its encoder to match
- in absence of the above, the configuration would be set to IEEE
  defaults.

>From our  understanding , this is essentially what most hardware/driver
combinations are doing today in the absence of a way for users to
control the behavior.

SHOW FEC option:
root@tor: ethtool --show-fec  swp1
FEC parameters for swp1:
Autonegotiate:  off
FEC encodings:  RS

ETHTOOL DEVNAME output modification:

ethtool devname output:
root@tor:~# ethtool swp1
Settings for swp1:
root@hpe-7712-03:~# ethtool swp18
Settings for swp18:
Supported ports: [ FIBRE ]
Supported link modes:   4baseCR4/Full
4baseSR4/Full
4baseLR4/Full
10baseSR4/Full
10baseCR4/Full
10baseLR4_ER4/Full
Supported pause frame use: No
Supports auto-negotiation: Yes
Supported FEC modes: [RS | BaseR | None | Not reported]
Advertised link modes:  Not reported
Advertised pause frame use: No
Advertised auto-negotiation: No
Advertised FEC modes: [RS | BaseR | None | Not reported]
 One or more FEC modes
Speed: 10Mb/s
Duplex: Full
Port: FIBRE
PHYAD: 106
Transceiver: internal
Auto-negotiation: off
Link detected: yes

This patch includes following changes
a) New ETHTOOL_SFECPARAM/SFECPARAM API, handled by
  the new get_fecparam/set_fecparam callbacks, provides support
  for configuration of forward error correction modes.
b) Link mode bits for FEC modes i.e. None (No FEC mode), RS, BaseR/FC
  are defined so that users can configure these fec modes for supported
  and advertising fields as part of link autonegotiation.

Signed-off-by: Vidya Sagar Ravipati 
---
 include/linux/ethtool.h  |  4 
 include/uapi/linux/ethtool.h | 53 ++--
 net/core/ethtool.c   | 34 
 3 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 9ded8c6..79a0bab 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -372,5 +372,9 @@ struct ethtool_ops {
  struct ethtool_link_ksettings *);
int (*set_link_ksettings)(struct net_device *,
  const struct ethtool_link_ksettings *);
+   int (*get_fecparam)(struct net_device *,
+ struct ethtool_fecparam *);
+   int (*set_fecparam)(struct net_device *,
+ struct ethtool_fecparam *);
 };
 #endif /* _LINUX_ETHTOOL_H */
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index 099a420..28ea382 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -1224,6 +1224,51 @@ struct ethtool_per_queue_op {
chardata[];
 };
 
+/**
+ * struct ethtool_fecparam - Ethernet forward error correction(fec) parameters
+ * @cmd: Command number = %ETHTOOL_GFECPARAM or %ETHTOOL_SFECPARAM
+ * @autoneg: Flag to enable autonegotiation of fec modes(rs,baser)
+ *  (D44:47 of base link code word)
+ * @fec: Bitmask of supported FEC modes
+ * @rsvd: Reserved for future extensions. i.e FEC bypass feature.
+ *
+ * Drivers should reject a non-zero setting of @autoneg when
+ * autoneogotiation is disabled (or not supported) for the