Re: [PATCH net-next v5 2/3] net: macb: implement ethtool_ops.get|set_channels()

[Nicolai Buchwitz <nb@tipi-net.de>]

Hi Théo

On 16.10.2026 18:58, Théo Lebrun wrote:
> bp->num_queues is the total number of queues and is constant from 
> probe.
> Introduce bp->max_num_queues which takes the current role of
> bp->num_queues and allow `0 < bp->num_queues <= bp->max_num_queues`.
> MACB/GEM does not know about Rx/Tx specific queues; it only has
> combined queues.
> 
> set_channels is reserved to devices with MACB_CAPS_QUEUE_DISABLE.
> The tieoff workaround would not work as packets would still be routed
> into queues with a tieoff descriptor.
> 
> Implement .set_channels() operation by refusing if netif_running(). We
> need context-swapping support upstream for proper on-the-fly channel
> count change.
> 
> Note we refuse the operation if any TAPRIO rules are in place on queues
> that would get disabled. If we allow the operation to take place, then
> a transmission window would be reserved but unusable.
> 
> Nit: fix an alignment issue inside gem_ethtool_ops which does not
> deserve its own patch.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---

> [...]

> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c

> [...]

> +static int macb_set_channels(struct net_device *netdev,
> +			     struct ethtool_channels *ch)
> +{
> +	struct macb *bp = netdev_priv(netdev);
> +	unsigned int count = ch->combined_count;
> +	int ret;
> +
> +	/* MACB_CAPS_QUEUE_DISABLE means that the field QUEUE_DISABLE/BIT0 in
> +	 * the per-queue RBQP/TBQP registers disable Tx/Rx queues. If we 
> don't
> +	 * have that capability we can have multiple queues but we must 
> always
> +	 * run with all enabled.
> +	 */
> +	if (!(bp->caps & MACB_CAPS_QUEUE_DISABLE))
> +		return -EOPNOTSUPP;
> +
> +	/* TODO: reuse context swapping once it is merged. */

Once context swapping is merged, this needs to be addressed in v6 ;-)

> +	if (netif_running(bp->netdev))
> +		return -EBUSY;
> +
> +	/* If we allow disable a queue which has TAPRIO active, we'll keep a
> +	 * transmission window which will be unusable.
> +	 */
> +	if (bp->taprio_gated_queues >> count) {
> +		netdev_err(bp->netdev, "TAPRIO schedule gates queues >= %u; destroy 
> the qdisc before reducing channels\n",
> +			   count);
> +		return -EBUSY;
> +	}

I think this can get stuck:

- delete the  taprio qdisc after the link has been down for a while
- device is is runtime suspend
- taprio setup bails out before _destroy() is called
- ethtool -L keeps failing with -EBUSY, even though there is no qdisc 
anymore

Something like:

   ip link set eth0 up
   tc qdisc replace dev eth0 root taprio ... flags 0x2
   ip link set eth0 down
   sleep 1
   tc qdisc del dev eth0 root
   ethtool -L eth0 combined 1

> [...]

Thanks,
Nicolai