Accept action IDs on DirectCounterEntry in Stratum-bfrt

[pudelkoM]

Those are still valid and the action field has to be ignored, according to the P4RT spec.

[pudelkoM]

Thanks @daniele-moro for reporting.

[pudelkoM]

cc @pierventre
We might have code in ONOS that was affected by this, too.

[codecov]

Codecov Report

Merging #942 (0e11c66) into main (e031360) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #942      +/-   ##
==========================================
+ Coverage   79.19%   79.20%   +0.01%     
==========================================
  Files         339      339              
  Lines       30924    30918       -6     
==========================================
- Hits        24489    24488       -1     
+ Misses       6435     6430       -5     

Impacted Files	Coverage Δ
stratum/hal/lib/barefoot/bfrt_table_manager.cc	25.83% <ø> (+0.08%)	⬆️

[daniele-moro]

I think ONOS code is not affected but wait for confirmation from @pierventre. Direct counters in ONOS are either read as part of a read of table entries, or doing reads by specifying only the table entry handle (i.e., match + priority). See: https://github.com/opennetworkinglab/onos/blob/dc08c95eed882936edf47527ca64d416009e8514/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeFlowRuleProgrammable.java#L536-L551

[pierventre]

I think ONOS code is not affected but wait for confirmation from @pierventre. Direct counters in ONOS are either read as part of a read of table entries, or doing reads by specifying only the table entry handle (i.e., match + priority). See: https://github.com/opennetworkinglab/onos/blob/dc08c95eed882936edf47527ca64d416009e8514/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeFlowRuleProgrammable.java#L536-L551

Yup - we talked about this in private me and @pudelkoM and I confirm what you are saying. On our end we can remove some code around encodeKey for TableEntryCodec but IMHO is not really worth.

[pudelkoM]

Sounds good to me. "affected" was poorly worded here. "potential chance for simplification" is more appropriate as old behavior will continue to work.