[ovs-dev] [threaded-put 13/21] ofproto: Move all statistics accounting into xlate_actions().

Ethan Jackson ethan at nicira.com
Mon Dec 9 02:45:19 UTC 2013


This patch moves statistics accounting for netflow, bonding, netdev,
and mirroring inside xlate_actions().  Moving all statistics into one
place makes it very difficult to mess up.

Signed-off-by: Ethan Jackson <ethan at nicira.com>
---
 ofproto/ofproto-dpif-xlate.c | 34 ++++++++++++++++++
 ofproto/ofproto-dpif-xlate.h |  4 +--
 ofproto/ofproto-dpif.c       | 84 +-------------------------------------------
 3 files changed, 37 insertions(+), 85 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index a03bc37..da03f06 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1148,6 +1148,11 @@ output_normal(struct xlate_ctx *ctx, const struct xbundle *out_xbundle,
             /* No slaves enabled, so drop packet. */
             return;
         }
+
+        if (ctx->xin->resubmit_stats) {
+            bond_account(out_xbundle->bond, &ctx->xin->flow, vid,
+                         ctx->xin->resubmit_stats->n_bytes);
+        }
     }
 
     old_tci = *flow_tci;
@@ -3125,6 +3130,10 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
     }
 
     in_port = get_ofp_port(ctx.xbridge, flow->in_port.ofp_port);
+    if (in_port && in_port->is_tunnel && ctx.xin->resubmit_stats) {
+        netdev_vport_inc_rx(in_port->netdev, ctx.xin->resubmit_stats);
+    }
+
     special = process_special(&ctx, flow, in_port, ctx.xin->packet);
     if (special) {
         ctx.xout->slow |= special;
@@ -3178,6 +3187,31 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
         ctx.xout->slow |= SLOW_ACTION;
     }
 
+    if (ctx.xin->resubmit_stats) {
+        mirror_update_stats(ctx.xbridge->mbridge, xout->mirrors,
+                            ctx.xin->resubmit_stats->n_packets,
+                            ctx.xin->resubmit_stats->n_bytes);
+
+        if (ctx.xbridge->netflow) {
+            const struct ofpact *ofpacts;
+            size_t ofpacts_len;
+
+            ofpacts_len = actions->ofpacts_len;
+            ofpacts = actions->ofpacts;
+            if (ofpacts_len == 0
+                || ofpacts->type != OFPACT_CONTROLLER
+                || ofpact_next(ofpacts) < ofpact_end(ofpacts, ofpacts_len)) {
+                /* Only update netflow if we don't have controller flow.  We don't
+                 * report NetFlow expiration messages for such facets because they
+                 * are just part of the control logic for the network, not real
+                 * traffic. */
+                netflow_flow_update(ctx.xbridge->netflow, flow,
+                                    xout->nf_output_iface,
+                                    ctx.xin->resubmit_stats);
+            }
+        }
+    }
+
     ofpbuf_uninit(&ctx.stack);
     ofpbuf_uninit(&ctx.action_set);
 
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 27bfa74..f4ef1e1 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -155,8 +155,8 @@ int xlate_receive(const struct dpif_backer *, struct ofpbuf *packet,
 void xlate_actions(struct xlate_in *, struct xlate_out *)
     OVS_EXCLUDED(xlate_rwlock);
 void xlate_in_init(struct xlate_in *, struct ofproto_dpif *,
-                   const struct flow *, struct rule_dpif *,
-                   uint16_t tcp_flags, const struct ofpbuf *packet);
+                   const struct flow *, struct rule_dpif *, uint16_t tcp_flags,
+                   const struct ofpbuf *packet);
 void xlate_out_uninit(struct xlate_out *);
 void xlate_actions_for_side_effects(struct xlate_in *);
 void xlate_out_copy(struct xlate_out *dst, const struct xlate_out *src);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index de628fb..ed52671 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -284,7 +284,6 @@ struct facet {
     long long int prev_used;     /* Used time from last stats push. */
 
     /* Accounting. */
-    uint64_t accounted_bytes;    /* Bytes processed by facet_account(). */
     uint16_t tcp_flags;          /* TCP flags seen for this 'rule'. */
 
     struct xlate_out xout;
@@ -316,7 +315,6 @@ static void flow_push_stats(struct ofproto_dpif *, struct flow *,
                             struct dpif_flow_stats *, bool may_learn);
 static void facet_push_stats(struct facet *, bool may_learn);
 static void facet_learn(struct facet *);
-static void facet_account(struct facet *);
 static void push_all_stats(void);
 
 static bool facet_is_controller_flow(struct facet *);
@@ -2773,13 +2771,6 @@ get_ofp_port(const struct ofproto_dpif *ofproto, ofp_port_t ofp_port)
     return ofport ? ofport_dpif_cast(ofport) : NULL;
 }
 
-static struct ofport_dpif *
-get_odp_port(const struct ofproto_dpif *ofproto, odp_port_t odp_port)
-{
-    struct ofport_dpif *port = odp_port_to_ofport(ofproto->backer, odp_port);
-    return port && &ofproto->up == port->up.ofproto ? port : NULL;
-}
-
 static void
 ofproto_port_from_dpif_port(struct ofproto_dpif *ofproto,
                             struct ofproto_port *ofproto_port,
@@ -3535,10 +3526,8 @@ update_subfacet_stats(struct subfacet *subfacet,
     subfacet->dp_byte_count = stats->n_bytes;
     subfacet_update_stats(subfacet, &diff);
 
-    if (facet->accounted_bytes < facet->byte_count) {
+    if (diff.n_packets) {
         facet_learn(facet);
-        facet_account(facet);
-        facet->accounted_bytes = facet->byte_count;
     }
 }
 
@@ -3809,11 +3798,6 @@ facet_create(const struct flow_miss *miss)
     classifier_insert(&ofproto->facets, &facet->cr);
     ovs_rwlock_unlock(&ofproto->facets.rwlock);
 
-    if (ofproto->netflow && !facet_is_controller_flow(facet)) {
-        netflow_flow_update(ofproto->netflow, &facet->flow,
-                            facet->xout.nf_output_iface, &miss->stats);
-    }
-
     return facet;
 }
 
@@ -3933,54 +3917,6 @@ facet_learn(struct facet *facet)
     facet_push_stats(facet, true);
 }
 
-static void
-facet_account(struct facet *facet)
-{
-    const struct nlattr *a;
-    unsigned int left;
-    ovs_be16 vlan_tci;
-    uint64_t n_bytes;
-
-    if (!facet->xout.has_normal || !facet->ofproto->has_bonded_bundles) {
-        return;
-    }
-    n_bytes = facet->byte_count - facet->accounted_bytes;
-
-    /* This loop feeds byte counters to bond_account() for rebalancing to use
-     * as a basis.  We also need to track the actual VLAN on which the packet
-     * is going to be sent to ensure that it matches the one passed to
-     * bond_choose_output_slave().  (Otherwise, we will account to the wrong
-     * hash bucket.)
-     *
-     * We use the actions from an arbitrary subfacet because they should all
-     * be equally valid for our purpose. */
-    vlan_tci = facet->flow.vlan_tci;
-    NL_ATTR_FOR_EACH_UNSAFE (a, left, facet->xout.odp_actions.data,
-                             facet->xout.odp_actions.size) {
-        const struct ovs_action_push_vlan *vlan;
-        struct ofport_dpif *port;
-
-        switch (nl_attr_type(a)) {
-        case OVS_ACTION_ATTR_OUTPUT:
-            port = get_odp_port(facet->ofproto, nl_attr_get_odp_port(a));
-            if (port && port->bundle && port->bundle->bond) {
-                bond_account(port->bundle->bond, &facet->flow,
-                             vlan_tci_to_vid(vlan_tci), n_bytes);
-            }
-            break;
-
-        case OVS_ACTION_ATTR_POP_VLAN:
-            vlan_tci = htons(0);
-            break;
-
-        case OVS_ACTION_ATTR_PUSH_VLAN:
-            vlan = nl_attr_get(a);
-            vlan_tci = vlan->vlan_tci;
-            break;
-        }
-    }
-}
-
 /* Returns true if the only action for 'facet' is to send to the controller.
  * (We don't report NetFlow expiration messages for such facets because they
  * are just part of the control logic for the network, not real traffic). */
@@ -4027,10 +3963,6 @@ facet_flush_stats(struct facet *facet)
     }
 
     facet_push_stats(facet, false);
-    if (facet->accounted_bytes < facet->byte_count) {
-        facet_account(facet);
-        facet->accounted_bytes = facet->byte_count;
-    }
 
     if (ofproto->netflow && !facet_is_controller_flow(facet)) {
         netflow_expire(ofproto->netflow, &facet->flow);
@@ -4232,21 +4164,14 @@ facet_reset_counters(struct facet *facet)
     facet->byte_count = 0;
     facet->prev_packet_count = 0;
     facet->prev_byte_count = 0;
-    facet->accounted_bytes = 0;
 }
 
 static void
 flow_push_stats(struct ofproto_dpif *ofproto, struct flow *flow,
                 struct dpif_flow_stats *stats, bool may_learn)
 {
-    struct ofport_dpif *in_port;
     struct xlate_in xin;
 
-    in_port = get_ofp_port(ofproto, flow->in_port.ofp_port);
-    if (in_port && in_port->is_tunnel) {
-        netdev_vport_inc_rx(in_port->up.netdev, stats);
-    }
-
     xlate_in_init(&xin, ofproto, flow, NULL, stats->tcp_flags, NULL);
     xin.resubmit_stats = stats;
     xin.may_learn = may_learn;
@@ -4271,13 +4196,6 @@ facet_push_stats(struct facet *facet, bool may_learn)
         facet->prev_packet_count = facet->packet_count;
         facet->prev_byte_count = facet->byte_count;
         facet->prev_used = facet->used;
-
-        if (facet->ofproto->netflow && !facet_is_controller_flow(facet)) {
-            netflow_flow_update(facet->ofproto->netflow, &facet->flow,
-                                facet->xout.nf_output_iface, &stats);
-        }
-        mirror_update_stats(facet->ofproto->mbridge, facet->xout.mirrors,
-                            stats.n_packets, stats.n_bytes);
         flow_push_stats(facet->ofproto, &facet->flow, &stats, may_learn);
     }
 }
-- 
1.8.1.2




More information about the dev mailing list