p4lang / pna

Portable NIC Architecture
Apache License 2.0
55 stars 21 forks source link

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

Closed usha1830 closed 1 year ago

usha1830 commented 2 years ago

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 commented 1 year ago

@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.

usha1830 commented 1 year ago

@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 commented 1 year ago

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