Fix per-queue statistics for devices with more than one queue.
The output data pointer is currently reset in each loop iteration,
effectively summing all queue statistics in the first four u64 values.
The summary values are not even labeled correctly. For example, if eth0 has
2 queues, ethtool -S eth0 shows:
q0_tx_pkt_n: 374 (actually tx_pkt_n over all queues)
q0_tx_irq_n: 23 (actually tx_normal_irq_n over all queues)
q1_tx_pkt_n: 462 (actually rx_pkt_n over all queues)
q1_tx_irq_n: 446 (actually rx_normal_irq_n over all queues)
q0_rx_pkt_n: 0
q0_rx_irq_n: 0
q1_rx_pkt_n: 0
q1_rx_irq_n: 0
Fixes: 133466c3bbe1 ("net: stmmac: use per-queue 64 bit statistics where necessary")
Cc: [email protected]
Signed-off-by: Petr Tesarik <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index f628411ae4ae..112a36a698f1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -543,15 +543,12 @@ static void stmmac_get_per_qstats(struct stmmac_priv *priv, u64 *data)
u32 rx_cnt = priv->plat->rx_queues_to_use;
unsigned int start;
int q, stat;
- u64 *pos;
char *p;
- pos = data;
for (q = 0; q < tx_cnt; q++) {
struct stmmac_txq_stats *txq_stats = &priv->xstats.txq_stats[q];
struct stmmac_txq_stats snapshot;
- data = pos;
do {
start = u64_stats_fetch_begin(&txq_stats->syncp);
snapshot = *txq_stats;
@@ -559,17 +556,15 @@ static void stmmac_get_per_qstats(struct stmmac_priv *priv, u64 *data)
p = (char *)&snapshot + offsetof(struct stmmac_txq_stats, tx_pkt_n);
for (stat = 0; stat < STMMAC_TXQ_STATS; stat++) {
- *data++ += (*(u64 *)p);
+ *data++ = (*(u64 *)p);
p += sizeof(u64);
}
}
- pos = data;
for (q = 0; q < rx_cnt; q++) {
struct stmmac_rxq_stats *rxq_stats = &priv->xstats.rxq_stats[q];
struct stmmac_rxq_stats snapshot;
- data = pos;
do {
start = u64_stats_fetch_begin(&rxq_stats->syncp);
snapshot = *rxq_stats;
@@ -577,7 +572,7 @@ static void stmmac_get_per_qstats(struct stmmac_priv *priv, u64 *data)
p = (char *)&snapshot + offsetof(struct stmmac_rxq_stats, rx_pkt_n);
for (stat = 0; stat < STMMAC_RXQ_STATS; stat++) {
- *data++ += (*(u64 *)p);
+ *data++ = (*(u64 *)p);
p += sizeof(u64);
}
}
--
2.43.0
On Fri, Jan 05, 2024 at 09:16:42PM +0100, Petr Tesarik wrote:
> Fix per-queue statistics for devices with more than one queue.
>
> The output data pointer is currently reset in each loop iteration,
> effectively summing all queue statistics in the first four u64 values.
>
> The summary values are not even labeled correctly. For example, if eth0 has
> 2 queues, ethtool -S eth0 shows:
>
> q0_tx_pkt_n: 374 (actually tx_pkt_n over all queues)
> q0_tx_irq_n: 23 (actually tx_normal_irq_n over all queues)
> q1_tx_pkt_n: 462 (actually rx_pkt_n over all queues)
> q1_tx_irq_n: 446 (actually rx_normal_irq_n over all queues)
> q0_rx_pkt_n: 0
> q0_rx_irq_n: 0
> q1_rx_pkt_n: 0
> q1_rx_irq_n: 0
>
> Fixes: 133466c3bbe1 ("net: stmmac: use per-queue 64 bit statistics where necessary")
> Cc: [email protected]
> Signed-off-by: Petr Tesarik <[email protected]>
Good catch! I mixed this with the statics sum up for
stmmac_qstats_string[].
Reviewed-by: Jisheng Zhang <[email protected]>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> index f628411ae4ae..112a36a698f1 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> @@ -543,15 +543,12 @@ static void stmmac_get_per_qstats(struct stmmac_priv *priv, u64 *data)
> u32 rx_cnt = priv->plat->rx_queues_to_use;
> unsigned int start;
> int q, stat;
> - u64 *pos;
> char *p;
>
> - pos = data;
> for (q = 0; q < tx_cnt; q++) {
> struct stmmac_txq_stats *txq_stats = &priv->xstats.txq_stats[q];
> struct stmmac_txq_stats snapshot;
>
> - data = pos;
> do {
> start = u64_stats_fetch_begin(&txq_stats->syncp);
> snapshot = *txq_stats;
> @@ -559,17 +556,15 @@ static void stmmac_get_per_qstats(struct stmmac_priv *priv, u64 *data)
>
> p = (char *)&snapshot + offsetof(struct stmmac_txq_stats, tx_pkt_n);
> for (stat = 0; stat < STMMAC_TXQ_STATS; stat++) {
> - *data++ += (*(u64 *)p);
> + *data++ = (*(u64 *)p);
> p += sizeof(u64);
> }
> }
>
> - pos = data;
> for (q = 0; q < rx_cnt; q++) {
> struct stmmac_rxq_stats *rxq_stats = &priv->xstats.rxq_stats[q];
> struct stmmac_rxq_stats snapshot;
>
> - data = pos;
> do {
> start = u64_stats_fetch_begin(&rxq_stats->syncp);
> snapshot = *rxq_stats;
> @@ -577,7 +572,7 @@ static void stmmac_get_per_qstats(struct stmmac_priv *priv, u64 *data)
>
> p = (char *)&snapshot + offsetof(struct stmmac_rxq_stats, rx_pkt_n);
> for (stat = 0; stat < STMMAC_RXQ_STATS; stat++) {
> - *data++ += (*(u64 *)p);
> + *data++ = (*(u64 *)p);
> p += sizeof(u64);
> }
> }
> --
> 2.43.0
>
On Fri, Jan 05, 2024 at 09:16:42PM +0100, Petr Tesarik wrote:
> Fix per-queue statistics for devices with more than one queue.
>
> The output data pointer is currently reset in each loop iteration,
> effectively summing all queue statistics in the first four u64 values.
>
> The summary values are not even labeled correctly. For example, if eth0 has
> 2 queues, ethtool -S eth0 shows:
>
> q0_tx_pkt_n: 374 (actually tx_pkt_n over all queues)
> q0_tx_irq_n: 23 (actually tx_normal_irq_n over all queues)
> q1_tx_pkt_n: 462 (actually rx_pkt_n over all queues)
> q1_tx_irq_n: 446 (actually rx_normal_irq_n over all queues)
> q0_rx_pkt_n: 0
> q0_rx_irq_n: 0
> q1_rx_pkt_n: 0
> q1_rx_irq_n: 0
>
> Fixes: 133466c3bbe1 ("net: stmmac: use per-queue 64 bit statistics where necessary")
> Cc: [email protected]
> Signed-off-by: Petr Tesarik <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
Hello:
This patch was applied to netdev/net.git (main)
by David S. Miller <[email protected]>:
On Fri, 5 Jan 2024 21:16:42 +0100 you wrote:
> Fix per-queue statistics for devices with more than one queue.
>
> The output data pointer is currently reset in each loop iteration,
> effectively summing all queue statistics in the first four u64 values.
>
> The summary values are not even labeled correctly. For example, if eth0 has
> 2 queues, ethtool -S eth0 shows:
>
> [...]
Here is the summary with links:
- [net] net: stmmac: fix ethtool per-queue statistics
https://git.kernel.org/netdev/net/c/61921bdaa132
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html