[PATCH net-next 0/2] Fixes for taprio xstats

[PATCH net-next 0/2] Fixes for taprio xstats

[Vladimir Oltean]

1. Taprio classes correspond to TXQs, and thus, class stats are TXQ
   stats not TC stats.
2. Drivers reporting taprio xstats should report xstats for *this*
   taprio, not for a previous one.

Vladimir Oltean (2):
  net/sched: taprio: report class offload stats per TXQ, not per TC
  net: enetc: reset taprio stats when taprio is deleted

 .../net/ethernet/freescale/enetc/enetc_qos.c  | 29 ++++++++++---------
 include/net/pkt_sched.h                       | 10 +++----
 net/sched/sch_taprio.c                        |  8 ++---
 3 files changed, 25 insertions(+), 22 deletions(-)

-- 
2.34.1

[PATCH net-next 1/2] net/sched: taprio: report class offload stats per TXQ, not per TC

[Vladimir Oltean]

The taprio Qdisc creates child classes per netdev TX queue, but
taprio_dump_class_stats() currently reports offload statistics per
traffic class. Traffic classes are groups of TXQs sharing the same
dequeue priority, so this is incorrect and we shouldn't be bundling up
the TXQ stats when reporting them, as we currently do in enetc.

Modify the API from taprio to drivers such that they report TXQ offload
stats and not TC offload stats.

There is no change in the UAPI or in the global Qdisc stats.

Fixes: 6c1adb650c8d ("net/sched: taprio: add netlink reporting for offload statistics counters")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 .../net/ethernet/freescale/enetc/enetc_qos.c  | 20 +++++++------------
 include/net/pkt_sched.h                       | 10 +++++-----
 net/sched/sch_taprio.c                        |  8 ++++----
 3 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
index 71157eba1fbe..58cdd67bb573 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
@@ -160,20 +160,14 @@ static void enetc_taprio_stats(struct net_device *ndev,
 	stats->window_drops = window_drops;
 }
 
-static void enetc_taprio_tc_stats(struct net_device *ndev,
-				  struct tc_taprio_qopt_tc_stats *tc_stats)
+static void enetc_taprio_queue_stats(struct net_device *ndev,
+				     struct tc_taprio_qopt_queue_stats *queue_stats)
 {
-	struct tc_taprio_qopt_stats *stats = &tc_stats->stats;
+	struct tc_taprio_qopt_stats *stats = &queue_stats->stats;
 	struct enetc_ndev_priv *priv = netdev_priv(ndev);
-	int tc = tc_stats->tc;
-	u64 window_drops = 0;
-	int i;
+	int queue = queue_stats->queue;
 
-	for (i = 0; i < priv->num_tx_rings; i++)
-		if (priv->tx_ring[i]->prio == tc)
-			window_drops += priv->tx_ring[i]->stats.win_drop;
-
-	stats->window_drops = window_drops;
+	stats->window_drops = priv->tx_ring[queue]->stats.win_drop;
 }
 
 static int enetc_taprio_replace(struct net_device *ndev,
@@ -208,8 +202,8 @@ int enetc_setup_tc_taprio(struct net_device *ndev, void *type_data)
 	case TAPRIO_CMD_STATS:
 		enetc_taprio_stats(ndev, &offload->stats);
 		break;
-	case TAPRIO_CMD_TC_STATS:
-		enetc_taprio_tc_stats(ndev, &offload->tc_stats);
+	case TAPRIO_CMD_QUEUE_STATS:
+		enetc_taprio_queue_stats(ndev, &offload->queue_stats);
 		break;
 	default:
 		err = -EOPNOTSUPP;
diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index 518febb91c9f..e98aac9d5ad5 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -191,7 +191,7 @@ enum tc_taprio_qopt_cmd {
 	TAPRIO_CMD_REPLACE,
 	TAPRIO_CMD_DESTROY,
 	TAPRIO_CMD_STATS,
-	TAPRIO_CMD_TC_STATS,
+	TAPRIO_CMD_QUEUE_STATS,
 };
 
 /**
@@ -208,8 +208,8 @@ struct tc_taprio_qopt_stats {
 	u64 tx_overruns;
 };
 
-struct tc_taprio_qopt_tc_stats {
-	int tc;
+struct tc_taprio_qopt_queue_stats {
+	int queue;
 	struct tc_taprio_qopt_stats stats;
 };
 
@@ -227,8 +227,8 @@ struct tc_taprio_qopt_offload {
 	union {
 		/* TAPRIO_CMD_STATS */
 		struct tc_taprio_qopt_stats stats;
-		/* TAPRIO_CMD_TC_STATS */
-		struct tc_taprio_qopt_tc_stats tc_stats;
+		/* TAPRIO_CMD_QUEUE_STATS */
+		struct tc_taprio_qopt_queue_stats queue_stats;
 		/* TAPRIO_CMD_REPLACE */
 		struct {
 			struct tc_mqprio_qopt_offload mqprio;
diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 5076da103f63..20d2702f862e 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -2457,9 +2457,9 @@ static int taprio_dump_class_stats(struct Qdisc *sch, unsigned long cl,
 {
 	struct netdev_queue *dev_queue = taprio_queue_get(sch, cl);
 	struct tc_taprio_qopt_offload offload = {
-		.cmd = TAPRIO_CMD_TC_STATS,
-		.tc_stats = {
-			.tc = cl - 1,
+		.cmd = TAPRIO_CMD_QUEUE_STATS,
+		.queue_stats = {
+			.queue = cl - 1,
 		},
 	};
 	struct Qdisc *child;
@@ -2469,7 +2469,7 @@ static int taprio_dump_class_stats(struct Qdisc *sch, unsigned long cl,
 	    qdisc_qstats_copy(d, child) < 0)
 		return -1;
 
-	return taprio_dump_xstats(sch, d, &offload, &offload.tc_stats.stats);
+	return taprio_dump_xstats(sch, d, &offload, &offload.queue_stats.stats);
 }
 
 static void taprio_walk(struct Qdisc *sch, struct qdisc_walker *arg)
-- 
2.34.1

[PATCH net-next 2/2] net: enetc: reset taprio stats when taprio is deleted

[Vladimir Oltean]

Currently, the window_drop stats persist even if an incorrect Qdisc was
removed from the interface and a new one is installed. This is because
the enetc driver keeps the state, and that is persistent across multiple
Qdiscs.

To resolve the issue, clear all win_drop counters from all TX queues
when the currently active Qdisc is removed. These counters are zero
by default. The counters visible in ethtool -S are also affected,
but I don't care very much about preserving those enough to keep them
monotonically incrementing.

Fixes: 4802fca8d1af ("net: enetc: report statistics counters for taprio")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc_qos.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
index 58cdd67bb573..9d74104df7c8 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
@@ -127,6 +127,14 @@ static int enetc_setup_taprio(struct enetc_ndev_priv *priv,
 	return 0;
 }
 
+static void enetc_reset_taprio_stats(struct enetc_ndev_priv *priv)
+{
+	int i;
+
+	for (i = 0; i < priv->num_tx_rings; i++)
+		priv->tx_ring[i]->stats.win_drop = 0;
+}
+
 static void enetc_reset_taprio(struct enetc_ndev_priv *priv)
 {
 	struct enetc_hw *hw = &priv->si->hw;
@@ -145,6 +153,7 @@ static void enetc_taprio_destroy(struct net_device *ndev)
 
 	enetc_reset_taprio(priv);
 	enetc_reset_tc_mqprio(ndev);
+	enetc_reset_taprio_stats(priv);
 }
 
 static void enetc_taprio_stats(struct net_device *ndev,
-- 
2.34.1

Re: [PATCH net-next 0/2] Fixes for taprio xstats

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri,  9 Jun 2023 16:59:15 +0300 you wrote:
> 1. Taprio classes correspond to TXQs, and thus, class stats are TXQ
>    stats not TC stats.
> 2. Drivers reporting taprio xstats should report xstats for *this*
>    taprio, not for a previous one.
> 
> Vladimir Oltean (2):
>   net/sched: taprio: report class offload stats per TXQ, not per TC
>   net: enetc: reset taprio stats when taprio is deleted
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] net/sched: taprio: report class offload stats per TXQ, not per TC
    https://git.kernel.org/netdev/net-next/c/2b84960fc5dd
  - [net-next,2/2] net: enetc: reset taprio stats when taprio is deleted
    https://git.kernel.org/netdev/net-next/c/f1e668d29c57

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html