netdev - Re: [PATCH ver2] Avoid enqueuing skb for default qdiscs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <OF6B2AD304.3AD05621-ON652576BD.0047556A-652576BD.004B1F23@in.ibm.com>
Date:	Mon, 1 Feb 2010 19:12:06 +0530
From:	Krishna Kumar2 <krkumar2@...ibm.com>
To:	mbizon@...ebox.fr
Cc:	davem@...emloft.net, herbert@...dor.apana.org.au,
	Jarek Poplawski <jarkao2@...il.com>, kaber@...sh.net,
	netdev@...r.kernel.org
Subject: Re: [PATCH ver2] Avoid enqueuing skb for default qdiscs

Hi Maxime,

Maxime Bizon <mbizon@...ebox.fr> wrote on 02/01/2010 03:22:20 PM:

> Sorry old stuff, but I just noticed this.
>
> One side effect of this patch is that tc now shows a lot of requeue for
> slow interfaces (slow dsl link in my case), because sch_direct_xmit is
> called even if device stopped its queue.

sch_direct_xmit can be called from dev_queue_xmit for a
stopped device only if the device had xmit the previous
skb, stopped the device and returned OK. Then the next
sch_d_x will dequeue and requeue the new skb. But from
now on, sch_d_x is never called from dev_queue_xmit
since qdisc_qlen(q) > 0. Instead dev_queue_xmit enqueue's
the skb and calls qdisc_restart, which calls dequeue_skb.
dequeue_skb finds there is an earlier gso_skb but since
the device is stopped, it returns NULL. Hence there
should be only one requeue (and dequeue) for every stop.

BTW, I don't think this patch would change the earlier
behavior. The old code would have done the same thing.

> My first reflex was too investigate for a misbehaving driver (returning
> TX_BUSY), but start_xmit is not even called, sch_direct_xmit notices
> that queue is stopped, and just does a dequeue/requeue.

As I explained, this should happen only once per stop event.
Could you tell which driver is having this problem? Is it
waking up too early, eg, it might be stopping when the hw tx
descriptor is full but waking up when a few slots open up,
and those will get filled up immediately on fast systems.
Then you will see a lot of requeue's.

> Since we're talking about slow interfaces, this has no impact on
> performance, the requeue counter incrementing is just a bit scary.
>
> Shouldn't we check for stopped queue earlier to avoid this ?

dev_queue_xmit is checking for qdisc_qlen == 0 before calling
sch_direct_xmit, which is enough for all stopped queue except
the first time. Also, it's not possible to check for stopped
Q earlier in qdisc_restart() due to multiqueue implementation.

Please let me know if you find something different from what
I have described.

thanks,

- KK

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html