From patchwork Fri May 6 22:29:23 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 619496 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (archives.nicira.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 3r1mfn0B3Kz9t3w for ; Sat, 7 May 2016 08:29:37 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 154DD22C3D6; Fri, 6 May 2016 15:29:35 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx1e3.cudamail.com (mx1.cudamail.com [69.90.118.67]) by archives.nicira.com (Postfix) with ESMTPS id B0CC3109E8 for ; Fri, 6 May 2016 15:29:33 -0700 (PDT) Received: from bar5.cudamail.com (localhost [127.0.0.1]) by mx1e3.cudamail.com (Postfix) with ESMTPS id DA2A34203FE for ; Fri, 6 May 2016 16:29:32 -0600 (MDT) X-ASG-Debug-ID: 1462573771-09eadd4da866160001-byXFYA Received: from mx3-pf1.cudamail.com ([192.168.14.2]) by bar5.cudamail.com with ESMTP id tubd46YtDLjfSEBK (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 06 May 2016 16:29:31 -0600 (MDT) X-Barracuda-Envelope-From: blp@ovn.org X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.2 Received: from unknown (HELO relay4-d.mail.gandi.net) (217.70.183.196) by mx3-pf1.cudamail.com with ESMTPS (DHE-RSA-AES256-SHA encrypted); 6 May 2016 22:29:30 -0000 Received-SPF: pass (mx3-pf1.cudamail.com: SPF record at ovn.org designates 217.70.183.196 as permitted sender) X-Barracuda-Apparent-Source-IP: 217.70.183.196 X-Barracuda-RBL-IP: 217.70.183.196 Received: from mfilter30-d.gandi.net (mfilter30-d.gandi.net [217.70.178.161]) by relay4-d.mail.gandi.net (Postfix) with ESMTP id 5D1D7172098; Sat, 7 May 2016 00:29:29 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at mfilter30-d.gandi.net Received: from relay4-d.mail.gandi.net ([IPv6:::ffff:217.70.183.196]) by mfilter30-d.gandi.net (mfilter30-d.gandi.net [::ffff:10.0.15.180]) (amavisd-new, port 10024) with ESMTP id FRGw4TZT1UuR; Sat, 7 May 2016 00:29:27 +0200 (CEST) X-Originating-IP: 173.228.112.241 Received: from ovn.org (173-228-112-241.dsl.dynamic.fusionbroadband.com [173.228.112.241]) (Authenticated sender: blp@ovn.org) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 530CA17209A; Sat, 7 May 2016 00:29:25 +0200 (CEST) Date: Fri, 6 May 2016 15:29:23 -0700 X-CudaMail-Envelope-Sender: blp@ovn.org From: Ben Pfaff To: mweglicx X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-V1-505053148 X-CudaMail-DTE: 050616 X-CudaMail-Originating-IP: 217.70.183.196 Message-ID: <20160506222923.GL13791@ovn.org> X-ASG-Orig-Subj: [##CM-V1-505053148##]Re: [ovs-dev] [PATCH] Extended statistics patch. References: <1462437961-16621-1-git-send-email-michalx.weglicki@intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1462437961-16621-1-git-send-email-michalx.weglicki@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Barracuda-Connect: UNKNOWN[192.168.14.2] X-Barracuda-Start-Time: 1462573771 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-ASG-Whitelist: Header =?UTF-8?B?eFwtY3VkYW1haWxcLXdoaXRlbGlzdFwtdG8=?= X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 Cc: dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH] Extended statistics patch. X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@openvswitch.org Sender: "dev" 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 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 --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