Re: [PATCH net] net/sched: fix netdevice reference leaks in attach_one_default_qdisc()

[Jakub Kicinski]

On Thu, 25 Aug 2022 20:29:21 +0800 wanghai (M) wrote:
> 在 2022/8/19 23:58, wanghai (M) 写道:
> > Here are the details of the failure. Do I need to do cleanup under the 
> > failed path?
> >
> > If a dev has multiple queues and queue 0 fails to attach qdisc
> > because there is no memory in attach_one_default_qdisc(). Then
> > dev->qdisc will be noop_qdisc by default. But the other queues
> > may be able to successfully attach to default qdisc.
> >
> > In this case, the fallback to noqueue process will be triggered
> >
> > static void attach_default_qdiscs(struct net_device *dev)
> > {
> >     ...
> >     if (!netif_is_multiqueue(dev) ||
> >         dev->priv_flags & IFF_NO_QUEUE) {
> >             ...
> >             netdev_for_each_tx_queue(dev, attach_one_default_qdisc, 
> > NULL); // queue 0 attach failed because -ENOBUFS, but the other queues 
> > attach successfully
> >             qdisc = txq->qdisc_sleeping;
> >             rcu_assign_pointer(dev->qdisc, qdisc); // dev->qdisc = 
> > &noop_qdisc
> >             ...
> >     }
> >     ...
> >     if (qdisc == &noop_qdisc) {
> >         ...
> >         netdev_for_each_tx_queue(dev, attach_one_default_qdisc, NULL); 
> > // Re-attach, but not release the previously created qdisc
> >         ...
> >     }
> > }
>
> Do you have any other suggestions for this patch? Any replies would be 
> appreciated.

Sorry for the silence and thanks for a solid explanation!

Can we do a:

netdev_for_each_tx_queue(dev, shutdown_scheduler_queue, &noop_qdisc);

before trying to re-attach, to clear out any non-noop qdisc that may
have gotten assigned?