PSA: Is drop allowed in egress?

[samar-abdi]

I noticed that both buffering engine and PRE have a drop method. I imagine, we will be using this call to drop packets (replacing the mark_to_drop() in v1model). Does it make sense for a packet to be dropped in the egress pipeline?

If yes, doesn't that imply setting the egress_port to some drop port? But we cannot modify egress_port in egress!

If no, what is the reason for this restriction?

[cc10512]

The reason for a drop in BQE is to allow dropping in egress, which happens if you have tables such as acl, or other validation. Drop is in fact initiated by setting the metadata -- we discussed this in the WG at the July 10 meeting. The methods on the externs are vestiges, even though much better for raising such issues!

If you look at section 4.2.1, the pseudo-code checks the drop metadata and drops the packet, so you do not actually need to set the egress_port.

[jafingerhut]

As far as I can see in the latest version of the PSA draft document, the PacketReplicationEngine and BufferingQueueingEngine externs have 0 methods. Unless someone knows of a reason they should have methods in them, they could be completely removed from psa.p4 and the PSA spec with no changes in functionality provided.

[jafingerhut]

I agree with Calin that someone might want to drop a packet in the egress pipeline, in some cases, and because of the drop flag, it does not require changing the value of the egress_port field to drop a packet.

[vgurevich]

@jafingerhut ,

On getting rid of PRE/BQE. I think it totally makes sense in the data plane, but I'd like to keep them around till the P4 API group figures out what do they want to do in the control plane, since these externs (especially PRE) have fairly involved configuration.

What do you think?

[samar-abdi]

Oops!! I was looking at the stale version of psa.p4 in my fork! Based on the latest version, everything makes perfect sense. Sorry about the trouble.

One quick note though: In the egress pipeline, if a table drops a packet, and a subsequent table matches on egress_port, there is potential for un-intended behavior, since the egress_port cannot be unset to reflect the "drop." Of course, it is the responsibility of the P4 programmer to add a match on the ostd.drop field as well in such cases, but it might be worth clarifying it in the spec with an example. Happy to do this if others feel it is warranted.

Also, I noticed this statement for egress processing logic:
"enqueue one packet for output port istd.egress_port"
should it be dequeue?

As for PRE /BQE, I would request keeping them around and adding the control-plane API comments like we do for other externs (once the API is finalized).

[jafingerhut]

Regarding enqueue vs. dequeue question, the thinking behind that statement is not that the packet would go to a queue that was in the PacketReplicationEngine, the way a packet would after finishing ingress processing.

The thinking was that some implementations might have a queue of packets that finished egress processing, waiting to be transmitted on an output port. I'm open to suggested clarifications, if that is currently confusing.

[jafingerhut]

Regarding an example that matches on the 'drop' flag in egress, in addition to the 'egress_port', to distinguish drop vs. no-drop cases, is this something you expect might trip up people who have written P4_14 programs with egress tables that matched on egress_port=DROP_PORT?

There is also the possibility of having example programs in the Git repo that could be in appendices, or not even included in the spec at all, except perhaps by a file name reference.

[jafingerhut]

@vgurevich Leaving PRE and BQE extern objects explicitly in the PSA for now, on the assumption that we may want them to have a control plane API, even if they have no methods that can be called from a P4 program, makes sense to me (especially for PRE, where control plane APIs for configuring multicast groups sounds like something that ought to have a portable spec for, if such a thing can be agreed upon).

[cc10512]

@jafingerhut Can we close this issue then?

[jafingerhut]

The only possible action item I can see from the comments above is this suggestion from @samar-abdi: "Of course, it is the responsibility of the P4 programmer to add a match on the ostd.drop field as well in such cases, but it might be worth clarifying it in the spec with an example. Happy to do this if others feel it is warranted."

I have no objections to such an example, but would prefer to work on other example programs than that one.

[cc10512]

Since @samar-abdi has virtually volunteered, let's assign this to him.

[jafingerhut]

@samar-abdi Is the example program you described above still something you would want to write?

[samar-abdi]

Since the semantics of behavior at the end of egress is well defined, I don't think we really need this example. We can close on this.

[jafingerhut]

Thanks. Closing this issue now, then.