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

PSA Update clone operations to use clone_session_id #515

Merged

Conversation

jafingerhut
Copy link
Collaborator

Some text in the clone section describes what the control plane must
configure for each clone session.

Some text in the clone section describes what the control plane must
configure for each clone session.
@jafingerhut
Copy link
Collaborator Author

@samar-abdi As for other issue updating PSA on multicast replication, this PR contains proposed details for PRE configuration of clone sessions.

recirculated at the end of egress processing, as a normal unicast
packet would at the end of egress processing.

Since it is an expected common case to clone packets to the CPU, every
Copy link
Member

Choose a reason for hiding this comment

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

Thanks @jafingerhut

clone sessions that the control plane may configure in the PRE. For
each clone session, the control plane may configure the `egress_port`
and `class_of_service` values that should be associated with packets
cloned using that session.

Choose a reason for hiding this comment

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

Thanks @jafingerhut
Are we mandating that these are the only two configurations that a PSA PRE will support? What about truncation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am open to adding a truncation configuration value per clone session, if folks want it. @hanw @vgurevich thoughts on that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In general, I think of PSA as saying "at least this much, but an implementation is free to do more". Of course, taking advantage of more reduces portability, I know. If you know of other configuration options besides the ones listed that you want to discuss, this is a good place to ask about them.

Choose a reason for hiding this comment

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

OK, but we will have packet length in the P4Runtime API regardless, with the assumption that either switch SW or ASIC will support it.

Copy link
Member

@antoninbas antoninbas Dec 8, 2017

Choose a reason for hiding this comment

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

We can have it in P4 Runtime if there is interest, but that kind of line-rate packet operation (I'm talking about truncation) has to be supported in the hardware.

Choose a reason for hiding this comment

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

I saw this for the truncate operation:

For all copies of the packet made at the end of ingress processing,
truncate the payload to be at most the specified number of bytes.
Specifying 0 is legal, and causes only packet headers to be sent, with
no payload.

I suppose this applies to clones and multicast replicas.

Choose a reason for hiding this comment

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

@jafingerhut
Section 6.3.4

Copy link
Collaborator Author

@jafingerhut jafingerhut Dec 12, 2017

Choose a reason for hiding this comment

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

We may want a single place that summarizes how to truncate operations are supposed to work. For example, here is a little note stuck in Section 6.2 right now "Note: Truncation is not currently supported for cloned packets. The functionality of a truncated cloned packet can be replaced with sending a digest." There are also notes about truncation not needing to be supported for resubmit and recirculate packets.

I suspect it may be a good idea to add truncate options/parameters in the clone session configuration state. I will make an issue to discuss at next PSA WG meeting, so people have a chance to comment.

If there are no known issues with this PR other than whether truncate options should be included in the per-clone-session configuration state, I suggest we merge this in, and add that with a later PR after discussion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@samar-abdi Looks like the current PSA draft contradicts itself in different sections on whether truncation must be supported for cloned packets, or not. I have created this issue that will hopefully be discussed at next PSA WG, and resolved soon: #519

Choose a reason for hiding this comment

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

@jafingerhut I am ok with merging this PR and dealing with truncation separately. Thanks!

Copy link
Contributor

@cc10512 cc10512 left a comment

Choose a reason for hiding this comment

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

Thanks!

@jafingerhut
Copy link
Collaborator Author

jafingerhut commented Dec 13, 2017

@cc10512 OK to merge these changes in, do you think? There is now a separate issue specifically for the truncation behavior, for clones and all other kinds of packets, so that topic should be covered by the other issue.

@cc10512
Copy link
Contributor

cc10512 commented Dec 14, 2017

Yup, I didn't change my mind :)

@jafingerhut jafingerhut merged commit 33ec8c1 into p4lang:master Dec 14, 2017
@jafingerhut
Copy link
Collaborator Author

OK, merged it in, then.

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.

4 participants