Re: [PATCH net v2 8/8] net/sched: ets: clamp quantum in parse and fallback paths

Re: [PATCH net v2 8/8] net/sched: ets: clamp quantum in parse and fallback paths

[Jakub Kicinski]

On Sat, 29 Aug 2026 04:12:29 -0400 Jamal Hadi Salim wrote:
> ets_qdisc_change() falls back to psched_mtu() with no floor for bands
> without an explicit quantum. With a crafted size table qdisc_pkt_len
> reaches ~2 GiB, so a zero psched_mtu on a headerless device makes the
> deficit-refill loop spin under the qdisc lock.
> 
> Move the floor into ets_quantum_parse() so explicitly configured quanta
> are also clamped to [256, 1<<20], not just the fallback path.
> 
> Conditions to recreate the bug:
>   CONFIG_NET_SCH_ETS=y. Requires CAP_NET_ADMIN (namespace-local via
>   unshare -Urn suffices).
> 
>   tc qdisc add dev dummy0 root ets bands 3 strict 2 quanta 1 1
> 
> Fixes: dcc68b4d8084 ("net: sch_ets: Add a new Qdisc")
> Reported-by: vega@nebusec.ai
> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
> Tested-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: stable@vger.kernel.org
> ---
>  net/sched/sch_ets.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/net/sched/sch_ets.c b/net/sched/sch_ets.c
> index 25fcf4079fec..6cc902a03838 100644
> --- a/net/sched/sch_ets.c
> +++ b/net/sched/sch_ets.c
> @@ -83,11 +83,7 @@ static int ets_quantum_parse(struct Qdisc *sch, const struct nlattr *attr,
>  			     unsigned int *quantum,
>  			     struct netlink_ext_ack *extack)
>  {
> -	*quantum = nla_get_u32(attr);
> -	if (!*quantum) {
> -		NL_SET_ERR_MSG(extack, "ETS quantum cannot be zero");
> -		return -EINVAL;
> -	}
> +	*quantum = clamp_t(u32, nla_get_u32(attr), 256, 1 << 20);
>  	return 0;
>  }
>  
> @@ -632,11 +628,13 @@ static int ets_qdisc_change(struct Qdisc *sch, struct nlattr *opt,
>  			return err;
>  	}
>  	/* If there are more bands than strict + quanta provided, the remaining
> -	 * ones are ETS with quantum of MTU. Initialize the missing values here.
> +	 * ones are ETS with quantum of max(MTU, 256). Initialize the missing
> +	 * values here.
>  	 */
>  	for (i = nstrict; i < nbands; i++) {
>  		if (!quanta[i])
> -			quanta[i] = psched_mtu(qdisc_dev(sch));
> +			quanta[i] = clamp_t(u32, (u32)psched_mtu(qdisc_dev(sch)),
> +					    256, 1 << 20);
>  	}
>  
>  	/* Before commit, make sure we can allocate all new qdiscs */

Does this run afoul of one of the tdc cases?

# not ok 39 41f5 - ETS offload where the sum of quanta wraps u32
# Could not match regex pattern. Verify command output:
# qdisc ets 8004: root refcnt 5 offloaded bands 3 quanta 1048576 256 256 priomap 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 

https://github.com/p4tc-dev/tc-executor/blob/storage-dbg/artifacts/799938/1-tdc-sh/stdout

Re: [PATCH net v2 8/8] net/sched: ets: clamp quantum in parse and fallback paths

[Jamal Hadi Salim]

On Sun, Aug 30, 2026 at 2:03 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sat, 29 Aug 2026 04:12:29 -0400 Jamal Hadi Salim wrote:
> > ets_qdisc_change() falls back to psched_mtu() with no floor for bands
> > without an explicit quantum. With a crafted size table qdisc_pkt_len
> > reaches ~2 GiB, so a zero psched_mtu on a headerless device makes the
> > deficit-refill loop spin under the qdisc lock.
> >
> > Move the floor into ets_quantum_parse() so explicitly configured quanta
> > are also clamped to [256, 1<<20], not just the fallback path.
> >
> > Conditions to recreate the bug:
> >   CONFIG_NET_SCH_ETS=y. Requires CAP_NET_ADMIN (namespace-local via
> >   unshare -Urn suffices).
> >
> >   tc qdisc add dev dummy0 root ets bands 3 strict 2 quanta 1 1
> >
> > Fixes: dcc68b4d8084 ("net: sch_ets: Add a new Qdisc")
> > Reported-by: vega@nebusec.ai
> > Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
> > Tested-by: Victor Nogueira <victor@mojatatu.com>
> > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > Cc: stable@vger.kernel.org
> > ---
> >  net/sched/sch_ets.c | 12 +++++-------
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/net/sched/sch_ets.c b/net/sched/sch_ets.c
> > index 25fcf4079fec..6cc902a03838 100644
> > --- a/net/sched/sch_ets.c
> > +++ b/net/sched/sch_ets.c
> > @@ -83,11 +83,7 @@ static int ets_quantum_parse(struct Qdisc *sch, const struct nlattr *attr,
> >                            unsigned int *quantum,
> >                            struct netlink_ext_ack *extack)
> >  {
> > -     *quantum = nla_get_u32(attr);
> > -     if (!*quantum) {
> > -             NL_SET_ERR_MSG(extack, "ETS quantum cannot be zero");
> > -             return -EINVAL;
> > -     }
> > +     *quantum = clamp_t(u32, nla_get_u32(attr), 256, 1 << 20);
> >       return 0;
> >  }
> >
> > @@ -632,11 +628,13 @@ static int ets_qdisc_change(struct Qdisc *sch, struct nlattr *opt,
> >                       return err;
> >       }
> >       /* If there are more bands than strict + quanta provided, the remaining
> > -      * ones are ETS with quantum of MTU. Initialize the missing values here.
> > +      * ones are ETS with quantum of max(MTU, 256). Initialize the missing
> > +      * values here.
> >        */
> >       for (i = nstrict; i < nbands; i++) {
> >               if (!quanta[i])
> > -                     quanta[i] = psched_mtu(qdisc_dev(sch));
> > +                     quanta[i] = clamp_t(u32, (u32)psched_mtu(qdisc_dev(sch)),
> > +                                         256, 1 << 20);
> >       }
> >
> >       /* Before commit, make sure we can allocate all new qdiscs */
>
> Does this run afoul of one of the tdc cases?
>
> # not ok 39 41f5 - ETS offload where the sum of quanta wraps u32
> # Could not match regex pattern. Verify command output:
> # qdisc ets 8004: root refcnt 5 offloaded bands 3 quanta 1048576 256 256 priomap 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2
>
> https://github.com/p4tc-dev/tc-executor/blob/storage-dbg/artifacts/799938/1-tdc-sh/stdout

Yikes. I will resend with this fixed.

cheers.
jamal

Re: [PATCH net v2 8/8] net/sched: ets: clamp quantum in parse and fallback paths

[Jamal Hadi Salim]

On Sun, Aug 30, 2026 at 2:59 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On Sun, Aug 30, 2026 at 2:03 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Sat, 29 Aug 2026 04:12:29 -0400 Jamal Hadi Salim wrote:
> > > ets_qdisc_change() falls back to psched_mtu() with no floor for bands
> > > without an explicit quantum. With a crafted size table qdisc_pkt_len
> > > reaches ~2 GiB, so a zero psched_mtu on a headerless device makes the
> > > deficit-refill loop spin under the qdisc lock.
> > >
> > > Move the floor into ets_quantum_parse() so explicitly configured quanta
> > > are also clamped to [256, 1<<20], not just the fallback path.
> > >
> > > Conditions to recreate the bug:
> > >   CONFIG_NET_SCH_ETS=y. Requires CAP_NET_ADMIN (namespace-local via
> > >   unshare -Urn suffices).
> > >
> > >   tc qdisc add dev dummy0 root ets bands 3 strict 2 quanta 1 1
> > >
> > > Fixes: dcc68b4d8084 ("net: sch_ets: Add a new Qdisc")
> > > Reported-by: vega@nebusec.ai
> > > Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
> > > Tested-by: Victor Nogueira <victor@mojatatu.com>
> > > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > > Cc: stable@vger.kernel.org
> > > ---
> > >  net/sched/sch_ets.c | 12 +++++-------
> > >  1 file changed, 5 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/net/sched/sch_ets.c b/net/sched/sch_ets.c
> > > index 25fcf4079fec..6cc902a03838 100644
> > > --- a/net/sched/sch_ets.c
> > > +++ b/net/sched/sch_ets.c
> > > @@ -83,11 +83,7 @@ static int ets_quantum_parse(struct Qdisc *sch, const struct nlattr *attr,
> > >                            unsigned int *quantum,
> > >                            struct netlink_ext_ack *extack)
> > >  {
> > > -     *quantum = nla_get_u32(attr);
> > > -     if (!*quantum) {
> > > -             NL_SET_ERR_MSG(extack, "ETS quantum cannot be zero");
> > > -             return -EINVAL;
> > > -     }
> > > +     *quantum = clamp_t(u32, nla_get_u32(attr), 256, 1 << 20);
> > >       return 0;
> > >  }
> > >
> > > @@ -632,11 +628,13 @@ static int ets_qdisc_change(struct Qdisc *sch, struct nlattr *opt,
> > >                       return err;
> > >       }
> > >       /* If there are more bands than strict + quanta provided, the remaining
> > > -      * ones are ETS with quantum of MTU. Initialize the missing values here.
> > > +      * ones are ETS with quantum of max(MTU, 256). Initialize the missing
> > > +      * values here.
> > >        */
> > >       for (i = nstrict; i < nbands; i++) {
> > >               if (!quanta[i])
> > > -                     quanta[i] = psched_mtu(qdisc_dev(sch));
> > > +                     quanta[i] = clamp_t(u32, (u32)psched_mtu(qdisc_dev(sch)),
> > > +                                         256, 1 << 20);
> > >       }
> > >
> > >       /* Before commit, make sure we can allocate all new qdiscs */
> >
> > Does this run afoul of one of the tdc cases?
> >
> > # not ok 39 41f5 - ETS offload where the sum of quanta wraps u32
> > # Could not match regex pattern. Verify command output:
> > # qdisc ets 8004: root refcnt 5 offloaded bands 3 quanta 1048576 256 256 priomap 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2 2
> >
> > https://github.com/p4tc-dev/tc-executor/blob/storage-dbg/artifacts/799938/1-tdc-sh/stdout
>
> Yikes. I will resend with this fixed.

I was waiting for nipa sashiko to say something but nothing happened.
Gemini nipa had nothing meaningful to say - So i will send v3 with
just the tdc test fixed.

cheers,
jamal
> cheers.
> jamal