Previous patch adds more work being executed from the ovs-rcu thread.
Executing big chunks of work from ovs-rcu thread should be avoided,
however, as the rcu callback facility is shared resource used by all
threads.
This patch generalizes the exising "rule_executes" list to be useful
for also other kinds of work items. First new use is for sending flow
removed messages from the main thread instead of the ovs-rcu thread.
Even more work could be executed outside of the ovs-rcu thread if the
size of the 'deferred_work' list was made unlimited.
Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
---
ofproto/ofproto-provider.h | 10 +--
ofproto/ofproto.c | 195 +++++++++++++++++++++++++++++---------------
2 files changed, 134 insertions(+), 71 deletions(-)
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 0d25f68..4f08888 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -113,13 +113,15 @@ struct ofproto {
/* OpenFlow connections. */
struct connmgr *connmgr;
- /* Delayed rule executions.
+ /* Delayed work.
*
* We delay calls to ->ofproto_class->rule_execute() past releasing
* ofproto_mutex during a flow_mod, because otherwise a "learn" action
* triggered by the executing the packet would try to recursively modify
- * the flow table and reacquire the global lock. */
- struct guarded_list rule_executes; /* Contains "struct rule_execute"s. */
+ * the flow table and reacquire the global lock.
+ */
+ struct guarded_list deferred_work; /* Contains
+ * "struct deferred_work_item"s. */
/* Linux VLAN device support (e.g. "eth0.10" for VLAN 10.)
*
@@ -433,8 +435,6 @@ struct rule_actions {
};
BUILD_ASSERT_DECL(offsetof(struct rule_actions, ofpacts) % OFPACT_ALIGNTO ==
0);
-const struct rule_actions *rule_actions_create(const struct ofpact *, size_t);
-void rule_actions_destroy(const struct rule_actions *);
bool ofproto_rule_has_out_port(const struct rule *, ofp_port_t port)
OVS_REQUIRES(ofproto_mutex);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index cf5fa34..7a807bf 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -160,20 +160,56 @@ static void rule_criteria_destroy(struct rule_criteria *);
static enum ofperr collect_rules_loose(struct ofproto *,
const struct rule_criteria *,
struct rule_collection *);
+
+/* Ofproto deferred work queue. */
+
+enum deferred_work_type {
+ DEFERRED_RULE_EXECUTE,
+ DEFERRED_SEND_REMOVED
+};
/* A packet that needs to be passed to rule_execute().
*
* (We can't do this immediately from ofopgroup_complete() because that holds
* ofproto_mutex, which rule_execute() needs released.) */
struct rule_execute {
- struct ovs_list list_node; /* In struct ofproto's "rule_executes" list. */
struct rule *rule; /* Owns a reference to the rule. */
ofp_port_t in_port;
struct dp_packet *packet; /* Owns the packet. */
};
-static void run_rule_executes(struct ofproto *) OVS_EXCLUDED(ofproto_mutex);
-static void destroy_rule_executes(struct ofproto *);
+struct deferred_work_item {
+ struct ovs_list list_node; /* In struct ofproto's "deferred_work" list. */
+ enum deferred_work_type type;
+ union {
+ struct rule_execute re;
+ struct ofputil_flow_removed fr;
+ };
+};
+
+/* The sizes of the deferred work items vary a lot, so we use this to calculate
+ * the exact size needed. */
+static inline size_t
+deferred_work_size(enum deferred_work_type type OVS_UNUSED)
+{
+ return offsetof(struct deferred_work_item, re)
+ + (type == DEFERRED_RULE_EXECUTE ? sizeof(struct rule_execute)
+ : type == DEFERRED_SEND_REMOVED ? sizeof(struct
ofputil_flow_removed)
+ : 0);
+}
+
+static struct deferred_work_item *
+deferred_work_alloc(enum deferred_work_type type)
+{
+ struct deferred_work_item *dwi = xmalloc(deferred_work_size(type));
+
+ dwi->type = type;
+
+ return dwi;
+}
+
+static void run_deferred_work(struct ofproto *) OVS_EXCLUDED(ofproto_mutex);
+static void destroy_deferred_work(struct ofproto *);
struct learned_cookie {
union {
@@ -232,7 +268,7 @@ struct ofport_usage {
};
/* rule. */
-static void ofproto_rule_send_removed(struct rule *)
+static void ofproto_schedule_rule_send_removed(struct rule *)
OVS_EXCLUDED(ofproto_mutex);
static bool rule_is_readonly(const struct rule *);
static void ofproto_rule_insert__(struct ofproto *, struct rule *)
@@ -344,6 +380,9 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1,
5);
/* The default value of true waits for flow restore. */
static bool flow_restore_wait = true;
+static const struct rule_actions *
+rule_actions_create(const struct ofpact *ofpacts, size_t ofpacts_len);
+
/* Must be called to initialize the ofproto library.
*
* The caller may pass in 'iface_hints', which contains an shash of
@@ -552,7 +591,7 @@ ofproto_create(const char *datapath_name, const char
*datapath_type,
hmap_init(&ofproto->learned_cookies);
list_init(&ofproto->expirable);
ofproto->connmgr = connmgr_create(ofproto, datapath_name, datapath_name);
- guarded_list_init(&ofproto->rule_executes);
+ guarded_list_init(&ofproto->deferred_work);
ofproto->vlan_bitmap = NULL;
ofproto->vlans_changed = false;
ofproto->min_mtu = INT_MAX;
@@ -1529,10 +1568,10 @@ ofproto_destroy__(struct ofproto *ofproto)
{
struct oftable *table;
- destroy_rule_executes(ofproto);
+ destroy_deferred_work(ofproto);
delete_group(ofproto, OFPG_ALL);
- guarded_list_destroy(&ofproto->rule_executes);
+ guarded_list_destroy(&ofproto->deferred_work);
ovs_rwlock_destroy(&ofproto->groups_rwlock);
hmap_destroy(&ofproto->groups);
@@ -1681,7 +1720,7 @@ ofproto_run(struct ofproto *p)
VLOG_ERR_RL(&rl, "%s: run failed (%s)", p->name, ovs_strerror(error));
}
- run_rule_executes(p);
+ run_deferred_work(p);
/* Restore the eviction group heap invariant occasionally. */
if (p->eviction_group_timer < time_msec()) {
@@ -2752,7 +2791,7 @@ ofproto_rule_destroy__(struct rule *rule)
OVS_NO_THREAD_SAFETY_ANALYSIS
{
cls_rule_destroy(CONST_CAST(struct cls_rule *, &rule->cr));
- rule_actions_destroy(rule_get_actions(rule));
+ free(CONST_CAST(struct rule_actions *, rule->actions));
ovs_mutex_destroy(&rule->mutex);
rule->ofproto->ofproto_class->rule_dealloc(rule);
}
@@ -2765,7 +2804,7 @@ rule_destroy_cb(struct rule *rule)
if (rule->flags & OFPUTIL_FF_SEND_FLOW_REM
&& rule->removed_reason != OVS_OFPRR_NONE
&& !rule_is_hidden(rule)) {
- ofproto_rule_send_removed(rule);
+ ofproto_schedule_rule_send_removed(rule);
}
rule->ofproto->ofproto_class->rule_destruct(rule);
ofproto_rule_destroy__(rule);
@@ -2889,7 +2928,7 @@ static uint32_t get_provider_meter_id(const struct
ofproto *,
/* Creates and returns a new 'struct rule_actions', whose actions are a copy
* of from the 'ofpacts_len' bytes of 'ofpacts'. */
-const struct rule_actions *
+static const struct rule_actions *
rule_actions_create(const struct ofpact *ofpacts, size_t ofpacts_len)
{
struct rule_actions *actions;
@@ -2905,15 +2944,6 @@ rule_actions_create(const struct ofpact *ofpacts, size_t
ofpacts_len)
return actions;
}
-/* Free the actions after the RCU quiescent period is reached. */
-void
-rule_actions_destroy(const struct rule_actions *actions)
-{
- if (actions) {
- ovsrcu_postpone(free, CONST_CAST(struct rule_actions *, actions));
- }
-}
-
/* Returns true if 'rule' has an OpenFlow OFPAT_OUTPUT or OFPAT_ENQUEUE action
* that outputs to 'port' (output to OFPP_FLOOD and OFPP_ALL doesn't count). */
bool
@@ -2944,46 +2974,71 @@ ofproto_rule_has_out_group(const struct rule *rule,
uint32_t group_id)
}
static void
-rule_execute_destroy(struct rule_execute *e)
+deferred_work_destroy(struct deferred_work_item *dwi)
{
- ofproto_rule_unref(e->rule);
- list_remove(&e->list_node);
- free(e);
+ switch (dwi->type) {
+ case DEFERRED_RULE_EXECUTE:
+ ofproto_rule_unref(dwi->re.rule);
+ break;
+
+ case DEFERRED_SEND_REMOVED:
+ break;
+ }
+ list_remove(&dwi->list_node);
+ free(dwi);
}
-/* Executes all "rule_execute" operations queued up in ofproto->rule_executes,
+/* Executes all "rule_execute" operations queued up in ofproto->deferred_work,
* by passing them to the ofproto provider. */
static void
-run_rule_executes(struct ofproto *ofproto)
+run_deferred_work(struct ofproto *ofproto)
OVS_EXCLUDED(ofproto_mutex)
{
- struct rule_execute *e, *next;
- struct ovs_list executes;
+ struct deferred_work_item *e, *next;
+ struct ovs_list deferred_work;
- guarded_list_pop_all(&ofproto->rule_executes, &executes);
- LIST_FOR_EACH_SAFE (e, next, list_node, &executes) {
- struct flow flow;
+ guarded_list_pop_all(&ofproto->deferred_work, &deferred_work);
+ LIST_FOR_EACH_SAFE (e, next, list_node, &deferred_work) {
+ switch (e->type) {
+ case DEFERRED_RULE_EXECUTE: {
+ struct flow flow;
- flow_extract(e->packet, &flow);
- flow.in_port.ofp_port = e->in_port;
- ofproto->ofproto_class->rule_execute(e->rule, &flow, e->packet);
+ flow_extract(e->re.packet, &flow);
+ flow.in_port.ofp_port = e->re.in_port;
+ ofproto->ofproto_class->rule_execute(e->re.rule, &flow,
+ e->re.packet);
+ break;
+ }
+ case DEFERRED_SEND_REMOVED:
+ /* XXX: Not sure if the mutex is needed here or not. */
+ ovs_mutex_lock(&ofproto_mutex);
+ connmgr_send_flow_removed(ofproto->connmgr, &e->fr);
+ ovs_mutex_unlock(&ofproto_mutex);
+ break;
+ }
- rule_execute_destroy(e);
+ deferred_work_destroy(e);
}
}
/* Destroys and discards all "rule_execute" operations queued up in
- * ofproto->rule_executes. */
+ * ofproto->deferred_work. */
static void
-destroy_rule_executes(struct ofproto *ofproto)
+destroy_deferred_work(struct ofproto *ofproto)
{
- struct rule_execute *e, *next;
- struct ovs_list executes;
+ struct deferred_work_item *e, *next;
+ struct ovs_list deferred_work;
- guarded_list_pop_all(&ofproto->rule_executes, &executes);
- LIST_FOR_EACH_SAFE (e, next, list_node, &executes) {
- dp_packet_delete(e->packet);
- rule_execute_destroy(e);
+ guarded_list_pop_all(&ofproto->deferred_work, &deferred_work);
+ LIST_FOR_EACH_SAFE (e, next, list_node, &deferred_work) {
+ switch (e->type) {
+ case DEFERRED_RULE_EXECUTE:
+ dp_packet_delete(e->re.packet);
+ break;
+ case DEFERRED_SEND_REMOVED:
+ break;
+ }
+ deferred_work_destroy(e);
}
}
@@ -5083,29 +5138,35 @@ delete_flow_start_strict(struct ofproto *ofproto,
/* This may only be called by rule_destroy_cb()! */
static void
-ofproto_rule_send_removed(struct rule *rule)
+ofproto_schedule_rule_send_removed(struct rule *rule)
OVS_EXCLUDED(ofproto_mutex)
{
- struct ofputil_flow_removed fr;
+ struct deferred_work_item *dwi;
+ struct ofputil_flow_removed *fr;
long long int used;
- minimatch_expand(&rule->cr.match, &fr.match);
- fr.priority = rule->cr.priority;
+ dwi = deferred_work_alloc(DEFERRED_SEND_REMOVED);
+ fr = &dwi->fr;
+
+ minimatch_expand(&rule->cr.match, &fr->match);
+ fr->priority = rule->cr.priority;
- ovs_mutex_lock(&ofproto_mutex);
- fr.cookie = rule->flow_cookie;
- fr.reason = rule->removed_reason;
- fr.table_id = rule->table_id;
- calc_duration(rule->created, time_msec(),
- &fr.duration_sec, &fr.duration_nsec);
ovs_mutex_lock(&rule->mutex);
- fr.idle_timeout = rule->idle_timeout;
- fr.hard_timeout = rule->hard_timeout;
+ fr->cookie = rule->flow_cookie;
+ fr->reason = rule->removed_reason;
+ fr->table_id = rule->table_id;
+ calc_duration(rule->created, time_msec(),
+ &fr->duration_sec, &fr->duration_nsec);
+ fr->idle_timeout = rule->idle_timeout;
+ fr->hard_timeout = rule->hard_timeout;
ovs_mutex_unlock(&rule->mutex);
- rule->ofproto->ofproto_class->rule_get_stats(rule, &fr.packet_count,
- &fr.byte_count, &used);
- connmgr_send_flow_removed(rule->ofproto->connmgr, &fr);
- ovs_mutex_unlock(&ofproto_mutex);
+
+ rule->ofproto->ofproto_class->rule_get_stats(rule, &fr->packet_count,
+ &fr->byte_count, &used);
+ if (!guarded_list_push_back(&rule->ofproto->deferred_work,
+ &dwi->list_node, 1024)) {
+ free(dwi);
+ }
}
/* Sends an OpenFlow "flow removed" message with the given 'reason' (either
@@ -5221,7 +5282,7 @@ handle_flow_mod__(struct ofproto *ofproto, struct
ofputil_flow_mod *fm,
ofmonitor_flush(ofproto->connmgr);
ovs_mutex_unlock(&ofproto_mutex);
- run_rule_executes(ofproto);
+ run_deferred_work(ofproto);
return error;
}
@@ -6741,7 +6802,7 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id,
uint16_t flags)
ofmonitor_flush(ofproto->connmgr);
ovs_mutex_unlock(&ofproto_mutex);
- run_rule_executes(ofproto);
+ run_deferred_work(ofproto);
}
/* The bundle is discarded regardless the outcome. */
@@ -7065,20 +7126,22 @@ send_buffered_packet(const struct flow_mod_requester
*req, uint32_t buffer_id,
error = ofconn_pktbuf_retrieve(req->ofconn, buffer_id, &packet,
&in_port);
if (packet) {
+ struct deferred_work_item *dwi;
struct rule_execute *re;
- ofproto_rule_ref(rule);
+ dwi = deferred_work_alloc(DEFERRED_RULE_EXECUTE);
+ re = &dwi->re;
- re = xmalloc(sizeof *re);
+ ofproto_rule_ref(rule);
re->rule = rule;
re->in_port = in_port;
re->packet = packet;
- if (!guarded_list_push_back(&ofproto->rule_executes,
- &re->list_node, 1024)) {
+ if (!guarded_list_push_back(&ofproto->deferred_work,
+ &dwi->list_node, 1024)) {
ofproto_rule_unref(rule);
dp_packet_delete(re->packet);
- free(re);
+ free(dwi);
}
} else {
ofconn_send_error(req->ofconn, req->request, error);
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
