move recirculate flag to ingress #504
Conversation
| @@ -105,6 +105,7 @@ struct psa_ingress_output_metadata_t { | |||
| ClassOfService_t clone_class_of_service; // 0 | |||
| bool drop; // true | |||
| bool resubmit; // false | |||
| bool recirculate; // false | |||
There was a problem hiding this comment.
There is no need to add any fields to psa_ingress_output_metadata_t, is there? If the recirculate decision is controlled by assigning a value to ostd.egress_port of PORT_RECIRCULATE during the ingress control block, then such a flag seems redundant to me.
Unless perhaps you are thinking that the ingress code does not assign a value of PORT_RECIRCULATE to ostd.egress_port to cause the packet to recirculate, but instead assigns true to this ostd.recirculate flag instead? If so, then we'll need more changes at various places to make things consistent with that idea.
There was a problem hiding this comment.
If the thinking is that the ingress control block should do "ostd.egress_port = PORT_RECIRCULATE;" to cause a packet to recirculate (also ostd.drop = false to avoid dropping the packet), then I'd recommend removing this new recirculate flag, and also change the text describing the behavior of platform_port_valid() so that it treats PORT_RECIRCULATE as a valid output port.
There was a problem hiding this comment.
Another related question -- if ingress specifies to recirculate a packet by setting ostd.egress_port to PORT_RECIRCULATE, can the egress code create a clone with clone_port = PORT_RECIRCULATE, and that will cause the clone to go through egress processing and then recirculate?
There was a problem hiding this comment.
I submitted a PR with an example for recirculation. The idea is the ingress code will set the ostd.egress_port to PORT_RECIRCULATE to cause the packet to recirculate, but the istd.recirculate is still needed in egress deparser to decide if recirculate_meta should be assigned or not.
No description provided.