From: Justin Pettit <jpettit@nicira.com>
Date: Tue, 11 Jun 2013 01:09:53 +0000 (-0700)
Subject: ofproto-dpif: Handle failed flow 'put's.
X-Git-Tag: v1.11.0~68
X-Git-Url: http://git.cascardo.info/?p=cascardo%2Fovs.git;a=commitdiff_plain;h=3c378294ec20759468320a021d9f4ac72f4ea174

ofproto-dpif: Handle failed flow 'put's.

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@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>
---

diff --git a/lib/dpif.c b/lib/dpif.c
index 370de3c71..e9b47375d 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 (EEXIST), so lower the log level to debug for these
+     * types of messages. */
+    return (error && error != EEXIST) ? VLL_WARN : VLL_DBG;
 }
 
 static bool
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index a2eb54e65..b4add10ce 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -66,6 +66,7 @@ COVERAGE_DEFINE(facet_changed_rule);
 COVERAGE_DEFINE(facet_revalidate);
 COVERAGE_DEFINE(facet_unexpected);
 COVERAGE_DEFINE(facet_suppress);
+COVERAGE_DEFINE(subfacet_install_fail);
 
 /* Maximum depth of flow table recursion (due to resubmit actions) in a
  * flow translation. */
@@ -3548,6 +3549,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 marked as uninstalled if the operation fails. */
+    struct subfacet *subfacet;
 };
 
 /* Sends an OFPT_PACKET_IN message for 'packet' of type OFPR_NO_MATCH to each
@@ -3639,6 +3644,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;
@@ -3795,6 +3801,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;
@@ -4131,8 +4138,18 @@ 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++) {
+        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);
         }
@@ -5403,7 +5420,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;