Re: [LARTC] [HTB] htb_dequeue_tree assertion (kernel 2.4.21-ac4)

[devik <devik@cdi.cz>]

If you read comment above htb_dequeue_tree, it should be called
only when it is sure that there are packets inside of the level/prio.
It is known by other HTB mechanism (per-level activity lists).

Thus the bugtrap is to catch case where class was inserted
into activity list because it had packets in its sub-qdisc
but when we actually decide to dequeue - it has no packet.
It is weird - can qdisc lose packets even when dequeue was
not called ??

-------------------------------
    Martin Devera aka devik
Linux kernel QoS/HTB maintainer
  http://luxik.cdi.cz/~devik/

On Fri, 18 Jul 2003, Wilfried Weissmann wrote:

> Hello,
>
> I think the BUG_TRAP() in the htb_dequeue_tree() is wrong. First it
> checks if the class pointer "cl" is NULL, which is obviously right. But
> I do not understand why we also check whenever the queue length of the
> leaf queue is zero "cl->un.leaf.q->q.qlen". I would have put that in the
> expression of the "if" statements that comes afterwards. A queue length
> of 0 is not an error condition that should be reported (please, correct
> me if I misunderstood the code).
> I can pretty much reliably trigger the assertion with a well utilized
> gigabit ethernet link when I flush and reactivate the TC configuration
> every 3 seconds. It looks like the error occurs only when confiuration
> changes are made.
> I will some some more tests on monday when I am back at the office to
> verify that the queue length is (not) the problem.
>
> bye,
> wilfried
>
> static struct sk_buff *
> htb_dequeue_tree(struct htb_sched *q,int prio,int level)
> {
> 	struct sk_buff *skb = NULL;
> 	//struct htb_sched *q = (struct htb_sched *)sch->data;
> 	struct htb_class *cl,*start;
> 	/* look initial class up in the row */
> 	start = cl = htb_lookup_leaf (q->row[level]+prio,prio,q->ptr[level]+prio);
>
> 	do {
> 		BUG_TRAP(cl && cl->un.leaf.q->q.qlen); if (!cl) return NULL;
> 		HTB_DBG(4,1,"htb_deq_tr prio=%d lev=%d cl=%X defic=%d\n",
> 				prio,level,cl->classid,cl->un.leaf.deficit[level]);
>
> 		if (likely((skb = cl->un.leaf.q->dequeue(cl->un.leaf.q)) != NULL))
> 			break;
> 		if (!cl->warned) {
>
> _______________________________________________
> LARTC mailing list / LARTC@mailman.ds9a.nl
> http://mailman.ds9a.nl/mailman/listinfo/lartc HOWTO: http://lartc.org/
>
>

_______________________________________________
LARTC mailing list / LARTC@mailman.ds9a.nl
http://mailman.ds9a.nl/mailman/listinfo/lartc HOWTO: http://lartc.org/