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 <[email protected]> > > Cc: Jiri Pirko <[email protected]>; [email protected]; Nogah > > Frankel <[email protected]>; [email protected]; Ido Schimmel > > <[email protected]>; mlxsw <[email protected]>; [email protected]; > > [email protected] > > 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...
