On Sat, Jul 26, 2008 at 03:21:42PM +0200, Patrick McHardy wrote:

David Miller wrote:

quoted

From: Jarek Poplawski <redacted>
Date: Fri, 25 Jul 2008 11:37:57 +0000

quoted

On Fri, Jul 25, 2008 at 03:57:12AM -0700, David Miller wrote:

quoted

Even TCP depends upon that NET_XMIT_* return value having some real
meaning, remember the HTB bug we tracked down last week? :-)

Do you mean this bug you've forgotten to fix yet?

I have not forgotten, it is in TODO list. :)

I want to also inquire with Patrick about whether he is going to
perform the audit in this area which he said he wanted to do :-)

I'm a bit backlogged, so it will take some more time.

quoted

Well, we have patch which fixes the problem for the tester, so we
could just merge that and at least have HTB fixed.  But we know there
are other instances of the same return value propagation problem and
Patrick's audit is sure to turn up other similar turds...

My proposal (as before) is to apply this patch now (it could hit more
people than we know), but I would do small changes yet:
- to do htb_activate() if (cl->un.leaf.q->q.qlen),
- to skip bstats.packets and .bytes updating for anything but
  NET_XMIT_SUCCESS,
so, both things just like in sch_hfsc.

The other problem that affects all qdiscs supporting actions is
TC_ACT_QUEUED/TC_ACT_STOLEN getting mapped to NET_XMIT_SUCCESS
even though the packet is not queued, corrupting upper qdiscs'
qlen counters.

Why can't we (temporarily) simply check such cl->un.leaf.q->q.qlen
before and after enqueing?

Jarek P.