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

P4 DPDK on horizon; BMV2 #408

Open
KrisNey-MSFT opened this issue Jul 27, 2023 · 31 comments
Open

P4 DPDK on horizon; BMV2 #408

KrisNey-MSFT opened this issue Jul 27, 2023 · 31 comments
Assignees

Comments

@KrisNey-MSFT
Copy link
Collaborator

KrisNey-MSFT commented Jul 27, 2023

DPDK BE test/experiment
The engineers who work on p4c-dpdk back end have been notified (and provided some of the details on the reasons for failure to load that I included in the issues). These issues are not their highest priority tasks, so not clear when they might be able to address them.

Compiles fine, needs to load into the software switch.

Just experimental, open source P4 BE DPDK code that produces the spec file (possible bug), but still low priority - feel free to check them out! How does P4C work???

Intel Eng not opposed to pursuing resolution of issues; need to check w/Reshma to see if issues will be fixed within a timeframe.

https://github.com/p4lang/p4c/tree/main/backends/dpdk
https://github.com/p4lang/p4c/issues?q=is%3Aissue+is%3Aopen+label%3Adash-blocker

Usha Gupta is coordinating (GitHub ID)
Ping Github id @usha1830 if you are interested in discussing those issues in more detail.
This is Usha Gupta, Intel engineer who has developed most of the p4c-dpdk back end code.
You can also ping me at @jafingerhut if I might be of help, but I'm more familiar with the symptoms than the p4c-dpdk back end C++ code.

@KrisNey-MSFT
Copy link
Collaborator Author

More update next week

@usha1830
Copy link

@KrisNey-MSFT @reshmaintel
The issues mentioned in https://github.com/p4lang/p4c/issues?q=is%3Aissue+is%3Aopen+label%3Adash-blocker are fixed and merged in p4c main branch.
Please let me know if there are any further issues.

@chrispsommers
Copy link
Collaborator

Hi @jfingerh would you be able to try loading the compiled DASH p4 code into the updated p4-dpdk backend and see if it is happy?

@jafingerhut
Copy link
Contributor

I have tried it with the version of DASH at the time I filed that issue in late March/early April, and there are no errors compiling that version, and no errors loading it into the P4-DPDK software switch.

I then tested compiling the latest version of DASH P4 code. It compiles with warnings (but no errors -- mostly warnings about unused parameters/fields/extern instances), but it gives errors when attempting to load into the P4-DPDK software switch. I have not root-caused it, but will pass on how to reproduce it to some Intel engineers who know how to do that more quickly than I do.

@jafingerhut
Copy link
Contributor

I have created a new issue for p4c-dpdk to track this latest discovered "dash blocker": p4lang/p4c#4156

I plan to use the "dash blocker" tag for any such issues, to make them easier to find: https://github.com/p4lang/p4c/issues?q=is%3Aissue+is%3Aopen+label%3Adash-blocker

@KrisNey-MSFT
Copy link
Collaborator Author

@jafingerhut - should we close this one, and continue work with 4156? Or keep this one open for tracking? TY, KrisNey-MSFT.

@jafingerhut
Copy link
Contributor

If you want a DASH-specific place to track what is missing for DASH, it seems best to continue to have an issue in this repo for that purpose. It seems reasonable to continue to use this issue until P4-DPDK can pass a first packet through for whatever is at the time the current version of the DASH P4 reference code.

@KrisNey-MSFT
Copy link
Collaborator Author

Andy - issue with P4-DPDK backend. The code at [https://github.com/sonic-net/DASH/blob/main/dash-pipeline/bmv2/dash_service_tunnel.p4#L7-L24] which performs 128-bit masking on IPv6 addresses exposes a deficiency in p4-dpdk backend, it can't perform 128-bit math.
Action - @jafingerhut will pursue it with the p4-dpdk team. He may also propose a workaround/compromise and file as an issue or PR.

@jafingerhut
Copy link
Contributor

I will continue to mark issues that prevent P4-DPDK from being a sufficient software switch for DASH P4 reference code with a "dash-blocker" tag. Here is the current list (2 as of 2023-Sep-25): https://github.com/p4lang/p4c/issues?q=is%3Aissue+is%3Aopen+label%3Adash-blocker

There were also two issues last Friday, but over the weekend I closed one that was fixed, and opened another. This is the newest one, which to fix in the way I propose there would require changes in not only the p4c-dpdk compiler, but also in the support within the DPDK software switch for arithmetic operations on operands wider than 64 its: p4lang/p4c#4174

I can talk to P4-DPDK developers about such an enhancement, but as usual, I do not have much knowledge yet of when such significant enhancements might be made.

@KrisNey-MSFT
Copy link
Collaborator Author

Per 9/27/2023 Community Call, Andy is going to lobby for these enhancements in P4 group.
Can check back another week or so.

@KrisNey-MSFT
Copy link
Collaborator Author

Same status, check next week

@KrisNey-MSFT
Copy link
Collaborator Author

Ping sent to Intel team

@KrisNey-MSFT
Copy link
Collaborator Author

@usha1830 fixed 1 issue, working on the 2nd.
Defined a header w/bit fields not multiples of 8, the P4 DPDK implementation was swapping these in the wire order of bits. Not DASH specific, but was likely fixed.

Current DASH P4 opers on 128bit - people at Intel know about this.

@ashutosh-agrawal discussing w/P4 DPDK C target team. Right now only support 64bit operation. Not the compiler, it's the target. 2 different components need to be fixed. DASH just needs bit-wise... and an or, sub and add, shift...etc...

@KrisNey-MSFT
Copy link
Collaborator Author

Waiting for Intel staff for answers to the above.

@KrisNey-MSFT
Copy link
Collaborator Author

Hi @ashutosh-agrawal and @usha1830 - I didn't see you in the call today. We were curious whether there is an update to this Issue? TY, Kristina

@KrisNey-MSFT
Copy link
Collaborator Author

Compiler and soft switch work. PRs pending...

@KrisNey-MSFT
Copy link
Collaborator Author

P4 DPDK will be faster, yet heavy lifting required to design

@jafingerhut
Copy link
Contributor

An update from some experiments run in the last few days: I found additional issues that seem like they would block DASH using P4-DPDK. As far as I can tell, these are not issues with the p4c-dpdk P4 compiler, but with the P4Runtime API server implementation for P4-DPDK. I have filed these two public issues asking questions about the difficulties I have found:

@chrispsommers
Copy link
Collaborator

Hi Andy, thanks for exploring this and keeping us up to date!

@KrisNey-MSFT
Copy link
Collaborator Author

KrisNey-MSFT commented Feb 26, 2024 via email

@KrisNey-MSFT
Copy link
Collaborator Author

128 bit support - changes need to be picked up by Compiler team

@KrisNey-MSFT
Copy link
Collaborator Author

Looking to find resource at Intel, Usha is no longer with the team. We cannot run a subset of tests with either bmv2 or P4 DPDK. Priority = Medium.

@KrisNey-MSFT
Copy link
Collaborator Author

Sosutha (possible resource) from Intel

@shweta503 shweta503 assigned Sosutha and unassigned shweta503 Aug 21, 2024
@KrisNey-MSFT
Copy link
Collaborator Author

hi @Sosutha - although we didn't have time during the call today...would you happen to have an update for this one? We can discuss tomorrow in Behavioral Model call :)

@Sosutha
Copy link
Collaborator

Sosutha commented Sep 9, 2024

Implementation for 128 bit support for bitwise instructions is started now

@KrisNey-MSFT
Copy link
Collaborator Author

Thank you @Sosutha ! @r12f and @jafingerhut for visiblity...

@KrisNey-MSFT
Copy link
Collaborator Author

PR perhaps open next week in open source.

@Sosutha
Copy link
Collaborator

Sosutha commented Oct 2, 2024

PR is in progress. Testcase addition in progress. It will be raised in open source this week

@KrisNey-MSFT
Copy link
Collaborator Author

Could try this on your fork @Sosutha ? Is this published to docker pub? Or wait for the PR to be merged in order to use the docker file?

@Sosutha
Copy link
Collaborator

Sosutha commented Oct 9, 2024

@KrisNey-MSFT, It is not clear to me what you are asking me to try. PR for 128bit bitwise operations in dpdk is here. p4lang/p4c#4952

@KrisNey-MSFT
Copy link
Collaborator Author

Link to Intel PR

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

No branches or pull requests

9 participants