[ovs-dev] [PATCH 2/4] ofproto-dpif: Handle failed flow 'put's.
Justin Pettit
jpettit at nicira.com
Thu Jun 20 22:16:43 UTC 2013
If a flow cannot be installed in the datapath, we should notice
this and not treat it as installed. This becomes an issue with
megaflows, since a batch of unique flows may come in that generate
a single new datapath megaflow that covers them. Since userspace
doesn't know whether the datapath supports megaflows, each unique
flow will get a separate flow entry (which overlap when masks are
applied) and all except the first will get rejected by a megaflow-
supporting datapath as duplicates.
Signed-off-by: Justin Pettit <jpettit at nicira.com>
---
lib/dpif.c | 7 ++++++-
ofproto/ofproto-dpif.c | 24 ++++++++++++++++++++++--
2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/lib/dpif.c b/lib/dpif.c
index 4193dcb..fa62cec 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1261,7 +1261,12 @@ log_operation(const struct dpif *dpif, const char *operation, int error)
static enum vlog_level
flow_message_log_level(int error)
{
- return error ? VLL_WARN : VLL_DBG;
+ /* If flows arrive in a batch, userspace may push down multiple
+ * unique flow definitions that overlap when wildcards are applied.
+ * Kernels that support flow wildcarding will reject these flows as
+ * duplicates (EINVAL), so lower the log level to debug for these
+ * types of messages. */
+ return (error && error != EINVAL) ? VLL_WARN : VLL_DBG;
}
static bool
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 6fa7894..7ab3e7f 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -69,6 +69,7 @@ COVERAGE_DEFINE(facet_changed_rule);
COVERAGE_DEFINE(facet_revalidate);
COVERAGE_DEFINE(facet_unexpected);
COVERAGE_DEFINE(facet_suppress);
+COVERAGE_DEFINE(subfacet_install_fail);
struct flow_miss;
struct facet;
@@ -3245,6 +3246,10 @@ struct flow_miss_op {
uint64_t slow_stub[128 / 8]; /* Buffer for compose_slow_path() */
struct xlate_out xout;
bool xout_garbage; /* 'xout' needs to be uninitialized? */
+
+ /* If this is a "put" op, then a pointer to the subfacet that should
+ * be destroyed if the operation fails. */
+ struct subfacet *subfacet;
};
/* Sends an OFPT_PACKET_IN message for 'packet' of type OFPR_NO_MATCH to each
@@ -3307,6 +3312,7 @@ init_flow_miss_execute_op(struct flow_miss *miss, struct ofpbuf *packet,
eth_pop_vlan(packet);
}
+ op->subfacet = NULL;
op->xout_garbage = false;
op->dpif_op.type = DPIF_OP_EXECUTE;
op->dpif_op.u.execute.key = miss->key;
@@ -3461,6 +3467,7 @@ handle_flow_miss_with_facet(struct flow_miss *miss, struct facet *facet,
op->xout_garbage = false;
op->dpif_op.type = DPIF_OP_FLOW_PUT;
+ op->subfacet = subfacet;
put->flags = DPIF_FP_CREATE | DPIF_FP_MODIFY;
put->key = miss->key;
put->key_len = miss->key_len;
@@ -3771,8 +3778,19 @@ handle_miss_upcalls(struct dpif_backer *backer, struct dpif_upcall *upcalls,
}
dpif_operate(backer->dpif, dpif_ops, n_ops);
- /* Free memory. */
for (i = 0; i < n_ops; i++) {
+ /* Destroy subfacets for flows that weren't installed. */
+ if (dpif_ops[i]->error != 0
+ && flow_miss_ops[i].dpif_op.type == DPIF_OP_FLOW_PUT
+ && flow_miss_ops[i].subfacet) {
+ struct subfacet *subfacet = flow_miss_ops[i].subfacet;
+
+ COVERAGE_INC(subfacet_install_fail);
+
+ subfacet->path = SF_NOT_INSTALLED;
+ }
+
+ /* Free memory. */
if (flow_miss_ops[i].xout_garbage) {
xlate_out_uninit(&flow_miss_ops[i].xout);
}
@@ -5037,7 +5055,9 @@ subfacet_install(struct subfacet *subfacet, const struct ofpbuf *odp_actions,
subfacet_reset_dp_stats(subfacet, stats);
}
- if (!ret) {
+ if (ret) {
+ COVERAGE_INC(subfacet_install_fail);
+ } else {
subfacet->path = path;
}
return ret;
--
1.7.5.4
More information about the dev
mailing list