p4lang / pna

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

Add extern function recirculate() #114

Open jfingerh opened 1 year ago

jafingerhut commented 1 year ago

Explicitly document what is preserved with packets when they are recirculated.

One thought is that all user-defined metadata is preserved, but this is not universally implementable by all vendors wanting to support PNA.

One mechanism that seems universally implementable is to explicitly add one or more headers to the packet that must be preserved during recirculation.

Another potential mechanism would be that there are one or more arguments to the recirculate operation that are specified by the P4 developer, and this causes those fields to be preserved by the device. For one example of this approach, see the v1model and PSA architectures (they are not the same as each other in the P4 source code mechanisms, but both require the P4 developer to be explicit in what they want to save vs. not).

Another idea would be to have different operations for preserving vs. not preserving user-defined metadata, or some way to specify exactly which subset of user-defined metadata is saved, e.g. copying it into a header or struct explicitly in the user's P4 code. (perhaps calling one resubmit vs. recirculate).

AI Andy: Present v1model and PSA mechanisms (maybe also TNA?) for preserving metadata at next PNA meeting (probably 2nd week Feb 2023).

thantry commented 1 year ago

It would be good to also specify precisely what intrinsic metadata changes, and if that is agreed upon behavior between all architectures

thomascalvert-xlnx commented 1 year ago

If PNA supports mutable table entries that is also a mechanism for storing state in-between passes.

Various implementations will have different cost functions for preserving context in: metadata vs packet headers vs tables.

jfingerh commented 1 year ago

There was a discussion about how to control which subset of metadata should be preserved with the packet after a recirculate operation, with notes here: https://docs.google.com/document/d/1vX5GStrE01Pbj6d-liuuHF-4sYXjc601n5zJ4FHQXpM/edit#bookmark=id.uqs2nsxo6mx0

Below is another variation that is somewhat of a combination of some other ideas discussed before, plus one new thing not mentioned during the 2023-Feb-13 meeting.

Proposal:

Thoughts?

jafingerhut commented 1 year ago

Mario: If you want a significant amount of user metadata that is assigned in the main parser, and read and/or written in the main control or main deparser, but NOT recirculated, the approach described in the comment above would not handle that well.

Andy: We could consider applying a small tweak to the previous comment where there are two user_metadata parameters: one that is explicitly preserved across recirculation, the other defined NEVER to be preserved across recirculation.

Thomas: The approach of adding headers for preserving recirculated metadata could interact badly with hardware accelerators, e.g. IPsec encrypt/decrypt on the recircucation path, unless they were configured somehow to ignore the user-defined custom headers.

jafingerhut commented 1 year ago

Some notes from 2023-Mar-20 discussion of this topic:

Thomas: What about recirculate(single_bit_W_value); extern call, where single_bit_W_value is the ONLY value promised to be preserved with the recirculated packet, and that value is copied into some intrinsic metadata input field that we define, e.g. recirc_data. This new input value would be an input to the MainParser and the MainControl. The new recirculation_data field would be in structs named pna_main_parser_input_metadata_t and pna_main_input_metadata_t Alan: Why not a user-defined struct type rather than a single field? Thomas: The main reason is to lower the bar to something we think everyone can support for sure. Why have this feature at all, vs. not? The main reason to consider it is to enable a way to preserve at least a LITTLE bit of data with the packet, WITHOUT requiring the P4 developer from adding headers before recirculating.

jafingerhut commented 1 year ago

2023-Apr-03 discussion: Sounds reasonable to update this PR to add one bit type to recirculate() extern function as its only parameter, and that value becomes the value of a new recirculate_data field in the parser input and main control input on the next pass.

Should we do this, too? Also have a 0-argument variant of recirculate in case a user has no such data they care to include with the packet, in case a target can optimize in this case.

jafingerhut commented 1 year ago

@jfingerh Reminder from Andy to himself to make progress on this issue.

jafingerhut commented 1 year ago

With commit 4 of this PR, I believe I have addressed all of the earlier comments above, except for this proposed idea:

I would recommend that such a different operation be part of a separate PR from this one, if someone wants to champion that proposal.