* [PATCH net] net: ethernet: ti: icssg: guard PA stat lookups
@ 2026-06-16 14:35 Philippe Schenker
2026-06-18 9:10 ` Simon Horman
0 siblings, 1 reply; 3+ messages in thread
From: Philippe Schenker @ 2026-06-16 14:35 UTC (permalink / raw)
To: netdev
Cc: Philippe Schenker, danishanwar, rogerq, linux-arm-kernel, stable,
Andrew Lunn, David Carlier, David S. Miller, Eric Dumazet,
Jacob Keller, Jakub Kicinski, Kevin Hao, Meghana Malladi,
Paolo Abeni, Simon Horman, Vadim Fedorenko, linux-kernel
From: Philippe Schenker <philippe.schenker@impulsing.ch>
icssg_ndo_get_stats64() unconditionally calls emac_get_stat_by_name()
with FW PA stat names regardless of whether the PA stats block is
present on the hardware. emac_get_stat_by_name() already guards the
PA stats lookup with `if (emac->prueth->pa_stats)`; when that pointer
is NULL the lookup falls through to netdev_err() and returns -EINVAL.
Because ndo_get_stats64 is polled regularly by the networking stack
this produces thousands of log entries of the form:
icssg-prueth icssg1-eth end0: Invalid stats FW_RX_ERROR
A secondary consequence is that the int(-EINVAL) return value is
implicitly widened to a near-ULLONG_MAX unsigned value when accumulated
into the __u64 fields of rtnl_link_stats64, silently corrupting the
rx_errors, rx_dropped and tx_dropped counters reported by `ip -s link`.
Every other PA-aware code path in the driver is already guarded with
the same `if (emac->prueth->pa_stats)` check. Apply the same guard
here.
Fixes: 0d15a26b247d ("net: ti: icssg-prueth: Add ICSSG FW Stats")
Signed-off-by: Philippe Schenker <philippe.schenker@impulsing.ch>
Cc: danishanwar@ti.com
Cc: rogerq@kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: stable@vger.kernel.org
---
drivers/net/ethernet/ti/icssg/icssg_common.c | 48 +++++++++++---------
1 file changed, 27 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/ti/icssg/icssg_common.c b/drivers/net/ethernet/ti/icssg/icssg_common.c
index a28a608f9bf4..2dbb8f717de0 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_common.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_common.c
@@ -1628,28 +1628,34 @@ void icssg_ndo_get_stats64(struct net_device *ndev,
stats->rx_over_errors = emac_get_stat_by_name(emac, "rx_over_errors");
stats->multicast = emac_get_stat_by_name(emac, "rx_multicast_frames");
- stats->rx_errors = ndev->stats.rx_errors +
- emac_get_stat_by_name(emac, "FW_RX_ERROR") +
- 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");
- 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") +
- emac_get_stat_by_name(emac, "FW_INF_SAV") +
- emac_get_stat_by_name(emac, "FW_INF_SA_DL") +
- emac_get_stat_by_name(emac, "FW_INF_PORT_BLOCKED") +
- 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");
+ stats->rx_errors = ndev->stats.rx_errors;
+ stats->rx_dropped = ndev->stats.rx_dropped;
stats->tx_errors = ndev->stats.tx_errors;
- stats->tx_dropped = ndev->stats.tx_dropped +
- emac_get_stat_by_name(emac, "FW_RTU_PKT_DROP") +
- emac_get_stat_by_name(emac, "FW_TX_DROPPED_PACKET") +
- emac_get_stat_by_name(emac, "FW_TX_TS_DROPPED_PACKET") +
- emac_get_stat_by_name(emac, "FW_TX_JUMBO_FRM_CUTOFF");
+ stats->tx_dropped = ndev->stats.tx_dropped;
+
+ if (emac->prueth->pa_stats) {
+ stats->rx_errors +=
+ emac_get_stat_by_name(emac, "FW_RX_ERROR") +
+ 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");
+ stats->rx_dropped +=
+ emac_get_stat_by_name(emac, "FW_DROPPED_PKT") +
+ emac_get_stat_by_name(emac, "FW_INF_PORT_DISABLED") +
+ emac_get_stat_by_name(emac, "FW_INF_SAV") +
+ emac_get_stat_by_name(emac, "FW_INF_SA_DL") +
+ emac_get_stat_by_name(emac, "FW_INF_PORT_BLOCKED") +
+ 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");
+ stats->tx_dropped +=
+ emac_get_stat_by_name(emac, "FW_RTU_PKT_DROP") +
+ emac_get_stat_by_name(emac, "FW_TX_DROPPED_PACKET") +
+ emac_get_stat_by_name(emac, "FW_TX_TS_DROPPED_PACKET") +
+ emac_get_stat_by_name(emac, "FW_TX_JUMBO_FRM_CUTOFF");
+ }
}
EXPORT_SYMBOL_GPL(icssg_ndo_get_stats64);
--
2.54.0
base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
branch: fix-icssg_common-pa-stats-errors__master-7-1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] net: ethernet: ti: icssg: guard PA stat lookups
2026-06-16 14:35 [PATCH net] net: ethernet: ti: icssg: guard PA stat lookups Philippe Schenker
@ 2026-06-18 9:10 ` Simon Horman
2026-06-18 9:29 ` Philippe Schenker
0 siblings, 1 reply; 3+ messages in thread
From: Simon Horman @ 2026-06-18 9:10 UTC (permalink / raw)
To: Philippe Schenker
Cc: netdev, Philippe Schenker, danishanwar, rogerq, linux-arm-kernel,
stable, Andrew Lunn, David Carlier, David S. Miller, Eric Dumazet,
Jacob Keller, Jakub Kicinski, Kevin Hao, Meghana Malladi,
Paolo Abeni, Vadim Fedorenko, linux-kernel
On Tue, Jun 16, 2026 at 04:35:34PM +0200, Philippe Schenker wrote:
> From: Philippe Schenker <philippe.schenker@impulsing.ch>
>
> icssg_ndo_get_stats64() unconditionally calls emac_get_stat_by_name()
> with FW PA stat names regardless of whether the PA stats block is
> present on the hardware. emac_get_stat_by_name() already guards the
> PA stats lookup with `if (emac->prueth->pa_stats)`; when that pointer
> is NULL the lookup falls through to netdev_err() and returns -EINVAL.
> Because ndo_get_stats64 is polled regularly by the networking stack
> this produces thousands of log entries of the form:
>
> icssg-prueth icssg1-eth end0: Invalid stats FW_RX_ERROR
>
> A secondary consequence is that the int(-EINVAL) return value is
> implicitly widened to a near-ULLONG_MAX unsigned value when accumulated
> into the __u64 fields of rtnl_link_stats64, silently corrupting the
> rx_errors, rx_dropped and tx_dropped counters reported by `ip -s link`.
>
> Every other PA-aware code path in the driver is already guarded with
> the same `if (emac->prueth->pa_stats)` check. Apply the same guard
> here.
>
> Fixes: 0d15a26b247d ("net: ti: icssg-prueth: Add ICSSG FW Stats")
nit: no blank line between tags
>
> Signed-off-by: Philippe Schenker <philippe.schenker@impulsing.ch>
>
> Cc: danishanwar@ti.com
> Cc: rogerq@kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: stable@vger.kernel.org
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] net: ethernet: ti: icssg: guard PA stat lookups
2026-06-18 9:10 ` Simon Horman
@ 2026-06-18 9:29 ` Philippe Schenker
0 siblings, 0 replies; 3+ messages in thread
From: Philippe Schenker @ 2026-06-18 9:29 UTC (permalink / raw)
To: Simon Horman
Cc: netdev, danishanwar, rogerq, linux-arm-kernel, stable,
Andrew Lunn, David Carlier, David S. Miller, Eric Dumazet,
Jacob Keller, Jakub Kicinski, Kevin Hao, Meghana Malladi,
Paolo Abeni, Vadim Fedorenko, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1794 bytes --]
Hi Simon
Thanks for the review and I'll send a v2 with that blank line removed.
Saw it right after sending the patch.
Philippe
On Thu, 2026-06-18 at 10:10 +0100, Simon Horman wrote:
> On Tue, Jun 16, 2026 at 04:35:34PM +0200, Philippe Schenker wrote:
> > From: Philippe Schenker <philippe.schenker@impulsing.ch>
> >
> > icssg_ndo_get_stats64() unconditionally calls
> > emac_get_stat_by_name()
> > with FW PA stat names regardless of whether the PA stats block is
> > present on the hardware. emac_get_stat_by_name() already guards
> > the
> > PA stats lookup with `if (emac->prueth->pa_stats)`; when that
> > pointer
> > is NULL the lookup falls through to netdev_err() and returns -
> > EINVAL.
> > Because ndo_get_stats64 is polled regularly by the networking stack
> > this produces thousands of log entries of the form:
> >
> > icssg-prueth icssg1-eth end0: Invalid stats FW_RX_ERROR
> >
> > A secondary consequence is that the int(-EINVAL) return value is
> > implicitly widened to a near-ULLONG_MAX unsigned value when
> > accumulated
> > into the __u64 fields of rtnl_link_stats64, silently corrupting the
> > rx_errors, rx_dropped and tx_dropped counters reported by `ip -s
> > link`.
> >
> > Every other PA-aware code path in the driver is already guarded
> > with
> > the same `if (emac->prueth->pa_stats)` check. Apply the same guard
> > here.
> >
> > Fixes: 0d15a26b247d ("net: ti: icssg-prueth: Add ICSSG FW Stats")
>
> nit: no blank line between tags
>
> >
> > Signed-off-by: Philippe Schenker <philippe.schenker@impulsing.ch>
> >
> > Cc: danishanwar@ti.com
> > Cc: rogerq@kernel.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: stable@vger.kernel.org
>
> Reviewed-by: Simon Horman <horms@kernel.org>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-18 9:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-16 14:35 [PATCH net] net: ethernet: ti: icssg: guard PA stat lookups Philippe Schenker
2026-06-18 9:10 ` Simon Horman
2026-06-18 9:29 ` Philippe Schenker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox