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  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:	Tue, 22 May 2007 11:51:23 -0500
From:	Milton Miller <>
To:	"Kok, Auke" <>
Cc:	Jeff Garzik <>, Scott Feldman <>,,
	Jeff Kirsher <>,
	John Ronciak <>,
	Jesse Brandeburg <>,, David Acker <>
Subject: Re: [PATCH] fix e100 rx path on ARM (was [PATCH] e100 rx: or s and el bits)

On May 21, 2007, at 12:45 PM, Kok, Auke wrote:

> Milton Miller wrote:
>> On May 18, 2007, at 12:11 PM, David Acker wrote:
>>> Kok, Auke wrote:
>>>> First impression just came in: It seems RX performance is dropped 
>>>> to 10mbit. TX is unaffected and runs at 94mbit/tcp, but RX the new 
>>>> code seems to misbehave and  fluctuate, dropping below 10mbit after 
>>>> a few netperf runs and staying there...
>>>> ideas?
>>> I found the problem.  Another casualty of working with two different 
>>> kernels at once...arg.
>>> The blank rfd needs to have its el-bit clear now.  Here is the new 
>>> and improved patch.
>> Proceeding with the review:
>> Coding style:
>> (1) if body on seperate line.
>> (2) space after if before (
>> (3) The other enums in this driver are not ALL_CAPS
>> (4) This driver doesn't do CONSTANT != value but value != enum
>>      (see nic->mac for examples)
> I sent Milton my copy of this patch which has these style issues 
> corrected and
> applies cleanly to a recent git tree. If anyone else specifically 
> wants a copy
> let me know.
> Auke

It addressed 1 and 2, and applies, but did not address 3 and 4.

But the bigger point is it didn't address the holes I identified.

I think we need to change the logic to reclaim the size from 0
only if we are restarting, and make rx_indicate look ahead to
rx->next if it encounters a !EL size 0 buffer.  Without this we
are doing a "prohibited" rx_start to a running machine.  The
device can still see this size 0 !EL state.  Also we will get
stuck when the device finds the window between the two writes.

We can remove some register pressure by finding old_before_last_rfd
when we are ready to use it, just comparing old_before_last_rx
to new.

Also, as I pointed out, the rx_to_start change to start_reciever
is compicated and unnecessary, as rx_to_clean can always be used
and it was the starting point before the changes.

As far as the RU_SUSPENDED, I don't think we need it, instead
we should poll the device.

Here is my proposal:
rx_indicate can stop when it hits the packet with EL.  If it
hits a packet with size 0, look ahead to rx->next to see if
it is complete, if so complete this one otherwise leave it
as next to clean.  After the rx_indicate loop, try to allocate
more skbs.  If we are successful, then fixup the before-next
as we do now.  Then check the device status for RNR, and if
its stopped then set rx_to_clean rfd size back to ETH_LEN
and restart the reciever.

This does have a small hole: if we only add one packet at
a time we will end up with all size 0 descriptors in the
lopp.   We can detect that and not remove EL from the old
before-next unless we are restarting.  That would imply
moving the status poll before we allocate the list.


To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to
More majordomo info at

Powered by blists - more mailing lists