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

Add ethernet udp tx/rx module #1010

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from
Open

Conversation

jjyy-Huang
Copy link

@jjyy-Huang jjyy-Huang commented Jan 10, 2023

Context, Motivation & Description

The udp project is here.

  • The goal of this PR is to create a UDP/IP Tx/Rx module with SpinalHDL for Xilinx 100GbE Ethernet-Subsystem (MRMAC/CMAC).

  • the module support following features:

    • UDP Tx/Rx without checksum
    • IP Tx/Rx
    • Ethernet Frame Layer2 (pre-add MAC adn EtherType)
    • 256b data width
    • Fully pipeline

Impact on code generation

Zero

Checklist

  • Unit tests were added
  • API changes are or will be documented:

TODO

  • Redesign Rx module control logic
  • Add ARP request module
  • Add ICMP module
  • I/O data support 384b/512b
  • Use cocotb for verification

@Dolu1990 Dolu1990 self-requested a review January 10, 2023 13:08
@jjyy-Huang jjyy-Huang marked this pull request as draft January 12, 2023 01:46

import UserConfiguration._

object rotateLeftByte {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rotate operation could be added to the Bits data type directly :)
Maybe in a more general form ?
data: Bits, bias: UInt, sliceWidth : BitCount ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ummm...

Actually, there is rotateLeft() in BitVector,

def rotateLeft(that: UInt): T = {
    ...
}

I wonder if it would be better to use switch instead of bits shift one by one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be better to use switch instead of bits shift one by one.

Hmm, the main challenge is to ensure the synthesis tool can infer a proper barrel shifter using log2up(width) stages, that's why it is done in shifts sequence.

I would say the main limitation of the current implementation is that there is now parameter to specify how many bits to rotate per unit.

@Dolu1990
Copy link
Member

Hi ^^

So, i think for the doc, as it is a specific thing with quite a few abstraction, it would need to have some on the RTD

@jjyy-Huang
Copy link
Author

Hi ^^

So, i think for the doc, as it is a specific thing with quite a few abstraction, it would need to have some on the RTD

Sorry for the late reply.

The doc will be added with more details of implementation in the next commit

@jjyy-Huang
Copy link
Author

Hi ^^
So, i think for the doc, as it is a specific thing with quite a few abstraction, it would need to have some on the RTD

I'm sorry for waiting on this document for a long time.

The following link is the doc about this module design.

https://github.com/jjyy-Huang/SpinalHDL-ethernet/blob/udp/README.md

@jjyy-Huang jjyy-Huang marked this pull request as ready for review February 15, 2023 11:50
@Dolu1990
Copy link
Member

Thanks :)
I will check it, just have quite a high stack of work at the moment

@Dolu1990
Copy link
Member

Hi,

Sorry for the delay, i'm running with toooo many thing on hands XD

So, in some ways, i a bit scared integrating too much things in the spinal.lib, especialy fresh things.
What's about keeping https://github.com/jjyy-Huang/SpinalHDL-ethernet and adding a reference to it in :
https://spinalhdl.github.io/SpinalDoc-RTD/master/SpinalHDL/Introduction/Projects%20using%20SpinalHDL.html#repositories

This maybe more scalable in terms of project management ? That way, poeple have the freedom to do things without having to worry to much about SpinalHDL upstream, while still having some visibility via the documentation pointers.
On my side It solve the maintenance issue i currently have with so many things now being in the Spinal lib :)

One thing, is that in your repo, you can extends the already existing spinal.lib package, for instance :
https://github.com/SpinalHDL/NaxRiscv/tree/main/src/main/scala/spinal/lib/bus/cmb

That way, it provide a forward way for easy integration when things are very stable / extended :)

Let's me know if that sounds good for you ?

@jjyy-Huang
Copy link
Author

Hi,

Sorry for the late reply, I changed my email and can't receive news from GitHub.

I appreciate your suggestion. I will integrate it into my work process and continue to make improvements as I move forward.

Thanks again for your input, and please let me know if you have any further suggestions or feedback.

@Readon Readon added feature ✨ Feature idea with clear API defined wontfix The issue would not be fixed labels Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ✨ Feature idea with clear API defined wontfix The issue would not be fixed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants