p4lang / pna

Portable NIC Architecture
Apache License 2.0
55 stars 21 forks source link

Add hash algorithms to pna.p4 #70

Closed hesingh closed 1 year ago

hesingh commented 1 year ago

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.

hesingh commented 1 year ago

If folks would like an explicit flag to be provided to a hash, we could use the defines below or put them in an enum and add another hash api with such a flag.

#define IP_FLOW_HASH_DST_ADDR (1<<1)
#define IP_FLOW_HASH_PROTO (1<<2)
#define IP_FLOW_HASH_SRC_PORT (1<<3)
#define IP_FLOW_HASH_DST_PORT (1<<4)
#define IP_FLOW_HASH_REVERSE_SRC_DST (1<<5)    // symmetric hash
thomascalvert-xlnx commented 1 year ago

I think that the commit's summary & description need to be improved - at least match the PR title.

Otherwise LGTM.

jfingerh commented 1 year ago

@thomascalvert-xlnx Please do recommend specific text that you think would be good in the commit message for @hesingh, if you have ideas.

thomascalvert-xlnx commented 1 year ago

Please do recommend specific text that you think would be good in the commit message for @hesingh, if you have ideas.

The current commit message is simply: "Add hash". I suggested using the PR title, "Add hash algorithms to pna.p4".

hesingh commented 1 year ago

I don't think a commit message can be changed after commit.

jfingerh commented 1 year ago

@hesingh If not, then a new PR can be created with a different commit message.

thomascalvert-xlnx commented 1 year ago

Too late now, but git commit --amend is the most convenient way to change an existing commit's message.

https://www.atlassian.com/git/tutorials/rewriting-history

https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History