[PATCH net-next] net: sched: avoid writing on noop_qdisc

[PATCH net-next] net: sched: avoid writing on noop_qdisc

[Eric Dumazet]

While noop_qdisc.gso_skb and noop_qdisc.skb_bad_txq are not used
in other places, it seems not correct to overwrite their fields
in dev_init_scheduler_queue().

noop_qdisc is essentially a shared and read-only object, even if
it is not marked as const because of some implementation detail.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/sch_generic.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 3023929852e8c4aaa4172861d2d0beff17e25f27..de1663f7d3ad6e2c06cd2e031036cef5979366a5 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -572,6 +572,18 @@ struct Qdisc noop_qdisc = {
 	.dev_queue	=	&noop_netdev_queue,
 	.running	=	SEQCNT_ZERO(noop_qdisc.running),
 	.busylock	=	__SPIN_LOCK_UNLOCKED(noop_qdisc.busylock),
+	.gso_skb = {
+		.next = (struct sk_buff *)&noop_qdisc.gso_skb,
+		.prev = (struct sk_buff *)&noop_qdisc.gso_skb,
+		.qlen = 0,
+		.lock = __SPIN_LOCK_UNLOCKED(noop_qdisc.gso_skb.lock),
+	},
+	.skb_bad_txq = {
+		.next = (struct sk_buff *)&noop_qdisc.skb_bad_txq,
+		.prev = (struct sk_buff *)&noop_qdisc.skb_bad_txq,
+		.qlen = 0,
+		.lock = __SPIN_LOCK_UNLOCKED(noop_qdisc.skb_bad_txq.lock),
+	},
 };
 EXPORT_SYMBOL(noop_qdisc);
 
@@ -1273,8 +1285,6 @@ static void dev_init_scheduler_queue(struct net_device *dev,
 
 	rcu_assign_pointer(dev_queue->qdisc, qdisc);
 	dev_queue->qdisc_sleeping = qdisc;
-	__skb_queue_head_init(&qdisc->gso_skb);
-	__skb_queue_head_init(&qdisc->skb_bad_txq);
 }
 
 void dev_init_scheduler(struct net_device *dev)
-- 
2.19.0.605.g01d371f741-goog

Re: [PATCH net-next] net: sched: avoid writing on noop_qdisc

[David Miller]

From: Eric Dumazet <edumazet@google.com>
Date: Tue,  9 Oct 2018 15:20:50 -0700

> While noop_qdisc.gso_skb and noop_qdisc.skb_bad_txq are not used
> in other places, it seems not correct to overwrite their fields
> in dev_init_scheduler_queue().
> 
> noop_qdisc is essentially a shared and read-only object, even if
> it is not marked as const because of some implementation detail.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied, thanks.