Re: [PATCH v2 4/4] net: ntb_netdev: Support ethtool channels for multi-queue

[Jakub Kicinski]

On Sat, 28 Feb 2026 23:55:38 +0900 Koichiro Den wrote:
> +static const struct ntb_queue_handlers ntb_netdev_handlers;

can we move some code around to avoid this forward declaration?

> +static int ntb_set_channels(struct net_device *ndev,
> +			    struct ethtool_channels *channels)
> +{
> +	struct ntb_netdev *dev = netdev_priv(ndev);
> +	unsigned int new = channels->combined_count;
> +	unsigned int old = dev->num_queues;
> +	bool running = netif_running(ndev);
> +	struct ntb_netdev_queue *queue;
> +	unsigned int q, created;
> +	int rc = 0;
> +
> +	if (channels->rx_count || channels->tx_count || channels->other_count)
> +		return -EINVAL;

Pretty sure core will protect from this since your get will return 0
for corresponding max values

> +	if (!new || new > ndev->num_tx_queues)
> +		return -ERANGE;

same, core should protect against this

> +	if (new == old)
> +		return 0;
> +
> +	if (new < old) {
> +		if (running)
> +			for (q = new; q < old; q++)
> +				netif_stop_subqueue(ndev, q);
> +
> +		rc = netif_set_real_num_queues(ndev, new, new);
> +		if (rc)
> +			goto out_restore;
> +
> +		/* Publish new queue count before invalidating QP pointers */
> +		dev->num_queues = new;
> +
> +		for (q = new; q < old; q++) {
> +			queue = &dev->queues[q];
> +
> +			if (running) {
> +				ntb_transport_link_down(queue->qp);
> +				ntb_netdev_queue_rx_drain(queue);
> +				timer_delete_sync(&queue->tx_timer);
> +			}
> +
> +			ntb_transport_free_queue(queue->qp);
> +			queue->qp = NULL;
> +		}
> +
> +		goto out_restore;
> +	}
> +
> +	created = old;
> +	for (q = old; q < new; q++) {
> +		queue = &dev->queues[q];
> +
> +		queue->ntdev = dev;
> +		queue->qid = q;
> +		queue->qp = ntb_transport_create_queue(queue, dev->client_dev,
> +						       &ntb_netdev_handlers);
> +		if (!queue->qp) {
> +			rc = -ENOSPC;
> +			goto err_new;
> +		}
> +		created++;
> +
> +		if (!running)
> +			continue;
> +
> +		timer_setup(&queue->tx_timer, ntb_netdev_tx_timer, 0);
> +
> +		rc = ntb_netdev_queue_rx_fill(ndev, queue);
> +		if (rc)
> +			goto err_new;
> +
> +		/*
> +		 * Carrier may already be on due to other QPs. Keep the new
> +		 * subqueue stopped until we get a Link Up event for this QP.
> +		 */
> +		netif_stop_subqueue(ndev, q);
> +	}
> +
> +	rc = netif_set_real_num_queues(ndev, new, new);
> +	if (rc)
> +		goto err_new;
> +
> +	dev->num_queues = new;
> +
> +	if (running)
> +		for (q = old; q < new; q++)
> +			ntb_transport_link_up(dev->queues[q].qp);
> +
> +	return 0;
> +
> +err_new:
> +	if (running) {
> +		unsigned int rollback = created;
> +
> +		while (rollback-- > old) {
> +			queue = &dev->queues[rollback];
> +			ntb_transport_link_down(queue->qp);
> +			ntb_netdev_queue_rx_drain(queue);
> +			timer_delete_sync(&queue->tx_timer);
> +		}
> +	}
> +
> +	while (created-- > old) {
> +		queue = &dev->queues[created];
> +		ntb_transport_free_queue(queue->qp);
> +		queue->qp = NULL;
> +	}
> +
> +out_restore:
> +	if (running) {
> +		ntb_netdev_sync_subqueues(dev);
> +		ntb_netdev_update_carrier(dev);
> +	}
> +	return rc;
> +}

The logic LGTM, but the function is a little long.
Would it be possible to split it up?