-
Notifications
You must be signed in to change notification settings - Fork 446
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
Incorrect P4Info generation when different actions have same name annotation #2755
Comments
I have collected most of the P4 programs given on issues that I have added the label control_plane_api to (currently 4 of these open as of 2022-Jan-19 - link to all open ones here: https://github.com/p4lang/p4c/labels/control_plane_api), and added them to the I have also written a description of what each of them do with the latest version of p4c as of 2022-Jan-19 (with the particular commit SHA I tested with), and my opinion of which of them have correct behavior from p4c, and which seem to be bugs. This is a link to what I believe is one possible way to fix the root cause of these issues. I am not saying it is the only way, but I think it goes to the heart of the issue, i.e. it isn't any kind of hack or workaround: https://github.com/jafingerhut/p4-guide/tree/master/name-annotations#one-potential-way-to-fix-the-root-cause-of-some-of-these-issues |
Copying a note from Mihai Budiu to this open issue, so it remains easily found while this issue is still open: "There is one more wrinkle to consider: for P4-14 programs the compiler will actually create a copy of each action for each context where it is used (e.g., each table), and it will attach a @name annotation to it. All the copies will have the same @name." |
I discussed this issue with Vladimir Gurevich on 2022-Mar-16 for his opinion on this troublesome P4_16 program in particular: https://github.com/jafingerhut/p4-guide/blob/master/name-annotations/actions-5-same-name-annot.p4#L7-L15 with the same @name annotation on two functionally different actions, and he agreed with me that except for those @name annotations, this looks like a perfectly reasonable program, but with the @name annotations there appears to be nothing that a P4 API generation pass could do but give an error for it (which the open source p4c's P4Info generation does not as of 2022-Mar-16, but instead writes an incorrect P4Info file). Vladimir recalled that one possible reason for this might be that when translating P4_14 to P4_16, P4_14 explicitly allows the same action name to be used by two different tables, where one of those tables has a direct resource, e.g. direct counter or meter, and the other does not, and that is a reason that at least a P4 compiler back end would need to specialize that action for the two tables. However, we believe that this specialization of actions could be delayed to a compiler back end, or at least certainly later than the time when control plane APIs are generated from the IR. |
Attached zip file issue2755.zip has two similar but slightly different P4_16 source files, and the P4Info files I obtained by running with a recent (2021-Apr-30) version of p4c source code. Both P4Info files appear to be wrong in a similar way.
In my opinion, the correct behavior here would be to give a compile time error because two actions in the source code have the same
@name
annotation.issue2755.zip
The text was updated successfully, but these errors were encountered: