From: Jakub Kicinski <kuba@kernel.org>
To: David Yang <mmyangfl@gmail.com>
Cc: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Paolo Abeni <pabeni@redhat.com>, Simon Horman <horms@kernel.org>,
Jamal Hadi Salim <jhs@mojatatu.com>,
Jiri Pirko <jiri@resnulli.us>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next] net/sched: introduce TCQ_F_IN_HW to track successful qdisc offload
Date: Wed, 13 May 2026 17:42:34 -0700 [thread overview]
Message-ID: <20260513174234.3e78e67b@kernel.org> (raw)
In-Reply-To: <20260509122238.2792915-1-mmyangfl@gmail.com>
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
prev parent reply other threads:[~2026-05-14 0:42 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-09 12:22 [PATCH net-next] net/sched: introduce TCQ_F_IN_HW to track successful qdisc offload David Yang
2026-05-14 0:42 ` Jakub Kicinski [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260513174234.3e78e67b@kernel.org \
--to=kuba@kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=jhs@mojatatu.com \
--cc=jiri@resnulli.us \
--cc=linux-kernel@vger.kernel.org \
--cc=mmyangfl@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox