Add the DASH BMv2 model to the tested programs.

[fruffy]

Another large BMv2 program to test on.

[fruffy]

I believe there is a bug in the way Inline handles table names. It converts the table name into outbound_acl_stage1:dash_acl_rule|dash_acl instead of outbound_acl_stage1.

Alternatively, this might just be an illegal @name?

[fruffy]

@jafingerhut You mentioned the dash programs were slightly unusual in the way they were written. Were you referring to the table names, for example? Are names like stage1:dash_acl_rule|dash_acl legal P4Runtime names?

[jafingerhut]

@fruffy I had forgotten about the unusual characters in @name annotations, which are a bit odd, true.

More likely what I was referring to is that there are new custom match_kinds and syntax of proposed non-standard P4 language constructs in some of the original source files (e.g. search for a match_kind 'list' and I think also 'list_port', and a keyword 'struct_graph' in some of the files), that give errors if you attempt to compile them with the latest published p4c. Those are only enabled with certain preprocessor symbol #define settings that you probably did not use.

[jafingerhut]

@fruffy By the way, are the unusual characters in @name annotations causing problems within p4c itself? e.g. causing intermediate P4_16 files to be generated that cannot be parsed as P4_16 source code by p4c? Or some other issues?

It seems worth mentioning this fact to the authors of the DASH P4 program, in case they are unaware of it.

[fruffy]

@fruffy By the way, are the unusual characters in @name annotations causing problems within p4c itself? e.g. causing intermediate P4_16 files to be generated that cannot be parsed as P4_16 source code by p4c? Or some other issues?

It seems worth mentioning this fact to the authors of the DASH P4 program, in case they are unaware of it.

Yes, the Inlining pass uses a rename function which only substitutes the illegal character .. https://github.com/p4lang/p4c/pull/3885/files#diff-83c9053ca537ebc42f1cef836c057a52ddbd2668cab22f89ef28839523643812R206
A name annotation can have many more unusual characters, all of which would lead to table names that are invalid.
I am not sure what the expected behavior is here.

[fruffy]

Rerequesting to make sure that the changes to inlining.cpp are acceptable.

[jafingerhut]

I have created this issue on the DASH repo pointing out the possible issue with @name annotations having characters that are not legal in P4 identifiers.

@fruffy I am guessing that if the string contents of a @name annotation were a legal P4 identifier name, there should be no problems encountered?

[apinski-cavium]

I remember asking about @name specifically on keys as the p4c will definitely create @name annotations like "a & 15".

[mihaibudiu]

https://p4.org/p4-spec/docs/P4-16-v-1.2.3.html#sec-cp-names describes how control-plane names are calculated.
I think this change is not conformant to the spec.

[apinski-cavium]

https://p4.org/p4-spec/docs/P4-16-v-1.2.3.html#sec-cp-names describes how control-plane names are calculated. I think this change is not conformant to the spec.

p4lang/p4-spec#1141 ...

[fruffy]

https://p4.org/p4-spec/docs/P4-16-v-1.2.3.html#sec-cp-names describes how control-plane names are calculated. I think this change is not conformant to the spec.

In what way is this not conformant? I believe the control plane will still just refer to the @name, which is unchanged.

[mihaibudiu]

Is there a test case exercising the new inline code?
Then we can tell whether the result is wrong.

[fruffy]

Is there a test case exercising the new inline code? Then we can tell whether the result is wrong.

Done. the compiler seems to be handling the characters well. And so does the PTF framework, surprisingly. The STF framework does not.

[fruffy]

Any remaining concerns with this PR?

[mihaibudiu]

I have approved