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

[PATCH net-next v6 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
can be found at:
https://lore.kernel.org/netdev/20260902040221.83734-1-hemendranaik@gmail.com/

---
Changelog:

v6:
- 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                | 20 ++++-
 net/sched/sch_fq_pie.c                        | 86 ++++++++++++++++++-
 .../tc-testing/tc-tests/qdiscs/fq_pie.json    | 27 ++++++
 3 files changed, 130 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH net-next v6 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.

Derive delay with div_u64() from the full 64-bit nanosecond value rather
than truncating to u32 before dividing, which wrapped for sojourn times
above ~4.295 s, and widen avg_dq_rate to u64 before scaling it by
PSCHED_TICKS_PER_SEC.

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 | 16 +++++++
 net/sched/sch_fq_pie.c         | 86 +++++++++++++++++++++++++++++++++-
 2 files changed, 101 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index 490efd288526..9c0c9787f0c3 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -953,6 +953,20 @@ 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 +977,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..675861f578b8 100644
--- a/net/sched/sch_fq_pie.c
+++ b/net/sched/sch_fq_pie.c
@@ -511,7 +511,9 @@ 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 +565,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

Re: [PATCH net-next v6 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 6 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 4 (1)

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 fourth paragraph of the commit message is written as a bug
  fix ("Derive delay with div_u64() ... rather than truncating to u32…
- [Medium] net/sched: UAPI backward compatibility broken for
  tc_fq_pie_xstats size
- [Low] The new ABI field `tc_fq_pie_cl_stats.avg_dq_rate` is `__u32`
  documented as bytes/second, but fq_pie_dump_class_stats() computes the…
- [Low] The new class enumeration numbers flows 1..flows_cnt and encodes
  the number into the 16-bit minor field of tcm_handle.
- [Low] fq_pie_dump_class_stats() exports the per-flow probability as
  `READ_ONCE(flow->vars.prob) << BITS_PER_BYTE` with no bound check.
- [Low] Claim: fq_pie_dump_class_stats() samples per-flow PIE state
  (flow->vars.prob, flow->vars.qdelay, flow->vars.avg_dq_rate,…

Pre-existing issues:
- [Low] fq_pie_class_ops omits .tcf_block/.bind_tcf/.unbind_tcf while
  sch_fq_pie.c still allocates a tcf_block in fq_pie_init()…

--

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

This adds a Qdisc_class_ops table to sch_fq_pie with walk, dump and
dump_stats so that per-flow PIE state can be read with 'tc -s class show'.
It extends tc_fq_pie_xstats with a type discriminator plus an embedded
tc_fq_pie_cl_stats, growing the structure from 36 to 64 bytes.

A few questions below.

First, about the last paragraph of the changelog:

    Derive delay with div_u64() from the full 64-bit nanosecond value rather
    than truncating to u32 before dividing, which wrapped for sojourn times
    above ~4.295 s, and widen avg_dq_rate to u64 before scaling it by
    PSCHED_TICKS_PER_SEC.

[Severity: Medium]
Is this paragraph describing a change made by this patch?  The diff is
purely additive and net/sched/sch_fq_pie.c had no delay or avg_dq_rate
conversion before it, so the new fq_pie_dump_class_stats() is not replacing
any truncating computation.

The code that actually contains both of the described defects is
pie_dump_stats() in net/sched/sch_pie.c, which this patch does not touch and
does not mention, and there is no Fixes: tag:

net/sched/sch_pie.c:pie_dump_stats() {
	...
	.delay		= ((u32)PSCHED_TICKS2NS(READ_ONCE(q->vars.qdelay))) /
			   NSEC_PER_USEC,
	...
	if (st.dq_rate_estimating)
		st.avg_dq_rate = READ_ONCE(q->vars.avg_dq_rate) *
				 (PSCHED_TICKS_PER_SEC) >> PIE_SCALE;
	...
}

Could the paragraph either be reworded as a deliberate difference from the
pie qdisc, or the series extended to fix pie_dump_stats() as well?  The
later patch in the series that re-documents tc_pie_xstats.delay as
microseconds leaves the wrapping conversion in place.

> diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
> index 490efd2885267..9c0c9787f0c32 100644
> --- a/include/uapi/linux/pkt_sched.h
> +++ b/include/uapi/linux/pkt_sched.h
> @@ -953,6 +953,20 @@ 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: Low]
Since this is a new ABI field, is __u32 wide enough for avg_dq_rate?
fq_pie_dump_class_stats() computes the value in u64 and then stores it into
this 32-bit member with no saturation:

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

With PSCHED_TICKS_PER_SEC == 15625000 and PIE_SCALE == 8 the internal
estimator value is scaled by 61035.15625, so a per-flow rate above
U32_MAX bytes/second (about 34.36 Gbit/s) wraps: an internal avg_dq_rate of
81920 (5,000,000,000 B/s) is reported as 705,032,704.

Widening the member to __u64 or clamping with min_t() is still possible
before the layout is frozen.  It does mirror the existing
tc_pie_xstats.avg_dq_rate width, so if matching that is intentional it
would be worth saying so.

> +	__u32 dq_rate_estimating;	/* is avg_dq_rate being calculated? */
> +};

[ ... ]

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

[ ... ]

> @@ -563,7 +565,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]
Can the class id for the last flow collide with the qdisc handle when
flows_cnt is at its maximum?  fq_pie_change() accepts 65536 flows:

net/sched/sch_fq_pie.c:fq_pie_change() {
	...
	if (!q->flows_cnt || q->flows_cnt > 65536) {
		NL_SET_ERR_MSG_MOD(extack,
				   "Number of flows must range in [1..65536]");
	...
}

and fq_pie_walk() numbers classes one-based with tc_qdisc_stats_dump(sch,
i + 1, arg), so the last bucket arrives here as cl == 0x10000.  TC_H_MIN
masks with 0xffff, so minor becomes 0 and the RTM_NEWTCLASS message keeps
the tcm_handle that tc_fill_tclass() already set from the qdisc, i.e. X:0.

That handle cannot be used to address the flow afterwards: fq_pie_find()
always returns 0 and fq_pie_classify() only honours minors in
[1, flows_cnt]:

	if (TC_H_MAJ(skb->priority) == sch->handle &&
	    TC_H_MIN(skb->priority) > 0 &&
	    TC_H_MIN(skb->priority) <= q->flows_cnt)

The same encoding exists in net/sched/sch_fq_codel.c with the same 65536
limit, so this is a faithful copy of existing behaviour, but capping the
emitted ids at 65535 would avoid it here.

> +
> +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: Low]
Can this shift discard the high bits of prob?  pie_calculate_probability()
in net/sched/sch_pie.c does not clamp vars->prob to MAX_PROB
(U64_MAX >> 8) on the increase path; its only guard is the full u64 wrap:

net/sched/sch_pie.c:pie_calculate_probability() {
	...
	WRITE_ONCE(vars->prob, vars->prob + delta);

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

Sustained congestion can add up to roughly 4% of MAX_PROB per tupdate
(the 2% cap plus MAX_PROB/50 when qdelay exceeds 250 ms), so vars.prob can
grow past MAX_PROB.  The shift by 8 then drops the top bits and a flow that
pie_drop_early() is dropping unconditionally is reported with a near-zero
probability.

Would min_t(u64, READ_ONCE(flow->vars.prob), MAX_PROB) before scaling be
reasonable?  The same expression already exists in pie_dump_stats(), so
fixing both would keep them consistent.

> +		xstats.class_stats.delay =
> +			div_u64(PSCHED_TICKS2NS(READ_ONCE(flow->vars.qdelay)),
> +				NSEC_PER_USEC);
> +		xstats.class_stats.deficit = READ_ONCE(flow->deficit);

[Severity: Low]
This isn't a bug, but a note for anyone reading the sampling here: the
64-bit flow->vars.prob and flow->vars.qdelay are read without the qdisc
lock, since tc_fill_tclass() passes NULL as the lock to
gnet_stats_start_copy_compat(), so the snapshot is not internally
consistent and can tear on 32-bit kernels.

That matches the direction set by commit 5154561d9b119f ("net/sched:
sch_pie: annotate data-races in pie_dump_stats()") and commit
6d4106e8df94c0, and fq_codel_dump_class_stats() samples flow->cvars.* and
flow->deficit the same way, so no change seems needed.

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

[Severity: Low]
This is a pre-existing gap and is not introduced by this patch, but since
the new ops table is the natural place to address it: sch_fq_pie.c still
allocates a block in fq_pie_init() with
tcf_block_get(&q->block, &q->filter_list, sch, extack) and classifies every
packet through q->filter_list in fq_pie_classify(), yet no filter can ever
be attached.

net/sched/cls_api.c:__tcf_qdisc_find() {
	...
	if (!cops->tcf_block) {
		NL_SET_ERR_MSG(extack, "Class doesn't support blocks");
		err = -EOPNOTSUPP;
		goto errout_qdisc;
	}
	...
}

So rcu_dereference_bh(q->filter_list) in fq_pie_classify() is always NULL
and the "optional external classifier" path stays dead code.  Would it make
sense to either wire up a root-only .tcf_block plus .bind_tcf/.unbind_tcf
the way fq_codel_class_ops does, or drop the unused block and filter_list?

> +
>  static struct Qdisc_ops fq_pie_qdisc_ops __read_mostly = {
> +	.cl_ops		= &fq_pie_class_ops,
>  	.id		= "fq_pie",

[ ... ]

> @@ -963,6 +977,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 */

[Severity: Medium]
Growing struct tc_fq_pie_xstats from 36 to 64 bytes is visible to
userspace in both directions, and the changelog only argues the one
direction - that the nine existing __u32 counters keep their offsets, so
an old tc still parses a new kernel's payload.  The other direction is a
new tc on an old kernel: iproute2's fq_pie_print_xstats() rejects the
attribute outright if RTA_PAYLOAD(xstats) is smaller than
sizeof(struct tc_fq_pie_xstats), so a tc rebuilt against this header
(sizeof == 64) silently stops printing anything for 'tc -s qdisc show
dev ... fq_pie' on every kernel that still emits the 36-byte payload
from fq_pie_dump_stats().

That is not a kernel regression as such, but it does mean a distro tc
update loses fq_pie statistics on older kernels, so it would be good to
have it spelled out.  Does the accompanying iproute2 change parse
defensively, e.g. copying min(RTA_PAYLOAD(xstats), sizeof(st)) bytes
into a zeroed local struct rather than bailing out?  The enum values
help here: TCA_FQ_PIE_XSTATS_QDISC is 0, so a short read that is
zero-filled still resolves to the qdisc variant, and a length-tolerant
parse needs no extra magic.  Could that reasoning be added to the
changelog?

The alternative would be to leave tc_fq_pie_xstats at 36 bytes
entirely and emit a separate struct (just tc_fq_pie_cl_stats, with its
own discriminator if one is needed) from fq_pie_dump_class_stats(), so
the qdisc path's payload length never changes.  fq_codel could put the
type first and union the two variants in tc_fq_codel_xstats because the
discriminator was there from day one; fq_pie cannot do that without
moving offsets, which is exactly why appending here perturbs the qdisc
path as well.  Was that option considered and rejected?

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

[PATCH net-next v6 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 only
after 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

Re: [PATCH net-next v6 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 3 potential
issue(s) to consider.

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

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).

- [High] New tdc case 83c0 in tools/testing/selftests/tc-testing/tc-
  tests/qdiscs/fq_pie.json asserts `matchPattern` = "class fq_pie…
- [Medium] Case 83c0's assertion depends on fq_pie having a backlogged
  flow at the exact instant the dump runs, because fq_pie_walk()…
- [Low] Case 83c0 claims to cover the new per-flow fq_pie statistics but
  only checks that the literal label words "prob", "delay" and "deficit"…

--

Verified against the tree. Report follows.

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

This adds tdc case 83c0, which builds a TBF plus fq_pie hierarchy on
$DUMMY, drives traffic through it with ping, and then greps the output of
'tc -s class show' for an fq_pie class line carrying 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 @@
> +        "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"
> +        ],

[Severity: Medium]
Can this case fail intermittently depending on the host environment?  The
assertion needs a backlogged flow at the instant the dump runs, because
fq_pie_walk() in net/sched/sch_fq_pie.c skips every flow whose flowchain is
empty:

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

The only step that creates that backlog is the ping line, and its exit
status is discarded with '|| true'.  If ping is not installed, or the
installed iputils rejects the fractional -i 0.001 / -W 0.01 arguments, the
fq_pie queue stays empty and 'tc -s class show' prints no fq_pie class at
all.  tdc.py then takes this branch:

	elif int(tidx["matchCount"]) != 0:
	    res.set_result(ResultState.fail)
	    res.set_failmsg('No output generated by verify command.')

which reports 'No output generated by verify command' rather than pointing
at the missing traffic precondition.

There is also a timing aspect in the passing case: TBF is configured with
'rate 8bit burst 100b', so the bucket starts full and the first frame (an
~42-byte ARP request or a ~98-byte ICMP echo) is dequeued right away.  A run
where only one frame gets enqueued leaves the flow list empty at dump time
even though ping itself "succeeded".  Would it be better to check the ping
result, or to enqueue traffic in a way that does not depend on neighbour
solicitation timing?

[Severity: Low]
This isn't a bug, but with 'flows 1' there is exactly one flow, so the
class-id to flow-index contract between fq_pie_walk() and
fq_pie_dump_class_stats() is not really exercised:

net/sched/sch_fq_pie.c:fq_pie_walk() {
	...
		if (!tc_qdisc_stats_dump(sch, i + 1, arg))
	...
}

net/sched/sch_fq_pie.c:fq_pie_dump_class_stats() {
	u32 idx = cl - 1;
	...
}

A kernel that attributed statistics to the wrong flow would still satisfy
the pattern.  Two of the five fields in struct tc_fq_pie_cl_stats,
avg_dq_rate and dq_rate_estimating, are not referenced by the test at all,
and none of the unit conversions (probability scaling, PSCHED ticks to
microseconds, byte credits, avg_dq_rate scaling) are checked.  Would using
more than one flow plus matchJSON on 'tc -s -j class show' with numeric
assertions cover the kernel side more directly?

> +        "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: High]
Can this case ever pass with a released iproute2?  The kernel exports the
per-flow counters only as a binary TCA_STATS_APP payload:

net/sched/sch_fq_pie.c:fq_pie_dump_class_stats() {
	...
	if (idx < q->flows_cnt)
		return gnet_stats_copy_app(d, &xstats, sizeof(xstats));
	...
}

Turning that blob into the literal tokens "prob", "delay" and "deficit"
requires the companion iproute2 change mentioned in the commit message,
"tc: fq_pie: add support for printing per-flow PIE statistics", which is not
merged.  Shipped tc prints only the qdisc-level names (pkts_in, overlimit,
dropped, ecn_mark and friends), so re.findall() returns zero hits and
tdc.py fails the case:

	if len(match_index) != int(tidx["matchCount"]):
	    res.set_result(ResultState.fail)
	    res.set_failmsg('Could not match regex pattern. Verify command output:\n{}'.format(procout))

So the test appears to land in the tree permanently red on every tc-testing
run.  tdc has a probe mechanism for exactly this situation:

	if 'dependsOn' in tidx:
	    ...
	    if (p.returncode != 0):
	        res.set_result(ResultState.skip)
	        res.set_errormsg('probe command: test skipped.')

Would it make sense to add a 'dependsOn' probe so the case skips when the
userspace support is absent, or to assert only what current tc prints?

A related question: the pattern hard-codes the exact label text and ordering
of the not-yet-reviewed userspace formatter.  If the iproute2 side lands with
different wording or field order (say "drop_prob", or JSON-only output), does
this kernel selftest then break while the kernel itself is unchanged?

The commit message says the counters "are printed only after the companion
iproute2 change", but the case has no fallback pattern and no skip
condition, so that dependency reads as an aside rather than as the reason the
test cannot pass today.

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

[PATCH net-next v6 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 9c0c9787f0c3..969fa30efae7 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 v6 3/3] net/sched: pie: correct tc_pie_xstats field documentation

[netdev-bot+sashiko]

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

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

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 patch now documents tc_pie_xstats.avg_dq_rate
  unconditionally as "bytes/second", but pie_dump_stats()…
- [Medium] The patch now documents tc_pie_xstats.delay as "current delay
  in microseconds" with no range caveat, but pie_dump_stats()…
- [Low] Missing `Fixes:` tag.

--

net/sched: pie: correct tc_pie_xstats field documentation

This updates the comments on struct tc_pie_xstats so the documented units
for the delay and avg_dq_rate fields match what pie_dump_stats() actually
exports.  It is described as documentation-only, touching
include/uapi/linux/pkt_sched.h alone.

A few questions about the newly documented contract below.

> diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
> index 9c0c9787f0c32..969fa30efae7f 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 */

[Severity: Medium]
Is the microsecond description accurate for all values the kernel can
report here?  pie_dump_stats() in net/sched/sch_pie.c truncates the
nanosecond value to u32 before dividing:

net/sched/sch_pie.c:pie_dump_stats() {
	struct tc_pie_xstats st = {
		.prob		= READ_ONCE(q->vars.prob) << BITS_PER_BYTE,
		.delay		= ((u32)PSCHED_TICKS2NS(READ_ONCE(q->vars.qdelay))) /
				   NSEC_PER_USEC,
	...
}

PSCHED_TICKS2NS() in include/net/pkt_sched.h is:

#define PSCHED_TICKS2NS(x)		((s64)(x) << PSCHED_SHIFT)

so the (u32) cast discards the high bits, and a qdelay above roughly
4.295 s wraps modulo 2^32 ns.  A 5 s sojourn time would be reported as
(5e9 mod 2^32) / 1000 = 705032 us rather than 5000000 us, even though the
__u32 field can hold up to about 4294 s.  Delays in that range look
reachable with a large user-settable TCA_PIE_LIMIT or in ECN mode on a
low-rate link.

The peer producer of the identically documented field does the division on
the full 64-bit value:

net/sched/sch_fq_pie.c:fq_pie_dump_class_stats() {
	...
		xstats.class_stats.delay =
			div_u64(PSCHED_TICKS2NS(READ_ONCE(flow->vars.qdelay)),
				NSEC_PER_USEC);
	...
}

The preceding patch in this series, "net/sched: sch_fq_pie: add per-flow
statistics via class ops", calls out this same truncation ("which wrapped
for sojourn times above ~4.295 s") and fixes it only in sch_fq_pie.c.

Would it make sense to convert sch_pie.c to div_u64() in the same series so
the two ABI producers agree with the comment being added here?

>  	__u32 avg_dq_rate;		/* current average dq_rate in
> -					 * bits/pie_time
> +					 * bytes/second
>  					 */

[Severity: Medium]
Does the bytes/second wording hold on 32-bit kernels?  pie_dump_stats()
does the scaling without widening:

net/sched/sch_pie.c:pie_dump_stats() {
	...
	/* 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;
	...
}

q->vars.avg_dq_rate is u32 (include/net/pie.h) and PSCHED_TICKS_PER_SEC is

#define PSCHED_TICKS_PER_SEC		PSCHED_NS2TICKS(NSEC_PER_SEC)

which is 15625000 of type long.  On ILP32 both operands stay 32-bit, so the
product wraps once avg_dq_rate exceeds 4294967295 / 15625000, about 274,
i.e. a real drain rate above roughly 16.8 MB/s.  With an internal
avg_dq_rate of 512 the expected export is 512 * 15625000 >> 8 = 31250000
bytes/s, which fits __u32, but the value actually dumped is
(512 * 15625000 mod 2^32) >> 8 = 14472784.

sch_fq_pie.c widens before scaling:

net/sched/sch_fq_pie.c:fq_pie_dump_class_stats() {
	...
			xstats.class_stats.avg_dq_rate =
				((u64)READ_ONCE(flow->vars.avg_dq_rate)
				* PSCHED_TICKS_PER_SEC) >> PIE_SCALE;
	...
}

The earlier patch in this series states it "widen[s] avg_dq_rate to u64
before scaling it by PSCHED_TICKS_PER_SEC", but that widening is still
missing in sch_pie.c while this patch asserts the bytes/second contract for
it.  Could the same (u64) cast be added to pie_dump_stats()?

[Severity: Low]
This isn't a bug, but should this carry a Fixes: tag?  The wrong units in
struct tc_pie_xstats date back to the original PIE submission: commit
d4b36210c2e6 ("net: pkt_sched: PIE AQM scheme") added both the header
comments claiming ms and bits/pie_time and the pie_dump_stats() code that
already exported microseconds and "dq_rate in bytes per sec".  A

Fixes: d4b36210c2e6 ("net: pkt_sched: PIE AQM scheme")

line would record where the user-facing units first went wrong.

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

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

[Jakub Kicinski]

On Fri, 18 Sep 2026 02:12:22 +0530 Hemendra M. Naik wrote:
> This patch series is accompanied by a companion iproute2 patch series that
> can be found at:
> https://lore.kernel.org/netdev/20260902040221.83734-1-hemendranaik@gmail.com/

Does not apply any more. Also can you put it on a branch on github to
make it easier to pull please ? 

Please rebase and repost both the kernel and the iproute2. 

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

[Hemendra M. Naik]

Hi Jakub,

We've posted the new patches for kernel (v7) and iproute2 (v6) 
after rebasing.

The mailing list threads can be tracked here:
- Kernel v7: https://lore.kernel.org/netdev/20260919060125.38063-1-hemendranaik@gmail.com/
- iproute2 v6: https://lore.kernel.org/netdev/20260919061254.39678-1-hemendranaik@gmail.com/

As requested, we have also posted the latest 
patches on GitHub which can be found here:

- 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/

Thanks,
Hemendra