ofproto-dpif: Unset DPIF_FP_MODIFY flag when creating a new flo
[cascardo/ovs.git] / ofproto / ofproto-dpif.c
index 3cdb884..aa087b2 100644 (file)
@@ -792,6 +792,10 @@ static void update_max_subfacet_count(struct ofproto_dpif *ofproto);
  * for debugging the asynchronous flow_mod implementation.) */
 static bool clogged;
 
+/* By default, flows in the datapath are wildcarded (megaflows).  They
+ * may be disabled with the "ovs-appctl dpif/disable-megaflows" command. */
+static bool enable_megaflows = true;
+
 /* All existing ofproto_dpif instances, indexed by ->up.name. */
 static struct hmap all_ofproto_dpifs = HMAP_INITIALIZER(&all_ofproto_dpifs);
 
@@ -3767,11 +3771,7 @@ handle_flow_miss_with_facet(struct flow_miss *miss, struct facet *facet,
     struct subfacet *subfacet;
     struct ofpbuf *packet;
 
-    subfacet = subfacet_create(facet, miss, now);
     want_path = facet->xout.slow ? SF_SLOW_PATH : SF_FAST_PATH;
-    if (stats) {
-        subfacet_update_stats(subfacet, stats);
-    }
 
     LIST_FOR_EACH (packet, list_node, &miss->packets) {
         struct flow_miss_op *op = &ops[*n_ops];
@@ -3796,6 +3796,27 @@ handle_flow_miss_with_facet(struct flow_miss *miss, struct facet *facet,
         }
     }
 
+    /* Don't install the flow if it's the result of the "userspace"
+     * action for an already installed facet.  This can occur when a
+     * datapath flow with wildcards has a "userspace" action and flows
+     * sent to userspace result in a different subfacet, which will then
+     * be rejected as overlapping by the datapath. */
+    if (miss->upcall_type == DPIF_UC_ACTION
+        && !list_is_empty(&facet->subfacets)) {
+        if (stats) {
+            facet->used = MAX(facet->used, stats->used);
+            facet->packet_count += stats->n_packets;
+            facet->byte_count += stats->n_bytes;
+            facet->tcp_flags |= stats->tcp_flags;
+        }
+        return;
+    }
+
+    subfacet = subfacet_create(facet, miss, now);
+    if (stats) {
+        subfacet_update_stats(subfacet, stats);
+    }
+
     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;
@@ -3803,13 +3824,15 @@ handle_flow_miss_with_facet(struct flow_miss *miss, struct facet *facet,
         subfacet->path = want_path;
 
         ofpbuf_use_stack(&op->mask, &op->maskbuf, sizeof op->maskbuf);
-        odp_flow_key_from_mask(&op->mask, &facet->xout.wc.masks,
-                               &miss->flow, UINT32_MAX);
+        if (enable_megaflows) {
+            odp_flow_key_from_mask(&op->mask, &facet->xout.wc.masks,
+                                   &miss->flow, UINT32_MAX);
+        }
 
         op->xout_garbage = false;
         op->dpif_op.type = DPIF_OP_FLOW_PUT;
         op->subfacet = subfacet;
-        put->flags = DPIF_FP_CREATE | DPIF_FP_MODIFY;
+        put->flags = DPIF_FP_CREATE;
         put->key = miss->key;
         put->key_len = miss->key_len;
         put->mask = op->mask.data;
@@ -4154,6 +4177,18 @@ handle_miss_upcalls(struct dpif_backer *backer, struct dpif_upcall *upcalls,
 
             COVERAGE_INC(subfacet_install_fail);
 
+            /* Zero-out subfacet counters when installation failed, but
+             * datapath reported hits.  This should not happen and
+             * indicates a bug, since if the datapath flow exists, we
+             * should not be attempting to create a new subfacet.  A
+             * buggy datapath could trigger this, so just zero out the
+             * counters and log an error. */
+            if (subfacet->dp_packet_count || subfacet->dp_byte_count) {
+                VLOG_ERR_RL(&rl, "failed to install subfacet for which "
+                            "datapath reported hits");
+                subfacet->dp_packet_count = subfacet->dp_byte_count = 0;
+            }
+
             subfacet->path = SF_NOT_INSTALLED;
         }
 
@@ -5411,7 +5446,8 @@ subfacet_install(struct subfacet *subfacet, const struct ofpbuf *odp_actions,
     enum dpif_flow_put_flags flags;
     int ret;
 
-    flags = DPIF_FP_CREATE | DPIF_FP_MODIFY;
+    flags = subfacet->path == SF_NOT_INSTALLED ? DPIF_FP_CREATE
+                                               : DPIF_FP_MODIFY;
     if (stats) {
         flags |= DPIF_FP_ZERO_STATS;
     }
@@ -5423,8 +5459,10 @@ subfacet_install(struct subfacet *subfacet, const struct ofpbuf *odp_actions,
     }
 
     ofpbuf_use_stack(&mask, &maskbuf, sizeof maskbuf);
-    odp_flow_key_from_mask(&mask, &facet->xout.wc.masks,
-                           &facet->flow, UINT32_MAX);
+    if (enable_megaflows) {
+        odp_flow_key_from_mask(&mask, &facet->xout.wc.masks,
+                               &facet->flow, UINT32_MAX);
+    }
 
     ret = dpif_flow_put(ofproto->backer->dpif, flags, subfacet->key,
                         subfacet->key_len,  mask.data, mask.size,
@@ -6694,6 +6732,10 @@ xlate_fin_timeout(struct xlate_ctx *ctx,
     if (ctx->xin->tcp_flags & (TCP_FIN | TCP_RST) && ctx->rule) {
         struct rule_dpif *rule = ctx->rule;
 
+        if (list_is_empty(&rule->up.expirable)) {
+            list_insert(&ctx->ofproto->up.expirable, &rule->up.expirable);
+        }
+
         reduce_timeout(oft->fin_idle_timeout, &rule->up.idle_timeout);
         reduce_timeout(oft->fin_hard_timeout, &rule->up.hard_timeout);
     }
@@ -7088,12 +7130,12 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
      *   to another device without any modifications this will cause us to
      *   insert a new tag since the original one was stripped off by the
      *   VLAN device.
-     * - Tunnel 'flow' is largely cleared when transitioning between
-     *   the input and output stages since it does not make sense to output
-     *   a packet with the exact headers that it was received with (i.e.
-     *   the destination IP is us).  The one exception is the tun_id, which
-     *   is preserved to allow use in later resubmit lookups and loads into
-     *   registers.
+     * - Tunnel metadata as received is retained in 'flow'. This allows
+     *   tunnel metadata matching also in later tables.
+     *   Since a kernel action for setting the tunnel metadata will only be
+     *   generated with actual tunnel output, changing the tunnel metadata
+     *   values in 'flow' (such as tun_id) will only have effect with a later
+     *   tunnel output action.
      * - Tunnel 'base_flow' is completely cleared since that is what the
      *   kernel does.  If we wish to maintain the original values an action
      *   needs to be generated. */
@@ -8240,7 +8282,7 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, int argc, const char *argv[],
 {
     const char *dpname = argv[1];
     struct ofproto_dpif *ofproto;
-    struct ofpbuf odp_key;
+    struct ofpbuf odp_key, odp_mask;
     struct ofpbuf *packet;
     struct initial_vals initial_vals;
     struct ds result;
@@ -8250,6 +8292,7 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, int argc, const char *argv[],
     packet = NULL;
     ofpbuf_init(&odp_key, 0);
     ds_init(&result);
+    ofpbuf_init(&odp_mask, 0);
 
     ofproto = ofproto_dpif_lookup(dpname);
     if (!ofproto) {
@@ -8277,7 +8320,7 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, int argc, const char *argv[],
 
             /* Convert string to datapath key. */
             ofpbuf_init(&odp_key, 0);
-            error = odp_flow_from_string(flow_s, NULL, &odp_key, NULL);
+            error = odp_flow_from_string(flow_s, NULL, &odp_key, &odp_mask);
             if (error) {
                 unixctl_command_reply_error(conn, "Bad flow syntax");
                 goto exit;
@@ -8352,6 +8395,7 @@ exit:
     ds_destroy(&result);
     ofpbuf_delete(packet);
     ofpbuf_uninit(&odp_key);
+    ofpbuf_uninit(&odp_mask);
 }
 
 static void
@@ -8729,6 +8773,48 @@ ofproto_unixctl_dpif_dump_megaflows(struct unixctl_conn *conn,
     ds_destroy(&ds);
 }
 
+/* Disable using the megaflows.
+ *
+ * This command is only needed for advanced debugging, so it's not
+ * documented in the man page. */
+static void
+ofproto_unixctl_dpif_disable_megaflows(struct unixctl_conn *conn,
+                                       int argc OVS_UNUSED,
+                                       const char *argv[] OVS_UNUSED,
+                                       void *aux OVS_UNUSED)
+{
+    struct ofproto_dpif *ofproto;
+
+    enable_megaflows = false;
+
+    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+        flush(&ofproto->up);
+    }
+
+    unixctl_command_reply(conn, "megaflows disabled");
+}
+
+/* Re-enable using megaflows.
+ *
+ * This command is only needed for advanced debugging, so it's not
+ * documented in the man page. */
+static void
+ofproto_unixctl_dpif_enable_megaflows(struct unixctl_conn *conn,
+                                      int argc OVS_UNUSED,
+                                      const char *argv[] OVS_UNUSED,
+                                      void *aux OVS_UNUSED)
+{
+    struct ofproto_dpif *ofproto;
+
+    enable_megaflows = true;
+
+    HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+        flush(&ofproto->up);
+    }
+
+    unixctl_command_reply(conn, "megaflows enabled");
+}
+
 static void
 ofproto_unixctl_dpif_dump_flows(struct unixctl_conn *conn,
                                 int argc OVS_UNUSED, const char *argv[],
@@ -8748,8 +8834,17 @@ ofproto_unixctl_dpif_dump_flows(struct unixctl_conn *conn,
 
     HMAP_FOR_EACH (subfacet, hmap_node, &ofproto->subfacets) {
         struct facet *facet = subfacet->facet;
+        struct odputil_keybuf maskbuf;
+        struct ofpbuf mask;
+
+        ofpbuf_use_stack(&mask, &maskbuf, sizeof maskbuf);
+        if (enable_megaflows) {
+            odp_flow_key_from_mask(&mask, &facet->xout.wc.masks,
+                                   &facet->flow, UINT32_MAX);
+        }
 
-        odp_flow_key_format(subfacet->key, subfacet->key_len, &ds);
+        odp_flow_format(subfacet->key, subfacet->key_len,
+                        mask.data, mask.size, &ds);
 
         ds_put_format(&ds, ", packets:%"PRIu64", bytes:%"PRIu64", used:",
                       subfacet->dp_packet_count, subfacet->dp_byte_count);
@@ -8838,6 +8933,10 @@ ofproto_dpif_unixctl_init(void)
                              ofproto_unixctl_dpif_del_flows, NULL);
     unixctl_command_register("dpif/dump-megaflows", "bridge", 1, 1,
                              ofproto_unixctl_dpif_dump_megaflows, NULL);
+    unixctl_command_register("dpif/disable-megaflows", "", 0, 0,
+                             ofproto_unixctl_dpif_disable_megaflows, NULL);
+    unixctl_command_register("dpif/enable-megaflows", "", 0, 0,
+                             ofproto_unixctl_dpif_enable_megaflows, NULL);
 }
 \f
 /* Linux VLAN device support (e.g. "eth0.10" for VLAN 10.)