Re: [ovs-dev] [PATCH v2 1/5] rstp: Init a recursive mutex for rstp.

[nickcooper-zhangtonghao]

> On May 19, 2017, at 6:47 AM, Ben Pfaff  wrote:
> 
> On Wed, May 10, 2017 at 04:15:02AM -0700, nickcooper-zhangtonghao wrote:
>> * This patch will be used for next patch. The 'rstp/show' command,
>> which uses the mutex, calls functions which also use the mutex.
>> We should init it as a recursive mutex.
>> 
>> * Some rstp tests of OvS, which run with ovstest, does not run rstp_init
>> in the bridge_init. We should call rstp_init when creating the rstp
>> and stp also do that, otherwise tests fail.
>> 
>> * This patch remove the rstp_mutex in header file and make it static
>> in c file because we only use it in lib/rstp.c
>> 
>> Signed-off-by: nickcooper-zhangtonghao 
> 
> Thanks for working on the rstp code!
> 
> This patch causes a huge number of build failures with Clang, like this:
> 
>In file included from ../lib/rstp.c:32:
>../lib/rstp.h:135:18: error: use of undeclared identifier 'rstp_mutex'
>../include/openvswitch/compiler.h:136:57: note: expanded from macro 
> 'OVS_EXCLUDED'
> 
> I think that's the only reason that rstp_mutex is global.


Thanks for your review, the v3 patches don’t remove the rstp_mutex from header 
file
and we can build it with gcc and clang. Because of recursive mutex, I remove 
the 
OVS_EXCLUDED in the lib/rstp.[ch] files. Thanks very much.


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 1/5] rstp: Init a recursive mutex for rstp.

[Ben Pfaff]

On Wed, May 10, 2017 at 04:15:02AM -0700, nickcooper-zhangtonghao wrote:
> * This patch will be used for next patch. The 'rstp/show' command,
> which uses the mutex, calls functions which also use the mutex.
> We should init it as a recursive mutex.
> 
> * Some rstp tests of OvS, which run with ovstest, does not run rstp_init
> in the bridge_init. We should call rstp_init when creating the rstp
> and stp also do that, otherwise tests fail.
> 
> * This patch remove the rstp_mutex in header file and make it static
> in c file because we only use it in lib/rstp.c
> 
> Signed-off-by: nickcooper-zhangtonghao 

Thanks for working on the rstp code!

This patch causes a huge number of build failures with Clang, like this:

In file included from ../lib/rstp.c:32:
../lib/rstp.h:135:18: error: use of undeclared identifier 'rstp_mutex'
../include/openvswitch/compiler.h:136:57: note: expanded from macro 
'OVS_EXCLUDED'

I think that's the only reason that rstp_mutex is global.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 1/5] rstp: Init a recursive mutex for rstp.

[nickcooper-zhangtonghao]

* This patch will be used for next patch. The 'rstp/show' command,
which uses the mutex, calls functions which also use the mutex.
We should init it as a recursive mutex.

* Some rstp tests of OvS, which run with ovstest, does not run rstp_init
in the bridge_init. We should call rstp_init when creating the rstp
and stp also do that, otherwise tests fail.

* This patch remove the rstp_mutex in header file and make it static
in c file because we only use it in lib/rstp.c

Signed-off-by: nickcooper-zhangtonghao 
---
 lib/rstp.c | 15 ---
 lib/rstp.h |  6 --
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/lib/rstp.c b/lib/rstp.c
index 907a907..6f1c1e3 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -50,7 +50,7 @@
 
 VLOG_DEFINE_THIS_MODULE(rstp);
 
-struct ovs_mutex rstp_mutex = OVS_MUTEX_INITIALIZER;
+static struct ovs_mutex rstp_mutex;
 
 static struct ovs_list all_rstps__ = OVS_LIST_INITIALIZER(_rstps__);
 static struct ovs_list *const all_rstps OVS_GUARDED_BY(rstp_mutex) = 
_rstps__;
@@ -239,8 +239,15 @@ void
 rstp_init(void)
 OVS_EXCLUDED(rstp_mutex)
 {
-unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, rstp_unixctl_tcn,
- NULL);
+static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
+
+if (ovsthread_once_start()) {
+ovs_mutex_init_recursive(_mutex);
+
+unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, 
rstp_unixctl_tcn,
+ NULL);
+ovsthread_once_done();
+}
 }
 
 /* Creates and returns a new RSTP instance that initially has no ports. */
@@ -255,6 +262,8 @@ rstp_create(const char *name, rstp_identifier 
bridge_address,
 
 VLOG_DBG("Creating RSTP instance");
 
+rstp_init();
+
 rstp = xzalloc(sizeof *rstp);
 rstp->name = xstrdup(name);
 
diff --git a/lib/rstp.h b/lib/rstp.h
index 4942d59..78e07fb 100644
--- a/lib/rstp.h
+++ b/lib/rstp.h
@@ -36,12 +36,6 @@
 #include "compiler.h"
 #include "util.h"
 
-/* Thread Safety: Callers passing in RSTP and RSTP port object
- * pointers must hold a reference to the passed object to ensure that
- * the object does not become stale while it is being accessed. */
-
-extern struct ovs_mutex rstp_mutex;
-
 #define RSTP_MAX_PORTS 4095
 
 struct dp_packet;
-- 
1.8.3.1



___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev