[PATCH net-next v10 0/4] net/sched: sch_fq_pie: add per-flow class statistics

[PATCH net-next v10 0/4] net/sched: sch_fq_pie: add per-flow class statistics

[Hemendra M. Naik]

FQ-PIE runs an independent PIE controller per flow but exposes no
per-flow statistics. This series wires up fq_pie_class_ops to expose
per-flow AQM state (prob, delay, deficit, avg_dq_rate) via
'tc -s class show', following a similar pattern as FQ-CoDel.

This patch series is accompanied by a companion iproute2 patch series that
will be posted on net-next as well.

Both the kernel and iproute2 patches can also be found in GitHub:
- https://github.com/H-N41K/linux-net-next/tree/fq_pie_class_stats/
- https://github.com/H-N41K/iproute2-next/tree/fq_pie_class_stats

---
Changelog:

v10: 
- Update Documentation/netlink/specs/tc.yaml for fq_pie

v9: https://lore.kernel.org/netdev/20260928043358.6866-1-hemendranaik@gmail.com/
- Add a blank line between structs in commit 1/3.

v8: https://lore.kernel.org/netdev/20260922205924.32173-1-hemendranaik@gmail.com/
- No code changes since v7.
- Reword commit message to reflect usage of div_u64().

v7: https://lore.kernel.org/netdev/20260919060125.38063-1-hemendranaik@gmail.com/
- No code changes since v6.
- Rebased over the latest net-next tree and re-submitted patch.

v6: https://lore.kernel.org/netdev/20260917204225.275251-1-hemendranaik@gmail.com/
- Address Sashiko review comments:
  - Revert the fq_pie flows range to [1..65536]. 
  - Also drop the accompanying selftest 83be change.
  - Compute the per-flow delay with div_u64() on the full 64-bit
    nanosecond value.
  - Widen avg_dq_rate to u64 before scaling it by PSCHED_TICKS_PER_SEC.
  - Document the 36 to 64 byte xstats growth in the patch 1 commit message.
  - Fix selftest 83c0. The ping payload exceeded the TBF burst so no
    packet ever reached fq_pie, and the missing -W left the run waiting
    out ping's default timeout.

v5: https://lore.kernel.org/netdev/20260902035231.81866-1-hemendranaik@gmail.com/
- Addressed Sashiko review comments:
 - Omitted .tcf_block / .bind_tcf / .unbind_tcf from cl_ops (statistics
   only being exported; filter attach to fq_pie is now disabled).
 - Dropped empty tc_fq_pie_xqd_stats placeholder; class_stats is a direct
   struct member.
 - Limited flows to [1..65535] so per-flow class handles fit in a 16-bit
   TC minor.
 - Rewrote selftest 83c0 with TBF + fq_pie, ping traffic, and
   matchCount 1 on per-flow stats output.
 - Updated selftest 83be for the new flows limit.
 - Dropped the tools/include UAPI mirror changes from patch 3/3.

v4: https://lore.kernel.org/netdev/20260727164056.106203-1-hemendranaik@gmail.com/
- Fixed unaligned commit message; moved typo fixes to another patch.

v3: https://lore.kernel.org/netdev/20260630183702.170798-1-hemendranaik@gmail.com/
- No changes since v2.
- Resent after the previous submission was deferred due to the
  net-next tree closing during review.
- Updated corresponding iproute2 patch in response to review comments;
  kernel patches unchanged.

v2: https://lore.kernel.org/netdev/20260614125000.6058-1-hemendranaik@gmail.com/
- Addressed ABI backward compatibility issue for tc_fq_pie_xstats.

v1: https://lore.kernel.org/netdev/20260531125314.22492-1-hemendranaik@gmail.com/
- Initial submission

Hemendra M. Naik (4):
  net/sched: sch_fq_pie: add per-flow statistics via class ops
  selftests: tc-testing: add fq_pie per-flow class stats test
  net/sched: pie: correct tc_pie_xstats field documentation
  Documentation/netlink/specs: tc: describe fq_pie per-flow class stats

 Documentation/netlink/specs/tc.yaml           | 32 +++++++
 include/uapi/linux/pkt_sched.h                | 21 ++++-
 net/sched/sch_fq_pie.c                        | 88 ++++++++++++++++++-
 .../tc-testing/tc-tests/qdiscs/fq_pie.json    | 27 ++++++
 4 files changed, 165 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH net-next v10 1/4] net/sched: sch_fq_pie: add per-flow statistics via class ops

[Hemendra M. Naik]

FQ-PIE schedules independent PIE controllers per flow but exposes no
per-flow AQM state. Without class-level statistics there is no way to
observe the per-flow drop probability, queue delay, deficit or dequeue
rate from userspace.

Extend tc_fq_pie_xstats with a type discriminator and tc_fq_pie_cl_stats
for per-flow metrics. The nine existing __u32 counters keep their
offsets, type is appended at offset 36 and class_stats follows it,
growing the structure from 36 to 64 bytes.

Wire up fq_pie_class_ops with walk, dump, and dump_stats only so that
'tc -s class show' reports per-flow state (prob, delay, deficit,
avg_dq_rate, dq_rate_estimating). Filter attachment callbacks
(.tcf_block, .bind_tcf, .unbind_tcf) are omitted on purpose.

Signed-off-by: Hemendra M. Naik <hemendranaik@gmail.com>
Signed-off-by: Vishal Kamath <vishy0777@gmail.com>
Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
---
 include/uapi/linux/pkt_sched.h | 17 +++++++
 net/sched/sch_fq_pie.c         | 88 +++++++++++++++++++++++++++++++++-
 2 files changed, 104 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index 490efd288526..fef848eaaab2 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -953,6 +953,21 @@ enum {
 };
 #define TCA_FQ_PIE_MAX   (__TCA_FQ_PIE_MAX - 1)
 
+enum {
+	TCA_FQ_PIE_XSTATS_QDISC,
+	TCA_FQ_PIE_XSTATS_CLASS,
+};
+
+struct tc_fq_pie_cl_stats {
+	__u64 prob;			/* current probability */
+	__u32 delay;			/* current delay in microseconds */
+	__s32 deficit;			/* number of remaining byte credits */
+	__u32 avg_dq_rate;		/* current average dq_rate in
+					 * bytes/second
+					 */
+	__u32 dq_rate_estimating;	/* is avg_dq_rate being calculated? */
+};
+
 struct tc_fq_pie_xstats {
 	__u32 packets_in;	/* total number of packets enqueued */
 	__u32 dropped;		/* packets dropped due to fq_pie_action */
@@ -963,6 +978,8 @@ struct tc_fq_pie_xstats {
 	__u32 new_flows_len;	/* count of flows in new list */
 	__u32 old_flows_len;	/* count of flows in old list */
 	__u32 memory_usage;	/* total memory across all queues */
+	__u32 type;
+	struct tc_fq_pie_cl_stats class_stats;
 };
 
 /* CBS */
diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c
index 5982847df8f8..c834c8f45898 100644
--- a/net/sched/sch_fq_pie.c
+++ b/net/sched/sch_fq_pie.c
@@ -511,7 +511,11 @@ static int fq_pie_dump(struct Qdisc *sch, struct sk_buff *skb)
 static int fq_pie_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
 {
 	struct fq_pie_sched_data *q = qdisc_priv(sch);
-	struct tc_fq_pie_xstats st = { 0 };
+
+	struct tc_fq_pie_xstats st = {
+		.type	= TCA_FQ_PIE_XSTATS_QDISC,
+	};
+
 	struct list_head *pos;
 
 	sch_tree_lock(sch);
@@ -563,7 +567,89 @@ static void fq_pie_destroy(struct Qdisc *sch)
 	kvfree(q->flows);
 }
 
+static struct Qdisc *fq_pie_leaf(struct Qdisc *sch, unsigned long arg)
+{
+	return NULL;
+}
+
+static unsigned long fq_pie_find(struct Qdisc *sch, u32 classid)
+{
+	return 0;
+}
+
+static int fq_pie_dump_class(struct Qdisc *sch, unsigned long cl,
+			     struct sk_buff *skb, struct tcmsg *tcm)
+{
+	tcm->tcm_handle |= TC_H_MIN(cl);
+	return 0;
+}
+
+static int fq_pie_dump_class_stats(struct Qdisc *sch, unsigned long cl,
+				   struct gnet_dump *d)
+{
+	struct fq_pie_sched_data *q = qdisc_priv(sch);
+	struct gnet_stats_queue qs = { 0 };
+	struct tc_fq_pie_xstats xstats;
+	u32 idx = cl - 1;
+
+	if (idx < q->flows_cnt) {
+		const struct fq_pie_flow *flow = &q->flows[idx];
+
+		memset(&xstats, 0, sizeof(xstats));
+		xstats.type = TCA_FQ_PIE_XSTATS_CLASS;
+		xstats.class_stats.prob =
+			READ_ONCE(flow->vars.prob) << BITS_PER_BYTE;
+		xstats.class_stats.delay =
+			div_u64(PSCHED_TICKS2NS(READ_ONCE(flow->vars.qdelay)),
+				NSEC_PER_USEC);
+		xstats.class_stats.deficit = READ_ONCE(flow->deficit);
+		xstats.class_stats.dq_rate_estimating =
+			READ_ONCE(q->p_params.dq_rate_estimator);
+
+		if (xstats.class_stats.dq_rate_estimating) {
+			xstats.class_stats.avg_dq_rate =
+				((u64)READ_ONCE(flow->vars.avg_dq_rate)
+				* PSCHED_TICKS_PER_SEC) >> PIE_SCALE;
+		}
+
+		qs.qlen    = READ_ONCE(flow->qlen);
+		qs.backlog = READ_ONCE(flow->backlog);
+	}
+	if (gnet_stats_copy_queue(d, NULL, &qs, qs.qlen) < 0)
+		return -1;
+	if (idx < q->flows_cnt)
+		return gnet_stats_copy_app(d, &xstats, sizeof(xstats));
+	return 0;
+}
+
+static void fq_pie_walk(struct Qdisc *sch, struct qdisc_walker *arg)
+{
+	struct fq_pie_sched_data *q = qdisc_priv(sch);
+	unsigned int i;
+
+	if (arg->stop)
+		return;
+
+	for (i = 0; i < q->flows_cnt; i++) {
+		if (list_empty(&q->flows[i].flowchain)) {
+			arg->count++;
+			continue;
+		}
+		if (!tc_qdisc_stats_dump(sch, i + 1, arg))
+			break;
+	}
+}
+
+static const struct Qdisc_class_ops fq_pie_class_ops = {
+	.leaf		=	fq_pie_leaf,
+	.find		=	fq_pie_find,
+	.dump		=	fq_pie_dump_class,
+	.dump_stats	=	fq_pie_dump_class_stats,
+	.walk		=	fq_pie_walk,
+};
+
 static struct Qdisc_ops fq_pie_qdisc_ops __read_mostly = {
+	.cl_ops		= &fq_pie_class_ops,
 	.id		= "fq_pie",
 	.priv_size	= sizeof(struct fq_pie_sched_data),
 	.enqueue	= fq_pie_qdisc_enqueue,
-- 
2.34.1

[PATCH net-next v10 2/4] selftests: tc-testing: add fq_pie per-flow class stats test

[Hemendra M. Naik]

Add tc-testing case 83c0: build a TBF + fq_pie hierarchy on $DUMMY, push
traffic through it with ping, and verify that 'tc -s class show' reports
an fq_pie class.

The per-flow counters themselves (prob, delay, deficit) are printed with
the companion iproute2 change, "tc: fq_pie: add support for printing
per-flow PIE statistics"

Signed-off-by: Hemendra M. Naik <hemendranaik@gmail.com>
Signed-off-by: Vishal Kamath <vishy0777@gmail.com>
Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
---
 .../tc-testing/tc-tests/qdiscs/fq_pie.json    | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq_pie.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq_pie.json
index 229fe1bf4a90..15b2667285eb 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq_pie.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq_pie.json
@@ -40,5 +40,32 @@
         "matchPattern": "qdisc fq_pie 1: root refcnt [0-9]+ limit 1p",
         "matchCount": "1",
         "teardown": ["$TC qdisc del dev $DEV1 handle 1: root"]
+    },
+    {
+        "id": "83c0",
+        "name": "FQ-PIE class stats accessible via tc class show",
+        "category": [
+            "qdisc",
+            "fq_pie"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$IP link set dev $DUMMY up || true",
+            "$IP addr add 10.10.11.10/24 dev $DUMMY || true",
+            "$TC qdisc add dev $DUMMY root handle 1: tbf rate 8bit burst 100b latency 100ms",
+            "$TC qdisc add dev $DUMMY parent 1:1 handle 2: fq_pie limit 100 flows 1",
+            "ping -c 50 -i 0.001 -W 0.01 -s 56 10.10.11.11 -I $DUMMY > /dev/null 2>&1 || true"
+        ],
+        "cmdUnderTest": "$TC -s class show dev $DUMMY",
+        "expExitCode": "0",
+        "verifyCmd": "$TC -s class show dev $DUMMY",
+        "matchPattern": "class fq_pie 2:[0-9]+ .*prob .*delay .*deficit",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DUMMY handle 1: root",
+            "$IP addr del 10.10.11.10/24 dev $DUMMY || true"
+        ]
     }
 ]
-- 
2.34.1

[PATCH net-next v10 3/4] net/sched: pie: correct tc_pie_xstats field documentation

[Hemendra M. Naik]

The comments describing struct tc_pie_xstats do not match the values
exported by the kernel.

Update the delay field comment to microseconds (PSCHED_TICKS2NS() /
NSEC_PER_USEC). Update avg_dq_rate to bytes/second (avg_dq_rate *
PSCHED_TICKS_PER_SEC >> PIE_SCALE).

Documentation-only; no UAPI layout or runtime change. Touch
include/uapi/linux/pkt_sched.h only

Signed-off-by: Hemendra M. Naik <hemendranaik@gmail.com>
Signed-off-by: Vishal Kamath <vishy0777@gmail.com>
Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
---
 include/uapi/linux/pkt_sched.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index fef848eaaab2..445a041cd103 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -920,9 +920,9 @@ enum {
 
 struct tc_pie_xstats {
 	__u64 prob;			/* current probability */
-	__u32 delay;			/* current delay in ms */
+	__u32 delay;			/* current delay in microseconds */
 	__u32 avg_dq_rate;		/* current average dq_rate in
-					 * bits/pie_time
+					 * bytes/second
 					 */
 	__u32 dq_rate_estimating;	/* is avg_dq_rate being calculated? */
 	__u32 packets_in;		/* total number of packets enqueued */
-- 
2.34.1

[PATCH net-next v10 4/4] Documentation/netlink/specs: tc: describe fq_pie per-flow class stats

[Hemendra M. Naik]

Add the tc-fq-pie-cl-stats header for fq_pie per flow (class) stats in Documentation/netlink/specs/tc.yaml
with its five fields: prob, delay, deficit, avg-dq-rate, fq-rate-estimating.

Extend the tc-fq-pie-xstats fixed header with the type discriminator and the per-flow class_stats fields
that follow the nine legacy __u32 qdisc counters in struct tc_fq_pie_xstats.

Signed-off-by: Hemendra M. Naik <hemendranaik@gmail.com>
Signed-off-by: Vishal Kamath <vishy0777@gmail.com>
Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
---
 Documentation/netlink/specs/tc.yaml | 33 +++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/Documentation/netlink/specs/tc.yaml b/Documentation/netlink/specs/tc.yaml
index b6ccc645b0e3..b159c4b563c7 100644
--- a/Documentation/netlink/specs/tc.yaml
+++ b/Documentation/netlink/specs/tc.yaml
@@ -894,6 +894,30 @@ definitions:
         name: memory-limit
         type: u32
         doc: Memory limit in bytes
+  -
+    name: tc-fq-pie-cl-stats
+    type: struct
+    members:
+      -
+        name: prob
+        type: u64
+        doc: Current drop probability of the flow
+      -
+        name: delay
+        type: u32
+        doc: Current queue delay of the flow in microseconds
+      -
+        name: deficit
+        type: s32
+        doc: Number of remaining byte credits
+      -
+        name: avg-dq-rate
+        type: u32
+        doc: Current average dq_rate in bytes/second
+      -
+        name: dq-rate-estimating
+        type: u32
+        doc: Is avg-dq-rate being calculated?
   -
     name: tc-fq-pie-xstats
     type: struct
@@ -934,6 +958,15 @@ definitions:
         name: memory-usage
         type: u32
         doc: Total memory across all queues
+      -
+        name: type
+        type: u32
+        doc: TCA_FQ_PIE_XSTATS_QDISC (0) or TCA_FQ_PIE_XSTATS_CLASS (1)
+      -
+        name: class-stats
+        type: binary
+        struct: tc-fq-pie-cl-stats
+        doc: Class statistics of a particular flow (invoked via tc class show)
   -
     name: tc-fq-qd-stats
     type: struct
-- 
2.34.1

Re: [PATCH net-next v10 1/4] net/sched: sch_fq_pie: add per-flow statistics via class ops

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 3

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new uAPI field `tc_fq_pie_cl_stats.avg_dq_rate` is
  declared `__u32` and documented as "current average dq_rate in…
- [Medium] fq_pie_dump_class_stats() reads flow->deficit, flow->qlen and
  flow->backlog with READ_ONCE() and no qdisc lock: tc_fill_tclass()…
- [Low] The commit message says the patch makes 'tc -s class show' report
  per-flow prob, delay, deficit, avg_dq_rate and dq_rate_estimating.
- [Low] fq_pie_change() accepts flows_cnt values from 1 to 65536.
- [Low] Setting `.cl_ops` makes the tc core treat fq_pie as classful,
  which changes user-visible behaviour that the commit message does not…

--

net/sched: sch_fq_pie: add per-flow statistics via class ops

This patch adds a type discriminator and a tc_fq_pie_cl_stats member to
tc_fq_pie_xstats, which grows the struct from 36 to 64 bytes. It also wires
up fq_pie_class_ops so that each active flow is dumped as a class with its
PIE state.

> diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
> index 490efd2885267..fef848eaaab23 100644
> --- a/include/uapi/linux/pkt_sched.h
> +++ b/include/uapi/linux/pkt_sched.h
> @@ -953,6 +953,21 @@ enum {
>  };
>  #define TCA_FQ_PIE_MAX   (__TCA_FQ_PIE_MAX - 1)
>  
> +enum {
> +	TCA_FQ_PIE_XSTATS_QDISC,
> +	TCA_FQ_PIE_XSTATS_CLASS,
> +};
> +
> +struct tc_fq_pie_cl_stats {
> +	__u64 prob;			/* current probability */
> +	__u32 delay;			/* current delay in microseconds */
> +	__s32 deficit;			/* number of remaining byte credits */
> +	__u32 avg_dq_rate;		/* current average dq_rate in
> +					 * bytes/second
> +					 */

[Severity: Medium]
Can avg_dq_rate overflow? It is a __u32 documented in bytes/second, but
fq_pie_dump_class_stats() computes it as a u64:

    xstats.class_stats.avg_dq_rate =
        ((u64)READ_ONCE(flow->vars.avg_dq_rate)
        * PSCHED_TICKS_PER_SEC) >> PIE_SCALE;

The result is stored without saturation.

pie_process_dequeue() sets vars.avg_dq_rate to
(dq_count << PIE_SCALE) / dtime. That can represent rates above 2^32
bytes/s (about 4.29 GB/s, or roughly 34 Gbit/s). If dq_rate_estimator is
enabled and one flow drains faster than that, the reported value would
wrap.

This is a new uAPI field and can't be widened later. Would __u64 be a better
fit? tc_pie_xstats uses the same width, but the netlink guidelines prefer
wider types for new values.

> +	__u32 dq_rate_estimating;	/* is avg_dq_rate being calculated? */
> +};
> +
>  struct tc_fq_pie_xstats {
>  	__u32 packets_in;	/* total number of packets enqueued */
>  	__u32 dropped;		/* packets dropped due to fq_pie_action */
> @@ -963,6 +978,8 @@ struct tc_fq_pie_xstats {
>  	__u32 new_flows_len;	/* count of flows in new list */
>  	__u32 old_flows_len;	/* count of flows in old list */
>  	__u32 memory_usage;	/* total memory across all queues */
> +	__u32 type;
> +	struct tc_fq_pie_cl_stats class_stats;
>  };

[Severity: Low]
The commit message says:

  Wire up fq_pie_class_ops with walk, dump, and dump_stats only so that
  'tc -s class show' reports per-flow state (prob, delay, deficit,
  avg_dq_rate, dq_rate_estimating).

Since type is appended at offset 36, the first 36 bytes of a class record
look exactly like a qdisc record. fq_pie_dump_class_stats() zeroes the whole
struct and fills only type and class_stats. It then sends the result as
TCA_STATS_APP.

The current tc fq_pie xstats printer handles both qdisc and class dumps, and
it only knows the 36-byte layout. With an unmodified tc, would every
per-flow class line print "pkts_in 0 overlimit 0 ... memory_used 0" and
none of the per-flow values?

The iproute2 dependency is only mentioned in the later commit "selftests:
tc-testing: add fq_pie per-flow class stats test". Could this commit message
say that a companion iproute2 change is needed? It could also note that
older tools can't tell class records from qdisc records.

> diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c
> index 5982847df8f85..c834c8f45898b 100644
> --- a/net/sched/sch_fq_pie.c
> +++ b/net/sched/sch_fq_pie.c

[ ... ]

> @@ -563,7 +567,89 @@ static void fq_pie_destroy(struct Qdisc *sch)
>  	kvfree(q->flows);
>  }
>  

[ ... ]

> +static int fq_pie_dump_class(struct Qdisc *sch, unsigned long cl,
> +			     struct sk_buff *skb, struct tcmsg *tcm)
> +{
> +	tcm->tcm_handle |= TC_H_MIN(cl);
> +	return 0;
> +}

[Severity: Low]
What happens to the handle when flows_cnt is 65536? fq_pie_change()
accepts that value:

    if (!q->flows_cnt || q->flows_cnt > 65536) {

fq_pie_walk() calls tc_qdisc_stats_dump(sch, i + 1, arg), so cl can reach
65536. TC_H_MIN(65536) is 0, and tc_fill_tclass() has already set
tcm->tcm_handle = q->handle, whose minor is 0.

Wouldn't the class for flow index 65535 then be reported as maj:0, the same
handle as the qdisc itself?

fq_codel has the same pattern, but this patch brings it to fq_pie.

> +
> +static int fq_pie_dump_class_stats(struct Qdisc *sch, unsigned long cl,
> +				   struct gnet_dump *d)
> +{
> +	struct fq_pie_sched_data *q = qdisc_priv(sch);
> +	struct gnet_stats_queue qs = { 0 };
> +	struct tc_fq_pie_xstats xstats;
> +	u32 idx = cl - 1;
> +
> +	if (idx < q->flows_cnt) {
> +		const struct fq_pie_flow *flow = &q->flows[idx];
> +
> +		memset(&xstats, 0, sizeof(xstats));
> +		xstats.type = TCA_FQ_PIE_XSTATS_CLASS;
> +		xstats.class_stats.prob =
> +			READ_ONCE(flow->vars.prob) << BITS_PER_BYTE;
> +		xstats.class_stats.delay =
> +			div_u64(PSCHED_TICKS2NS(READ_ONCE(flow->vars.qdelay)),
> +				NSEC_PER_USEC);
> +		xstats.class_stats.deficit = READ_ONCE(flow->deficit);
> +		xstats.class_stats.dq_rate_estimating =
> +			READ_ONCE(q->p_params.dq_rate_estimator);
> +
> +		if (xstats.class_stats.dq_rate_estimating) {
> +			xstats.class_stats.avg_dq_rate =
> +				((u64)READ_ONCE(flow->vars.avg_dq_rate)
> +				* PSCHED_TICKS_PER_SEC) >> PIE_SCALE;
> +		}
> +
> +		qs.qlen    = READ_ONCE(flow->qlen);
> +		qs.backlog = READ_ONCE(flow->backlog);

[Severity: Medium]
Are the writers of deficit, qlen and backlog annotated to match these reads?
tc_fill_tclass() calls gnet_stats_start_copy_compat() with a NULL lock. That
means this function runs under RTNL only, without the qdisc lock.

The fast path still uses plain stores under the qdisc root lock. In
fq_pie_qdisc_enqueue():

    sel_flow->deficit = q->quantum;
    sel_flow->qlen = 0;
    sel_flow->backlog = 0;
    ...
    sel_flow->qlen++;
    sel_flow->backlog += pkt_len;

In fq_pie_qdisc_dequeue():

    flow->deficit += q->quantum;
    ...
    flow->qlen--;
    flow->deficit -= pkt_len;
    flow->backlog -= pkt_len;

With READ_ONCE() on the reader side only, won't KCSAN report these as data
races? qlen and backlog are also read as two separate lockless loads, so the
two values might not match each other.

The PIE vars fields read here (prob, qdelay, avg_dq_rate) do use
WRITE_ONCE() in sch_pie.c. fq_codel uses WRITE_ONCE() for flow->deficit and
q->backlogs[], and it takes sch_tree_lock() for its qlen snapshot. Should
fq_pie do the same?

> +	}

[ ... ]

> +static const struct Qdisc_class_ops fq_pie_class_ops = {
> +	.leaf		=	fq_pie_leaf,
> +	.find		=	fq_pie_find,
> +	.dump		=	fq_pie_dump_class,
> +	.dump_stats	=	fq_pie_dump_class_stats,
> +	.walk		=	fq_pie_walk,
> +};
> +
>  static struct Qdisc_ops fq_pie_qdisc_ops __read_mostly = {
> +	.cl_ops		= &fq_pie_class_ops,

[Severity: Low]
Setting .cl_ops makes the tc core treat fq_pie as classful. That changes
some user-visible behaviour the commit message doesn't mention. The message
also says only walk, dump and dump_stats are wired up, but the patch adds
.leaf and .find stubs as well.

Before this patch, adding a filter with an fq_pie parent failed in
__tcf_qdisc_find() with -EINVAL and "Qdisc not classful". Now it reaches:

    if (!cops->tcf_block) {
        NL_SET_ERR_MSG(extack, "Class doesn't support blocks");
        err = -EOPNOTSUPP;

Grafting a child qdisc under an fq_pie class used to fail in qdisc_leaf()
with -EOPNOTSUPP and "Parent qdisc is not classful". Now fq_pie_find()
returns 0, so it fails with -ENOENT and "Specified class not found".

Because fq_pie_find() always returns 0, a classid listed by 'tc class show'
also can't be fetched with RTM_GETTCLASS.

fq_codel uses the same pattern. Is this change in errno and extack intended,
and could the commit message mention it?

The commit message also describes dq_rate_estimating as per-flow state.
However, fq_pie_dump_class_stats() copies the qdisc-wide
q->p_params.dq_rate_estimator into every class record. Could that wording be
adjusted?

>  	.id		= "fq_pie",
>  	.priv_size	= sizeof(struct fq_pie_sched_data),
>  	.enqueue	= fq_pie_qdisc_enqueue,

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109071017.438478-1-hemendranaik%40gmail.com

Re: [PATCH net-next v10 2/4] selftests: tc-testing: add fq_pie per-flow class stats test

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Test 83c0 in tools/testing/selftests/tc-testing/tc-
  tests/qdiscs/fq_pie.json does not do what the commit message says, and…

--

selftests: tc-testing: add fq_pie per-flow class stats test

This adds tc-testing case 83c0. The test builds a TBF + fq_pie hierarchy
on $DUMMY and pushes ping traffic through it. It then checks that
'tc -s class show' reports an fq_pie class.

> Add tc-testing case 83c0: build a TBF + fq_pie hierarchy on $DUMMY, push
> traffic through it with ping, and verify that 'tc -s class show' reports
> an fq_pie class.
>
> The per-flow counters themselves (prob, delay, deficit) are printed with
> the companion iproute2 change, "tc: fq_pie: add support for printing
> per-flow PIE statistics"

> diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq_pie.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq_pie.json
> index 229fe1bf4a906..15b2667285eb4 100644
> --- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq_pie.json
> +++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/fq_pie.json
> @@ -40,5 +40,32 @@

[ ... ]

> +    },
> +    {
> +        "id": "83c0",
> +        "name": "FQ-PIE class stats accessible via tc class show",

[ ... ]

> +        "cmdUnderTest": "$TC -s class show dev $DUMMY",
> +        "expExitCode": "0",
> +        "verifyCmd": "$TC -s class show dev $DUMMY",
> +        "matchPattern": "class fq_pie 2:[0-9]+ .*prob .*delay .*deficit",
> +        "matchCount": "1",

[Severity: Medium]
Will this test fail, rather than skip, on any iproute2 that doesn't have
the companion patch?

The commit message says the test checks that an fq_pie class is reported.
The matchPattern, however, also needs the words prob, delay and deficit
after the class line, and only the out-of-tree iproute2 change prints
them.

On the kernel side, fq_pie_dump_class_stats() in net/sched/sch_fq_pie.c
only sends a binary blob:

net/sched/sch_fq_pie.c:fq_pie_dump_class_stats() {
    ...
	xstats.type = TCA_FQ_PIE_XSTATS_CLASS;
    ...
	return gnet_stats_copy_app(d, &xstats, sizeof(xstats));
}

Current iproute2 decodes fq_pie xstats with the qdisc layout (pkts_in,
overlimit, overmemory, dropped, ecn_mark, new_flow_count and so on). None
of those labels is prob, delay or deficit. The tbf class 1:1 is printed
before the fq_pie class, so nothing after "class fq_pie 2:1" can supply
those words either.

That means re.findall() gets 0 matches against a matchCount of 1, and
run_one_test() in tdc.py marks the test as failed:

tools/testing/selftests/tc-testing/tdc.py:run_one_test() {
    ...
	if len(match_index) != int(tidx["matchCount"]):
	    res.set_result(ResultState.fail)
    ...
}

tdc can already skip tests that depend on a userspace feature, through
the dependsOn key, which run_one_test() checks. mirred.json uses it, for
example:

    "dependsOn": "$TC actions add action mirred help 2>&1 | grep -q blockid",

Test 83c0 has no dependsOn probe. As a result, a correct kernel tested
with stock iproute2 would show a new tc-testing failure. The selftest
would also depend on an output format that only exists in an unmerged
userspace patch.

Would it make sense to either add a dependsOn probe for the new iproute2
output, or narrow the pattern to just the "class fq_pie 2:N" line as the
commit message describes? The commit message could then be updated to
say which approach the test uses.

> +        "teardown": [
> +            "$TC qdisc del dev $DUMMY handle 1: root",
> +            "$IP addr del 10.10.11.10/24 dev $DUMMY || true"
> +        ]
>      }
>  ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109071017.438478-1-hemendranaik%40gmail.com

Re: [PATCH net-next v10 3/4] net/sched: pie: correct tc_pie_xstats field documentation

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The patch fixes the unit comments for struct tc_pie_xstats in
  include/uapi/linux/pkt_sched.h: delay is now 'microseconds' and…

--

net/sched: pie: correct tc_pie_xstats field documentation

This patch updates the comments on struct tc_pie_xstats in the uapi header.
The delay field is now documented as microseconds and avg_dq_rate as
bytes/second, which matches what pie_dump_stats() exports. There is no
change to the UAPI layout or to runtime behaviour.

> diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
> index fef848eaaab23..445a041cd103f 100644
> --- a/include/uapi/linux/pkt_sched.h
> +++ b/include/uapi/linux/pkt_sched.h
> @@ -920,9 +920,9 @@ enum {
>  
>  struct tc_pie_xstats {
>  	__u64 prob;			/* current probability */
> -	__u32 delay;			/* current delay in ms */
> +	__u32 delay;			/* current delay in microseconds */
>  	__u32 avg_dq_rate;		/* current average dq_rate in
> -					 * bits/pie_time
> +					 * bytes/second
>  					 */

[Severity: Low]
Should the netlink YAML spec for this struct get the same correction?

The tc-pie-xstats definition in Documentation/netlink/specs/tc.yaml still
uses the old units:

        name: delay
        type: u32
        doc: Current delay in ms
      -
        name: avg-dq-rate
        type: u32
        doc: Current average dq rate in bits/pie-time

pie_dump_stats() in net/sched/sch_pie.c exports microseconds and
bytes/second:

	.delay = ((u32)PSCHED_TICKS2NS(READ_ONCE(q->vars.qdelay))) / NSEC_PER_USEC,
	...
	st.avg_dq_rate = READ_ONCE(q->vars.avg_dq_rate) *
			 (PSCHED_TICKS_PER_SEC) >> PIE_SCALE;

With this patch applied, the header and the YAML spec give different units
for the same uAPI struct.

The later commit "Documentation/netlink/specs: tc: describe fq_pie per-flow
class stats" edits tc.yaml. It documents the matching fq_pie fields as
microseconds and bytes/second, but leaves tc-pie-xstats as it is, so tc.yaml
contradicts itself at the end of the series.

The commit message says:

    Touch include/uapi/linux/pkt_sched.h only

Could tc.yaml be updated here as well, or in a follow-up patch?

>  	__u32 dq_rate_estimating;	/* is avg_dq_rate being calculated? */
>  	__u32 packets_in;		/* total number of packets enqueued */

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109071017.438478-1-hemendranaik%40gmail.com

Re: [PATCH net-next v10 4/4] Documentation/netlink/specs: tc: describe fq_pie per-flow class stats

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] This patch appends `type` (u32) and `class-stats` (binary,
  struct tc-fq-pie-cl-stats, 24 bytes) to the `tc-fq-pie-xstats` fixed…
- [Low] The commit message names one of the five fields "fq-rate-
  estimating".
- [Low] The new spec docs leave out how to interpret the fq_pie xstats
  payload, which now has two meanings.

--

Documentation/netlink/specs: tc: describe fq_pie per-flow class stats

This commit adds a tc-fq-pie-cl-stats struct to the tc netlink spec. The
struct describes the fq_pie per-flow class stats. The commit also extends
the tc-fq-pie-xstats fixed header with a type discriminator and a
class-stats member placed after the nine legacy qdisc counters.

> Add the tc-fq-pie-cl-stats header for fq_pie per flow (class) stats in Documentation/netlink/specs/tc.yaml
> with its five fields: prob, delay, deficit, avg-dq-rate, fq-rate-estimating.

[Severity: Low]
Should the fifth field be dq-rate-estimating rather than fq-rate-estimating?

The member this patch adds is dq-rate-estimating. The UAPI field is
dq_rate_estimating in struct tc_fq_pie_cl_stats, and
fq_pie_dump_class_stats() fills xstats.class_stats.dq_rate_estimating.

Two smaller points: the body lines are longer than 75 columns, and the
new struct is called a "header".

> diff --git a/Documentation/netlink/specs/tc.yaml b/Documentation/netlink/specs/tc.yaml
> index b6ccc645b0e33..b159c4b563c7a 100644
> --- a/Documentation/netlink/specs/tc.yaml
> +++ b/Documentation/netlink/specs/tc.yaml
> @@ -894,6 +894,30 @@ definitions:
>          name: memory-limit
>          type: u32
>          doc: Memory limit in bytes
> +  -
> +    name: tc-fq-pie-cl-stats
> +    type: struct
> +    members:
> +      -
> +        name: prob
> +        type: u64
> +        doc: Current drop probability of the flow

[ ... ]

> @@ -934,6 +958,15 @@ definitions:
>          name: memory-usage
>          type: u32
>          doc: Total memory across all queues
> +      -
> +        name: type
> +        type: u32
> +        doc: TCA_FQ_PIE_XSTATS_QDISC (0) or TCA_FQ_PIE_XSTATS_CLASS (1)

[Severity: Low]
Should the spec say which members are valid for each type value? Neither
this doc nor the existing docs for packets-in through memory-usage do.

fq_pie_dump_class_stats() zeroes the whole struct and fills only
class_stats:

net/sched/sch_fq_pie.c:fq_pie_dump_class_stats() {
    ...
		memset(&xstats, 0, sizeof(xstats));
		xstats.type = TCA_FQ_PIE_XSTATS_CLASS;
		xstats.class_stats.prob =
			READ_ONCE(flow->vars.prob) << BITS_PER_BYTE;
    ...
}

fq_pie_dump_stats() does the opposite. It sets type to
TCA_FQ_PIE_XSTATS_QDISC and leaves class_stats zeroed.

tca-stats-app-msg maps fq_pie to this one fixed header for both qdisc
and class messages, so YNL decodes and prints every member in both
cases. A class dump will show packets-in: 0, dropped: 0 and so on.

Could the docs note that the legacy counters only apply when type is
TCA_FQ_PIE_XSTATS_QDISC, and class-stats only when it is
TCA_FQ_PIE_XSTATS_CLASS?

The prob doc in tc-fq-pie-cl-stats also just says "Current drop
probability of the flow". Because of the << BITS_PER_BYTE above, the
value is a fixed-point fraction where U64_MAX means 100%. Can the doc
state that scale? The existing tc-pie-xstats prob doc doesn't give a
scale either.

> +      -
> +        name: class-stats
> +        type: binary
> +        struct: tc-fq-pie-cl-stats
> +        doc: Class statistics of a particular flow (invoked via tc class show)

[Severity: Medium]
Does this break YNL decoding against kernels from before the per-flow
stats change?

Adding type and class-stats makes YNL compute tc-fq-pie-xstats as 64
bytes. Older kernels send a 36-byte TCA_STATS_APP payload for fq_pie:
nine __u32 counters, copied with sizeof(st) in fq_pie_dump_stats().

The fq_pie entry in tca-stats-app-msg only has
fixed-header: tc-fq-pie-xstats. So _decode_sub_msg() passes attr.raw
directly to _decode_struct(), and _decode_struct() does not check the
remaining length:

tools/net/ynl/pyynl/lib/ynl.py:_decode_struct() {
    ...
            else:
                format_ = NlAttr.get_format(m.type, m.byte_order)
                [ value ] = format_.unpack_from(data, offset)
                offset += format_.size
    ...
}

The type member sits at offset 36. On a 36-byte buffer, unpack_from()
raises struct.error, and nothing in ynl.py catches it.

So with the updated spec, a getqdisc stats dump on an older kernel with
an fq_pie qdisc configured fails to decode.

The previous 36-byte definition worked with both old kernels (exact fit)
and new ones (trailing bytes ignored). Could _decode_struct() handle
short payloads, or could the spec be changed in some other way so it
still decodes against older kernels?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109071017.438478-1-hemendranaik%40gmail.com