[PATCH net-next v2 0/2] Add ICSSG firmware stats related to HSR

[PATCH net-next v2 0/2] Add ICSSG firmware stats related to HSR

[MD Danish Anwar]

This series adds HSR and LRE firmware PA statistics to the TI ICSSG
ethtool stats interface, and places static_assert() guards next to the
stat descriptor arrays to catch count mismatches at build time.

Patch 1 adds static_assert() immediately after each of
icssg_all_miig_stats[] and icssg_all_pa_stats[] in icssg_stats.h,
verifying that ICSSG_NUM_MIIG_STATS and ICSSG_NUM_PA_STATS stay in
sync with the actual array sizes.

Patch 2 adds ten new firmware counters for HSR forwarding-path drops,
host-egress-path drops, and LRE duplicate-detection, updates
icssg_ndo_get_stats64() to fold the relevant counters into rx_errors
and rx_dropped, bumps ICSSG_NUM_PA_STATS to 42 (caught immediately by
the static_assert from patch 1 if the constant is ever left behind),
and documents all new entries in icssg_prueth.rst.

Changes in v2:
 - Drop the ARRAY_SIZE()-based macro approach from v1 (which caused
   binary bloat by pulling the static const arrays into every TU via
   icssg_prueth.h) as suggested by David Carlier <devnexen@gmail.com>
 - Add static_assert() next to each array in icssg_stats.h instead,
   keeping the numeric #defines and the original include graph. As
   suggested by David Carlier <devnexen@gmail.com>

v1 https://lore.kernel.org/all/20260512060627.3781329-1-danishanwar@ti.com/

MD Danish Anwar (2):
  net: ti: icssg: Add static_assert to guard stat array counts
  net: ti: icssg: Add HSR and LRE PA statistics

 .../device_drivers/ethernet/ti/icssg_prueth.rst    | 10 ++++++++++
 drivers/net/ethernet/ti/icssg/icssg_common.c       |  7 +++++--
 drivers/net/ethernet/ti/icssg/icssg_prueth.h       |  2 +-
 drivers/net/ethernet/ti/icssg/icssg_stats.h        | 14 ++++++++++++++
 drivers/net/ethernet/ti/icssg/icssg_switch_map.h   | 10 ++++++++++
 5 files changed, 40 insertions(+), 3 deletions(-)


base-commit: 18dc8e6d15d7a30888beec46a1e01ca0f98508fa
-- 
2.34.1

[PATCH net-next v2 1/2] net: ti: icssg: Add static_assert to guard stat array counts

[MD Danish Anwar]

Place static_assert() immediately after each of icssg_all_miig_stats[]
and icssg_all_pa_stats[] in icssg_stats.h to verify at build time that
ICSSG_NUM_MIIG_STATS and ICSSG_NUM_PA_STATS stay in sync with the
actual array sizes. This turns a silent miscount into a build error
should either the constant or the array be updated independently.

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 drivers/net/ethernet/ti/icssg/icssg_stats.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/ti/icssg/icssg_stats.h b/drivers/net/ethernet/ti/icssg/icssg_stats.h
index 5ec0b38e0c67..6f4400d8a0f6 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_stats.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_stats.h
@@ -155,6 +155,8 @@ static const struct icssg_miig_stats icssg_all_miig_stats[] = {
 	ICSSG_MIIG_STATS(tx_bytes, true),
 };
 
+static_assert(ARRAY_SIZE(icssg_all_miig_stats) == ICSSG_NUM_MIIG_STATS);
+
 #define ICSSG_PA_STATS(field)	\
 {				\
 	#field,			\
@@ -201,4 +203,6 @@ static const struct icssg_pa_stats icssg_all_pa_stats[] = {
 	ICSSG_PA_STATS(FW_HOST_EGRESS_Q_EXP_OVERFLOW),
 };
 
+static_assert(ARRAY_SIZE(icssg_all_pa_stats) == ICSSG_NUM_PA_STATS);
+
 #endif /* __NET_TI_ICSSG_STATS_H */
-- 
2.34.1

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

[MD Danish Anwar]

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.

Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
---
 .../device_drivers/ethernet/ti/icssg_prueth.rst        | 10 ++++++++++
 drivers/net/ethernet/ti/icssg/icssg_common.c           |  7 +++++--
 drivers/net/ethernet/ti/icssg/icssg_prueth.h           |  2 +-
 drivers/net/ethernet/ti/icssg/icssg_stats.h            | 10 ++++++++++
 drivers/net/ethernet/ti/icssg/icssg_switch_map.h       | 10 ++++++++++
 5 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/Documentation/networking/device_drivers/ethernet/ti/icssg_prueth.rst b/Documentation/networking/device_drivers/ethernet/ti/icssg_prueth.rst
index da21ddf431bb..b0bda7327b2a 100644
--- a/Documentation/networking/device_drivers/ethernet/ti/icssg_prueth.rst
+++ b/Documentation/networking/device_drivers/ethernet/ti/icssg_prueth.rst
@@ -54,3 +54,13 @@ These statistics are as follows,
  - ``FW_HOST_TX_PKT_CNT``: Number of valid packets copied by RTU0 to Tx queues
  - ``FW_HOST_EGRESS_Q_PRE_OVERFLOW``: Host Egress Q (Pre-emptible) Overflow Counter
  - ``FW_HOST_EGRESS_Q_EXP_OVERFLOW``: Host Egress Q (Pre-emptible) Overflow Counter
+ - ``FW_HSR_FWD_CHECK_FAIL_DROP``: Packets dropped on the HSR forwarding path due to failed checks
+ - ``FW_HSR_HE_CHECK_FAIL_DROP``: Packets dropped on the host egress path due to failed checks
+ - ``FW_HSR_SKIP_HOST_DUP_DISCARD_FRAMES``: Frames for which the host duplicate discard check was skipped
+ - ``FW_LRE_CNT_UNIQUE_RX``: Number of frames received with no duplicate detected
+ - ``FW_LRE_CNT_DUPLICATE_RX``: Number of frames received for which exactly one duplicate was detected
+ - ``FW_LRE_CNT_MULTIPLE_RX``: Number of frames received for which more than one duplicate was detected
+ - ``FW_LRE_CNT_RX``: Number of HSR/PRP tagged frames received
+ - ``FW_LRE_CNT_TX``: Number of HSR/PRP tagged frames sent
+ - ``FW_LRE_CNT_OWN_RX``: Number of HSR/PRP tagged frames received whose source MAC matches the node's own address
+ - ``FW_LRE_CNT_ERRWRONGLAN``: Number of frames received with a wrong LAN identifier, PRP only
diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index a28a608f9bf4..e7a51a9eee24 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -1633,7 +1633,8 @@ void icssg_ndo_get_stats64(struct net_device *ndev,
 			    emac_get_stat_by_name(emac, "FW_RX_EOF_SHORT_FRMERR") +
 			    emac_get_stat_by_name(emac, "FW_RX_B0_DROP_EARLY_EOF") +
 			    emac_get_stat_by_name(emac, "FW_RX_EXP_FRAG_Q_DROP") +
-			    emac_get_stat_by_name(emac, "FW_RX_FIFO_OVERRUN");
+			    emac_get_stat_by_name(emac, "FW_RX_FIFO_OVERRUN") +
+			    emac_get_stat_by_name(emac, "FW_LRE_CNT_ERRWRONGLAN");
 	stats->rx_dropped = ndev->stats.rx_dropped +
 			    emac_get_stat_by_name(emac, "FW_DROPPED_PKT") +
 			    emac_get_stat_by_name(emac, "FW_INF_PORT_DISABLED") +
@@ -1643,7 +1644,9 @@ void icssg_ndo_get_stats64(struct net_device *ndev,
 			    emac_get_stat_by_name(emac, "FW_INF_DROP_TAGGED") +
 			    emac_get_stat_by_name(emac, "FW_INF_DROP_PRIOTAGGED") +
 			    emac_get_stat_by_name(emac, "FW_INF_DROP_NOTAG") +
-			    emac_get_stat_by_name(emac, "FW_INF_DROP_NOTMEMBER");
+			    emac_get_stat_by_name(emac, "FW_INF_DROP_NOTMEMBER") +
+			    emac_get_stat_by_name(emac, "FW_HSR_FWD_CHECK_FAIL_DROP") +
+			    emac_get_stat_by_name(emac, "FW_HSR_HE_CHECK_FAIL_DROP");
 	stats->tx_errors  = ndev->stats.tx_errors;
 	stats->tx_dropped = ndev->stats.tx_dropped +
 			    emac_get_stat_by_name(emac, "FW_RTU_PKT_DROP") +
diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
index df93d15c5b78..60a8aedd334b 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
@@ -57,7 +57,7 @@
 
 #define ICSSG_MAX_RFLOWS	8	/* per slice */
 
-#define ICSSG_NUM_PA_STATS	32
+#define ICSSG_NUM_PA_STATS	42
 #define ICSSG_NUM_MIIG_STATS	60
 /* Number of ICSSG related stats */
 #define ICSSG_NUM_STATS (ICSSG_NUM_MIIG_STATS + ICSSG_NUM_PA_STATS)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_stats.h b/drivers/net/ethernet/ti/icssg/icssg_stats.h
index 6f4400d8a0f6..08b5ab6f93da 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_stats.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_stats.h
@@ -201,6 +201,16 @@ static const struct icssg_pa_stats icssg_all_pa_stats[] = {
 	ICSSG_PA_STATS(FW_HOST_TX_PKT_CNT),
 	ICSSG_PA_STATS(FW_HOST_EGRESS_Q_PRE_OVERFLOW),
 	ICSSG_PA_STATS(FW_HOST_EGRESS_Q_EXP_OVERFLOW),
+	ICSSG_PA_STATS(FW_HSR_FWD_CHECK_FAIL_DROP),
+	ICSSG_PA_STATS(FW_HSR_HE_CHECK_FAIL_DROP),
+	ICSSG_PA_STATS(FW_HSR_SKIP_HOST_DUP_DISCARD_FRAMES),
+	ICSSG_PA_STATS(FW_LRE_CNT_UNIQUE_RX),
+	ICSSG_PA_STATS(FW_LRE_CNT_DUPLICATE_RX),
+	ICSSG_PA_STATS(FW_LRE_CNT_MULTIPLE_RX),
+	ICSSG_PA_STATS(FW_LRE_CNT_RX),
+	ICSSG_PA_STATS(FW_LRE_CNT_TX),
+	ICSSG_PA_STATS(FW_LRE_CNT_OWN_RX),
+	ICSSG_PA_STATS(FW_LRE_CNT_ERRWRONGLAN),
 };
 
 static_assert(ARRAY_SIZE(icssg_all_pa_stats) == ICSSG_NUM_PA_STATS);
diff --git a/drivers/net/ethernet/ti/icssg/icssg_switch_map.h b/drivers/net/ethernet/ti/icssg/icssg_switch_map.h
index 7e053b8af3ec..bd2d54dd7f45 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_switch_map.h
+++ b/drivers/net/ethernet/ti/icssg/icssg_switch_map.h
@@ -266,5 +266,15 @@
 #define FW_HOST_TX_PKT_CNT		0x0250
 #define FW_HOST_EGRESS_Q_PRE_OVERFLOW	0x0258
 #define FW_HOST_EGRESS_Q_EXP_OVERFLOW	0x0260
+#define FW_HSR_FWD_CHECK_FAIL_DROP		0x0500
+#define FW_HSR_HE_CHECK_FAIL_DROP		0x0508
+#define FW_HSR_SKIP_HOST_DUP_DISCARD_FRAMES	0x0510
+#define FW_LRE_CNT_UNIQUE_RX			0x0518
+#define FW_LRE_CNT_DUPLICATE_RX			0x0520
+#define FW_LRE_CNT_MULTIPLE_RX			0x0528
+#define FW_LRE_CNT_RX				0x0530
+#define FW_LRE_CNT_TX				0x0538
+#define FW_LRE_CNT_OWN_RX			0x0540
+#define FW_LRE_CNT_ERRWRONGLAN			0x0548
 
 #endif /* __NET_TI_ICSSG_SWITCH_MAP_H  */
-- 
2.34.1

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

[Jakub Kicinski]

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?

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

[Luka Gejak]

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

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

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, 
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.

Best regards,
Luka Gejak

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

[Paolo Abeni]

On 5/14/26 9:56 AM, MD Danish Anwar wrote:
> @@ -201,6 +201,16 @@ static const struct icssg_pa_stats icssg_all_pa_stats[] = {
>  	ICSSG_PA_STATS(FW_HOST_TX_PKT_CNT),
>  	ICSSG_PA_STATS(FW_HOST_EGRESS_Q_PRE_OVERFLOW),
>  	ICSSG_PA_STATS(FW_HOST_EGRESS_Q_EXP_OVERFLOW),
> +	ICSSG_PA_STATS(FW_HSR_FWD_CHECK_FAIL_DROP),
> +	ICSSG_PA_STATS(FW_HSR_HE_CHECK_FAIL_DROP),
> +	ICSSG_PA_STATS(FW_HSR_SKIP_HOST_DUP_DISCARD_FRAMES),

Sashiko noted that this statistic name exceed the ethtool string limit.

/P

Re: [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@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.

Re: [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@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

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

[Felix Maurer]

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