It was only used to guard against unintialized list.
Signed-off-by: Jarno Rajahalme <[email protected]>
---
lib/rstp-common.h | 1 -
lib/rstp-state-machines.c | 273 ++++++++++++++++++++-------------------------
lib/rstp.c | 89 +++++++--------
3 files changed, 164 insertions(+), 199 deletions(-)
diff --git a/lib/rstp-common.h b/lib/rstp-common.h
index d798ba2..2be5861 100644
--- a/lib/rstp-common.h
+++ b/lib/rstp-common.h
@@ -867,7 +867,6 @@ struct rstp {
/* Ports */
struct list ports OVS_GUARDED_BY(rstp_mutex);
- uint16_t ports_count OVS_GUARDED_BY(rstp_mutex);
struct ovs_refcount ref_cnt;
diff --git a/lib/rstp-state-machines.c b/lib/rstp-state-machines.c
index 5ae7124..c40449d 100644
--- a/lib/rstp-state-machines.c
+++ b/lib/rstp-state-machines.c
@@ -190,18 +190,16 @@ move_rstp__(struct rstp *rstp)
rstp->changes = false;
port_role_selection_sm(rstp);
- if (rstp->ports_count > 0) {
- LIST_FOR_EACH (p, node, &rstp->ports) {
- if (p->rstp_state != RSTP_DISABLED) {
- port_receive_sm(p);
- bridge_detection_sm(p);
- port_information_sm(p);
- port_role_transition_sm(p);
- port_state_transition_sm(p);
- topology_change_sm(p);
- port_transmit_sm(p);
- port_protocol_migration_sm(p);
- }
+ LIST_FOR_EACH (p, node, &rstp->ports) {
+ if (p->rstp_state != RSTP_DISABLED) {
+ port_receive_sm(p);
+ bridge_detection_sm(p);
+ port_information_sm(p);
+ port_role_transition_sm(p);
+ port_state_transition_sm(p);
+ topology_change_sm(p);
+ port_transmit_sm(p);
+ port_protocol_migration_sm(p);
}
}
num_iterations++;
@@ -219,19 +217,17 @@ void decrease_rstp_port_timers__(struct rstp *r)
{
struct rstp_port *p;
- if (r->ports_count > 0) {
- LIST_FOR_EACH (p, node, &r->ports) {
- decrement_timer(&p->hello_when);
- decrement_timer(&p->tc_while);
- decrement_timer(&p->fd_while);
- decrement_timer(&p->rcvd_info_while);
- decrement_timer(&p->rr_while);
- decrement_timer(&p->rb_while);
- decrement_timer(&p->mdelay_while);
- decrement_timer(&p->edge_delay_while);
- decrement_timer(&p->tx_count);
- p->uptime += 1;
- }
+ LIST_FOR_EACH (p, node, &r->ports) {
+ decrement_timer(&p->hello_when);
+ decrement_timer(&p->tc_while);
+ decrement_timer(&p->fd_while);
+ decrement_timer(&p->rcvd_info_while);
+ decrement_timer(&p->rr_while);
+ decrement_timer(&p->rb_while);
+ decrement_timer(&p->mdelay_while);
+ decrement_timer(&p->edge_delay_while);
+ decrement_timer(&p->tx_count);
+ p->uptime += 1;
}
r->changes = true;
move_rstp__(r);
@@ -254,10 +250,8 @@ updt_role_disabled_tree(struct rstp *r)
{
struct rstp_port *p;
- if (r->ports_count > 0) {
- LIST_FOR_EACH (p, node, &r->ports) {
- p->selected_role = ROLE_DISABLED;
- }
+ LIST_FOR_EACH (p, node, &r->ports) {
+ p->selected_role = ROLE_DISABLED;
}
}
@@ -267,10 +261,8 @@ clear_reselect_tree(struct rstp *r)
{
struct rstp_port *p;
- if (r->ports_count > 0) {
- LIST_FOR_EACH (p, node, &r->ports) {
- p->reselect = false;
- }
+ LIST_FOR_EACH (p, node, &r->ports) {
+ p->reselect = false;
}
}
@@ -287,32 +279,30 @@ updt_roles_tree__(struct rstp *r)
/* Letter c1) */
r->root_times = r->bridge_times;
/* Letters a) b) c) */
- if (r->ports_count > 0) {
- LIST_FOR_EACH (p, node, &r->ports) {
- uint32_t old_root_path_cost;
- uint32_t root_path_cost;
+ LIST_FOR_EACH (p, node, &r->ports) {
+ uint32_t old_root_path_cost;
+ uint32_t root_path_cost;
- if (p->info_is != INFO_IS_RECEIVED) {
- continue;
- }
- /* [17.6] */
- candidate_vector = p->port_priority;
- candidate_vector.bridge_port_id = p->port_id;
- old_root_path_cost = candidate_vector.root_path_cost;
- root_path_cost = old_root_path_cost + p->port_path_cost;
- candidate_vector.root_path_cost = root_path_cost;
-
- if ((candidate_vector.designated_bridge_id & 0xffffffffffffULL) ==
- (r->bridge_priority.designated_bridge_id & 0xffffffffffffULL))
{
- break;
- }
- if (compare_rstp_priority_vector(&candidate_vector, &best_vector)
- == SUPERIOR) {
- best_vector = candidate_vector;
- r->root_times = p->port_times;
- r->root_times.message_age++;
- vsel = p->port_number;
- }
+ if (p->info_is != INFO_IS_RECEIVED) {
+ continue;
+ }
+ /* [17.6] */
+ candidate_vector = p->port_priority;
+ candidate_vector.bridge_port_id = p->port_id;
+ old_root_path_cost = candidate_vector.root_path_cost;
+ root_path_cost = old_root_path_cost + p->port_path_cost;
+ candidate_vector.root_path_cost = root_path_cost;
+
+ if ((candidate_vector.designated_bridge_id & 0xffffffffffffULL) ==
+ (r->bridge_priority.designated_bridge_id & 0xffffffffffffULL)) {
+ break;
+ }
+ if (compare_rstp_priority_vector(&candidate_vector, &best_vector)
+ == SUPERIOR) {
+ best_vector = candidate_vector;
+ r->root_times = p->port_times;
+ r->root_times.message_age++;
+ vsel = p->port_number;
}
}
r->root_priority = best_vector;
@@ -320,63 +310,59 @@ updt_roles_tree__(struct rstp *r)
VLOG_DBG("%s: new Root is "RSTP_ID_FMT"", r->name,
RSTP_ID_ARGS(r->root_priority.root_bridge_id));
/* Letters d) e) */
- if (r->ports_count > 0) {
- LIST_FOR_EACH (p, node, &r->ports) {
- p->designated_priority_vector.root_bridge_id =
- r->root_priority.root_bridge_id;
- p->designated_priority_vector.root_path_cost =
- r->root_priority.root_path_cost;
- p->designated_priority_vector.designated_bridge_id =
- r->bridge_identifier;
- p->designated_priority_vector.designated_port_id =
- p->port_id;
- p->designated_times = r->root_times;
- p->designated_times.hello_time = r->bridge_times.hello_time;
- }
+ LIST_FOR_EACH (p, node, &r->ports) {
+ p->designated_priority_vector.root_bridge_id =
+ r->root_priority.root_bridge_id;
+ p->designated_priority_vector.root_path_cost =
+ r->root_priority.root_path_cost;
+ p->designated_priority_vector.designated_bridge_id =
+ r->bridge_identifier;
+ p->designated_priority_vector.designated_port_id =
+ p->port_id;
+ p->designated_times = r->root_times;
+ p->designated_times.hello_time = r->bridge_times.hello_time;
}
- if (r->ports_count > 0) {
- LIST_FOR_EACH (p, node, &r->ports) {
- switch (p->info_is) {
- case INFO_IS_DISABLED:
- p->selected_role = ROLE_DISABLED;
- break;
- case INFO_IS_AGED:
+ LIST_FOR_EACH (p, node, &r->ports) {
+ switch (p->info_is) {
+ case INFO_IS_DISABLED:
+ p->selected_role = ROLE_DISABLED;
+ break;
+ case INFO_IS_AGED:
+ p->updt_info = true;
+ p->selected_role = ROLE_DESIGNATED;
+ break;
+ case INFO_IS_MINE:
+ p->selected_role = ROLE_DESIGNATED;
+ if (compare_rstp_priority_vector(&p->port_priority,
+ &p->designated_priority_vector)
+ != SAME
+ || !rstp_times_equal(&p->designated_times, &r->root_times)) {
p->updt_info = true;
- p->selected_role = ROLE_DESIGNATED;
- break;
- case INFO_IS_MINE:
- p->selected_role = ROLE_DESIGNATED;
- if (compare_rstp_priority_vector(&p->port_priority,
-
&p->designated_priority_vector)
- != SAME
- || !rstp_times_equal(&p->designated_times,
&r->root_times)) {
- p->updt_info = true;
- }
- break;
- case INFO_IS_RECEIVED:
- if (vsel == p->port_number) { /* Letter i) */
- p->selected_role = ROLE_ROOT;
+ }
+ break;
+ case INFO_IS_RECEIVED:
+ if (vsel == p->port_number) { /* Letter i) */
+ p->selected_role = ROLE_ROOT;
+ p->updt_info = false;
+ } else if (compare_rstp_priority_vector(
+ &p->designated_priority_vector, &p->port_priority)
+ == INFERIOR) {
+ if (p->port_priority.designated_bridge_id !=
+ r->bridge_identifier) {
+ p->selected_role = ROLE_ALTERNATE;
p->updt_info = false;
- } else if (compare_rstp_priority_vector(
- &p->designated_priority_vector, &p->port_priority)
- == INFERIOR) {
- if (p->port_priority.designated_bridge_id !=
- r->bridge_identifier) {
- p->selected_role = ROLE_ALTERNATE;
- p->updt_info = false;
- } else {
- p->selected_role = ROLE_BACKUP;
- p->updt_info = false;
- }
} else {
- p->selected_role = ROLE_DESIGNATED;
- p->updt_info = true;
+ p->selected_role = ROLE_BACKUP;
+ p->updt_info = false;
}
- break;
- default:
- OVS_NOT_REACHED();
- /* no break */
+ } else {
+ p->selected_role = ROLE_DESIGNATED;
+ p->updt_info = true;
}
+ break;
+ default:
+ OVS_NOT_REACHED();
+ /* no break */
}
}
seq_change(connectivity_seq_get());
@@ -388,16 +374,14 @@ set_selected_tree(struct rstp *r)
{
struct rstp_port *p;
- if (r->ports_count > 0) {
- LIST_FOR_EACH (p, node, &r->ports) {
- if (p->reselect) {
- return;
- }
- }
- LIST_FOR_EACH (p, node, &r->ports) {
- p->selected = true;
+ LIST_FOR_EACH (p, node, &r->ports) {
+ if (p->reselect) {
+ return;
}
}
+ LIST_FOR_EACH (p, node, &r->ports) {
+ p->selected = true;
+ }
}
static int
@@ -432,13 +416,11 @@ port_role_selection_sm(struct rstp *r)
PORT_ROLE_SELECTION_SM_ROLE_SELECTION;
/* no break */
case PORT_ROLE_SELECTION_SM_ROLE_SELECTION:
- if (r->ports_count > 0) {
- LIST_FOR_EACH (p, node, &r->ports) {
- if (p->reselect) {
- r->port_role_selection_sm_state =
- PORT_ROLE_SELECTION_SM_ROLE_SELECTION_EXEC;
- break;
- }
+ LIST_FOR_EACH (p, node, &r->ports) {
+ if (p->reselect) {
+ r->port_role_selection_sm_state =
+ PORT_ROLE_SELECTION_SM_ROLE_SELECTION_EXEC;
+ break;
}
}
break;
@@ -1278,10 +1260,8 @@ set_re_root_tree(struct rstp_port *p)
struct rstp_port *p1;
r = p->rstp;
- if (r->ports_count > 0) {
- LIST_FOR_EACH (p1, node, &r->ports) {
- p1->re_root = true;
- }
+ LIST_FOR_EACH (p1, node, &r->ports) {
+ p1->re_root = true;
}
}
@@ -1293,10 +1273,8 @@ set_sync_tree(struct rstp_port *p)
struct rstp_port *p1;
r = p->rstp;
- if (r->ports_count > 0) {
- LIST_FOR_EACH (p1, node, &r->ports) {
- p1->sync = true;
- }
+ LIST_FOR_EACH (p1, node, &r->ports) {
+ p1->sync = true;
}
}
@@ -1383,11 +1361,9 @@ re_rooted(struct rstp_port *p)
struct rstp_port *p1;
r = p->rstp;
- if (r->ports_count > 0) {
- LIST_FOR_EACH (p1, node, &r->ports) {
- if ((p1 != p) && (p1->rr_while != 0)) {
- return false;
- }
+ LIST_FOR_EACH (p1, node, &r->ports) {
+ if ((p1 != p) && (p1->rr_while != 0)) {
+ return false;
}
}
return true;
@@ -1399,12 +1375,10 @@ all_synced(struct rstp *r)
{
struct rstp_port *p;
- if (r->ports_count > 0) {
- LIST_FOR_EACH (p, node, &r->ports) {
- if (!(p->selected && p->role == p->selected_role &&
- (p->role == ROLE_ROOT || p->synced == true))) {
- return false;
- }
+ LIST_FOR_EACH (p, node, &r->ports) {
+ if (!(p->selected && p->role == p->selected_role &&
+ (p->role == ROLE_ROOT || p->synced == true))) {
+ return false;
}
}
return true;
@@ -1859,14 +1833,11 @@ set_tc_prop_tree(struct rstp_port *p)
struct rstp_port *p1;
r = p->rstp;
- if (r->ports_count > 0) {
- LIST_FOR_EACH (p1, node, &r->ports) {
- /* Set tc_prop on every port, except the one calling this
- * function.
- */
- if (p1->port_number != p->port_number) {
- p1->tc_prop = true;
- }
+ LIST_FOR_EACH (p1, node, &r->ports) {
+ /* Set tc_prop on every port, except the one calling this
+ * function. */
+ if (p1->port_number != p->port_number) {
+ p1->tc_prop = true;
}
}
}
diff --git a/lib/rstp.c b/lib/rstp.c
index 223df01..d5abd20 100644
--- a/lib/rstp.c
+++ b/lib/rstp.c
@@ -171,12 +171,12 @@ rstp_unref(struct rstp *rstp)
if (rstp && ovs_refcount_unref(&rstp->ref_cnt) == 1) {
ovs_mutex_lock(&rstp_mutex);
- /* Each RSTP port poits back to struct rstp without holding a
+ /* Each RSTP port points back to struct rstp without holding a
* reference for that pointer. This is OK as we never move
* ports from one bridge to another, and holders always
* release their ports before releasing the bridge. This
* means that there should be not ports at this time. */
- ovs_assert(rstp->ports_count == 0);
+ ovs_assert(list_is_empty(&rstp->ports));
list_remove(&rstp->node);
ovs_mutex_unlock(&rstp_mutex);
@@ -251,6 +251,10 @@ rstp_create(const char *name, rstp_identifier
bridge_address,
rstp = xzalloc(sizeof *rstp);
rstp->name = xstrdup(name);
+ /* Initialize the ports list before calling any setters,
+ * so that the state machines will see an empty ports list. */
+ list_init(&rstp->ports);
+
ovs_mutex_lock(&rstp_mutex);
/* Set bridge address. */
rstp_set_bridge_address__(rstp, bridge_address);
@@ -272,8 +276,6 @@ rstp_create(const char *name, rstp_identifier
bridge_address,
rstp->changes = false;
rstp->begin = true;
- /* Initialize the ports list. */
- list_init(&rstp->ports);
ovs_refcount_init(&rstp->ref_cnt);
list_push_back(all_rstps, &rstp->node);
@@ -291,6 +293,8 @@ static void
set_bridge_priority__(struct rstp *rstp)
OVS_REQUIRES(rstp_mutex)
{
+ struct rstp_port *p;
+
rstp->bridge_priority.root_bridge_id = rstp->bridge_identifier;
rstp->bridge_priority.designated_bridge_id = rstp->bridge_identifier;
VLOG_DBG("%s: new bridge identifier: "RSTP_ID_FMT"", rstp->name,
@@ -299,13 +303,9 @@ set_bridge_priority__(struct rstp *rstp)
/* [17.13] When the bridge address changes, recalculates all priority
* vectors.
*/
- if (rstp->ports_count > 0) {
- struct rstp_port *p;
-
- LIST_FOR_EACH (p, node, &rstp->ports) {
- p->selected = false;
- p->reselect = true;
- }
+ LIST_FOR_EACH (p, node, &rstp->ports) {
+ p->selected = false;
+ p->reselect = true;
}
rstp->changes = true;
updt_roles_tree__(rstp);
@@ -417,6 +417,7 @@ reinitialize_rstp__(struct rstp *rstp)
{
struct rstp temp;
static struct list ports;
+ struct rstp_port *p;
/* Copy rstp in temp */
temp = *rstp;
@@ -427,6 +428,11 @@ reinitialize_rstp__(struct rstp *rstp)
/* Initialize rstp. */
rstp->name = temp.name;
+
+ /* Initialize the ports list before calling any setters,
+ * so that the state machines will see an empty ports list. */
+ list_init(&rstp->ports);
+
/* Set bridge address. */
rstp_set_bridge_address__(rstp, temp.address);
/* Set default parameters values. */
@@ -447,16 +453,14 @@ reinitialize_rstp__(struct rstp *rstp)
rstp->node = temp.node;
rstp->changes = false;
rstp->begin = true;
- rstp->ports = ports;
- rstp->ports_count = temp.ports_count;
- if (rstp->ports_count > 0) {
- struct rstp_port *p;
+ /* Restore ports. */
+ rstp->ports = ports;
- LIST_FOR_EACH (p, node, &rstp->ports) {
- reinitialize_port__(p);
- }
+ LIST_FOR_EACH (p, node, &rstp->ports) {
+ reinitialize_port__(p);
}
+
rstp->ref_cnt = temp.ref_cnt;
}
@@ -570,19 +574,17 @@ rstp_set_bridge_transmit_hold_count__(struct rstp *rstp,
int new_transmit_hold_count)
OVS_REQUIRES(rstp_mutex)
{
- struct rstp_port *p;
-
if (new_transmit_hold_count >= RSTP_MIN_TRANSMIT_HOLD_COUNT
&& new_transmit_hold_count <= RSTP_MAX_TRANSMIT_HOLD_COUNT) {
+ struct rstp_port *p;
+
VLOG_DBG("%s: set RSTP Transmit Hold Count to %d", rstp->name,
new_transmit_hold_count);
/* Resetting txCount on all ports [17.13]. */
rstp->transmit_hold_count = new_transmit_hold_count;
- if (rstp->ports_count > 0) {
- LIST_FOR_EACH (p, node, &rstp->ports) {
- p->tx_count = 0;
- }
+ LIST_FOR_EACH (p, node, &rstp->ports) {
+ p->tx_count = 0;
}
}
}
@@ -777,15 +779,13 @@ rstp_check_and_reset_fdb_flush(struct rstp *rstp)
needs_flush = false;
ovs_mutex_lock(&rstp_mutex);
- if (rstp->ports_count > 0){
- LIST_FOR_EACH (p, node, &rstp->ports) {
- if (p->fdb_flush) {
- needs_flush = true;
- /* fdb_flush should be reset by the filtering database
- * once the entries are removed if rstp_version is TRUE, and
- * immediately if stp_version is TRUE.*/
- p->fdb_flush = false;
- }
+ LIST_FOR_EACH (p, node, &rstp->ports) {
+ if (p->fdb_flush) {
+ needs_flush = true;
+ /* fdb_flush should be reset by the filtering database
+ * once the entries are removed if rstp_version is TRUE, and
+ * immediately if stp_version is TRUE.*/
+ p->fdb_flush = false;
}
}
ovs_mutex_unlock(&rstp_mutex);
@@ -850,11 +850,9 @@ rstp_get_port__(struct rstp *rstp, uint16_t port_number)
ovs_assert(rstp && port_number > 0 && port_number <= RSTP_MAX_PORTS);
- if (rstp->ports_count > 0) {
- LIST_FOR_EACH (port, node, &rstp->ports) {
- if (port->port_number == port_number) {
- return port;
- }
+ LIST_FOR_EACH (port, node, &rstp->ports) {
+ if (port->port_number == port_number) {
+ return port;
}
}
return NULL;
@@ -1054,7 +1052,6 @@ rstp_add_port(struct rstp *rstp)
rstp_port_set_state__(p, RSTP_DISCARDING);
list_push_back(&rstp->ports, &p->node);
- rstp->ports_count++;
rstp->changes = true;
move_rstp__(rstp);
VLOG_DBG("%s: added port "RSTP_PORT_ID_FMT"", rstp->name, p->port_id);
@@ -1088,8 +1085,8 @@ rstp_port_unref(struct rstp_port *rp)
rstp = rp->rstp;
rstp_port_set_state__(rp, RSTP_DISABLED);
list_remove(&rp->node);
- rstp->ports_count--;
- VLOG_DBG("%s: removed port "RSTP_PORT_ID_FMT"", rstp->name,
rp->port_id);
+ VLOG_DBG("%s: removed port "RSTP_PORT_ID_FMT"", rstp->name,
+ rp->port_id);
ovs_mutex_unlock(&rstp_mutex);
free(rp);
}
@@ -1240,12 +1237,10 @@ rstp_get_root_port(struct rstp *rstp)
struct rstp_port *p;
ovs_mutex_lock(&rstp_mutex);
- if (rstp->ports_count > 0){
- LIST_FOR_EACH (p, node, &rstp->ports) {
- if (p->port_id == rstp->root_port_id) {
- ovs_mutex_unlock(&rstp_mutex);
- return p;
- }
+ LIST_FOR_EACH (p, node, &rstp->ports) {
+ if (p->port_id == rstp->root_port_id) {
+ ovs_mutex_unlock(&rstp_mutex);
+ return p;
}
}
ovs_mutex_unlock(&rstp_mutex);
--
1.7.10.4
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
