On 8/17/26 6:47 PM, Jakub Kicinski wrote:
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
ibmveth: Report MQ-aware RX counts in ethtool get_channels
The patch changes ibmveth_get_channels() to report max_rx as
IBMVETH_MAX_RX_QUEUES when the adapter has multi-queue firmware support
(else 1), and to report rx_count from the driver's published
num_rx_queues instead of netdev->real_num_rx_queues.
diff --git a/drivers/net/ethernet/ibm/ibmveth.c
b/drivers/net/ethernet/ibm/ibmveth.c
index 3202b657c9b8f..5d4ca4b3d3d07 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -2522,11 +2522,16 @@ static void ibmveth_get_ethtool_stats(struct net_device
*dev,
static void ibmveth_get_channels(struct net_device *netdev,
struct ethtool_channels *channels)
{
+ struct ibmveth_adapter *adapter = netdev_priv(netdev);
+
channels->max_tx = ibmveth_real_max_tx_queues();
channels->tx_count = netdev->real_num_tx_queues;
- channels->max_rx = netdev->real_num_rx_queues;
- channels->rx_count = netdev->real_num_rx_queues;
+ if (adapter->multi_queue)
+ channels->max_rx = IBMVETH_MAX_RX_QUEUES;
+ else
+ channels->max_rx = 1;
+ channels->rx_count = ibmveth_get_num_rx_queues(adapter);
}
[Severity: Medium]
At this point in the series, does raising max_rx here make RX channel
requests silently succeed without changing anything?
ibmveth_set_channels() only derives its work from tx_count:
unsigned int old = netdev->real_num_tx_queues,
goal = channels->tx_count;
...
if (!(netdev->flags & IFF_UP))
return netif_set_real_num_tx_queues(netdev, goal);
channels->rx_count is never read there. The core validation in
ethtool_set_channels() only bounds the request by the driver-reported
maximum:
if (channels.rx_count > curr.max_rx ||
channels.tx_count > curr.max_tx ||
so with max_rx now 16, "ethtool -L ethX rx N" for any N in 1..16 that
differs from the current RX count passes validation, enters
ibmveth_set_channels() with tx_count unchanged, runs no LTB loop
iterations, and returns 0. The netlink path in ethnl_set_channels()
applies the same max_rx-only ceiling and maps the 0 return to success.
Before this change max_rx equalled real_num_rx_queues, so an increase was
rejected with -EINVAL by the core; afterwards it is accepted and ignored,
and "ethtool -l" still shows the old rx_count.
For the record, this appears to be resolved later in the same series: the
final patch "ibmveth: Wire ethtool set_channels to MQ RX queue resize"
makes ibmveth_set_channels() read channels->rx_count and apply it via
ibmveth_resize_rx_channels() with range validation of
1..IBMVETH_MAX_RX_QUEUES, so the get/set pair agrees at the end of the
series. Would it be worth ordering that patch before this one, or
mentioning the intermediate state in the changelog?
Yes. At this commit set_channels still only looks at tx_count, so
ethtool -L rx N can pass core validation and return success without
changing RX. Raising max_rx here just widens that window from the
live count to 16. v6 names the intermediate state in the commit
message, which covers your second option.
The reorder is not done: channel reporting stays reviewable on its
own, separate from stats and from the resize. Patch 14 reads
rx_count and calls resize; patch 15 finishes the down-path and caps
max_rx once mq_fallback latches.
[Severity: Low]
A second, smaller point about the same lines: max_rx is derived only from
adapter->multi_queue and does not consult adapter->mq_fallback. After
firmware refuses multi-queue, ibmveth_apply_mq_fallback() defers the
downgrade to the next open:
if (!adapter->mq_fallback)
return;
adapter->mq_fallback = false;
...
adapter->multi_queue = 0;
ibmveth_publish_num_rx_queues(adapter, 1);
so between the firmware rejection and the next open, "ethtool -l" keeps
reporting max_rx = 16 and rx_count = 8 while the adapter is going to run
single-queue. Should get_channels() also take mq_fallback into account?
Also, at this patch the reported maximum of IBMVETH_MAX_RX_QUEUES (16) is
above anything the driver publishes, since ibmveth_probe() caps the RX
queue count:
adapter->multi_queue = 1;
ibmveth_publish_num_rx_queues(adapter,
min(num_online_cpus(),
IBMVETH_DEFAULT_QUEUES));
The end-of-series patch "ibmveth: Wire ethtool set_channels to MQ RX
queue resize" does make 1..IBMVETH_MAX_RX_QUEUES genuinely settable via
ibmveth_resize_rx_channels(), so this is only about the intermediate
state and the stale reporting while mq_fallback is latched.
Yes. While the latch is set, -l can still show MQ max_rx / rx_count
until the next open applies fallback — apply_mq_fallback() runs at
open entry, so get_channels() here still reads multi_queue and the
published count only, not the latch directly. Not taken here:
patch 15 consults the latch by capping max_rx at the live count,
not by understating rx_count. Clamping rx_count would turn the next
TX-only -L into a silent RX shrink.
Thanks,
Mingming
