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

[Ratheesh Kannoth]

On 2026-09-22 at 07:49:13, netdev-bot+sashiko@xxxxxxxxxx (netdev-bot+sashiko@xxxxxxxxxx) wrote:
> Thank you for your contribution! Sashiko AI review found 17 potential
> issue(s) to consider.
>
> Critical: 0 · High: 7 · Medium: 9 · Low: 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] otx2_mqprio_restart_netdev() manipulates the core-owned
>   __LINK_STATE_START bit directly (clear_bit()/set_bit()) to fake…
> - [High] When otx2_nix_tm_clear_queue_shaper() fails, otx2_mqprio_down()
>   deliberately returns the error and keeps pfvf->mqprio.rate_limit =…
> - [High] otx2_mqprio_netdev_tc_work() reads pfvf->mqprio.rate_limit and
>   pfvf->old_mq_snap before taking rtnl_lock() and never re-validates…
> - [High] Every early validation return in otx2_setup_tc_mqprio()
>   (interface not UP, wrong shaper, no CIR+PIR, SDP rep, PFC enabled, XDP…
> - [High] A successful mqprio replacement is rolled back when
>   NETIF_F_HW_TC is not enabled, which is the default for these devices.
> - [High] Replacing a software mqprio root with an offloaded mqprio
>   succeeds but loses the newly installed traffic-class layout.
> - [High] The new mqprio.netdev_tc_work dereferences pfvf->netdev and
>   pfvf->old_mq_snap and mutates core netdev TC state, but…
> - [Medium] Two changes to code shared by all PF/VF configurations are
>   bundled into this feature patch with no mention in the commit message:…
> - [Medium] The commit message claims failed setup 'rolls back via netdev
>   restart' and the code reports 'mqprio: replace failed; prior…
> - [Medium] otx2_mqprio_max_rate_bytes_ps() derives a maximum *rate*
>   (bytes/s) from the NIX *burst-size* constants MAX_BURST_SIZE (130816…
> - [Medium] The driver reconstructs the core qdisc replace state machine:
>   otx2_mqprio_keep_netdev_tc() strcmp()s…
> - [Medium] otx2_txschq_stop() now zeroes pfvf->hw.txschq_cnt[] on every
>   interface stop, but otx2_get_smq_idx() computes 'idx = qidx %…
> - [Medium] A failed mqprio replacement can leave the surviving old qdisc
>   inconsistent with both the driver's cached state and the programmed…
> - [Medium] pf->refill_wrk (devm_kcalloc'd array of INIT_DELAYED_WORK'd
>   struct refill_work, allocated in otx2_config_nix_queues() from…
> - [Medium] INIT_WORK(&nic->mqprio.netdev_tc_work,
>   otx2_mqprio_netdev_tc_work) is performed in otx2_init_tc(), which runs…
> - [Medium] otx2_nix_tm_set_queue_shaper() programs both MDQ PIR and CIR
>   via otx2_get_txschq_rate_regval(pfvf, rate, 0), and burst == 0 in…
> - [Low] Two new comments describe the hard-coded burst value 65536 in the
>   HTB shaper path as 'the kernel-side default burst when HTB does not…
>
Will address in v17.

pw-bot: changes-requested