diff mbox

[dpdk-dev,v3,4/7] ethdev: remove HW specific stats in stats structs

Message ID 1435323558-169985-5-git-send-email-maryam.tahhan@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Tahhan, Maryam June 26, 2015, 12:59 p.m. UTC 
  Remove non generic stats in rte_stats_strings and mark the relevant
fields in struct rte_eth_stats as deprecated.

Signed-off-by: Maryam Tahhan <maryam.tahhan@intel.com>
---
 doc/guides/rel_notes/abi.rst  | 11 +++++++++++
 lib/librte_ether/rte_ethdev.c |  9 ---------
 lib/librte_ether/rte_ethdev.h | 30 ++++++++++++++++++++----------
 3 files changed, 31 insertions(+), 19 deletions(-)

Comments

Kyle Larose June 26, 2015, 2:03 p.m. UTC | #1 
On Fri, Jun 26, 2015 at 8:59 AM, Maryam Tahhan <maryam.tahhan@intel.com>
wrote:

> Remove non generic stats in rte_stats_strings and mark the relevant
> fields in struct rte_eth_stats as deprecated.
>
> Signed-off-by: Maryam Tahhan <maryam.tahhan@intel.com>
> ---
>  doc/guides/rel_notes/abi.rst  | 11 +++++++++++
>  lib/librte_ether/rte_ethdev.c |  9 ---------
>  lib/librte_ether/rte_ethdev.h | 30 ++++++++++++++++++++----------
>  3 files changed, 31 insertions(+), 19 deletions(-)
>
> diff --git a/doc/guides/rel_notes/abi.rst b/doc/guides/rel_notes/abi.rst
> index f00a6ee..957b13f 100644
> --- a/doc/guides/rel_notes/abi.rst
> +++ b/doc/guides/rel_notes/abi.rst
> @@ -38,3 +38,14 @@ Examples of Deprecation Notices
>
>  Deprecation Notices
>  -------------------
> +* The following fields have been deprecated in rte_eth_stats:
> +  * uint64_t imissed
> +  * uint64_t ibadcrc
> +  * uint64_t ibadlen
> +  * uint64_t imcasts
> +  * uint64_t fdirmatch
> +  * uint64_t fdirmiss
> +  * uint64_t tx_pause_xon
> +  * uint64_t rx_pause_xon
> +  * uint64_t tx_pause_xoff
> +  * uint64_t rx_pause_xoff
>
>
Are CRC errors (ibadcrc) truly hardware specific? Which NIC (aside from
purely virtual ones) does not have a MAC which does frame checksumming?
Likewise, which NIC doesn't drop because the PCI bus/cpu/etc is too busy to
pull packets off of it (imissed)?

Debugging interactions with NICs is hard enough with only CRC errors and
missed packets to go on. Without those it is close to impossible. CRC
errors are almost guaranteed any time a bare-metal application is deployed:
dirty fiber, bad SFPs, etc. How will users of the application be able to
determine why their packets are dropping if they can only see "in errors"?

I understand that we want to avoid placing too much useless information
into these statistics structures. However, without a hardware-independent
way of accessing fairly standard networking-equipment diagnostics, I feel
like any real-world application using DPDK will be terribly cumbersome to
build: every single one will need to develop an abstraction layer which
detects the attached NICs, and loads an appropriate driver to integrate
with the xstats api.

Is there any plan for such an API? If not, is it really a good idea to
deprecate these stats?

Thanks,

Kyle
diff mbox

Patch

diff --git a/doc/guides/rel_notes/abi.rst b/doc/guides/rel_notes/abi.rst
index f00a6ee..957b13f 100644
--- a/doc/guides/rel_notes/abi.rst
+++ b/doc/guides/rel_notes/abi.rst
@@ -38,3 +38,14 @@  Examples of Deprecation Notices
 
 Deprecation Notices
 -------------------
+* The following fields have been deprecated in rte_eth_stats:
+  * uint64_t imissed
+  * uint64_t ibadcrc
+  * uint64_t ibadlen
+  * uint64_t imcasts
+  * uint64_t fdirmatch
+  * uint64_t fdirmiss
+  * uint64_t tx_pause_xon
+  * uint64_t rx_pause_xon
+  * uint64_t tx_pause_xoff
+  * uint64_t rx_pause_xoff
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 6451621..8176baf 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -125,17 +125,8 @@  static const struct rte_eth_xstats_name_off rte_stats_strings[] = {
 	{"rx_bytes", offsetof(struct rte_eth_stats, ibytes)},
 	{"tx_bytes", offsetof(struct rte_eth_stats, obytes)},
 	{"tx_errors", offsetof(struct rte_eth_stats, oerrors)},
-	{"rx_missed_errors", offsetof(struct rte_eth_stats, imissed)},
-	{"rx_crc_errors", offsetof(struct rte_eth_stats, ibadcrc)},
-	{"rx_bad_length_errors", offsetof(struct rte_eth_stats, ibadlen)},
 	{"rx_errors", offsetof(struct rte_eth_stats, ierrors)},
 	{"alloc_rx_buff_failed", offsetof(struct rte_eth_stats, rx_nombuf)},
-	{"fdir_match", offsetof(struct rte_eth_stats, fdirmatch)},
-	{"fdir_miss", offsetof(struct rte_eth_stats, fdirmiss)},
-	{"tx_flow_control_xon", offsetof(struct rte_eth_stats, tx_pause_xon)},
-	{"rx_flow_control_xon", offsetof(struct rte_eth_stats, rx_pause_xon)},
-	{"tx_flow_control_xoff", offsetof(struct rte_eth_stats, tx_pause_xoff)},
-	{"rx_flow_control_xoff", offsetof(struct rte_eth_stats, rx_pause_xoff)},
 };
 #define RTE_NB_STATS (sizeof(rte_stats_strings) / sizeof(rte_stats_strings[0]))
 
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index f1219ac..a38d49a 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -193,19 +193,29 @@  struct rte_eth_stats {
 	uint64_t opackets;  /**< Total number of successfully transmitted packets.*/
 	uint64_t ibytes;    /**< Total number of successfully received bytes. */
 	uint64_t obytes;    /**< Total number of successfully transmitted bytes. */
-	uint64_t imissed;   /**< Total of RX missed packets (e.g full FIFO). */
-	uint64_t ibadcrc;   /**< Total of RX packets with CRC error. */
-	uint64_t ibadlen;   /**< Total of RX packets with bad length. */
+	/**< Deprecated; Total of RX missed packets (e.g full FIFO). */
+	uint64_t imissed;
+	/**< Deprecated; Total of RX packets with CRC error. */
+	uint64_t ibadcrc;
+	/**< Deprecated; Total of RX packets with bad length. */
+	uint64_t ibadlen;
 	uint64_t ierrors;   /**< Total number of erroneous received packets. */
 	uint64_t oerrors;   /**< Total number of failed transmitted packets. */
-	uint64_t imcasts;   /**< Total number of multicast received packets. */
+	uint64_t imcasts;
+	/**< Deprecated; Total number of multicast received packets. */
 	uint64_t rx_nombuf; /**< Total number of RX mbuf allocation failures. */
-	uint64_t fdirmatch; /**< Total number of RX packets matching a filter. */
-	uint64_t fdirmiss;  /**< Total number of RX packets not matching any filter. */
-	uint64_t tx_pause_xon;  /**< Total nb. of XON pause frame sent. */
-	uint64_t rx_pause_xon;  /**< Total nb. of XON pause frame received. */
-	uint64_t tx_pause_xoff; /**< Total nb. of XOFF pause frame sent. */
-	uint64_t rx_pause_xoff; /**< Total nb. of XOFF pause frame received. */
+	uint64_t fdirmatch;
+	/**< Deprecated; Total number of RX packets matching a filter. */
+	uint64_t fdirmiss;
+	/**< Deprecated; Total number of RX packets not matching any filter. */
+	uint64_t tx_pause_xon;
+	 /**< Deprecated; Total nb. of XON pause frame sent. */
+	uint64_t rx_pause_xon;
+	/**< Deprecated; Total nb. of XON pause frame received. */
+	uint64_t tx_pause_xoff;
+	/**< Deprecated; Total nb. of XOFF pause frame sent. */
+	uint64_t rx_pause_xoff;
+	/**< Deprecated; Total nb. of XOFF pause frame received. */
 	uint64_t q_ipackets[RTE_ETHDEV_QUEUE_STAT_CNTRS];
 	/**< Total number of queue RX packets. */
 	uint64_t q_opackets[RTE_ETHDEV_QUEUE_STAT_CNTRS];