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 a parameter to top level parser and controls for vendor-specific … #115

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jfingerh
Copy link
Contributor

…metadata

@apinski-cavium
Copy link
Contributor

+1 from me.

@thantry
Copy link

thantry commented Jan 30, 2023

Would we support copying from vendor metadata to user defined metadata, as part of the P4 program?

@jafingerhut
Copy link
Collaborator

AI Andy: Add a section to PNA spec and pna.p4 header file that explicitly describes all things that vendors are expected to customize in their product's version of the pna.p4 header file.

@stolsma
Copy link
Member

stolsma commented Jan 30, 2023

As I do understand this addition, I prefer that also a detailed "when and how is this supposed to be used" text somewhere in the PNA document to prevent the per browser vendor HTML/CSS properties charade in the browser world...

@jafingerhut
Copy link
Collaborator

Alan: What about more fine-grained control, e.g.

  • 1 in and 1 inout struct type for mainparser
  • 1 in and 1 inout struct type for maincontrol
  • 1 in and 1 inout struct type for deparser

Andy: For our P4 compiler, we were thinking of even mixing fields that were read-only vs. ones that were read-write mixed together in the same struct, and have the compiler back end enforce the read-only restrictions on fields that were read-only. This makes less good use of the front-end checking for direction 'in' parameters, but enables us to across different compiler releases change a field from read-only to read-write, WITHOUT having to move the field between structs, which could be an imposition on customer P4 programs.

@thomascalvert-xlnx
Copy link
Member

thomascalvert-xlnx commented Mar 13, 2023

I would also suggest 3 inout structures, one for each parser/control/deparser.

From a mechanics point of view, would we expect cross-platform PNA code to look something like:

#include <pna.p4>
#ifdef VENDOR_A
#include <pna_vendor_a.p4>
#endif
#ifdef VENDOR_B
#include <pna_vendor_b.p4>
#endif

// ...later in some code....
#ifdef VENDOR_A
    vmeta.foo = 42;
#endif
#ifdef VENDOR_B
    vmeta.bar = 123;
#endif

or do we expect each vendor to fork pna.p4 ?

@jafingerhut
Copy link
Collaborator

@thomascalvert-xlnx My thinking was that each vendor would have their own customized version of the pna.p4 include file. There is already a section in psa.p4 and pna.p4 include files that explicitly says that vendors are expected to customize the bit widths of certain typedef's for things like port ids, multicast group ids, etc.

I will create a separate issue in Github to track the even more explicit documentation of where we expect vendors to modify pna.p4 for themselves, vs. where we do not expect them to.

@jafingerhut
Copy link
Collaborator

jafingerhut commented Mar 13, 2023

Here is a new Github issue to track the idea of documenting precisely how pna.p4 include files are expected to differ across different targets, vs. which parts we expect to remain the same: #116

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.

6 participants