diff mbox series

[ovs-dev,RESEND,ovs,v3,1/4] dpif-netdev: Fix the meter buckets overflow.

Message ID 20210303144658.47858-2-xiangxia.m.yue@gmail.com
State Accepted
Headers show
Series dpif-netdev meter fixes bug and improves. | expand

Commit Message

Tonghao Zhang March 3, 2021, 2:46 p.m. UTC 
From: Tonghao Zhang <xiangxia.m.yue@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"

It was overflow when we set the rate to 4294968, because "burst_size" in
the ofputil_meter_band is uint32_t type. This patch remove the "up"
in the dp_meter_band struction, and introduce "rate", "burst_size" and
"bucket" (uint64_t) to userspace datapath's meter band. This patch don't
change the public API and structure.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 lib/dpif-netdev.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

Comments

Ilya Maximets March 30, 2021, 8:52 p.m. UTC | #1 
On 3/3/21 3:46 PM, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@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"
> 
> It was overflow when we set the rate to 4294968, because "burst_size" in
> the ofputil_meter_band is uint32_t type. This patch remove the "up"
> in the dp_meter_band struction, and introduce "rate", "burst_size" and
> "bucket" (uint64_t) to userspace datapath's meter band. This patch don't
> change the public API and structure.
> 
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---

Thanks!  Applied to master and backported down to 2.12.

Bets regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4381c618f1be..78f3eef5381b 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -279,8 +279,9 @@  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) */
+    uint32_t rate;
+    uint64_t bucket; /* In 1/1000 packets (for PKTPS), or in bits (for KBPS) */
+    uint64_t burst_size;
     uint64_t packet_count;
     uint64_t byte_count;
 };
@@ -6206,9 +6207,9 @@  dp_netdev_run_meter(struct dp_netdev *dp, struct dp_packet_batch *packets_,
         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;
+        band->bucket += (uint64_t) delta_t * band->rate;
+        if (band->bucket > band->burst_size) {
+            band->bucket = band->burst_size;
         }
 
         /* Drain the bucket for all the packets, if possible. */
@@ -6226,8 +6227,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 +6247,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;
                         }
                     }
@@ -6329,16 +6330,13 @@  dpif_netdev_meter_set(struct dpif *dpif, ofproto_meter_id meter_id,
             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].rate = config->bands[i].rate;
+        meter->bands[i].burst_size = config->bands[i].burst_size * 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].burst_size / meter->bands[i].rate;
         if (band_max_delta_t > meter->max_delta_t) {
             meter->max_delta_t = band_max_delta_t;
         }