Message ID | 20210517072248.1743-1-taoyunxiang@cmss.chinamobile.com |
---|---|
State | Superseded |
Headers | show |
Series | [ovs-dev,v2] Improve concurrency by adjust flow-limit | expand |
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
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 --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);
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(-)