OpenCores

* Ethernet MAC 10/100 Mbps

Issue List
Race condition in eth_wishbone.v #4
Open ocghost opened this issue almost 17 years ago
ocghost commented almost 17 years ago

I am using the avalon component from MaCo in a NIOS II system on an altera EP2C20.

I observed corrupted ethernet frames in memory received by the opencores ethernet mac. I changed my driver to fill each Rx buffer with 0xe5 prior to handing it over to the MAC and to do the CRC check in software after a frame is received. I observed, that the data was stored at BdAddress+4, one Dword off towards higher memory locations.

I traced this problem down to being a race condition in eth_wishbone.v. After writing the last word of data to an rx buffer, the status is written to the BD an an new BD is read.

When the writing of the last dword to memory takes longer than it takes to start the new BD, the final m_wb_ack_i increments the BD address of the new BD. :-(

I changed the following assignment:

old: assign StartRxPointerRead = RxBDRead & RxBDReady; new: assign StartRxPointerRead = RxBDReady & ~MasterWbRX;

MasterWbRX is active, while the RX DMA engine is writing data to the buffer. This way, reading the new buffers address is delayed until the pending bus transaction has finished.

But now I get BUSY interupts! :-O

BUSY – Busy This bit indicates that a buffer was received and discarded due to a lack of buffers. It is cleared by writing 1 to it. This bit appears regardless to the IRQ bits in the Receive or Transmit Buffer Descriptors.

My guess is, that it now takes too long to read the new BD, so the new BD is not yet ready, when the Rx has new data. Shouldn't the Rx FIFO resolve this situation?

I have no idea, what to do.

Thanks in advance,

Mario

ocghost commented almost 17 years ago

I ran into the same problem before. I made the following change and it seemed working for me.

in eth_wishbone.v

change: assign RxStatusWrite = ShiftEnded & RxEn & RxEn_q;

to: assign RxStatusWrite = ShiftEnded & RxEn & RxEn_q & ~m_wb_stb_o;

Basically it delays loading the new buffer pointer until the last word of the previous frame is acknowledged. Let me know if it works for you.

Jun

galland commented over 15 years ago

I'm running into the same problem, though with my code, what happens is that the first two 32bit words are not written to the RX buffer.

About what you mention with your asignment, there is no easy way, in this code, to determine when the wishbone transferences have ended. If you see the code: it is RxBufferAlmostEmpty (line 1212) the FIFO signal that triggers ShiftEnded (line 2266), which triggers RxStatusWrite (1967), which triggers StartRxBDRead (1837), which triggers RxBDRead (1845), which triggers RxBDReady (1862), which triggers StartRXPointerRead (1898), which triggers RXPointerRead (1906), which FINALLY triggers RxReady (1890) and the update of the address RxPointerMSB (1921) which finally drives me crazy...

So we find that it is RxBufferAlmostEmpty the one that is triggering the final change in the address being written to!! So BD writing is independent of the data transfer itself! This obviously leads to the problems we are facing. But who makes MasterWbRX go down? If you take a look at the casex in line 1045, it is WriteRxDataToMemory who stops the WB operations, and this signal is assigned, in line 2215, to ~RxBufferEmpty, which should be the one that triggered all the previous stuff, in order to make BD writing and data transfer dependent as it should be.

MARIO: The problem with the solution proposed you propose is that, eliminating RxBDRead you make StartRxPointerRead be continuously at '1' instead of rising for only one cycle (which was obtained thanks to RxBDRead that was driven down after RxBDReady became 1) that finally causes, among other things, that RxPointerMSB is never incremented and the signal RxReady doesn't stay at '0' when the next packet is received and thus causes major malfunction.

JUN: The solution you comment looks pretty logical to me, I see that m_wb_stb_o is driven low, with cyc (inside the casex of line 1045), when the FIFO is empty. I had doubts about the time in which ShiftEnded was at '1' (line 2266) but it maintains the level until RxStatusWrite is activated. I'll try it out and see if it solves my thing. Thanks!

I think the way the code has been ordered, though very elegant, is not the best to make it easily understandable (though I am not the right person to complain about that). Anyways I think this is a quality core.

galland commented over 15 years ago

As Jun said, the solution that effectively works is to change: assign RxStatusWrite = ShiftEnded & RxEn & RxEn_q; to: assign RxStatusWrite = ShiftEnded & RxEn & RxEn_q & ~m_wb_stb_o;

Project maintainers please check it and update the code as many people are having this problem.


Assignee
No one
Labels
Bug