Subject: Re: [Kgdb-bugreport] [PATCH] 8139too: harden against TX ring overflow

From: Jeff Garzik <[email-masked]>
Date: Thu, 24 May 2007 17:39:22 -0400

Sergei Shtylyov wrote:
> Hello, I wrote:
>> This driver's 4-packet deep TX queue is too sensible to the "careless" 
>> callers
>> ignoring its state (like netpoll in trapped mode), so add "queue full" 
>> check at
>> the start of the hard_start_xmit() method (only under #ifndef 
>> RTL8139_NDEBUG,
>> otherwise the queue will get stuck once dirty pointer gets out of 
>> sync); switch
>> to using appropriate mnemonics for the return values while at it.
>> Also, the out-of-sync dirty pointer check is misplaced in 
>> rtl8139_tx_interrupt()
>> which causes TX descriptors to be inspected more than once in case the 
>> pointer
>> really gets out-of-sync (and incrementing the dirty pointer always by 
>> 4 is just
>> not enough, e.g. KGDBoE managed to stuff 20+ extra buffers into the 
>> queue) --
>> place it before the loop and limit the loop to only look through 4 
>> descriptors
>> at most, so that already overwritten descriptors are just not counted.
> 
>> Signed-off-by: Sergei Shtylyov <[email-masked]>
> 
>    Jeff, do you have any opinion on this patch?

The out-of-sync dirty pointer check is leftover boilerplate, and not 
really indicative of anything except for some code to be removed.

As for the other stuff, I would say "fix the caller".  We don't need to 
hack every driver for cases where netpoll is being dumb.

	Jeff