Re: [PATCH net v3 4/6] net/sched: fq_pie: clamp default quantum to avoid signed overflow

On Tue, Aug 25, 2026 at 5:43 AM Paolo Abeni <[email protected]> wrote:
>
> On 8/25/26 11:16 AM, Jamal Hadi Salim wrote:
> > On Tue, Aug 25, 2026 at 4:33 AM Paolo Abeni <[email protected]> wrote:
> >> On 8/22/26 9:55 PM, Jamal Hadi Salim wrote:
> >>> fq_pie_init() sets q->quantum = psched_mtu(qdisc_dev(sch)) without
> >>> clamping. A device with a huge MTU (e.g. dummy with max_mtu == 0
> >>> accepting MTU 2147483634) makes psched_mtu() return 0x80000000, which
> >>> overflows the signed flow->deficit to INT_MIN in fq_pie_qdisc_dequeue(),
> >>> causing an infinite loop and soft lockup. Emulate fq_pie_policy which
> >>> is already bounded to [1, 1 << 20]; clamp the default to [256, 1 << 20].
> >>> 256 matches fq_codel's floor and is a sane minimum for a DRR quantum.
> >>>
> >>> Conditions to recreate the bug: a device whose MTU (plus
> >>> hard_header_len) wraps psched_mtu() into the sign bit (e.g. a dummy
> >>> device with max_mtu == 0 accepting MTU 2147483634). Requires
> >>> CAP_NET_ADMIN in a user namespace.
> >>>
> >>> Fixes: ec97ecf1ebe4 ("net: sched: add Flow Queue PIE packet scheduler")
> >>> Reported-by: [email protected]
> >>> Tested-by: Victor Nogueira <[email protected]>
> >>> Signed-off-by: Jamal Hadi Salim <[email protected]>
> >>> ---
> >>>  net/sched/sch_fq_pie.c | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c
> >>> index 069e1facd413..b27d95418707 100644
> >>> --- a/net/sched/sch_fq_pie.c
> >>> +++ b/net/sched/sch_fq_pie.c
> >>> @@ -427,7 +427,8 @@ static int fq_pie_init(struct Qdisc *sch, struct nlattr *opt,
> >>>       pie_params_init(&q->p_params);
> >>>       sch->limit = 10 * 1024;
> >>>       q->p_params.limit = sch->limit;
> >>> -     q->quantum = psched_mtu(qdisc_dev(sch));
> >>> +     q->quantum = clamp_t(u32, psched_mtu(qdisc_dev(sch)),
> >>> +                          256, 1 << 20);
> >>
> >> Sashiko thinks that the soft lookup is still reachable via pie_change:
> >>
> >> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260822195509.112717-1-jhs%40mojatatu.com
> >>
> >> and has similar concerns for patch 6/6, too. It marks the issues as
> >> pre-existing, but AFAICS they overlap with the things addressed here.
> >>
> >> WDYT?
> >
> > You are right, they overlap. I had them as followups (with a few
> > others derived from the sashiko feedback with justification that the
> > v3 init-path clamps are independently correct and the stab cap already
> > mitigates the change-path worst case to a stall; but those two a
> > (adding max(256U, ...) to both fq_pie_change() and sfq_change(),
> > matching the fq_codel_change()) are more serious.
> > So if you'd prefer a v4 respin of the whole series, I can do that.
>
> I initially did not notice that the _change path would lead to
> "upper-bounded" stall, I think a follow-up is fine.

I will prioritize moving those two i mentioned as a followup as soon
as this patchset goes in.

> > Sashiko is a double edge sword - i think code quality is improving but
> > it feels like the work load has doubled ;->
>
> FWIW, I agree with the "double edge" assessment.
> A reference we must keep in mind is that there is no way back, so we
> need to adapt somehow.
>

Agreed ;-> Will take a while to adopt...

cheers,
jamal
> > Here's what i had as followups (some still to be vetted, just noting
> > what sashiko is stating to be reviewed later when cycles available and
> > potential followup patches sent):
> > - sch_dualpi2 unclamped psched_mtu
> > - sch_pie unclamped psched_mtu → AQM disable / div-by-zero
> > - hhf TCA_HHF_HH_FLOWS_LIMIT unbounded
> > - fq_pie_change() / sfq_change() 256 floor missing (one that you bring up here)
> > - DRR/ETS quantum=0 spin (have a patch, was reported already as a bug by vega@)
> > - Consider two separate clamps for fq_codel/sch_codel (nipa
> > gpt-5-6-sol-3-15): quantum in [256, FQ_CODEL_QUANTUM_MAX], mtubounded
> > separately (no 256 floor on mtu)
> FWIW, LGTM!
>
> /P
>