Upon close inspection, it appears that it's not possible for
actions to differ between subfacets belonging to a given facet.
Given this fact, it makes sense to move datapath actions from
subfacets to their parent facets. It's both conceptually more
straightforward, and necessary for future threading and megaflow
work.
Justin Pettit fixed a serious learning bug.
Co-authored-by: Justin Pettit <jpet...@nicira.com>
Signed-off-by: Ethan Jackson <et...@nicira.com>
---
ofproto/ofproto-dpif.c | 449 ++++++++++++++++++------------------------------
1 file changed, 164 insertions(+), 285 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index c4f7d25..754f4f3 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -364,8 +364,6 @@ enum subfacet_path {
SF_SLOW_PATH, /* Send-to-userspace action is installed. */
};
-static const char *subfacet_path_to_string(enum subfacet_path);
-
/* A dpif flow and actions associated with a facet.
*
* See also the large comment on struct facet. */
@@ -385,19 +383,8 @@ struct subfacet {
uint64_t dp_packet_count; /* Last known packet count in the datapath. */
uint64_t dp_byte_count; /* Last known byte count in the datapath. */
- /* Datapath actions.
- *
- * These should be essentially identical for every subfacet in a facet, but
- * may differ in trivial ways due to VLAN splinters. */
- size_t actions_len; /* Number of bytes in actions[]. */
- struct nlattr *actions; /* Datapath actions. */
-
- enum slow_path_reason slow; /* 0 if fast path may be used. */
enum subfacet_path path; /* Installed in datapath? */
- /* Initial values of the packet that may be needed later. */
- struct initial_vals initial_vals;
-
/* Datapath port the packet arrived on. This is needed to remove
* flows for ports that are no longer part of the bridge. Since the
* flow definition only has the OpenFlow port number and the port is
@@ -422,15 +409,13 @@ static void subfacet_reset_dp_stats(struct subfacet *,
static void subfacet_update_time(struct subfacet *, long long int used);
static void subfacet_update_stats(struct subfacet *,
const struct dpif_flow_stats *);
-static void subfacet_make_actions(struct subfacet *,
- const struct ofpbuf *packet,
- struct ofpbuf *odp_actions);
static int subfacet_install(struct subfacet *,
- const struct nlattr *actions, size_t actions_len,
- struct dpif_flow_stats *, enum slow_path_reason);
+ const struct ofpbuf *odp_actions,
+ struct dpif_flow_stats *);
static void subfacet_uninstall(struct subfacet *);
-static enum subfacet_path subfacet_want_path(enum slow_path_reason);
+static enum slow_path_reason subfacet_slow_reason(const struct subfacet *);
+static enum subfacet_path subfacet_want_path(const struct subfacet *subfacet);
/* An exact-match instantiation of an OpenFlow flow.
*
@@ -484,18 +469,21 @@ struct facet {
struct netflow_flow nf_flow; /* Per-flow NetFlow tracking data. */
uint8_t tcp_flags; /* TCP flags seen for this 'rule'. */
- /* Properties of datapath actions.
- *
- * Every subfacet has its own actions because actions can differ slightly
- * between splintered and non-splintered subfacets due to the VLAN tag
- * being initially different (present vs. absent). All of them have these
- * properties in common so we just store one copy of them here. */
bool has_learn; /* Actions include NXAST_LEARN? */
bool has_normal; /* Actions output to OFPP_NORMAL? */
bool has_fin_timeout; /* Actions include NXAST_FIN_TIMEOUT? */
tag_type tags; /* Tags that would require revalidation. */
mirror_mask_t mirrors; /* Bitmap of dependent mirrors. */
+ /* Datapath actions. */
+ struct ofpbuf odp_actions;
+ uint64_t odp_actions_stub[1024 / 8];
+
+ /* Initial values of the packet that may be needed later. */
+ struct initial_vals initial_vals;
+
+ enum slow_path_reason slow; /* 0 if fast path may be used. */
+
/* Storage for a single subfacet, to reduce malloc() time and space
* overhead. (A facet always has at least one subfacet and in the common
* case has exactly one subfacet. However, 'one_subfacet' may not
@@ -506,8 +494,7 @@ struct facet {
long long int learn_rl; /* Rate limiter for facet_learn(). */
};
-static struct facet *facet_create(struct rule_dpif *,
- const struct flow *, uint32_t hash);
+static struct facet *facet_create(const struct flow_miss *, uint32_t hash);
static void facet_remove(struct facet *);
static void facet_free(struct facet *);
@@ -527,8 +514,6 @@ static void facet_learn(struct facet *);
static void facet_account(struct facet *);
static void push_all_stats(void);
-static struct subfacet *facet_get_subfacet(struct facet *);
-
static bool facet_is_controller_flow(struct facet *);
struct ofport_dpif {
@@ -3727,43 +3712,37 @@ handle_flow_miss_with_facet(struct flow_miss *miss,
struct facet *facet,
struct ofpbuf *packet;
subfacet = subfacet_create(facet, miss, now);
+ want_path = subfacet_want_path(subfacet);
LIST_FOR_EACH (packet, list_node, &miss->packets) {
struct flow_miss_op *op = &ops[*n_ops];
struct dpif_flow_stats stats;
- struct ofpbuf odp_actions;
handle_flow_miss_common(facet->rule, packet, &miss->flow);
- ofpbuf_use_stub(&odp_actions, op->stub, sizeof op->stub);
- if (!subfacet->actions || subfacet->slow) {
- subfacet_make_actions(subfacet, packet, &odp_actions);
+ if (want_path != SF_FAST_PATH) {
+ struct action_xlate_ctx ctx;
+
+ action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
+ &facet->initial_vals, facet->rule, 0,
+ packet);
+ xlate_actions_for_side_effects(&ctx, facet->rule->up.ofpacts,
+ facet->rule->up.ofpacts_len);
}
dpif_flow_stats_extract(&facet->flow, packet, now, &stats);
subfacet_update_stats(subfacet, &stats);
- if (subfacet->actions_len) {
+ if (facet->odp_actions.size) {
struct dpif_execute *execute = &op->dpif_op.u.execute;
init_flow_miss_execute_op(miss, packet, op);
- if (!subfacet->slow) {
- execute->actions = subfacet->actions;
- execute->actions_len = subfacet->actions_len;
- ofpbuf_uninit(&odp_actions);
- } else {
- execute->actions = odp_actions.data;
- execute->actions_len = odp_actions.size;
- op->garbage = ofpbuf_get_uninit_pointer(&odp_actions);
- }
-
+ execute->actions = facet->odp_actions.data,
+ execute->actions_len = facet->odp_actions.size;
(*n_ops)++;
- } else {
- ofpbuf_uninit(&odp_actions);
}
}
- want_path = subfacet_want_path(subfacet->slow);
if (miss->upcall_type == DPIF_UC_MISS || subfacet->path != want_path) {
struct flow_miss_op *op = &ops[(*n_ops)++];
struct dpif_flow_put *put = &op->dpif_op.u.flow_put;
@@ -3776,10 +3755,11 @@ handle_flow_miss_with_facet(struct flow_miss *miss,
struct facet *facet,
put->key = miss->key;
put->key_len = miss->key_len;
if (want_path == SF_FAST_PATH) {
- put->actions = subfacet->actions;
- put->actions_len = subfacet->actions_len;
+ put->actions = facet->odp_actions.data;
+ put->actions_len = facet->odp_actions.size;
} else {
- compose_slow_path(ofproto, &facet->flow, subfacet->slow,
+ compose_slow_path(ofproto, &facet->flow,
+ subfacet_slow_reason(subfacet),
op->stub, sizeof op->stub,
&put->actions, &put->actions_len);
}
@@ -3811,7 +3791,7 @@ handle_flow_miss(struct flow_miss *miss, struct
flow_miss_op *ops,
return;
}
- facet = facet_create(rule, &miss->flow, hash);
+ facet = facet_create(miss, hash);
now = facet->used;
} else {
now = time_msec();
@@ -4557,7 +4537,8 @@ expire_subfacets(struct ofproto_dpif *ofproto, int
dp_max_idle)
&ofproto->subfacets) {
long long int cutoff;
- cutoff = (subfacet->slow & (SLOW_CFM | SLOW_BFD | SLOW_LACP | SLOW_STP)
+ cutoff = (subfacet->facet->slow & (SLOW_CFM | SLOW_BFD | SLOW_LACP
+ | SLOW_STP)
? special_cutoff
: normal_cutoff);
if (subfacet->used < cutoff) {
@@ -4618,33 +4599,49 @@ rule_expire(struct rule_dpif *rule)
/* Facets. */
-/* Creates and returns a new facet owned by 'rule', given a 'flow'.
+/* Creates and returns a new facet based on 'miss'.
*
* The caller must already have determined that no facet with an identical
- * 'flow' exists in 'ofproto' and that 'flow' is the best match for 'rule' in
- * the ofproto's classifier table.
+ * 'miss->flow' exists in 'miss->ofproto'.
*
- * 'hash' must be the return value of flow_hash(flow, 0).
+ * 'hash' must be the return value of flow_hash(miss->flow, 0).
*
* The facet will initially have no subfacets. The caller should create (at
* least) one subfacet with subfacet_create(). */
static struct facet *
-facet_create(struct rule_dpif *rule, const struct flow *flow, uint32_t hash)
+facet_create(const struct flow_miss *miss, uint32_t hash)
{
- struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
+ struct ofproto_dpif *ofproto = miss->ofproto;
+ struct action_xlate_ctx ctx;
struct facet *facet;
facet = xzalloc(sizeof *facet);
facet->used = time_msec();
+ facet->flow = miss->flow;
+ facet->initial_vals = miss->initial_vals;
+ facet->rule = rule_dpif_lookup(ofproto, &facet->flow);
+ facet->learn_rl = time_msec() + 500;
+
hmap_insert(&ofproto->facets, &facet->hmap_node, hash);
- list_push_back(&rule->facets, &facet->list_node);
- facet->rule = rule;
- facet->flow = *flow;
+ list_push_back(&facet->rule->facets, &facet->list_node);
list_init(&facet->subfacets);
netflow_flow_init(&facet->nf_flow);
netflow_flow_update_time(ofproto->netflow, &facet->nf_flow, facet->used);
- facet->learn_rl = time_msec() + 500;
+ ofpbuf_use_stub(&facet->odp_actions, &facet->odp_actions_stub,
+ sizeof facet->odp_actions_stub);
+ action_xlate_ctx_init(&ctx, ofproto, &facet->flow, &facet->initial_vals,
+ facet->rule, 0, NULL);
+ ctx.may_learn = true;
+ xlate_actions(&ctx, facet->rule->up.ofpacts, facet->rule->up.ofpacts_len,
+ &facet->odp_actions);
+ facet->tags = ctx.tags;
+ facet->has_learn = ctx.has_learn;
+ facet->has_normal = ctx.has_normal;
+ facet->has_fin_timeout = ctx.has_fin_timeout;
+ facet->nf_flow.output_iface = ctx.nf_output_iface;
+ facet->mirrors = ctx.mirrors;
+ facet->slow = ctx.slow;
return facet;
}
@@ -4652,7 +4649,10 @@ facet_create(struct rule_dpif *rule, const struct flow
*flow, uint32_t hash)
static void
facet_free(struct facet *facet)
{
- free(facet);
+ if (facet) {
+ ofpbuf_uninit(&facet->odp_actions);
+ free(facet);
+ }
}
/* Executes, within 'ofproto', the 'n_actions' actions in 'actions' on
@@ -4718,8 +4718,6 @@ static void
facet_learn(struct facet *facet)
{
struct ofproto_dpif *ofproto = ofproto_dpif_cast(facet->rule->up.ofproto);
- struct subfacet *subfacet= CONTAINER_OF(list_front(&facet->subfacets),
- struct subfacet, list_node);
long long int now = time_msec();
struct action_xlate_ctx ctx;
@@ -4736,8 +4734,7 @@ facet_learn(struct facet *facet)
return;
}
- action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
- &subfacet->initial_vals,
+ action_xlate_ctx_init(&ctx, ofproto, &facet->flow, &facet->initial_vals,
facet->rule, facet->tcp_flags, NULL);
ctx.may_learn = true;
xlate_actions_for_side_effects(&ctx, facet->rule->up.ofpacts,
@@ -4748,7 +4745,6 @@ static void
facet_account(struct facet *facet)
{
struct ofproto_dpif *ofproto = ofproto_dpif_cast(facet->rule->up.ofproto);
- struct subfacet *subfacet = facet_get_subfacet(facet);
const struct nlattr *a;
unsigned int left;
ovs_be16 vlan_tci;
@@ -4768,8 +4764,8 @@ facet_account(struct facet *facet)
* We use the actions from an arbitrary subfacet because they should all
* be equally valid for our purpose. */
vlan_tci = facet->flow.vlan_tci;
- NL_ATTR_FOR_EACH_UNSAFE (a, left,
- subfacet->actions, subfacet->actions_len) {
+ NL_ATTR_FOR_EACH_UNSAFE (a, left, facet->odp_actions.data,
+ facet->odp_actions.size) {
const struct ovs_action_push_vlan *vlan;
struct ofport_dpif *port;
@@ -4900,50 +4896,21 @@ facet_lookup_valid(struct ofproto_dpif *ofproto, const
struct flow *flow,
return facet;
}
-/* Return a subfacet from 'facet'. A facet consists of one or more
- * subfacets, and this function returns one of them. */
-static struct subfacet *facet_get_subfacet(struct facet *facet)
+/* Returns the slow_path_reason of 'subfacet', or 0 if 'subfacet' may be
+ * installed in the fast path. */
+static enum slow_path_reason
+subfacet_slow_reason(const struct subfacet *subfacet)
{
- return CONTAINER_OF(list_front(&facet->subfacets), struct subfacet,
- list_node);
-}
-
-static const char *
-subfacet_path_to_string(enum subfacet_path path)
-{
- switch (path) {
- case SF_NOT_INSTALLED:
- return "not installed";
- case SF_FAST_PATH:
- return "in fast path";
- case SF_SLOW_PATH:
- return "in slow path";
- default:
- return "<error>";
- }
+ enum slow_path_reason reason;
+ reason = subfacet->key_fitness == ODP_FIT_TOO_LITTLE ? SLOW_MATCH : 0;
+ return subfacet->facet->slow | reason;
}
-/* Returns the path in which a subfacet should be installed if its 'slow'
- * member has the specified value. */
+/* Returns the path in which a subfacet should be installed. */
static enum subfacet_path
-subfacet_want_path(enum slow_path_reason slow)
+subfacet_want_path(const struct subfacet *subfacet)
{
- return slow ? SF_SLOW_PATH : SF_FAST_PATH;
-}
-
-/* Returns true if 'subfacet' needs to have its datapath flow updated,
- * supposing that its actions have been recalculated as 'want_actions' and that
- * 'slow' is nonzero iff 'subfacet' should be in the slow path. */
-static bool
-subfacet_should_install(struct subfacet *subfacet, enum slow_path_reason slow,
- const struct ofpbuf *want_actions)
-{
- enum subfacet_path want_path = subfacet_want_path(slow);
- return (want_path != subfacet->path
- || (want_path == SF_FAST_PATH
- && (subfacet->actions_len != want_actions->size
- || memcmp(subfacet->actions, want_actions->data,
- subfacet->actions_len))));
+ return subfacet_slow_reason(subfacet) ? SF_SLOW_PATH : SF_FAST_PATH;
}
static bool
@@ -4952,12 +4919,13 @@ facet_check_consistency(struct facet *facet)
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 15);
struct ofproto_dpif *ofproto = ofproto_dpif_cast(facet->rule->up.ofproto);
+ struct ds s = DS_EMPTY_INITIALIZER;
uint64_t odp_actions_stub[1024 / 8];
struct ofpbuf odp_actions;
+ struct action_xlate_ctx ctx;
struct rule_dpif *rule;
- struct subfacet *subfacet;
bool may_log = false;
bool ok;
@@ -4967,9 +4935,6 @@ facet_check_consistency(struct facet *facet)
if (!ok) {
may_log = !VLOG_DROP_WARN(&rl);
if (may_log) {
- struct ds s;
-
- ds_init(&s);
flow_format(&s, &facet->flow);
ds_put_format(&s, ": facet associated with wrong rule (was "
"table=%"PRIu8",", facet->rule->up.table_id);
@@ -4977,82 +4942,60 @@ facet_check_consistency(struct facet *facet)
ds_put_format(&s, ") (should have been table=%"PRIu8",",
rule->up.table_id);
cls_rule_format(&rule->up.cr, &s);
- ds_put_char(&s, ')');
-
- VLOG_WARN("%s", ds_cstr(&s));
- ds_destroy(&s);
+ ds_put_cstr(&s, ")\n");
}
}
/* Check the datapath actions for consistency. */
ofpbuf_use_stub(&odp_actions, odp_actions_stub, sizeof odp_actions_stub);
- LIST_FOR_EACH (subfacet, list_node, &facet->subfacets) {
- enum subfacet_path want_path;
- struct action_xlate_ctx ctx;
- struct ds s;
+ action_xlate_ctx_init(&ctx, ofproto, &facet->flow, &facet->initial_vals,
+ rule, 0, NULL);
+ xlate_actions(&ctx, rule->up.ofpacts, rule->up.ofpacts_len, &odp_actions);
- action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
- &subfacet->initial_vals, rule, 0, NULL);
- xlate_actions(&ctx, rule->up.ofpacts, rule->up.ofpacts_len,
- &odp_actions);
+ /* The actions for slow-path flows may legitimately vary from one
+ * packet to the next. We're done. */
+ if (facet->slow && facet->slow == ctx.slow) {
+ goto exit;
+ }
- if (subfacet->path == SF_NOT_INSTALLED) {
- /* This only happens if the datapath reported an error when we
- * tried to install the flow. Don't flag another error here. */
- continue;
- }
+ if (facet->slow == ctx.slow
+ && ofpbuf_equal(&facet->odp_actions, &odp_actions)) {
+ goto exit;
+ }
- want_path = subfacet_want_path(subfacet->slow);
- if (want_path == SF_SLOW_PATH && subfacet->path == SF_SLOW_PATH) {
- /* The actions for slow-path flows may legitimately vary from one
- * packet to the next. We're done. */
- continue;
- }
+ if (ok) {
+ may_log = !VLOG_DROP_WARN(&rl);
+ ok = false;
+ }
- if (!subfacet_should_install(subfacet, subfacet->slow, &odp_actions)) {
- continue;
- }
+ if (!may_log) {
+ goto exit;
+ }
- /* Inconsistency! */
- if (ok) {
- may_log = !VLOG_DROP_WARN(&rl);
- ok = false;
- }
- if (!may_log) {
- /* Rate-limited, skip reporting. */
- continue;
- }
+ flow_format(&s, &facet->flow);
+ ds_put_cstr(&s, ": inconsistency in facet");
- ds_init(&s);
- odp_flow_key_format(subfacet->key, subfacet->key_len, &s);
-
- ds_put_cstr(&s, ": inconsistency in subfacet");
- if (want_path != subfacet->path) {
- enum odp_key_fitness fitness = subfacet->key_fitness;
-
- ds_put_format(&s, " (%s, fitness=%s)",
- subfacet_path_to_string(subfacet->path),
- odp_key_fitness_to_string(fitness));
- ds_put_format(&s, " (should have been %s)",
- subfacet_path_to_string(want_path));
- } else if (want_path == SF_FAST_PATH) {
- ds_put_cstr(&s, " (actions were: ");
- format_odp_actions(&s, subfacet->actions,
- subfacet->actions_len);
- ds_put_cstr(&s, ") (correct actions: ");
- format_odp_actions(&s, odp_actions.data, odp_actions.size);
- ds_put_char(&s, ')');
- } else {
- ds_put_cstr(&s, " (actions: ");
- format_odp_actions(&s, subfacet->actions,
- subfacet->actions_len);
- ds_put_char(&s, ')');
- }
+ if (!ofpbuf_equal(&facet->odp_actions, &odp_actions)) {
+ ds_put_cstr(&s, " (actions were: ");
+ format_odp_actions(&s, facet->odp_actions.data,
+ facet->odp_actions.size);
+ ds_put_cstr(&s, ") (correct actions: ");
+ format_odp_actions(&s, odp_actions.data, odp_actions.size);
+ ds_put_cstr(&s, ")");
+ }
+
+ if (facet->slow != ctx.slow) {
+ ds_put_format(&s, " slow path incorrect. should be %s",
+ ctx.slow ? "true" : "false");
+ }
+
+exit:
+ if (may_log) {
+ ds_chomp(&s, '\n');
VLOG_WARN("%s", ds_cstr(&s));
- ds_destroy(&s);
}
ofpbuf_uninit(&odp_actions);
-
+ ds_destroy(&s);
return ok;
}
@@ -5065,24 +5008,20 @@ facet_check_consistency(struct facet *facet)
* where it is and recompiles its actions anyway.
*
* - If any of 'facet''s subfacets correspond to a new flow according to
- * ofproto_receive(), 'facet' is removed. */
+ * ofproto_receive(), 'facet' is removed.
+ *
+ * - If 'facet->slow' changed, 'facet' is removed. */
static void
facet_revalidate(struct facet *facet)
{
struct ofproto_dpif *ofproto = ofproto_dpif_cast(facet->rule->up.ofproto);
- struct actions {
- struct nlattr *odp_actions;
- size_t actions_len;
- };
- struct actions *new_actions;
- struct action_xlate_ctx ctx;
uint64_t odp_actions_stub[1024 / 8];
struct ofpbuf odp_actions;
+ struct action_xlate_ctx ctx;
struct rule_dpif *new_rule;
struct subfacet *subfacet;
- int i;
COVERAGE_INC(facet_revalidate);
@@ -5112,67 +5051,45 @@ facet_revalidate(struct facet *facet)
* We do not modify any 'facet' state yet, because we might need to, e.g.,
* emit a NetFlow expiration and, if so, we need to have the old state
* around to properly compose it. */
-
- /* If the datapath actions changed or the installability changed,
- * then we need to talk to the datapath. */
- i = 0;
- new_actions = NULL;
- memset(&ctx, 0, sizeof ctx);
ofpbuf_use_stub(&odp_actions, odp_actions_stub, sizeof odp_actions_stub);
- LIST_FOR_EACH (subfacet, list_node, &facet->subfacets) {
- enum slow_path_reason slow;
-
- action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
- &subfacet->initial_vals, new_rule, 0, NULL);
- xlate_actions(&ctx, new_rule->up.ofpacts, new_rule->up.ofpacts_len,
- &odp_actions);
-
- slow = (subfacet->slow & SLOW_MATCH) | ctx.slow;
- if (subfacet_should_install(subfacet, slow, &odp_actions)) {
- struct dpif_flow_stats stats;
+ action_xlate_ctx_init(&ctx, ofproto, &facet->flow, &facet->initial_vals,
+ new_rule, 0, NULL);
+ xlate_actions(&ctx, new_rule->up.ofpacts, new_rule->up.ofpacts_len,
+ &odp_actions);
+
+ /* A facet's slow path reason should only change under dramatic
+ * circumstances. Rather than try to update everything, it's simpler to
+ * remove the facet and start over. */
+ if (facet->slow != ctx.slow) {
+ facet_remove(facet);
+ goto exit;
+ }
- subfacet_install(subfacet,
- odp_actions.data, odp_actions.size, &stats, slow);
- subfacet_update_stats(subfacet, &stats);
+ if (!ofpbuf_equal(&facet->odp_actions, &odp_actions)) {
+ LIST_FOR_EACH(subfacet, list_node, &facet->subfacets) {
+ if (subfacet->path == SF_FAST_PATH) {
+ struct dpif_flow_stats stats;
- if (!new_actions) {
- new_actions = xcalloc(list_size(&facet->subfacets),
- sizeof *new_actions);
+ subfacet_install(subfacet, &odp_actions, &stats);
+ subfacet_update_stats(subfacet, &stats);
}
- new_actions[i].odp_actions = xmemdup(odp_actions.data,
- odp_actions.size);
- new_actions[i].actions_len = odp_actions.size;
}
- i++;
- }
- ofpbuf_uninit(&odp_actions);
-
- if (new_actions) {
facet_flush_stats(facet);
+
+ ofpbuf_clear(&facet->odp_actions);
+ ofpbuf_put(&facet->odp_actions, odp_actions.data, odp_actions.size);
}
/* Update 'facet' now that we've taken care of all the old state. */
facet->tags = ctx.tags;
+ facet->slow = ctx.slow;
facet->nf_flow.output_iface = ctx.nf_output_iface;
facet->has_learn = ctx.has_learn;
facet->has_normal = ctx.has_normal;
facet->has_fin_timeout = ctx.has_fin_timeout;
facet->mirrors = ctx.mirrors;
- i = 0;
- LIST_FOR_EACH (subfacet, list_node, &facet->subfacets) {
- subfacet->slow = (subfacet->slow & SLOW_MATCH) | ctx.slow;
-
- if (new_actions && new_actions[i].odp_actions) {
- free(subfacet->actions);
- subfacet->actions = new_actions[i].odp_actions;
- subfacet->actions_len = new_actions[i].actions_len;
- }
- i++;
- }
- free(new_actions);
-
if (facet->rule != new_rule) {
COVERAGE_INC(facet_changed_rule);
list_remove(&facet->list_node);
@@ -5181,6 +5098,9 @@ facet_revalidate(struct facet *facet)
facet->used = new_rule->up.created;
facet->prev_used = facet->used;
}
+
+exit:
+ ofpbuf_uninit(&odp_actions);
}
/* Updates 'facet''s used time. Caller is responsible for calling
@@ -5278,13 +5198,12 @@ flow_push_stats(struct facet *facet, const struct
dpif_flow_stats *stats)
{
struct rule_dpif *rule = facet->rule;
struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
- struct subfacet *subfacet = facet_get_subfacet(facet);
struct action_xlate_ctx ctx;
ofproto_rule_update_used(&rule->up, stats->used);
action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
- &subfacet->initial_vals, rule, 0, NULL);
+ &facet->initial_vals, rule, 0, NULL);
ctx.resubmit_stats = stats;
xlate_actions_for_side_effects(&ctx, rule->up.ofpacts,
rule->up.ofpacts_len);
@@ -5357,13 +5276,7 @@ subfacet_create(struct facet *facet, struct flow_miss
*miss,
subfacet->created = now;
subfacet->dp_packet_count = 0;
subfacet->dp_byte_count = 0;
- subfacet->actions_len = 0;
- subfacet->actions = NULL;
- subfacet->slow = (subfacet->key_fitness == ODP_FIT_TOO_LITTLE
- ? SLOW_MATCH
- : 0);
subfacet->path = SF_NOT_INSTALLED;
- subfacet->initial_vals = miss->initial_vals;
subfacet->odp_in_port = miss->odp_in_port;
ofproto->subfacet_add_count++;
@@ -5386,7 +5299,6 @@ subfacet_destroy__(struct subfacet *subfacet)
hmap_remove(&ofproto->subfacets, &subfacet->hmap_node);
list_remove(&subfacet->list_node);
free(subfacet->key);
- free(subfacet->actions);
if (subfacet != &facet->one_subfacet) {
free(subfacet);
}
@@ -5433,38 +5345,6 @@ subfacet_destroy_batch(struct ofproto_dpif *ofproto,
}
}
-/* Composes the datapath actions for 'subfacet' based on its rule's actions.
- * Translates the actions into 'odp_actions', which the caller must have
- * initialized and is responsible for uninitializing. */
-static void
-subfacet_make_actions(struct subfacet *subfacet, const struct ofpbuf *packet,
- struct ofpbuf *odp_actions)
-{
- struct facet *facet = subfacet->facet;
- struct rule_dpif *rule = facet->rule;
- struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
-
- struct action_xlate_ctx ctx;
-
- action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
- &subfacet->initial_vals, rule, 0, packet);
- xlate_actions(&ctx, rule->up.ofpacts, rule->up.ofpacts_len, odp_actions);
- facet->tags = ctx.tags;
- facet->has_learn = ctx.has_learn;
- facet->has_normal = ctx.has_normal;
- facet->has_fin_timeout = ctx.has_fin_timeout;
- facet->nf_flow.output_iface = ctx.nf_output_iface;
- facet->mirrors = ctx.mirrors;
-
- subfacet->slow = (subfacet->slow & SLOW_MATCH) | ctx.slow;
- if (subfacet->actions_len != odp_actions->size
- || memcmp(subfacet->actions, odp_actions->data, odp_actions->size)) {
- free(subfacet->actions);
- subfacet->actions_len = odp_actions->size;
- subfacet->actions = xmemdup(odp_actions->data, odp_actions->size);
- }
-}
-
/* Updates 'subfacet''s datapath flow, setting its actions to 'actions_len'
* bytes of actions in 'actions'. If 'stats' is non-null, statistics counters
* in the datapath will be zeroed and 'stats' will be updated with traffic new
@@ -5472,25 +5352,29 @@ subfacet_make_actions(struct subfacet *subfacet, const
struct ofpbuf *packet,
*
* Returns 0 if successful, otherwise a positive errno value. */
static int
-subfacet_install(struct subfacet *subfacet,
- const struct nlattr *actions, size_t actions_len,
- struct dpif_flow_stats *stats,
- enum slow_path_reason slow)
+subfacet_install(struct subfacet *subfacet, const struct ofpbuf *odp_actions,
+ struct dpif_flow_stats *stats)
{
struct facet *facet = subfacet->facet;
struct ofproto_dpif *ofproto = ofproto_dpif_cast(facet->rule->up.ofproto);
- enum subfacet_path path = subfacet_want_path(slow);
+ const struct nlattr *actions = odp_actions->data;
+ size_t actions_len = odp_actions->size;
+
uint64_t slow_path_stub[128 / 8];
enum dpif_flow_put_flags flags;
+ enum subfacet_path path;
int ret;
+ path = subfacet_want_path(subfacet);
+
flags = DPIF_FP_CREATE | DPIF_FP_MODIFY;
if (stats) {
flags |= DPIF_FP_ZERO_STATS;
}
if (path == SF_SLOW_PATH) {
- compose_slow_path(ofproto, &facet->flow, slow,
+ compose_slow_path(ofproto, &facet->flow,
+ subfacet_slow_reason(subfacet),
slow_path_stub, sizeof slow_path_stub,
&actions, &actions_len);
}
@@ -5508,13 +5392,6 @@ subfacet_install(struct subfacet *subfacet,
return ret;
}
-static int
-subfacet_reinstall(struct subfacet *subfacet, struct dpif_flow_stats *stats)
-{
- return subfacet_install(subfacet, subfacet->actions, subfacet->actions_len,
- stats, subfacet->slow);
-}
-
/* If 'subfacet' is installed in the datapath, uninstalls it. */
static void
subfacet_uninstall(struct subfacet *subfacet)
@@ -7947,7 +7824,7 @@ send_active_timeout(struct ofproto_dpif *ofproto, struct
facet *facet)
if (subfacet->path == SF_FAST_PATH) {
struct dpif_flow_stats stats;
- subfacet_reinstall(subfacet, &stats);
+ subfacet_install(subfacet, &facet->odp_actions, &stats);
subfacet_update_stats(subfacet, &stats);
}
}
@@ -8611,17 +8488,19 @@ ofproto_unixctl_dpif_dump_flows(struct unixctl_conn
*conn,
}
ds_put_cstr(&ds, ", actions:");
- if (subfacet->slow) {
+ if (subfacet->facet->slow) {
uint64_t slow_path_stub[128 / 8];
const struct nlattr *actions;
size_t actions_len;
- compose_slow_path(ofproto, &subfacet->facet->flow, subfacet->slow,
+ compose_slow_path(ofproto, &subfacet->facet->flow,
+ subfacet->facet->slow,
slow_path_stub, sizeof slow_path_stub,
&actions, &actions_len);
format_odp_actions(&ds, actions, actions_len);
} else {
- format_odp_actions(&ds, subfacet->actions, subfacet->actions_len);
+ format_odp_actions(&ds, subfacet->facet->odp_actions.data,
+ subfacet->facet->odp_actions.size);
}
ds_put_char(&ds, '\n');
}
--
1.7.9.5
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
