Re: [ovs-dev] [PATCH v6 07/18] lib/rstp: Coding style fixes.
Acked-by: Daniele Venturino
2014-08-21 1:57 GMT+02:00 Jarno Rajahalme :
> Signed-off-by: Jarno Rajahalme
> ---
> lib/rstp.c | 132
> ++--
> lib/rstp.h | 12 +++---
> 2 files changed, 81 insertions(+), 63 deletions(-)
>
> diff --git a/lib/rstp.c b/lib/rstp.c
> index b0ad613..17830d9 100644
> --- a/lib/rstp.c
> +++ b/lib/rstp.c
> @@ -27,6 +27,7 @@
> */
>
> #include
> +
> #include "rstp.h"
> #include "rstp-common.h"
> #include "rstp-state-machines.h"
> @@ -58,7 +59,7 @@ static void set_bridge_priority__(struct rstp *);
> static void reinitialize_rstp__(struct rstp *);
> static bool is_port_number_taken__(struct rstp *, int, struct rstp_port
> *);
> static uint16_t rstp_first_free_number__(struct rstp *, struct rstp_port
> *);
> -static void rstp_initialize_port(struct rstp_port *p);
> +static void rstp_initialize_port__(struct rstp_port *);
>
> const char *
> rstp_state_name(enum rstp_state state)
> @@ -96,12 +97,11 @@ rstp_port_role_name(enum rstp_port_role role)
> }
> }
>
> +/* Caller has to hold a reference to prevent 'rstp' from being deleted
> + * while we are taking a new reference. */
> struct rstp *
> -rstp_ref(struct rstp *rstp_)
> +rstp_ref(struct rstp *rstp)
> {
> -struct rstp *rstp;
> -
> -rstp = rstp_;
> if (rstp) {
> ovs_refcount_ref(&rstp->ref_cnt);
> }
> @@ -112,11 +112,11 @@ rstp_ref(struct rstp *rstp_)
> void
> rstp_unref(struct rstp *rstp)
> {
> -struct rstp_port *p;
> -
> if (rstp && ovs_refcount_unref(&rstp->ref_cnt) == 1) {
> ovs_mutex_lock(&mutex);
> if (rstp->ports_count > 0) {
> +struct rstp_port *p;
> +
> LIST_FOR_EACH (p, node, &rstp->ports) {
> rstp_delete_port(p);
> }
> @@ -128,7 +128,9 @@ rstp_unref(struct rstp *rstp)
> }
> }
>
> -/* Returns the port number. */
> +/* Returns the port number. Mutex is needed to guard against
> + * concurrent reinitialization (which can temporarily clear the
> + * port_number). */
> int
> rstp_port_number(const struct rstp_port *p)
> {
> @@ -164,15 +166,15 @@ rstp_received_bpdu(struct rstp_port *p, const void
> *bpdu, size_t bpdu_size)
> void
> rstp_init(void)
> {
> - unixctl_command_register("rstp/tcn", "[bridge]", 0, 1,
> rstp_unixctl_tcn,
> - NULL);
> +unixctl_command_register("rstp/tcn", "[bridge]", 0, 1,
> rstp_unixctl_tcn,
> + NULL);
> }
>
> /* Creates and returns a new RSTP instance that initially has no ports. */
> struct rstp *
> rstp_create(const char *name, rstp_identifier bridge_address,
> -void (*send_bpdu)(struct ofpbuf *bpdu, int port_no, void *aux),
> -void *aux)
> +void (*send_bpdu)(struct ofpbuf *bpdu, int port_no, void
> *aux),
> +void *aux)
> {
> static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> struct rstp *rstp;
> @@ -205,12 +207,14 @@ rstp_create(const char *name, rstp_identifier
> bridge_address,
> rstp->changes = false;
> rstp->begin = true;
>
> -ovs_mutex_lock(&mutex);
> /* Initialize the ports list. */
> list_init(&rstp->ports);
> ovs_refcount_init(&rstp->ref_cnt);
> +
> +ovs_mutex_lock(&mutex);
> list_push_back(all_rstps, &rstp->node);
> ovs_mutex_unlock(&mutex);
> +
> VLOG_DBG("RSTP instance creation done");
> return rstp;
> }
> @@ -236,6 +240,7 @@ rstp_set_bridge_address(struct rstp *rstp,
> rstp_identifier bridge_address)
>
> VLOG_DBG("%s: set bridge address to: "RSTP_ID_FMT"", rstp->name,
> RSTP_ID_ARGS(bridge_address));
> +
> ovs_mutex_lock(&mutex);
> rstp->address = bridge_address;
> rstp->bridge_identifier = bridge_address;
> @@ -246,8 +251,8 @@ rstp_set_bridge_address(struct rstp *rstp,
> rstp_identifier bridge_address)
> */
> if (rstp->ports_count > 0) {
> LIST_FOR_EACH (p, node, &rstp->ports) {
> -p->selected = 0;
> -p->reselect = 1;
> +p->selected = false;
> +p->reselect = true;
> }
> }
> rstp->changes = true;
> @@ -289,7 +294,7 @@ rstp_set_bridge_priority(struct rstp *rstp, int
> new_priority)
> (new_priority / 4096) * 4096);
> ovs_mutex_lock(&mutex);
> rstp->priority = (new_priority / 4096) * 4096;
> -rstp->bridge_identifier &= 0xULL;
> +rstp->bridge_identifier &= 0xULL;
> rstp->bridge_identifier |=
>(uint64_t) ((new_priority / 4096) * 4096)
> << 48;
> set_bridge_priority__(rstp);
> @@ -297,8 +302,8 @@ rstp_set_bridge_priority(struct rstp *rstp, int
> new_priority)
> /* [17.13] */
> if (rstp->ports_count > 0){
> LIST_FOR_EACH (p, node, &rstp->ports) {
> -p->selected = 0;
> -p->reselect = 1;
> +
[ovs-dev] [PATCH v6 07/18] lib/rstp: Coding style fixes.
Signed-off-by: Jarno Rajahalme
---
lib/rstp.c | 132 ++--
lib/rstp.h | 12 +++---
2 files changed, 81 insertions(+), 63 deletions(-)
diff --git a/lib/rstp.c b/lib/rstp.c
index b0ad613..17830d9 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -27,6 +27,7 @@
*/
#include
+
#include "rstp.h"
#include "rstp-common.h"
#include "rstp-state-machines.h"
@@ -58,7 +59,7 @@ static void set_bridge_priority__(struct rstp *);
static void reinitialize_rstp__(struct rstp *);
static bool is_port_number_taken__(struct rstp *, int, struct rstp_port *);
static uint16_t rstp_first_free_number__(struct rstp *, struct rstp_port *);
-static void rstp_initialize_port(struct rstp_port *p);
+static void rstp_initialize_port__(struct rstp_port *);
const char *
rstp_state_name(enum rstp_state state)
@@ -96,12 +97,11 @@ rstp_port_role_name(enum rstp_port_role role)
}
}
+/* Caller has to hold a reference to prevent 'rstp' from being deleted
+ * while we are taking a new reference. */
struct rstp *
-rstp_ref(struct rstp *rstp_)
+rstp_ref(struct rstp *rstp)
{
-struct rstp *rstp;
-
-rstp = rstp_;
if (rstp) {
ovs_refcount_ref(&rstp->ref_cnt);
}
@@ -112,11 +112,11 @@ rstp_ref(struct rstp *rstp_)
void
rstp_unref(struct rstp *rstp)
{
-struct rstp_port *p;
-
if (rstp && ovs_refcount_unref(&rstp->ref_cnt) == 1) {
ovs_mutex_lock(&mutex);
if (rstp->ports_count > 0) {
+struct rstp_port *p;
+
LIST_FOR_EACH (p, node, &rstp->ports) {
rstp_delete_port(p);
}
@@ -128,7 +128,9 @@ rstp_unref(struct rstp *rstp)
}
}
-/* Returns the port number. */
+/* Returns the port number. Mutex is needed to guard against
+ * concurrent reinitialization (which can temporarily clear the
+ * port_number). */
int
rstp_port_number(const struct rstp_port *p)
{
@@ -164,15 +166,15 @@ rstp_received_bpdu(struct rstp_port *p, const void *bpdu,
size_t bpdu_size)
void
rstp_init(void)
{
- unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, rstp_unixctl_tcn,
- NULL);
+unixctl_command_register("rstp/tcn", "[bridge]", 0, 1, rstp_unixctl_tcn,
+ NULL);
}
/* Creates and returns a new RSTP instance that initially has no ports. */
struct rstp *
rstp_create(const char *name, rstp_identifier bridge_address,
-void (*send_bpdu)(struct ofpbuf *bpdu, int port_no, void *aux),
-void *aux)
+void (*send_bpdu)(struct ofpbuf *bpdu, int port_no, void *aux),
+void *aux)
{
static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
struct rstp *rstp;
@@ -205,12 +207,14 @@ rstp_create(const char *name, rstp_identifier
bridge_address,
rstp->changes = false;
rstp->begin = true;
-ovs_mutex_lock(&mutex);
/* Initialize the ports list. */
list_init(&rstp->ports);
ovs_refcount_init(&rstp->ref_cnt);
+
+ovs_mutex_lock(&mutex);
list_push_back(all_rstps, &rstp->node);
ovs_mutex_unlock(&mutex);
+
VLOG_DBG("RSTP instance creation done");
return rstp;
}
@@ -236,6 +240,7 @@ rstp_set_bridge_address(struct rstp *rstp, rstp_identifier
bridge_address)
VLOG_DBG("%s: set bridge address to: "RSTP_ID_FMT"", rstp->name,
RSTP_ID_ARGS(bridge_address));
+
ovs_mutex_lock(&mutex);
rstp->address = bridge_address;
rstp->bridge_identifier = bridge_address;
@@ -246,8 +251,8 @@ rstp_set_bridge_address(struct rstp *rstp, rstp_identifier
bridge_address)
*/
if (rstp->ports_count > 0) {
LIST_FOR_EACH (p, node, &rstp->ports) {
-p->selected = 0;
-p->reselect = 1;
+p->selected = false;
+p->reselect = true;
}
}
rstp->changes = true;
@@ -289,7 +294,7 @@ rstp_set_bridge_priority(struct rstp *rstp, int
new_priority)
(new_priority / 4096) * 4096);
ovs_mutex_lock(&mutex);
rstp->priority = (new_priority / 4096) * 4096;
-rstp->bridge_identifier &= 0xULL;
+rstp->bridge_identifier &= 0xULL;
rstp->bridge_identifier |=
(uint64_t) ((new_priority / 4096) * 4096) << 48;
set_bridge_priority__(rstp);
@@ -297,8 +302,8 @@ rstp_set_bridge_priority(struct rstp *rstp, int
new_priority)
/* [17.13] */
if (rstp->ports_count > 0){
LIST_FOR_EACH (p, node, &rstp->ports) {
-p->selected = 0;
-p->reselect = 1;
+p->selected = false;
+p->reselect = true;
}
}
rstp->changes = true;
@@ -311,9 +316,10 @@ rstp_set_bridge_priority(struct rstp *rstp, int
new_priority)
void
rstp_set_bridge_ageing_time(struct rstp *rstp, int new_ageing_time)
{
-if (new_ageing_time >= RSTP_MIN_AGEING_TIME &&
-new_agei
