-
Notifications
You must be signed in to change notification settings - Fork 78
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
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.