| Message ID | 1555707869-12349-1-git-send-email-u9012063@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,PATCHv2] dpif-netdev: fix meter at high packet rate. | expand |
On Fri, Apr 19, 2019 at 2:04 PM William Tu <u9012063@gmail.com> 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 updating the time when across > millisecond boundary. > > Fixes: 05f9e707e194 ("dpif-netdev: Use microsecond granularity.") > Cc: Ilya Maximets <i.maximets@samsung.com> > Cc: Yi-Hung Wei <yihung.wei@gmail.com> > Signed-off-by: William Tu <u9012063@gmail.com> > --- > lib/dpif-netdev.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index c20f875ee097..7a572df641f6 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -5549,7 +5549,7 @@ 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 */ > + long_delta_t = now / 1000 - meter->used / 1000; /* msec */ > > /* Make sure delta_t will not be too large, so that bucket will not > * wrap around below. */ > @@ -5557,7 +5557,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_, > ? meter->max_delta_t : (uint32_t)long_delta_t; > > /* Update meter stats. */ > - meter->used = now; > + meter->used = delta_t > 0 ? now : meter->used; Hi William, Thanks for the patch. I think the check here is not necessary. Without the check here, we can still refill the bucket on every millisecond boundary. Best, -Yi-Hung
On Fri, Apr 19, 2019 at 3:18 PM Yi-Hung Wei <yihung.wei@gmail.com> wrote: > On Fri, Apr 19, 2019 at 2:04 PM William Tu <u9012063@gmail.com> 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 updating the time when across > > millisecond boundary. > > > > Fixes: 05f9e707e194 ("dpif-netdev: Use microsecond granularity.") > > Cc: Ilya Maximets <i.maximets@samsung.com> > > Cc: Yi-Hung Wei <yihung.wei@gmail.com> > > Signed-off-by: William Tu <u9012063@gmail.com> > > --- > > lib/dpif-netdev.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > > index c20f875ee097..7a572df641f6 100644 > > --- a/lib/dpif-netdev.c > > +++ b/lib/dpif-netdev.c > > @@ -5549,7 +5549,7 @@ 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 */ > > + long_delta_t = now / 1000 - meter->used / 1000; /* msec */ > > > > /* Make sure delta_t will not be too large, so that bucket will not > > * wrap around below. */ > > @@ -5557,7 +5557,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct > dp_packet_batch *packets_, > > ? meter->max_delta_t : (uint32_t)long_delta_t; > > > > /* Update meter stats. */ > > - meter->used = now; > > + meter->used = delta_t > 0 ? now : meter->used; > Hi William, > > Thanks for the patch. > > I think the check here is not necessary. Without the check here, we > can still refill the bucket on every millisecond boundary. > > Yes you're right. I will resend v3 patch Thanks William > Best, > > -Yi-Hung >
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index c20f875ee097..7a572df641f6 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -5549,7 +5549,7 @@ 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 */ + long_delta_t = now / 1000 - meter->used / 1000; /* msec */ /* Make sure delta_t will not be too large, so that bucket will not * wrap around below. */ @@ -5557,7 +5557,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_, ? meter->max_delta_t : (uint32_t)long_delta_t; /* Update meter stats. */ - meter->used = now; + meter->used = delta_t > 0 ? now : meter->used; meter->packet_count += cnt; bytes = 0; DP_PACKET_BATCH_FOR_EACH (i, packet, packets_) {
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 updating the time when across millisecond boundary. Fixes: 05f9e707e194 ("dpif-netdev: Use microsecond granularity.") Cc: Ilya Maximets <i.maximets@samsung.com> Cc: Yi-Hung Wei <yihung.wei@gmail.com> Signed-off-by: William Tu <u9012063@gmail.com> --- lib/dpif-netdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)