Xilinx / xup_vitis_network_example

VNx: Vitis Network Examples
Other
137 stars 43 forks source link

CMAC: multi-mode padding and RSFEC sub-mode 1 #100

Closed quetric closed 1 year ago

quetric commented 1 year ago

I implemented selectable 60B/64B padding via flags settable through the CMAC USER_REG. I had to do some false paths trickery to get the design to meet timing - it's unclear from PG203 which clock is associated with the user_reg output signal from the CMAC. As the UDP stack does its own padding, I wasn't able to test beyond just making sure that the RTT benchmark still runs to completion.

I also activated RSFEC sub-mode 1 according to PG203 by writing into the RSFEC_CONFIG_INDICATION_CORRECTION register at 0x1000 and then resetting the RX and TX cores. With this mode enabled I see about 50ns increase in RTT.

mariodruiz commented 1 year ago

Hi @quetric,

Thank you for this PR. What is the reason for having runtime selectable padding? I think the design and sw would be much simpler if the padding was a compile time parameter.

quetric commented 1 year ago

Hi @mariodruiz

60B padding is the standard ethernet requirement. 64B padding aims to replicate the padding that EasyNet uses in their CMAC. The EasyNet maintainers claim that their design requires this non-standard padding mode. With selectable padding, the VNx CMAC would be able to serve both the VNx UDP network layer and the EasyNet TCP network layer.

You're right that the padding could be selected at compile-time though. Would you prefer that approach?

mariodruiz commented 1 year ago

60B padding is the standard ethernet requirement. 64B padding aims to replicate the padding that EasyNet uses in their CMAC. The EasyNet maintainers claim that their design requires this non-standard padding mode. With selectable padding, the VNx CMAC would be able to serve both the VNx UDP network layer and the EasyNet TCP network layer.

This is completely fine.

You're right that the padding could be selected at compile-time though. Would you prefer that approach?

A compile-time parameter seems more appropriate to me, as the padding won't really change at runtime. I would prefer a compile-time selectable variable. It can be passed from the Makefile to the tcl script

quetric commented 1 year ago

My latest update reflects your preference. Let me know if anything else needs changing before I move on to testing this version.

mariodruiz commented 1 year ago

@quetric just remove IntEnum. The rest looks good to me

quetric commented 1 year ago

Made the requested change. Also set default padding mode to 1 (60B padding) which should be safest option. I'll mark the PR ready for review once I have a test of this version.

quetric commented 1 year ago

Tested with RTT through switch.

mariodruiz commented 1 year ago

Hi @quetric,

It looks good, as last request. Can you edit this README to reflect the changes on RF-FEC and padding?

quetric commented 1 year ago

@mariodruiz I edited both the main readme and the Ethernet readme to properly describe the current CMAC config.

mariodruiz commented 1 year ago

@quetric thank you for your contribution.