Re: [PATCH net 1/2] net/sched: fq_codel: match the no-drop threshold to the packet size

[netdev-bot+sashiko@kernel.org]

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] sch_codel has the same 256-byte floor on the CoDel no-drop
  threshold.

--

net/sched: fq_codel: match the no-drop threshold to the packet size

This patch splits the clamp in fq_codel_init(). q->quantum keeps the
[256, FQ_CODEL_QUANTUM_MAX] range. q->cparams.mtu now follows psched_mtu()
with only the upper bound, so CoDel's no-drop threshold matches the real
packet size on links where the MTU plus hard_header_len is below 256.

Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260819143136.57350-1-jhs@mojatatu.com

> diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
> index 969b2510b0b86..e6c87a32950f9 100644
> --- a/net/sched/sch_fq_codel.c
> +++ b/net/sched/sch_fq_codel.c
> @@ -517,8 +517,8 @@ static int fq_codel_init(struct Qdisc *sch, struct nlattr *opt,
>  	q->flows_cnt = 1024;
>  	q->memory_limit = 32 << 20; /* 32 MBytes */
>  	q->drop_batch_size = 64;
> -	mtu = clamp_t(u32, psched_mtu(qdisc_dev(sch)), 256, FQ_CODEL_QUANTUM_MAX);
> -	q->quantum = mtu;
> +	mtu = min_t(u32, psched_mtu(qdisc_dev(sch)), FQ_CODEL_QUANTUM_MAX);
> +	q->quantum = clamp_t(u32, mtu, 256, FQ_CODEL_QUANTUM_MAX);

[Severity: Medium]
Should the plain codel qdisc get the same change? At this commit,
codel_init() in net/sched/sch_codel.c still puts the 256 floor on the
CoDel no-drop threshold:

net/sched/sch_codel.c:codel_init() {
    ...
	q->params.mtu = clamp_t(u32, psched_mtu(qdisc_dev(sch)), 256, 1 << 20);
    ...
}

Both qdiscs use the same exemption in codel_should_drop()
(include/net/codel_impl.h):

    *backlog <= params->mtu

So if a codel qdisc is attached to a device where psched_mtu() is below
256 (e.g. a CAN interface), the path codel_init() ->
__codel_qdisc_dequeue() -> codel_should_drop() still exempts more than one
packet of backlog. That is the same delay in drop or mark eligibility that
this patch describes for fq_codel.

It looks like the next patch in this series, "net/sched: sch_codel: match
the no-drop threshold to the packet size", fixes this by changing the line
to:

	q->params.mtu = min_t(u32, psched_mtu(qdisc_dev(sch)), 1 << 20);

That means the gap is only present between the two commits.

>  	INIT_LIST_HEAD(&q->new_flows);
>  	INIT_LIST_HEAD(&q->old_flows);
>  	codel_params_init(&q->cparams);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-34MS.v1.20260925165535%40mojatatu.com