Netdev List
 help / color / mirror / Atom feed
From: Luka Gejak <luka.gejak@linux.dev>
To: Jakub Kicinski <kuba@kernel.org>,
	MD Danish Anwar <danishanwar@ti.com>,
	Felix Maurer <fmaurer@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>, Simon Horman <horms@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Roger Quadros <rogerq@kernel.org>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	Meghana Malladi <m-malladi@ti.com>,
	Jacob Keller <jacob.e.keller@intel.com>,
	David Carlier <devnexen@gmail.com>,
	Vadim Fedorenko <vadim.fedorenko@linux.dev>,
	Kevin Hao <haokexin@gmail.com>,
	netdev@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	luka.gejak@linux.dev
Subject: Re: [PATCH net-next v2 2/2] net: ti: icssg: Add HSR and LRE PA statistics
Date: Tue, 19 May 2026 07:55:55 +0200	[thread overview]
Message-ID: <E30AAC96-01D2-4A23-B562-126087DEB7FA@linux.dev> (raw)
In-Reply-To: <20260518184506.694c584e@kernel.org>

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

  reply	other threads:[~2026-05-19  5:56 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-14  7:56 [PATCH net-next v2 0/2] Add ICSSG firmware stats related to HSR MD Danish Anwar
2026-05-14  7:56 ` [PATCH net-next v2 1/2] net: ti: icssg: Add static_assert to guard stat array counts MD Danish Anwar
2026-05-14  7:56 ` [PATCH net-next v2 2/2] net: ti: icssg: Add HSR and LRE PA statistics MD Danish Anwar
2026-05-19  1:45   ` Jakub Kicinski
2026-05-19  5:55     ` Luka Gejak [this message]
2026-05-19 23:56       ` Jakub Kicinski
2026-05-19  9:29   ` Paolo Abeni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E30AAC96-01D2-4A23-B562-126087DEB7FA@linux.dev \
    --to=luka.gejak@linux.dev \
    --cc=andrew+netdev@lunn.ch \
    --cc=corbet@lwn.net \
    --cc=danishanwar@ti.com \
    --cc=davem@davemloft.net \
    --cc=devnexen@gmail.com \
    --cc=edumazet@google.com \
    --cc=fmaurer@redhat.com \
    --cc=haokexin@gmail.com \
    --cc=horms@kernel.org \
    --cc=jacob.e.keller@intel.com \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m-malladi@ti.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=rogerq@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=vadim.fedorenko@linux.dev \
    --cc=vladimir.oltean@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox