diff mbox

[ovs-dev,5/9] ovs-ofctl: Merge dump_stats_transaction() into dump_transaction().

Message ID 1451423377-3805-5-git-send-email-blp@ovn.org
State Deferred
Headers show

Commit Message

Ben Pfaff Dec. 29, 2015, 9:09 p.m. UTC
The callers call dump_stats_transaction() for OFPST_* messages and
dump_transaction() for other messages, but the callee can easily
distinguish the two types, so this commit eliminates the difference for the
callers to simplify use.

This will be more valuable in an upcoming commit in which a single
ofputil_encode_*() function can produce an OFPT_* request for some
OpenFlow versions and an OFPST_* request for others.  (Specifically, OF1.4
changes OFPT_QUEUE_GET_CONFIG_REQUEST into OFPST_QUEUE_DESC.)

Also merges dump_trivial_stats_transaction() into
dump_trivial_transaction() for the same reason.

Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 utilities/ovs-ofctl.c | 123 ++++++++++++++++++++++----------------------------
 1 file changed, 54 insertions(+), 69 deletions(-)
diff mbox

Patch

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 0d57f85..e56f372 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -532,78 +532,64 @@  send_openflow_buffer(struct vconn *vconn, struct ofpbuf *buffer)
 static void
 dump_transaction(struct vconn *vconn, struct ofpbuf *request)
 {
-    struct ofpbuf *reply;
-
-    run(vconn_transact(vconn, request, &reply), "talking to %s",
-        vconn_get_name(vconn));
-    ofp_print(stdout, reply->data, reply->size, verbosity + 1);
-    ofpbuf_delete(reply);
-}
-
-static void
-dump_trivial_transaction(const char *vconn_name, enum ofpraw raw)
-{
-    struct ofpbuf *request;
-    struct vconn *vconn;
-
-    open_vconn(vconn_name, &vconn);
-    request = ofpraw_alloc(raw, vconn_get_version(vconn), 0);
-    dump_transaction(vconn, request);
-    vconn_close(vconn);
-}
+    const struct ofp_header *oh = request->data;
+    if (ofpmsg_is_stat_request(oh)) {
+        ovs_be32 send_xid = oh->xid;
+        enum ofpraw request_raw;
+        enum ofpraw reply_raw;
+        bool done = false;
 
-static void
-dump_stats_transaction(struct vconn *vconn, struct ofpbuf *request)
-{
-    const struct ofp_header *request_oh = request->data;
-    ovs_be32 send_xid = request_oh->xid;
-    enum ofpraw request_raw;
-    enum ofpraw reply_raw;
-    bool done = false;
-
-    ofpraw_decode_partial(&request_raw, request->data, request->size);
-    reply_raw = ofpraw_stats_request_to_reply(request_raw,
-                                              request_oh->version);
+        ofpraw_decode_partial(&request_raw, request->data, request->size);
+        reply_raw = ofpraw_stats_request_to_reply(request_raw, oh->version);
 
-    send_openflow_buffer(vconn, request);
-    while (!done) {
-        ovs_be32 recv_xid;
-        struct ofpbuf *reply;
+        send_openflow_buffer(vconn, request);
+        while (!done) {
+            ovs_be32 recv_xid;
+            struct ofpbuf *reply;
 
-        run(vconn_recv_block(vconn, &reply), "OpenFlow packet receive failed");
-        recv_xid = ((struct ofp_header *) reply->data)->xid;
-        if (send_xid == recv_xid) {
-            enum ofpraw raw;
+            run(vconn_recv_block(vconn, &reply),
+                "OpenFlow packet receive failed");
+            recv_xid = ((struct ofp_header *) reply->data)->xid;
+            if (send_xid == recv_xid) {
+                enum ofpraw raw;
 
-            ofp_print(stdout, reply->data, reply->size, verbosity + 1);
+                ofp_print(stdout, reply->data, reply->size, verbosity + 1);
 
-            ofpraw_decode(&raw, reply->data);
-            if (ofptype_from_ofpraw(raw) == OFPTYPE_ERROR) {
-                done = true;
-            } else if (raw == reply_raw) {
-                done = !ofpmp_more(reply->data);
+                ofpraw_decode(&raw, reply->data);
+                if (ofptype_from_ofpraw(raw) == OFPTYPE_ERROR) {
+                    done = true;
+                } else if (raw == reply_raw) {
+                    done = !ofpmp_more(reply->data);
+                } else {
+                    ovs_fatal(0, "received bad reply: %s",
+                              ofp_to_string(reply->data, reply->size,
+                                            verbosity + 1));
+                }
             } else {
-                ovs_fatal(0, "received bad reply: %s",
-                          ofp_to_string(reply->data, reply->size,
-                                        verbosity + 1));
+                VLOG_DBG("received reply with xid %08"PRIx32" "
+                         "!= expected %08"PRIx32, recv_xid, send_xid);
             }
-        } else {
-            VLOG_DBG("received reply with xid %08"PRIx32" "
-                     "!= expected %08"PRIx32, recv_xid, send_xid);
+            ofpbuf_delete(reply);
         }
+    } else {
+        struct ofpbuf *reply;
+
+        run(vconn_transact(vconn, request, &reply), "talking to %s",
+            vconn_get_name(vconn));
+        ofp_print(stdout, reply->data, reply->size, verbosity + 1);
         ofpbuf_delete(reply);
     }
 }
 
 static void
-dump_trivial_stats_transaction(const char *vconn_name, enum ofpraw raw)
+dump_trivial_transaction(const char *vconn_name, enum ofpraw raw)
 {
     struct ofpbuf *request;
     struct vconn *vconn;
 
     open_vconn(vconn_name, &vconn);
     request = ofpraw_alloc(raw, vconn_get_version(vconn), 0);
-    dump_stats_transaction(vconn, request);
+    dump_transaction(vconn, request);
     vconn_close(vconn);
 }
 
@@ -710,7 +696,7 @@  ofctl_show(struct ovs_cmdl_context *ctx)
 
     if (!has_ports) {
         request = ofputil_encode_port_desc_stats_request(version, OFPP_ANY);
-        dump_stats_transaction(vconn, request);
+        dump_transaction(vconn, request);
     }
     dump_trivial_transaction(vconn_name, OFPRAW_OFPT_GET_CONFIG_REQUEST);
     vconn_close(vconn);
@@ -719,13 +705,13 @@  ofctl_show(struct ovs_cmdl_context *ctx)
 static void
 ofctl_dump_desc(struct ovs_cmdl_context *ctx)
 {
-    dump_trivial_stats_transaction(ctx->argv[1], OFPRAW_OFPST_DESC_REQUEST);
+    dump_trivial_transaction(ctx->argv[1], OFPRAW_OFPST_DESC_REQUEST);
 }
 
 static void
 ofctl_dump_tables(struct ovs_cmdl_context *ctx)
 {
-    dump_trivial_stats_transaction(ctx->argv[1], OFPRAW_OFPST_TABLE_REQUEST);
+    dump_trivial_transaction(ctx->argv[1], OFPRAW_OFPST_TABLE_REQUEST);
 }
 
 static void
@@ -737,7 +723,7 @@  ofctl_dump_table_features(struct ovs_cmdl_context *ctx)
     open_vconn(ctx->argv[1], &vconn);
     request = ofputil_encode_table_features_request(vconn_get_version(vconn));
 
-    /* The following is similar to dump_trivial_stats_transaction(), but it
+    /* The following is similar to dump_trivial_transaction(), but it
      * maintains the previous 'ofputil_table_features' from one stats reply
      * message to the next, which allows duplication to be eliminated in the
      * output across messages.  Otherwise the output is much larger and harder
@@ -816,7 +802,7 @@  ofctl_dump_table_desc(struct ovs_cmdl_context *ctx)
     open_vconn(ctx->argv[1], &vconn);
     request = ofputil_encode_table_desc_request(vconn_get_version(vconn));
     if (request) {
-        dump_stats_transaction(vconn, request);
+        dump_transaction(vconn, request);
     }
 
     vconn_close(vconn);
@@ -1095,7 +1081,7 @@  ofctl_dump_flows__(int argc, char *argv[], bool aggregate)
     struct vconn *vconn;
 
     vconn = prepare_dump_flows(argc, argv, aggregate, &request);
-    dump_stats_transaction(vconn, request);
+    dump_transaction(vconn, request);
     vconn_close(vconn);
 }
 
@@ -1231,7 +1217,7 @@  ofctl_queue_stats(struct ovs_cmdl_context *ctx)
     }
 
     request = ofputil_encode_queue_stats_request(vconn_get_version(vconn), &oqs);
-    dump_stats_transaction(vconn, request);
+    dump_transaction(vconn, request);
     vconn_close(vconn);
 }
 
@@ -1732,7 +1718,7 @@  ofctl_monitor(struct ovs_cmdl_context *ctx)
 
             msg = ofpbuf_new(0);
             ofputil_append_flow_monitor_request(&fmr, msg);
-            dump_stats_transaction(vconn, msg);
+            dump_transaction(vconn, msg);
             fflush(stdout);
         } else {
             ovs_fatal(0, "%s: unsupported \"monitor\" argument", arg);
@@ -1795,7 +1781,7 @@  ofctl_dump_ports(struct ovs_cmdl_context *ctx)
     open_vconn(ctx->argv[1], &vconn);
     port = ctx->argc > 2 ? str_to_port_no(ctx->argv[1], ctx->argv[2]) : OFPP_ANY;
     request = ofputil_encode_dump_ports_request(vconn_get_version(vconn), port);
-    dump_stats_transaction(vconn, request);
+    dump_transaction(vconn, request);
     vconn_close(vconn);
 }
 
@@ -1810,7 +1796,7 @@  ofctl_dump_ports_desc(struct ovs_cmdl_context *ctx)
     port = ctx->argc > 2 ? str_to_port_no(ctx->argv[1], ctx->argv[2]) : OFPP_ANY;
     request = ofputil_encode_port_desc_stats_request(vconn_get_version(vconn),
                                                      port);
-    dump_stats_transaction(vconn, request);
+    dump_transaction(vconn, request);
     vconn_close(vconn);
 }
 
@@ -2453,7 +2439,7 @@  ofctl_dump_group_stats(struct ovs_cmdl_context *ctx)
     request = ofputil_encode_group_stats_request(vconn_get_version(vconn),
                                                  group_id);
     if (request) {
-        dump_stats_transaction(vconn, request);
+        dump_transaction(vconn, request);
     }
 
     vconn_close(vconn);
@@ -2475,7 +2461,7 @@  ofctl_dump_group_desc(struct ovs_cmdl_context *ctx)
     request = ofputil_encode_group_desc_request(vconn_get_version(vconn),
                                                 group_id);
     if (request) {
-        dump_stats_transaction(vconn, request);
+        dump_transaction(vconn, request);
     }
 
     vconn_close(vconn);
@@ -2490,7 +2476,7 @@  ofctl_dump_group_features(struct ovs_cmdl_context *ctx)
     open_vconn(ctx->argv[1], &vconn);
     request = ofputil_encode_group_features_request(vconn_get_version(vconn));
     if (request) {
-        dump_stats_transaction(vconn, request);
+        dump_transaction(vconn, request);
     }
 
     vconn_close(vconn);
@@ -3104,9 +3090,8 @@  ofctl_meter_request__(const char *bridge, const char *str,
 
     protocol = open_vconn_for_flow_mod(bridge, &vconn, usable_protocols);
     version = ofputil_protocol_to_ofp_version(protocol);
-    dump_stats_transaction(vconn,
-                           ofputil_encode_meter_request(version, type,
-                                                        mm.meter.meter_id));
+    dump_transaction(vconn, ofputil_encode_meter_request(version, type,
+                                                         mm.meter.meter_id));
     vconn_close(vconn);
 }