When several pmd threads race for the meter lock, the time 'now' each pmd runs 
will
have no guarantee in the order they really happen.

For example, pmd A runs meter at time x, and pmd B runs meter at time x+1. 
However
if pmd B get the lock and run meter first, meter->used will be assgined to x+1, 
and
then pmd A would run with delta_t equal to 0, the calculation for buckets is 
good now.
But pmd A will set meter->used to x, move the the time back a little.

If pmd B or other pmd runs it again, it will get a delta_t greater than it 
should be
and buckets is bigger than we expect. With really high thoughput and more than 
four
pmd threads, the deviation here will grow up very high.

This fix makes sure the time will only be added up.

Signed-off-by: Wan Junjie <[email protected]>
---
 lib/dpif-netdev.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 3acf5512a..26b584579 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -7160,8 +7160,7 @@ 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 */
+    long long int long_delta_t, delta_t;
     const size_t cnt = dp_packet_batch_size(packets_);
     uint32_t bytes, volume;
     int exceeded_band[NETDEV_MAX_BURST];
@@ -7184,7 +7183,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct 
dp_packet_batch *packets_,
 
     ovs_mutex_lock(&meter->lock);
     /* All packets will hit the meter at the same time. */
-    long_delta_t = now / 1000 - meter->used / 1000; /* msec */
+    long_delta_t = now - meter->used;
 
     if (long_delta_t < 0) {
         /* This condition means that we have several threads fighting for a
@@ -7196,11 +7195,11 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct 
dp_packet_batch *packets_,
 
     /* 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 = (long_delta_t > (long long int)meter->max_delta_t * 1000)
+        ? (meter->max_delta_t * 1000) : long_delta_t;
 
     /* Update meter stats. */
-    meter->used = now;
+    meter->used += long_delta_t;
     meter->packet_count += cnt;
     bytes = 0;
     DP_PACKET_BATCH_FOR_EACH (i, packet, packets_) {
@@ -7228,7 +7227,7 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct 
dp_packet_batch *packets_,
         band = &meter->bands[m];
         max_bucket_size = band->burst_size * 1000ULL;
         /* Update band's bucket. */
-        band->bucket += (uint64_t) delta_t * band->rate;
+        band->bucket += (delta_t * band->rate / 1000);
         if (band->bucket > max_bucket_size) {
             band->bucket = max_bucket_size;
         }
-- 
2.33.0

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to