[PATCH net-next] net/sched: taprio: prepare taprio_dump() for RTNL removal

[PATCH net-next] net/sched: taprio: prepare taprio_dump() for RTNL removal

[Eric Dumazet]

We soon will no longer hold RTNL in qdisc dumps.

Add READ_ONCE()/WRITE_ONCE() annotations.

Note: taprio already uses RCU to protect most of its fields.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/sch_taprio.c | 42 ++++++++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 45245157e00a69311304f8c69f9ee9d80bc4f4d8..71b690e1974dad8fbab7e12998e03f86a0847a98 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -308,10 +308,10 @@ static void taprio_update_queue_max_sdu(struct taprio_sched *q,
 
 		if (max_sdu != U32_MAX) {
 			sched->max_frm_len[tc] = max_sdu + dev->hard_header_len;
-			sched->max_sdu[tc] = max_sdu;
+			WRITE_ONCE(sched->max_sdu[tc], max_sdu);
 		} else {
 			sched->max_frm_len[tc] = U32_MAX; /* never oversized */
-			sched->max_sdu[tc] = 0;
+			WRITE_ONCE(sched->max_sdu[tc], 0);
 		}
 	}
 }
@@ -1771,8 +1771,8 @@ static int taprio_parse_tc_entries(struct Qdisc *sch,
 	}
 
 	for (tc = 0; tc < TC_QOPT_MAX_QUEUE; tc++) {
-		q->max_sdu[tc] = max_sdu[tc];
-		q->fp[tc] = fp[tc];
+		WRITE_ONCE(q->max_sdu[tc], max_sdu[tc]);
+		WRITE_ONCE(q->fp[tc], fp[tc]);
 		if (fp[tc] != TC_FP_EXPRESS)
 			have_preemption = true;
 	}
@@ -1852,12 +1852,14 @@ static int taprio_change(struct Qdisc *sch, struct nlattr *opt,
 		return -EINVAL;
 	}
 
-	if (q->flags != TAPRIO_FLAGS_INVALID && q->flags != taprio_flags) {
-		NL_SET_ERR_MSG_MOD(extack,
-				   "Changing 'flags' of a running schedule is not supported");
-		return -EOPNOTSUPP;
+	if (q->flags != taprio_flags) {
+		if (q->flags != TAPRIO_FLAGS_INVALID) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Changing 'flags' of a running schedule is not supported");
+			return -EOPNOTSUPP;
+		}
+		WRITE_ONCE(q->flags, taprio_flags);
 	}
-	q->flags = taprio_flags;
 
 	/* Needed for length_to_duration() during netlink attribute parsing */
 	taprio_set_picos_per_byte(dev, q, extack);
@@ -1940,7 +1942,8 @@ static int taprio_change(struct Qdisc *sch, struct nlattr *opt,
 			goto unlock;
 		}
 
-		q->txtime_delay = nla_get_u32(tb[TCA_TAPRIO_ATTR_TXTIME_DELAY]);
+		WRITE_ONCE(q->txtime_delay,
+			   nla_get_u32(tb[TCA_TAPRIO_ATTR_TXTIME_DELAY]));
 	}
 
 	if (!TXTIME_ASSIST_IS_ENABLED(q->flags) &&
@@ -2283,8 +2286,8 @@ static int dump_schedule(struct sk_buff *msg,
 }
 
 static int taprio_dump_tc_entries(struct sk_buff *skb,
-				  struct taprio_sched *q,
-				  struct sched_gate_list *sched)
+				  const struct taprio_sched *q,
+				  const struct sched_gate_list *sched)
 {
 	struct nlattr *n;
 	int tc;
@@ -2298,10 +2301,11 @@ static int taprio_dump_tc_entries(struct sk_buff *skb,
 			goto nla_put_failure;
 
 		if (nla_put_u32(skb, TCA_TAPRIO_TC_ENTRY_MAX_SDU,
-				sched->max_sdu[tc]))
+				READ_ONCE(sched->max_sdu[tc])))
 			goto nla_put_failure;
 
-		if (nla_put_u32(skb, TCA_TAPRIO_TC_ENTRY_FP, q->fp[tc]))
+		if (nla_put_u32(skb, TCA_TAPRIO_TC_ENTRY_FP,
+				READ_ONCE(q->fp[tc])))
 			goto nla_put_failure;
 
 		nla_nest_end(skb, n);
@@ -2387,6 +2391,7 @@ static int taprio_dump(struct Qdisc *sch, struct sk_buff *skb)
 	struct sched_gate_list *oper, *admin;
 	struct tc_mqprio_qopt opt = { 0 };
 	struct nlattr *nest, *sched_nest;
+	u32 txtime_delay;
 
 	mqprio_qopt_reconstruct(dev, &opt);
 
@@ -2404,14 +2409,15 @@ static int taprio_dump(struct Qdisc *sch, struct sk_buff *skb)
 	if (q->flags && nla_put_u32(skb, TCA_TAPRIO_ATTR_FLAGS, q->flags))
 		goto options_error;
 
-	if (q->txtime_delay &&
-	    nla_put_u32(skb, TCA_TAPRIO_ATTR_TXTIME_DELAY, q->txtime_delay))
+	txtime_delay = READ_ONCE(q->txtime_delay);
+	if (txtime_delay &&
+	    nla_put_u32(skb, TCA_TAPRIO_ATTR_TXTIME_DELAY, txtime_delay))
 		goto options_error;
 
 	rcu_read_lock();
 
-	oper = rtnl_dereference(q->oper_sched);
-	admin = rtnl_dereference(q->admin_sched);
+	oper = rcu_dereference(q->oper_sched);
+	admin = rcu_dereference(q->admin_sched);
 
 	if (oper && taprio_dump_tc_entries(skb, q, oper))
 		goto options_error_rcu;
-- 
2.54.0.545.g6539524ca2-goog

Re: [PATCH net-next] net/sched: taprio: prepare taprio_dump() for RTNL removal

[Eric Dumazet]

On Thu, Apr 30, 2026 at 11:42 PM Eric Dumazet <edumazet@google.com> wrote:
>
> We soon will no longer hold RTNL in qdisc dumps.
>
> Add READ_ONCE()/WRITE_ONCE() annotations.
>
> Note: taprio already uses RCU to protect most of its fields.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

I think sashiko feedback is a bit wrong.

<quote>
Should the reads of q->flags in taprio_dump() also be annotated, given
that the write side in taprio_change() is now using WRITE_ONCE()?
The same expression reads q->flags twice:
if (q->flags && nla_put_u32(skb, TCA_TAPRIO_ATTR_FLAGS, q->flags))
and the earlier check in taprio_dump() also does a plain read:
if (!FULL_OFFLOAD_IS_ENABLED(q->flags) &&
nla_put_s32(skb, TCA_TAPRIO_ATTR_SCHED_CLOCKID, q->clockid))
Without an annotation, is the compiler free to reload q->flags between
the guard and the nla_put_u32() argument, so the guard could observe one
value while the emitted attribute reflects a different one once RTNL is
dropped from the dump path?

</quote>

My answer: q->flags can not be changed on a live taprio qdisc, it is
set once (and the logic is even visible in the patch diff)

<quote>
On a related note, q->clockid is read in taprio_dump() as:
nla_put_s32(skb, TCA_TAPRIO_ATTR_SCHED_CLOCKID, q->clockid)
and written plainly in taprio_parse_clockid() (q->clockid = clockid;)
and in taprio_init() (q->clockid = -1;).
Is q->clockid intentionally left out of this preparation, or should it
get READ_ONCE()/WRITE_ONCE() treatment along with q->flags and
q->txtime_delay? If it is intentional, would a note in the commit
message help explain why clockid is exempt?
</quote>

Same answer.  q->clockid can not change on a live taprio qdisc.

Re: [PATCH net-next] net/sched: taprio: prepare taprio_dump() for RTNL removal

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri,  1 May 2026 06:42:47 +0000 you wrote:
> We soon will no longer hold RTNL in qdisc dumps.
> 
> Add READ_ONCE()/WRITE_ONCE() annotations.
> 
> Note: taprio already uses RCU to protect most of its fields.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> 
> [...]

Here is the summary with links:
  - [net-next] net/sched: taprio: prepare taprio_dump() for RTNL removal
    https://git.kernel.org/netdev/net-next/c/c4994aee0292

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