Looks Good. Ethan
On Fri, May 27, 2011 at 15:16, Ben Pfaff <[email protected]> wrote: > Some hardware supports reporting packet or byte counters but not both, so > OVS has to be prepared for that. > > Suggested-by: Justin Pettit <[email protected]> > --- > This patch applies after the async-flow-mod changes (currently pushed > to the "next" branch). > > include/openflow/nicira-ext.h | 12 ++++++------ > lib/ofp-util.c | 26 ++++++++++++++++++++------ > lib/ofp-util.h | 12 ++++++------ > ofproto/ofproto.c | 22 ++++++++++++++++++++-- > ofproto/private.h | 3 ++- > 5 files changed, 54 insertions(+), 21 deletions(-) > > diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h > index 9107928..eb76f28 100644 > --- a/include/openflow/nicira-ext.h > +++ b/include/openflow/nicira-ext.h > @@ -1293,8 +1293,8 @@ struct nx_flow_stats { > ovs_be16 match_len; /* Length of nx_match. */ > uint8_t pad2[4]; /* Align to 64 bits. */ > ovs_be64 cookie; /* Opaque controller-issued identifier. */ > - ovs_be64 packet_count; /* Number of packets in flow. */ > - ovs_be64 byte_count; /* Number of bytes in flow. */ > + ovs_be64 packet_count; /* Number of packets, UINT64_MAX if unknown. */ > + ovs_be64 byte_count; /* Number of bytes, UINT64_MAX if unknown. */ > /* Followed by: > * - Exactly match_len (possibly 0) bytes containing the nx_match, then > * - Exactly (match_len + 7)/8*8 - match_len (between 0 and 7) bytes of > @@ -1329,10 +1329,10 @@ OFP_ASSERT(sizeof(struct nx_aggregate_stats_request) > == 32); > * OFPST_AGGREGATE reply). */ > struct nx_aggregate_stats_reply { > struct nicira_stats_msg nsm; > - ovs_be64 packet_count; /* Number of packets in flows. */ > - ovs_be64 byte_count; /* Number of bytes in flows. */ > - ovs_be32 flow_count; /* Number of flows. */ > - uint8_t pad[4]; /* Align to 64 bits. */ > + ovs_be64 packet_count; /* Number of packets, UINT64_MAX if unknown. > */ > + ovs_be64 byte_count; /* Number of bytes, UINT64_MAX if unknown. */ > + ovs_be32 flow_count; /* Number of flows. */ > + uint8_t pad[4]; /* Align to 64 bits. */ > }; > OFP_ASSERT(sizeof(struct nx_aggregate_stats_reply) == 48); > > diff --git a/lib/ofp-util.c b/lib/ofp-util.c > index e72c47a..7f2ac16 100644 > --- a/lib/ofp-util.c > +++ b/lib/ofp-util.c > @@ -1270,6 +1270,16 @@ ofputil_decode_flow_stats_reply(struct > ofputil_flow_stats *fs, > return 0; > } > > +/* Returns 'count' unchanged except that UINT64_MAX becomes 0. > + * > + * We use this in situations where OVS internally uses UINT64_MAX to mean > + * "value unknown" but OpenFlow 1.0 does not define any unknown value. */ > +static uint64_t > +unknown_to_zero(uint64_t count) > +{ > + return count != UINT64_MAX ? count : 0; > +} > + > /* Appends an OFPST_FLOW or NXST_FLOW reply that contains the data in 'fs' to > * those already present in the list of ofpbufs in 'replies'. 'replies' > should > * have been initialized with ofputil_start_stats_reply(). */ > @@ -1297,8 +1307,10 @@ ofputil_append_flow_stats_reply(const struct > ofputil_flow_stats *fs, > ofs->idle_timeout = htons(fs->idle_timeout); > ofs->hard_timeout = htons(fs->hard_timeout); > memset(ofs->pad2, 0, sizeof ofs->pad2); > - put_32aligned_be64(&ofs->packet_count, htonll(fs->packet_count)); > - put_32aligned_be64(&ofs->byte_count, htonll(fs->byte_count)); > + put_32aligned_be64(&ofs->packet_count, > + htonll(unknown_to_zero(fs->packet_count))); > + put_32aligned_be64(&ofs->byte_count, > + htonll(unknown_to_zero(fs->byte_count))); > memcpy(ofs->actions, fs->actions, act_len); > } else if (osm->type == htons(OFPST_VENDOR)) { > struct nx_flow_stats *nfs; > @@ -1342,8 +1354,10 @@ ofputil_encode_aggregate_stats_reply( > struct ofp_aggregate_stats_reply *asr; > > asr = ofputil_make_stats_reply(sizeof *asr, request, &msg); > - put_32aligned_be64(&asr->packet_count, htonll(stats->packet_count)); > - put_32aligned_be64(&asr->byte_count, htonll(stats->byte_count)); > + put_32aligned_be64(&asr->packet_count, > + htonll(unknown_to_zero(stats->packet_count))); > + put_32aligned_be64(&asr->byte_count, > + htonll(unknown_to_zero(stats->byte_count))); > asr->flow_count = htonl(stats->flow_count); > } else if (request->type == htons(OFPST_VENDOR)) { > struct nx_aggregate_stats_reply *nasr; > @@ -1437,8 +1451,8 @@ ofputil_encode_flow_removed(const struct > ofputil_flow_removed *fr, > ofr->duration_sec = htonl(fr->duration_sec); > ofr->duration_nsec = htonl(fr->duration_nsec); > ofr->idle_timeout = htons(fr->idle_timeout); > - ofr->packet_count = htonll(fr->packet_count); > - ofr->byte_count = htonll(fr->byte_count); > + ofr->packet_count = htonll(unknown_to_zero(fr->packet_count)); > + ofr->byte_count = htonll(unknown_to_zero(fr->byte_count)); > } else if (flow_format == NXFF_NXM) { > struct nx_flow_removed *nfr; > int match_len; > diff --git a/lib/ofp-util.h b/lib/ofp-util.h > index e35fc46..26fa78b 100644 > --- a/lib/ofp-util.h > +++ b/lib/ofp-util.h > @@ -161,8 +161,8 @@ struct ofputil_flow_stats { > uint32_t duration_nsec; > uint16_t idle_timeout; > uint16_t hard_timeout; > - uint64_t packet_count; > - uint64_t byte_count; > + uint64_t packet_count; /* Packet count, UINT64_MAX if unknown. */ > + uint64_t byte_count; /* Byte count, UINT64_MAX if unknown. */ > union ofp_action *actions; > size_t n_actions; > }; > @@ -174,8 +174,8 @@ void ofputil_append_flow_stats_reply(const struct > ofputil_flow_stats *, > > /* Aggregate stats reply, independent of flow format. */ > struct ofputil_aggregate_stats { > - uint64_t packet_count; > - uint64_t byte_count; > + uint64_t packet_count; /* Packet count, UINT64_MAX if unknown. */ > + uint64_t byte_count; /* Byte count, UINT64_MAX if unknown. */ > uint32_t flow_count; > }; > > @@ -191,8 +191,8 @@ struct ofputil_flow_removed { > uint32_t duration_sec; > uint32_t duration_nsec; > uint16_t idle_timeout; > - uint64_t packet_count; > - uint64_t byte_count; > + uint64_t packet_count; /* Packet count, UINT64_MAX if unknown. */ > + uint64_t byte_count; /* Byte count, UINT64_MAX if unknown. */ > }; > > int ofputil_decode_flow_removed(struct ofputil_flow_removed *, > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > index 90c93c4..99cef8f 100644 > --- a/ofproto/ofproto.c > +++ b/ofproto/ofproto.c > @@ -1964,6 +1964,7 @@ handle_aggregate_stats_request(struct ofconn *ofconn, > struct ofproto *ofproto = ofconn_get_ofproto(ofconn); > struct flow_stats_request request; > struct ofputil_aggregate_stats stats; > + bool unknown_packets, unknown_bytes; > struct ofpbuf *reply; > struct list rules; > struct rule *rule; > @@ -1981,6 +1982,7 @@ handle_aggregate_stats_request(struct ofconn *ofconn, > } > > memset(&stats, 0, sizeof stats); > + unknown_packets = unknown_bytes = false; > LIST_FOR_EACH (rule, ofproto_node, &rules) { > uint64_t packet_count; > uint64_t byte_count; > @@ -1988,10 +1990,26 @@ handle_aggregate_stats_request(struct ofconn *ofconn, > ofproto->ofproto_class->rule_get_stats(rule, &packet_count, > &byte_count); > > - stats.packet_count += packet_count; > - stats.byte_count += byte_count; > + if (packet_count == UINT64_MAX) { > + unknown_packets = true; > + } else { > + stats.packet_count += packet_count; > + } > + > + if (byte_count == UINT64_MAX) { > + unknown_bytes = true; > + } else { > + stats.byte_count += byte_count; > + } > + > stats.flow_count++; > } > + if (unknown_packets) { > + stats.packet_count = UINT64_MAX; > + } > + if (unknown_bytes) { > + stats.byte_count = UINT64_MAX; > + } > > reply = ofputil_encode_aggregate_stats_reply(&stats, osm); > ofconn_send_reply(ofconn, reply); > diff --git a/ofproto/private.h b/ofproto/private.h > index 17166d4..0b12824 100644 > --- a/ofproto/private.h > +++ b/ofproto/private.h > @@ -672,7 +672,8 @@ struct ofproto_class { > > /* Obtains statistics for 'rule', storing the number of packets that have > * matched it in '*packet_count' and the number of bytes in those packets > - * in '*byte_count'. */ > + * in '*byte_count'. UINT64_MAX indicates that the packet count or byte > + * count is unknown. */ > void (*rule_get_stats)(struct rule *rule, uint64_t *packet_count, > uint64_t *byte_count); > > -- > 1.7.4.4 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
