From: Ethan Jackson Date: Wed, 13 Nov 2013 02:18:01 +0000 (-0800) Subject: ofproto: Move all statistics accounting into xlate_actions(). X-Git-Tag: sliver-openvswitch-2.1.90-1~10^2~196 X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=d6fc5f57bb57fede0ddad2272a734a6212eda6b2;p=sliver-openvswitch.git ofproto: Move all statistics accounting into xlate_actions(). 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 Acked-by: Ben Pfaff --- diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index cea46583e..402f54bcf 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -219,10 +219,10 @@ static void do_xlate_actions(const struct ofpact *, size_t ofpacts_len, struct xlate_ctx *); static void xlate_actions__(struct xlate_in *, struct xlate_out *) OVS_REQ_RDLOCK(xlate_rwlock); -static void xlate_normal(struct xlate_ctx *); -static void xlate_report(struct xlate_ctx *, const char *); -static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port, - uint8_t table_id, bool may_packet_in); + static void xlate_normal(struct xlate_ctx *); + static void xlate_report(struct xlate_ctx *, const char *); + static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port, + uint8_t table_id, bool may_packet_in); static bool input_vid_is_valid(uint16_t vid, struct xbundle *, bool warn); static uint16_t input_vid_to_vlan(const struct xbundle *, uint16_t vid); static void output_normal(struct xlate_ctx *, const struct xbundle *, @@ -559,8 +559,8 @@ xlate_receive(const struct dpif_backer *backer, struct ofpbuf *packet, } xport = xport_lookup(tnl_port_should_receive(flow) - ? tnl_port_receive(flow) - : odp_port_to_ofport(backer, flow->in_port.odp_port)); + ? tnl_port_receive(flow) + : odp_port_to_ofport(backer, flow->in_port.odp_port)); flow->in_port.ofp_port = xport ? xport->ofp_port : OFPP_NONE; if (!xport) { @@ -794,10 +794,10 @@ bucket_is_alive(const struct xlate_ctx *ctx, } return !ofputil_bucket_has_liveness(bucket) || - (bucket->watch_port != OFPP_ANY && - odp_port_is_alive(ctx, bucket->watch_port)) || - (bucket->watch_group != OFPG_ANY && - group_is_alive(ctx, bucket->watch_group, depth + 1)); + (bucket->watch_port != OFPP_ANY && + odp_port_is_alive(ctx, bucket->watch_port)) || + (bucket->watch_group != OFPG_ANY && + group_is_alive(ctx, bucket->watch_group, depth + 1)); } static const struct ofputil_bucket * @@ -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; @@ -1207,7 +1212,7 @@ is_mac_learning_update_needed(const struct mac_learning *ml, const struct flow *flow, struct flow_wildcards *wc, int vlan, struct xbundle *in_xbundle) - OVS_REQ_RDLOCK(ml->rwlock) +OVS_REQ_RDLOCK(ml->rwlock) { struct mac_entry *mac; @@ -1247,7 +1252,7 @@ static void update_learning_table__(const struct xbridge *xbridge, const struct flow *flow, struct flow_wildcards *wc, int vlan, struct xbundle *in_xbundle) - OVS_REQ_WRLOCK(xbridge->ml->rwlock) +OVS_REQ_WRLOCK(xbridge->ml->rwlock) { struct mac_entry *mac; @@ -1356,7 +1361,7 @@ is_admissible(struct xlate_ctx *ctx, struct xport *in_port, || mac_entry_is_grat_arp_locked(mac))) { ovs_rwlock_unlock(&xbridge->ml->rwlock); xlate_report(ctx, "SLB bond thinks this packet looped back, " - "dropping"); + "dropping"); return false; } ovs_rwlock_unlock(&xbridge->ml->rwlock); @@ -3128,6 +3133,13 @@ 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); + if (in_port->bfd) { + bfd_account_rx(in_port->bfd, ctx.xin->resubmit_stats); + } + } + special = process_special(&ctx, flow, in_port, ctx.xin->packet); if (special) { ctx.xout->slow |= special; @@ -3181,6 +3193,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 27bfa74a5..f4ef1e1fd 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 ee08fb560..17ddb421d 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -286,7 +286,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; @@ -318,7 +317,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 *); @@ -2820,13 +2818,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, @@ -3582,10 +3573,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; } } @@ -3856,11 +3845,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; } @@ -3980,54 +3964,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). */ @@ -4074,10 +4010,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); @@ -4283,24 +4215,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); - if (in_port->bfd) { - bfd_account_rx(in_port->bfd, stats); - } - } - xlate_in_init(&xin, ofproto, flow, NULL, stats->tcp_flags, NULL); xin.resubmit_stats = stats; xin.may_learn = may_learn; @@ -4325,13 +4247,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); } }