diff mbox series

[ovs-dev,v2] Improve concurrency by adjust flow-limit

Message ID 20210517072248.1743-1-taoyunxiang@cmss.chinamobile.com
State Superseded
Headers show
Series [ovs-dev,v2] Improve concurrency by adjust flow-limit | expand

Commit Message

taoyunxiang May 17, 2021, 7:22 a.m. UTC
OVS uses a set of gentle mechanisms to control the number of flows in 
datapath. However, it is difficult for the number of datapath flows to 
reach a high level in a short time.

Through some tests with Apache benchmark, I found the point is that
the value of flow-limit increases too slowly, but decreases very quickly. 

This change has two improvements. One is the flow-limit will adjust more 
linearly. The other one is, when the duration is small, the flow-limit will
increase faster.

Through this change, the test result of Apache benchmark can reach 12k from 5k,
the test command is as follows:

ab -n 200000 -c 500 -r http://1.1.1.2/

v1->v2: Make flow-limit changes linearly. Add a limitation to make flow_limit
won't overflow UINT_MAX.


Signed-off-by: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
Cc: Ben Pfaff <blp@ovn.org>

---
 ofproto/ofproto-dpif-upcall.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

Comments

0-day Robot May 17, 2021, 7:59 a.m. UTC | #1
Bleep bloop.  Greetings Tao YunXiang, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line has trailing whitespace
#50 FILE: ofproto/ofproto-dpif-upcall.c:970:
             * and float is to make the flow_limit change more linear. */ 

WARNING: Line is 80 characters long (recommended limit is 79)
#53 FILE: ofproto/ofproto-dpif-upcall.c:973:
                flow_limit = (int) ((float) (flow_limit) / (duration / 1000.0));

Lines checked: 65, Warnings: 2, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
taoyunupt May 24, 2021, 6:10 a.m. UTC | #2
Hi Ben,
    Please help to review this patch ,which to optimize the flow-limit adjustment.


    The v1 patch links to http://patchwork.ozlabs.org/project/openvswitch/patch/20210511075200.19037-1-taoyunxiang@cmss.chinamobile.com/




Thanks,
Yun


At 2021-05-17 15:22:48, "Tao YunXiang" <taoyunxiang@cmss.chinamobile.com> wrote:
>OVS uses a set of gentle mechanisms to control the number of flows in 
>datapath. However, it is difficult for the number of datapath flows to 
>reach a high level in a short time.
>
>Through some tests with Apache benchmark, I found the point is that
>the value of flow-limit increases too slowly, but decreases very quickly. 
>
>This change has two improvements. One is the flow-limit will adjust more 
>linearly. The other one is, when the duration is small, the flow-limit will
>increase faster.
>
>Through this change, the test result of Apache benchmark can reach 12k from 5k,
>the test command is as follows:
>
>ab -n 200000 -c 500 -r http://1.1.1.2/
>
>v1->v2: Make flow-limit changes linearly. Add a limitation to make flow_limit
>won't overflow UINT_MAX.
>
>
>Signed-off-by: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
>Cc: Ben Pfaff <blp@ovn.org>
>
>---
> ofproto/ofproto-dpif-upcall.c | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
>diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
>index ccf97266c..805409165 100644
>--- a/ofproto/ofproto-dpif-upcall.c
>+++ b/ofproto/ofproto-dpif-upcall.c
>@@ -963,14 +963,20 @@ udpif_revalidator(void *arg)
> 
>             duration = MAX(time_msec() - start_time, 1);
>             udpif->dump_duration = duration;
>-            if (duration > 2000) {
>-                flow_limit /= duration / 1000;
>-            } else if (duration > 1300) {
>-                flow_limit = flow_limit * 3 / 4;
>-            } else if (duration < 1000 &&
>-                       flow_limit < n_flows * 1000 / duration) {
>-                flow_limit += 1000;
>+
>+            /* Use duration as a reference, adjust the value of flow_limit,
>+             * when the duration is short, increase the flow_limit, and vice
>+             * versa. The purpose of using multiple conversions between int
>+             * and float is to make the flow_limit change more linear. */ 
>+
>+            if (duration >= 1000) {
>+                flow_limit = (int) ((float) (flow_limit) / (duration / 1000.0));
>+            } else if (flow_limit * (1000.0 / duration) >= UINT_MAX) {
>+                flow_limit = ofproto_flow_limit;
>+            } else {
>+                flow_limit = (int) (flow_limit * (1000.0 / duration));
>             }
>+
>             flow_limit = MIN(ofproto_flow_limit, MAX(flow_limit, 1000));
>             atomic_store_relaxed(&udpif->flow_limit, flow_limit);
> 
>-- 
>2.17.1
>
>
>
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index ccf97266c..805409165 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -963,14 +963,20 @@  udpif_revalidator(void *arg)
 
             duration = MAX(time_msec() - start_time, 1);
             udpif->dump_duration = duration;
-            if (duration > 2000) {
-                flow_limit /= duration / 1000;
-            } else if (duration > 1300) {
-                flow_limit = flow_limit * 3 / 4;
-            } else if (duration < 1000 &&
-                       flow_limit < n_flows * 1000 / duration) {
-                flow_limit += 1000;
+
+            /* Use duration as a reference, adjust the value of flow_limit,
+             * when the duration is short, increase the flow_limit, and vice
+             * versa. The purpose of using multiple conversions between int
+             * and float is to make the flow_limit change more linear. */ 
+
+            if (duration >= 1000) {
+                flow_limit = (int) ((float) (flow_limit) / (duration / 1000.0));
+            } else if (flow_limit * (1000.0 / duration) >= UINT_MAX) {
+                flow_limit = ofproto_flow_limit;
+            } else {
+                flow_limit = (int) (flow_limit * (1000.0 / duration));
             }
+
             flow_limit = MIN(ofproto_flow_limit, MAX(flow_limit, 1000));
             atomic_store_relaxed(&udpif->flow_limit, flow_limit);