Lack of documentation on the optional bytes counter increment parameter

[qobilidop]

This optional parameter has been added in p4c:

https://github.com/p4lang/p4c/blob/94994988db3c6f2d0c39a95135592524c5c5293b/p4include/pna.p4#L375-L378

https://github.com/p4lang/p4c/blob/94994988db3c6f2d0c39a95135592524c5c5293b/p4include/dpdk/pna.p4#L382-L390

PNA spec refers to the PSA spec for the counter extern definition, but it doesn't explain this parameter.

[jfingerh]

The DPDK implementers, e.g. I believe Cristian Dumitrescu, wanted to add this parameter. I believe the rationale is that it was easy to do, and it generalizes what the counter extern can do, by allowing a P4 programmer to choose to add an arbitrary value to a counter, rather than restricting it to adding the length of the current packet (in units of bytes) to the counter.

If saying something like that in a comment just before the method would clarify it, and satisfy your concern, I would recommend that someone create a PR adding such a comment.

This parameter is NOT in the PSA specification, partly because there are P4-programmable implementations that do not support this flexibility -- their counter externs ONLY allow adding the length of the current packet (in units of bytes) to the byte counter.

[jfingerh]

@cristian-dumitrescu Pinging you to see if you have any corrections to my guesses above.

[jafingerhut]

@usha1830 Also pinging you in case you can comment.

[qobilidop]

@jfingerh Thanks for your explanation!

Having some comments would help. And I can volunteer to do the work.

I also wonder if this could be better explained in the PNA spec, as it currently just says:

pna/PNA.mdk

Line 810 in dbe1ea7

See the PSA specification for definitions of all of these externs.

Giving the impression that these externs are the same as in PSA. But as explained in your comment, they are actually different. I wonder if having a section in the PNA spec explaining differences like this would be helpful?

[jafingerhut]

@qobilidop The DPDK implementation of PNA chose to make this generalization. That doesn't mean it is part of the PNA specification. If you want to add this comment, I suggest adding it in the DPDK-specific pna.p4 include file only, not in the the PNA spec.

That is, unless the majority of PNA implementers of hardware targets agree that they would like to implement this generalization, too, which I am pretty sure they would say no.

[qobilidop]

I see. Then maybe this parameter should only appear in dpdk/pna.p4, but not pna.p4?

[jafingerhut]

I have only recently learned from @usha1830 that p4c/p4include/pna.p4 is used by p4test, and p4c/p4include/dpdk/pna.p4 is used by p4c-dpdk. I am not claiming that it must be that way -- just that it is that way right now. I don't know at this moment the full consequences of doing as you suggest.