[ovs-dev] [PATCH 1/4] ofproto-dpif: Add 'force-miss-model' configuration

Simon Horman horms at verge.net.au
Wed Jun 26 08:25:55 UTC 2013


From: Joe Stringer <joe at wand.net.nz>

This adds support for specifying flow miss handling behaviour at
runtime, through a new "other-config" option in the Open_vSwitch table.
This takes precedence over flow-eviction-threshold.

By default, the behaviour is the same as before. If force-miss-model is
set to 'with-facets', then flow miss handling will always result in the
creation of new facets and flow-eviction-threshold will be ignored. If
force-miss-model is set to 'without-facets', then flow miss handling will never
result in the creation of new facets (effectively the same as setting the
flow-eviction-threshold to 0, which is not currently configurable).

We intend to use this configuration option in the testsuite to force
particular code paths to be used, allowing us to improve test coverage.

Signed-off-by: Joe Stringer <joe at wand.net.nz>
---
v14
* Configure using strings: 'auto', 'with-facets', 'without-facets'
  Use enums rather than unsigned
v13
* Initial post
---
 ofproto/ofproto-dpif.c     |  9 +++++++++
 ofproto/ofproto-provider.h |  4 ++++
 ofproto/ofproto.c          |  8 ++++++++
 ofproto/ofproto.h          |  8 ++++++++
 vswitchd/bridge.c          | 23 +++++++++++++++++++++++
 vswitchd/vswitch.xml       | 22 ++++++++++++++++++++++
 6 files changed, 74 insertions(+)

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index c6a7abc..85e5376 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3361,6 +3361,15 @@ flow_miss_should_make_facet(struct flow_miss *miss, struct flow_wildcards *wc)
     struct dpif_backer *backer = miss->ofproto->backer;
     uint32_t hash;
 
+    switch (flow_miss_model) {
+    case OFPROTO_HANDLE_MISS_AUTO:
+        break;
+    case OFPROTO_HANDLE_MISS_WITH_FACETS:
+        return true;
+    case OFPROTO_HANDLE_MISS_WITHOUT_FACETS:
+        return false;
+    }
+
     if (!backer->governor) {
         size_t n_subfacets;
 
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 565cb01..1655c3a 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -238,6 +238,10 @@ struct rule {
  * ofproto-dpif implementation */
 extern unsigned flow_eviction_threshold;
 
+/* Determines which model to use for handling misses in the ofproto-dpif
+ * implementation */
+extern enum ofproto_flow_miss_model flow_miss_model;
+
 static inline struct rule *
 rule_from_cls_rule(const struct cls_rule *cls_rule)
 {
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 5e6a252..98ea0fa 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -221,6 +221,7 @@ static size_t n_ofproto_classes;
 static size_t allocated_ofproto_classes;
 
 unsigned flow_eviction_threshold = OFPROTO_FLOW_EVICTION_THRESHOLD_DEFAULT;
+enum ofproto_flow_miss_model flow_miss_model = OFPROTO_HANDLE_MISS_AUTO;
 
 /* Map from datapath name to struct ofproto, for use by unixctl commands. */
 static struct hmap all_ofprotos = HMAP_INITIALIZER(&all_ofprotos);
@@ -573,6 +574,13 @@ ofproto_set_flow_eviction_threshold(unsigned threshold)
                                   threshold);
 }
 
+/* Sets the path for handling flow misses. */
+void
+ofproto_set_flow_miss_model(unsigned model)
+{
+    flow_miss_model = model;
+}
+
 /* If forward_bpdu is true, the NORMAL action will forward frames with
  * reserved (e.g. STP) destination Ethernet addresses. if forward_bpdu is false,
  * the NORMAL action will drop these frames. */
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 2609c94..8c90b86 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -216,6 +216,13 @@ int ofproto_port_dump_done(struct ofproto_port_dump *);
 #define OFPROTO_FLOW_EVICTION_THRESHOLD_DEFAULT  2500
 #define OFPROTO_FLOW_EVICTION_THRESHOLD_MIN 100
 
+/* How flow misses should be handled in ofproto-dpif */
+enum ofproto_flow_miss_model {
+    OFPROTO_HANDLE_MISS_AUTO,           /* Based on flow eviction threshold. */
+    OFPROTO_HANDLE_MISS_WITH_FACETS,    /* Always create facets. */
+    OFPROTO_HANDLE_MISS_WITHOUT_FACETS  /* Always handle without facets.*/
+};
+
 const char *ofproto_port_open_type(const char *datapath_type,
                                    const char *port_type);
 int ofproto_port_add(struct ofproto *, struct netdev *, ofp_port_t *ofp_portp);
@@ -237,6 +244,7 @@ void ofproto_set_extra_in_band_remotes(struct ofproto *,
                                        const struct sockaddr_in *, size_t n);
 void ofproto_set_in_band_queue(struct ofproto *, int queue_id);
 void ofproto_set_flow_eviction_threshold(unsigned threshold);
+void ofproto_set_flow_miss_model(unsigned model);
 void ofproto_set_forward_bpdu(struct ofproto *, bool forward_bpdu);
 void ofproto_set_mac_table_config(struct ofproto *, unsigned idle_time,
                                   size_t max_entries);
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index abae7f5..4ac2b26 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -196,6 +196,7 @@ static size_t bridge_get_controllers(const struct bridge *br,
 static void bridge_add_del_ports(struct bridge *,
                                  const unsigned long int *splinter_vlans);
 static void bridge_refresh_ofp_port(struct bridge *);
+static void bridge_configure_flow_miss_model(const char *opt);
 static void bridge_configure_datapath_id(struct bridge *);
 static void bridge_configure_netflow(struct bridge *);
 static void bridge_configure_forward_bpdu(struct bridge *);
@@ -499,6 +500,9 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
         smap_get_int(&ovs_cfg->other_config, "flow-eviction-threshold",
                      OFPROTO_FLOW_EVICTION_THRESHOLD_DEFAULT));
 
+    bridge_configure_flow_miss_model(smap_get(&ovs_cfg->other_config,
+                                              "force-miss-model"));
+
     /* Destroy "struct bridge"s, "struct port"s, and "struct iface"s according
      * to 'ovs_cfg' while update the "if_cfg_queue", with only very minimal
      * configuration otherwise.
@@ -804,6 +808,25 @@ port_configure(struct port *port)
     free(s.lacp_slaves);
 }
 
+static void
+bridge_configure_flow_miss_model(const char *opt)
+{
+    enum ofproto_flow_miss_model model = OFPROTO_HANDLE_MISS_AUTO;
+
+    if (opt) {
+        if (strcmp(opt, "with-facets")) {
+            model = OFPROTO_HANDLE_MISS_WITH_FACETS;
+            VLOG_INFO("Handling all flow misses by creating facets.\n");
+        }
+        if (strcmp(opt, "without-facets")) {
+            model = OFPROTO_HANDLE_MISS_WITHOUT_FACETS;
+            VLOG_INFO("Handling all flow misses without creating facets.\n");
+        }
+    }
+
+    ofproto_set_flow_miss_model(model);
+}
+
 /* Pick local port hardware address and datapath ID for 'br'. */
 static void
 bridge_configure_datapath_id(struct bridge *br)
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index b962849..12780d6 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -136,6 +136,28 @@
           The default is 2500.  Values below 100 will be rounded up to 100.
         </p>
       </column>
+
+      <column name="other_config" key="force-miss-model">
+        <p>
+          Specifies userspace behaviour for handling flow misses. This takes
+          precedence over flow-eviction-threshold.
+        </p>
+        <p>
+          <dl>
+            <dt><code>auto</code></dt>
+            <dd>Handle automatically based on the flow-eviction-threshold and
+            the flow setup governer (default, recommended).</dd>
+            <dt><code>with-facets</code></dt>
+            <dd>Always create facets. Expensive kernel flow creation and
+            statistics tracking is always performed, even on flows with only
+            a small number of packets.</dd>
+            <dt><code>without-facets</code></dt>
+            <dd>Always handle without facets. Forces flow misses to be handled
+            in userspace. May cause an increase in CPU usage and packet loss
+            on high throughput.</dd>
+          </dl>
+        </p>
+      </column>
     </group>
 
     <group title="Status">
-- 
1.8.2.1




More information about the dev mailing list