Re: [PATCH net v2 5/5] net: mvneta: keep the TX rate limits across open and MTU change

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message of "net: mvneta: keep the TX rate limits
  across open and MTU change" does not say how the bug was discovered,…

--

net: mvneta: keep the TX rate limits across open and MTU change

This commit stores the mqprio bucket refill value for each rate limited
TX queue. It then programs the rate limits again from
mvneta_start_dev(). Without this, mvneta_txq_hw_init() leaves every
limited queue running at 10.23 Mbit/s after the interface is brought up
again or its MTU is changed.

> Tested on a Turris Omnia (Armada 385) with 1 Mbit/s limits on a
> 100 Mbit/s link. After the link was set down and up, or the MTU was
> changed, each busy TX queue carried about 10 Mbit/s. With this patch it
> stays at about 1 Mbit/s.

[Severity: Low]
This isn't a bug, but the commit message may be missing two statements
that the netdev process now asks for in fixes.

The "Bug fixes" section of Documentation/process/maintainer-netdev.rst
says:

    All bug fixes must state how the issue was discovered and validated,
    and how the fix was tested.

and:

    The testing statement should explain whether the fix was tested
    against the reproducer and against kernel selftests.

The paragraph above names the hardware and gives before and after
numbers from the reproducer, so that part is covered.

It doesn't say how the problem was found, though. The document gives
examples such as "observed on a machine in real use" and "discovered by
manual code inspection". There is also no Reported-by or similar trailer
that would imply it.

It also doesn't say whether kernel selftests were run.

Could the commit message get a sentence on how the issue was found, and
another on whether selftests were run? This requirement was tightened
by the baseline commit f56daaadfecae1 ("docs: netdev: try to force
agents to disclose info for fixes").

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261103125021.70845-1-pepe.schlehofer%40gmail.com