nsg-ethz / p4-utils

Extension to Mininet that makes P4 networks easier to build
GNU General Public License v2.0
175 stars 65 forks source link

Meter rates: docs state units/second, while code uses units/microsecond #74

Open Trigary opened 1 week ago

Trigary commented 1 week ago

The Thrift API's meter_array_set_rates, meter_set_rates, and meter_get_rates methods all state (in their documentation) that the meter rates are specified in units/second (where units is bytes or packets). But the actual implementations work with units/microsecond.

For example, meter_array_set_rates passes the specified rate value to the BmMeterRateConfig class, which specifies that it works with units_per_micros.

I do not know whether this bug also exists in the P4Runtime API.

In my opinion, a public API dealing with units/second is preferred, and the implementation should be responsible for converting the rates to/from units/microsecond. But fixing this bug by changing the implementation would be a breaking change, potentially breaking existing code that depends on this library. Changing the documentation would also fix this issue and it would not be a breaking change, but I believe the end result would be less favorable.