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 hash algorithms to pna.p4 #91

Merged
merged 1 commit into from
Nov 28, 2022
Merged

Add hash algorithms to pna.p4 #91

merged 1 commit into from
Nov 28, 2022

Conversation

hesingh
Copy link
Contributor

@hesingh hesingh commented Nov 16, 2022

Currently hash algorithms are TODO in PNA specification. This PR attempts to complete the TODO and also add defines to use as a flag to hash api to use hash on only source, destination, or both IP/IPv6 addresses, and protocol.

One has to start somewhere and thus psa.p4 hash algorithms have been copied to pna.p4. Also, none of the hash algorithms look like one a Nic can't support.

Copy link
Collaborator

@jafingerhut jafingerhut left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me. If a particular NIC cannot support some of these, it is pretty common practice for particular targets to only support the subset that they can, and I expect that to be true of PNA target devices as well.

@hesingh
Copy link
Contributor Author

hesingh commented Nov 17, 2022

Please merge, thanks.

@jfingerh
Copy link
Contributor

@hesingh Happy to do so after having at least an announcement and call for objections or concerns in the next public PNA work group meeting next Monday. I do not anticipate any, but I'm trying not to ram things through without such notification.

@thantry
Copy link

thantry commented Nov 21, 2022

Would be good to add Toeplitz and Xor hash, which are common in many NICs for RSS.

@thomascalvert-xlnx
Copy link
Member

LGTM.

Agree that Toeplitz needs adding, but equally happy for that to be in a separate commit.

@jfingerh
Copy link
Contributor

@thantry @thomascalvert-xlnx Thanks for review. Yes, we should add other hash algorithms as desired. For Toeplitz, this issue is tracking that, and from previous conversations it sounds like there might be additional control plane and/or data plane APIs desired there, and if so, merely adding a new enum value to the list is not enough, and I'd rather someone propose all of the pieces necessary in a PR addressing that issue: #19

For XOR, I have created a separate issue to track that: #93 I understand the desire to pile all the desired changes into single PRs, but hopefully we can get to a place where separate PRs can be moved through the pipeline of writing, approvals, and merging quickly enough that we are comfortable doing separate PRs.

@jafingerhut jafingerhut merged commit a4d9546 into p4lang:main Nov 28, 2022
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.

None yet

6 participants