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

[PATCH net-next v9 0/3] 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:

v9:
- 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 (3):
  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

 include/uapi/linux/pkt_sched.h                | 21 ++++-
 net/sched/sch_fq_pie.c                        | 88 ++++++++++++++++++-
 .../tc-testing/tc-tests/qdiscs/fq_pie.json    | 27 ++++++
 3 files changed, 133 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH net-next v9 1/3] 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..8cd99ff40d0a 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 v9 2/3] 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 v9 3/3] 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

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

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 4 · Low: 4

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] When fq_pie is created with 'flows 65536', the last flow is
  dumped with the qdisc's own handle instead of a class ID.
- [Medium] The new uAPI field tc_fq_pie_cl_stats.avg_dq_rate is '__u32'
  and documented as bytes/second.
- [Medium] Once the PIE controller's probability goes above MAX_PROB, the
  exported per-flow drop probability wraps to a small value.
- [Medium] fq_pie_dump_class_stats() reads per-flow state without holding
  the qdisc lock or having matching writer annotations.
- [Low] The patch adds a new binary struct (struct tc_fq_pie_cl_stats) to
  the netlink uAPI and grows the existing struct tc_fq_pie_xstats blob…
- [Low] The discriminator 'type' is placed at offset 36, after the nine
  qdisc counters, instead of first in a union as sch_fq_codel does.
- [Low] fq_pie now has cl_ops, but .tcf_block, .bind_tcf and .unbind_tcf
  are left out, and the commit message only says this is 'on purpose'…
- [Low] The in-tree netlink YAML spec was not updated for the new uAPI
  layout, and none of the three patches in the series touches it.

--

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

This commit extends struct tc_fq_pie_xstats with a type discriminator and
a new struct tc_fq_pie_cl_stats, which grows it from 36 to 64 bytes. It
also adds fq_pie_class_ops with walk, dump and dump_stats, so that
'tc -s class show' reports per-flow 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]
Is __u32 wide enough for a rate in bytes/second?

fq_pie_dump_class_stats() computes the value in 64 bits and then assigns
it to this field:

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

Any rate above U32_MAX bytes/s (about 4.29 GB/s, or roughly 34 Gbit/s)
loses its high bits without any warning.

pie_process_dequeue() computes (dq_count << PIE_SCALE) / dtime at tick
granularity. With a dtime of 1 tick and a 64KB GSO skb, the internal
estimator can already go past that limit.

tc_pie_xstats has the same narrow type, but this is a new uAPI field.
Would __u64 fit better here? Another option is to saturate the value and
document that.

> +	__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]
This isn't a bug, but would separate attributes work better than a binary
struct here? The netlink guidelines discourage binary structures inside
attributes, and sch_cake uses nested attributes inside TCA_STATS_APP.

With this layout:

- Every qdisc dump from fq_pie_dump_stats() carries 28 bytes of class
  fields that are always zero.
- Every class dump from fq_pie_dump_class_stats() carries the nine qdisc
  counters (36 bytes), which are always zero.
- Any future qdisc or class counter can only go after class_stats, so
  the payload grows for both message types.

The layout itself looks right: type at offset 36, class_stats at 40, 64
bytes in total. Could the commit message explain why this design was
chosen?

[Severity: Low]
How does existing userspace handle the class payloads? sch_fq_codel puts
type first, followed by a union. Here type comes after the nine qdisc
counters, so a tool built before this field existed cannot tell a class
payload from a qdisc payload.

Setting .cl_ops also means tc_dump_tclass_qdisc() now walks fq_pie
classes. Before this patch it reported none. An unmodified iproute2 would
call its fq_pie print_xstats() for each active flow in
'tc -s class show' and print a line of zeros:

    pkts_in 0 overlimit 0 dropped 0 ...

It also goes the other way. Userspace rebuilt against the 64-byte header
gets 36-byte payloads from older kernels. That only works if the tool
zero-fills its buffer and copies min(payload, sizeof). A tool that
rejects short payloads would drop the output.

Should the commit message mention both of these?

[Severity: Low]
Should Documentation/netlink/specs/tc.yaml be updated along with this?
The tc-fq-pie-xstats struct there still lists only the nine original u32
members. It has no type or class-stats member, and there is no
tc-fq-pie-cl-stats struct.

The fq_pie entry of the TCA_STATS_APP sub-message still says:

        value: fq_pie
        fixed-header: tc-fq-pie-xstats

YNL and other spec-driven tools would decode only the first 36 bytes.
They would never see the discriminator or the per-flow fields.
tc-fq-codel-xstats in the same spec does model its type member.

tc.yaml is still unchanged at the end of the series, after
'net/sched: pie: correct tc_pie_xstats field documentation'.

>  };
>  
>  /* CBS */
> 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);

[Severity: Medium]
What class handle does the last flow get when fq_pie is configured with
flows 65536?

fq_pie_change() accepts that value:

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

fq_pie_walk() passes cl = i + 1, so flow index 65535 is dumped with
cl = 0x10000. TC_H_MIN() masks that to 0. tc_fill_tclass() has already
set tcm_handle to q->handle, so this flow is reported as maj:0. That is
the qdisc's own handle, not a class ID.

fq_pie_hash() + 1 can be 65536, so traffic does reach that flow. The
existing tc-testing case also creates 'fq_pie flows 65536'. sch_fq_codel
has the same pattern.

Should the walk handle this flow differently? If not, could the commit
message mention the limitation?

> +	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;

[Severity: Medium]
Can this report a small probability for a flow that is actually being
dropped 100% of the time?

MAX_PROB is U64_MAX >> 8. pie_calculate_probability() does not cap
positive updates at MAX_PROB:

net/sched/sch_pie.c:pie_calculate_probability() {
    ...
	if (qdelay > (PSCHED_NS2TICKS(250 * NSEC_PER_MSEC)))
		delta += MAX_PROB / (100 / 2);

	WRITE_ONCE(vars->prob, vars->prob + delta);

	if (delta > 0) {
		/* prevent overflow */
		if (vars->prob < oldprob) {
    ...
}

This check only catches a full u64 wrap. Under sustained delay, prob can
reach about 1.03 * MAX_PROB, and pie_drop_early() then drops every
eligible packet. The shift by BITS_PER_BYTE throws away the upper eight
bits, so the exported value would be about 3%.

Would it make sense to clamp to MAX_PROB before the shift? sch_pie's
pie_dump_stats() uses the same conversion.

> +		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 these reads safe without the qdisc lock?

tc_fill_tclass() calls gnet_stats_start_copy_compat() with a NULL lock,
so this callback runs under RTNL only.

The writers hold the root qdisc lock and use plain compound updates. In
fq_pie_qdisc_enqueue():

		sel_flow->qlen++;
		sel_flow->backlog += pkt_len;

In fq_pie_qdisc_dequeue():

	flow->qlen--;
	flow->deficit -= pkt_len;
	flow->backlog -= pkt_len;

None of these writers use WRITE_ONCE(), so KCSAN would flag these races.
sch_fq_codel uses WRITE_ONCE() for the equivalent fields.

The unlocked reads cause a few more problems:

- vars.prob and vars.qdelay are 64 bits wide, so READ_ONCE() can tear on
  32-bit kernels.
- pie_calculate_probability() stores vars->prob + delta before it
  corrects an underflow to 0. A reader could see a wrapped value close
  to 100% instead of 0.
- qlen and backlog are read separately and can disagree.

fq_pie_dump_stats() in the same file takes sch_tree_lock() around its
reads. Could fq_pie_dump_class_stats() snapshot the flow under
sch_tree_lock() as well? d->lock is NULL here, so that should not
deadlock. Alternatively, the writers could use WRITE_ONCE() as fq_codel
does.

> +	}

[ ... ]

> +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,
> +};

[Severity: Low]
The commit message says the filter callbacks are "omitted on purpose".
What is the reason?

Now that cl_ops is non-NULL but has no .tcf_block, two things change.

First, 'tc filter add ... parent <fq_pie handle>:' now fails in
__tcf_qdisc_find() with a different errno and extack:

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

Before this patch it failed with -EINVAL and "Qdisc not classful".

Second, fq_pie_init() already allocates a block with
tcf_block_get(&q->block, &q->filter_list, ...). fq_pie_classify() maps
filter results onto the same 1..flows_cnt range that fq_pie_walk() now
publishes:

		if (TC_H_MIN(res.classid) <= q->flows_cnt)
			return TC_H_MIN(res.classid);

Without cl_ops->tcf_block, nothing can reach that block, so this path is
dead code. sch_fq_codel, sch_sfq and sch_cake all provide tcf_block,
bind_tcf and unbind_tcf.

Should these callbacks be wired up, or should the unreachable
filter_list/block code be removed? Either way, could the commit message
explain the choice?

[ ... ]

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

Re: [PATCH net-next v9 2/3] 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 needs an unreleased iproute2 to pass, and the…

--

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

This commit adds tc-testing case 83c0. The test builds a TBF + fq_pie
hierarchy on $DUMMY, sends ping traffic through it, and checks the output
of tc -s class show. The commit message says a companion iproute2 change
prints the per-flow counters.

> 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 @@
>          "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",

[ ... ]

> +        "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]

With a released iproute2, will this test fail instead of being skipped?

The commit message says the test will:

  "verify that 'tc -s class show' reports an fq_pie class."

It also says prob, delay and deficit are printed by the companion iproute2
change "tc: fq_pie: add support for printing per-flow PIE statistics".
However, the matchPattern requires all three labels:

    class fq_pie 2:[0-9]+ .*prob .*delay .*deficit

That iproute2 change does not seem to be merged yet. Released tc prints
the fq_pie xstats as pkts_in, overlimit, overmemory, dropped, ecn_mark,
new_flow_count, new_flows_len, old_flows_len, memory_used and
memory_limit.

With that tc, fq_pie_dump_class_stats() still produces the
"class fq_pie 2:1" line. But run_one_test() in tdc.py does this:

    match_index = re.findall(match_pattern, procout)
    if len(match_index) != int(tidx["matchCount"]):
        res.set_result(ResultState.fail)

It finds 0 matches, so 83c0 is reported as FAIL. For CI using stock
iproute2, would this show up as a kernel regression even though the
kernel side works?

tdc.py already supports a dependsOn probe, which skips a test when the
command returns non-zero. Other tests use it to gate on newer iproute2
features. For example, mirred.json has:

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

Could 83c0 add a similar dependsOn check that detects the new tc output?
Also, could the commit message say that the test checks the prob, delay
and deficit fields as well as the fq_pie class line?

No later patch in the series seems to change this test.

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

Re: [PATCH net-next v9 3/3] 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 units for struct tc_pie_xstats in
  include/uapi/linux/pkt_sched.h, but misses the other in-tree…

--

net/sched: pie: correct tc_pie_xstats field documentation

This commit changes the comments on struct tc_pie_xstats in the uapi
pkt_sched.h header so they match the values pie_dump_stats() exports.
The delay field is now documented in microseconds and avg_dq_rate in
bytes/second. There is no layout or runtime change.

> 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 Documentation/netlink/specs/tc.yaml get the same fix?

The tc-pie-xstats struct in the tc netlink spec still has the old units:

Documentation/netlink/specs/tc.yaml:
      -
        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
per second:

	.delay		= ((u32)PSCHED_TICKS2NS(READ_ONCE(q->vars.qdelay))) /
				   NSEC_PER_USEC,
	...
	/* unscale and return dq_rate in bytes per sec */
	if (st.dq_rate_estimating)
		st.avg_dq_rate = READ_ONCE(q->vars.avg_dq_rate) *
				 (PSCHED_TICKS_PER_SEC) >> PIE_SCALE;

After this patch, the header and the spec describe the same fields with
different units. The generated netlink spec documentation will still
show "ms" and "bits/pie-time".

The commit message says:

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

Could the patch update the delay and avg-dq-rate doc strings in tc.yaml
too? The same file already uses "microseconds" for similar fields, for
example delay-c and delay-l in tc-dualpi2-xstats.

>  	__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/20260928043358.6866-1-hemendranaik%40gmail.com