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.
milton
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
