Re: [PATCH net-next v2 2/2] net: ti: icssg: Add HSR and LRE PA statistics

[Felix Maurer <fmaurer@redhat.com>]

Hi everyone,

On Tue, May 19, 2026 at 07:55:55AM +0200, Luka Gejak wrote:
> On May 19, 2026 3:45:06 AM GMT+02:00, Jakub Kicinski <kuba@kernel.org> wrote:
> >On Thu, 14 May 2026 13:26:05 +0530 MD Danish Anwar wrote:
> >> Add new firmware PA statistics counters for HSR and LRE to the ethtool
> >> statistics exposed by the ICSSG driver.
> >>
> >> New statistics added:
> >>  - FW_HSR_FWD_CHECK_FAIL_DROP: Packets dropped on the HSR forwarding path
> >>  - FW_HSR_HE_CHECK_FAIL_DROP: Packets dropped on the HSR host egress path
> >>  - FW_HSR_SKIP_HOST_DUP_DISCARD_FRAMES: Frames with duplicate discard
> >>    skipped
> >>  - FW_LRE_CNT_UNIQUE/DUPLICATE/MULTIPLE_RX: LRE duplicate detection
> >>    counters
> >>  - FW_LRE_CNT_RX/TX: LRE per-port frame counters
> >>  - FW_LRE_CNT_OWN_RX: Own HSR tagged frames received
> >>  - FW_LRE_CNT_ERRWRONGLAN: Frames with wrong LAN identifier (PRP)
> >>
> >> Document the new HSR/LRE statistics in icssg_prueth.rst.
> >
> >To an untrained eye these stats look like stuff that could
> >be standardized across drivers.
> >
> >Luka, Felix, others on CC, do you think we should expose these
> >from HSR over netlink as "standard" offload stats different drivers
> >can plug into or not worth it?
>
> Hi Jakub,
> I think there is a case for standardizing part of this, but I would
> not standardize the whole set as-is.
>
> The LRE counters look generic enough to me, especially:
>  - unique rx
>  - duplicate rx
>  - multiple rx
>  - rx / tx
>  - own rx
>  - wrong LAN, PRP only

I'm very much in favor of having standardized stats for hsr hardware
offloads that the drivers can supply. The list above looks about right,
I'd add "frames with errors" and "(proxy) node table entry count" as
well and that "own rx" is HSR only.

In general, I don't think we need to standardize this ourselves but can
adapt to the counters that the SNMP MIB for IEC 62439-3 [1] already has.
It's part of the standard and IMHO we should gather these counters from
offloads (and later supply the same set from our sw implementation, but
the current netlink interface for hsr is quite messy). For reference,
the list in the MIB is (no need to fully adopt this naming):
- lreCntTx{A,B,C}: Sent frames per-port (for A,B only tagged frames)
- lreCntRx{A,B,C}: Received frames per-port (for A,B only tagged frames)
- lreCntErrWrongLan{A,B}: Received frames per-port with wrong LAN ID
  (only for PRP)
- lreCntErrWrongLanC: Received frames on interlink port of HSR-PRP
  RedBox with wrong LAN ID
- lreCntErrors{A,B,C}: Received frames with errors per-port
- lreCnt{,Proxy}Nodes: Nodes in the (proxy) node table
- lreCntUnique{A,B,C}: Frames only received once, per-port
- lreCntDuplicate{A,B,C}: Frames received with exactly one duplicate,
  per-port
- lreCntMulti{A,B,C}: Frames received with more than one duplicate,
  per-port
- lreCntOwnRx{A,B}: Frames received per-port (A,B) that originated from
  this node, only for HSR rings

Note that we can not currently completely distinguish
Unique/Duplicate/Multi in the kernel implementation and their meaning is
not entirely clear to me from the MIB.

The explanations in the MIB in [1] are otherwise quite explicit for each
of the counters but we may want to adapt the meaning of "port C" to the
counters. For example, there is lreCntRx{A,B,C} for received HSR/PRP
tagged frames (by the LRE). Port A and B are clear, but for port C the
meaning is "number of frames received from the application interface of
a DANP or DANH or the number of number of frames received on the
interlink of a RedBox". IMHO, we should consider separating "application
interface" (what the kernel calls master) and the interlink port because
these two are not mutually exclusive in the kernel (nor in the NICs that
support hardware offload).

Thanks,
   Felix


[1]: you can find it for example here: https://mibbrowser.online/mibdb_search.php?mib=IEC-62439-3-MIB