netdev - RE: [PATCH 2/2] qdisc_restart - couple of optimizations.

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]

Date:	Thu, 14 Jun 2007 09:48:09 +0530
From:	Krishna Kumar2 <krkumar2@...ibm.com>
To:	"Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@...el.com>
Cc:	davem@...emloft.net, herbert@...dor.apana.org.au, hadil@...erus.ca,
	kaber@...sh.net, kumarkr@...ux.vnet.ibm.com,
	netdev@...r.kernel.org, tgraf@...g.ch
Subject: RE: [PATCH 2/2] qdisc_restart - couple of optimizations.

Hi Peter,

Thanks for your feedback.

> > - netif_queue_stopped need not be called inside qdisc_restart as
> >   it has been called already in qdisc_run() before the first skb
> >   is sent, and in __qdisc_run() after each intermediate skb is
> >   sent (note : we are the only sender, so the queue cannot get
> >   stopped while the tx lock was got in the ~LLTX case).
>
> I somewhat disagree here.  The underlying driver can conceivably stop
> the device queue even if the stack holds the queue lock during an
> interrupt to clean Tx descriptors, and it finds it's out of them or
> needs to grab the device for whatever reason.  Granted this is a corner
> case, and the net effect would be a simple requeue of the skb, but
> checking the status of the queue at the last possible moment before
> entering the driver could alleviate the requeue in the time between
> ->dequeue() from the qdisc, and hard_start_xmit() if an event like I
> mentioned happened.

After seeing a few drivers, I understand that the rx intr (to clean
TX descriptors) can only enable the queue and not stop the queue (as
it will normally not queue any packets and only clean up the sent
ones). I don't find any way that the driver can stop the queue once
the top layer determines it is OK to send and is enqueing.

It is a wasted check for almost every packet (and IMHO opinion, for
every packet). And as you said - if a driver were written differently
to stop the queue even in the clean path, then that rare event (should
be very rare as we checked for stop_queue just a few instrutions
earlier) will result in a requeue, but the normal path is not penalized.

Thanks,

- KK

> I'm ok with it either way, especially since this is a corner case.  But
> it does need to be considered that it can happen.

-
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