From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rcdn-iport-6.cisco.com (rcdn-iport-6.cisco.com [173.37.86.77]) by dpdk.org (Postfix) with ESMTP id AD933689B for ; Thu, 12 Jan 2017 10:57:11 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=21127; q=dns/txt; s=iport; t=1484215031; x=1485424631; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=Dt06fM926lYxfBbsn8jMb4OJjjR0Z4KOqIIfMuOANnE=; b=c8Ik2why2dPDXgTM+TlobS62d9TPMc2avA0xBFZ+roGflOti5GzTCoqx SMJVInZd0QLKKEyr0DEsyEq1UVsrT2oV7xba0wjsjS0tVeM42i1nD5wAd mLm3tFI0oZB0MuBrP/eUzqMX2CQsHxG1/Y8EcBsDTnt/pPWzjPUHSDDo6 s=; X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0ATAQCOUndY/5ldJa1dGQEBAQEBAQEBA?= =?us-ascii?q?QEBBwEBAQEBgzsBAQEBAR+BbAeNUZITkxmCD4INgmyDNgKCDz8UAQIBAQEBAQE?= =?us-ascii?q?BYyiEaQEBAQQnEz8MBAIBCBEEAQEBHgkHMhQJCAIEAQ0FCIh4sig6ihIBAQEBA?= =?us-ascii?q?QEBAQEBAQEBAQEBAQEBAQEdhkWEYYosBY8cjA4BkUyCAIUMg02GFZJjAR84gUQ?= =?us-ascii?q?VhG4cgV9zh1kBgQwBAQE?= X-IronPort-AV: E=Sophos;i="5.33,349,1477958400"; d="scan'208";a="193817813" Received: from rcdn-core-2.cisco.com ([173.37.93.153]) by rcdn-iport-6.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Jan 2017 09:56:54 +0000 Received: from XCH-RTP-016.cisco.com (xch-rtp-016.cisco.com [64.101.220.156]) by rcdn-core-2.cisco.com (8.14.5/8.14.5) with ESMTP id v0C9urOT029964 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Thu, 12 Jan 2017 09:56:54 GMT Received: from xch-rtp-017.cisco.com (64.101.220.157) by XCH-RTP-016.cisco.com (64.101.220.156) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Thu, 12 Jan 2017 04:56:53 -0500 Received: from xch-rtp-017.cisco.com ([64.101.220.157]) by XCH-RTP-017.cisco.com ([64.101.220.157]) with mapi id 15.00.1210.000; Thu, 12 Jan 2017 04:56:53 -0500 From: "Hanoch Haim (hhaim)" To: Adrien Mazarguil , Shahaf Shuler CC: "dev@dpdk.org" , Elad Persiko Thread-Topic: [PATCH] net/mlx5: support extended statistics Thread-Index: AQHSa+iC+nWZygbbpkmVcDbf9ys/jqEz0tQAgADIx4A= Date: Thu, 12 Jan 2017 09:56:52 +0000 Message-ID: References: <1484124942-26279-1-git-send-email-shahafs@mellanox.com> <20170111165446.GP12822@6wind.com> In-Reply-To: <20170111165446.GP12822@6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [64.103.125.60] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] net/mlx5: support extended statistics X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Jan 2017 09:57:12 -0000 Hi Shahaf,=20 1) I would add *all* the hw counters to PF $ethtool -S enp135s0f0 NIC statistics: rx_packets: 54 rx_bytes: 3240 tx_packets: 138 tx_bytes: 8280 tx_tso_packets: 0 tx_tso_bytes: 0 tx_tso_inner_packets: 0 tx_tso_inner_bytes: 0 rx_lro_packets: 0 rx_lro_bytes: 0 rx_csum_unnecessary: 0 rx_csum_none: 54 rx_csum_complete: 0 rx_csum_unnecessary_inner: 0 tx_csum_partial: 0 tx_csum_partial_inner: 0 tx_csum_none: 138 tx_queue_stopped: 0 tx_queue_wake: 0 tx_queue_dropped: 0 rx_sw_lro_aggregated: 0 rx_sw_lro_flushed: 0 rx_sw_lro_no_desc: 0 rx_wqe_err: 0 rx_cqe_compress_pkts: 0 rx_cqe_compress_blks: 0 rx_mpwqe_filler: 0 rx_mpwqe_frag: 0 link_down_events_phy: 0 rx_out_of_buffer: 0 rx_vport_unicast_packets: 26126070627 rx_vport_unicast_bytes: 1328543705864986 tx_vport_unicast_packets: 687072959078 tx_vport_unicast_bytes: 3339188884659844 rx_vport_multicast_packets: 0 rx_vport_multicast_bytes: 0 tx_vport_multicast_packets: 0 tx_vport_multicast_bytes: 0 rx_vport_broadcast_packets: 4543553 rx_vport_broadcast_bytes: 272614872 tx_vport_broadcast_packets: 4543637 tx_vport_broadcast_bytes: 272619912 rx_vport_rdma_unicast_packets: 430134 rx_vport_rdma_unicast_bytes: 32690184 tx_vport_rdma_unicast_packets: 0 tx_vport_rdma_unicast_bytes: 0 rx_vport_rdma_multicast_packets: 0 rx_vport_rdma_multicast_bytes: 0 tx_vport_rdma_multicast_packets: 0 tx_vport_rdma_multicast_bytes: 0 tx_packets_phy: 11756775169850 rx_packets_phy: 2246825269635 rx_crc_errors_phy: 0 tx_bytes_phy: 3404211507236837 rx_bytes_phy: 1342896754459327 tx_multicast_phy: 0 tx_broadcast_phy: 4552688 rx_multicast_phy: 0 rx_broadcast_phy: 4552604 rx_in_range_len_errors_phy: 0 rx_out_of_range_len_phy: 0 rx_oversize_pkts_phy: 1135778701 rx_symbol_err_phy: 0 tx_mac_control_phy: 16579953 rx_mac_control_phy: 16670966 rx_unsupported_op_phy: 0 rx_pause_ctrl_phy: 16670966 tx_pause_ctrl_phy: 16579953 rx_discards_phy: 12244164648 tx_discards_phy: 0 tx_errors_phy: 0 rx_undersize_pkts_phy: 0 rx_fragments_phy: 0 rx_jabbers_phy: 0 rx_64_bytes_phy: 911948813404 rx_65_to_127_bytes_phy: 335422614022 rx_128_to_255_bytes_phy: 153383903056 rx_256_to_511_bytes_phy: 80464779917 rx_512_to_1023_bytes_phy: 184356224678 rx_1024_to_1518_bytes_phy: 313733619509 rx_1519_to_2047_bytes_phy: 235669614881 rx_2048_to_4095_bytes_phy: 4051420173 rx_4096_to_8191_bytes_phy: 1011852034 rx_8192_to_10239_bytes_phy: 27710327250 time_since_last_clear_phy: 2998621833 symbol_errors_phy: 0 sync_headers_errors_phy: 0 edpl/bip_errors_lane0_phy: 0 edpl/bip_errors_lane1_phy: 0 edpl/bip_errors_lane2_phy: 0 edpl/bip_errors_lane3_phy: 0 fc_corrected_blocks_lane0_phy: 0 fc_corrected_blocks_lane1_phy: 0 fc_corrected_blocks_lane2_phy: 0 fc_corrected_blocks_lane3_phy: 0 fc_uncorrectable_lane0_phy: 0 fc_uncorrectable_lane1_phy: 0 fc_uncorrectable_lane2_phy: 0 fc_uncorrectable_lane3_phy: 0 rs_corrected_blocks_phy: 0 rs_uncorrectable_blocks_phy: 0 rs_no_errors_blocks_phy: 58566832662545 rs_single_error_blocks_phy: 0 rs_corrected_symbols_total_phy: 0 rs_corrected_symbols_lane0_phy: 0 rs_corrected_symbols_lane1_phy: 0 rs_corrected_symbols_lane2_phy: 0 rs_corrected_symbols_lane3_phy: 0 rx_prio0_bytes: 1342896754459327 rx_prio0_packets: 2234564434021 tx_prio0_bytes: 3404210446119845 tx_prio0_packets: 11756758589897 rx_prio1_bytes: 0 rx_prio1_packets: 0 tx_prio1_bytes: 0 tx_prio1_packets: 0 rx_prio2_bytes: 0 rx_prio2_packets: 0 tx_prio2_bytes: 0 tx_prio2_packets: 0 rx_prio3_bytes: 0 rx_prio3_packets: 0 tx_prio3_bytes: 0 tx_prio3_packets: 0 rx_prio4_bytes: 0 rx_prio4_packets: 0 tx_prio4_bytes: 0 tx_prio4_packets: 0 rx_prio5_bytes: 0 rx_prio5_packets: 0 tx_prio5_bytes: 0 tx_prio5_packets: 0 rx_prio6_bytes: 0 rx_prio6_packets: 0 tx_prio6_bytes: 0 tx_prio6_packets: 0 rx_prio7_bytes: 0 rx_prio7_packets: 0 tx_prio7_bytes: 0 tx_prio7_packets: 0 rx0_packets: 54 rx0_bytes: 3240 rx0_csum_complete 2) Add the right HW counters to the VF=20 tx28_nop: 0 tx28_queue_stopped: 0 tx28_queue_wake: 0 tx28_queue_dropped: 0 tx29_packets: 0 tx29_bytes: 0 tx29_tso_packets: 0 tx29_tso_bytes: 0 tx29_tso_inner_packets: 0 tx29_tso_inner_bytes: 0 tx29_csum_none: 0 tx29_csum_partial: 0 tx29_csum_partial_inner: 0 tx29_nop: 0 tx29_queue_stopped: 0 3) Implement the stats_get/reset using HW counters=20 .stats_get =3D mlx5_stats_get, .stats_reset =3D mlx5_stats_reset, Thanks, Hanoh -----Original Message----- From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]=20 Sent: Wednesday, January 11, 2017 6:55 PM To: Shahaf Shuler Cc: dev@dpdk.org; Elad Persiko; Hanoch Haim (hhaim) Subject: Re: [PATCH] net/mlx5: support extended statistics Hi Shahaf, Thanks, I have a few comments, most of them relatively minor. Please see be= low. On Wed, Jan 11, 2017 at 10:55:42AM +0200, Shahaf Shuler wrote: > Implement xstats_*() DPDK callbacks >=20 > Signed-off-by: Shahaf Shuler > Signed-off-by: Elad Persiko > Signed-off-by: Hanoch Haim > --- > drivers/net/mlx5/mlx5.c | 3 + > drivers/net/mlx5/mlx5.h | 12 ++ > drivers/net/mlx5/mlx5_defs.h | 3 + > drivers/net/mlx5/mlx5_stats.c | 342 ++++++++++++++++++++++++++++++++++= ++++++ > drivers/net/mlx5/mlx5_trigger.c | 1 + > 5 files changed, 361 insertions(+) >=20 > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index=20 > 6293c1f..3359d3c 100644 > --- a/drivers/net/mlx5/mlx5.c > +++ b/drivers/net/mlx5/mlx5.c > @@ -199,6 +199,9 @@ > .link_update =3D mlx5_link_update, > .stats_get =3D mlx5_stats_get, > .stats_reset =3D mlx5_stats_reset, > + .xstats_get =3D mlx5_xstats_get, > + .xstats_reset =3D mlx5_xstats_reset, > + .xstats_get_names =3D mlx5_xstats_get_names, > .dev_infos_get =3D mlx5_dev_infos_get, > .dev_supported_ptypes_get =3D mlx5_dev_supported_ptypes_get, > .vlan_filter_set =3D mlx5_vlan_filter_set, diff --git=20 > a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index=20 > ee62e04..034a05e 100644 > --- a/drivers/net/mlx5/mlx5.h > +++ b/drivers/net/mlx5/mlx5.h > @@ -89,6 +89,11 @@ enum { > PCI_DEVICE_ID_MELLANOX_CONNECTX5EXVF =3D 0x101a, }; > =20 > +struct mlx5_xstats_ctrl { > + uint64_t shadow[MLX5_MAX_XSTATS]; > + uint16_t stats_n; /* Number of device stats. */ }; > + > struct priv { > struct rte_eth_dev *dev; /* Ethernet device. */ > struct ibv_context *ctx; /* Verbs context. */ @@ -142,6 +147,7 @@=20 > struct priv { > struct fdir_queue *fdir_drop_queue; /* Flow director drop queue. */ > LIST_HEAD(mlx5_flows, rte_flow) flows; /* RTE Flow rules. */ > uint32_t link_speed_capa; /* Link speed capabilities. */ > + struct mlx5_xstats_ctrl xstats_ctrl; /* Extended stats control. */ > rte_spinlock_t lock; /* Lock for control functions. */ }; > =20 > @@ -250,8 +256,14 @@ int mlx5_dev_rss_reta_update(struct rte_eth_dev=20 > *, > =20 > /* mlx5_stats.c */ > =20 > +void priv_xstats_init(struct priv *); > void mlx5_stats_get(struct rte_eth_dev *, struct rte_eth_stats *); =20 > void mlx5_stats_reset(struct rte_eth_dev *); > +int mlx5_xstats_get(struct rte_eth_dev *, > + struct rte_eth_xstat *, unsigned int); void=20 > +mlx5_xstats_reset(struct rte_eth_dev *); int=20 > +mlx5_xstats_get_names(struct rte_eth_dev *, > + struct rte_eth_xstat_name *, unsigned int); > =20 > /* mlx5_vlan.c */ > =20 > diff --git a/drivers/net/mlx5/mlx5_defs.h=20 > b/drivers/net/mlx5/mlx5_defs.h index b32816e..beabb70 100644 > --- a/drivers/net/mlx5/mlx5_defs.h > +++ b/drivers/net/mlx5/mlx5_defs.h > @@ -79,4 +79,7 @@ > /* Alarm timeout. */ > #define MLX5_ALARM_TIMEOUT_US 100000 > =20 > +/* Maximum number of extended statistics counters. */ #define=20 > +MLX5_MAX_XSTATS 32 > + > #endif /* RTE_PMD_MLX5_DEFS_H_ */ > diff --git a/drivers/net/mlx5/mlx5_stats.c=20 > b/drivers/net/mlx5/mlx5_stats.c index f2b5781..08a7656 100644 > --- a/drivers/net/mlx5/mlx5_stats.c > +++ b/drivers/net/mlx5/mlx5_stats.c > @@ -31,11 +31,16 @@ > * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE= . > */ > =20 > +#include > +#include > + > /* DPDK headers don't like -pedantic. */ #ifdef PEDANTIC #pragma=20 > GCC diagnostic ignored "-Wpedantic" > #endif > #include > +#include > +#include > #ifdef PEDANTIC > #pragma GCC diagnostic error "-Wpedantic" > #endif > @@ -44,6 +49,269 @@ > #include "mlx5_rxtx.h" > #include "mlx5_defs.h" > =20 > +struct mlx5_counter_ctrl { > + /* Name of the counter for dpdk user. */ "for dpdk user" is redundant given the purpose of this API. > + char dpdk_name[RTE_ETH_XSTATS_NAME_SIZE]; > + /* Name of the counter on the device table. */ > + char ctr_name[RTE_ETH_XSTATS_NAME_SIZE]; > + /* Index in the device counters table. */ > + uint16_t dev_table_idx; > +}; > + > +static struct mlx5_counter_ctrl mlx5_counters_init[] =3D { > + { > + .dpdk_name =3D "rx_port_unicast_bytes", > + .ctr_name =3D "rx_vport_unicast_bytes", > + .dev_table_idx =3D 0 Please add comma after ".dev_table_idx =3D 0" (same comment applies to the = following definitions). Note you may leave it out entirely since unspecifie= d fields are automatically initialized to zero. > + }, > + { > + .dpdk_name =3D "rx_port_multicast_bytes", > + .ctr_name =3D "rx_vport_multicast_bytes", > + .dev_table_idx =3D 0 > + }, > + { > + .dpdk_name =3D "rx_port_broadcast_bytes", > + .ctr_name =3D "rx_vport_broadcast_bytes", > + .dev_table_idx =3D 0 > + }, > + { > + .dpdk_name =3D "rx_port_unicast_packets", > + .ctr_name =3D "rx_vport_unicast_packets", > + .dev_table_idx =3D 0 > + }, > + { > + .dpdk_name =3D "rx_port_multicast_packets", > + .ctr_name =3D "rx_vport_multicast_packets", > + .dev_table_idx =3D 0 > + }, > + { > + .dpdk_name =3D "rx_port_broadcast_packets", > + .ctr_name =3D "rx_vport_broadcast_packets", > + .dev_table_idx =3D 0 > + }, > + { > + .dpdk_name =3D "tx_port_unicast_bytes", > + .ctr_name =3D "tx_vport_unicast_bytes", > + .dev_table_idx =3D 0 > + }, > + { > + .dpdk_name =3D "tx_port_multicast_bytes", > + .ctr_name =3D "tx_vport_multicast_bytes", > + .dev_table_idx =3D 0 > + }, > + { > + .dpdk_name =3D "tx_port_broadcast_bytes", > + .ctr_name =3D "tx_vport_broadcast_bytes", > + .dev_table_idx =3D 0 > + }, > + { > + .dpdk_name =3D "tx_port_unicast_packets", > + .ctr_name =3D "tx_vport_unicast_packets", > + .dev_table_idx =3D 0 > + }, > + { > + .dpdk_name =3D "tx_port_multicast_packets", > + .ctr_name =3D "tx_vport_multicast_packets", > + .dev_table_idx =3D 0 > + }, > + { > + .dpdk_name =3D "tx_port_broadcast_packets", > + .ctr_name =3D "tx_vport_broadcast_packets", > + .dev_table_idx =3D 0 > + }, > + { > + .dpdk_name =3D "rx_wqe_err", > + .ctr_name =3D "rx_wqe_err", > + .dev_table_idx =3D 0 > + }, > + { > + .dpdk_name =3D "rx_crc_errors_phy", > + .ctr_name =3D "rx_crc_errors_phy", > + .dev_table_idx =3D 0 > + }, > + { > + .dpdk_name =3D "rx_in_range_len_errors_phy", > + .ctr_name =3D "rx_in_range_len_errors_phy", > + .dev_table_idx =3D 0 > + }, > + { > + .dpdk_name =3D "rx_symbol_err_phy", > + .ctr_name =3D "rx_symbol_err_phy", > + .dev_table_idx =3D 0 > + }, > + { > + .dpdk_name =3D "tx_errors_phy", > + .ctr_name =3D "tx_errors_phy", > + .dev_table_idx =3D 0 > + }, > +}; > + > +const unsigned int xstats_n =3D RTE_DIM(mlx5_counters_init); Considering this global depends on mlx5_counters_init[] and is only used in= this file, it should be static. > + > +/** > + * Read device counters table. > + * > + * @param priv > + * Pointer to private structure. > + * @param[out] stats > + * Counters table output buffer. > + * > + * @return > + * 0 on success and stats is filled, negative on error. > + */ > +static int > +priv_read_dev_counters(struct priv *priv, uint64_t *stats) { > + struct mlx5_xstats_ctrl *xstats_ctrl =3D &priv->xstats_ctrl; > + unsigned int i; > + struct ifreq ifr; > + unsigned int stats_sz =3D (xstats_ctrl->stats_n * sizeof(uint64_t)) + > + sizeof(struct ethtool_stats); > + struct ethtool_stats et_stats[(stats_sz + ( > + sizeof(struct ethtool_stats) - 1)) / > + sizeof(struct ethtool_stats)]; > + > + et_stats->cmd =3D ETHTOOL_GSTATS; > + et_stats->n_stats =3D xstats_ctrl->stats_n; > + ifr.ifr_data =3D (caddr_t)et_stats; > + if (priv_ifreq(priv, SIOCETHTOOL, &ifr) !=3D 0) { > + WARN("unable to get statistic values"); > + return -1; > + } > + for (i =3D 0; (i !=3D xstats_n) ; ++i) > + stats[i] =3D (uint64_t) > + et_stats->data[mlx5_counters_init[i].dev_table_idx]; > + return 0; > +} > + > +/** > + * Init the strcutures to read device counters. Typo, "strcutures". > + * > + * @param priv > + * Pointer to private structure. > + */ > +void > +priv_xstats_init(struct priv *priv) > +{ > + struct mlx5_xstats_ctrl *xstats_ctrl =3D &priv->xstats_ctrl; > + unsigned int i; > + unsigned int j; > + char ifname[IF_NAMESIZE]; > + struct ifreq ifr; > + struct ethtool_drvinfo drvinfo; > + struct ethtool_gstrings *strings =3D NULL; > + unsigned int dev_stats_n; > + unsigned int str_sz; > + > + if (priv_get_ifname(priv, &ifname)) { > + WARN("unable to get interface name"); > + return; > + } > + /* How many statistics are available. */ > + drvinfo.cmd =3D ETHTOOL_GDRVINFO; > + ifr.ifr_data =3D (caddr_t)&drvinfo; > + if (priv_ifreq(priv, SIOCETHTOOL, &ifr) !=3D 0) { > + WARN("unable to get driver info"); > + return; > + } > + dev_stats_n =3D drvinfo.n_stats; > + if (dev_stats_n < 1) { > + WARN("no extended statistics available"); > + return; > + } > + xstats_ctrl->stats_n =3D dev_stats_n; > + /* Allocate memory to grab stat names and values. */ > + str_sz =3D dev_stats_n * ETH_GSTRING_LEN; > + strings =3D (struct ethtool_gstrings *) > + rte_malloc("xstats_strings", > + str_sz + sizeof(struct ethtool_gstrings), 0); > + if (!strings) { > + WARN("unable to allocate memory for xstats"); > + return; > + } > + strings->cmd =3D ETHTOOL_GSTRINGS; > + strings->string_set =3D ETH_SS_STATS; > + strings->len =3D dev_stats_n; > + ifr.ifr_data =3D (caddr_t)strings; > + if (priv_ifreq(priv, SIOCETHTOOL, &ifr) !=3D 0) { > + WARN("unable to get statistic names"); > + goto free; > + } > + for (j =3D 0; (j !=3D xstats_n); ++j) > + mlx5_counters_init[j].dev_table_idx =3D dev_stats_n; As a global, mlx5_counters_init[] affects all DPDK ports that use the mlx5 = PMD simultaneously. Are you sure calling priv_xstats_init() from mlx5_dev_start() is safe? I think this global should be const and initialized only once. > + for (i =3D 0; (i !=3D dev_stats_n); ++i) { > + const char *curr_string =3D (const char *) > + &strings->data[i * ETH_GSTRING_LEN]; > + > + for (j =3D 0; (j !=3D xstats_n); ++j) { > + if (!strcmp(mlx5_counters_init[j].ctr_name, > + curr_string)) { > + mlx5_counters_init[j].dev_table_idx =3D i; The above comment also applies here. The PMD instance should allocate its o= wn mapping as a priv field if there is no other choice. You could perhaps m= ake it part of the mlx5_xstats_ctrl structure. > + break; > + } > + } > + } > + for (j =3D 0; (j !=3D xstats_n); ++j) { > + if (mlx5_counters_init[j].dev_table_idx >=3D dev_stats_n) { > + WARN("Counters are not recognized"); Displaying the name of the counter that is not recognized could help users = determine what's doing on. Please make sure all info/warning/error messages= are helpful enough, e.g.: testpmd> show port xstats 0 ###### NIC extended statistics for port 0 mlx5: Counters are not recognized testpmd> # what? > + goto free; > + } > + } > + /* Copy to shadow at first time. */ > + assert(xstats_n <=3D MLX5_MAX_XSTATS); > + priv_read_dev_counters(priv, xstats_ctrl->shadow); > +free: > + rte_free(strings); > +} > + > +/** > + * Get device extended statistics. > + * > + * @param priv > + * Pointer to private structure. > + * @param[out] stats > + * Pointer to rte extended stats table. > + * > + * @return > + * Number of extended stats on success and stats is filled, > + * nagative on error. Typo, "nagative". > + */ > +static int > +priv_xstats_get(struct priv *priv, struct rte_eth_xstat *stats) { > + struct mlx5_xstats_ctrl *xstats_ctrl =3D &priv->xstats_ctrl; > + unsigned int i; > + uint64_t counters[xstats_n]; > + > + if (priv_read_dev_counters(priv, counters) < 0) > + return -1; > + for (i =3D 0; (i !=3D xstats_n) ; ++i) { > + stats[i].id =3D i; > + stats[i].value =3D (uint64_t) > + (counters[i] - xstats_ctrl->shadow[i]); I understand the purpose of the shadow array in this context, however to me= "shadow" implies some sort of caching is taking place. I think "init" or "= base" (as the base value or something) would make its purpose clearer. > + } > + return xstats_n; > +} > + > +/** > + * Reset device extended statistics. > + * > + * @param priv > + * Pointer to private structure. > + */ > +static void > +priv_xstats_reset(struct priv *priv) > +{ > + struct mlx5_xstats_ctrl *xstats_ctrl =3D &priv->xstats_ctrl; > + unsigned int i; > + uint64_t counters[xstats_n]; > + > + if (priv_read_dev_counters(priv, counters) < 0) > + return; > + for (i =3D 0; (i !=3D xstats_n) ; ++i) > + xstats_ctrl->shadow[i] =3D counters[i]; } > + > /** > * DPDK callback to get device statistics. > * > @@ -142,3 +410,77 @@ > #endif > priv_unlock(priv); > } > + > +/** > + * DPDK callback to get extended device statistics. > + * > + * @param dev > + * Pointer to Ethernet device structure. > + * @param[out] stats > + * Stats table output buffer. > + * @param n > + * The size of the stats table. > + * > + * @return > + * Number of xstats on success, negative on failure. > + */ > +int > +mlx5_xstats_get(struct rte_eth_dev *dev, > + struct rte_eth_xstat *stats, unsigned int n) { > + struct priv *priv =3D mlx5_get_priv(dev); > + int ret =3D xstats_n; > + > + if (n >=3D xstats_n && stats) { > + priv_lock(priv); > + ret =3D priv_xstats_get(priv, stats); > + priv_unlock(priv); > + } > + return ret; > +} > + > +/** > + * DPDK callback to clear device extended statistics. > + * > + * @param dev > + * Pointer to Ethernet device structure. > + */ > +void > +mlx5_xstats_reset(struct rte_eth_dev *dev) { > + struct priv *priv =3D mlx5_get_priv(dev); > + > + priv_lock(priv); > + priv_xstats_reset(priv); > + priv_unlock(priv); > +} > + > +/** > + * DPDK callback to retrieve names of extended device statistics > + * > + * @param dev > + * Pointer to Ethernet device structure. > + * @param[out] xstats_names > + * Block of memory to insert names into Let's call "block of memory" a "buffer"? There is also a missing period at = the end of this sentence.=20 > + * @param size > + * Number of names. "num" (or "n" as in the API) would make more sense than "size" here. > + * > + * @return > + * Number of xstats names. > + */ > +int > +mlx5_xstats_get_names(struct rte_eth_dev *dev, > + struct rte_eth_xstat_name *xstats_names, unsigned int size) { > + struct priv *priv =3D mlx5_get_priv(dev); > + unsigned int i; > + > + if (size >=3D xstats_n && xstats_names) { > + priv_lock(priv); > + for (i =3D 0; (i !=3D xstats_n) ; ++i) > + strcpy(xstats_names[i].name, > + mlx5_counters_init[i].dpdk_name); > + priv_unlock(priv); > + } > + return xstats_n; > +} > diff --git a/drivers/net/mlx5/mlx5_trigger.c=20 > b/drivers/net/mlx5/mlx5_trigger.c index 2399243..30addd2 100644 > --- a/drivers/net/mlx5/mlx5_trigger.c > +++ b/drivers/net/mlx5/mlx5_trigger.c > @@ -91,6 +91,7 @@ > priv_fdir_enable(priv); > priv_dev_interrupt_handler_install(priv, dev); > err =3D priv_flow_start(priv); > + priv_xstats_init(priv); > priv_unlock(priv); > return -err; > } > -- > 1.8.3.1 -- Adrien Mazarguil 6WIND