ofproto: Update facet stats when used time increases.
[sliver-openvswitch.git] / ofproto / ofproto.c
index 90f9710..590b792 100644 (file)
@@ -93,6 +93,10 @@ COVERAGE_DEFINE(ofproto_update_port);
 
 #include "sflow_api.h"
 
+/* Maximum depth of flow table recursion (due to NXAST_RESUBMIT actions) in a
+ * flow translation. */
+#define MAX_RESUBMIT_RECURSION 16
+
 struct rule;
 
 struct ofport {
@@ -123,7 +127,7 @@ struct action_xlate_ctx {
      *
      * This is normally null so the client has to set it manually after
      * calling action_xlate_ctx_init(). */
-    void (*resubmit_hook)(struct action_xlate_ctx *, const struct rule *);
+    void (*resubmit_hook)(struct action_xlate_ctx *, struct rule *);
 
     /* If true, the speciality of 'flow' should be checked before executing
      * its actions.  If special_cb returns false on 'flow' rendered
@@ -200,6 +204,8 @@ static void rule_insert(struct ofproto *, struct rule *);
 static void rule_remove(struct ofproto *, struct rule *);
 
 static void rule_send_removed(struct ofproto *, struct rule *, uint8_t reason);
+static void rule_get_stats(const struct rule *, uint64_t *packets,
+                           uint64_t *bytes);
 
 /* An exact-match instantiation of an OpenFlow flow. */
 struct facet {
@@ -221,6 +227,13 @@ struct facet {
     uint64_t packet_count;       /* Number of packets received. */
     uint64_t byte_count;         /* Number of bytes received. */
 
+    uint64_t dp_packet_count;    /* Last known packet count in the datapath. */
+    uint64_t dp_byte_count;      /* Last known byte count in the datapath. */
+
+    uint64_t rs_packet_count;    /* Packets pushed to resubmit children. */
+    uint64_t rs_byte_count;      /* Bytes pushed to resubmit children. */
+    long long int rs_used;       /* Used time pushed to resubmit children. */
+
     /* Number of bytes passed to account_cb.  This may include bytes that can
      * currently obtained from the datapath (thus, it can be greater than
      * byte_count). */
@@ -256,6 +269,7 @@ static void facet_make_actions(struct ofproto *, struct facet *,
                                const struct ofpbuf *packet);
 static void facet_update_stats(struct ofproto *, struct facet *,
                                const struct dpif_flow_stats *);
+static void facet_push_stats(struct ofproto *, struct facet *);
 
 /* ofproto supports two kinds of OpenFlow connections:
  *
@@ -411,6 +425,9 @@ static uint64_t pick_datapath_id(const struct ofproto *);
 static uint64_t pick_fallback_dpid(void);
 
 static int ofproto_expire(struct ofproto *);
+static void flow_push_stats(struct ofproto *, const struct rule *,
+                            struct flow *, uint64_t packets, uint64_t bytes,
+                            long long int used);
 
 static void handle_upcall(struct ofproto *, struct dpif_upcall *);
 
@@ -1509,6 +1526,8 @@ ofproto_flush_flows(struct ofproto *ofproto)
          * individually since we are about to blow away all the facets with
          * dpif_flow_flush(). */
         facet->installed = false;
+        facet->dp_packet_count = 0;
+        facet->dp_byte_count = 0;
         facet_remove(ofproto, facet);
     }
 
@@ -1840,6 +1859,7 @@ ofconn_run(struct ofconn *ofconn)
                 char *ofconn_name = ofconn_make_name(p, controller_name);
                 rconn_connect(ofconn->rconn, controller_name, ofconn_name);
                 free(ofconn_name);
+                free(controller_name);
             } else {
                 rconn_disconnect(ofconn->rconn);
             }
@@ -2143,8 +2163,8 @@ facet_execute(struct ofproto *ofproto, struct facet *facet,
     flow_extract_stats(&facet->flow, packet, &stats);
     if (execute_odp_actions(ofproto, &facet->flow,
                             facet->actions, facet->actions_len, packet)) {
-        facet_update_stats(ofproto, facet, &stats);
         facet->used = time_msec();
+        facet_update_stats(ofproto, facet, &stats);
         netflow_flow_update_time(ofproto->netflow,
                                  &facet->nf_flow, facet->used);
     }
@@ -2198,6 +2218,7 @@ rule_execute(struct ofproto *ofproto, struct rule *rule, uint16_t in_port,
         rule->used = time_msec();
         rule->packet_count++;
         rule->byte_count += size;
+        flow_push_stats(ofproto, rule, &flow, 1, size, rule->used);
     }
     ofpbuf_delete(odp_actions);
 }
@@ -2318,6 +2339,8 @@ facet_put__(struct ofproto *ofproto, struct facet *facet,
     flags = DPIF_FP_CREATE | DPIF_FP_MODIFY;
     if (stats) {
         flags |= DPIF_FP_ZERO_STATS;
+        facet->dp_packet_count = 0;
+        facet->dp_byte_count = 0;
     }
 
     ofpbuf_use_stack(&key, keybuf, sizeof keybuf);
@@ -2378,6 +2401,11 @@ facet_uninstall(struct ofproto *p, struct facet *facet)
             facet_update_stats(p, facet, &stats);
         }
         facet->installed = false;
+        facet->dp_packet_count = 0;
+        facet->dp_byte_count = 0;
+    } else {
+        assert(facet->dp_packet_count == 0);
+        assert(facet->dp_byte_count == 0);
     }
 }
 
@@ -2394,10 +2422,16 @@ facet_is_controller_flow(struct facet *facet)
 }
 
 /* Folds all of 'facet''s statistics into its rule.  Also updates the
- * accounting ofhook and emits a NetFlow expiration if appropriate.  */
+ * accounting ofhook and emits a NetFlow expiration if appropriate.  All of
+ * 'facet''s statistics in the datapath should have been zeroed and folded into
+ * its packet and byte counts before this function is called. */
 static void
 facet_flush_stats(struct ofproto *ofproto, struct facet *facet)
 {
+    assert(!facet->dp_byte_count);
+    assert(!facet->dp_packet_count);
+
+    facet_push_stats(ofproto, facet);
     facet_account(ofproto, facet, 0);
 
     if (ofproto->netflow && !facet_is_controller_flow(facet)) {
@@ -2416,6 +2450,8 @@ facet_flush_stats(struct ofproto *ofproto, struct facet *facet)
      * reinstalled. */
     facet->packet_count = 0;
     facet->byte_count = 0;
+    facet->rs_packet_count = 0;
+    facet->rs_byte_count = 0;
     facet->accounted_bytes = 0;
 
     netflow_flow_clear(&facet->nf_flow);
@@ -2535,6 +2571,7 @@ facet_revalidate(struct ofproto *ofproto, struct facet *facet)
         list_push_back(&new_rule->facets, &facet->list_node);
         facet->rule = new_rule;
         facet->used = new_rule->created;
+        facet->rs_used = facet->used;
     }
 
     ofpbuf_delete(odp_actions);
@@ -2662,10 +2699,6 @@ handle_set_config(struct ofconn *ofconn, const struct ofp_switch_config *osc)
     return 0;
 }
 
-/* Maximum depth of flow table recursion (due to NXAST_RESUBMIT actions) in a
- * flow translation. */
-#define MAX_RESUBMIT_RECURSION 16
-
 static void do_xlate_actions(const union ofp_action *in, size_t n_in,
                              struct action_xlate_ctx *ctx);
 
@@ -3462,46 +3495,6 @@ handle_port_stats_request(struct ofconn *ofconn, const struct ofp_header *oh)
     return 0;
 }
 
-/* Obtains statistic counters for 'rule' within 'p' and stores them into
- * '*packet_countp' and '*byte_countp'.  The returned statistics include
- * statistics for all of 'rule''s facets. */
-static void
-query_stats(struct ofproto *p, struct rule *rule,
-            uint64_t *packet_countp, uint64_t *byte_countp)
-{
-    uint32_t keybuf[ODPUTIL_FLOW_KEY_U32S];
-    uint64_t packet_count, byte_count;
-    struct facet *facet;
-    struct ofpbuf key;
-
-    /* Start from historical data for 'rule' itself that are no longer tracked
-     * by the datapath.  This counts, for example, facets that have expired. */
-    packet_count = rule->packet_count;
-    byte_count = rule->byte_count;
-
-    /* Ask the datapath for statistics on all of the rule's facets.
-     *
-     * Also, add any statistics that are not tracked by the datapath for each
-     * facet.  This includes, for example, statistics for packets that were
-     * executed "by hand" by ofproto via dpif_execute() but must be accounted
-     * to a rule. */
-    ofpbuf_use_stack(&key, keybuf, sizeof keybuf);
-    LIST_FOR_EACH (facet, list_node, &rule->facets) {
-        struct dpif_flow_stats stats;
-
-        ofpbuf_clear(&key);
-        odp_flow_key_from_flow(&key, &facet->flow);
-        dpif_flow_get(p->dpif, key.data, key.size, NULL, &stats);
-
-        packet_count += stats.n_packets + facet->packet_count;
-        byte_count += stats.n_bytes + facet->byte_count;
-    }
-
-    /* Return the stats to the caller. */
-    *packet_countp = packet_count;
-    *byte_countp = byte_count;
-}
-
 static void
 calc_flow_duration(long long int start, ovs_be32 *sec, ovs_be32 *nsec)
 {
@@ -3526,7 +3519,7 @@ put_ofp_flow_stats(struct ofconn *ofconn, struct rule *rule,
     act_len = sizeof *rule->actions * rule->n_actions;
     len = offsetof(struct ofp_flow_stats, actions) + act_len;
 
-    query_stats(ofconn->ofproto, rule, &packet_count, &byte_count);
+    rule_get_stats(rule, &packet_count, &byte_count);
 
     ofs = append_ofp_stats_reply(len, ofconn, replyp);
     ofs->length = htons(len);
@@ -3550,7 +3543,16 @@ put_ofp_flow_stats(struct ofconn *ofconn, struct rule *rule,
 static bool
 is_valid_table(uint8_t table_id)
 {
-    return table_id == 0 || table_id == 0xff;
+    if (table_id == 0 || table_id == 0xff) {
+        return true;
+    } else {
+        /* It would probably be better to reply with an error but there doesn't
+         * seem to be any appropriate value, so that might just be
+         * confusing. */
+        VLOG_WARN_RL(&rl, "controller asked for invalid table %"PRIu8,
+                     table_id);
+        return false;
+    }
 }
 
 static int
@@ -3591,7 +3593,7 @@ put_nx_flow_stats(struct ofconn *ofconn, struct rule *rule,
         return;
     }
 
-    query_stats(ofconn->ofproto, rule, &packet_count, &byte_count);
+    rule_get_stats(rule, &packet_count, &byte_count);
 
     act_len = sizeof *rule->actions * rule->n_actions;
 
@@ -3655,12 +3657,12 @@ handle_nxst_flow(struct ofconn *ofconn, const struct ofp_header *oh)
 }
 
 static void
-flow_stats_ds(struct ofproto *ofproto, struct rule *rule, struct ds *results)
+flow_stats_ds(struct rule *rule, struct ds *results)
 {
     uint64_t packet_count, byte_count;
     size_t act_len = sizeof *rule->actions * rule->n_actions;
 
-    query_stats(ofproto, rule, &packet_count, &byte_count);
+    rule_get_stats(rule, &packet_count, &byte_count);
 
     ds_put_format(results, "duration=%llds, ",
                   (time_msec() - rule->created) / 1000);
@@ -3688,7 +3690,7 @@ ofproto_get_all_flows(struct ofproto *p, struct ds *results)
 
     cls_cursor_init(&cursor, &p->cls, NULL);
     CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
-        flow_stats_ds(p, rule, results);
+        flow_stats_ds(rule, results);
     }
 }
 
@@ -3713,7 +3715,7 @@ query_aggregate_stats(struct ofproto *ofproto, struct cls_rule *target,
                 uint64_t packet_count;
                 uint64_t byte_count;
 
-                query_stats(ofproto, rule, &packet_count, &byte_count);
+                rule_get_stats(rule, &packet_count, &byte_count);
 
                 total_packets += packet_count;
                 total_bytes += byte_count;
@@ -3869,11 +3871,12 @@ handle_queue_stats_request(struct ofconn *ofconn, const struct ofp_header *oh)
     return 0;
 }
 
+/* Updates 'facet''s used time.  Caller is responsible for calling
+ * facet_push_stats() to update the flows which 'facet' resubmits into. */
 static void
 facet_update_time(struct ofproto *ofproto, struct facet *facet,
-                  const struct dpif_flow_stats *stats)
+                  long long int used)
 {
-    long long int used = stats->used;
     if (used > facet->used) {
         facet->used = used;
         if (used > facet->rule->used) {
@@ -3893,14 +3896,74 @@ static void
 facet_update_stats(struct ofproto *ofproto, struct facet *facet,
                    const struct dpif_flow_stats *stats)
 {
-    if (stats->n_packets) {
-        facet_update_time(ofproto, facet, stats);
+    if (stats->n_packets || stats->used > facet->used) {
+        facet_update_time(ofproto, facet, stats->used);
         facet->packet_count += stats->n_packets;
         facet->byte_count += stats->n_bytes;
+        facet_push_stats(ofproto, facet);
         netflow_flow_update_flags(&facet->nf_flow, stats->tcp_flags);
     }
 }
 
+static void
+facet_push_stats(struct ofproto *ofproto, struct facet *facet)
+{
+    uint64_t rs_packets, rs_bytes;
+
+    assert(facet->packet_count >= facet->rs_packet_count);
+    assert(facet->byte_count >= facet->rs_byte_count);
+    assert(facet->used >= facet->rs_used);
+
+    rs_packets = facet->packet_count - facet->rs_packet_count;
+    rs_bytes = facet->byte_count - facet->rs_byte_count;
+
+    if (rs_packets || rs_bytes || facet->used > facet->rs_used) {
+        facet->rs_packet_count = facet->packet_count;
+        facet->rs_byte_count = facet->byte_count;
+        facet->rs_used = facet->used;
+
+        flow_push_stats(ofproto, facet->rule, &facet->flow,
+                        rs_packets, rs_bytes, facet->used);
+    }
+}
+
+struct ofproto_push {
+    struct action_xlate_ctx ctx;
+    uint64_t packets;
+    uint64_t bytes;
+    long long int used;
+};
+
+static void
+push_resubmit(struct action_xlate_ctx *ctx, struct rule *rule)
+{
+    struct ofproto_push *push = CONTAINER_OF(ctx, struct ofproto_push, ctx);
+
+    if (rule) {
+        rule->packet_count += push->packets;
+        rule->byte_count += push->bytes;
+        rule->used = MAX(push->used, rule->used);
+    }
+}
+
+/* Pushes flow statistics to the rules which 'flow' resubmits into given
+ * 'rule''s actions. */
+static void
+flow_push_stats(struct ofproto *ofproto, const struct rule *rule,
+                struct flow *flow, uint64_t packets, uint64_t bytes,
+                long long int used)
+{
+    struct ofproto_push push;
+
+    push.packets = packets;
+    push.bytes = bytes;
+    push.used = used;
+
+    action_xlate_ctx_init(&push.ctx, ofproto, flow, NULL);
+    push.ctx.resubmit_hook = push_resubmit;
+    ofpbuf_delete(xlate_actions(&push.ctx, rule->actions, rule->n_actions));
+}
+
 /* Implements OFPFC_ADD and the cases for OFPFC_MODIFY and OFPFC_MODIFY_STRICT
  * in which no matching flow already exists in the flow table.
  *
@@ -4489,7 +4552,7 @@ handle_upcall(struct ofproto *p, struct dpif_upcall *upcall)
 /* Flow expiration. */
 
 static int ofproto_dp_max_idle(const struct ofproto *);
-static void ofproto_update_used(struct ofproto *);
+static void ofproto_update_stats(struct ofproto *);
 static void rule_expire(struct ofproto *, struct rule *);
 static void ofproto_expire_facets(struct ofproto *, int dp_max_idle);
 
@@ -4506,8 +4569,8 @@ ofproto_expire(struct ofproto *ofproto)
     struct cls_cursor cursor;
     int dp_max_idle;
 
-    /* Update 'used' for each flow in the datapath. */
-    ofproto_update_used(ofproto);
+    /* Update stats for each flow in the datapath. */
+    ofproto_update_stats(ofproto);
 
     /* Expire facets that have been idle too long. */
     dp_max_idle = ofproto_dp_max_idle(ofproto);
@@ -4530,9 +4593,19 @@ ofproto_expire(struct ofproto *ofproto)
     return MIN(dp_max_idle, 1000);
 }
 
-/* Update 'used' member of installed facets. */
+/* Update 'packet_count', 'byte_count', and 'used' members of installed facets.
+ *
+ * This function also pushes statistics updates to rules which each facet
+ * resubmits into.  Generally these statistics will be accurate.  However, if a
+ * facet changes the rule it resubmits into at some time in between
+ * ofproto_update_stats() runs, it is possible that statistics accrued to the
+ * old rule will be incorrectly attributed to the new rule.  This could be
+ * avoided by calling ofproto_update_stats() whenever rules are created or
+ * deleted.  However, the performance impact of making so many calls to the
+ * datapath do not justify the benefit of having perfectly accurate statistics.
+ */
 static void
-ofproto_update_used(struct ofproto *p)
+ofproto_update_stats(struct ofproto *p)
 {
     const struct dpif_flow_stats *stats;
     struct dpif_flow_dump dump;
@@ -4558,8 +4631,25 @@ ofproto_update_used(struct ofproto *p)
         facet = facet_find(p, &flow);
 
         if (facet && facet->installed) {
-            facet_update_time(p, facet, stats);
+
+            if (stats->n_packets >= facet->dp_packet_count) {
+                facet->packet_count += stats->n_packets - facet->dp_packet_count;
+            } else {
+                VLOG_WARN_RL(&rl, "unexpected packet count from the datapath");
+            }
+
+            if (stats->n_bytes >= facet->dp_byte_count) {
+                facet->byte_count += stats->n_bytes - facet->dp_byte_count;
+            } else {
+                VLOG_WARN_RL(&rl, "unexpected byte count from datapath");
+            }
+
+            facet->dp_packet_count = stats->n_packets;
+            facet->dp_byte_count = stats->n_bytes;
+
+            facet_update_time(p, facet, stats->used);
             facet_account(p, facet, stats->n_bytes);
+            facet_push_stats(p, facet);
         } else {
             /* There's a flow in the datapath that we know nothing about.
              * Delete it. */
@@ -4602,7 +4692,7 @@ ofproto_dp_max_idle(const struct ofproto *ofproto)
      * they receive additional data).
      *
      * This requires a second pass through the facets, in addition to the pass
-     * made by ofproto_update_used(), because the former function never looks
+     * made by ofproto_update_stats(), because the former function never looks
      * at uninstallable facets.
      */
     enum { BUCKET_WIDTH = ROUND_UP(100, TIME_UPDATE_INTERVAL) };
@@ -4806,6 +4896,31 @@ rule_send_removed(struct ofproto *p, struct rule *rule, uint8_t reason)
     }
 }
 
+/* Obtains statistics for 'rule' and stores them in '*packets' and '*bytes'.
+ * The returned statistics include statistics for all of 'rule''s facets. */
+static void
+rule_get_stats(const struct rule *rule, uint64_t *packets, uint64_t *bytes)
+{
+    uint64_t p, b;
+    struct facet *facet;
+
+    /* Start from historical data for 'rule' itself that are no longer tracked
+     * in facets.  This counts, for example, facets that have expired. */
+    p = rule->packet_count;
+    b = rule->byte_count;
+
+    /* Add any statistics that are tracked by facets.  This includes
+     * statistical data recently updated by ofproto_update_stats() as well as
+     * stats for packets that were executed "by hand" via dpif_execute(). */
+    LIST_FOR_EACH (facet, list_node, &rule->facets) {
+        p += facet->packet_count;
+        b += facet->byte_count;
+    }
+
+    *packets = p;
+    *bytes = b;
+}
+
 /* pinsched callback for sending 'ofp_packet_in' on 'ofconn'. */
 static void
 do_send_packet_in(struct ofpbuf *ofp_packet_in, void *ofconn_)
@@ -4994,7 +5109,7 @@ trace_format_flow(struct ds *result, int level, const char *title,
 }
 
 static void
-trace_resubmit(struct action_xlate_ctx *ctx, const struct rule *rule)
+trace_resubmit(struct action_xlate_ctx *ctx, struct rule *rule)
 {
     struct ofproto_trace *trace = CONTAINER_OF(ctx, struct ofproto_trace, ctx);
     struct ds *result = trace->result;