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:[email protected]]
发送时间: 2016年12月27日 2:38
收件人: zhangsha (A)
抄送: [email protected]; caihe; [email protected]; [email protected]
主题: 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 <[email protected]>
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 <[email protected]>
Signed-off-by: Alex Wang <[email protected]>
Acked-by: Ben Pfaff <[email protected]>
---
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
_______________________________________________
discuss mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
