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?
[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.
