Acked-by: Daniele Venturino <daniele.ventur...@m3s.it> 2014-08-21 1:57 GMT+02:00 Jarno Rajahalme <jrajaha...@nicira.com>:
> 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 > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev