[PATCH net v2 0/2] net: macb: fix the link speed the taprio setup reads

[PATCH net v2 0/2] net: macb: fix the link speed the taprio setup reads

[Aleksei Sviridkin]

Two small fixes in macb_taprio_setup_replace(), both in how it obtains
the link speed it scales the schedule with.

The first: it hands phylink_ethtool_ksettings_get() a stack variable
it never zeroed, while phylink fills only what the link mode provides
and even reads one field back from the caller. The second: the speed
check is written as "<= 0" on a u32, so SPEED_UNKNOWN passes it and
turns into a 1 ns hardware limit that every entry then exceeds.

Compile-tested against net; the driver has no test surface, and no
macb board here.

---
v2: both commit messages rewritten to name the uninitialised-memory
    case first and the link-down case as the one that happens; the
    grown declaration keeps reverse xmas tree order; the error text is
    "Invalid speed %d, link-down?" (Théo Lebrun). ethtool_validate_speed()
    was considered for the check and does not fit: it accepts
    SPEED_UNKNOWN by definition, which is exactly the value patch 2
    rejects (Andrew Lunn).
    https://lore.kernel.org/netdev/20260902080528.2211468-1-f@lex.la/


Aleksei Sviridkin (2):
  net: macb: zero the link settings taprio reads back
  net: macb: reject an unknown link speed in the taprio setup

 drivers/net/ethernet/cadence/macb_main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.53.0

[PATCH net v2 1/2] net: macb: zero the link settings taprio reads back

[Aleksei Sviridkin]

macb_taprio_setup_replace() calls phylink_ethtool_ksettings_get() with
an uninitialised kset, and kset is not only an out-parameter. On a
fixed link, or an in-band link with no PHY, phylink writes speed and
duplex only if kset->base.rate_matching already reads RATE_MATCH_NONE,
a field it never writes itself; in PHY mode before the PHY is attached
it writes port and supported and nothing more. Either way the speed
read back afterwards can be stack garbage. The ethtool core zeroes the
structure on every path into the op, which is why its callers never
see this; taprio is the only in-kernel caller passing its own variable.

Fixes: 89934dbf169e ("net: macb: Add TAPRIO traffic scheduling support")
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
v2: message rewritten, declaration in reverse xmas tree order (Théo
    Lebrun).

 drivers/net/ethernet/cadence/macb_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 76ee4f506033..61838084989a 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4295,9 +4295,9 @@ static int macb_taprio_setup_replace(struct net_device *netdev,
 	u64 total_on_time = 0, start_time_sec = 0, start_time = conf->base_time;
 	u32 configured_queues = 0, speed = 0, start_time_nsec;
 	struct macb_queue_enst_config *enst_queue;
-	struct tc_taprio_sched_entry *entry;
+	struct ethtool_link_ksettings kset = {};
 	struct macb *bp = netdev_priv(netdev);
-	struct ethtool_link_ksettings kset;
+	struct tc_taprio_sched_entry *entry;
 	struct macb_queue *queue;
 	u32 queue_mask;
 	u8 queue_id;
-- 
2.53.0

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

[Aleksei Sviridkin]

speed is a u32, so SPEED_UNKNOWN arrives as 0xffffffff and passes the
"speed <= 0" check, which only ever catches zero. That is what an
autonegotiating link reports while it is down: the limit derived from
the speed collapses to a nanosecond at most and the first entry fails
with a misleading "exceeds hardware limit". Zero stays covered, it is
what an interface that was never opened reports, and
enst_max_hw_interval() divides by it. Say which case it was in the
error.

Fixes: 89934dbf169e ("net: macb: Add TAPRIO traffic scheduling support")
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
ethtool_validate_speed() accepts SPEED_UNKNOWN (and zero), so it cannot
replace this check.

v2: error text and message per Théo Lebrun.

 drivers/net/ethernet/cadence/macb_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 61838084989a..202bc688978c 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4324,8 +4324,8 @@ static int macb_taprio_setup_replace(struct net_device *netdev,
 	}
 
 	speed = kset.base.speed;
-	if (unlikely(speed <= 0)) {
-		netdev_err(netdev, "Invalid speed: %d\n", speed);
+	if (unlikely(speed == SPEED_UNKNOWN || !speed)) {
+		netdev_err(netdev, "Invalid speed %d, link-down?\n", speed);
 		return -EINVAL;
 	}
 
-- 
2.53.0

Re: [PATCH net v2 1/2] net: macb: zero the link settings taprio reads back

[Paolo Abeni]

On 9/3/26 2:36 PM, Aleksei Sviridkin wrote:
> macb_taprio_setup_replace() calls phylink_ethtool_ksettings_get() with
> an uninitialised kset, and kset is not only an out-parameter. On a
> fixed link, or an in-band link with no PHY, phylink writes speed and
> duplex only if kset->base.rate_matching already reads RATE_MATCH_NONE,
> a field it never writes itself; in PHY mode before the PHY is attached
> it writes port and supported and nothing more. Either way the speed
> read back afterwards can be stack garbage. The ethtool core zeroes the
> structure on every path into the op, which is why its callers never
> see this; taprio is the only in-kernel caller passing its own variable.
> 
> Fixes: 89934dbf169e ("net: macb: Add TAPRIO traffic scheduling support")
> Assisted-by: LLM

Not blocking the series for this, but note that you should actually
specify the model you used.

/P

Re: [PATCH net v2 0/2] net: macb: fix the link speed the taprio setup reads

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Thu,  3 Sep 2026 12:36:50 +0000 you wrote:
> Two small fixes in macb_taprio_setup_replace(), both in how it obtains
> the link speed it scales the schedule with.
> 
> The first: it hands phylink_ethtool_ksettings_get() a stack variable
> it never zeroed, while phylink fills only what the link mode provides
> and even reads one field back from the caller. The second: the speed
> check is written as "<= 0" on a u32, so SPEED_UNKNOWN passes it and
> turns into a 1 ns hardware limit that every entry then exceeds.
> 
> [...]

Here is the summary with links:
  - [net,v2,1/2] net: macb: zero the link settings taprio reads back
    https://git.kernel.org/netdev/net/c/0523d5c52a45
  - [net,v2,2/2] net: macb: reject an unknown link speed in the taprio setup
    https://git.kernel.org/netdev/net/c/2b6c0e25a3d7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net v2 1/2] net: macb: zero the link settings taprio reads back

[Aleksei Sviridkin]

On Tue, Sep 08, 2026 at 11:44:55AM +0200, Paolo Abeni wrote:
> Not blocking the series for this, but note that you should actually
> specify the model you used.

I took the bare form from coding-assistants.rst, which asks for
"Assisted-by: LLM" since 816d9992d9ed ("coding-assistants: simplify
attribution"). The reasoning there makes sense to me: naming the model
advertises the vendor and says little about the patch. Does netdev have
its own rule on top of that?

The other half I am unsure about is the routing. The work goes through a
router, so an honest tag on one of these patches would name seven models
from three vendors. How is that meant to be written?

Re: [PATCH net v2 1/2] net: macb: zero the link settings taprio reads back

[Paolo Abeni]

On 9/8/26 5:32 PM, Aleksei Sviridkin wrote:
> On Tue, Sep 08, 2026 at 11:44:55AM +0200, Paolo Abeni wrote:
>> Not blocking the series for this, but note that you should actually
>> specify the model you used.
> 
> I took the bare form from coding-assistants.rst, which asks for
> "Assisted-by: LLM" since 816d9992d9ed ("coding-assistants: simplify
> attribution"). 
I actually I had to sync-up my own knowledge, sorry for the noise.

/P