diff mbox

[ovs-dev] Extended statistics patch.

Message ID 20160506222923.GL13791@ovn.org
State Not Applicable
Headers show

Commit Message

Ben Pfaff May 6, 2016, 10:29 p.m. UTC 
On Thu, May 05, 2016 at 09:46:01AM +0100, mweglicx wrote:
> Implementation of new statistics extension for DPDK ports:
> - Add new counters definition to netdev struct and open flow,
>   based on RFC2819.
> - Initialize netdev statistics as "filtered out"
>   before passing it to particular netdev implementation
>   (because of that change, statistics which are not
>   collected are reported as filtered out, and some
>   unit tests were modified in this respect).
> - New statistics are retrieved using experimenter code and
>   are printed as a result to ofctl dump-ports.
> - New counters are available for OpenFlow 1.4+.
> - Add new vendor id: INTEL_VENDOR_ID.
> - New statistics are printed to output via ofctl only if those
>   are present in reply message.
> - Add new file header: include/openflow/intel-ext.h which
>   contains new statistics definition.
> - Extended statistics are implemented only for dpdk-physical
>   and dpdk-vhost port types.
> - Dpdk-physical implementation uses xstats to collect statistics.
> - Dpdk-vhost implements only part of statistics (RX packet sized
>   based counters).
> 
> Signed-off-by: Michal Weglicki <michalx.weglicki@intel.com>

This is nice, thank you.

In ofp-print.c, it appears that every call to print_port_stat_cond()
passes UINT64_MAX as 'filter' and 1 as 'more'.  I think that these
parameters could be omitted.  I removed them.

The tree had two different kinds of software network devices that
handled stats different: netdev-vport initializes the counters it
handles to 0 and left the rest as UINT64_MAX, whereas netdev-dummy
zeroed everything it didn't know.  Neither one is perfect but the former
seems a little better, so I changed netdev-dummy to work that way too.

I applied this to master, folding in the following incremental.

--8<--------------------------cut here-------------------------->8--
diff mbox

Patch

diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 5acb4e1..7f0ee76 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -1075,7 +1075,11 @@  netdev_dummy_get_stats(const struct netdev *netdev, struct netdev_stats *stats)
     struct netdev_dummy *dev = netdev_dummy_cast(netdev);
 
     ovs_mutex_lock(&dev->mutex);
-    *stats = dev->stats;
+    /* Passing only collected counters */
+    stats->tx_packets = dev->stats.tx_packets;
+    stats->tx_bytes = dev->stats.tx_bytes;
+    stats->rx_packets = dev->stats.rx_packets;
+    stats->rx_bytes = dev->stats.rx_bytes;
     ovs_mutex_unlock(&dev->mutex);
 
     return 0;
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 6cd136d..b21d76f 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1685,17 +1685,10 @@  print_port_stat(struct ds *string, const char *leader, uint64_t stat, int more)
 }
 
 static void
-print_port_stat_cond(struct ds *string, const char *leader, uint64_t stat,
-                     uint64_t filter, int more)
+print_port_stat_cond(struct ds *string, const char *leader, uint64_t stat)
 {
-    if (stat != filter) {
-        print_port_stat(string, leader, stat, more);
-    } else {
-        if (0 == more) {
-            /* Line will be terminated even if filtering
-             * condition isn't met. */
-            print_port_stat(string, "", stat, more);
-        }
+    if (stat != UINT64_MAX) {
+        ds_put_format(string, "%s%"PRIu64", ", leader, stat);
     }
 }
 
@@ -1769,29 +1762,29 @@  ofp_print_ofpst_port_reply(struct ds *string, const struct ofp_header *oh,
         ds_init(&string_ext_stats);
 
         print_port_stat_cond(&string_ext_stats, "1_to_64_packets=",
-                             ps.stats.rx_1_to_64_packets, UINT64_MAX, 1);
+                             ps.stats.rx_1_to_64_packets);
         print_port_stat_cond(&string_ext_stats, "65_to_127_packets=",
-                             ps.stats.rx_65_to_127_packets, UINT64_MAX, 1);
+                             ps.stats.rx_65_to_127_packets);
         print_port_stat_cond(&string_ext_stats, "128_to_255_packets=",
-                             ps.stats.rx_128_to_255_packets, UINT64_MAX, 1);
+                             ps.stats.rx_128_to_255_packets);
         print_port_stat_cond(&string_ext_stats, "256_to_511_packets=",
-                             ps.stats.rx_256_to_511_packets, UINT64_MAX, 1);
+                             ps.stats.rx_256_to_511_packets);
         print_port_stat_cond(&string_ext_stats, "512_to_1023_packets=",
-                             ps.stats.rx_512_to_1023_packets, UINT64_MAX, 1);
+                             ps.stats.rx_512_to_1023_packets);
         print_port_stat_cond(&string_ext_stats, "1024_to_1522_packets=",
-                             ps.stats.rx_1024_to_1522_packets, UINT64_MAX, 1);
+                             ps.stats.rx_1024_to_1522_packets);
         print_port_stat_cond(&string_ext_stats, "1523_to_max_packets=",
-                             ps.stats.rx_1523_to_max_packets, UINT64_MAX, 1);
+                             ps.stats.rx_1523_to_max_packets);
         print_port_stat_cond(&string_ext_stats, "broadcast_packets=",
-                             ps.stats.rx_broadcast_packets, UINT64_MAX, 1);
+                             ps.stats.rx_broadcast_packets);
         print_port_stat_cond(&string_ext_stats, "undersized_errors=",
-                             ps.stats.rx_undersized_errors, UINT64_MAX, 1);
+                             ps.stats.rx_undersized_errors);
         print_port_stat_cond(&string_ext_stats, "oversize_errors=",
-                             ps.stats.rx_oversize_errors, UINT64_MAX, 1);
+                             ps.stats.rx_oversize_errors);
         print_port_stat_cond(&string_ext_stats, "rx_fragmented_errors=",
-                             ps.stats.rx_fragmented_errors, UINT64_MAX, 1);
+                             ps.stats.rx_fragmented_errors);
         print_port_stat_cond(&string_ext_stats, "rx_jabber_errors=",
-                             ps.stats.rx_jabber_errors, UINT64_MAX, 1);
+                             ps.stats.rx_jabber_errors);
 
         if (string_ext_stats.length != 0) {
             /* If at least one statistics counter is reported: */
@@ -1805,23 +1798,23 @@  ofp_print_ofpst_port_reply(struct ds *string, const struct ofp_header *oh,
         ds_init(&string_ext_stats);
 
         print_port_stat_cond(&string_ext_stats, "1_to_64_packets=",
-                             ps.stats.tx_1_to_64_packets, UINT64_MAX, 1);
+                             ps.stats.tx_1_to_64_packets);
         print_port_stat_cond(&string_ext_stats, "65_to_127_packets=",
-                             ps.stats.tx_65_to_127_packets, UINT64_MAX, 1);
+                             ps.stats.tx_65_to_127_packets);
         print_port_stat_cond(&string_ext_stats, "128_to_255_packets=",
-                             ps.stats.tx_128_to_255_packets, UINT64_MAX, 1);
+                             ps.stats.tx_128_to_255_packets);
         print_port_stat_cond(&string_ext_stats, "256_to_511_packets=",
-                             ps.stats.tx_256_to_511_packets, UINT64_MAX, 1);
+                             ps.stats.tx_256_to_511_packets);
         print_port_stat_cond(&string_ext_stats, "512_to_1023_packets=",
-                             ps.stats.tx_512_to_1023_packets, UINT64_MAX, 1);
+                             ps.stats.tx_512_to_1023_packets);
         print_port_stat_cond(&string_ext_stats, "1024_to_1522_packets=",
-                             ps.stats.tx_1024_to_1522_packets, UINT64_MAX, 1);
+                             ps.stats.tx_1024_to_1522_packets);
         print_port_stat_cond(&string_ext_stats, "1523_to_max_packets=",
-                             ps.stats.tx_1523_to_max_packets, UINT64_MAX, 1);
+                             ps.stats.tx_1523_to_max_packets);
         print_port_stat_cond(&string_ext_stats, "multicast_packets=",
-                             ps.stats.tx_multicast_packets, UINT64_MAX, 1);
+                             ps.stats.tx_multicast_packets);
         print_port_stat_cond(&string_ext_stats, "broadcast_packets=",
-                             ps.stats.tx_broadcast_packets, UINT64_MAX, 1);
+                             ps.stats.tx_broadcast_packets);
 
         if (string_ext_stats.length != 0) {
             /* If at least one statistics counter is reported: */
diff --git a/tests/learn.at b/tests/learn.at
index 31ff4fd..68bde87 100644
--- a/tests/learn.at
+++ b/tests/learn.at
@@ -329,11 +329,11 @@  NXST_FLOW reply:
 AT_CHECK(
   [(ovs-ofctl dump-ports br0 2; ovs-ofctl dump-ports br0 3) | strip_xids], [0],
   [OFPST_PORT reply: 1 ports
-  port  2: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
-           tx pkts=1, bytes=60, drop=0, errs=0, coll=0
+  port  2: rx pkts=0, bytes=0, drop=?, errs=?, frame=?, over=?, crc=?
+           tx pkts=1, bytes=60, drop=?, errs=?, coll=?
 OFPST_PORT reply: 1 ports
-  port  3: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
-           tx pkts=9, bytes=540, drop=0, errs=0, coll=0
+  port  3: rx pkts=0, bytes=0, drop=?, errs=?, frame=?, over=?, crc=?
+           tx pkts=9, bytes=540, drop=?, errs=?, coll=?
 ])
 
 OVS_VSWITCHD_STOP
@@ -380,11 +380,11 @@  done
 AT_CHECK(
   [(ovs-ofctl dump-ports br0 2; ovs-ofctl dump-ports br0 3) | strip_xids], [0],
   [OFPST_PORT reply: 1 ports
-  port  2: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
-           tx pkts=2, bytes=120, drop=0, errs=0, coll=0
+  port  2: rx pkts=0, bytes=0, drop=?, errs=?, frame=?, over=?, crc=?
+           tx pkts=2, bytes=120, drop=?, errs=?, coll=?
 OFPST_PORT reply: 1 ports
-  port  3: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
-           tx pkts=18, bytes=1080, drop=0, errs=0, coll=0
+  port  3: rx pkts=0, bytes=0, drop=?, errs=?, frame=?, over=?, crc=?
+           tx pkts=18, bytes=1080, drop=?, errs=?, coll=?
 ])
 
 # Check for the learning entry.
@@ -466,11 +466,11 @@  done
 AT_CHECK(
   [(ovs-ofctl dump-ports br0 2; ovs-ofctl dump-ports br0 3) | strip_xids], [0],
   [OFPST_PORT reply: 1 ports
-  port  2: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
-           tx pkts=3, bytes=180, drop=0, errs=0, coll=0
+  port  2: rx pkts=0, bytes=0, drop=?, errs=?, frame=?, over=?, crc=?
+           tx pkts=3, bytes=180, drop=?, errs=?, coll=?
 OFPST_PORT reply: 1 ports
-  port  3: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
-           tx pkts=17, bytes=1020, drop=0, errs=0, coll=0
+  port  3: rx pkts=0, bytes=0, drop=?, errs=?, frame=?, over=?, crc=?
+           tx pkts=17, bytes=1020, drop=?, errs=?, coll=?
 ])
 
 # Check for the learning entry.
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index ea01a53..29d9369 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5163,17 +5163,17 @@  IFCOUNTERS
 	status=0
 	in_octets=0
 	in_unicasts=0
-	in_multicasts=0
+	in_multicasts=4294967295
 	in_broadcasts=4294967295
-	in_discards=0
-	in_errors=0
+	in_discards=4294967295
+	in_errors=4294967295
 	in_unknownprotos=4294967295
 	out_octets=120
 	out_unicasts=2
 	out_multicasts=4294967295
 	out_broadcasts=4294967295
-	out_discards=0
-	out_errors=0
+	out_discards=4294967295
+	out_errors=4294967295
 	promiscuous=0
 IFCOUNTERS
 	dgramSeqNo=2
@@ -5186,17 +5186,17 @@  IFCOUNTERS
 	status=0
 	in_octets=138
 	in_unicasts=3
-	in_multicasts=0
+	in_multicasts=4294967295
 	in_broadcasts=4294967295
-	in_discards=0
-	in_errors=0
+	in_discards=4294967295
+	in_errors=4294967295
 	in_unknownprotos=4294967295
 	out_octets=120
 	out_unicasts=2
 	out_multicasts=4294967295
 	out_broadcasts=4294967295
-	out_discards=0
-	out_errors=0
+	out_discards=4294967295
+	out_errors=4294967295
 	promiscuous=0
 IFCOUNTERS
 	dgramSeqNo=2
@@ -5209,17 +5209,17 @@  IFCOUNTERS
 	status=0
 	in_octets=84
 	in_unicasts=2
-	in_multicasts=0
+	in_multicasts=4294967295
 	in_broadcasts=4294967295
-	in_discards=0
-	in_errors=0
+	in_discards=4294967295
+	in_errors=4294967295
 	in_unknownprotos=4294967295
 	out_octets=180
 	out_unicasts=3
 	out_multicasts=4294967295
 	out_broadcasts=4294967295
-	out_discards=0
-	out_errors=0
+	out_discards=4294967295
+	out_errors=4294967295
 	promiscuous=0
 IFCOUNTERS
 	dgramSeqNo=3
@@ -5232,17 +5232,17 @@  IFCOUNTERS
 	status=0
 	in_octets=0
 	in_unicasts=0
-	in_multicasts=0
+	in_multicasts=4294967295
 	in_broadcasts=4294967295
-	in_discards=0
-	in_errors=0
+	in_discards=4294967295
+	in_errors=4294967295
 	in_unknownprotos=4294967295
 	out_octets=120
 	out_unicasts=2
 	out_multicasts=4294967295
 	out_broadcasts=4294967295
-	out_discards=0
-	out_errors=0
+	out_discards=4294967295
+	out_errors=4294967295
 	promiscuous=0
 IFCOUNTERS
 	dgramSeqNo=3
@@ -5255,17 +5255,17 @@  IFCOUNTERS
 	status=0
 	in_octets=138
 	in_unicasts=3
-	in_multicasts=0
+	in_multicasts=4294967295
 	in_broadcasts=4294967295
-	in_discards=0
-	in_errors=0
+	in_discards=4294967295
+	in_errors=4294967295
 	in_unknownprotos=4294967295
 	out_octets=120
 	out_unicasts=2
 	out_multicasts=4294967295
 	out_broadcasts=4294967295
-	out_discards=0
-	out_errors=0
+	out_discards=4294967295
+	out_errors=4294967295
 	promiscuous=0
 IFCOUNTERS
 	dgramSeqNo=3
@@ -5278,17 +5278,17 @@  IFCOUNTERS
 	status=0
 	in_octets=84
 	in_unicasts=2
-	in_multicasts=0
+	in_multicasts=4294967295
 	in_broadcasts=4294967295
-	in_discards=0
-	in_errors=0
+	in_discards=4294967295
+	in_errors=4294967295
 	in_unknownprotos=4294967295
 	out_octets=180
 	out_unicasts=3
 	out_multicasts=4294967295
 	out_broadcasts=4294967295
-	out_discards=0
-	out_errors=0
+	out_discards=4294967295
+	out_errors=4294967295
 	promiscuous=0
 OPENFLOWPORT
 	datapath_id=18364758544493064720
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 0f3cecf..f8c0d07 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -84,8 +84,8 @@  OVS_VSWITCHD_START
 AT_CHECK([ovs-ofctl -vwarn dump-ports br0], [0], [stdout])
 AT_CHECK([strip_xids < stdout], [0], [dnl
 OFPST_PORT reply: 1 ports
-  port LOCAL: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
-           tx pkts=0, bytes=0, drop=0, errs=0, coll=0
+  port LOCAL: rx pkts=0, bytes=0, drop=?, errs=?, frame=?, over=?, crc=?
+           tx pkts=0, bytes=0, drop=?, errs=?, coll=?
 ])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
@@ -95,8 +95,8 @@  OVS_VSWITCHD_START
 AT_CHECK([ovs-ofctl -O OpenFlow12 -vwarn dump-ports br0], [0], [stdout])
 AT_CHECK([strip_xids < stdout], [0], [dnl
 OFPST_PORT reply (OF1.2): 1 ports
-  port LOCAL: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
-           tx pkts=0, bytes=0, drop=0, errs=0, coll=0
+  port LOCAL: rx pkts=0, bytes=0, drop=?, errs=?, frame=?, over=?, crc=?
+           tx pkts=0, bytes=0, drop=?, errs=?, coll=?
 ])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
@@ -104,11 +104,11 @@  AT_CLEANUP
 AT_SETUP([ofproto - port stats - (OpenFlow 1.4)])
 OVS_VSWITCHD_START
 AT_CHECK([ovs-ofctl -O OpenFlow14 -vwarn dump-ports br0], [0], [stdout])
-AT_CHECK([strip_xids < stdout | sed 's/duration=[[0-9.]]*s/duration=?s/' | sed '/rfc2819/d'],
+AT_CHECK([strip_xids < stdout | sed 's/duration=[[0-9.]]*s/duration=?s/'],
   [0], [dnl
 OFPST_PORT reply (OF1.4): 1 ports
-  port LOCAL: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
-           tx pkts=0, bytes=0, drop=0, errs=0, coll=0
+  port LOCAL: rx pkts=0, bytes=0, drop=?, errs=?, frame=?, over=?, crc=?
+           tx pkts=0, bytes=0, drop=?, errs=?, coll=?
            duration=?s
 ])
 OVS_VSWITCHD_STOP