There's no particular reason for the function controlling the number
of threads to be bound up with dpif_recv_set(). This patch breaks
them up, but as a side effect means threads will run doing nothing
when datapath upcall receiving is disabled.
Signed-off-by: Ethan Jackson <et...@nicira.com>
---
ofproto/ofproto-dpif-upcall.c | 13 +++++--------
ofproto/ofproto-dpif-upcall.h | 2 +-
ofproto/ofproto-dpif.c | 21 ++++++---------------
ofproto/ofproto-provider.h | 2 +-
ofproto/ofproto.c | 13 +++++--------
ofproto/ofproto.h | 2 +-
vswitchd/bridge.c | 2 +-
7 files changed, 20 insertions(+), 35 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index a7bf38d..285b092 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -145,7 +145,7 @@ udpif_destroy(struct udpif *udpif)
struct flow_miss_batch *fmb;
struct drop_key *drop_key;
- udpif_recv_set(udpif, 0, false);
+ udpif_set_threads(udpif, 0);
while ((drop_key = drop_key_next(udpif))) {
drop_key_destroy(drop_key);
@@ -162,15 +162,12 @@ udpif_destroy(struct udpif *udpif)
free(udpif);
}
-/* Tells 'udpif' to begin or stop handling flow misses depending on the value
- * of 'enable'. 'n_handlers' is the number of upcall_handler threads to
- * create. Passing 'n_handlers' as zero is equivalent to passing 'enable' as
- * false. */
+/* Tells 'udpif' how many threads it should use to handle upcalls. Disables
+ * all threads if 'n_handlers' is zero. 'udpif''s datapath handle must have
+ * packet reception enabled before starting threads. */
void
-udpif_recv_set(struct udpif *udpif, size_t n_handlers, bool enable)
+udpif_set_threads(struct udpif *udpif, size_t n_handlers)
{
- n_handlers = enable ? n_handlers : 0;
-
/* Stop the old threads (if any). */
if (udpif->handlers && udpif->n_handlers != n_handlers) {
size_t i;
diff --git a/ofproto/ofproto-dpif-upcall.h b/ofproto/ofproto-dpif-upcall.h
index da75719..3ff3e4b 100644
--- a/ofproto/ofproto-dpif-upcall.h
+++ b/ofproto/ofproto-dpif-upcall.h
@@ -33,7 +33,7 @@ struct dpif_backer;
* module. */
struct udpif *udpif_create(struct dpif_backer *, struct dpif *);
-void udpif_recv_set(struct udpif *, size_t n_workers, bool enable);
+void udpif_set_threads(struct udpif *, size_t n_handlers);
void udpif_destroy(struct udpif *);
void udpif_wait(struct udpif *);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index e186b41..a8eb0cb 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -454,9 +454,6 @@ struct dpif_backer {
unsigned max_n_subfacet; /* Maximum number of flows */
unsigned avg_n_subfacet; /* Average number of flows. */
long long int avg_subfacet_life; /* Average life span of subfacets. */
-
- /* Number of upcall handling threads. */
- unsigned int n_handler_threads;
};
/* All existing ofproto_backer instances, indexed by ofproto->up.type. */
@@ -693,22 +690,15 @@ type_run(const char *type)
error = dpif_recv_set(backer->dpif, backer->recv_set_enable);
if (error) {
- udpif_recv_set(backer->udpif, 0, false);
VLOG_ERR("Failed to enable receiving packets in dpif.");
return error;
}
- udpif_recv_set(backer->udpif, n_handler_threads,
- backer->recv_set_enable);
dpif_flow_flush(backer->dpif);
backer->need_revalidate = REV_RECONFIGURE;
}
- /* If the n_handler_threads is reconfigured, call udpif_recv_set()
- * to reset the handler threads. */
- if (backer->n_handler_threads != n_handler_threads) {
- udpif_recv_set(backer->udpif, n_handler_threads,
- backer->recv_set_enable);
- backer->n_handler_threads = n_handler_threads;
+ if (backer->recv_set_enable) {
+ udpif_set_threads(backer->udpif, n_handlers);
}
if (backer->need_revalidate) {
@@ -1214,9 +1204,10 @@ open_dpif_backer(const char *type, struct dpif_backer
**backerp)
close_dpif_backer(backer);
return error;
}
- udpif_recv_set(backer->udpif, n_handler_threads,
- backer->recv_set_enable);
- backer->n_handler_threads = n_handler_threads;
+
+ if (backer->recv_set_enable) {
+ udpif_set_threads(backer->udpif, n_handlers);
+ }
backer->max_n_subfacet = 0;
backer->avg_n_subfacet = 0;
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 2844e4c..8b5c5bf 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -456,7 +456,7 @@ extern unsigned flow_eviction_threshold;
/* Number of upcall handler threads. Only affects the ofproto-dpif
* implementation. */
-extern unsigned n_handler_threads;
+extern size_t n_handlers;
/* Determines which model to use for handling misses in the ofproto-dpif
* implementation */
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index c4ce8a2..1d87def 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -305,9 +305,10 @@ static size_t allocated_ofproto_classes;
struct ovs_mutex ofproto_mutex = OVS_MUTEX_INITIALIZER;
unsigned flow_eviction_threshold = OFPROTO_FLOW_EVICTION_THRESHOLD_DEFAULT;
-unsigned n_handler_threads;
enum ofproto_flow_miss_model flow_miss_model = OFPROTO_HANDLE_MISS_AUTO;
+size_t n_handlers;
+
/* Map from datapath name to struct ofproto, for use by unixctl commands. */
static struct hmap all_ofprotos = HMAP_INITIALIZER(&all_ofprotos);
@@ -733,14 +734,10 @@ ofproto_set_mac_table_config(struct ofproto *ofproto,
unsigned idle_time,
/* Sets number of upcall handler threads. The default is
* (number of online cores - 2). */
void
-ofproto_set_n_handler_threads(unsigned limit)
+ofproto_set_threads(size_t n_handlers_)
{
- if (limit) {
- n_handler_threads = limit;
- } else {
- int n_proc = sysconf(_SC_NPROCESSORS_ONLN);
- n_handler_threads = n_proc > 2 ? n_proc - 2 : 1;
- }
+ int threads = MAX(sysconf(_SC_NPROCESSORS_ONLN) - 2, 1);
+ n_handlers = n_handlers_ ? n_handlers_ : threads;
}
void
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 903d1f4..50185ff 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -251,7 +251,7 @@ 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);
-void ofproto_set_n_handler_threads(unsigned limit);
+void ofproto_set_threads(size_t n_handlers);
void ofproto_set_dp_desc(struct ofproto *, const char *dp_desc);
int ofproto_set_snoops(struct ofproto *, const struct sset *snoops);
int ofproto_set_netflow(struct ofproto *,
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index c2307be..f22aaf8 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -502,7 +502,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch
*ovs_cfg)
smap_get_int(&ovs_cfg->other_config, "flow-eviction-threshold",
OFPROTO_FLOW_EVICTION_THRESHOLD_DEFAULT));
- ofproto_set_n_handler_threads(
+ ofproto_set_threads(
smap_get_int(&ovs_cfg->other_config, "n-handler-threads", 0));
bridge_configure_flow_miss_model(smap_get(&ovs_cfg->other_config,
--
1.8.1.2
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
