ofp-table: Better summarize table features and statistics.

Before this patch, most dump-table-stats outputs would contain about
250 lines of the form:

  table #: ditto

With this patch, they have one line like this:

  tables 2...254: ditto

which is much easier to read.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Justin Pettit <jpettit@ovn.org>

diff --git a/include/openvswitch/ofp-table.h b/include/openvswitch/ofp-table.h
index d06ccf5ce9e3be34cfacb42cb46182e0bb469736..7b1152a2871cf59794509efe739f1c2ecbb5be21 100644 (file)
--- a/include/openvswitch/ofp-table.h
+++ b/include/openvswitch/ofp-table.h
@@ -276,7 +276,10 @@ void ofputil_table_features_format(
     const struct ofputil_table_features *prev_features,
     const struct ofputil_table_stats *stats,
     const struct ofputil_table_stats *prev_stats,
-    const struct ofputil_table_map *table_map);
+    const struct ofputil_table_map *table_map,
+    int *first_ditto, int *last_ditto);
+void ofputil_table_features_format_finish(struct ds *,
+                                          int first_ditto, int last_ditto);
 
 /* Abstract table stats.
  *

diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index e05a969a82b0bc2d84beda6a0fb5b93b2cefeb69..10d6f5962ef12c32cfb76264faf28914d39ae7d7 100644 (file)
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -232,18 +232,19 @@ ofp_print_table_features_reply(struct ds *s, const struct ofp_header *oh,
     struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length));
 
     struct ofputil_table_features prev;
+    int first_ditto = -1, last_ditto = -1;
     for (int i = 0; ; i++) {
         struct ofputil_table_features tf;
         int retval;
 
         retval = ofputil_decode_table_features(&b, &tf, true);
         if (retval) {
+            ofputil_table_features_format_finish(s, first_ditto, last_ditto);
             return retval != EOF ? retval : 0;
         }
 
-        ds_put_char(s, '\n');
         ofputil_table_features_format(s, &tf, i ? &prev : NULL, NULL, NULL,
-                                      table_map);
+                                      table_map, &first_ditto, &last_ditto);
         prev = tf;
     }
 }
@@ -574,6 +575,7 @@ ofp_print_table_stats_reply(struct ds *string, const struct ofp_header *oh,
 
     struct ofputil_table_features prev_features;
     struct ofputil_table_stats prev_stats;
+    int first_ditto = -1, last_ditto = -1;
     for (int i = 0;; i++) {
         struct ofputil_table_features features;
         struct ofputil_table_stats stats;
@@ -581,14 +583,15 @@ ofp_print_table_stats_reply(struct ds *string, const struct ofp_header *oh,
 
         retval = ofputil_decode_table_stats_reply(&b, &stats, &features);
         if (retval) {
+            ofputil_table_features_format_finish(string,
+                                                 first_ditto, last_ditto);
             return retval != EOF ? retval : 0;
         }
 
-        ds_put_char(string, '\n');
         ofputil_table_features_format(string,
                                       &features, i ? &prev_features : NULL,
                                       &stats, i ? &prev_stats : NULL,
-                                      table_map);
+                                      table_map, &first_ditto, &last_ditto);
         prev_features = features;
         prev_stats = stats;
     }

diff --git a/lib/ofp-table.c b/lib/ofp-table.c
index 5f14fcc3a9f393c5dd5b461d04b6b1886ba546d0..71197f6442501743b505b64da67098b6f0deda95 100644 (file)
--- a/lib/ofp-table.c
+++ b/lib/ofp-table.c
@@ -1395,21 +1395,31 @@ ofputil_table_features_format(
     const struct ofputil_table_features *prev_features,
     const struct ofputil_table_stats *stats,
     const struct ofputil_table_stats *prev_stats,
-    const struct ofputil_table_map *table_map)
+    const struct ofputil_table_map *table_map,
+    int *first_ditto, int *last_ditto)
 {
-    int i;
+    bool same_stats = !stats || (prev_stats
+                                 && table_stats_equal(stats, prev_stats));
+    bool same_features = prev_features && table_features_equal(features,
+                                                               prev_features);
+    if (same_stats && same_features && !features->name[0]) {
+        if (*first_ditto < 0) {
+            *first_ditto = features->table_id;
+        }
+        *last_ditto = features->table_id;
+        return;
+    }
+    ofputil_table_features_format_finish(s, *first_ditto, *last_ditto);
+    *first_ditto = -1;
 
-    ds_put_format(s, "  table ");
+    ds_put_format(s, "\n  table ");
     ofputil_format_table(features->table_id, table_map, s);
     if (features->name[0]) {
         ds_put_format(s, " (\"%s\")", features->name);
     }
     ds_put_char(s, ':');
 
-    bool same_stats = prev_stats && table_stats_equal(stats, prev_stats);
-    bool same_features = prev_features && table_features_equal(features,
-                                                               prev_features);
-    if ((!stats || same_stats) && same_features) {
+    if (same_stats && same_features) {
         ds_put_cstr(s, " ditto");
         return;
     }
@@ -1479,6 +1489,8 @@ ofputil_table_features_format(
             ds_put_cstr(s, "    (same matching)\n");
         } else {
             ds_put_cstr(s, "    matching:\n");
+
+            int i;
             BITMAP_FOR_EACH_1 (i, MFF_N_IDS, features->match.bm) {
                 const struct mf_field *f = mf_from_id(i);
                 bool mask = bitmap_is_set(features->mask.bm, i);
@@ -1493,6 +1505,22 @@ ofputil_table_features_format(
         }
     }
 }
+
+void
+ofputil_table_features_format_finish(struct ds *s,
+                                     int first_ditto, int last_ditto)
+{
+    if (first_ditto < 0) {
+        return;
+    }
+
+    ds_put_char(s, '\n');
+    if (first_ditto == last_ditto) {
+        ds_put_format(s, "  table %d: ditto\n", first_ditto);
+    } else {
+        ds_put_format(s, "  tables %d...%d: ditto\n", first_ditto, last_ditto);
+    }
+}
 \f
 /* Table stats. */
 

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index c1be55b8f75f1412a19070bb844ca8cd81829f72..4f7e47e192d86f89b81fc8c5487efb2ed3ba6d97 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -8981,17 +8981,16 @@ AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
 NXST_FLOW reply:
 ])
 
-(echo "OFPST_TABLE reply (OF1.3) (xid=0x2):
+AT_CHECK([ovs-ofctl -O OpenFlow13 dump-tables br0], [0],
+  [OFPST_TABLE reply (OF1.3) (xid=0x2):
   table 0:
     active=1, lookup=0, matched=0
 
   table 1:
     active=0, lookup=0, matched=0
-"
- for i in `seq 2 253`; do
-     printf '  table %d: ditto\n' $i
- done) > expout
-AT_CHECK([ovs-ofctl -O OpenFlow13 dump-tables br0 ], [0], [expout])
+
+  tables 2...253: ditto
+])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
@@ -9027,26 +9026,24 @@ NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=14 in_port=1 (via no_match) data_l
 vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,dl_type=0x1234
 ])
 
-(echo "OFPST_TABLE reply (OF1.3) (xid=0x2):
+AT_CHECK([ovs-ofctl -O OpenFlow13 dump-tables br0], [0], [dnl
+OFPST_TABLE reply (OF1.3) (xid=0x2):
   table 0:
     active=0, lookup=0, matched=0
-"
- for i in `seq 1 253`; do
-     printf '  table %d: ditto\n' $i
- done) > expout
-AT_CHECK([ovs-ofctl -O OpenFlow13 dump-tables br0 ], [0], [expout])
 
-(echo "OFPST_TABLE reply (OF1.3) (xid=0x2):
+  tables 1...253: ditto
+])
+
+AT_CHECK([ovs-ofctl -O OpenFlow13 dump-tables br1], [0], [dnl
+OFPST_TABLE reply (OF1.3) (xid=0x2):
   table 0:
     active=0, lookup=3, matched=0
 
   table 1:
     active=0, lookup=0, matched=0
-"
- for i in `seq 2 253`; do
-     printf '  table %d: ditto\n' $i
- done) > expout
-AT_CHECK([ovs-ofctl -O OpenFlow13 dump-tables br1 ], [0], [expout])
+
+  tables 2...253: ditto
+])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
@@ -9138,18 +9135,18 @@ AT_CHECK([ovs-ofctl -O OpenFlow13 dump-flows br0 | ofctl_strip | sort], [0], [dn
 OFPST_FLOW reply (OF1.3):
 ])
 
-(echo "OFPST_TABLE reply (OF1.3) (xid=0x2):
+AT_CHECK([ovs-ofctl -O OpenFlow13 dump-tables br0], [0], [dnl
+OFPST_TABLE reply (OF1.3) (xid=0x2):
   table 0:
     active=1, lookup=3, matched=3
 
   table 1: ditto
+
   table 2:
     active=0, lookup=0, matched=0
-"
- for i in `seq 3 253`; do
-     printf '  table %d: ditto\n' $i
- done) > expout
-AT_CHECK([ovs-ofctl -O OpenFlow13 dump-tables br0 ], [0], [expout])
+
+  tables 3...253: ditto
+])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
@@ -9188,7 +9185,8 @@ AT_CHECK([ovs-ofctl -O OpenFlow11 dump-flows br0 | ofctl_strip | sort], [0], [dn
 OFPST_FLOW reply (OF1.1):
 ])
 
-(echo "OFPST_TABLE reply (OF1.3) (xid=0x2):
+AT_CHECK([ovs-ofctl -O OpenFlow13 dump-tables br0], [0], [dnl
+OFPST_TABLE reply (OF1.3) (xid=0x2):
   table 0:
     active=0, lookup=3, matched=0
 
@@ -9197,11 +9195,9 @@ OFPST_FLOW reply (OF1.1):
 
   table 2:
     active=0, lookup=0, matched=0
-"
- for i in `seq 3 253`; do
-     printf '  table %d: ditto\n' $i
- done) > expout
-AT_CHECK([ovs-ofctl -O OpenFlow13 dump-tables br0 ], [0], [expout])
+
+  tables 3...253: ditto
+])
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP

diff --git a/tests/ofproto.at b/tests/ofproto.at
index 046187f25d66fe0b8456ea1fdf22bd5e055c0020..16700223d471e8295a1dbcdb76772eced35c832e 100644 (file)
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -2257,12 +2257,7 @@ head_table() {
 
 ' "$1"
 }
-ditto() {
-    for i in `seq $1 $2`; do
-        printf '  table %d: ditto\n' $i
-    done
-}
-(head_table; ditto 1 253) > expout
+(head_table; echo '  tables 1...253: ditto') > expout
 AT_CHECK([ovs-ofctl dump-tables br0], [0], [expout])
 # Change the configuration.
 AT_CHECK(
@@ -2284,7 +2279,7 @@ AT_CHECK(
     active=0, lookup=0, matched=0
     max_entries=1000000
     (same matching)
-'; ditto 3 253) > expout
+'; echo '  tables 3...253: ditto') > expout
 AT_CHECK([ovs-ofctl dump-tables br0], [0], [expout])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
@@ -2333,12 +2328,7 @@ head_table() {
 
 '
 }
-ditto() {
-    for i in `seq $1 $2`; do
-        printf '  table %d: ditto\n' $i
-    done
-}
-(head_table; ditto 1 253) > expout
+(head_table; echo '  tables 1...253: ditto') > expout
 AT_CHECK([ovs-ofctl dump-tables br0 | strip_xids], [0], [expout])
 OVS_VSWITCHD_STOP(["/240\.0\.0\.1/d"])
 AT_CLEANUP
@@ -2397,11 +2387,6 @@ head_table() {
 
 ' "$1"
 }
-ditto() {
-    for i in `seq $1 $2`; do
-        printf '  table %d: ditto\n' $i
-    done
-}
 tail_table() {
     printf '  table 253:
     active=0, lookup=0, matched=0
@@ -2414,7 +2399,7 @@ tail_table() {
     (same matching)
 '
 }
-(head_table; ditto 1 252; tail_table) > expout
+(head_table; printf '  tables 1...252: ditto\n\n'; tail_table) > expout
 AT_CHECK([ovs-ofctl -O OpenFlow12 dump-tables br0], [0], [expout])
 # Change the configuration.
 AT_CHECK(
@@ -2442,7 +2427,7 @@ AT_CHECK(
     max_entries=1000000
     (same instructions)
     (same matching)
-'; ditto 3 252; tail_table) > expout
+'; printf '  tables 3...252: ditto\n\n'; tail_table) > expout
 AT_CHECK([ovs-ofctl -O OpenFlow12 dump-tables br0], [0], [expout])
 OVS_VSWITCHD_STOP
 AT_CLEANUP

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index aa8f6971cbe4765b102c3de21c7f400d2cb2c096..0c2dce7e7d303a833052349bc2223610fd93e43f 100644 (file)
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -888,6 +888,8 @@ ofctl_dump_table_features(struct ovs_cmdl_context *ctx)
     bool done = false;
 
     struct ofputil_table_features prev;
+    int first_ditto = -1, last_ditto = -1;
+    struct ds s = DS_EMPTY_INITIALIZER;
     int n = 0;
 
     send_openflow_buffer(vconn, request);
@@ -922,12 +924,10 @@ ofctl_dump_table_features(struct ovs_cmdl_context *ctx)
                         break;
                     }
 
-                    struct ds s = DS_EMPTY_INITIALIZER;
                     ofputil_table_features_format(
                         &s, &tf, n ? &prev : NULL, NULL, NULL,
-                        tables_to_show(ctx->argv[1]));
-                    puts(ds_cstr(&s));
-                    ds_destroy(&s);
+                        tables_to_show(ctx->argv[1]),
+                        &first_ditto, &last_ditto);
 
                     prev = tf;
                     n++;
@@ -946,6 +946,11 @@ ofctl_dump_table_features(struct ovs_cmdl_context *ctx)
         ofpbuf_delete(reply);
     }
 
+    ofputil_table_features_format_finish(&s, first_ditto, last_ditto);
+    const char *p = ds_cstr(&s);
+    puts(p + (*p == '\n'));
+    ds_destroy(&s);
+
     vconn_close(vconn);
 }