ofproto-dpif: Set flow-eviction-threshold globally.

With the single datapath, it no longer makes sense to have a per ofproto flow eviction threshold. This patch moves the flow eviction threshold to the Open_vSwitch table making the setting global, though still treated separately for each ofproto. A future patch will unify flow eviction on a per datapath basis. Signed-off-by: Ethan Jackson <[email protected]>
smehner1 · Jun 7, 2013 · 380f49c · 380f49c
1 parent dc54ef3
commit 380f49c
Show file tree

Hide file tree

Showing 7 changed files with 41 additions and 50 deletions.
diff --git a/NEWS b/NEWS
@@ -6,6 +6,7 @@ post-v1.11.0
 	added "remote_ip=flow" and "local_ip=flow" options.
     - New "check-oftest" Makefile target for running OFTest against Open
       vSwitch.  See README-OFTest for details.
+    - The flow eviction threshold has been moved to the Open_vSwitch table.
 
 
 v1.11.0 - xx xxx xxxx

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
@@ -1645,7 +1645,7 @@ run(struct ofproto *ofproto_)
          * For hysteresis, the number of subfacets to drop the governor is
          * smaller than the number needed to trigger its creation. */
         n_subfacets = hmap_count(&ofproto->subfacets);
-        if (n_subfacets * 4 < ofproto->up.flow_eviction_threshold
+        if (n_subfacets * 4 < flow_eviction_threshold
             && governor_is_idle(ofproto->governor)) {
             governor_destroy(ofproto->governor);
             ofproto->governor = NULL;
@@ -3675,7 +3675,7 @@ flow_miss_should_make_facet(struct ofproto_dpif *ofproto,
         size_t n_subfacets;
 
         n_subfacets = hmap_count(&ofproto->subfacets);
-        if (n_subfacets * 2 <= ofproto->up.flow_eviction_threshold) {
+        if (n_subfacets * 2 <= flow_eviction_threshold) {
             return true;
         }
 
@@ -4501,7 +4501,7 @@ subfacet_max_idle(const struct ofproto_dpif *ofproto)
      * that is installed in the kernel gets dropped in the appropriate bucket.
      * After the histogram has been built, we compute the cutoff so that only
      * the most-recently-used 1% of subfacets (but at least
-     * ofproto->up.flow_eviction_threshold flows) are kept cached.  At least
+     * flow_eviction_threshold flows) are kept cached.  At least
      * the most-recently-used bucket of subfacets is kept, so actually an
      * arbitrary number of subfacets can be kept in any given expiration run
      * (though the next run will delete most of those unless they receive
@@ -4520,7 +4520,7 @@ subfacet_max_idle(const struct ofproto_dpif *ofproto)
     int i;
 
     total = hmap_count(&ofproto->subfacets);
-    if (total <= ofproto->up.flow_eviction_threshold) {
+    if (total <= flow_eviction_threshold) {
         return N_BUCKETS * BUCKET_WIDTH;
     }
 
@@ -4539,7 +4539,7 @@ subfacet_max_idle(const struct ofproto_dpif *ofproto)
     do {
         subtotal += buckets[bucket++];
     } while (bucket < N_BUCKETS &&
-             subtotal < MAX(ofproto->up.flow_eviction_threshold, total / 100));
+             subtotal < MAX(flow_eviction_threshold, total / 100));
 
     if (VLOG_IS_DBG_ENABLED()) {
         struct ds s;

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
@@ -48,9 +48,6 @@ struct ofproto {
     /* Settings. */
     uint64_t fallback_dpid;     /* Datapath ID if no better choice found. */
     uint64_t datapath_id;       /* Datapath ID. */
-    unsigned flow_eviction_threshold; /* Threshold at which to begin flow
-                                       * table eviction. Only affects the
-                                       * ofproto-dpif implementation */
     bool forward_bpdu;          /* Option to allow forwarding of BPDU frames
                                  * when NORMAL action is invoked. */
     char *mfr_desc;             /* Manufacturer (NULL for default)b. */
@@ -233,6 +230,10 @@ struct rule {
                                  * is expirable, otherwise empty. */
 };
 
+/* Threshold at which to begin flow table eviction. Only affects the
+ * ofproto-dpif implementation */
+extern unsigned flow_eviction_threshold;
+
 static inline struct rule *
 rule_from_cls_rule(const struct cls_rule *cls_rule)
 {

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
@@ -220,6 +220,8 @@ static const struct ofproto_class **ofproto_classes;
 static size_t n_ofproto_classes;
 static size_t allocated_ofproto_classes;
 
+unsigned flow_eviction_threshold = OFPROTO_FLOW_EVICTION_THRESHOLD_DEFAULT;
+
 /* Map from datapath name to struct ofproto, for use by unixctl commands. */
 static struct hmap all_ofprotos = HMAP_INITIALIZER(&all_ofprotos);
 
@@ -408,8 +410,6 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     hmap_insert(&all_ofprotos, &ofproto->hmap_node,
                 hash_string(ofproto->name, 0));
     ofproto->datapath_id = 0;
-    ofproto_set_flow_eviction_threshold(ofproto,
-                                        OFPROTO_FLOW_EVICTION_THRESHOLD_DEFAULT);
     ofproto->forward_bpdu = false;
     ofproto->fallback_dpid = pick_fallback_dpid();
     ofproto->mfr_desc = NULL;
@@ -566,13 +566,10 @@ ofproto_set_in_band_queue(struct ofproto *ofproto, int queue_id)
 /* Sets the number of flows at which eviction from the kernel flow table
  * will occur. */
 void
-ofproto_set_flow_eviction_threshold(struct ofproto *ofproto, unsigned threshold)
+ofproto_set_flow_eviction_threshold(unsigned threshold)
 {
-    if (threshold < OFPROTO_FLOW_EVICTION_THRESHOLD_MIN) {
-        ofproto->flow_eviction_threshold = OFPROTO_FLOW_EVICTION_THRESHOLD_MIN;
-    } else {
-        ofproto->flow_eviction_threshold = threshold;
-    }
+    flow_eviction_threshold = MAX(OFPROTO_FLOW_EVICTION_THRESHOLD_MIN,
+                                  threshold);
 }
 
 /* If forward_bpdu is true, the NORMAL action will forward frames with

diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
@@ -236,7 +236,7 @@ void ofproto_reconnect_controllers(struct ofproto *);
 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(struct ofproto *, unsigned threshold);
+void ofproto_set_flow_eviction_threshold(unsigned threshold);
 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
@@ -197,7 +197,6 @@ 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_datapath_id(struct bridge *);
-static void bridge_configure_flow_eviction_threshold(struct bridge *);
 static void bridge_configure_netflow(struct bridge *);
 static void bridge_configure_forward_bpdu(struct bridge *);
 static void bridge_configure_mac_table(struct bridge *);
@@ -495,6 +494,10 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
     ovs_assert(!reconfiguring);
     reconfiguring = true;
 
+    ofproto_set_flow_eviction_threshold(
+        smap_get_int(&ovs_cfg->other_config, "flow-eviction-threshold",
+                     OFPROTO_FLOW_EVICTION_THRESHOLD_DEFAULT));
+
     /* 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.
@@ -613,7 +616,6 @@ bridge_reconfigure_continue(const struct ovsrec_open_vswitch *ovs_cfg)
             }
         }
         bridge_configure_mirrors(br);
-        bridge_configure_flow_eviction_threshold(br);
         bridge_configure_forward_bpdu(br);
         bridge_configure_mac_table(br);
         bridge_configure_remotes(br, managers, n_managers);
@@ -623,6 +625,13 @@ bridge_reconfigure_continue(const struct ovsrec_open_vswitch *ovs_cfg)
         bridge_configure_stp(br);
         bridge_configure_tables(br);
         bridge_configure_dp_desc(br);
+
+        if (smap_get(&br->cfg->other_config, "flow-eviction-threshold")) {
+            /* XXX: Remove this warning message eventually. */
+            VLOG_WARN_ONCE("As of June 2013, flow-eviction-threshold has been"
+                           " moved to the Open_vSwitch table.  Ignoring its"
+                           " setting in the bridge table.");
+        }
     }
     free(managers);
 
@@ -1559,23 +1568,6 @@ iface_create(struct bridge *br, struct if_cfg *if_cfg, uint16_t ofp_port)
     return ok;
 }
 
-/* Set Flow eviction threshold */
-static void
-bridge_configure_flow_eviction_threshold(struct bridge *br)
-{
-    const char *threshold_str;
-    unsigned threshold;
-
-    threshold_str = smap_get(&br->cfg->other_config,
-                             "flow-eviction-threshold");
-    if (threshold_str) {
-        threshold = strtoul(threshold_str, NULL, 10);
-    } else {
-        threshold = OFPROTO_FLOW_EVICTION_THRESHOLD_DEFAULT;
-    }
-    ofproto_set_flow_eviction_threshold(br->ofproto, threshold);
-}
-
 /* Set forward BPDU option. */
 static void
 bridge_configure_forward_bpdu(struct bridge *br)

diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
@@ -122,6 +122,20 @@
           functions use the above config option during hot upgrades.
         </p>
       </column>
+
+      <column name="other_config" key="flow-eviction-threshold"
+              type='{"type": "integer", "minInteger": 0}'>
+        <p>
+          A number of flows as a nonnegative integer.  This sets number of
+          flows at which eviction from the datapath flow table will be
+          triggered.  If there are a large number of flows then increasing this
+          value to around the number of flows present can result in reduced CPU
+          usage and packet loss.
+        </p>
+        <p>
+          The default is 1000.  Values below 100 will be rounded up to 100.
+        </p>
+      </column>
     </group>
 
     <group title="Status">
@@ -598,20 +612,6 @@
         datapath ID.
       </column>
 
-      <column name="other_config" key="flow-eviction-threshold"
-              type='{"type": "integer", "minInteger": 0}'>
-        <p>
-          A number of flows as a nonnegative integer.  This sets number of
-          flows at which eviction from the kernel flow table will be triggered.
-          If there are a large number of flows then increasing this value to
-          around the number of flows present can result in reduced CPU usage
-          and packet loss.
-        </p>
-        <p>
-          The default is 1000.  Values below 100 will be rounded up to 100.
-        </p>
-      </column>
-
       <column name="other_config" key="forward-bpdu"
               type='{"type": "boolean"}'>
         Option to allow forwarding of BPDU frames when NORMAL action is