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 two variants of PNA #51

Closed
wants to merge 1 commit into from
Closed

Conversation

jfingerh
Copy link
Contributor

No description provided.

@shirshyad
Copy link

High Level Comments

  • To me based on IPSEC requirement which requires packet parsing once before decrypt action and once again post decrypt action, two architectures are being proposed. I am not sure this approach of architecture variants based on offload functionality will set a precedence that can lead to more architectures in the future either because offloads are implemented uniquely by various vendors or due to constraints of tooling. If a common abstraction is not possible to arrive at, then should we leave IPSEC offload out of standardization effort and let vendor specific extern/s express IPSEC at the cost of portability ?
  • It would help to hear from the end user of P4 who would use NIC from multiple vendors. We should consider their inputs before we checkin two version of PNA architectures.
  • If there is conclusion that more than one architectures is unavoidable, then should we just describe abstract model that merges both PNA1 and PNA2 and state in the specification that every vendor need not implement all the blocks of the abstract model and hence call out that portability is impacted.


#ifndef __PNA1_P4__
#define __PNA1_P4__

Copy link

@shirshyad shirshyad Aug 15, 2022

Choose a reason for hiding this comment

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

Should we not edit pna.p4 instead of adding pna1.p4 ? We may remove PreControl from pna.p4 and add a new pna2.p4 file that expresses second architecture with pre-preparer and pre-control.

@jafingerhut
Copy link
Collaborator

I plan to close this PR as soon as I determine that there is nothing I want to salvage from it. It uses an approach that I have given up pursuing for inclusion in PNA.

@jfingerh
Copy link
Contributor Author

I have looked through the changes in this PR, and did not find any changes that were unrelated to the 2-parser/2-control vs. 1-parser/1-control idea, so closing this PR as abandoned.

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.

3 participants