From 380f49c4a1ffaf5efa136985cff92de52bc8b582 Mon Sep 17 00:00:00 2001 From: Ethan Jackson Date: Tue, 4 Jun 2013 13:22:46 -0700 Subject: [PATCH] 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 --- NEWS | 1 + ofproto/ofproto-dpif.c | 10 +++++----- ofproto/ofproto-provider.h | 7 ++++--- ofproto/ofproto.c | 13 +++++-------- ofproto/ofproto.h | 2 +- vswitchd/bridge.c | 30 +++++++++++------------------- vswitchd/vswitch.xml | 28 ++++++++++++++-------------- 7 files changed, 41 insertions(+), 50 deletions(-) diff --git a/NEWS b/NEWS index 790824d81..478f1fafa 100644 --- 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 index 85826dcee..ca11ed2c2 100644 --- 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 index db0d58912..23d9180c9 100644 --- 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 index 3788c2f60..c768d7eb6 100644 --- 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 index 18241e767..5f8244c5a 100644 --- 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 index f5c420267..35138105e 100644 --- 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 @@ done: 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 index a17eae768..c6750d60f 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -122,6 +122,20 @@ functions use the above config option during hot upgrades.

+ + +

+ 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. +

+

+ The default is 1000. Values below 100 will be rounded up to 100. +

+ @@ -598,20 +612,6 @@ datapath ID. - -

- 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. -

-

- The default is 1000. Values below 100 will be rounded up to 100. -

- - Option to allow forwarding of BPDU frames when NORMAL action is -- 2.43.0