Re: [PATCH net-next v02 5/5] hinic3: Ensure that the value of max_qps is not changed

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

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).

- [Medium] The patch has a Fixes: tag, so it will be treated as a stable
  bug fix.

--

hinic3: Ensure that the value of max_qps is not changed

This patch stops hinic3_try_to_enable_rss() from setting nic_dev->max_qps
to 1 when RSS setup fails. On that path it now sets only
nic_dev->q_params.num_qps to 1. It also changes the max_qps <= 1 check to
max_qps == 1.

> nic_dev->max_qps is used to allocate hardware resources. Therefore,
> it is assigned the value 1 only when its value is abnormal. In other
> scenarios, the value cannot be changed. However, the value of
> nic_dev->q_params.num_qps can be dynamically changed.
>
> Fixes: 1f3838b84a63 ("hinic3: Add Rss function")

[Severity: Medium]
Is this description accurate? The users of nic_dev->max_qps appear to
size only software arrays:

hinic3_alloc_txqs()          -> nic_dev->txqs
hinic3_alloc_rxqs()          -> nic_dev->rxqs
hinic3_init_intr_coalesce()  -> nic_dev->intr_coalesce
hinic3_setup_num_qps()       -> nic_dev->qps_msix_entries

The hardware queue capacity comes from nic_io->max_qps, which is set on
its own:

drivers/net/ethernet/huawei/hinic3/hinic3_nic_io.c:hinic3_init_nic_io() {
    ...
	nic_io->max_qps = hinic3_func_max_qnum(hwdev);
    ...
}

The removed nic_dev->max_qps = 1 assignments never change that value.

What failure does the Fixes: tag refer to? hinic3_try_to_enable_rss() has
one caller, hinic3_sw_init(). It runs before hinic3_alloc_txrxqs(), and
before hinic3_setup_num_qps() at open time. So with the old code, every
array sized from max_qps was sized at 1 after an RSS failure, and the
sizes matched.

At open, hinic3_config_num_qps() also forces num_qps to 1 whenever RSS is
disabled:

drivers/net/ethernet/huawei/hinic3/hinic3_netdev_ops.c:hinic3_config_num_qps() {
    ...
	if (!test_bit(HINIC3_RSS_ENABLE, &nic_dev->flags))
		q_params->num_qps = 1;
    ...
}

So after an RSS failure the device runs one queue, both before and after
this patch.

The only visible change seems to be on the RSS failure path: the
per-queue software arrays are now sized for the hardware maximum instead
of 1.

Could the commit message name the symptom being fixed, or the later change
that needs max_qps to stay at the hardware capacity? If neither exists,
would this fit better in net-next as a cleanup without the Fixes: tag?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1794371175.git.gongfan1%40huawei.com