rstp: shift learned MAC addresses to new Root port.

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.

Signed-off-by: Daniele Venturino <daniele.venturino@m3s.it>
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>

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 */

lib/rstp-state-machines.c

@@ -290,7 +290,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) */
@@ -320,12 +327,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) {
+        r->root_changed = true;
+    }
     /* Letters d) e) */
     HMAP_FOR_EACH (p, node, &r->ports) {
         p->designated_priority_vector.root_bridge_id =

lib/rstp.c

@@ -711,21 +711,32 @@ 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 =
         is_port_number_available__(port->rstp, port_number, port)
         ? 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 @@ rstp_check_and_reset_fdb_flush(struct rstp *rstp, struct rstp_port **port)
         (*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,9 +882,54 @@ 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;
+
+    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;
+
+    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;
+
+    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. */
@@ -902,17 +963,15 @@ 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)
+rstp_get_port_aux__(struct rstp *rstp, uint16_t port_number)
+    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. */
@@ -1084,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;
@@ -1095,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);
@@ -1324,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;
 

lib/rstp.h

@@ -164,6 +164,14 @@ 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 *)
+    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 *)
+    OVS_EXCLUDED(rstp_mutex);
 
 /* Bridge setters */
 void rstp_set_bridge_address(struct rstp *, rstp_identifier bridge_address)
@@ -232,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 */

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);
@@ -2127,11 +2128,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)
+                    || rstp_get_old_root_aux(ofproto->rstp) != ofport) {
+                    bundle_flush_macs(ofport->bundle, false);
+                }
             }
         }
         fwd_change = rstp_forward_in_state(ofport->rstp_state)
@@ -2178,7 +2183,16 @@ 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)
+                || rstp_get_old_root_aux(ofproto->rstp) != ofport) {
+                bundle_flush_macs(ofport->bundle, false);
+            }
+        }
+
+        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);
         }
     }
 }
@@ -2526,6 +2540,25 @@ 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 *ofproto = old->ofproto;
+    struct mac_learning *ml = ofproto->ml;
+    struct mac_entry *mac, *next_mac;
+
+    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(&ml->rwlock);
+}
+
 static struct ofbundle *
 bundle_lookup(const struct ofproto_dpif *ofproto, void *aux)
 {