This patch-set fix 4 bugs.
Fixes: Solve an LRO stall issue
Fixes: Avoid concurrent updates on 32-bit systems.
Fixes: Ensure secure concurrent access to adaptive_rx_coal.
Fixes: Ensure max_qps is always greater than or equal to num_qps
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1791858167.git.gongfan1%40huawei.com
This changes the return value when the loop exits on the LRO
replenish threshold. It used to return nr_pkts and now returns budget.
In hinic3_poll():
work_done = hinic3_rx_poll(irq_cfg->rxq, budget);
busy |= work_done >= budget;
if (busy)
return budget;
As a result, the threshold exit now skips napi_complete_done(),
hinic3_net_dim() and the MSI-X re-enable. NAPI keeps polling instead
of waiting for the next interrupt.
This seems unrelated to statistics, and the commit message does not
mention it. Was it intended? If it fixes an LRO stall, could it go in
a separate patch with its own explanation and a Fixes: tag?
https://sashiko.dev/#/patchset/cover.1792149958.git.gongfan1%40huawei.com
In hinic3_get_stats64() in hinic3_netdev_ops.c, the reader retry loops
accumulate txq_stats and rxq_stats using '+=':
do {
start = u64_stats_fetch_begin(&txq_stats->syncp);
bytes += txq_stats->bytes;
...
} while (u64_stats_fetch_retry(&txq_stats->syncp, start));
If the loop retries due to concurrent updates on 32-bit systems, the counters
are added multiple times. This was previously benign since the fields weren't
populated by the driver, but these new updates will cause incorrect statistics
on 32-bit platforms.
https://sashiko.dev/#/patchset/cover.1774684571.git.zhuyikai1%40h-partners.com
When disabling adaptive RX coalescing via ethtool, does the driver
cancel any pending Dynamic Interrupt Moderation (DIM) work items?
If adaptive coalescing is disabled but an already-scheduled DIM worker
executes asynchronously in a workqueue, could it blindly apply dynamically
calculated DIM values to the hardware, silently overwriting the user's newly
configured static parameters?
https://sashiko.dev/#/patchset/cover.1792671176.git.gongfan1%40huawei.com
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1792671176.git.gongfan1%40huawei.com
After that, hinic3_sw_init()->hinic3_try_to_enable_rss() can shrink
max_qps to 1 when alloc_rss_resource() or hinic3_set_hw_rss_parameters()
fails:
hinic3_rss.c:hinic3_try_to_enable_rss() {
...
err = alloc_rss_resource(netdev);
if (err) {
nic_dev->max_qps = 1;
goto err_reset_q_params;
}
...
}
Only hinic3_vport_up() lowers real_num_*_queues, and only on a successful
open. A device-scope dump before the first successful open (or after opens
that fail before hinic3_vport_up()) still does this:
netdev_nl_stats_by_netdev()
netdev_stat_queue_sum(netdev, 0, netdev->real_num_rx_queues, &rx_sum,
0, netdev->real_num_tx_queues, &tx_sum);
There is no netdev->up check on this path, so i runs from 1 to
func_max_qnum - 1 against a one-entry array.
Would this read past the end of the rxqs and txqs slab objects and return
those values to user space? NETDEV_CMD_QSTATS_GET does not require
GENL_ADMIN_PERM, so any local user can issue it.
The same applies to nic_dev->txqs[i] in hinic3_get_queue_stats_tx().
Before this patch hinic3 had no stat_ops, and hinic3_get_stats64() bounds
its loops by max_qps.
Fan Gong (4):
Fixes: Solve an LRO stall issue
Fixes: Avoid concurrent updates on 32-bit systems.
Fixes: Ensure secure concurrent access to adaptive_rx_coal.
Fixes: Ensure max_qps is always greater than or equal to num_qps
.../net/ethernet/huawei/hinic3/hinic3_irq.c | 2 +-
.../net/ethernet/huawei/hinic3/hinic3_main.c | 2 +-
.../huawei/hinic3/hinic3_netdev_ops.c | 59 +++++++++----------
.../net/ethernet/huawei/hinic3/hinic3_rss.c | 11 ++--
.../net/ethernet/huawei/hinic3/hinic3_rx.c | 7 ++-
5 files changed, 39 insertions(+), 42 deletions(-)
base-commit: 73022f605c447f31610b94ed4a983d2b4bf2c721
--
2.43.0