From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 8F1857F58 for ; Mon, 9 May 2016 19:56:57 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga104.fm.intel.com with ESMTP; 09 May 2016 10:56:57 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,601,1455004800"; d="scan'208";a="962090090" Received: from irsmsx153.ger.corp.intel.com ([163.33.192.75]) by fmsmga001.fm.intel.com with ESMTP; 09 May 2016 10:56:55 -0700 Received: from irsmsx102.ger.corp.intel.com ([169.254.2.35]) by IRSMSX153.ger.corp.intel.com ([169.254.9.140]) with mapi id 14.03.0248.002; Mon, 9 May 2016 18:56:54 +0100 From: "Van Haaren, Harry" To: Rasesh Mody , "dev@dpdk.org" CC: "Dept-EngDPDKDev@qlogic.com" Thread-Topic: [dpdk-dev] [PATCH 6/9] qede: add support for xstats Thread-Index: AQHRqBlnh606mzQr+0eWk7qJxIlw0J+w4xqA Date: Mon, 9 May 2016 17:56:54 +0000 Message-ID: References: <1462595421-22505-1-git-send-email-rasesh.mody@qlogic.com> <1462595421-22505-7-git-send-email-rasesh.mody@qlogic.com> In-Reply-To: <1462595421-22505-7-git-send-email-rasesh.mody@qlogic.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZmZjNzk3MDktYmQwZi00NjQzLTk5MWMtOGRlM2Q3Y2YwNWVmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IkZaZUJMUGxEa2VZMXprN2pHdmFsRjhQOVF6UWtNb0FwOFwvK0x6REhvaUlZPSJ9 x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 6/9] qede: add support for xstats X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 May 2016 17:56:58 -0000 Hi Rasesh, Some comments about xstats strings below, please refer to the docs here: http://dpdk.org/doc/guides/prog_guide/poll_mode_drv.html#extended-statistic= s-api Due to my familiarity with ixgbe, I will use it as an example: other PMDs s= hould be identical.=20 I understand these may seem trivial and pointless changes, but ensuring the= xstats strings are consistent is vital for apps to find the metadata conta= ined. Regards, -Harry > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rasesh Mody > Subject: [dpdk-dev] [PATCH 6/9] qede: add support for xstats >=20 > This patch adds support for extended statistics for QEDE PMD. > =09 > +static const struct rte_eth_xstats qede_eth_stats[] =3D { > + {"no_buff_discards", > + offsetof(struct ecore_eth_stats, no_buff_discards)}, Is provided by ethdev as rx_mbuf_allocation_errors http://dpdk.org/browse/dpdk/tree/lib/librte_ether/rte_ethdev.c#n93 > + {"packet_too_big_discard", > + offsetof(struct ecore_eth_stats, packet_too_big_discard)}, Is this not the same as rx_oversize below? It reads like it is - perhaps tr= y be more descriptive (provide a direction, and differentiate it from the c= ounter below)=20 > + {"ttl0_discard", offsetof(struct ecore_eth_stats, ttl0_discard)}, This needs a direction, "rx", I suggest "rx_ttl_zero_dropped"=20 > + {"rx_ucast_bytes", offsetof(struct ecore_eth_stats, rx_ucast_bytes)}, > + {"rx_mcast_bytes", offsetof(struct ecore_eth_stats, rx_mcast_bytes)}, > + {"rx_bcast_bytes", offsetof(struct ecore_eth_stats, rx_bcast_bytes)}, > + {"rx_ucast_pkts", offsetof(struct ecore_eth_stats, rx_ucast_pkts)}, > + {"rx_mcast_pkts", offsetof(struct ecore_eth_stats, rx_mcast_pkts)}, > + {"rx_bcast_pkts", offsetof(struct ecore_eth_stats, rx_bcast_pkts)}, expand these names - unicast, packets. Otherwise formatting is good. > + {"mftag_filter_discards", > + offsetof(struct ecore_eth_stats, mftag_filter_discards)}, > + {"mac_filter_discards", > + offsetof(struct ecore_eth_stats, mac_filter_discards)}, Direction, "mftag"? > + {"tx_ucast_bytes", offsetof(struct ecore_eth_stats, tx_ucast_bytes)}, > + {"tx_mcast_bytes", offsetof(struct ecore_eth_stats, tx_mcast_bytes)}, > + {"tx_bcast_bytes", offsetof(struct ecore_eth_stats, tx_bcast_bytes)}, > + {"tx_ucast_pkts", offsetof(struct ecore_eth_stats, tx_ucast_pkts)}, > + {"tx_mcast_pkts", offsetof(struct ecore_eth_stats, tx_mcast_pkts)}, > + {"tx_bcast_pkts", offsetof(struct ecore_eth_stats, tx_bcast_pkts)}, Same as RX (expand to unicast etc, packets) > + {"tx_err_drop_pkts", > + offsetof(struct ecore_eth_stats, tx_err_drop_pkts)}, > + {"tpa_coalesced_pkts", > + offsetof(struct ecore_eth_stats, tpa_coalesced_pkts)}, > + {"tpa_coalesced_events", > + offsetof(struct ecore_eth_stats, tpa_coalesced_events)}, > + {"tpa_aborts_num", offsetof(struct ecore_eth_stats, tpa_aborts_num)}, > + {"tpa_not_coalesced_pkts", > + offsetof(struct ecore_eth_stats, tpa_not_coalesced_pkts)}, > + {"tpa_coalesced_bytes", > + offsetof(struct ecore_eth_stats, tpa_coalesced_bytes)}, "tpa" should be human-readable, and these need a direction (unless this cou= nter explicitly has no affinity to rx/tx) > + {"rx_64_byte_packets", > + offsetof(struct ecore_eth_stats, rx_64_byte_packets)}, <> > + {"rx_9217_to_16383_byte_packets", offsetof(struct ecore_eth_stats, > + rx_9217_to_16383_byte_packets)}, Formatting not consistent, see docs linked at top. =09 > + {"rx_mac_crtl_frames", > + offsetof(struct ecore_eth_stats, rx_mac_crtl_frames)}, and=20 > + {"rx_prio_flow_ctrl_frames", > + offsetof(struct ecore_eth_stats, rx_pfc_frames)}, Expand crtl > + {"rx_oversize_packets", > + offsetof(struct ecore_eth_stats, rx_oversize_packets)}, Does this packet get received properly? If so, packets is the right counter= . Otherwise this should be errors: http://dpdk.org/browse/dpdk/tree/drivers/net/ixgbe/ixgbe_ethdev.c#n617 > + {"rx_jabbers", offsetof(struct ecore_eth_stats, rx_jabbers)}, rx_jabber_errors > + {"rx_undersize_packets", > + offsetof(struct ecore_eth_stats, rx_undersize_packets)}, Same as oversize. > + {"rx_fragments", offsetof(struct ecore_eth_stats, rx_fragments)}, rx_fragment_packets? http://dpdk.org/browse/dpdk/tree/drivers/net/ixgbe/ixgbe_ethdev.c#n615 > + {"tx_64_byte_packets", > + offsetof(struct ecore_eth_stats, tx_64_byte_packets)}, Same as RX size comment.