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

[nickcooper-zhangtonghao]

> On Jun 1, 2017, at 7:01 AM, Ben Pfaff  wrote:
> 
> On Fri, May 19, 2017 at 12:20:39AM -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.
>> 
>> * Because of recursive mutex, this patch remove the OVS_EXCLUDED in
>> list/rstp.[ch] files.
>> 
>> * 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.
>> 
>> Signed-off-by: nickcooper-zhangtonghao 
> 
> I think I'd prefer to add internal versions of functions that don't take
> the locks, instead.  Did you consider that approach?

The v4 patches has been submitted. We use the internal functions to implement 
the ‘rstp/show’. 


http://patchwork.ozlabs.org/patch/769478/ 

http://patchwork.ozlabs.org/patch/769476/ 

http://patchwork.ozlabs.org/patch/769477/ 

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

[ovs-dev] [PATCH v3 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.

* Because of recursive mutex, this patch remove the OVS_EXCLUDED in
list/rstp.[ch] files.

* 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.

Signed-off-by: nickcooper-zhangtonghao 
---
 lib/rstp.c |  54 ++---
 lib/rstp.h | 131 +
 2 files changed, 56 insertions(+), 129 deletions(-)

diff --git a/lib/rstp.c b/lib/rstp.c
index 907a907..9ad1b0d 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;
+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__;
@@ -161,7 +161,6 @@ rstp_port_role_name(enum rstp_port_role role)
  * while taking a new reference. */
 struct rstp *
 rstp_ref(struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
 {
 if (rstp) {
 ovs_refcount_ref(>ref_cnt);
@@ -172,7 +171,6 @@ rstp_ref(struct rstp *rstp)
 /* Frees RSTP struct when reference count reaches zero. */
 void
 rstp_unref(struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
 {
 if (rstp && ovs_refcount_unref_relaxed(>ref_cnt) == 1) {
 ovs_mutex_lock(_mutex);
@@ -197,7 +195,6 @@ rstp_unref(struct rstp *rstp)
  * port_number). */
 int
 rstp_port_get_number(const struct rstp_port *p)
-OVS_EXCLUDED(rstp_mutex)
 {
 int number;
 
@@ -214,7 +211,6 @@ static void rstp_unixctl_tcn(struct unixctl_conn *, int 
argc,
 /* Decrements the State Machines' timers. */
 void
 rstp_tick_timers(struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 decrease_rstp_port_timers__(rstp);
@@ -225,7 +221,6 @@ rstp_tick_timers(struct rstp *rstp)
 void
 rstp_port_received_bpdu(struct rstp_port *rp, const void *bpdu,
 size_t bpdu_size)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 /* Only process packets on ports that have RSTP enabled. */
@@ -237,10 +232,16 @@ rstp_port_received_bpdu(struct rstp_port *rp, const void 
*bpdu,
 
 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. */
@@ -249,12 +250,13 @@ rstp_create(const char *name, rstp_identifier 
bridge_address,
 void (*send_bpdu)(struct dp_packet *bpdu, void *port_aux,
   void *rstp_aux),
 void *aux)
-OVS_EXCLUDED(rstp_mutex)
 {
 struct rstp *rstp;
 
 VLOG_DBG("Creating RSTP instance");
 
+rstp_init();
+
 rstp = xzalloc(sizeof *rstp);
 rstp->name = xstrdup(name);
 
@@ -338,7 +340,6 @@ rstp_set_bridge_address__(struct rstp *rstp, 
rstp_identifier bridge_address)
 /* Sets the bridge address. */
 void
 rstp_set_bridge_address(struct rstp *rstp, rstp_identifier bridge_address)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 rstp_set_bridge_address__(rstp, bridge_address);
@@ -347,7 +348,6 @@ rstp_set_bridge_address(struct rstp *rstp, rstp_identifier 
bridge_address)
 
 const char *
 rstp_get_name(const struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
 {
 char *name;
 
@@ -359,7 +359,6 @@ rstp_get_name(const struct rstp *rstp)
 
 rstp_identifier
 rstp_get_bridge_id(const struct rstp *rstp)
-OVS_EXCLUDED(rstp_mutex)
 {
 rstp_identifier bridge_id;
 
@@ -391,7 +390,6 @@ rstp_set_bridge_priority__(struct rstp *rstp, int 
new_priority)
 
 void
 rstp_set_bridge_priority(struct rstp *rstp, int new_priority)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 rstp_set_bridge_priority__(rstp, new_priority);
@@ -413,7 +411,6 @@ rstp_set_bridge_ageing_time__(struct rstp *rstp, int 
new_ageing_time)
 
 void
 rstp_set_bridge_ageing_time(struct rstp *rstp, int new_ageing_time)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);
 rstp_set_bridge_ageing_time__(rstp, new_ageing_time);
@@ -509,7 +506,6 @@ rstp_set_bridge_force_protocol_version__(struct rstp *rstp,
 void
 rstp_set_bridge_force_protocol_version(struct rstp *rstp,
 enum rstp_force_protocol_version new_force_protocol_version)
-OVS_EXCLUDED(rstp_mutex)
 {
 ovs_mutex_lock(_mutex);