[ovs-dev] [PATHCH v2] Trigger bridge reconfigure from ofproto layer

[gyang at nicira.com]

From: Guolin Yang <gyang at nicira.com>

There are two models in OVS in configure datapath port:
1. Datapath port created before user space bridge is configured.
   In this model, datapath can be deleted or added independent of
   user-space.
2. Datapath port created when ovsdb is requested to add the relevant
   port.

Traditionally OVS supports the second model which is used in Linux
platform.

With more platform support, OVS requires to support first model. In
this case, datapath port change detected by ofproto layer need to trigger
brdige to reconfigure so that bridge and ofproto layers can be
synchronized.

This change introduced a API for ofproto to request reconfiguration and
for bridge to detect the request.

Signed-off-by: Guolin Yang <gyang at nicira.com>
---
v1-->v2
Add comments based on Ben's review feedback.
---
 ofproto/ofproto-dpif.c |   31 +++++++++++++++++++++++++++++++
 vswitchd/bridge.c      |    7 ++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b91b3df..5342831 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -515,6 +515,9 @@ struct ofproto_dpif {
  * may be disabled with the "ovs-appctl dpif/disable-megaflows" command. */
 static bool enable_megaflows = true;
 
+/* ofproto requires bridge reconfigure */
+static bool need_reconfigure = false;
+
 /* All existing ofproto_dpif instances, indexed by ->up.name. */
 static struct hmap all_ofproto_dpifs = HMAP_INITIALIZER(&all_ofproto_dpifs);
 
@@ -870,6 +873,20 @@ type_run(const char *type)
     return 0;
 }
 
+/* ofproto layer set need configure to true to request
+ * bridge reconfiguration, while bridge will clear the
+ * the need by passing false to the API. */
+void
+ofproto_set_need_reconfigure(bool need)
+{
+    need_reconfigure = need;
+}
+
+bool
+ofproto_need_reconfigure(void) {
+    return need_reconfigure;
+}
+
 /* Check for and handle port changes in 'backer''s dpif. */
 static void
 process_dpif_port_changes(struct dpif_backer *backer)
@@ -961,6 +978,20 @@ process_dpif_port_change(struct dpif_backer *backer, const char *devname)
         /* The port was added, but we don't know with which
          * ofproto we should associate it.  Delete it. */
         dpif_port_del(backer->dpif, port.port_no);
+        /* While there are two ways of adding/deleting datapath
+         * port:
+         * 1. Datapath added/deleted as requested by bridge layer
+         * 2. Datapath added/deleted independent of bridge layer
+         *
+         * In second case, when a datapath port is added/deleted,
+         * normally there will be other process to program the bridge
+         * (through ovsdb) to synchronize the user space and
+         * datapath. However there are cases where bridge configuration
+         * is not changed, but datapath port may be changed, in this
+         * case, ofproto layer detects the change and request the bridge
+         * to reconfigure to synchronize user space and datapath
+         * configuration. */
+        ofproto_set_need_reconfigure(true);
     } else {
         struct ofport_dpif *ofport;
 
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index da2dc42..88b1e8c 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2404,7 +2404,9 @@ bridge_run(void)
             }
         }
 
-        if (ovsdb_idl_get_seqno(idl) != idl_seqno || vlan_splinters_changed) {
+        if (ovsdb_idl_get_seqno(idl) != idl_seqno || vlan_splinters_changed ||
+            ofproto_need_reconfigure()) {
+            ofproto_set_need_reconfigure(false);
             idl_seqno = ovsdb_idl_get_seqno(idl);
             if (cfg) {
                 reconf_txn = ovsdb_idl_txn_create(idl);
@@ -2513,6 +2515,9 @@ bridge_wait(void)
         poll_immediate_wake();
     }
 
+    if (ofproto_need_reconfigure()) {
+        poll_immediate_wake();
+    }
     sset_init(&types);
     ofproto_enumerate_types(&types);
     SSET_FOR_EACH (type, &types) {
-- 
1.7.9.5