[ovs-dev] ofproto-dpif-upcall: Fix for flow limit issue in revalidator

Message ID 1533256463-21018-1-git-send-email-vishal.deep.ajmera@ericsson.com
State Accepted
Headers show
Series
  • [ovs-dev] ofproto-dpif-upcall: Fix for flow limit issue in revalidator
Related show

Commit Message

Vishal Deep Ajmera Aug. 3, 2018, 12:34 a.m.
When the revalidator thread takes a long time to dump data path
flows (e.g. due to busy CPU), it reduces the maximum limit for
new flows that can be added. This results in more upcalls for
packets which do not find data path flows and temporarily reduces
overall throughput. When the situation improves and the revalidator
gets enough CPU cycles, it should increase the flow limit allowing
more flows to get inserted.

Currently the flow limit does not increase if the existing number of
flows is less than 2000 and does not allow any new flows due to
incorrect condition check. This results in a permanent drop in
performance in OVS with no automatic recovery.

This patch fixes the conditional check for increasing flow limit.

Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com>
---
 ofproto/ofproto-dpif-upcall.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Ben Pfaff Aug. 15, 2018, 8:10 p.m. | #1
On Fri, Aug 03, 2018 at 06:04:23AM +0530, Vishal Deep Ajmera wrote:
> When the revalidator thread takes a long time to dump data path
> flows (e.g. due to busy CPU), it reduces the maximum limit for
> new flows that can be added. This results in more upcalls for
> packets which do not find data path flows and temporarily reduces
> overall throughput. When the situation improves and the revalidator
> gets enough CPU cycles, it should increase the flow limit allowing
> more flows to get inserted.
> 
> Currently the flow limit does not increase if the existing number of
> flows is less than 2000 and does not allow any new flows due to
> incorrect condition check. This results in a permanent drop in
> performance in OVS with no automatic recovery.
> 
> This patch fixes the conditional check for increasing flow limit.
> 
> Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com>

Thanks, applied to master.

Patch

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 85f5792..6222207 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -931,8 +931,8 @@  udpif_revalidator(void *arg)
                 flow_limit /= duration / 1000;
             } else if (duration > 1300) {
                 flow_limit = flow_limit * 3 / 4;
-            } else if (duration < 1000 && n_flows > 2000
-                       && flow_limit < n_flows * 1000 / duration) {
+            } else if (duration < 1000 &&
+                       flow_limit < n_flows * 1000 / duration) {
                 flow_limit += 1000;
             }
             flow_limit = MIN(ofproto_flow_limit, MAX(flow_limit, 1000));