Adding packet length to Meter execute and Counter count methods for targets that may need it

[usha1830]

Proposing to add an optional parameter to specify the packet length for Meter extern's execute method and and Counter extern's count method.

If we can add this parameter as optional to the pna specification, it would help DPDK target to comply with the spec. For targets that don’t need packet length, it can be simply ignored.

[jfingerh]

@usha1830 Do you still want this PR? I think we discussed it privately several weeks ago, but I haven't followed up with you since then.

[shirshyad]

Optional increment is not applicable to packet counter. It may apply to byte counter.
Can we revise this and look into a way address application of optional increment only on byte counters ?

[usha1830]

We will handle this in p4c and expect the packet length only in case of byte counters. We can choose to either reject the programs with packet length parameter passed to packet counters or ignore that parameter with a warning.

[shirshyad]

For P4c or backend to reject optional increment argument, instance of Counter type need to be qualified so that P4C/ backend knows it a byte counter instance versus packet counter instance. Is this something that's possible with the way the Counter<W,S> class is defined ? If not, I am not sure how we can error out increment argument if the method is invoked on packet counter instance. (not on byte counter object or on packet_and_byte counter object)

[jfingerh]

Whether a counter instance is packet, byte, or packet & byte, is known during compile time based upon the arguments to the constructor call that created the instance. The P4 compiler should have all of the information required to know which kind each counter instance is.

[shirshyad]

Also should we look into adding a new method instead of modifying existing one ?

[usha1830]

Yes.

[apinski-cavium]

Is there a reason why you use @optional here when doing the following should also work:

void count(in S index);
void count(in S index, in bit<32> increment);

I don't see why @optional is needed when you can do it similarly via overloading.

[usha1830]

Yes. We will remove @optional.

[apinski-cavium]

Is there a reason why you use @optional here when doing the following should also work:

void count(in S index);
void count(in S index, in bit<32> increment);

I don't see why @optional is needed when you can do it similarly via overloading.

[usha1830]

@usha1830 Do you still want this PR? I think we discussed it privately several weeks ago, but I haven't followed up with you since then.

@jfingerh I will close this PR.
As per our discussion earlier, we will create new methods which take packet length and will also remove the @optional.

In case of counter 'count' method, an overloaded function would work as @apinski-cavium pointed out. However, in case of Meter 'execute' , we may have to change the method name to avoid the conflict between the below two signatures. p4c doesn't seem to resolve overloaded methods based on type.

PNA_MeterColor_t execute(in S index, in PNA_MeterColor_t color);
conflicts with
PNA_MeterColor_t execute(in S index, in bit<32> pkt_len);

[usha1830]

Closing this PR as discussed.
@kamleshbhalui is working on a new PR for adding new methods for count and execute for dpdk.