On Fri, 12 Jan 2018 09:29:22 +0000 Nogah Frankel wrote: > > -----Original Message----- > > From: Yuval Mintz > > Sent: Friday, January 12, 2018 10:47 AM > > To: Jakub Kicinski <kubak...@wp.pl> > > Cc: Jiri Pirko <j...@resnulli.us>; netdev@vger.kernel.org; Nogah > > Frankel <nog...@mellanox.com>; da...@davemloft.net; Ido Schimmel > > <ido...@mellanox.com>; mlxsw <ml...@mellanox.com>; j...@mojatatu.com; > > xiyou.wangc...@gmail.com > > Subject: RE: [patch net-next 5/5] mlxsw: spectrum: qdiscs: Support > > stats for PRIO qdisc > > > > > Hm. You you need this just because you didn't add the backlog > > > > > pointer to destroy? AFAIK on destroy we are free to reset > > > > > stats as well, thus simplifying your driver... Let me know > > > > > if I misunderstand. > > The problem of doing it in destroy is when one qdisc is replacing > another. I want to be able to destroy the old qdisc to "make room" > for the new one before I get the destroy command for the old qdisc > (that will come just after the replace command for the new qdisc). > If I am saying that the destroy changes the stats, I need to save > some data about the old qdisc till I get the destroy command for it.
Agreed, maintaining a coherent destroy behavior would be problematic when successful replace with a new qdisc (e.g. different handle) is involved :( Besides the momentary stats seem to be reset before destroy so not touching them may be in fact more correct. I need to look into the propagation done in qdisc_tree_reduce_backlog(), it worries me. If we start stacking the qdiscs (e.g. red on top of prio) it could mess with the root's backlog...
