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

[Jakub Kicinski]

On Tue, 19 May 2026 07:55:55 +0200 Luka Gejak wrote:
> On May 19, 2026 3:45:06 AM GMT+02:00, Jakub Kicinski <kuba at 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?  
> 
> 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
> 
> Those are protocol/LRE concepts rather than TI firmware details, so
> exposing them from the HSR/PRP layer sounds useful. I would expect 
> both the software implementation and offloaded implementations to be 
> able to provide at least some of them, with unsupported counters 
> omitted or reported as not available.
> I would not put the firmware check/drop counters in the same standard
> bucket, though:
>  - FW_HSR_FWD_CHECK_FAIL_DROP
>  - FW_HSR_HE_CHECK_FAIL_DROP
>  - FW_HSR_SKIP_HOST_DUP_DISCARD_FRAMES

Thanks for the breakdown!

> Those sound more like implementation/debug counters for the ICSSG
> firmware pipeline. They are still useful in ethtool driver stats, but 
> I would be hesitant to bake their exact semantics into HSR UAPI.
> So my preference would be:
>  1. Keep driver-private ethtool stats for the full firmware counter set.
>  2. Add a small HSR/PRP standard stats set separately, limited to
>     well-defined LRE counters.
>  3. Make the HSR layer expose them, with offload drivers plugging in via
>     an optional callback or offload stats op.
>  4. Define the counters carefully, including whether they are per-HSR
>     device or per-port A/B, and what PRP-only counters mean for HSR.
> 
> I do not think this patch should blindly become the UAPI definition, 

Not at all, the unique / multiple stats gave me pause. We should
only put in the standard API what can be easily and unambiguously
defined given the protocol spec.

> but I do think it points at a useful follow-up. If we want to avoid 
> adding driver-private names first and then standardizing different 
> names later, then it may be worth asking Danish to split the 
> protocol-level LRE counters out and route those through a common HSR 
> stats interface.

As a general policy we ask for standard stats to be added first and
ethtool to only contain what didn't fit in the standard ones.
There are some technical reasons but it's mostly a mindset thing.