Subject: Re: [PATCH net-next] net/sched: introduce TCQ_F_IN_HW to track successful qdisc offload

From: Jakub Kicinski <[email-masked]>
Date: Wed, 13 May 2026 17:42:34 -0700

On Sat,  9 May 2026 20:22:33 +0800 David Yang wrote:
> qdisc_offload_dump_helper(), introduced for RED in commit 602f3baf2218
> ("net_sch: red: Add offload ability to RED qdisc"), re-evaluates offload
> liveness on every dump because a parent qdisc change can alter the
> offload state without any qdisc-specific callback being invoked.  In
> this path -EOPNOTSUPP means "I do not offload this qdisc currently",
> not "I do not support statistics at all".
> 
> Only mlxsw and nfp handle this correctly by maintaining internal qdisc
> tracking (they need it for RED offload).  Every other qdisc type that
> supports offload (ETS, PRIO, TBF, FIFO, MQ) calls TC_*_DESTROY
> unconditionally in its destroy path regardless of whether a previous
> TC_*_REPLACE succeeded.  Moreover, without a flag recording whether
> REPLACE was accepted, a simple driver that queries hardware directly
> rather than maintaining a software shadow has no way to answer STATS
> correctly: it must either always return -EOPNOTSUPP (hiding real
> offloads from stats) or always return 0 (false positive, claiming a
> qdisc is offloaded when it never was).
> 
> Gating TC_*_STATS or TC_*_DESTROY on prior TC_*_REPLACE / TC_*_GRAFT
> success is not an option: sch_gred relies on receiving STATS even after
> offload ends in order to adjust backlog counters, and silently skipping
> DESTROY for a qdisc that was previously offloaded would leak hardware
> state.
> 
> Introduce TCQ_F_IN_HW to indicate that a qdisc has been successfully
> loaded to hardware via TC_*_REPLACE.  The flag is set by the new
> qdisc_offload_change_helper() on success and consumed by the new
> qdisc_offload_destroy_helper() which skips the driver call entirely
> when it is clear.  This is orthogonal to TCQ_F_OFFLOADED, which remains
> the liveness indicator re-evaluated on every dump.
> 
> With this a simple driver can be stateless: on STATS it queries
> hardware by handle and returns -EOPNOTSUPP when the entry is absent (the
> core will clear TCQ_F_OFFLOADED but TCQ_F_IN_HW remains, so a subsequent
> destroy still cleans up); on DESTROY it need not defend against "was
> this ever offloaded?" because the core already verified TCQ_F_IN_HW.
> The driver no longer needs to maintain its own registry of offloaded
> qdisc handles.

I read this 3 times and I'm still not 100% sure what the main problem
you're trying to fix is. You don't know if the DELETE is for the
offloaded qdisc? Can't you just store the handle of what's offloaded
in the driver?

The qdisc offload is a bit of a mess without a dedicated maintainer.
Sweeping changes like this are a bit burdensome for the core
maintainers..
-- 
pw-bot: cr