alexforencich / verilog-ethernet

Verilog Ethernet components for FPGA implementation
MIT License
2.29k stars 703 forks source link

UDP data loss after storing the header. The data is not getting stored in the temp buffer even if the ready is enabled. #105

Open fpgapsyc opened 2 years ago

fpgapsyc commented 2 years ago
udp_data_skip

@alexforencich @lomotos10

alexforencich commented 2 years ago

I see no issues in the trace. Going to need some more context.

Edit: Ah, maybe the first byte is getting dropped (tready low, tvalid high, tdata changes). This is coming out of the core UDP stack?

fpgapsyc commented 2 years ago

Yes. There is a byte missing. The output is from the udp core. When the tready is low, the data has to be stored in a temp buffer and then put that temp data to output when tready is high. But that is not happening here.

alexforencich commented 2 years ago

I think I might have fixed the problem; try again with the latest git version and let me know if you're still seeing the same issue.

fpgapsyc commented 2 years ago

Dear alex,

The issue is not yet fixed. Please note the attached image, you can see that store_udp_payload_axis_temp_to_output is not high due to which we are having a byte loss.

udp_Screenshot 2022-01-01 193449

the s_ip_payload_axis_tready_reg is not getting low in the next cyxle after the s_ip_payoad_axis_tready_next. Please check the supporting image for the mentioned.

udp_Screenshot 2022-01-01 194227

Thanks

alexforencich commented 2 years ago

What the heck, s_ip_payload_axis_tready_next goes low, but s_ip_payload_axis_tready_reg does not follow one cycle later like it's supposed to.

alexforencich commented 2 years ago

Is this an ILA trace or a simulation trace? If it's an ILA trace, did Vivado report any timing violations? If it's a simulation trace, how are you generating the clock signal in the testbench?

fpgapsyc commented 2 years ago

Its a Simulation trace. I am using xilinx gigabit ethernet ip. That ip is generating the required axis clock which i am feeding to stack. In Ip_rx its working as it should, but in udp it is not.

alexforencich commented 2 years ago

Where is m_udp_payload_axis_tready coming from?

fpgapsyc commented 2 years ago

I am using a example design of vcu118 where the udp packet is being looped back. I even tried tieing the ready pin with 1. The same issue occured.

alexforencich commented 2 years ago

Unmodified example design? Interesting, I will take a closer look at how the loopback logic is set up. So far it seems to be some sort of simulation race condition, possibly it is some difference between icarus verilog and the Vivado simulator, but that could indicate that some portion of the HDL needs some reworking.

fpgapsyc commented 2 years ago

No. The loopback logic which checks for the udp source port and loopsback using the fifo is taken without any modifications. I have put them in the top file where i have made input and output declarations which will connect to the ethernet phy. The RGMII interface and phy reset signals. But for the loopback, the logic is used as it is from the example design. I am sending a sample packet from my pattern generator to the gigabit ip through transmit path and the receiver path is connected to udp stack.

fpgapsyc commented 2 years ago

The ip_rx module also have the same logic similar to the udp_rx except for the header decoding logic. The ip_rx is working good but udp_rx didnt. I am not sure why that happens. Please let me know if i am doing any mistake in simulation of the code.

alexforencich commented 2 years ago

Please try replacing

assign m_udp_payload_axis_tready_int_early = m_udp_payload_axis_tready || (!temp_m_udp_payload_axis_tvalid_reg && (!m_udp_payload_axis_tvalid_reg || !m_udp_payload_axis_tvalid_int));

with

assign m_udp_payload_axis_tready_int_early = !temp_m_udp_payload_axis_tvalid_reg && (!m_udp_payload_axis_tvalid_reg || m_udp_payload_axis_tready);

and let me know if that changes anything.

fpgapsyc commented 2 years ago

No, there is no change. still tready_reg does not go low after the tready_next goes low.

udpScreenshot 2022-01-02 031046
alexforencich commented 2 years ago

Well, in that case I think I need to be able to run your testbench on my end to figure out what the simulation issue is. Please see if you can replicate the issue with the unmodified example design, that way all you need to do is send me the top-level testbench that I can run in Vivado.

fpgapsyc commented 2 years ago

I will try to replicate the issue using the unmodified example design. So that i can provide the top level file.

fpgapsyc commented 2 years ago

I replicated only the UDP complete block with the loopback function enabled, that was able provide the output properly. But in the replicated design, the udp_hdr_reg is high untill the tlast of the udp payload. But in the integrated design, the hdr_valid is high for only one cycle and goes low

image for the hdr valid untill the tlast of udp payload

udp_hdr_valid_tlast

image for the hdr valid untill the tlast of udp payload (integrated design)

udp_hdr_valid_1_cycle