[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