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

[MD Danish Anwar]

Hi Jakub,

On 20/05/26 5:26 am, Jakub Kicinski wrote:
> 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.

What should be the next steps here? Is there any existing defined set of
stats where I could populate stats from ICSSG firmware for HSR (similar
to ndo_get_stats64 callback). Or de we need to implement a new callback
that will do this for HSR.

I agree with Luka on the categorization,

Below stats can be generic,
 - unique rx
 - duplicate rx
 - multiple rx
 - rx / tx
 - own rx
 - wrong LAN, PRP only

Below stats can be driver specific and can be pulled using `ethtool -S`
on child interfaces of HSR

 - FW_HSR_FWD_CHECK_FAIL_DROP
 - FW_HSR_HE_CHECK_FAIL_DROP
 - FW_HSR_SKIP_HOST_DUP_DISCARD_FRAMES

Let me know if I should go ahead and implement this.

-- 
Thanks and Regards,
Danish