> > > This seems like the right solution, except that you shouldn't even > > > need to store a counter. Just use the one in the child qdisc. > > > BTW, I think you are justified in assuming that this counter can only > > > change in two ways: decrease when dequeue is called (but possibly by > > > more than one packet) and increase by 1 when enqueue is called. > > > And I guess also requeue. Which raises another problem, since that's > > > not called by the parent, right? > > > > No - you can't use the q.qlen of inner. It is under inner's control > > and you can't change it. And the assumption how it can change should > > not be used unless you have some rules for all qdisc how to change > > it. > > The only way I can make sense of this is that we must be > miscommunicating. I do not propose that HTB alter an SFQ q.qlen. > I propose that HTB read it. I think you were suggesting that HTB > make its own copy of it, and I was just suggesting that there's no > need for that copy. I know. And yes I propose secondary copy for classfull ones and yes there is problem reading inner's (SFQ's f.e.) q.qlen. What I do jjust now - I read SFQ's qlen. It is ok for detecting backlog because SFQ MUST ensure that it is correct. So that to be clear - I have NO problem reading backlog info. However I (and CBQ too) have problem to maintain our own qlen ! > > Somewhere in sources you can read that one of qdisc duties is > > to ensure that it's q.qlen value is correct. It doesn't restrict > > how it can change and when. > > I think it's pretty much necessary from the implementation viewpoint > to make some assumptions. In particular, you don't want to look at > all inactive leaves every time you do a dequeue, just to make sure > that they haven't somehow manufactured their own packets. Once the > queue is empty you expect it to stay empty until you call enqueue, > right? right. What you know: - each qdisc's (own) qlen is valid at all (for us important) times - qdisc's qlen can increase only after successfull (!) enqueue These are only guaranteed ones which I've seen described in sources. But decrease can come from dequeue, drop (it is reality now) and nothing prevent qdisc to drop some other packets (if it updates qlen accordingly) - that's what your "old dropper" wants to do. Consequences below .. > > I'd suggest for each leaf to maintain last_qlen variable which would > > start with 0 (also q.qlen will be 0). Then time to time (in enqueue, > > dequeue, drop ....) you can "repair" your own q.qlen by doing: > > q.qlen += cl->q.qlen - cl->last_qlen; cl->last_qlen = cl->q.qlen; > > for each leaf cl. > > Hence xl->last_qlen tells you what part of total q.qlen is "owned" by > > cl. > > Simple and robust, isn't it ? > > I don't get it. A qdisc is already supposed to maintain its qlen. > You don't think that's being done correctly now? Why do you want this > other data? You're proposing something that will require changing > every existing qdisc? Or is this the class (in htb code) trying to > keep a copy of the data in its qdisc? If the latter, then I guess I > don't object (since it's only in your code and transparent to me), but > I still don't see why you need it. Qdisc's Q qlen is: Q.qlen = Q.extra_qlen + sum over all children ch (ch.qlen) Q.extra_qlen are skb's in Q's own queues (HTB's direct queue, SFQ's hash, PFIFO's main queue, TBF's queue, CBQ has no such one). Because Q.enqueue enqueues packet into its own queue of into child qdisc (and two rules above hold) you can simply increase Q.qlen in case of Q.enqueue. But say this scenario: HTB.enqueue, skb goes to SFQ's queue, HTB.qlen = SFQ.qlen = 1 [1] HTB.enqueue, skb goes to SFQ's queue, HTB.qlen = SFQ.qlen = 2 [2] time passed HTB.enqueue, skb goes to SFQ's queue, SFQ decides to drop two too old skb's, HTB.qlen = 3, SFQ.qlen = 1 [3] Now HTB.qlen is bad! Here is I would maintain copy of SFQ's qlen [in brackets] I'd catch that SFQ.qlen decreased by two (under my hands) because 1-[3]=-2 and I can correct HTB.qlen accordingly to 1. It needs to be done only in equrur,dequeue and drop. If some inner qdisc changes in time between, parent qdisc will ignore this change in backlog size until next dequeue/enqueue of that class. > > > I like those, but also suggested that HTB would be a good place to add > > > code for dropping stale packets. If you're interested in doing that I > > > should mention: > > > Not all packets come with timestamps (e.g., locally generated ones). > > > Solution: at enqueue, if the timestamp is 0, set it to the current > > > time. > > > > It would be better done in fifo/sfq. It is because HTB can do it > > only when dequeuing and at that time some packets were already dropped > > by SFQ because if has had queues full of "old" packets. > > I see, you suggest that I could do better if enqueue started with > something like > while (oldest packet expired) drop oldest packet > Of course this is still only an approximation. There will still be > times when you have to drop a packet but it turns out that, before the > next one is dequeued another packet will have expired (so if you only > knew that you could have saved the packet you dropped before). yes somethink like :) If we would maintain these copies above then you could drop any number of packets. It would be helpful to drop all of these in both dequeue & enqueue to assure that you will not dequeue old packet if too much time passes between last enqueue and dequeue. > > SFQ should maintain pqio queue of all packets and look at head of > > the list sometimes to keep its queues frre of old packets and thus > > to have room for new ones. > > I suppose SFQ could still do something like above even if HTB were > dropping expired packets at dequeue. And this would save all qdiscs > the work of dropping expired packets at dequeue, which is still needed > even if you also check at enqueue. Also, only dropping at dequeue is as I write above, SFQ should do it at both sides .. > a good approximation to what you want in the common case where the > queue size is too large for the low rate assigned to the class. I > argue it's much better to do this in a few classful qdiscs than to ask > for it to be done by all the others. I agree. But as I said before - it will lead to dropping new packets due to queue full of old packets. I'm not sure whether it has any benefit then. Simply hack this into HTB, measure and if it will make something better the it is worth of implementing. I'm not convinced about it. With fixed queue size and minimal assured rate you always know how old packet can be in the queue ... devik
