> The problem must be that the port number and the hash value get out of sync,
> i.e., when the port number is changed. Please try the patch below on top of
> your patch and check if you still have the same problem.
With your modifications pertaining the hashmap those tests passed.
I had to revert this change in ofproto-dpif:
> - struct ofport_dpif *old_root;
> - struct ofport_dpif *new_root;
> if (rstp_shift_root_learned_address(ofproto->rstp)) {
> - old_root = rstp_get_old_root_aux(ofproto->rstp);
> - new_root = rstp_get_new_root_aux(ofproto->rstp);
> - bundle_move(old_root->bundle, new_root->bundle);
> + bundle_move(rstp_get_old_root_aux(ofproto->rstp),
> + rstp_get_new_root_aux(ofproto->rstp));
> rstp_reset_root_changed(ofproto->rstp);
> }
since bundle_move() expects two “struct ofbundle" as arguments.
Regards,
Daniele
> Il giorno 14/nov/2014, alle ore 22:56, Jarno Rajahalme
> <[email protected]> ha scritto:
>
>
> On Nov 14, 2014, at 9:19 AM, Daniele Venturino <[email protected]
> <mailto:[email protected]>> wrote:
>
>> Why lose the ‘_WITH_HASH’? It seems pointless to iterate over all the ports,
>> when we just care of the port with the ‘port_number’?
>>
>> With the _WITH_HASH a test on learned addresses shifting fails. It seems
>> like it doesn't even enter the FOR_EACH cycle when the port_number is not 1,
>> maybe there is a problem with the hash lookup?
>>
>
> The problem must be that the port number and the hash value get out of sync,
> i.e., when the port number is changed. Please try the patch below on top of
> your patch and check if you still have the same problem.
>
>> Can the bundles really be from different bridges, as they both are managed
>> by the same RSTP state machine? If both ofproto’s are the same, then this
>> function can be simplified.
>>
>> I guess they're from the same bridge. The shift is between two ports of the
>> same bridge. Feel free to suggest any improvement on this.
>>
>
> I’ll add an assert to that effect.
>
> Jarno
>
> diff --git a/lib/rstp-state-machines.c b/lib/rstp-state-machines.c
> index 705ab61..02e3f98 100644
> --- a/lib/rstp-state-machines.c
> +++ b/lib/rstp-state-machines.c
> @@ -296,7 +296,7 @@ updt_roles_tree__(struct rstp *r)
>
> old_root_port_number = r->root_port_id & 0x00ff;
> if (old_root_port_number) {
> - r->old_root_aux = rstp_get_port_aux(r, old_root_port_number);
> + r->old_root_aux = rstp_get_port_aux__(r, old_root_port_number);
> }
> vsel = -1;
> best_vector = r->bridge_priority;
> @@ -336,15 +336,15 @@ updt_roles_tree__(struct rstp *r)
> RSTP_ID_ARGS(r->root_priority.root_bridge_id));
> new_root_port_number = r->root_port_id & 0x00ff;
> if (new_root_port_number) {
> - r->new_root_aux = rstp_get_port_aux(r, new_root_port_number);
> + r->new_root_aux = rstp_get_port_aux__(r, new_root_port_number);
> }
> /* Shift learned MAC addresses from an old Root Port to an existing
> * Alternate Port. */
> if (!r->root_changed
> - && new_root_old_role == ROLE_ALTERNATE
> - && new_root_port_number
> - && old_root_port_number
> - && new_root_port_number != old_root_port_number) {
> + && new_root_old_role == ROLE_ALTERNATE
> + && new_root_port_number
> + && old_root_port_number
> + && new_root_port_number != old_root_port_number) {
> r->root_changed = true;
> }
> /* Letters d) e) */
> diff --git a/lib/rstp.c b/lib/rstp.c
> index e2420e8..05fa634 100644
> --- a/lib/rstp.c
> +++ b/lib/rstp.c
> @@ -711,6 +711,8 @@ static void
> rstp_port_set_port_number__(struct rstp_port *port, uint16_t port_number)
> OVS_REQUIRES(rstp_mutex)
> {
> + int old_port_number = port->port_number;
> +
> /* If new_port_number is available, use it, otherwise use the first free
> * available port number. */
> port->port_number =
> @@ -718,14 +720,23 @@ rstp_port_set_port_number__(struct rstp_port *port,
> uint16_t port_number)
> ? port_number
> : rstp_first_free_number__(port->rstp, port);
>
> - set_port_id__(port);
> - /* [17.13] is not clear. I suppose that a port number change
> - * should trigger reselection like a port priority change. */
> - port->selected = false;
> - port->reselect = true;
> + if (port->port_number != old_port_number) {
> + set_port_id__(port);
> + /* [17.13] is not clear. I suppose that a port number change
> + * should trigger reselection like a port priority change. */
> + port->selected = false;
> + port->reselect = true;
> +
> + /* Adjust the ports hmap. */
> + if (!hmap_node_is_null(&port->node)) {
> + hmap_remove(&port->rstp->ports, &port->node);
> + }
> + hmap_insert(&port->rstp->ports, &port->node,
> + hash_int(port->port_number, 0));
>
> - VLOG_DBG("%s: set new RSTP port number %d", port->rstp->name,
> - port->port_number);
> + VLOG_DBG("%s: set new RSTP port number %d", port->rstp->name,
> + port->port_number);
> + }
> }
>
> /* Converts the link speed to a port path cost [Table 17-3]. */
> @@ -780,7 +791,11 @@ rstp_get_root_path_cost(const struct rstp *rstp)
> * pointer is returned if no port needs to flush its MAC learning table.
> * '*port' needs to be NULL in the first call to start the iteration. If
> * '*port' is passed as non-NULL, it must be the value set by the last
> - * invocation of this function. */
> + * invocation of this function.
> + *
> + * This function may only be called by the thread that creates and deletes
> + * ports. Otherwise this function is not thread safe, as the returned
> + * '*port' could become stale before it is used in the next invocation. */
> void *
> rstp_check_and_reset_fdb_flush(struct rstp *rstp, struct rstp_port **port)
> OVS_EXCLUDED(rstp_mutex)
> @@ -819,8 +834,9 @@ out:
> (*port)->fdb_flush = false;
> }
> ovs_mutex_unlock(&rstp_mutex);
> +
> return aux;
> - }
> +}
>
> /* Finds a port whose state has changed, and returns the aux pointer set for
> * the port. A NULL pointer is returned when no changed port is found. On
> @@ -866,40 +882,49 @@ rstp_get_next_changed_port_aux(struct rstp *rstp,
> struct rstp_port **portp)
> *portp = NULL;
> out:
> ovs_mutex_unlock(&rstp_mutex);
> +
> return aux;
> }
>
> bool
> -rstp_shift_root_learned_address(struct rstp *rstp) {
> - bool ret = false;
> +rstp_shift_root_learned_address(struct rstp *rstp)
> +{
> + bool ret;
> +
> ovs_mutex_lock(&rstp_mutex);
> - if (rstp->root_changed) {
> - ret = true;
> - }
> + ret = rstp->root_changed;
> ovs_mutex_unlock(&rstp_mutex);
> +
> return ret;
> }
>
> void *
> -rstp_get_old_root_aux(struct rstp *rstp) {
> - void *aux = NULL;
> +rstp_get_old_root_aux(struct rstp *rstp)
> +{
> + void *aux;
> +
> ovs_mutex_lock(&rstp_mutex);
> aux = rstp->old_root_aux;
> ovs_mutex_unlock(&rstp_mutex);
> +
> return aux;
> }
>
> void *
> -rstp_get_new_root_aux(struct rstp *rstp) {
> - void *aux = NULL;
> +rstp_get_new_root_aux(struct rstp *rstp)
> +{
> + void *aux;
> +
> ovs_mutex_lock(&rstp_mutex);
> aux = rstp->new_root_aux;
> ovs_mutex_unlock(&rstp_mutex);
> +
> return aux;
> }
>
> void
> -rstp_reset_root_changed(struct rstp *rstp) {
> +rstp_reset_root_changed(struct rstp *rstp)
> +{
> ovs_mutex_lock(&rstp_mutex);
> rstp->root_changed = false;
> ovs_mutex_unlock(&rstp_mutex);
> @@ -916,7 +941,8 @@ rstp_get_port__(struct rstp *rstp, uint16_t port_number)
>
> ovs_assert(rstp && port_number > 0 && port_number <= RSTP_MAX_PORTS);
>
> - HMAP_FOR_EACH (port, node, &rstp->ports) {
> + HMAP_FOR_EACH_WITH_HASH (port, node, hash_int(port_number, 0),
> + &rstp->ports) {
> if (port->port_number == port_number) {
> return port;
> }
> @@ -937,7 +963,7 @@ rstp_get_port(struct rstp *rstp, uint16_t port_number)
> }
>
> void *
> -rstp_get_port_aux(struct rstp *rstp, uint16_t port_number)
> +rstp_get_port_aux__(struct rstp *rstp, uint16_t port_number)
> OVS_REQUIRES(rstp_mutex)
> {
> struct rstp_port *p;
> @@ -1117,6 +1143,7 @@ rstp_add_port(struct rstp *rstp)
> struct rstp_port *p = xzalloc(sizeof *p);
>
> ovs_refcount_init(&p->ref_cnt);
> + hmap_node_nullify(&p->node);
>
> ovs_mutex_lock(&rstp_mutex);
> p->rstp = rstp;
> @@ -1128,7 +1155,6 @@ rstp_add_port(struct rstp *rstp)
> p->port_id);
>
> rstp_port_set_state__(p, RSTP_DISCARDING);
> - hmap_insert(&rstp->ports, &p->node, hash_int(p->port_number, 0));
> rstp->changes = true;
> move_rstp__(rstp);
> VLOG_DBG("%s: added port "RSTP_PORT_ID_FMT"", rstp->name, p->port_id);
> @@ -1357,6 +1383,7 @@ rstp_get_root_port(struct rstp *rstp)
> /* Returns the state of port 'p'. */
> enum rstp_state
> rstp_port_get_state(const struct rstp_port *p)
> + OVS_EXCLUDED(rstp_mutex)
> {
> enum rstp_state state;
>
> diff --git a/lib/rstp.h b/lib/rstp.h
> index 3b40a00..8b57761 100644
> --- a/lib/rstp.h
> +++ b/lib/rstp.h
> @@ -164,15 +164,13 @@ void *rstp_get_next_changed_port_aux(struct rstp *,
> struct rstp_port **)
> void rstp_port_set_mac_operational(struct rstp_port *,
> bool new_mac_operational)
> OVS_EXCLUDED(rstp_mutex);
> -bool
> -rstp_shift_root_learned_address(struct rstp *rstp)
> +bool rstp_shift_root_learned_address(struct rstp *)
> OVS_EXCLUDED(rstp_mutex);
> void *rstp_get_old_root_aux(struct rstp *)
> OVS_EXCLUDED(rstp_mutex);
> void *rstp_get_new_root_aux(struct rstp *)
> OVS_EXCLUDED(rstp_mutex);
> -void
> -rstp_reset_root_changed(struct rstp *)
> +void rstp_reset_root_changed(struct rstp *)
> OVS_EXCLUDED(rstp_mutex);
>
> /* Bridge setters */
> @@ -242,8 +240,8 @@ void rstp_port_get_status(const struct rstp_port *,
> uint16_t *id,
> int *rx_count, int *error_count, int *uptime)
> OVS_EXCLUDED(rstp_mutex);
>
> -void * rstp_get_port_aux(struct rstp *rstp, uint16_t port_number)
> - OVS_EXCLUDED(rstp_mutex);
> +void * rstp_get_port_aux__(struct rstp *rstp, uint16_t port_number)
> + OVS_REQUIRES(rstp_mutex);
>
>
> /* Internal API for rstp-state-machines.c */
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 0ec7b66..268aefa 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -2133,8 +2133,8 @@ update_rstp_port_state(struct ofport_dpif *ofport)
> != rstp_learn_in_state(state)) {
> /* XXX: Learning action flows should also be flushed. */
> if (ofport->bundle) {
> - if(!rstp_shift_root_learned_address(ofproto->rstp)
> - || ofport != rstp_get_old_root_aux(ofproto->rstp)) {
> + if (!rstp_shift_root_learned_address(ofproto->rstp)
> + || rstp_get_old_root_aux(ofproto->rstp) != ofport) {
> bundle_flush_macs(ofport->bundle, false);
> }
> }
> @@ -2184,20 +2184,16 @@ rstp_run(struct ofproto_dpif *ofproto)
> */
> while ((ofport = rstp_check_and_reset_fdb_flush(ofproto->rstp,
> &rp))) {
> if (!rstp_shift_root_learned_address(ofproto->rstp)
> - || ofport != rstp_get_old_root_aux(ofproto->rstp)) {
> + || rstp_get_old_root_aux(ofproto->rstp) != ofport) {
> bundle_flush_macs(ofport->bundle, false);
> }
> }
>
> - struct ofport_dpif *old_root;
> - struct ofport_dpif *new_root;
> if (rstp_shift_root_learned_address(ofproto->rstp)) {
> - old_root = rstp_get_old_root_aux(ofproto->rstp);
> - new_root = rstp_get_new_root_aux(ofproto->rstp);
> - bundle_move(old_root->bundle, new_root->bundle);
> + bundle_move(rstp_get_old_root_aux(ofproto->rstp),
> + rstp_get_new_root_aux(ofproto->rstp));
> rstp_reset_root_changed(ofproto->rstp);
> }
> -
> }
> }
>
> @@ -2544,21 +2540,23 @@ bundle_flush_macs(struct ofbundle *bundle, bool
> all_ofprotos)
> ovs_rwlock_unlock(&ml->rwlock);
> }
>
> -static void bundle_move(struct ofbundle *old, struct ofbundle *new) {
> - struct ofproto_dpif *old_ofproto = old->ofproto;
> - struct ofproto_dpif *new_ofproto = new->ofproto;
> - struct mac_learning *old_ml = old_ofproto->ml;
> +static void
> +bundle_move(struct ofbundle *old, struct ofbundle *new)
> +{
> + struct ofproto_dpif *ofproto = old->ofproto;
> + struct mac_learning *ml = ofproto->ml;
> struct mac_entry *mac, *next_mac;
>
> - old_ofproto->backer->need_revalidate = REV_RECONFIGURE;
> - new_ofproto->backer->need_revalidate = REV_RECONFIGURE;
> - ovs_rwlock_wrlock(&old_ml->rwlock);
> - LIST_FOR_EACH_SAFE (mac, next_mac, lru_node, &old_ml->lrus) {
> + ovs_assert(new->ofproto == old->ofproto);
> +
> + ofproto->backer->need_revalidate = REV_RECONFIGURE;
> + ovs_rwlock_wrlock(&ml->rwlock);
> + LIST_FOR_EACH_SAFE (mac, next_mac, lru_node, &ml->lrus) {
> if (mac->port.p == old) {
> mac->port.p = new;
> }
> }
> - ovs_rwlock_unlock(&old_ml->rwlock);
> + ovs_rwlock_unlock(&ml->rwlock);
> }
>
> static struct ofbundle *
>
>
>
>> Daniele
>>
>> 2014-11-14 1:11 GMT+01:00 Jarno Rajahalme <[email protected]
>> <mailto:[email protected]>>:
>> Daniele,
>>
>> See comments inline.
>>
>> Thanks!
>>
>> Jarno
>>
>> On Nov 6, 2014, at 7:31 AM, Daniele Venturino <[email protected]
>> <mailto:[email protected]>> wrote:
>>
>> > All MAC addresses previously learned on a Root Port can be moved to an
>> > Alternate Port that becomes the new Root Port; i.e., Dynamic Filtering
>> > Entries for those addresses may be modified to show the new Root Port
>> > as
>> > their source, reducing the need to flood frames when recovering from
>> > some component failures.
>> >
>>
>> Indentation...
>>
>> > Signed-off-by: Daniele Venturino <[email protected]
>> > <mailto:[email protected]>>
>> > ---
>> > lib/rstp-common.h | 4 ++++
>> > lib/rstp-state-machines.c | 21 +++++++++++++++++++
>> > lib/rstp.c | 51
>> > ++++++++++++++++++++++++++++++++++++++---------
>> > lib/rstp.h | 10 ++++++++++
>> > ofproto/ofproto-dpif.c | 39 ++++++++++++++++++++++++++++++++++--
>> > 5 files changed, 114 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/lib/rstp-common.h b/lib/rstp-common.h
>> > index 6e56958..271db2a 100644
>> > --- a/lib/rstp-common.h
>> > +++ b/lib/rstp-common.h
>> > @@ -868,6 +868,10 @@ struct rstp {
>> > /* Interface to client. */
>> > void (*send_bpdu)(struct ofpbuf *bpdu, void *port_aux, void *rstp_aux);
>> > void *aux;
>> > +
>> > + bool root_changed;
>> > + void *old_root_aux;
>> > + void *new_root_aux;
>> > };
>> >
>> > #endif /* rstp-common.h */
>> > diff --git a/lib/rstp-state-machines.c b/lib/rstp-state-machines.c
>> > index 516093f..40eeea5 100644
>> > --- a/lib/rstp-state-machines.c
>> > +++ b/lib/rstp-state-machines.c
>> > @@ -287,7 +287,14 @@ updt_roles_tree__(struct rstp *r)
>> > struct rstp_port *p;
>> > int vsel;
>> > struct rstp_priority_vector best_vector, candidate_vector;
>> > + enum rstp_port_role new_root_old_role = ROLE_DESIGNATED;
>> > + uint16_t old_root_port_number = 0;
>> > + uint16_t new_root_port_number = 0;
>> >
>> > + old_root_port_number = r->root_port_id & 0x00ff;
>> > + if (old_root_port_number) {
>> > + r->old_root_aux = rstp_get_port_aux(r, old_root_port_number);
>> > + }
>> > vsel = -1;
>> > best_vector = r->bridge_priority;
>> > /* Letter c1) */
>> > @@ -317,12 +324,26 @@ updt_roles_tree__(struct rstp *r)
>> > r->root_times = p->port_times;
>> > r->root_times.message_age++;
>> > vsel = p->port_number;
>> > + new_root_old_role = p->role;
>> > }
>> > }
>> > r->root_priority = best_vector;
>> > r->root_port_id = best_vector.bridge_port_id;
>> > VLOG_DBG("%s: new Root is "RSTP_ID_FMT, r->name,
>> > RSTP_ID_ARGS(r->root_priority.root_bridge_id));
>> > + new_root_port_number = r->root_port_id & 0x00ff;
>> > + if (new_root_port_number) {
>> > + r->new_root_aux = rstp_get_port_aux(r, new_root_port_number);
>> > + }
>> > + /* Shift learned MAC addresses from an old Root Port to an existing
>> > + * Alternate Port. */
>> > + if (!r->root_changed
>> > + && new_root_old_role == ROLE_ALTERNATE
>> > + && new_root_port_number
>> > + && old_root_port_number
>> > + && new_root_port_number != old_root_port_number) {
>>
>> Indentation above is wrong, the lines should line right after the opening
>> parenthesis above.
>>
>> > + r->root_changed = true;
>> > + }
>> > /* Letters d) e) */
>> > HMAP_FOR_EACH (p, node, &r->ports) {
>> > p->designated_priority_vector.root_bridge_id =
>> > diff --git a/lib/rstp.c b/lib/rstp.c
>> > index fd42a7d..5256740 100644
>> > --- a/lib/rstp.c
>> > +++ b/lib/rstp.c
>> > @@ -867,6 +867,42 @@ out:
>> > return aux;
>> > }
>> >
>> > +bool
>> > +rstp_shift_root_learned_address(struct rstp *rstp) {
>> > + bool ret = false;
>> > + ovs_mutex_lock(&rstp_mutex);
>> > + if (rstp->root_changed) {
>> > + ret = true;
>> > + }
>> > + ovs_mutex_unlock(&rstp_mutex);
>> > + return ret;
>> > +}
>> > +
>>
>> Format function definitions like this:
>>
>> bool
>> rstp_shift_root_learned_address(struct rstp *rstp)
>> {
>> bool ret;
>>
>> ovs_mutex_lock(&rstp_mutex);
>> ret = rstp->root_changed;
>> ovs_mutex_unlock(&rstp_mutex);
>>
>> return ret;
>> }
>>
>> > +void *
>> > +rstp_get_old_root_aux(struct rstp *rstp) {
>> > + void *aux = NULL;
>> > + ovs_mutex_lock(&rstp_mutex);
>> > + aux = rstp->old_root_aux;
>> > + ovs_mutex_unlock(&rstp_mutex);
>> > + return aux;
>> > +}
>> > +
>> > +void *
>> > +rstp_get_new_root_aux(struct rstp *rstp) {
>> > + void *aux = NULL;
>> > + ovs_mutex_lock(&rstp_mutex);
>> > + aux = rstp->new_root_aux;
>> > + ovs_mutex_unlock(&rstp_mutex);
>> > + return aux;
>> > +}
>> > +
>> > +void
>> > +rstp_reset_root_changed(struct rstp *rstp) {
>> > + ovs_mutex_lock(&rstp_mutex);
>> > + rstp->root_changed = false;
>> > + ovs_mutex_unlock(&rstp_mutex);
>> > +}
>> > +
>> > /* Returns the port in 'rstp' with number 'port_number'.
>> > *
>> > * XXX: May only be called while concurrent deletion of ports is excluded.
>> > */
>> > @@ -878,8 +914,7 @@ rstp_get_port__(struct rstp *rstp, uint16_t
>> > port_number)
>> >
>> > ovs_assert(rstp && port_number > 0 && port_number <= RSTP_MAX_PORTS);
>> >
>> > - HMAP_FOR_EACH_WITH_HASH (port, node, hash_int(port_number, 0),
>> > - &rstp->ports) {
>> > + HMAP_FOR_EACH (port, node, &rstp->ports) {
>>
>> Why lose the ‘_WITH_HASH’? It seems pointless to iterate over all the ports,
>> when we just care of the port with the ‘port_number’?
>>
>> > if (port->port_number == port_number) {
>> > return port;
>> > }
>> > @@ -901,16 +936,14 @@ rstp_get_port(struct rstp *rstp, uint16_t
>> > port_number)
>> >
>> > void *
>> > rstp_get_port_aux(struct rstp *rstp, uint16_t port_number)
>> > - OVS_EXCLUDED(rstp_mutex)
>> > + OVS_REQUIRES(rstp_mutex)
>> > {
>> > struct rstp_port *p;
>> > - void *aux;
>> > -
>> > - ovs_mutex_lock(&rstp_mutex);
>> > p = rstp_get_port__(rstp, port_number);
>> > - aux = p->aux;
>> > - ovs_mutex_unlock(&rstp_mutex);
>> > - return aux;
>> > + if (p) {
>> > + return p->aux;
>> > + }
>> > + return NULL;
>> > }
>> >
>> > /* Updates the port_enabled parameter. */
>> > diff --git a/lib/rstp.h b/lib/rstp.h
>> > index b05cdf2..3b40a00 100644
>> > --- a/lib/rstp.h
>> > +++ b/lib/rstp.h
>> > @@ -164,6 +164,16 @@ void *rstp_get_next_changed_port_aux(struct rstp *,
>> > struct rstp_port **)
>> > void rstp_port_set_mac_operational(struct rstp_port *,
>> > bool new_mac_operational)
>> > OVS_EXCLUDED(rstp_mutex);
>> > +bool
>> > +rstp_shift_root_learned_address(struct rstp *rstp)
>> > + OVS_EXCLUDED(rstp_mutex);
>>
>> Function declarations should have the return type and the function name on
>> the same line.
>>
>> > +void *rstp_get_old_root_aux(struct rstp *)
>> > + OVS_EXCLUDED(rstp_mutex);
>> > +void *rstp_get_new_root_aux(struct rstp *)
>> > + OVS_EXCLUDED(rstp_mutex);
>> > +void
>> > +rstp_reset_root_changed(struct rstp *)
>> > + OVS_EXCLUDED(rstp_mutex);
>> >
>>
>> Here, too.
>>
>> > /* Bridge setters */
>> > void rstp_set_bridge_address(struct rstp *, rstp_identifier bridge_address)
>> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> > index 2fd9896..2389a18 100644
>> > --- a/ofproto/ofproto-dpif.c
>> > +++ b/ofproto/ofproto-dpif.c
>> > @@ -141,6 +141,7 @@ static void bundle_del_port(struct ofport_dpif *);
>> > static void bundle_run(struct ofbundle *);
>> > static void bundle_wait(struct ofbundle *);
>> > static void bundle_flush_macs(struct ofbundle *, bool);
>> > +static void bundle_move(struct ofbundle *, struct ofbundle *);
>> >
>> > static void stp_run(struct ofproto_dpif *ofproto);
>> > static void stp_wait(struct ofproto_dpif *ofproto);
>> > @@ -2096,11 +2097,15 @@ update_rstp_port_state(struct ofport_dpif *ofport)
>> > netdev_get_name(ofport->up.netdev),
>> > rstp_state_name(ofport->rstp_state),
>> > rstp_state_name(state));
>> > +
>> > if (rstp_learn_in_state(ofport->rstp_state)
>> > != rstp_learn_in_state(state)) {
>> > /* xxx Learning action flows should also be flushed. */
>> > if (ofport->bundle) {
>> > - bundle_flush_macs(ofport->bundle, false);
>> > + if(!rstp_shift_root_learned_address(ofproto->rstp)
>> > + || ofport != rstp_get_old_root_aux(ofproto->rstp)) {
>> > + bundle_flush_macs(ofport->bundle, false);
>> > + }
>> > }
>> > }
>> > fwd_change = rstp_forward_in_state(ofport->rstp_state)
>> > @@ -2147,8 +2152,21 @@ rstp_run(struct ofproto_dpif *ofproto)
>> > * p->fdb_flush) and not periodically.
>> > */
>> > while ((ofport = rstp_check_and_reset_fdb_flush(ofproto->rstp,
>> > &rp))) {
>> > - bundle_flush_macs(ofport->bundle, false);
>> > + if (!rstp_shift_root_learned_address(ofproto->rstp)
>> > + || ofport != rstp_get_old_root_aux(ofproto->rstp)) {
>> > + bundle_flush_macs(ofport->bundle, false);
>> > + }
>> > + }
>> > +
>> > + struct ofport_dpif *old_root;
>> > + struct ofport_dpif *new_root;
>> > + if (rstp_shift_root_learned_address(ofproto->rstp)) {
>> > + old_root = rstp_get_old_root_aux(ofproto->rstp);
>> > + new_root = rstp_get_new_root_aux(ofproto->rstp);
>> > + bundle_move(old_root->bundle, new_root->bundle);
>> > + rstp_reset_root_changed(ofproto->rstp);
>> > }
>>
>> Better write the latter part as:
>>
>> if (rstp_shift_root_learned_address(ofproto->rstp)) {
>> bundle_move(rstp_get_old_root_aux(ofproto->rstp),
>> rstp_get_new_root_aux(ofproto->rstp));
>> rstp_reset_root_changed(ofproto->rstp);
>> }
>>
>>
>> > +
>> > }
>> > }
>> >
>> > @@ -2495,6 +2513,23 @@ bundle_flush_macs(struct ofbundle *bundle, bool
>> > all_ofprotos)
>> > ovs_rwlock_unlock(&ml->rwlock);
>> > }
>> >
>> > +static void bundle_move(struct ofbundle *old, struct ofbundle *new) {
>> > + struct ofproto_dpif *old_ofproto = old->ofproto;
>> > + struct ofproto_dpif *new_ofproto = new->ofproto;
>>
>> Can the bundles really be from different bridges, as they both are managed
>> by the same RSTP state machine? If both ofproto’s are the same, then this
>> function can be simplified.
>>
>> > + struct mac_learning *old_ml = old_ofproto->ml;
>> > + struct mac_entry *mac, *next_mac;
>> > +
>> > + old_ofproto->backer->need_revalidate = REV_RECONFIGURE;
>> > + new_ofproto->backer->need_revalidate = REV_RECONFIGURE;
>> > + ovs_rwlock_wrlock(&old_ml->rwlock);
>> > + LIST_FOR_EACH_SAFE (mac, next_mac, lru_node, &old_ml->lrus) {
>> > + if (mac->port.p == old) {
>> > + mac->port.p = new;
>> > + }
>> > + }
>> > + ovs_rwlock_unlock(&old_ml->rwlock);
>> > +}
>> > +
>> > static struct ofbundle *
>> > bundle_lookup(const struct ofproto_dpif *ofproto, void *aux)
>> > {
>> > --
>> > 1.8.1.2
>> >
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
