[ovs-discuss] \\答复: It takes too long to create 512 bridges in ovs2.5.0

Ben Pfaff blp at ovn.org
Fri Jan 13 20:44:22 UTC 2017


This does seem like a worthwhile change.  Will you submit it as an
official patch?

Thanks,

Ben.

On Thu, Jan 12, 2017 at 08:33:44AM +0000, zhangsha (A) wrote:
> Hi, Ben
> Thanks for your reply, which means a lot to me. 
> It is much faster if I create 512 bridges in a single transaction, like restarting service. But This is not feasible in all the tests. Sometimes I need it to be more extensive(e.g., creating a new virtual machine at any time).
> So, I am thinking maybe I can move the xlate_txn_start() and xlate_txn_commit() out of the loop(See the following patch, please)?
> As a global variable, new_xcfg only needs to be allocated once time at the beginning and to be commited at the end, other than in loop body(This causes many unnecessary allocations, free, and synchronize, I think).
> 
> ofproto/ofproto-dpif.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index b101d22..0019d0a 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -667,6 +667,7 @@ type_run(const char *type)
>          }
>          backer->need_revalidate = 0;
>  
> +        xlate_txn_start();
>          HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
>              struct ofport_dpif *ofport;
>              struct ofbundle *bundle;
> @@ -675,7 +676,6 @@ type_run(const char *type)
>                  continue;
>              }
>  
> -            xlate_txn_start();
>              xlate_ofproto_set(ofproto, ofproto->up.name,
>                                ofproto->backer->dpif, ofproto->ml,
>                                ofproto->stp, ofproto->rstp, ofproto->ms,
> @@ -709,9 +709,9 @@ type_run(const char *type)
>                                   ofport->up.pp.state, ofport->is_tunnel,
>                                   ofport->may_enable);
>              }
> -            xlate_txn_commit();
>          }
>  
> +        xlate_txn_commit();
>          udpif_revalidate(backer->udpif);
>      }
> 
> -----邮件原件-----
> 发件人: Ben Pfaff [mailto:blp at ovn.org] 
> 发送时间: 2016年12月27日 2:38
> 收件人: zhangsha (A)
> 抄送: ovs-discuss at openvswitch.org; caihe; wryan at nicira.com; ethan at nicira.com
> 主题: Re: [ovs-discuss] It takes too long to create 512 bridges in ovs2.5.0
> 
> On Wed, Dec 14, 2016 at 02:33:39PM +0000, zhangsha (A) wrote:
> > Hi, all
> > 
> > In my test scenario, I found that it takes too long time, 488s, to create 512 bridges in ovs 2.5.0.
> > After analyzing the code and some tests, I found that more than 70% of the time was consumed in the implementation of function xlate_txn_commit.
> > Function type_run calls xlate_txn_start and xlate_txn_commit(calls ovsrcu_synchronize) to implement RCU locking in each cycle of the loop.
> > Function ovsrcu_synchronize() was called too much times if there are certain number bridges, which means a lot of time waste.
> > 
> > So, I was thinking if I can use ovsrcu_postpone replacing ovsrcu_synchronize to shorten the execution time of type_run when there are certain number bridges.
> > I have tested this, and the result indicates that the time of creating 512 bridges will be short down to 269s. Result and my patch is as follows:
> > 
> > 
> > ovsrcu_postpone
> > 
> > ovsrcu_synchronize
> > 
> > ovs
> > 
> > 269
> > 
> > 488
> > 
> > 
> > 
> > This is my patch:
> > ofproto/ofproto-dpif-xlate.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/ofproto/ofproto-dpif-xlate.c 
> > b/ofproto/ofproto-dpif-xlate.c index d5f18b6..59f4b5c 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -865,8 +865,9 @@ xlate_txn_commit(void)
> >      struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
> >      ovsrcu_set(&xcfgp, new_xcfg);
> > -    ovsrcu_synchronize();
> > -    xlate_xcfg_free(xcfg);
> > +    ovsrcu_postpone(xlate_xcfg_free, xcfg);
> >      new_xcfg = NULL;
> > }
> > 
> > Whether this modification will introduce any problems?
> 
> Yes.  See the following commit.
> 
> It sounds like you're creating each bridge in a separate database transaction.  If you create them in a single transaction, it will be much faster.
> 
> --8<--------------------------cut here-------------------------->8--
> 
> From 40a9c4c26be9b59c3494dd5900c21015ea7d27d4 Mon Sep 17 00:00:00 2001
> From: Alex Wang <alexw at nicira.com>
> Date: Fri, 7 Nov 2014 13:02:05 -0800
> Subject: [PATCH] ofproto-dpif-xlate: Allow direct destroy of previous config.
> 
> Before this commit, the ofproto-dpif-xlate module uses ovs-rcu to postpone the destroy of previous configuration.  However, the delayed close of object like 'struct netdev' could cause failure in immediate re-add or reconfigure of the same device.
> 
> To fix the above issue, this commit makes the ofproto-dpif-xlate module call ovsrcu_synchronize(), which waits for all threads to finish the use of reference to previous config.  Then, the module can just directly destroy the previous config.
> 
> Reported-by: Cian Ferriter <cian.ferriter at intel.com>
> Signed-off-by: Alex Wang <alexw at nicira.com>
> Acked-by: Ben Pfaff <blp at nicira.com>
> ---
>  ofproto/ofproto-dpif-xlate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 6bf6d6d..f781bc5 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -672,8 +672,8 @@ xlate_txn_commit(void)
>      struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
>  
>      ovsrcu_set(&xcfgp, new_xcfg);
> -    ovsrcu_postpone(xlate_xcfg_free, xcfg);
> -
> +    ovsrcu_synchronize();
> +    xlate_xcfg_free(xcfg);
>      new_xcfg = NULL;
>  }
>  
> --
> 2.10.2
> 


More information about the discuss mailing list