Re: [PATCH net 2/2] net: macb: reject an unknown link speed in the taprio setup

["Théo Lebrun" <theo.lebrun@bootlin.com>]

Hello Aleksei,

On Wed Sep 2, 2026 at 10:05 AM CEST, Aleksei Sviridkin wrote:
> speed is a u32, so SPEED_UNKNOWN arrives as 0xffffffff and passes the
> "speed <= 0" check. A taprio schedule installed while the link is down
> then has its hardware interval limit derived from that value, a limit
> that rounds to 1 ns, and the first entry fails with a misleading
> "exceeds hardware limit". Name the case instead of relying on the
> sign.

I don't understand the

   Name the case instead of relying on the sign.

sentence, it seems to contradict what really happens which is

   SPEED_UNKNOWN [...] passes the "speed <= 0" check

> Fixes: 89934dbf169e ("net: macb: Add TAPRIO traffic scheduling support")
> Assisted-by: LLM
> Signed-off-by: Aleksei Sviridkin <f@lex.la>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index a43855db1e45..81530b9257b2 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -4324,7 +4324,7 @@ static int macb_taprio_setup_replace(struct net_device *netdev,
>  	}
>  
>  	speed = kset.base.speed;
> -	if (unlikely(speed <= 0)) {
> +	if (unlikely(speed == SPEED_UNKNOWN || !speed)) {
>  		netdev_err(netdev, "Invalid speed: %d\n", speed);
>  		return -EINVAL;
>  	}

The zero case shouldn't happen, it's SPEED_UNKNOWN that can occur often
(on link down). It's basically the same as checking netif_running().

Doesn't the netdev_err() call deserve a fix in the SPEED_UNKNOWN case?
We might be printing "Invalid speed: 4294967295". I don't think we want
two format strings (0 or SPEED_UNKNOWN).
We could do "Invalid speed %d, link-down?\n".

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com