It was only used to guard against unintialized list. Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com> --- 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 dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev