On Thu, 17 Dec 2020 21:32:29 +0100 Davide Caratti wrote:

hello Jakub, and thanks for checking!

On Thu, 2020-12-17 at 11:05 -0800, Jakub Kicinski wrote:

quoted

On Wed, 16 Dec 2020 19:33:29 +0100 Davide Caratti wrote:  

quoted

+	if (q->qdiscs) {
+		for (i = 0; i < dev->num_tx_queues && q->qdiscs[i]; i++)
+			qdisc_reset(q->qdiscs[i]);  

Are you sure that we can't graft a NULL in the middle of the array?  

that should not happen, because child qdiscs are checked for being non-
NULL when they are created:

https://elixir.bootlin.com/linux/v5.10/source/net/sched/sch_taprio.c#L1674

and then assigned to q->qdiscs[i]. So, there might be NULL elements of
q->qdiscs[] in the middle of the array when taprio_reset() is called,
but it should be ok to finish the loop when we encounter the first one:
subsequent ones should be NULL as well.

Right, but that's init, look at taprio_graft(). The child qdiscs can be
replaced at any time. And the replacement can be NULL otherwise why
would graft check "if (new)"