ofproto-dpif: Revalidate when sFlow probability changes.
[cascardo/ovs.git] / ofproto / ofproto-dpif.c
index fdb4cd6..5dd7672 100644 (file)
@@ -57,6 +57,7 @@
 #include "ofproto-dpif-sflow.h"
 #include "ofproto-dpif-upcall.h"
 #include "ofproto-dpif-xlate.h"
+#include "ovs-rcu.h"
 #include "poll-loop.h"
 #include "seq.h"
 #include "simap.h"
@@ -238,6 +239,13 @@ COVERAGE_DEFINE(rev_port_toggled);
 COVERAGE_DEFINE(rev_flow_table);
 COVERAGE_DEFINE(rev_mac_learning);
 
+/* Stores mapping between 'recirc_id' and 'ofproto-dpif'. */
+struct dpif_backer_recirc_node {
+    struct hmap_node hmap_node;
+    struct ofproto_dpif *ofproto;
+    uint32_t recirc_id;
+};
+
 /* All datapaths of a given type share a single dpif backer instance. */
 struct dpif_backer {
     char *type;
@@ -256,6 +264,8 @@ struct dpif_backer {
 
     /* Recirculation. */
     struct recirc_id_pool *rid_pool;       /* Recirculation ID pool. */
+    struct hmap recirc_map;         /* Map of 'recirc_id's to 'ofproto's. */
+    struct ovs_mutex recirc_mutex;  /* Protects 'recirc_map'. */
     bool enable_recirc;   /* True if the datapath supports recirculation */
 
     /* True if the datapath supports variable-length
@@ -365,18 +375,6 @@ ofproto_dpif_flow_mod(struct ofproto_dpif *ofproto,
     ofproto_flow_mod(&ofproto->up, fm);
 }
 
-/* Resets the modified time for 'rule' or an equivalent rule. If 'rule' is not
- * in the classifier, but an equivalent rule is, unref 'rule' and ref the new
- * rule. Otherwise if 'rule' is no longer installed in the classifier,
- * reinstall it.
- *
- * Returns the rule whose modified time has been reset. */
-struct rule_dpif *
-ofproto_dpif_refresh_rule(struct rule_dpif *rule)
-{
-    return rule_dpif_cast(ofproto_refresh_rule(&rule->up));
-}
-
 /* Appends 'pin' to the queue of "packet ins" to be sent to the controller.
  * Takes ownership of 'pin' and pin->packet. */
 void
@@ -806,6 +804,26 @@ dealloc(struct ofproto *ofproto_)
     free(ofproto);
 }
 
+/* Called when 'ofproto' is destructed.  Checks for and clears any
+ * recirc_id leak. */
+static void
+dpif_backer_recirc_clear_ofproto(struct dpif_backer *backer,
+                                 struct ofproto_dpif *ofproto)
+{
+    struct dpif_backer_recirc_node *node;
+
+    ovs_mutex_lock(&backer->recirc_mutex);
+    HMAP_FOR_EACH (node, hmap_node, &backer->recirc_map) {
+        if (node->ofproto == ofproto) {
+            VLOG_ERR("recirc_id %"PRIu32", not freed when ofproto (%s) "
+                     "is destructed", node->recirc_id, ofproto->up.name);
+            hmap_remove(&backer->recirc_map, &node->hmap_node);
+            ovsrcu_postpone(free, node);
+        }
+    }
+    ovs_mutex_unlock(&backer->recirc_mutex);
+}
+
 static void
 close_dpif_backer(struct dpif_backer *backer)
 {
@@ -822,6 +840,8 @@ close_dpif_backer(struct dpif_backer *backer)
     hmap_destroy(&backer->odp_to_ofport_map);
     shash_find_and_delete(&all_dpif_backers, backer->type);
     recirc_id_pool_destroy(backer->rid_pool);
+    hmap_destroy(&backer->recirc_map);
+    ovs_mutex_destroy(&backer->recirc_mutex);
     free(backer->type);
     dpif_close(backer->dpif);
     free(backer);
@@ -929,6 +949,13 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
 
     shash_add(&all_dpif_backers, type, backer);
 
+    backer->enable_recirc = check_recirc(backer);
+    backer->variable_length_userdata = check_variable_length_userdata(backer);
+    backer->max_mpls_depth = check_max_mpls_depth(backer);
+    backer->rid_pool = recirc_id_pool_create();
+    ovs_mutex_init(&backer->recirc_mutex);
+    hmap_init(&backer->recirc_map);
+
     error = dpif_recv_set(backer->dpif, backer->recv_set_enable);
     if (error) {
         VLOG_ERR("failed to listen on datapath of type %s: %s",
@@ -936,10 +963,6 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
         close_dpif_backer(backer);
         return error;
     }
-    backer->enable_recirc = check_recirc(backer);
-    backer->variable_length_userdata = check_variable_length_userdata(backer);
-    backer->max_mpls_depth = check_max_mpls_depth(backer);
-    backer->rid_pool = recirc_id_pool_create();
 
     if (backer->recv_set_enable) {
         udpif_set_threads(backer->udpif, n_handlers, n_revalidators);
@@ -971,7 +994,7 @@ check_recirc(struct dpif_backer *backer)
     ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
     odp_flow_key_from_flow(&key, &flow, NULL, 0);
 
-    error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE | DPIF_FP_MODIFY,
+    error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE,
                           ofpbuf_data(&key), ofpbuf_size(&key), NULL, 0, NULL,
                           0, NULL);
     if (error && error != EEXIST) {
@@ -1055,11 +1078,6 @@ check_variable_length_userdata(struct dpif_backer *backer)
 
     switch (error) {
     case 0:
-        /* Variable-length userdata is supported.
-         *
-         * Purge received packets to avoid processing the nonsense packet we
-         * sent to userspace, then report success. */
-        dpif_recv_purge(backer->dpif);
         return true;
 
     case ERANGE:
@@ -1104,7 +1122,7 @@ check_max_mpls_depth(struct dpif_backer *backer)
         ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
         odp_flow_key_from_flow(&key, &flow, NULL, 0);
 
-        error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE | DPIF_FP_MODIFY,
+        error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE,
                               ofpbuf_data(&key), ofpbuf_size(&key), NULL, 0, NULL, 0, NULL);
         if (error && error != EEXIST) {
             if (error != EINVAL) {
@@ -1252,31 +1270,31 @@ add_internal_flows(struct ofproto_dpif *ofproto)
         return error;
     }
 
-    /* Continue non-recirculation rule lookups from table 0.
+    /* Drop any run away non-recirc rule lookups. Recirc_id has to be
+     * zero when reaching this rule.
      *
-     * (priority=2), recirc=0, actions=resubmit(, 0)
+     * (priority=2), recirc_id=0, actions=drop
      */
-    resubmit = ofpact_put_RESUBMIT(&ofpacts);
-    resubmit->ofpact.compat = 0;
-    resubmit->in_port = OFPP_IN_PORT;
-    resubmit->table_id = 0;
-
+    ofpbuf_clear(&ofpacts);
     match_init_catchall(&match);
     match_set_recirc_id(&match, 0);
-
     error = ofproto_dpif_add_internal_flow(ofproto, &match, 2, &ofpacts,
                                            &unused_rulep);
     if (error) {
         return error;
     }
 
-    /* Drop any run away recirc rule lookups. Recirc_id has to be
-     * non-zero when reaching this rule.
+    /* Continue rule lookups for not-matched recirc rules from table 0.
      *
-     * (priority=1), *, actions=drop
+     * (priority=1), actions=resubmit(, 0)
      */
-    ofpbuf_clear(&ofpacts);
+    resubmit = ofpact_put_RESUBMIT(&ofpacts);
+    resubmit->ofpact.compat = 0;
+    resubmit->in_port = OFPP_IN_PORT;
+    resubmit->table_id = 0;
+
     match_init_catchall(&match);
+
     error = ofproto_dpif_add_internal_flow(ofproto, &match, 1, &ofpacts,
                                            &unused_rulep);
 
@@ -1322,6 +1340,8 @@ destruct(struct ofproto *ofproto_)
     }
     guarded_list_destroy(&ofproto->pins);
 
+    dpif_backer_recirc_clear_ofproto(ofproto->backer, ofproto);
+
     mbridge_unref(ofproto->mbridge);
 
     netflow_unref(ofproto->netflow);
@@ -1744,6 +1764,7 @@ set_sflow(struct ofproto *ofproto_,
     struct dpif_sflow *ds = ofproto->sflow;
 
     if (sflow_options) {
+        uint32_t old_probability = ds ? dpif_sflow_get_probability(ds) : 0;
         if (!ds) {
             struct ofport_dpif *ofport;
 
@@ -1751,9 +1772,11 @@ set_sflow(struct ofproto *ofproto_,
             HMAP_FOR_EACH (ofport, up.hmap_node, &ofproto->up.ports) {
                 dpif_sflow_add_port(ds, &ofport->up, ofport->odp_port);
             }
-            ofproto->backer->need_revalidate = REV_RECONFIGURE;
         }
         dpif_sflow_set_options(ds, sflow_options);
+        if (dpif_sflow_get_probability(ds) != old_probability) {
+            ofproto->backer->need_revalidate = REV_RECONFIGURE;
+        }
     } else {
         if (ds) {
             dpif_sflow_unref(ds);
@@ -1826,14 +1849,14 @@ out:
 }
 
 static int
-get_cfm_status(const struct ofport *ofport_,
+get_cfm_status(const struct ofport *ofport_, bool force,
                struct ofproto_cfm_status *status)
 {
     struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
     int ret = 0;
 
     if (ofport->cfm) {
-        if (cfm_check_status_change(ofport->cfm)) {
+        if (cfm_check_status_change(ofport->cfm) || force) {
             status->faults = cfm_get_fault(ofport->cfm);
             status->flap_count = cfm_get_flap_count(ofport->cfm);
             status->remote_opstate = cfm_get_opup(ofport->cfm);
@@ -1868,13 +1891,13 @@ set_bfd(struct ofport *ofport_, const struct smap *cfg)
 }
 
 static int
-get_bfd_status(struct ofport *ofport_, struct smap *smap)
+get_bfd_status(struct ofport *ofport_, bool force, struct smap *smap)
 {
     struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
     int ret = 0;
 
     if (ofport->bfd) {
-        if (bfd_check_status_change(ofport->bfd)) {
+        if (bfd_check_status_change(ofport->bfd) || force) {
             bfd_get_status(ofport->bfd, smap);
         } else {
             ret = NO_STATUS_CHANGE;
@@ -2289,7 +2312,7 @@ bundle_destroy(struct ofbundle *bundle)
     }
 
     ofproto = bundle->ofproto;
-    mbridge_unregister_bundle(ofproto->mbridge, bundle->aux);
+    mbridge_unregister_bundle(ofproto->mbridge, bundle);
 
     ovs_rwlock_wrlock(&xlate_rwlock);
     xlate_bundle_remove(bundle);
@@ -3135,18 +3158,18 @@ ofproto_dpif_execute_actions(struct ofproto_dpif *ofproto,
     xin.resubmit_stats = &stats;
     xlate_actions(&xin, &xout);
 
+    execute.actions = ofpbuf_data(&xout.odp_actions);
+    execute.actions_len = ofpbuf_size(&xout.odp_actions);
+    execute.packet = packet;
+    execute.md = pkt_metadata_from_flow(flow);
+    execute.needs_help = (xout.slow & SLOW_ACTION) != 0;
+
+    /* Fix up in_port. */
     in_port = flow->in_port.ofp_port;
     if (in_port == OFPP_NONE) {
         in_port = OFPP_LOCAL;
     }
-    execute.actions = ofpbuf_data(&xout.odp_actions);
-    execute.actions_len = ofpbuf_size(&xout.odp_actions);
-    execute.packet = packet;
-    execute.md.tunnel = flow->tunnel;
-    execute.md.skb_priority = flow->skb_priority;
-    execute.md.pkt_mark = flow->pkt_mark;
     execute.md.in_port.odp_port = ofp_port_to_odp_port(ofproto, in_port);
-    execute.needs_help = (xout.slow & SLOW_ACTION) != 0;
 
     error = dpif_execute(ofproto->backer->dpif, &execute);
 
@@ -3216,16 +3239,7 @@ rule_dpif_lookup(struct ofproto_dpif *ofproto, struct flow *flow,
         if (wc) {
             wc->masks.recirc_id = UINT32_MAX;
         }
-
-        /* Start looking up from internal table for post recirculation flows
-         * or packets. We can also simply send all, including normal flows
-         * or packets to the internal table. They will not match any post
-         * recirculation rules except the 'catch all' rule that resubmit
-         * them to table 0.
-         *
-         * As an optimization, we send normal flows and packets to table 0
-         * directly, saving one table lookup.  */
-        table_id = flow->recirc_id ? TBL_INTERNAL : 0;
+        table_id = rule_dpif_lookup_get_init_table_id(flow);
     } else {
         table_id = 0;
     }
@@ -4783,20 +4797,66 @@ odp_port_to_ofp_port(const struct ofproto_dpif *ofproto, odp_port_t odp_port)
     }
 }
 
+struct ofproto_dpif *
+ofproto_dpif_recirc_get_ofproto(const struct dpif_backer *backer,
+                                uint32_t recirc_id)
+{
+    struct dpif_backer_recirc_node *node;
+
+    ovs_mutex_lock(&backer->recirc_mutex);
+    node = CONTAINER_OF(hmap_first_with_hash(&backer->recirc_map, recirc_id),
+                        struct dpif_backer_recirc_node, hmap_node);
+    ovs_mutex_unlock(&backer->recirc_mutex);
+
+    return node ? node->ofproto : NULL;
+}
+
 uint32_t
 ofproto_dpif_alloc_recirc_id(struct ofproto_dpif *ofproto)
 {
     struct dpif_backer *backer = ofproto->backer;
+    uint32_t recirc_id = recirc_id_alloc(backer->rid_pool);
 
-    return  recirc_id_alloc(backer->rid_pool);
+    if (recirc_id) {
+        struct dpif_backer_recirc_node *node = xmalloc(sizeof *node);
+
+        node->recirc_id = recirc_id;
+        node->ofproto = ofproto;
+
+        ovs_mutex_lock(&backer->recirc_mutex);
+        hmap_insert(&backer->recirc_map, &node->hmap_node, node->recirc_id);
+        ovs_mutex_unlock(&backer->recirc_mutex);
+    }
+
+    return recirc_id;
 }
 
 void
 ofproto_dpif_free_recirc_id(struct ofproto_dpif *ofproto, uint32_t recirc_id)
 {
     struct dpif_backer *backer = ofproto->backer;
+    struct dpif_backer_recirc_node *node;
+
+    ovs_mutex_lock(&backer->recirc_mutex);
+    node = CONTAINER_OF(hmap_first_with_hash(&backer->recirc_map, recirc_id),
+                        struct dpif_backer_recirc_node, hmap_node);
+    if (node) {
+        hmap_remove(&backer->recirc_map, &node->hmap_node);
+        ovs_mutex_unlock(&backer->recirc_mutex);
+        recirc_id_free(backer->rid_pool, node->recirc_id);
+
+        if (node->ofproto != ofproto) {
+            VLOG_ERR("recirc_id %"PRIu32", freed by incorrect ofproto (%s),"
+                     " expect ofproto (%s)", node->recirc_id, ofproto->up.name,
+                     node->ofproto->up.name);
+        }
 
-    recirc_id_free(backer->rid_pool, recirc_id);
+        /* RCU postpone the free, since other threads may be referring
+         * to 'node' at same time. */
+        ovsrcu_postpone(free, node);
+    } else {
+        ovs_mutex_unlock(&backer->recirc_mutex);
+    }
 }
 
 int