Re: [PATCH v19 net-next 2/2] octeontx2: add mqprio bandwidth offload for NIX TX schedulers

[Simon Horman]

On Mon, Oct 12, 2026 at 09:20:01AM +0530, Ratheesh Kannoth wrote:
> On 2026-10-10 at 15:31:10, netdev-bot+sashiko@xxxxxxxxxx (netdev-bot+sashiko@xxxxxxxxxx) wrote:
> > Thank you for your contribution! Sashiko AI review found 12 potential
> > issue(s) to consider.
> >
> > Critical: 0 · High: 1 · Medium: 9 (1) · Low: 2 (1)
> >
> > 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).
> >
> > - [High] There is a use-after-free race on the deferred netdev_tc_work
> >   during device removal.
> 
> Not addressing this one; the window is theoretical and the impact is nil in practice.
> 1. It needs a failing "tc qdisc replace" (validation failure on an existing
>    offload) to run on CPU A at the exact moment the PCI remove runs on CPU B,
>    with A reading defer_tc_work just before B sets it. The netdev is already
>    being torn down, so this is not a realistic configuration path.
> 2. Even if the work is queued in that window, it is a no-op: the handler
>    checks defer_tc_work (set by remove) and returns before taking RTNL or
>    touching any other state. The only access is that single flag read.
> 3. A use-after-free would also need system_wq to leave the item undispatched
>    for the whole remainder of remove (unregister_netdev, destroy_workqueue,
>    mbox teardown, AF detach, ...) until free_netdev(). That is not a
>    realistic scheduling delay.
> The flag is accessed with READ_ONCE()/WRITE_ONCE() and ordered by smp_wmb()
> before cancel_work_sync(), so any tc command that starts after shutdown
> begins will see it and not queue the work.
> 
> > - [Medium] Offload state is leaked when qdisc teardown fails.
> 
> Not addressing this one; the trigger is very rare and the failure is reported.
> The path only runs if the AF mailbox request that clears the per-queue
> shapers (otx2_nix_tm_clear_queue_shaper()) fails during "tc qdisc del". That
> means the AF/mailbox is already in a fault state, which is not a normal
> runtime condition.
> In that case the driver prints error message.
> Keeping the state is deliberate. The shapers are still programmed in
> hardware, so the cached rates and rate_limit continue to describe what the
> hardware is doing, and they are reapplied consistently on the next open.
> Dropping the software state while the hardware shapers are still active would
> leave the two out of sync.
> The core cannot veto qdisc teardown (mqprio_disable_offload() is void), so the
> driver can only report the error, which it does.

...

Reviewed-by: Simon Horman <horms@xxxxxxxxxx>