Dpdk always require packet len for Meter's method execute()

[kamleshbhalui]

No description provided.

[hanw]

I think a cleaner design would be to create a Meter that is specifically designed for DPDK.

extern DPDKMeter<S> {
  DPDKMeter(bit<32> n_meters, PSA_MeterType_t type);
  PSA_MeterColor_t execute(in S index, in PSA_MeterColor_t color, in bit<32> pkt_len);
}

In the dpdk backend, you can check if the P4 program uses the vanilla Meter extern. If so, report an error and suggest customer to use the DPDKMeter instead.

In addition, if we introduce DPDKMeter, we wouldn't need this PR that I did last year: p4lang#2818 and the dpdk/psa.p4 file would only contain the DPDK specific extern definition, instead of duplicating the entire psa.p4 file again. That would be a much cleaner solution.

[hanw]

Portability is not achievable with the current meter implementation in dpdk, We should make that explicit, instead of pretending the program is portable by supporting a slight variant of the Meter extern that is actually different.

[kamleshbhalui]

Should we have DPDKCounter too? Because PSA spec forbid the 2nd argument for count but dpdk adds a extra optional parameter increment?
void count(in S index, @optional in bit<32> increment);

[hanw]

Yes, DPDKCounter as well.

Please consider deduplicate the dpdk/psa.p4, revert the open-source p4c to contain only one copy of psa.p4 file, and add dpdk/psa-ext.p4 for dpdk specific extensions.

[kamleshbhalui]

Does the user explicitly have to include dpdk/psa-ext.p4 in the p4 program?

[kamleshbhalui]

or we should do something like this?

// for target specific extensions
#if defined(__TARGET_DPDK__)
#include "dpdk/psa_ext.p4"
#elif defined(__TARGET_BMV2__)
#include "bmv2/psa_ext.p4"
#else
// to get rid of unknown identifier when compile with p4test
#include "dpdk/psa_ext.p4"
#include "bmv2/psa_ext.p4"
#endif  // __TARGET_DPDK__

[hanw]

bmv2 should be able to implement the standard psa.p4, there is no reason to extend psa.p4 for bmv2. DPDK is a target with specific limitation, so it would require extension to meter and counter. p4test is also generic enough that it could use the standard psa.p4 as well.

the directory structure should look like
p4include/psa.p4
p4include/dpdk/psa_ext.p4
p4include/v1model.p4

you can remove the p4include/bmv2/ directory, and revert the PR I mentioned above.

[hanw]

If user program uses DPDKMeter or DPDKCounter, then it needs to include dpdk/psa_ext.p4

[kamleshbhalui]

All the suggestions incorporated, please review again.

[hanw]

Looks good to me

[usha1830]

We will have to add support for DpdkMeter and DpdkCounter in bfRT json and P4info.

[usha1830]

These are unrelated changes. Please remove it while creating public PR.