[ovs-dev] [PATCH v2 1/3] ovs-vswitchd: An option to wait for userspace flow restore to complete.

Gurucharan Shetty shettyg at nicira.com
Tue May 28 14:02:17 UTC 2013


While upgrading openvswitch, it helps to restore openflow flows before
starting packet processing.  Typically we want to restart openvswitch,
add the openflow flows and then start packet processing.

To do this, we look for the other_config:flow-restore-wait column
in the Open_vSwitch table during startup. If set as true, we disable
receiving packets from the datapath, expiring or flushing flows and
running any periodic ofproto activities. This option does not prevent
the addition and deletion of ports. Once this option is set to false,
we return to normal processing.

An upcoming commit will use this feature in Open vSwitch startup scripts.

Bug #16086.
Signed-off-by: Gurucharan Shetty <gshetty at nicira.com>
---
 ofproto/ofproto-dpif.c |   55 +++++++++++++++++++++++++++++++++++++++++++++---
 vswitchd/bridge.c      |   11 ++++++++++
 vswitchd/vswitch.xml   |   17 +++++++++++++++
 3 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index c4f7d25..31f04f3 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -72,6 +72,8 @@ COVERAGE_DEFINE(facet_suppress);
  * flow translation. */
 #define MAX_RESUBMIT_RECURSION 64
 
+extern bool flow_restore_wait;
+
 /* Number of implemented OpenFlow tables. */
 enum { N_TABLES = 255 };
 enum { TBL_INTERNAL = N_TABLES - 1 };    /* Used for internal hidden rules. */
@@ -671,6 +673,7 @@ struct dpif_backer {
     struct tag_set revalidate_set; /* Revalidate only matching facets. */
 
     struct hmap drop_keys; /* Set of dropped odp keys. */
+    bool recv_set_enable; /* Enables or disables receiving packets. */
 };
 
 /* All existing ofproto_backer instances, indexed by ofproto->up.type. */
@@ -947,6 +950,21 @@ type_run(const char *type)
         push_all_stats();
     }
 
+    /* If vswitchd started with other_config:flow_restore_wait set as "true",
+     * and the configuration has now changed to "false", enable receiving
+     * packets from the datapath. */
+    if (!backer->recv_set_enable && !flow_restore_wait) {
+        backer->recv_set_enable = true;
+
+        error = dpif_recv_set(backer->dpif, backer->recv_set_enable);
+        if (error) {
+            VLOG_ERR("Failed to enable receiving packets in dpif.");
+            return error;
+        }
+        dpif_flow_flush(backer->dpif);
+        backer->need_revalidate = REV_RECONFIGURE;
+    }
+
     if (backer->need_revalidate
         || !tag_set_is_empty(&backer->revalidate_set)) {
         struct tag_set revalidate_set = backer->revalidate_set;
@@ -1040,7 +1058,10 @@ type_run(const char *type)
         }
     }
 
-    if (timer_expired(&backer->next_expiration)) {
+    if (!backer->recv_set_enable) {
+        /* A minimum of 1000 ms delay. */
+        timer_set_duration(&backer->next_expiration, 1000);
+    } else if (timer_expired(&backer->next_expiration)) {
         int delay = expire(backer);
         timer_set_duration(&backer->next_expiration, delay);
     }
@@ -1106,6 +1127,11 @@ dpif_backer_run_fast(struct dpif_backer *backer, int max_batch)
 {
     unsigned int work;
 
+    /* If recv_set_enable is false, we should not handle upcalls. */
+    if (!backer->recv_set_enable) {
+        return 0;
+    }
+
     /* Handle one or more batches of upcalls, until there's nothing left to do
      * or until we do a fixed total amount of work.
      *
@@ -1305,9 +1331,16 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
     backer->need_revalidate = 0;
     simap_init(&backer->tnl_backers);
     tag_set_init(&backer->revalidate_set);
+    if (flow_restore_wait) {
+        backer->recv_set_enable = false;
+    } else {
+        backer->recv_set_enable = true;
+    }
     *backerp = backer;
 
-    dpif_flow_flush(backer->dpif);
+    if (backer->recv_set_enable) {
+        dpif_flow_flush(backer->dpif);
+    }
 
     /* Loop through the ports already on the datapath and remove any
      * that we don't need anymore. */
@@ -1331,7 +1364,7 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
 
     shash_add(&all_dpif_backers, type, backer);
 
-    error = dpif_recv_set(backer->dpif, true);
+    error = dpif_recv_set(backer->dpif, backer->recv_set_enable);
     if (error) {
         VLOG_ERR("failed to listen on datapath of type %s: %s",
                  type, strerror(error));
@@ -1569,6 +1602,12 @@ run_fast(struct ofproto *ofproto_)
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
     struct ofport_dpif *ofport;
 
+    /* Do not perform any periodic activity required by 'ofproto' while
+     * waiting for flow restore to complete. */
+    if (flow_restore_wait) {
+        return 0;
+    }
+
     HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
         port_run_fast(ofport);
     }
@@ -1584,6 +1623,12 @@ run(struct ofproto *ofproto_)
     struct ofbundle *bundle;
     int error;
 
+    /* Do not perform any periodic activity required by 'ofproto' while
+     * waiting for flow restore to complete. */
+    if (flow_restore_wait) {
+        return 0;
+    }
+
     if (!clogged) {
         complete_operations(ofproto);
     }
@@ -1658,6 +1703,10 @@ wait(struct ofproto *ofproto_)
     struct ofport_dpif *ofport;
     struct ofbundle *bundle;
 
+    if (flow_restore_wait) {
+        return;
+    }
+
     if (!clogged && !list_is_empty(&ofproto->completions)) {
         poll_immediate_wake();
     }
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index e10036c..db70b77 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -183,6 +183,9 @@ static long long int iface_stats_timer = LLONG_MIN;
 #define OFP_PORT_ACTION_WINDOW 10
 static bool reconfiguring = false;
 
+/* The default value of true waits for flow restore. */
+bool flow_restore_wait = true;
+
 static void add_del_bridges(const struct ovsrec_open_vswitch *);
 static void bridge_update_ofprotos(void);
 static void bridge_create(const struct ovsrec_bridge *);
@@ -2321,6 +2324,14 @@ bridge_run(void)
      * returns immediately. */
     bridge_init_ofproto(cfg);
 
+    /* bridge_run(), when called for the first time will always have
+     * flow_restore_wait as true. Once the value is false, we no longer
+     * need to check its value. */
+    if (flow_restore_wait) {
+        flow_restore_wait = smap_get_bool(&cfg->other_config,
+                                          "flow-restore-wait", false);
+    }
+
     /* Let each datapath type do the work that it needs to do. */
     sset_init(&types);
     ofproto_enumerate_types(&types);
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 4396779..3896946 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -104,6 +104,23 @@
           column or to <code>false</code> to explicitly disable it.
         </column>
 
+        <column name="other_config" key="flow-restore-wait"
+                type='{"type": "boolean"}'>
+          <p>
+          When <code>ovs-vswitchd</code> is started with this value set as
+          <code>true</code>, it will neither flush or expire previously set
+          datapath flows nor will it send and receive any packets to or from
+          the datapath.  When this value is later set to <code>false</code>,
+          <code>ovs-vswitchd</code> will start receiving packets from the
+          datapath and re-setup the flows.
+          </p>
+          <p>
+          This option is only useful when used during Open vSwitch upgrades or
+          restarts and flow restoration is needed before processing any new
+          packets.
+          </p>
+        </column>
+
         <column name="statistics" key="cpu"
                 type='{"type": "integer", "minInteger": 1}'>
           <p>
-- 
1.7.9.5




More information about the dev mailing list