Re: [PATCH 0/4] ethdev: report module signal status flags

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu,  8 Oct 2026 10:23:22 +0200
Roman Khromenok <roma55592@yandex.ru> wrote:

> The module EEPROM decoder shows the identity and the digital
> diagnostics of a module, but not the signal status, which is
> usually the first thing to check when a link does not come up.
> 
> Patches 1-2 report the SFF-8636 per-lane loss of signal,
> CDR loss of lock and Tx fault flags, as ethtool does since
> commit 045d8db ("sff-8636: report LOL / LOS / Tx Fault").
> The code is written for DPDK, not copied from ethtool.
> 
> Patches 3-4 report the SFF-8472 Rx_LOS and TX_FAULT state
> from page A2h byte 110. ethtool does not show it, so it is
> split out and can be dropped independently of patches 1-2.
> 
> The SFF-8636 flags are latched and cleared on read,
> so they report the events since the previous read;
> the SFF-8472 bits are the current state of the pins.
> 
> The series is based on next-net/for-main. It extends the same
> release notes entry as the testpmd patch 170851 ("app/testpmd: add
> command to decode module EEPROM"), so whichever is applied second
> needs a trivial context fixup there.
> 
> Roman Khromenok (4):
>   ethdev: report SFF-8636 lane status flags
>   test: check SFF-8636 lane status flags
>   ethdev: report SFF-8472 Rx LOS and Tx fault state
>   test: check SFF-8472 Rx LOS and Tx fault state

I am not an expert in this area, deferred to AI for review
and it reported one item worth checking.

Series: [PATCH 1/4..4/4] ethdev: report SFF module status flags
Author: Roman Khromenok

Summary
-------
Bit and offset mapping checked against SFF-8636 and SFF-8472:
bytes 3-5 lane split, Options 2/3/4 implemented bits, A0 byte 93
bits 4/5 and A2 byte 110 bits 1/2 are all correct. Lane string
format matches ethtool output. The SFF-8472 status is only shown
when DOM is supported (early return in sff_8472_show_all), which
is correct since byte 110 lives in A2.

Series depends on the module EEPROM decode series (the test file
and the release note entry are not in main yet); say so in the
cover letter.

Patch 1/4
---------
Warning: Rx LOS gated on Tx LOS implemented bit.

  /* There is no Rx LOS implemented bit, use the Tx one for both */
  if (data[SFF_8636_OPTION_4_OFFSET] & SFF_8636_O4_TX_LOS) {

Rx LOS has no implemented bit because it is not optional in
SFF-8636; only Tx LOS is (Options 4 bit 1). Gating Rx on the Tx
bit hides the most useful flag on every module without Tx LOS,
which is the case the commit message says this is for. ethtool
has the same limitation; no reason to copy it. Show Rx LOS
unconditionally:

  sff_show_lane_status("Rx loss of signal", SFF_MAX_CHANNEL_NUM,
                       SFF_8636_LANES_LOW(los), d);
  if (data[SFF_8636_OPTION_4_OFFSET] & SFF_8636_O4_TX_LOS)
          sff_show_lane_status("Tx loss of signal",
                               SFF_MAX_CHANNEL_NUM,
                               SFF_8636_LANES_HIGH(los), d);

and update the not_implemented test in 2/4 to expect
CHECK_FIELD("Rx loss of signal", "[ Yes, Yes, Yes, Yes ]").

Patch 2/4
---------
Info: fill_qsfp_signals() overwrites option bytes 193-195 instead
of setting bits. Any bits fill_qsfp() set there (page 01h/02h
provided, etc.) are lost, which can silently change what the
other decoders see. Use |= :

  data[QSFP_OPTIONS_2] |= 0x08;
  data[QSFP_OPTIONS_3] |= 0x30;
  data[QSFP_OPTIONS_4] |= 0x0a;

Patch 3/4
---------
Info: SFP shows "No" when clear, QSFP shows "None" for the same
field names ("Rx loss of signal", "Tx fault"). Telemetry
consumers keyed on name get two value conventions. Either use
"None"/"[ Yes ]" style for SFP too, or accept it and note it in
the commit message.

Info: release note line reads as a separate feature; "The
SFF-8472 decoder also reports ..." to match the 8636 line.

Patch 4/4
---------
Info: same as 2/4, data[SFP_ENH_OPTIONS] = 0x30 clears the
alarm/warning implemented bit (bit 7) if fill_sfp() sets it.
Use |= 0x30.