From: "Tahhan, Maryam" <maryam.tahhan@intel.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [PATCH 2/4] ethdev: expose extended error stats
Date: Mon, 22 Jun 2015 14:12:25 +0000 [thread overview]
Message-ID: <1A27633A6DA49C4A92FCD5D4312DBF536A43EB82@IRSMSX109.ger.corp.intel.com> (raw)
In-Reply-To: <6388992.ny7qEppTdV@xps13>
> 2015-06-05 18:35, Maryam Tahhan:
> > Extend rte_eth_xstats_get to retrieve additional stats from the device
> > driver as well the top level extended stats. Add additional drop
> > counters to the extended stats.
> >
> > Signed-off-by: Maryam Tahhan <maryam.tahhan@intel.com>
> [..]
> Patch 1/4 doesn't compile without patch 2/4.
The rebased patches should fix this issue.
>
> > --- a/lib/librte_ether/rte_ethdev.c
> > +++ b/lib/librte_ether/rte_ethdev.c
> > @@ -129,6 +129,8 @@ static const struct rte_eth_xstats_name_off
> rte_stats_strings[] = {
> > {"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)},
> > + {"rx_mac_err", offsetof(struct rte_eth_stats, imacerr)},
> > + {"rx_phy_err", offsetof(struct rte_eth_stats, iphyerr)},
> > {"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)}, @@ -136,6
> > +138,8 @@ static const struct rte_eth_xstats_name_off rte_stats_strings[]
> = {
> > {"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)},
> > + {"tx_drops", offsetof(struct rte_eth_stats, odrop)},
> > + {"rx_drops", offsetof(struct rte_eth_stats, idrop)},
> > };
> [...]
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -224,6 +224,10 @@ struct rte_eth_stats {
> >
> > /**< Total number of good bytes received from loopback,VF Only */
> > uint64_t olbbytes;
> > /**< Total number of good bytes transmitted to loopback,VF
> > Only */
> >
> > + uint64_t imacerr; /**< Total of RX packets with MAC Errors. */
> > + uint64_t iphyerr; /**< Total of RX packets with PHY Errors. */
> > + uint64_t idrop; /**< Total number of dropped received packets. */
> > + uint64_t odrop; /**< Total number of dropped transmitted
> > + packets. */
> > };
>
> You are extending the generic stats. This is not the idea behind xstats.
> The xstats are specific to the driver.
I'd followed the example of: http://patchwork.dpdk.org/dev/patchwork/patch/85/
to added generic extended stats (at least what I thought were generic). I think that
dropped packets should fall under struct rte_eth_stats, and should perhaps be left
there, as most NICs/drivers should be able to provide that number. Would this be an
agreeable solution?
I have no other way to expose the total MAC errors and the total PHY errors without
Adding counters into struct ixgbe_hw_stats, but I wasn't sure if this was allowable, is it?
The only other option is to round up all the errors into ierrors, without having the
granularity of what errors fall under. Is the latter option to sum up the values under one
umbrella preferred?
> Furthermore we should migrate some "not really generic stats" to xstats in
> order to keep only the really basic and common stats in rte_eth_stats.
> By the way, in order to avoid duplicated code when getting generic stats
> through xstats API, we need to change the implementation of
> rte_eth_xstats_get() to add generic stats automatically, even if the driver
> provide some xstats.
next prev parent reply other threads:[~2015-06-22 14:12 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-05 17:35 [PATCH 0/4] expose ixgbe extended stats to dpdk apps Maryam Tahhan
2015-06-05 17:35 ` [PATCH 1/4] ixgbe: expose extended error statistics Maryam Tahhan
2015-06-08 13:26 ` Tahhan, Maryam
2015-06-10 0:51 ` Stephen Hemminger
2015-06-10 14:24 ` Tahhan, Maryam
2015-06-17 12:52 ` Thomas Monjalon
2015-06-05 17:35 ` [PATCH 2/4] ethdev: expose extended error stats Maryam Tahhan
2015-06-17 13:58 ` Thomas Monjalon
2015-06-17 14:47 ` Kyle Larose
2015-06-22 14:12 ` Tahhan, Maryam [this message]
2015-06-22 15:12 ` Olivier MATZ
2015-06-22 15:35 ` Tahhan, Maryam
2015-06-05 17:35 ` [PATCH 3/4] testpmd: extend testpmd to show all extended stats Maryam Tahhan
2015-06-05 17:35 ` [PATCH 4/4] app: replace dump_cfg with proc_info Maryam Tahhan
2015-06-05 21:08 ` Thomas Monjalon
2015-06-08 13:45 ` Tahhan, Maryam
2015-06-08 14:22 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1A27633A6DA49C4A92FCD5D4312DBF536A43EB82@IRSMSX109.ger.corp.intel.com \
--to=maryam.tahhan@intel.com \
--cc=dev@dpdk.org \
--cc=thomas.monjalon@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.