netdev - Re: [RFC] [PATCH] Don't run __qdisc

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: <20090724083749.GA23521@gondor.apana.org.au>
Date:	Fri, 24 Jul 2009 16:37:49 +0800
From:	Herbert Xu <herbert@...dor.apana.org.au>
To:	Jarek Poplawski <jarkao2@...il.com>
Cc:	Krishna Kumar <krkumar2@...ibm.com>, davem@...emloft.net,
	netdev@...r.kernel.org
Subject: Re: [RFC] [PATCH] Don't run __qdisc_run() on a stopped TX queue

On Fri, Jul 24, 2009 at 08:30:06AM +0000, Jarek Poplawski wrote:
> On 20-07-2009 09:46, Krishna Kumar wrote:
> > From: Krishna Kumar <krkumar2@...ibm.com>
> > 
> > Driver sends an skb and stops the queue if no more space is
> > available on the device, and returns OK. When dev_queue_xmit
> > runs for the next skb, it enqueue's the skb & calls qdisc_run.
> > qdisc_run dequeue's the skb and finds the queue is stopped
> > after getting the HARD_LOCK_TX, and puts the skb back on
> > gso_skb (the next iteration fails faster at dequeue_skb).
> > 
> > This patch avoids calling __qdisc_run if the queue is stopped.
> > This also lets us remove the queue_stopped check in dequeue_skb
> > and in qdisc_restart.
> 
> It was designed wrt. multiqueue handling mainly, were such a check
> isn't enough at this place.

Well multiqueue TX was designed so that each CPU could have its
own queue.  While this isn't always the case due to resource
constraints and legacy applications, we should optimise for the
case where each CPU is using its own TX queue.

Of course we should still maintain correctness should the unexpected
occur.

So while I agree with Krishna's patch in principle, someone needs to
make sure that it still does the right thing (i.e., it doesn't
introduce potential starvation) if one CPU transmits to someone
else's queue.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
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