-
Notifications
You must be signed in to change notification settings - Fork 80
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
PSA add initial values of metadata, and details of packet contents #451
Conversation
p4-16/psa/PSA.mdk
Outdated
packet to RECIRC instead, or drop the packet. | ||
|
||
A single packet received by a PSA system from a port can result in 0, | ||
1, or many packets going out, all under control of the PSA program. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: P4 program
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I wrote "PSA program" after noticing at least one other place in the document that uses that phrase. In some cases, when saying that something is or is not allowed in a PSA program, it seems more precise than saying P4 program -- we don't want the PSA spec to limit what programs people expect to be able to write in other architectures.
I'm fine either way, and we can do a global search and replace "PSA program" -> "P4 program" if you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made the replacements in the new text of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should use P4 program and PSA implementation consistently throughout the document. If we agree, we can have these changes done during the editorial phase as well.
p4-16/psa/PSA.mdk
Outdated
multicast NM packet to multicast group 18, which is configured in | ||
the PacketReplicationEngine to have copies made to ports 5 (copy 2) | ||
and 7 (copy 3). | ||
+ Copy 1 performs egress processing, which does not create a CE2E |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Readability: do we need to mention what it does not do? if so, i would move it after explaining what the copy does. E.g.: Copy 1 performs egress processing and sends the packet on path NTCPU to the CPU port. It does not create a CE2E clone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose we could leave out the "does not make a clone" phrases here, with the assumption that none is made if it doesn't say anything about a clone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
p4-16/psa/PSA.mdk
Outdated
suitable testing of your PSA program, and/or creating in your PSA | ||
program a "recirculate limit" metadata field that is carried along | ||
with copies of a packet, similar to the IPv4 Time To Live header | ||
field. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we should allow for target implementations to also limit the number of resubmit/recirc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like this?
"A PSA implementation may drop resubmitted, recirculated, or egress-to-egress clone packets after an implementation-specific maximum number of times of the same original packet. If so, it should maintain counters of packets dropped for these reasons, and preferably record some debug information about the first few packets dropped for these reasons (perhaps only 1)."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the text of the previous comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, sounds good.
Replace occurrences of "PSA program" with "P4 program". Eliminate explicit mentions of "does not create a clone" in the example packet flow. Mention that a PSA implementation may implement a limit on the number of times a single original packet can be recirculated, resubmitted, or cloned from egress to egress.
4334875
to
9d271bb
Compare
No description provided.