Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LiteEthStream2UDPTX: fix for end of buffer tags #121

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hcab14
Copy link

@hcab14 hcab14 commented Nov 3, 2022

Please have a look. For it works, i.e. I can use the entire GBit ethernet bandwidth (using a butterstick).

I wonder how one could make a CI test for this.

@rowanG077
Copy link
Contributor

Why wasn't it possible to reach GBit ethernet speeds before this? If I read the code correctly this will double the bram usage. Which I'm not a fan of.

@hcab14
Copy link
Author

hcab14 commented Nov 4, 2022

It was possible to reach GBit speeds before but the last tags were not properly dealt with.

In two cases it did work as expected:

  1. last=0: the FIFO was always fully filled and then its contents were sent in one UDP packet
  2. last=1: FIFO content was sent one by one

But, e.g., if I wanted to send UDP packets with two FIFO entries, i.e., last was made to toggle it did not work because last is checked only in the IDLE state and not during the SEND state.

@rowanG077
Copy link
Contributor

rowanG077 commented Nov 4, 2022

I understand. I had this issue too.

But I think double buffering is not the correct path here due to such waste of resources. I dealt with it by building a custom FIFO and using the UDP raw interface.What I would advise here is to replace the SyncFIFO with the PacketFIFO. It should be possible to send full packets only. I didn't go this route because I'm not proficient in migen/litex.

The drawback is that the user will have to specify the length of the packet up front.

@enjoy-digital
Copy link
Owner

Thanks @hcab14, @rowanG077, while discussing with @hcab14 about the issue (on IRC IIRC), I also thought about replacing the SyncFIFO with a proper PacketFIFO. I'll try to have a closer look at this soon. We should indeed try to minimize resource usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants