[ovs-dev] [PATCH RESEND v2 1/3] dpif-netdev: Fix the meter buckets overflow.

xiangxia.m.yue at gmail.com xiangxia.m.yue at gmail.com
Wed Feb 24 12:21:48 UTC 2021


From: Tonghao Zhang <xiangxia.m.yue at gmail.com>

When setting the meter rate to 4.3+Gbps, there is an overflow, the
meters don't work as expected.

$ ovs-ofctl -O OpenFlow13 add-meter br-int "meter=1 kbps stats bands=type=drop rate=4294968"

Before the patch, the buckets of meters was stored in its burst_size
of ofputil_meter_band. It was overflow when we set the rate to 4294968.
This patch don't change the public API and structure. This patch remove
the "up" from dp_meter_band, and introduce the type, rate to datapath's
meter bands. Then datapath don't depend upper layer.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue at gmail.com>
---
 lib/dpif-netdev.c | 42 ++++++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4381c618f1be..614f6fef6b77 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -279,8 +279,10 @@ static bool dpcls_lookup(struct dpcls *cls,
     ( 1 << OFPMBT13_DROP )
 
 struct dp_meter_band {
-    struct ofputil_meter_band up; /* type, prec_level, pad, rate, burst_size */
-    uint32_t bucket; /* In 1/1000 packets (for PKTPS), or in bits (for KBPS) */
+    uint16_t type;
+    uint32_t rate;
+    uint32_t burst_size;
+    uint64_t bucket; /* In 1/1000 packets (for PKTPS), or in bits (for KBPS) */
     uint64_t packet_count;
     uint64_t byte_count;
 };
@@ -6203,12 +6205,14 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_,
     /* Update all bands and find the one hit with the highest rate for each
      * packet (if any). */
     for (int m = 0; m < meter->n_bands; ++m) {
-        band = &meter->bands[m];
+        uint64_t max_bucket_size;
 
+        band = &meter->bands[m];
         /* Update band's bucket. */
-        band->bucket += delta_t * band->up.rate;
-        if (band->bucket > band->up.burst_size) {
-            band->bucket = band->up.burst_size;
+        max_bucket_size = band->rate * 1000ULL;
+        band->bucket += (uint64_t)delta_t * band->rate;
+        if (band->bucket > max_bucket_size) {
+            band->bucket = max_bucket_size;
         }
 
         /* Drain the bucket for all the packets, if possible. */
@@ -6226,8 +6230,8 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_,
                  * (Only one band will be fired by a packet, and that
                  * can be different for each packet.) */
                 for (int i = band_exceeded_pkt; i < cnt; i++) {
-                    if (band->up.rate > exceeded_rate[i]) {
-                        exceeded_rate[i] = band->up.rate;
+                    if (band->rate > exceeded_rate[i]) {
+                        exceeded_rate[i] = band->rate;
                         exceeded_band[i] = m;
                     }
                 }
@@ -6246,8 +6250,8 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_,
                         /* Update the exceeding band for the exceeding packet.
                          * (Only one band will be fired by a packet, and that
                          * can be different for each packet.) */
-                        if (band->up.rate > exceeded_rate[i]) {
-                            exceeded_rate[i] = band->up.rate;
+                        if (band->rate > exceeded_rate[i]) {
+                            exceeded_rate[i] = band->rate;
                             exceeded_band[i] = m;
                         }
                     }
@@ -6324,21 +6328,15 @@ dpif_netdev_meter_set(struct dpif *dpif, ofproto_meter_id meter_id,
     for (i = 0; i < config->n_bands; ++i) {
         uint32_t band_max_delta_t;
 
-        /* Set burst size to a workable value if none specified. */
-        if (config->bands[i].burst_size == 0) {
-            config->bands[i].burst_size = config->bands[i].rate;
-        }
-
-        meter->bands[i].up = config->bands[i];
-        /* Convert burst size to the bucket units: */
-        /* pkts => 1/1000 packets, kilobits => bits. */
-        meter->bands[i].up.burst_size *= 1000;
-        /* Initialize bucket to empty. */
-        meter->bands[i].bucket = 0;
+        meter->bands[i].type = config->bands[i].type;
+        meter->bands[i].rate = config->bands[i].rate;
+        meter->bands[i].burst_size = config->bands[i].burst_size;
+        /* Start with a full bucket. */
+        meter->bands[i].bucket = meter->bands[i].rate * 1000ULL;
 
         /* Figure out max delta_t that is enough to fill any bucket. */
         band_max_delta_t
-            = meter->bands[i].up.burst_size / meter->bands[i].up.rate;
+            = meter->bands[i].bucket / meter->bands[i].rate;
         if (band_max_delta_t > meter->max_delta_t) {
             meter->max_delta_t = band_max_delta_t;
         }
-- 
2.27.0



More information about the dev mailing list