On Wed, Apr 10, 2019 at 5:14 AM Ilya Maximets <[email protected]> wrote: > > On 09.04.2019 20:59, William Tu wrote: > > When testing packet rate around 1Mpps with meter enabled, the frequency > > of hitting meter action becomes much higher, around 30us each time. > > As a result, the meter's calculation of 'uint32_t delta_t' becomes > > always 0 and meter action has no effect. This is due to the previous > > commit 05f9e707e194 divides the delta by 1000, in order to convert to > > msec granularity. The patch fixes it by using double to hold the delta > > value. > > > > Fixes: 05f9e707e194 ("dpif-netdev: Use microsecond granularity.") > > Cc: Ilya Maximets <[email protected]> > > Cc: Yi-Hung Wei <[email protected]> > > Signed-off-by: William Tu <[email protected]> > > --- > > Hi. Good catch. > > In fact, we may return previous behaviour by just dividing before > subtracting like this: > long_delta_t = now / 1000 - meter->used / 1000; /* msec */
But this is the same, the long_delta_t is still 0 because my (now - meter->used is less than 1000) > > I'm not much familiar with meters here. Is it OK if 'band->bucket' > grows only if we're crossing millisecond boundary? I think that will also work. William > Your change makes it grow smoothly on each call which wasn't the > case previously. > > Jarno, maybe you have some thoughts about this? > > Best regards, Ilya Maximets. > > > lib/dpif-netdev.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > > index 4d6d0c372236..61d0e9f2bf69 100644 > > --- a/lib/dpif-netdev.c > > +++ b/lib/dpif-netdev.c > > @@ -5525,8 +5525,8 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct > > dp_packet_batch *packets_, > > struct dp_meter *meter; > > struct dp_meter_band *band; > > struct dp_packet *packet; > > - long long int long_delta_t; /* msec */ > > - uint32_t delta_t; /* msec */ > > + double double_delta_t; /* msec */ > > + double delta_t; /* msec */ > > const size_t cnt = dp_packet_batch_size(packets_); > > uint32_t bytes, volume; > > int exceeded_band[NETDEV_MAX_BURST]; > > @@ -5549,12 +5549,12 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct > > dp_packet_batch *packets_, > > memset(exceeded_rate, 0, cnt * sizeof *exceeded_rate); > > > > /* All packets will hit the meter at the same time. */ > > - long_delta_t = (now - meter->used) / 1000; /* msec */ > > + double_delta_t = (double)(now - meter->used) / 1000.0; /* msec */ > > > > /* Make sure delta_t will not be too large, so that bucket will not > > * wrap around below. */ > > - delta_t = (long_delta_t > (long long int)meter->max_delta_t) > > - ? meter->max_delta_t : (uint32_t)long_delta_t; > > + delta_t = (double_delta_t > (long long int)meter->max_delta_t) > > + ? (double)meter->max_delta_t : double_delta_t; > > > > /* Update meter stats. */ > > meter->used = now; > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
