[ovs-dev] [pre-async 7/8] ofproto: Better abstract aggregate stats encoding and decoding.

Ben Pfaff blp at nicira.com
Tue May 24 23:25:19 UTC 2011


---
 lib/ofp-print.c   |    7 ++-
 lib/ofp-util.c    |   40 +++++++++++++++++--
 lib/ofp-util.h    |   11 +++++
 ofproto/ofproto.c |  113 +++++++++++------------------------------------------
 4 files changed, 74 insertions(+), 97 deletions(-)

diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index d1e9fd9..5c532c9 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1063,12 +1063,13 @@ ofp_print_ofpst_desc_reply(struct ds *string, const struct ofp_desc_stats *ods)
 }
 
 static void
-ofp_print_flow_stats_request(struct ds *string, const struct ofp_header *oh)
+ofp_print_flow_stats_request(struct ds *string,
+                             const struct ofp_stats_msg *osm)
 {
     struct flow_stats_request fsr;
     int error;
 
-    error = ofputil_decode_flow_stats_request(&fsr, oh);
+    error = ofputil_decode_flow_stats_request(&fsr, &osm->header);
     if (error) {
         ofp_print_error(string, error);
         return;
@@ -1451,7 +1452,7 @@ ofp_to_string__(const struct ofp_header *oh,
     case OFPUTIL_OFPST_AGGREGATE_REQUEST:
     case OFPUTIL_NXST_AGGREGATE_REQUEST:
         ofp_print_stats_request(string, oh);
-        ofp_print_flow_stats_request(string, oh);
+        ofp_print_flow_stats_request(string, msg);
         break;
 
     case OFPUTIL_OFPST_TABLE_REQUEST:
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 6d9ae64..b29b674 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1270,6 +1270,37 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
     return 0;
 }
 
+/* Converts abstract ofputil_aggregate_stats 'stats' into an OFPST_AGGREGATE or
+ * NXST_AGGREGATE reply according to 'flow_format', and returns the message. */
+struct ofpbuf *
+ofputil_encode_aggregate_stats_reply(
+    const struct ofputil_aggregate_stats *stats,
+    const struct ofp_stats_msg *request)
+{
+    struct ofpbuf *msg;
+
+    if (request->type == htons(OFPST_AGGREGATE)) {
+        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));
+        asr->flow_count = htonl(stats->flow_count);
+    } else if (request->type == htons(OFPST_VENDOR)) {
+        struct nx_aggregate_stats_reply *nasr;
+
+        nasr = ofputil_make_stats_reply(sizeof *nasr, request, &msg);
+        assert(nasr->nsm.subtype == htonl(NXST_AGGREGATE));
+        nasr->packet_count = htonll(stats->packet_count);
+        nasr->byte_count = htonll(stats->byte_count);
+        nasr->flow_count = htonl(stats->flow_count);
+    } else {
+        NOT_REACHED();
+    }
+
+    return msg;
+}
+
 /* Converts an OFPT_FLOW_REMOVED or NXT_FLOW_REMOVED message 'oh' into an
  * abstract ofputil_flow_removed in 'fr'.  Returns 0 if successful, otherwise
  * an OpenFlow error code. */
@@ -1579,8 +1610,8 @@ put_stats__(ovs_be32 xid, uint8_t ofp_type,
     }
 }
 
-/* Creates an ofp_stats_msg with the given 'type' and 'body_len' bytes of space
- * allocated for the 'body' member.  Returns the first byte of the 'body'
+/* Creates an ofp_stats_request with the given 'type' and 'body_len' bytes of
+ * space allocated for the 'body' member.  Returns the first byte of the 'body'
  * member. */
 void *
 ofputil_make_stats_request(size_t openflow_len, uint16_t ofpst_type,
@@ -1608,7 +1639,7 @@ put_stats_reply__(const struct ofp_stats_msg *request, struct ofpbuf *msg)
                 msg);
 }
 
-/* Creates an stats reply message with the given 'type' and 'body_len' bytes of
+/* Creates an ofp_stats_reply with the given 'type' and 'body_len' bytes of
  * space allocated for the 'body' member.  Returns the first byte of the 'body'
  * member. */
 void *
@@ -1662,7 +1693,8 @@ ofputil_append_stats_reply(size_t len, struct list *replies)
     return ofpbuf_put_uninit(ofputil_reserve_stats_reply(len, replies), len);
 }
 
-/* Returns the first byte of the body of the ofp_stats_msg in 'oh'. */
+/* Returns the first byte of the 'body' member of the ofp_stats_request or
+ * ofp_stats_reply in 'oh'. */
 const void *
 ofputil_stats_body(const struct ofp_header *oh)
 {
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index eb5f232..eec864c 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -170,6 +170,17 @@ struct ofputil_flow_stats {
 int ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *,
                                     struct ofpbuf *msg);
 
+/* Aggregate stats reply, independent of flow format. */
+struct ofputil_aggregate_stats {
+    uint64_t packet_count;
+    uint64_t byte_count;
+    uint32_t flow_count;
+};
+
+struct ofpbuf *ofputil_encode_aggregate_stats_reply(
+    const struct ofputil_aggregate_stats *stats,
+    const struct ofp_stats_msg *request);
+
 /* Flow removed message, independent of flow format. */
 struct ofputil_flow_removed {
     struct cls_rule rule;
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 94145ef..08ece9a 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -49,7 +49,6 @@
 
 VLOG_DEFINE_THIS_MODULE(ofproto);
 
-COVERAGE_DEFINE(ofproto_agg_request);
 COVERAGE_DEFINE(ofproto_error);
 COVERAGE_DEFINE(ofproto_flows_req);
 COVERAGE_DEFINE(ofproto_flush);
@@ -1940,24 +1939,30 @@ ofproto_port_get_cfm_fault(const struct ofproto *ofproto, uint16_t ofp_port)
             : -1);
 }
 
-static void
-query_aggregate_stats(struct ofproto *ofproto, struct cls_rule *target,
-                      ovs_be16 out_port, uint8_t table_id,
-                      uint64_t *total_packetsp, uint64_t *total_bytesp,
-                      uint32_t *n_flowsp)
+static int
+handle_aggregate_stats_request(struct ofconn *ofconn,
+                               const struct ofp_stats_msg *osm)
 {
-    uint64_t total_packets = 0;
-    uint64_t total_bytes = 0;
+    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
+    struct flow_stats_request request;
+    struct ofputil_aggregate_stats stats;
     struct classifier *cls;
-    int n_flows = 0;
+    struct ofpbuf *reply;
+    ovs_be16 out_port;
+    int error;
 
-    COVERAGE_INC(ofproto_agg_request);
+    error = ofputil_decode_flow_stats_request(&request, &osm->header);
+    if (error) {
+        return error;
+    }
+    out_port = htons(request.out_port);
 
-    FOR_EACH_MATCHING_TABLE (cls, table_id, ofproto) {
+    memset(&stats, 0, sizeof stats);
+    FOR_EACH_MATCHING_TABLE (cls, request.table_id, ofproto) {
         struct cls_cursor cursor;
         struct rule *rule;
 
-        cls_cursor_init(&cursor, cls, target);
+        cls_cursor_init(&cursor, cls, &request.match);
         CLS_CURSOR_FOR_EACH (rule, cr, &cursor) {
             if (!rule_is_hidden(rule) && rule_has_out_port(rule, out_port)) {
                 uint64_t packet_count;
@@ -1966,85 +1971,15 @@ query_aggregate_stats(struct ofproto *ofproto, struct cls_rule *target,
                 ofproto->ofproto_class->rule_get_stats(rule, &packet_count,
                                                        &byte_count);
 
-                total_packets += packet_count;
-                total_bytes += byte_count;
-                n_flows++;
+                stats.packet_count += packet_count;
+                stats.byte_count += byte_count;
+                stats.flow_count++;
             }
         }
     }
 
-    *total_packetsp = total_packets;
-    *total_bytesp = total_bytes;
-    *n_flowsp = n_flows;
-}
-
-static int
-handle_aggregate_stats_request(struct ofconn *ofconn,
-                               const struct ofp_flow_stats_request *request)
-{
-    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
-    struct ofp_aggregate_stats_reply *reply;
-    uint64_t total_packets, total_bytes;
-    struct cls_rule target;
-    struct ofpbuf *msg;
-    uint32_t n_flows;
-
-    ofputil_cls_rule_from_match(&request->match, 0, &target);
-    query_aggregate_stats(ofproto, &target, request->out_port,
-                          request->table_id,
-                          &total_packets, &total_bytes, &n_flows);
-
-    reply = ofputil_make_stats_reply(sizeof *reply, &request->osm, &msg);
-    reply->flow_count = htonl(n_flows);
-    put_32aligned_be64(&reply->packet_count, htonll(total_packets));
-    put_32aligned_be64(&reply->byte_count, htonll(total_bytes));
-    memset(reply->pad, 0, sizeof reply->pad);
-
-    ofconn_send_reply(ofconn, msg);
-
-    return 0;
-}
-
-static int
-handle_nxst_aggregate(struct ofconn *ofconn,
-                      const struct nx_aggregate_stats_request *nasr)
-{
-    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
-    struct nx_aggregate_stats_request *request;
-    struct nx_aggregate_stats_reply *reply;
-    uint64_t total_packets, total_bytes;
-    struct cls_rule target;
-    struct ofpbuf b;
-    struct ofpbuf *buf;
-    uint32_t n_flows;
-    int error;
-
-    ofpbuf_use_const(&b, nasr, ntohs(nasr->nsm.vsm.osm.header.length));
-
-    /* Dissect the message. */
-    request = ofpbuf_pull(&b, sizeof *request);
-    error = nx_pull_match(&b, ntohs(request->match_len), 0, &target);
-    if (error) {
-        return error;
-    }
-    if (b.size) {
-        return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
-    }
-
-    /* Count statistics. */
-    query_aggregate_stats(ofproto, &target, request->out_port,
-                          request->table_id,
-                          &total_packets, &total_bytes, &n_flows);
-
-    /* Reply. */
-    COVERAGE_INC(ofproto_flows_req);
-    reply = ofputil_make_stats_reply(sizeof *reply, &request->nsm.vsm.osm,
-                                     &buf);
-    reply->flow_count = htonl(n_flows);
-    reply->packet_count = htonll(total_packets);
-    reply->byte_count = htonll(total_bytes);
-    memset(reply->pad, 0, sizeof reply->pad);
-    ofconn_send_reply(ofconn, buf);
+    reply = ofputil_encode_aggregate_stats_reply(&stats, osm);
+    ofconn_send_reply(ofconn, reply);
 
     return 0;
 }
@@ -2620,6 +2555,7 @@ handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg)
         return handle_flow_stats_request(ofconn, msg->data);
 
     case OFPUTIL_OFPST_AGGREGATE_REQUEST:
+    case OFPUTIL_NXST_AGGREGATE_REQUEST:
         return handle_aggregate_stats_request(ofconn, msg->data);
 
     case OFPUTIL_OFPST_TABLE_REQUEST:
@@ -2635,9 +2571,6 @@ handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg)
     case OFPUTIL_NXST_FLOW_REQUEST:
         return handle_nxst_flow(ofconn, msg->data);
 
-    case OFPUTIL_NXST_AGGREGATE_REQUEST:
-        return handle_nxst_aggregate(ofconn, msg->data);
-
     case OFPUTIL_INVALID:
     case OFPUTIL_OFPT_HELLO:
     case OFPUTIL_OFPT_ERROR:
-- 
1.7.4.4




More information about the dev mailing list