p4lang / p4c

P4_16 reference compiler
https://p4.org/
Apache License 2.0
679 stars 444 forks source link

standard_metadata.egress_port should not be assigned in egress or ingress pipeline #1917

Open vksysd opened 5 years ago

vksysd commented 5 years ago

According to the P4-14 Version 1.0.4, the egress_port is a read-only value. However, if a value can be assigned to it in the egress pipeline using standard_metadata.egress_port = x and the compiler does not prevent it.

Please fix the compiler to throw an error in such cases when the programmer tries to assign value to the read-only fields of standard_metadata in ingress or egress pipelines.

jafingerhut commented 5 years ago

Just to add some detail here -- the front/mid-end of the p4c compiler should not do this. The way the v1model architecture is defined, there is nothing in the P4_16 language that makes egress_port read-only.

Such a new error/warning I think belongs in the BMv2 v1model back end of the compiler. Other similar checks could be made there, too.

mihaibudiu commented 5 years ago

Maybe we could gather a list of other checks that we can do and implement all of them?

jafingerhut commented 5 years ago

Here is a proposed list of access restrictions that p4c could check for access to v1model standard_metadata fields. Preferably someone else should review it to see if there is anything off:

read only, ingress or egress. The ones marked with (*) seem to me a bit unusual for a program to read during egress processing, but I cannot think of any good reason to make it illegal.

read or write during ingress, read only during egress:

read only, and only during egress:

I am not sure yet how BMv2 simple_switch uses this field, so no suggestion right now:

Proposed to be removed by PR https://github.com/p4lang/p4c/pull/1911 so if that is committed, the compiler will give an error for any access anywhere: