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.

Sign up for GitHub

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

Jump to bottom

PSA Update clone operations to use clone_session_id #515

Merged
merged 1 commit into from
Dec 14, 2017
Merged

PSA Update clone operations to use clone_session_id #515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 61 additions & 35 deletions p4-16/psa/PSA.mdk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,18 +481,26 @@ assist P4 developers in debugging their code.
packets, which are not required to support truncation.
}
if (ostd.clone) {
if (ostd.clone_class_of_service value is not supported) {
ostd.clone_class_of_service = 0;
// Recommmended to log error about unsupported
// ostd.clone_class_of_service value.
}
if (platform_port_valid(ostd.clone_port)) {
create a clone of the packet and send it to the packet
buffer with class of service ostd.clone_class_of_service,
after which it will start egress processing.
if (ostd.clone_session_id value is supported) {
cos = class_of_service configured for ostd.clone_session_id in PRE
ep = egress_port configured for ostd.clone_session_id in PRE
if (cos value is not supported) {
cos = 0;
// Recommmended to log error about unsupported cos
// value.
}
if (platform_port_valid(ep)) {
create a clone of the packet and send it to the packet
buffer with the egress_port ep and
class_of_service cos, after which it will start
egress processing.
} else {
// Do not create a clone. Recommended to log error
// about unsupported ep value.
}
} else {
// Do not create a clone. Recommended to log error about
// unsupported ostd.clone_port value.
// Do not create a clone. Recommmended to log error about
// unsupported ostd.clone_session_id value.
}
}
// Continue below, regardless of whether a clone was created.
Expand Down Expand Up @@ -685,18 +693,18 @@ packet contents and metadata when a packet begins egress processing.
+--------------+-------------+---------------+--------------+-----+
| EgressParser `istd` fields (type `psa_egress_parser_input_metadata_t`) |||||
+--------------+-------------+---------------+--------------+-----+
| `egress_port` | `ostd.egress_port` | from PRE | `ostd.clone_port` | `ostd.clone_port` |
| | of ingress packet | configuration | of cloned | of cloned |
| | | | ingress packet | egress packet |
| `egress_port` | `ostd.egress_port` | from PRE | from PRE configuration ||
| | of ingress packet | configuration | of clone session ||
| | | of multicast group | ||
+--------------+-------------+---------------+--------------+-----+
| `packet_path` | NORMAL_ | NORMAL_ | CLONE_I2E | CLONE_E2E |
| | UNICAST | MULTICAST | | |
+--------------+-------------+---------------+--------------+-----+
+--------------+-------------+---------------+--------------+-----+
| Egress `istd` fields (type `psa_egress_input_metadata_t`) |||||
+--------------+-------------+---------------+--------------+-----+
| `class_of_service` | `ostd.class_of_service` || `ostd.clone_class_of_service` of ||
| | of ingress packet || packet that caused this clone ||
| `class_of_service` | `ostd.class_of_service` || from PRE configuration ||
| | of ingress packet || of clone session ||
+--------------+-------------+---------------+--------------+-----+
| `egress_port` | Same value as received by EgressParser above. ||||
+--------------+-------------+---------------+--------------+-----+
Expand Down Expand Up @@ -816,19 +824,26 @@ the contents of several metadata fields in the struct
packets, which are not required to support truncation.
}
if (ostd.clone) {
if (ostd.clone_class_of_service value is not supported) {
ostd.clone_class_of_service = 0;
// Recommmended to log error about unsupported
// ostd.clone_class_of_service value.
}
if (platform_port_valid(ostd.clone_port)) {
create a copy of the packet and send it to the packet
buffer with class of service specified by
ostd.clone_class_of_service, after which it will start
egress processing.
if (ostd.clone_session_id value is supported) {
cos = class_of_service configured for ostd.clone_session_id in PRE
ep = egress_port configured for ostd.clone_session_id in PRE
if (cos value is not supported) {
cos = 0;
// Recommmended to log error about unsupported cos
// value.
}
if (platform_port_valid(ep)) {
create a clone of the packet and send it to the packet
buffer with the egress_port ep and
class_of_service cos, after which it will start
egress processing.
} else {
// Do not create a clone. Recommended to log error
// about unsupported ep value.
}
} else {
// Do not create a clone. Recommended to log error about
// unsupported ostd.clone_port value.
// Do not create a clone. Recommmended to log error about
// unsupported ostd.clone_session_id value.
}
}
// Continue below, regardless of whether a clone was created.
Expand Down Expand Up @@ -931,17 +946,28 @@ are those whose names begin with `clone` in types
`psa_ingress_output_metadata_t` and `psa_egress_output_metadata_t`.

```
bool clone;
PortId_t clone_port;
ClassOfService_t clone_class_of_service;
bool clone;
CloneSessionId_t clone_session_id;
```

The `clone` flag specifies whether a packet should be cloned. If
true, then a cloned packet should be generated at the end of the
pipeline. The `clone_port` specifies which port to send the cloned
packet to, after egress processing. The `clone_port` could be any
addressable port in the pipeline. For instance, it can be a port to
the control CPU or a port to the next hop router.
pipeline. The `clone_session_id` specifies one of several possible
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.
Copy link

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.

Copy link

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 •
edited
Loading

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.

Copy link

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.

Copy link

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 •
edited
Loading

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

Copy link

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!


The `egress_port` may be any port that can be used for normal unicast
packets, i.e. any normal port, `PORT_CPU`, or `PORT_RECIRCULATE`. For
the latter two values, the cloned packet will be sent to the CPU, or
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

PSA implementation begins with a clone session
`PSA_CLONE_SESSION_TO_CPU` initialized with `egress_port = PORT_CPU`
and `class_of_service = 0`.


### Clone Examples {#sec-clone-examples}
Expand Down
8 changes: 4 additions & 4 deletions p4-16/psa/examples/psa-example-clone-to-port.p4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ struct empty_metadata_t {

struct metadata {
fwd_metadata_t fwd_metadata;
clone_i2e_metadata_t clone_header;
clone_i2e_metadata_t clone_meta;
bit<3> custom_clone_id;
}

Expand Down Expand Up @@ -108,9 +108,9 @@ control ingress(inout headers hdr,
in psa_ingress_input_metadata_t istd,
inout psa_ingress_output_metadata_t ostd)
{
action do_clone (PortId_t port) {
action do_clone (CloneSessionId_t session_id) {
ostd.clone = true;
ostd.clone_port = port;
ostd.clone_session_id = session_id;
user_meta.custom_clone_id = 1;
}
table t {
Expand Down Expand Up @@ -146,7 +146,7 @@ parser EgressParserImpl(
}

state copy_clone_i2e_meta {
user_meta.clone_header = clone_i2e_meta;
user_meta.clone_meta = clone_i2e_meta;
transition parse_ethernet;
}

Expand Down
20 changes: 7 additions & 13 deletions p4-16/psa/psa.p4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ limitations under the License.

typedef bit<10> PortId_t;
typedef bit<10> MulticastGroup_t;
typedef bit<10> CloneSessionId_t;
typedef bit<3> ClassOfService_t;
typedef bit<14> PacketLength_t;
typedef bit<16> EgressInstance_t;
Expand All @@ -43,19 +44,23 @@ typedef error ParserError_t;
const PortId_t PORT_RECIRCULATE = 254;
const PortId_t PORT_CPU = 255;

const CloneSessionId_t PSA_CLONE_SESSION_TO_CPU = 0;
#endif // PSA_CORE_TYPES
#ifndef PSA_CORE_TYPES
#error "Please define the following types for PSA and the PSA_CORE_TYPES macro"
// BEGIN:Type_defns
typedef bit<unspecified> PortId_t;
typedef bit<unspecified> MulticastGroup_t;
typedef bit<unspecified> CloneSessionId_t;
typedef bit<unspecified> ClassOfService_t;
typedef bit<unspecified> PacketLength_t;
typedef bit<unspecified> EgressInstance_t;
typedef bit<unspecified> Timestamp_t;

const PortId_t PORT_RECIRCULATE = unspecified;
const PortId_t PORT_CPU = unspecified;

const CloneSessionId_t PSA_CLONE_SESSION_TO_CPU = unspecified;
// END:Type_defns

#endif
Expand Down Expand Up @@ -101,8 +106,7 @@ struct psa_ingress_output_metadata_t {
// Ingress control block begins executing.
ClassOfService_t class_of_service; // 0
bool clone; // false
PortId_t clone_port; // undefined
ClassOfService_t clone_class_of_service; // 0
CloneSessionId_t clone_session_id; // undefined
bool drop; // true
bool resubmit; // false
MulticastGroup_t multicast_group; // 0
Expand All @@ -124,7 +128,7 @@ struct psa_egress_output_metadata_t {
// The comment after each field specifies its initial value when the
// Egress control block begins executing.
bool clone; // false
ClassOfService_t clone_class_of_service; // 0
CloneSessionId_t clone_session_id; // undefined
bool drop; // false
bool recirculate; // false
bool truncate; // false
Expand Down Expand Up @@ -306,16 +310,6 @@ extern BufferingQueueingEngine {

}

// BEGIN:Clone_extern
extern clone_out {
/// Write @hdr into the ingress/egress clone engine.
/// @T can be a header type, a header stack, a header union, or a struct
/// containing fields with such types.
void emit<T>(in T hdr);
}
// END:Clone_extern


// BEGIN:Resubmit_extern
extern resubmit {
/// Write @hdr into the ingress packet buffer.
Expand Down