p4c-bm2-ss gives incorrect warning about unused counter

[jafingerhut]

See the first line containing "direct_counter" in the attached P4_16 program. 2017-Apr-08 version of p4c-bm2-ss gives a warning about ipv4_da_lpm_stats being unused. However, if I comment out that line, I get 2 errors, one for each time ipv4_da_lpm_stats.count() is called shortly afterwards.

Also, if I generate a bmv2 JSON file and run simple_switch on it and try to use the command 'counter_read ipv4_da_lpm_stats ' from simple_switch_CLI, it gives this error:

Error: Invalid counter name (ipv4_da_lpm_stats)

A similar P4_14 program works correctly when compiled with p4c-bmv2. I can create a separate issue for this behavior if desired.
bad-warning-of-unused-counter.p4.txt

[mihaibudiu]

This issue was introduced in 2c131b8, when the direct counters had a method count() added. Originally there was no such method in v1model.p4.
I believe that this method should not be present in v1model.p4.

[mihaibudiu]

See all the comments at #421 where this problem was anticipated.

[ChrisDodd]

There's some confusion as to how direct counters and meters work in the P4_16 standard architecture -- are they properties on tables, or are they externs with methods invoked in actions? The bmv2 backend mostly expecting them to be properties, which is what causes problems with this code. Interally it is buggy and is checking the wrong attribute name (a result of obfuscating the the names of evering behind static const cstring variables rather than just using string literals). With that fixed, the posted code still gives the warning as there are TWO counters, one created in the table and one created in the control. It should be giving an error about the count method being called on a counter not attached to the table, but is not. Fixing that (and the spurious warning requires changing the attribute in the table to be counters = ipv4_da_lpm_stats, attaching the counter created in the control to the table, rather than creating a second counter.

[mihaibudiu]

The standard architecture is one place, but v1model is another one.
It would be very nice if there was some documentation about the way these objects should be used.
Also, if they are to be used differently between PSA and BMv2 we will need special handling for the two cases in the back-end. Also, if the P4-14 to P4-16 translation targets BMv2 or PSA, it may need to be different!

[mihaibudiu]

@ChrisDodd : is this issue fixed with your latest commit?

[jafingerhut]

Thanks for the fixes to avoid the warnings here. Closing this issue is reasonable.