[dpdk-dev] [PATCH v4 8/8] app: add a new app proc_info
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Thursday, July 9, 2015 2:40 AM
> To: Tahhan, Maryam
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 8/8] app: add a new app proc_info
>
> 2015-07-05 18:40, Maryam Tahhan:
> > proc_info displays statistics information including extened stats for
>
> typo: extended
>
> > given DPDK ports and dumps the memory information for DPDK.
> >
> > Signed-off-by: Maryam Tahhan
> > ---
> > MAINTAINERS| 4 +
> > app/Makefile | 1 +
> > app/proc_info/Makefile | 45 +
> > app/proc_info/main.c | 512
> +
> > mk/rte.sdktest.mk | 4 +-
> > 5 files changed, 564 insertions(+), 2 deletions(-)
>
> A doc would be useful.
I will add a doc
>
> > +DIRS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += proc_info
>
> Why only Linux?
Dump_cfg was only supported on Linux also, I've only tested it on Linux.
>
> > +# Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
>
> Too old :)
>
> > + if ((!port->rx_queue_stats_mapping_enabled) && (!port-
> >tx_queue_stats_mapping_enabled)) {
> > + printf(" RX-packets: %-10"PRIu64" RX-missed: %-10"PRIu64"
> RX-bytes: "
> > + "%-"PRIu64"\n",
> > + stats.ipackets, stats.imissed, stats.ibytes);
> > + printf(" RX-badcrc: %-10"PRIu64" RX-badlen: %-10"PRIu64"
> RX-errors: "
> > + "%-"PRIu64"\n",
> > + stats.ibadcrc, stats.ibadlen, stats.ierrors);
> > + printf(" RX-nombuf: %-10"PRIu64"\n",
> > + stats.rx_nombuf);
> > + printf(" TX-packets: %-10"PRIu64" TX-errors: %-10"PRIu64"
> TX-bytes: "
> > + "%-"PRIu64"\n",
> > + stats.opackets, stats.oerrors, stats.obytes);
> > + } else {
> > + printf(" RX-packets: %10"PRIu64"RX-errors:
> %10"PRIu64
> > + "RX-bytes: %10"PRIu64"\n",
> > + stats.ipackets, stats.ierrors, stats.ibytes);
> > + printf(" RX-badcrc: %10"PRIu64"RX-badlen:
> %10"PRIu64
> > + " RX-errors: %10"PRIu64"\n",
> > + stats.ibadcrc, stats.ibadlen, stats.ierrors);
> > + printf(" RX-nombuf: %10"PRIu64"\n",
> > + stats.rx_nombuf);
> > + printf(" TX-packets: %10"PRIu64"TX-errors:
> %10"PRIu64
> > + "TX-bytes: %10"PRIu64"\n",
> > + stats.opackets, stats.oerrors, stats.obytes);
> > + }
>
> Why the formatting is different depending of device-specific stats mapping?
I will update this and remove the deprecated stats from being printed as well.
>
> > + if (port->rx_queue_stats_mapping_enabled) {
> > + printf("\n");
> > + for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS; i++) {
> > + printf(" Stats reg %2d RX-packets: %10"PRIu64
> > + "RX-errors: %10"PRIu64
> > + "RX-bytes: %10"PRIu64"\n",
> > + i, stats.q_ipackets[i], stats.q_errors[i],
> stats.q_ibytes[i]);
> > + }
> > + }
>
> Why restricting stats per queue to devices requiring a mapping?
I will change this
>
> > - $(RTE_OUTPUT)/app/dump_cfg --file-prefix=ring_perf ; \
>
> This should have been removed in previous patch.
I will remove this in the previous patch
[dpdk-dev] [PATCH v4 3/8] ethdev: expose extended error stats
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Wednesday, July 15, 2015 10:19 AM
> To: Tahhan, Maryam; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 3/8] ethdev: expose extended error stats
>
> Hi Maryam,
>
> The API of the driver-specific function should be the same than the generic
> one, described in rte_ethdev.h.
>
> What I mean is:
> - the xstats pointer is the place where the xstats should be written
> by the driver
> - the n argument is the number of entries in the xstats structure
>
> The driver should not have to worry about the generic stats written by the
> generic layer.
>
> Please see some comments below to fix it.
>
Hi Olivier
I have sent an updated patch set with the fixes recommended below.
Thanks for reviewing
Maryam
> On 07/05/2015 07:39 PM, Maryam Tahhan wrote:
> > Extend rte_eth_xstats_get to retrieve additional stats from the device
> > driver as well the ethdev generic stats.
> >
> > Signed-off-by: Maryam Tahhan
> > ---
> > drivers/net/ixgbe/ixgbe_ethdev.c | 7 ---
> > lib/librte_ether/rte_ethdev.c| 25 -
> > 2 files changed, 20 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index 7feb03c..5971d41 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -2035,6 +2035,7 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev,
> struct rte_eth_xstats *xstats,
> > total_qprdc = 0;
> > rxnfgpc = 0;
> > txdgpc = 0;
> > + count = n - IXGBE_NB_XSTATS;
> >
> > ixgbe_read_stats_registers(hw, hw_stats, _missed_rx,
> _qbrc,
> >_qprc,
> , , _qprdc); @@ -2047,13
> > +2048,13 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct
> > rte_eth_xstats *xstats,
> >
> > /* Extended stats */
> > for (i = 0; i < IXGBE_NB_XSTATS; i++) {
> > - snprintf(xstats[i].name, sizeof(xstats[i].name),
> > + snprintf(xstats[count].name, sizeof(xstats[i].name),
> > "%s", rte_ixgbe_stats_strings[i].name);
> > - xstats[i].value = *(uint64_t *)(((char *)hw_stats) +
> > + xstats[count++].value = *(uint64_t *)(((char *)hw_stats) +
> >
> rte_ixgbe_stats_strings[i].offset);
> > }
> >
> > - return count;
> > + return IXGBE_NB_XSTATS;
>
> The modifications in ixgbe driver can be removed: the patch 2/8 already
> provides everything that is required.
>
>
> > }
> >
> > static void
> > diff --git a/lib/librte_ether/rte_ethdev.c
> > b/lib/librte_ether/rte_ethdev.c index da915db..e392f60 100644
> > --- a/lib/librte_ether/rte_ethdev.c
> > +++ b/lib/librte_ether/rte_ethdev.c
> > @@ -1681,26 +1681,33 @@ rte_eth_xstats_get(uint8_t port_id, struct
> rte_eth_xstats *xstats,
> > {
> > struct rte_eth_stats eth_stats;
> > struct rte_eth_dev *dev;
> > - unsigned count, i, q;
> > + unsigned count = 0, i, q;
> > + signed xcount = 0;
> > uint64_t val, *stats_ptr;
> >
> > VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
> >
> > dev = _eth_devices[port_id];
> >
> > - /* implemented by the driver */
> > - if (dev->dev_ops->xstats_get != NULL)
> > - return (*dev->dev_ops->xstats_get)(dev, xstats, n);
> > -
> > - /* else, return generic statistics */
> > + /* Return generic statistics */
> > count = RTE_NB_STATS;
> > count += dev->data->nb_rx_queues * RTE_NB_RXQ_STATS;
> > count += dev->data->nb_tx_queues * RTE_NB_TXQ_STATS;
> > +
> > + /* implemented by the driver */
> > + if (dev->dev_ops->xstats_get != NULL) {
> > + /* Retrieve the xstats from the driver at the end of the
> > +* xstats struct.
> > +*/
> > + xcount = (*dev->dev_ops->xstats_get)(dev, xstats, n);
>
> it should be:
>
>xcount = (*dev->dev_ops->xstats_get)(dev, [count],
> (n > count) ? n - count : 0);
>
>
> > + if (xcount < 0)
> > + return xcount;
> > + }
> > +
> > if (n < count)
> > - return count;
> > + return count + xcount;
>
> it should be:
>
>if (n < count + xcount)
> return count + xcount;
>
>
> >
> > /* now fill the xstats structure */
> > -
> > count = 0;
> > rte_eth_stats_get(port_id, _stats);
> >
> > @@ -1742,7 +1749,7 @@ rte_eth_xstats_get(uint8_t port_id, struct
> rte_eth_xstats *xstats,
> > }
> > }
> >
> > - return count;
> > + return count + xcount;
> > }
> >
> > /* reset ethdev extended statistics */
> >
>
> Regards,
> Olivier
[dpdk-dev] [PATCH v1 1/1] ixgbe: Fix oerrors by setting it to 0
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Thursday, July 30, 2015 12:20 AM > To: Tahhan, Maryam > Cc: dev at dpdk.org; Ananyev, Konstantin > Subject: Re: [dpdk-dev] [PATCH v1 1/1] ixgbe: Fix oerrors by setting it to 0 > > > > Fix afebc86be1346136125af8026dc215f81c202c50. oerrors was txdgpc - > > > hw_stats->gptc, txdgpc is the number of packets DMA'ed by the host > > > and was being reset on every call to read stats so it could be < gptc. > > > Because we currently have no way to add txdgpc to struct hw_stats so > > > that we can maintain a persistent value per port oerrors has now > > > been set to 0. References to txdgpc is now removed as we don't use > > > it. This patch also removes rxnfgpc as it's not used anywhere. > > > > > > Signed-off-by: Maryam Tahhan > > Acked-by: Konstantin Ananyev > > Applied, thanks > > It's a bit sad. > Is it a consequence of forbidding updates in the base driver? Yes, that's exactly it. In the meantime I'm going to look at/investigate another way to allow us to maintain additional (anything not in struct hw_stats) per port stats/registers in addition to the base driver. All the best Maryam
[dpdk-dev] [PATCH 1/4] ixgbe: expose extended error statistics
> + stats->idrop = hw_stats->mngpdc + > + hw_stats->fcoerpdc + > + total_qbrc; Should use qprdc instead of total_qbrc
[dpdk-dev] [PATCH 4/4] app: replace dump_cfg with proc_info
> > Extend dump_cfg to also display statistcs information for given DPDK > > ports and rename the application to proc_info as it's now a utility > > doing a little more than just dumping the memory information for DPDK. > > > > Signed-off-by: Maryam Tahhan > > --- > > app/Makefile | 2 +- > > app/dump_cfg/Makefile | 45 - > > app/dump_cfg/main.c| 92 - > > app/proc_info/Makefile | 45 + > > app/proc_info/main.c | 525 > + > > mk/rte.sdktest.mk | 4 +- > > It looks promising, thanks. > Would you consider adding yourself as a maintainer of this app? Yep, I can do that. Should I add myself to the maintainers file in a separate patch, or as a reworked version of this patch? BR Maryam
[dpdk-dev] [PATCH 1/4] ixgbe: expose extended error statistics
> From: Stephen Hemminger [mailto:stephen at networkplumber.org] > Sent: Wednesday, June 10, 2015 1:51 AM > To: Tahhan, Maryam > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 1/4] ixgbe: expose extended error statistics > > On Fri, 5 Jun 2015 18:35:02 +0100 > Maryam Tahhan wrote: > > > Implement xstats_get() and xstats_reset() in dev_ops for ixgbe to > > expose detailed error statistics to DPDK applications. > > > > Signed-off-by: Maryam Tahhan > > Also, the bug where CRC is included in Tx byte count but not in the Rx byte > count has not been addressed. > > You seem to have ignored my earlier patch. I wasn't aware of your patch. But I will have a look. Best Regards Maryam
[dpdk-dev] [PATCH 2/4] ethdev: expose extended error stats
> 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
> [..]
> 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.
[dpdk-dev] [PATCH 2/4] ethdev: expose extended error stats
> >>> + 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? > > As Thomas explained, I think we should avoid adding fields in struct > rte_eth_stats. This structure already contains statistics that are specific to > some hardware drivers. We should try to go in the opposite direction and > remove all the fields that do not apply to all nics (physical or virtual). > For me, it > would be only something like (rx, tx, rx_bytes, tx_bytes, rx_errors, > tx_errors). > > The xstats framework allows you to add any driver or hardware specific stats > and I think it's the proper place to add them for ixgbe. > Alright that sounds good. > By the way, something could be modified in xstats framework. Today, the > behavior is: > - if ethdev->dev_ops->xstats_get != NULL, call it > - else, dump the generic stats in xstats format As it happens I did do this in: http://dpdk.org/dev/patchwork/patch/5324/ but I will reverse the order as what happens now is xstats from the driver are displayed first and will look at always dumping generic stats in xstats format as you mention below. > > A better behavior could be: > - Always dump the generic stats in xstats format > - and if thdev->dev_ops->xstats_get != NULL, add driver-specific stats > > This would avoid to duplicate code that dumps generic stats in all drivers. > > So, to summarize what I think should be done for stats/xstats: > 1. modify xstats behavior as described above 2. add the xstats dev_ops in > ixgbe, it will dump the new stats > > Bonus: > 3. identify all the specific stats that could be removed from >rte_eth_stats and start to mark them as deprecated. > Will do. > > Regards, > Olivier Thanks Maryam
[dpdk-dev] [PATCH v6 4/9] ethdev: remove HW specific stats in stats structs
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Monday, August 17, 2015 3:54 PM > To: Tahhan, Maryam; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v6 4/9] ethdev: remove HW specific stats in > stats structs > > Hi Maryam, > > On 07/15/2015 03:11 PM, Maryam Tahhan 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 > > --- > > doc/guides/rel_notes/abi.rst | 12 > > lib/librte_ether/rte_ethdev.c | 9 - > > lib/librte_ether/rte_ethdev.h | 30 -- > > 3 files changed, 32 insertions(+), 19 deletions(-) > > > > diff --git a/doc/guides/rel_notes/abi.rst > > b/doc/guides/rel_notes/abi.rst index 931e785..d5bf625 100644 > > --- a/doc/guides/rel_notes/abi.rst > > +++ b/doc/guides/rel_notes/abi.rst > > @@ -24,3 +24,15 @@ Deprecation Notices > > > > * The Macros RTE_HASH_BUCKET_ENTRIES_MAX and > RTE_HASH_KEY_LENGTH_MAX are > >deprecated and will be removed with version 2.2. > > + > > +* 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 > > Looking again at this patch, I'm wondering if "imissed" should be kept instead > of beeing deprecated. I think it could be useful to differentiate ierrors from > imissed, and it's not a hw-specific statistic. What do you think? > > One more comment: it seems these fields are marked as deprecated but they > are still used on other drivers (e1000, i40e, ...). > > Regards, > Olivier > Hi Olivier I can remove the deprecated status for imissed to leave the differentiation between errors and missed packets. igb and i40e will be updated soon to reflect this. I marked them as deprecated to deter their use in the future. Older instances/use will need to be resolved. Regards Maryam
[dpdk-dev] [PATCH v3 4/7] ethdev: remove HW specific stats in stats structs
On Fri, Jun 26, 2015 at 8:59 AM, Maryam Tahhan mailto:maryam.tahhan at 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 mailto:maryam.tahhan at 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 Hi Kyle If it?s just for debug/diagnostic purposes that this information is being used then I would recommend using the proc_info app which is already integrated with xstats will give you detailed error statistics. It runs as a DPDK secondary process. I?m not sure about crcerrors and imissed, I had taken feedback to my previous version of the patches onboard and made the changes based on that. Regards Maryam
[dpdk-dev] [PATCH v3 4/7] ethdev: remove HW specific stats in stats structs
> Hi Maryam, > I was not aware of the proc_info app. Is there any documentation on dpdk.org > about it, or should I browse the code? > Thanks, > Kyle Hi Kyle, It's the last patch in the patchset I posted. I haven't written up the documentation yet, but will do so, for now feel free to peruse the code. If you have any questions let me know. Regards Maryam
[dpdk-dev] [PATCH 0/3] vhost example based on user space vhost library.
Hi I see the eventfd module is still included... is this to support existing vhost implementations? Is Qemu's vhost-user supported? Thanks Maryam -Original Message- From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Cao, Waterman Sent: Thursday, August 7, 2014 3:29 PM To: Xie, Huawei; dev at dpdk.org Subject: Re: [dpdk-dev] [PATCH 0/3] vhost example based on user space vhost library. Tested-by: Waterman Cao This patch implements a simple vswitch by user vhost library, and is ready to integrate into DPDK.org. -Original Message- >From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Huawei Xie >Sent: Tuesday, August 5, 2014 11:58 PM >To: dev at dpdk.org >Subject: [dpdk-dev] [PATCH 0/3] vhost example based on user space vhost >library. > >This vhost example implements a simple vswitch using DPDK user space vhost >library(lib/librte_vhost) and VMDQ to demonstrate vhost's performance. >- Each virtio device is bound to a VMDQ pool and each pool is assigned the >mac/vlan of the virtio device. >- Packets arriving at a pool after l2 classifier will be moved to the virtio >device. >- Packets whose destination is a local virtio device will be delivered either >by a)software switching mode b)hardware l2 switch. >- zero copy is supported and could be configured through command line. > >Huawei Xie (3): > remove old vhost example > add lib/librte_vhost support in mk/rte.app.mk > add new vhost example > > examples/vhost/Makefile| 10 +- > examples/vhost/eventfd_link/Makefile | 39 - > examples/vhost/eventfd_link/eventfd_link.c | 205 - > examples/vhost/eventfd_link/eventfd_link.h | 79 -- > examples/vhost/libvirt/qemu-wrap.py|5 +- > examples/vhost/main.c | 1101 +- > examples/vhost/main.h | 85 +- > examples/vhost/vhost-net-cdev.c| 367 - > examples/vhost/vhost-net-cdev.h| 83 -- > examples/vhost/virtio-net.c| 1165 > examples/vhost/virtio-net.h| 147 > mk/rte.app.mk |5 + > 12 files changed, 585 insertions(+), 2706 deletions(-) delete mode > 100644 examples/vhost/eventfd_link/Makefile > delete mode 100644 examples/vhost/eventfd_link/eventfd_link.c > delete mode 100644 examples/vhost/eventfd_link/eventfd_link.h > delete mode 100644 examples/vhost/vhost-net-cdev.c delete mode 100644 > examples/vhost/vhost-net-cdev.h delete mode 100644 > examples/vhost/virtio-net.c delete mode 100644 > examples/vhost/virtio-net.h > >-- >1.8.1.4 > -- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
[dpdk-dev] [PATCH 0/3] vhost example based on user space vhost library.
Thanks I was looking at the wrong version of the patch :( -Original Message- From: Xie, Huawei Sent: Tuesday, August 19, 2014 10:07 AM To: Tahhan, Maryam; Cao, Waterman; dev at dpdk.org Cc: Long, Thomas Subject: RE: [dpdk-dev] [PATCH 0/3] vhost example based on user space vhost library. Hi Maryam: This patch removes eventfd kernel module from example. Qemu user space vhost support is within progress. This is our existing vhost implementation. With the new implementation, there should be no or minor change to the library API. BR. -huawei > -Original Message- > From: Tahhan, Maryam > Sent: Tuesday, August 19, 2014 4:51 PM > To: Cao, Waterman; Xie, Huawei; dev at dpdk.org > Cc: Long, Thomas > Subject: RE: [dpdk-dev] [PATCH 0/3] vhost example based on user space > vhost library. > > Hi > I see the eventfd module is still included... is this to support > existing vhost implementations? Is Qemu's vhost-user supported? > > Thanks > Maryam > > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Cao, Waterman > Sent: Thursday, August 7, 2014 3:29 PM > To: Xie, Huawei; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 0/3] vhost example based on user space > vhost library. > > Tested-by: Waterman Cao This patch > implements a simple vswitch by user vhost library, and is ready to integrate > into DPDK.org. > > -Original Message- > >From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Huawei Xie > >Sent: Tuesday, August 5, 2014 11:58 PM > >To: dev at dpdk.org > >Subject: [dpdk-dev] [PATCH 0/3] vhost example based on user space > >vhost > library. > > > >This vhost example implements a simple vswitch using DPDK user space > >vhost > library(lib/librte_vhost) and VMDQ to demonstrate vhost's performance. > >- Each virtio device is bound to a VMDQ pool and each pool is > >assigned the > mac/vlan of the virtio device. > >- Packets arriving at a pool after l2 classifier will be moved to the virtio > >device. > >- Packets whose destination is a local virtio device will be > >delivered either by > a)software switching mode b)hardware l2 switch. > >- zero copy is supported and could be configured through command line. > > > >Huawei Xie (3): > > remove old vhost example > > add lib/librte_vhost support in mk/rte.app.mk > > add new vhost example > > > > examples/vhost/Makefile| 10 +- > > examples/vhost/eventfd_link/Makefile | 39 - > > examples/vhost/eventfd_link/eventfd_link.c | 205 - > > examples/vhost/eventfd_link/eventfd_link.h | 79 -- > > examples/vhost/libvirt/qemu-wrap.py|5 +- > > examples/vhost/main.c | 1101 +- > > examples/vhost/main.h | 85 +- > > examples/vhost/vhost-net-cdev.c| 367 - > > examples/vhost/vhost-net-cdev.h| 83 -- > > examples/vhost/virtio-net.c| 1165 > > > > examples/vhost/virtio-net.h| 147 > > mk/rte.app.mk |5 + > > 12 files changed, 585 insertions(+), 2706 deletions(-) delete mode > > 100644 examples/vhost/eventfd_link/Makefile > > delete mode 100644 examples/vhost/eventfd_link/eventfd_link.c > > delete mode 100644 examples/vhost/eventfd_link/eventfd_link.h > > delete mode 100644 examples/vhost/vhost-net-cdev.c delete mode > > 100644 examples/vhost/vhost-net-cdev.h delete mode 100644 > > examples/vhost/virtio-net.c delete mode 100644 > > examples/vhost/virtio-net.h > > > >-- > >1.8.1.4 > > -- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
[dpdk-dev] [PATCH v3 0/7] Expose IXGBE extended stats to DPDK apps
> This patch set implements xstats_get() and xstats_reset() in dev_ops for ixgbe > to expose detailed error statistics to DPDK applications. The dump_cfg > application was extended to demonstrate the usage of retrieving statistics for > DPDK interfaces and renamed to proc_info in order reflect this new > functionality. This patch set also removes non generic statistics from the > statistics strings at the ethdev level and marks the relevant registers as > depricated in struct rte_eth_stats. > > v2: > - Fixed patch dependencies. > - Broke down patches into smaller logical changes. > > v3: > - Removes non-generic stats fields in rte_stats_strings and deprecates >the fields related to them in struct rte_eth_stats. > - Modifies rte_eth_xstats_get() to return generic stats and extended stats. > > Maryam Tahhan (7): > ixgbe: move stats register reads to a new function > ixgbe: add functions to get and reset xstats > ethdev: expose extended error stats > ethdev: remove HW specific stats in stats structs > ixgbe: add NIC specific stats removed from ethdev > app: remove dump_cfg > app: add a new app proc_info > > MAINTAINERS | 4 + > app/Makefile | 2 +- > app/dump_cfg/Makefile| 45 > app/dump_cfg/main.c | 92 --- > app/proc_info/Makefile | 45 > app/proc_info/main.c | 512 > +++ > doc/guides/rel_notes/abi.rst | 11 + > drivers/net/ixgbe/ixgbe_ethdev.c | 192 --- > lib/librte_ether/rte_ethdev.c| 29 ++- > lib/librte_ether/rte_ethdev.h| 30 ++- > mk/rte.sdktest.mk| 4 +- > 11 files changed, 762 insertions(+), 204 deletions(-) delete mode 100644 > app/dump_cfg/Makefile delete mode 100644 app/dump_cfg/main.c create > mode 100644 app/proc_info/Makefile create mode 100644 > app/proc_info/main.c > > -- > 1.9.3 Hi Olivier, I posted a new patch set with the suggested mods. Let me know if there are any issues. Thanks in advance. Best Regards, Maryam
[dpdk-dev] [PATCH v3 2/7] ixgbe: add functions to get and reset xstats
Best Regards,
Maryam
> -Original Message-
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Friday, July 3, 2015 2:16 PM
> To: Tahhan, Maryam; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 2/7] ixgbe: add functions to get and reset
> xstats
>
> Hi Maryam,
>
> On 06/26/2015 02:59 PM, Maryam Tahhan wrote:
> > Implement ixgbe_dev_xstats_reset and ixgbe_dev_xstats_get.
> >
> > Signed-off-by: Maryam Tahhan
> > ---
> > drivers/net/ixgbe/ixgbe_ethdev.c | 85
> >
> > 1 file changed, 85 insertions(+)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index 927021f..0f62bcb 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -131,7 +131,10 @@ static int ixgbe_dev_link_update(struct
> rte_eth_dev *dev,
> > int wait_to_complete);
> > static void ixgbe_dev_stats_get(struct rte_eth_dev *dev,
> > struct rte_eth_stats *stats);
> > +static int ixgbe_dev_xstats_get(struct rte_eth_dev *dev,
> > + struct rte_eth_xstats *xstats, unsigned n);
> > static void ixgbe_dev_stats_reset(struct rte_eth_dev *dev);
> > +static void ixgbe_dev_xstats_reset(struct rte_eth_dev *dev);
> > static int ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev
> *eth_dev,
> > uint16_t queue_id,
> > uint8_t stat_idx,
> > @@ -334,7 +337,9 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops
> = {
> > .allmulticast_disable = ixgbe_dev_allmulticast_disable,
> > .link_update = ixgbe_dev_link_update,
> > .stats_get= ixgbe_dev_stats_get,
> > + .xstats_get = ixgbe_dev_xstats_get,
> > .stats_reset = ixgbe_dev_stats_reset,
> > + .xstats_reset = ixgbe_dev_xstats_reset,
> > .queue_stats_mapping_set = ixgbe_dev_queue_stats_mapping_set,
> > .dev_infos_get= ixgbe_dev_info_get,
> > .mtu_set = ixgbe_dev_mtu_set,
> > @@ -414,6 +419,33 @@ static const struct eth_dev_ops
> ixgbevf_eth_dev_ops = {
> > .set_mc_addr_list = ixgbe_dev_set_mc_addr_list,
> > };
> >
> > +/* store statistics names and its offset in stats structure */
> > +struct rte_ixgbe_xstats_name_off {
> > + char name[RTE_ETH_XSTATS_NAME_SIZE];
> > + unsigned offset;
> > +};
> > +
> > +static const struct rte_ixgbe_xstats_name_off rte_ixgbe_stats_strings[] = {
> > + {"rx_illegal_byte_err", offsetof(struct ixgbe_hw_stats, errbc)},
> > + {"rx_len_err", offsetof(struct ixgbe_hw_stats, rlec)},
> > + {"rx_undersize_count", offsetof(struct ixgbe_hw_stats, ruc)},
> > + {"rx_oversize_count", offsetof(struct ixgbe_hw_stats, roc)},
> > + {"rx_fragment_count", offsetof(struct ixgbe_hw_stats, rfc)},
> > + {"rx_jabber_count", offsetof(struct ixgbe_hw_stats, rjc)},
> > + {"l3_l4_xsum_error", offsetof(struct ixgbe_hw_stats, xec)},
> > + {"mac_local_fault", offsetof(struct ixgbe_hw_stats, mlfc)},
> > + {"mac_remote_fault", offsetof(struct ixgbe_hw_stats, mrfc)},
> > + {"mac_short_pkt_discard", offsetof(struct ixgbe_hw_stats, mspdc)},
> > + {"fccrc_error", offsetof(struct ixgbe_hw_stats, fccrc)},
> > + {"fcoe_drop", offsetof(struct ixgbe_hw_stats, fcoerpdc)},
> > + {"fc_last_error", offsetof(struct ixgbe_hw_stats, fclast)},
> > + {"rx_broadcast_packets", offsetof(struct ixgbe_hw_stats, bprc)},
> > + {"mgmt_pkts_dropped", offsetof(struct ixgbe_hw_stats, mngpdc)}, };
> > +
> > +#define RTE_NB_XSTATS (sizeof(rte_ixgbe_stats_strings) / \
> > + sizeof(rte_ixgbe_stats_strings[0]))
> > +
>
> Maybe RTE_NB_XSTATS should be renamed in IXGBE_NB_XSTATS?
>
>
> > /**
> > * Atomically reads the link status information from global
> > * structure rte_eth_dev.
> > @@ -1982,6 +2014,59 @@ ixgbe_dev_stats_reset(struct rte_eth_dev
> *dev)
> > memset(stats, 0, sizeof(*stats));
> > }
> >
> > +static int
> > +ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstats
> *xstats,
> > +unsigned n)
> > +{
> > + struct ixgbe_hw *hw =
> > + IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> > + st
[dpdk-dev] [PATCH v3 2/7] ixgbe: add functions to get and reset xstats
>
>
>
> > >
> > >> +
> > >> +total_missed_rx = 0;
> > >> +total_qbrc = 0;
> > >> +total_qprc = 0;
> > >> +total_qprdc = 0;
> > >> +rxnfgpc = 0;
> > >> +txdgpc = 0;
> > >> +count = 0;
> > >> +
> > >> +ixgbe_read_stats_registers(hw, hw_stats, _missed_rx,
> > _qbrc,
> > >> + _qprc,
> > , , _qprdc);
> > >> +
> > >> +if (!xstats)
> > >> +return 0;
> > >
> > > this cannot happen except if n == 0.
> > > This condition is already tested above, and "count" should be returned.
> > >
> > >> +
> > >> +/* Error stats */
> > >> +for (i = 0; i < RTE_NB_XSTATS; i++) {
> > >> +snprintf(xstats[count].name, sizeof(xstats[count].name),
> > >> +"%s", rte_ixgbe_stats_strings[i].name);
> > >> +xstats[count++].value = *(uint64_t *)(((char
> > >> *)hw_stats) +
> > >> +
> > rte_ixgbe_stats_strings[i].offset);
> > >> +}
> > >> +
> > >> +return count;
> > >> +}
> > >
> > > Shouldn't it be xstats[i] instead of xstats[count] ?
> > >
> > > Does it work when using "show port in test-pmd"?
> >
> > ok I missed the 'count = 0' above.
> > So why using count instead of i ?
> >
> > Also, I think it would be clearer to use the constant IXGBE_NB_XSTATS
> > instead of count at the beginning of the function.
> >
> >
>
> Hi Olivier
>
> Actually, count should not be 0, it should be n, which is the passed in index
> from rte_eth_xstats_get()...
>
> Because we fill out the generic stats first in rte_eth_xstats_get() then we
> call
> ixgbe_dev_xstats_get to fill out the rest.
>
> I will fix this up
Hi Olivier,
I confused this change with a subsequent patch in the patch set... so yes for
this patch you are correct we can just use i... and leave count as 0.
in a subsequent patch I modify count to start at n, which is a passed in index
from in rte_eth_xstats_get()...
so I will change count for i in the loop here.
> > >
> > >> +
> > >> +static void
> > >> +ixgbe_dev_xstats_reset(struct rte_eth_dev *dev) {
> > >> +struct ixgbe_hw_stats *stats =
> > >> +IXGBE_DEV_PRIVATE_TO_STATS(dev->data-
> > >dev_private);
> > >> +
> > >> +/* HW registers are cleared on read */
> > >> +ixgbe_dev_xstats_get(dev, NULL, RTE_NB_XSTATS);
> > >> +
> > >> +/* Reset software totals */
> > >> +memset(stats, 0, sizeof(*stats)); }
> > >> +
> > >> static void
> > >> ixgbevf_dev_stats_get(struct rte_eth_dev *dev, struct
> > >> rte_eth_stats
> > >> *stats) {
> > >>
[dpdk-dev] [PATCH v2 0/3] Keep-alive enhancements
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton > Sent: Wednesday, May 18, 2016 10:30 AM > To: dev at dpdk.org; Mcnamara, John > Subject: [dpdk-dev] [PATCH v2 0/3] Keep-alive enhancements > > This patchset adds enhancements to the keepalive core monitoring > and reporting sub-system. The first is support for idled (sleeping and > frequency-stepped) CPU cores, and the second is support for > applications to be notified of active as well as faulted cores. The latter > is to allow core state to be relayed to external (secondary) processes, > which is demonstrated by changes to the l2fed-keepalive example. > > -- Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH v3] ixgbe: remove rx jabber from ierrors
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren > Sent: Tuesday, October 20, 2015 10:23 AM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v3] ixgbe: remove rx jabber from ierrors > > Remove receive jabber count (rjc) from ierrors count as the register overlaps > with the CRC error register, previously causing some packets to be counted > twice. > > Signed-off-by: Harry van Haaren > --- > > v3: Add details about register overlap > v2: Fix typo > > drivers/net/ixgbe/ixgbe_ethdev.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c > b/drivers/net/ixgbe/ixgbe_ethdev.c > index ec2918c..6e20e06 100644 > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > @@ -2206,7 +2206,6 @@ ixgbe_dev_stats_get(struct rte_eth_dev *dev, > struct rte_eth_stats *stats) > hw_stats->mlfc + > hw_stats->mrfc + > hw_stats->rfc + > - hw_stats->rjc + > hw_stats->fccrc + > hw_stats->fclast; > > -- > 1.9.1 Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH v2 1/3] rte: add keep alive functionality
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton
> Sent: Wednesday, September 30, 2015 10:05 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v2 1/3] rte: add keep alive functionality
>
> Adds functions for detecting and reporting the live-ness of LCores, the
> primary requirement of which is minimal overheads for the
> core(s) being checked. Core failures are notified via an application defined
> callback.
>
> Signed-off-by: Remy Horton
> ---
> lib/librte_eal/bsdapp/eal/Makefile| 1 +
> lib/librte_eal/common/Makefile| 2 +-
> lib/librte_eal/common/include/rte_keepalive.h | 140
> ++
> lib/librte_eal/common/rte_keepalive.c | 122
> ++
> lib/librte_eal/linuxapp/eal/Makefile | 1 +
> 5 files changed, 265 insertions(+), 1 deletion(-) create mode 100644
> lib/librte_eal/common/include/rte_keepalive.h
> create mode 100644 lib/librte_eal/common/rte_keepalive.c
>
> diff --git a/lib/librte_eal/bsdapp/eal/Makefile
> b/lib/librte_eal/bsdapp/eal/Makefile
> index a49dcec..65b293f 100644
> --- a/lib/librte_eal/bsdapp/eal/Makefile
> +++ b/lib/librte_eal/bsdapp/eal/Makefile
> @@ -80,6 +80,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) +=
> eal_common_thread.c
> SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += rte_malloc.c
> SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += malloc_elem.c
> SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += malloc_heap.c
> +SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += rte_keepalive.c
>
> CFLAGS_eal.o := -D_GNU_SOURCE
> #CFLAGS_eal_thread.o := -D_GNU_SOURCE
> diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
> index 0c43d6a..7f1757a 100644
> --- a/lib/librte_eal/common/Makefile
> +++ b/lib/librte_eal/common/Makefile
> @@ -40,7 +40,7 @@ INC += rte_string_fns.h rte_version.h INC +=
> rte_eal_memconfig.h rte_malloc_heap.h INC += rte_hexdump.h
> rte_devargs.h rte_dev.h INC += rte_pci_dev_feature_defs.h
> rte_pci_dev_features.h -INC += rte_malloc.h
> +INC += rte_malloc.h rte_keepalive.h
>
> ifeq ($(CONFIG_RTE_INSECURE_FUNCTION_WARNING),y)
> INC += rte_warnings.h
> diff --git a/lib/librte_eal/common/include/rte_keepalive.h
> b/lib/librte_eal/common/include/rte_keepalive.h
> new file mode 100644
> index 000..d67bf4b
> --- /dev/null
> +++ b/lib/librte_eal/common/include/rte_keepalive.h
> @@ -0,0 +1,140 @@
> +/*-
> + * BSD LICENSE
> + *
> + * Copyright 2015 Intel Shannon Ltd. All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in
> + * the documentation and/or other materials provided with the
> + * distribution.
> + * * Neither the name of Intel Corporation nor the names of its
> + * contributors may be used to endorse or promote products derived
> + * from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
> OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> + */
> +
> +/**
> + * @file keepalive.h
> + * DPDK RTE LCore Keepalive Monitor.
> + *
> + **/
> +
> +#ifndef _KEEPALIVE_H_
> +#define _KEEPALIVE_H_
> +
> +#include
> +
> +#ifndef RTE_KEEPALIVE_MAXCORES
> +/**
> + * Number of cores to track.
> + * @note Must be larger than the highest core id. */ #define
> +RTE_KEEPALIVE_MAXCORES RTE_MAX_LCORE #endif
> +
> +
> +/**
> + * Keepalive failure callback.
> + *
> + * Receives a data pointer passed to rte_keepalive_create() and the id
> +of the
> + * failed core.
> + */
> +typedef void (*rte_keepalive_failure_callback_t)(
> + void *data,
> + const int id_core);
> +
> +
> +/**
> + * Keepalive state structure.
> + * @internal
> + */
> +struct rte_keepalive {
> + /** Core Liveness. */
> + uint32_t __rte_cache_aligned
> state_flags[RTE_KEEPALIVE_MAXCORES];
> +
> + /**
[dpdk-dev] [PATCH v2 2/3] l2fwd: keep alive sample application
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton
> Sent: Wednesday, September 30, 2015 10:05 AM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v2 2/3] l2fwd: keep alive sample application
>
> Modification of l2fwd to demonstrate keep-alive functionality.
>
> Signed-off-by: Remy Horton
> ---
> examples/l2fwd/Makefile | 2 +-
> examples/l2fwd/main.c | 125
>
> 2 files changed, 118 insertions(+), 9 deletions(-)
>
> diff --git a/examples/l2fwd/Makefile b/examples/l2fwd/Makefile index
> 78feeeb..8647174 100644
> --- a/examples/l2fwd/Makefile
> +++ b/examples/l2fwd/Makefile
> @@ -39,7 +39,7 @@ RTE_TARGET ?= x86_64-native-linuxapp-gcc include
> $(RTE_SDK)/mk/rte.vars.mk
>
> # binary name
> -APP = l2fwd
> +APP = l2fwd-keepalive
>
Hi Remy,
Looks great overall, just a couple of comments
For backward compatibility the rename could be a nightmare? What do you think?
> # all source are stored in SRCS-y
> SRCS-y := main.c
> diff --git a/examples/l2fwd/main.c b/examples/l2fwd/main.c index
> 720fd5a..131e5a2 100644
> --- a/examples/l2fwd/main.c
> +++ b/examples/l2fwd/main.c
> @@ -1,7 +1,7 @@
> /*-
> * BSD LICENSE
> *
> - * Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + * Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
> * All rights reserved.
> *
> * Redistribution and use in source and binary forms, with or without
> @@ -68,6 +68,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #define RTE_LOGTYPE_L2FWD RTE_LOGTYPE_USER1
>
> @@ -135,13 +137,18 @@ struct l2fwd_port_statistics { struct
> l2fwd_port_statistics port_statistics[RTE_MAX_ETHPORTS];
>
> /* A tsc-based timer responsible for triggering statistics printout */
> -#define
> TIMER_MILLISECOND 200ULL /* around 1ms at 2 Ghz */
> +#define TIMER_MILLISECOND 1
> #define MAX_TIMER_PERIOD 86400 /* 1 day max */ static int64_t
> timer_period = 10 * TIMER_MILLISECOND * 1000; /* default period is 10
> seconds */
> +static int64_t check_period = 5; /* default check cycle is 5ms */
> +
> +/* Keepalive structure */
> +struct rte_keepalive * rte_global_keepalive_info;
>
> /* Print out statistics on packets dropped */ static void
> -print_stats(void)
> +print_stats(__attribute__((unused)) struct rte_timer *ptr_timer,
> + __attribute__((unused)) void *ptr_data)
> {
> uint64_t total_packets_dropped, total_packets_tx, total_packets_rx;
> unsigned portid;
> @@ -283,11 +290,23 @@ l2fwd_main_loop(void)
> portid);
> }
>
> + uint64_t tsc_initial = rte_rdtsc();
> + uint64_t tsc_lifetime = (rand()&0x07) * rte_get_tsc_hz();
> +
> while (1) {
> + /* Keepalive heartbeat */
> + rte_keepalive_mark_alive(rte_global_keepalive_info);
>
> cur_tsc = rte_rdtsc();
>
> /*
> + * Die randomly within 7 secs for demo purposes if
> + * keepalive enables
> + */
Typo: If keepalive is enabled
> + if (check_period > 0 && cur_tsc - tsc_initial > tsc_lifetime)
> + break;
> +
> + /*
>* TX burst queue drain
>*/
> diff_tsc = cur_tsc - prev_tsc;
> @@ -313,7 +332,7 @@ l2fwd_main_loop(void)
>
> /* do this only on master core */
> if (lcore_id ==
> rte_get_master_lcore()) {
> - print_stats();
> + print_stats(NULL, NULL);
> /* reset the timer */
> timer_tsc = 0;
> }
> @@ -357,6 +376,7 @@ l2fwd_usage(const char *prgname)
> printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
> " -p PORTMASK: hexadecimal bitmask of ports to configure\n"
> " -q NQ: number of queue (=ports) per lcore (default is 1)\n"
> +" -K PERIOD: Keepalive check period (5 default; 86400 max)\n"
> " -T PERIOD: statistics will be refreshed each PERIOD
> seconds (0 to disable, 10 default, 86400 maximum)\n",
> prgname);
> }
> @@ -412,6 +432,22 @@ l2fwd_parse_timer_period(const char *q_arg)
> return n;
> }
>
> +static int
> +l2fwd_parse_check_period(const char *q_arg) {
> + char *end = NULL;
> + int n;
> +
> + /* parse number string */
> + n = strtol(q_arg, , 10);
> + if ((q_arg[0] == '\0') || (end == NULL) || (*end != '\0'))
> + return -1;
> + if (n >= MAX_TIMER_PERIOD)
> + return -1;
> +
> + return n;
> +}
> +
> /* Parse the argument given in the command line of the application */ static
> int l2fwd_parse_args(int argc, char **argv) @@ -426,7 +462,7 @@
> l2fwd_parse_args(int argc, char **argv)
>
>
[dpdk-dev] [PATCH v3 02/11] doc: add extended statistics to prog_guide
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren > Sent: Thursday, October 22, 2015 4:48 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v3 02/11] doc: add extended statistics to > prog_guide > > Add extended statistic section to the programmers guide, poll mode driver > section. This section describes how the strings stats are formatted, and how > the client code can use this to gather information about the stat. > > Signed-off-by: Harry van Haaren > --- > doc/guides/prog_guide/poll_mode_drv.rst | 51 > - > 1 file changed, 50 insertions(+), 1 deletion(-) > > diff --git a/doc/guides/prog_guide/poll_mode_drv.rst > b/doc/guides/prog_guide/poll_mode_drv.rst > index 8780ba3..44cc9ce 100644 > --- a/doc/guides/prog_guide/poll_mode_drv.rst > +++ b/doc/guides/prog_guide/poll_mode_drv.rst > @@ -1,5 +1,5 @@ > .. BSD LICENSE > -Copyright(c) 2010-2014 Intel Corporation. All rights reserved. > +Copyright(c) 2010-2015 Intel Corporation. All rights reserved. > All rights reserved. > > Redistribution and use in source and binary forms, with or without @@ - > 294,3 +294,52 @@ Ethernet Device API ~~~ > > The Ethernet device API exported by the Ethernet PMDs is described in the > *DPDK API Reference*. > + > +Extended Statistics API > +~~~ > + > +The extended statistics API allows each individual PMD to expose a > +unique set of statistics. The client of the API provides an array of > +``struct rte_eth_xstats`` type. Each ``struct rte_eth_xstats`` contains > +a string and value pair. The amount of xstats exposed, and position of > +the statistic in the array must remain constant during runtime. > + > +A naming scheme exists for the strings exposed to clients of the API. > +This is to allow scraping of the API for statistics of interest. The > +naming scheme uses strings split by a single underscore ``_``. The scheme is > as follows: > + > +* direction > +* detail 1 > +* detail 2 > +* detail n > +* unit > + > +Examples of common statistics xstats strings, formatted to comply to > +the scheme proposed above: > + > +* ``rx_bytes`` > +* ``rx_crc_errors`` > +* ``tx_multicast_packets`` > + > +The scheme, although quite simple, allows flexibility in presenting and > +reading information from the statistic strings. The following example > +illustrates the naming scheme:``rx_packets``. In this example, the > +string is split into two components. The first component ``rx`` > +indicates that the statistic is associated with the receive side of the > +NIC. The second component ``packets`` indicates that the unit of measure is > packets. > + > +A more complicated example: ``tx_size_128_to_255_packets``. In this > +example, ``tx`` indicates transmission, ``size`` is the first detail, > +``128`` etc are more details, and ``packets`` indicates that this is a packet > counter. > + > +Some additions in the metadata scheme are as follows: > + > +* If the first part does not match ``rx`` or ``tx``, the statistic does > +not > + have an affinity with either recieve of transmit. > + > +* If the first letter of the second part is ``q`` and this ``q`` is > +followed > + by a number, this statistic is part of a specific queue. > + > +An example where queue numbers are used is as follows: ``tx_q7_bytes`` > +which indicates this statistic applies to queue number 7, and > +represents the number of transmitted bytes on that queue. > -- > 1.9.1 Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH v3 03/11] ethdev: update xstats_get() strings and Q handling
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren
> Sent: Thursday, October 22, 2015 4:48 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v3 03/11] ethdev: update xstats_get() strings and
> Q handling
>
> Update the strings used for presenting stats to adhere to the scheme
> previously presented. Updated xstats_get() function to handle Q information
> only if xstats() is not implemented in the PMD, providing the PMD with the
> needed flexibility to expose its extended Q stats.
>
> Signed-off-by: Harry van Haaren
> ---
> lib/librte_ether/rte_ethdev.c | 38 +++---
> 1 file changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index f593f6e..07f0c26 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -137,27 +137,30 @@ struct rte_eth_xstats_name_off { };
>
> static const struct rte_eth_xstats_name_off rte_stats_strings[] = {
> - {"rx_packets", offsetof(struct rte_eth_stats, ipackets)},
> - {"tx_packets", offsetof(struct rte_eth_stats, opackets)},
> - {"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_good_packets", offsetof(struct rte_eth_stats, ipackets)},
> + {"tx_good_packets", offsetof(struct rte_eth_stats, opackets)},
> + {"rx_good_bytes", offsetof(struct rte_eth_stats, ibytes)},
> + {"tx_good_bytes", offsetof(struct rte_eth_stats, obytes)},
Hi Harry
If there are any apps today scraping the existing stats this will cause an
issue.
I think the "good" description breaks the meaning of the various stats defined
in the header. For example rx_packets is Total number of successfully received
packets rather than the "good" packets.
BR
Maryam
> {"rx_errors", offsetof(struct rte_eth_stats, ierrors)},
> - {"alloc_rx_buff_failed", offsetof(struct rte_eth_stats, rx_nombuf)},
> + {"tx_errors", offsetof(struct rte_eth_stats, oerrors)},
> + {"rx_mbuf_allocation_errors", offsetof(struct rte_eth_stats,
> + rx_nombuf)},
> };
> +
> #define RTE_NB_STATS (sizeof(rte_stats_strings) /
> sizeof(rte_stats_strings[0]))
>
> static const struct rte_eth_xstats_name_off rte_rxq_stats_strings[] = {
> - {"rx_packets", offsetof(struct rte_eth_stats, q_ipackets)},
> - {"rx_bytes", offsetof(struct rte_eth_stats, q_ibytes)},
> + {"packets", offsetof(struct rte_eth_stats, q_ipackets)},
> + {"bytes", offsetof(struct rte_eth_stats, q_ibytes)},
> + {"errors", offsetof(struct rte_eth_stats, q_errors)},
> };
> +
> #define RTE_NB_RXQ_STATS (sizeof(rte_rxq_stats_strings) /\
> sizeof(rte_rxq_stats_strings[0]))
>
> static const struct rte_eth_xstats_name_off rte_txq_stats_strings[] = {
> - {"tx_packets", offsetof(struct rte_eth_stats, q_opackets)},
> - {"tx_bytes", offsetof(struct rte_eth_stats, q_obytes)},
> - {"tx_errors", offsetof(struct rte_eth_stats, q_errors)},
> + {"packets", offsetof(struct rte_eth_stats, q_opackets)},
> + {"bytes", offsetof(struct rte_eth_stats, q_obytes)},
> };
> #define RTE_NB_TXQ_STATS (sizeof(rte_txq_stats_strings) /\
> sizeof(rte_txq_stats_strings[0]))
> @@ -1666,8 +1669,6 @@ rte_eth_xstats_get(uint8_t port_id, struct
> rte_eth_xstats *xstats,
>
> /* Return generic statistics */
> count = RTE_NB_STATS;
> - count += dev->data->nb_rx_queues * RTE_NB_RXQ_STATS;
> - count += dev->data->nb_tx_queues * RTE_NB_TXQ_STATS;
>
> /* implemented by the driver */
> if (dev->dev_ops->xstats_get != NULL) { @@ -1679,6 +1680,9 @@
> rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstats *xstats,
>
> if (xcount < 0)
> return xcount;
> + } else {
> + count += dev->data->nb_rx_queues * RTE_NB_RXQ_STATS;
> + count += dev->data->nb_tx_queues * RTE_NB_TXQ_STATS;
> }
>
> if (n < count + xcount)
> @@ -1698,6 +1702,10 @@ rte_eth_xstats_get(uint8_t port_id, struct
> rte_eth_xstats *xstats,
> xstats[count++].value = val;
> }
>
> + /* if xstats_get() is implemented by the PMD, the Q stats are done */
> + if (dev->dev_ops->xstats_get != NULL)
> + return count + xcount;
> +
> /* per-rxq stats */
> for (q = 0; q < dev->data->nb_rx_queues; q++) {
> for (i = 0; i < RTE_NB_RXQ_STATS; i++) { @@ -1706,7 +1714,7
> @@ rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstats *xstats,
> q * sizeof(uint64_t));
> val = *stats_ptr;
> snprintf(xstats[count].name,
> sizeof(xstats[count].name),
> - "rx_queue_%u_%s", q,
> + "rx_q%u_%s", q,
>
[dpdk-dev] [PATCH v3 07/11] ixgbe: update statistic strings to scheme
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren
> Sent: Thursday, October 22, 2015 4:49 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH v3 07/11] ixgbe: update statistic strings to
> scheme
>
> Updated and add statistic strings as used by xstats_get().
>
> Signed-off-by: Harry van Haaren
> ---
> drivers/net/ixgbe/ixgbe_ethdev.c | 295
> ---
> 1 file changed, 273 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 1e5ffbf..2e9e260 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -506,29 +506,280 @@ struct rte_ixgbe_xstats_name_off { };
>
> static const struct rte_ixgbe_xstats_name_off rte_ixgbe_stats_strings[] = {
> - {"rx_illegal_byte_err", offsetof(struct ixgbe_hw_stats, errbc)},
> - {"rx_len_err", offsetof(struct ixgbe_hw_stats, rlec)},
> - {"rx_undersize_count", offsetof(struct ixgbe_hw_stats, ruc)},
> - {"rx_oversize_count", offsetof(struct ixgbe_hw_stats, roc)},
> - {"rx_fragment_count", offsetof(struct ixgbe_hw_stats, rfc)},
> - {"rx_jabber_count", offsetof(struct ixgbe_hw_stats, rjc)},
> - {"l3_l4_xsum_error", offsetof(struct ixgbe_hw_stats, xec)},
> - {"mac_local_fault", offsetof(struct ixgbe_hw_stats, mlfc)},
> - {"mac_remote_fault", offsetof(struct ixgbe_hw_stats, mrfc)},
> - {"mac_short_pkt_discard", offsetof(struct ixgbe_hw_stats, mspdc)},
> - {"fccrc_error", offsetof(struct ixgbe_hw_stats, fccrc)},
> - {"fcoe_drop", offsetof(struct ixgbe_hw_stats, fcoerpdc)},
> - {"fc_last_error", offsetof(struct ixgbe_hw_stats, fclast)},
> - {"rx_broadcast_packets", offsetof(struct ixgbe_hw_stats, bprc)},
> - {"rx_phy_multicast_packets", offsetof(struct ixgbe_hw_stats, mprc)},
> - {"mgmt_pkts_dropped", offsetof(struct ixgbe_hw_stats, mngpdc)},
> {"rx_crc_errors", offsetof(struct ixgbe_hw_stats, crcerrs)},
> - {"fdir_match", offsetof(struct ixgbe_hw_stats, fdirmatch)},
> - {"fdir_miss", offsetof(struct ixgbe_hw_stats, fdirmiss)},
> - {"tx_flow_control_xon", offsetof(struct ixgbe_hw_stats, lxontxc)},
> - {"rx_flow_control_xon", offsetof(struct ixgbe_hw_stats, lxonrxc)},
> - {"tx_flow_control_xoff", offsetof(struct ixgbe_hw_stats, lxofftxc)},
> - {"rx_flow_control_xoff", offsetof(struct ixgbe_hw_stats, lxoffrxc)},
> + {"rx_illegal_byte_errors", offsetof(struct ixgbe_hw_stats, illerrc)},
> + {"rx_error_bytes", offsetof(struct ixgbe_hw_stats, errbc)},
> + {"mac_local_errors", offsetof(struct ixgbe_hw_stats, mlfc)},
> + {"mac_remote_errors", offsetof(struct ixgbe_hw_stats, mrfc)},
> + {"rx_length_errors", offsetof(struct ixgbe_hw_stats, rlec)},
> + {"tx_xon_packets", offsetof(struct ixgbe_hw_stats, lxontxc)},
> + {"rx_xon_packets", offsetof(struct ixgbe_hw_stats, lxonrxc)},
> + {"tx_xoff_packets", offsetof(struct ixgbe_hw_stats, lxofftxc)},
> + {"rx_xoff_packets", offsetof(struct ixgbe_hw_stats, lxoffrxc)},
> + {"rx_size_64_packets", offsetof(struct ixgbe_hw_stats, prc64)},
> + {"rx_size_65_to_127_packets", offsetof(struct ixgbe_hw_stats,
> prc127)},
> + {"rx_size_128_to_255_packets", offsetof(struct ixgbe_hw_stats,
> prc255)},
> + {"rx_size_256_to_511_packets", offsetof(struct ixgbe_hw_stats,
> prc511)},
> + {"rx_size_512_to_1023_packets", offsetof(struct ixgbe_hw_stats,
> + prc1023)},
> + {"rx_size_1024_to_max_packets", offsetof(struct ixgbe_hw_stats,
> + prc1522)},
> + {"rx_broadcast_packets", offsetof(struct ixgbe_hw_stats, bprc)},
> + {"rx_multicast_packets", offsetof(struct ixgbe_hw_stats, mprc)},
> + {"rx_fragment_errors", offsetof(struct ixgbe_hw_stats, rfc)},
> + {"rx_undersize_errors", offsetof(struct ixgbe_hw_stats, ruc)},
> + {"rx_oversize_errors", offsetof(struct ixgbe_hw_stats, roc)},
> + {"rx_jabber_errors", offsetof(struct ixgbe_hw_stats, rjc)},
> + {"rx_managment_packets", offsetof(struct ixgbe_hw_stats, mngprc)},
> + {"rx_managment_dropped", offsetof(struct ixgbe_hw_stats, mngpdc)},
> + {"tx_managment_packets", offsetof(struct ixgbe_hw_stats, mngptc)},
> + {"rx_total_packets", offsetof(struct ixgbe_hw_stats, tpr)},
> + {"rx_total_bytes", offsetof(struct ixgbe_hw_stats, tor)},
> + {"tx_total_packets", offsetof(struct ixgbe_hw_stats, tpt)},
> + {"tx_size_64_packets", offsetof(struct ixgbe_hw_stats, ptc64)},
> + {"tx_size_65_to_127_packets", offsetof(struct ixgbe_hw_stats,
> ptc127)},
> + {"tx_size_128_to_255_packets", offsetof(struct ixgbe_hw_stats,
> ptc255)},
> + {"tx_size_256_to_511_packets", offsetof(struct ixgbe_hw_stats,
> ptc511)},
> + {"tx_size_512_to_1023_packets", offsetof(struct ixgbe_hw_stats,
> + ptc1023)},
> + {"tx_size_1024_to_max_packets", offsetof(struct ixgbe_hw_stats,
> + ptc1522)},
> + {"tx_multicast_packets",
[dpdk-dev] [PATCH v3 05/11] igb: add xstats() implementation
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren > Sent: Thursday, October 22, 2015 4:48 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v3 05/11] igb: add xstats() implementation > > Add xstats_get() and xstats_reset() functions to igb driver, and the > neccessary > strings to expose these NIC statistics. > > Signed-off-by: Harry van Haaren Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH v3 09/11] i40e: add xstats() implementation
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren > Sent: Thursday, October 22, 2015 4:49 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v3 09/11] i40e: add xstats() implementation > > Add xstats functions to i40e PMD, allowing extended statistics to be retrieved > from the NIC and exposed to the DPDK. > > Signed-off-by: Harry van Haaren Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH v3 03/11] ethdev: update xstats_get() strings and Q handling
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Tahhan, Maryam
> Sent: Friday, October 23, 2015 3:35 PM
> To: Van Haaren, Harry ; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 03/11] ethdev: update xstats_get() strings
> and Q handling
>
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren
> > Sent: Thursday, October 22, 2015 4:48 PM
> > To: dev at dpdk.org
> > Subject: [dpdk-dev] [PATCH v3 03/11] ethdev: update xstats_get()
> > strings and Q handling
> >
> > Update the strings used for presenting stats to adhere to the scheme
> > previously presented. Updated xstats_get() function to handle Q
> > information only if xstats() is not implemented in the PMD, providing
> > the PMD with the needed flexibility to expose its extended Q stats.
> >
> > Signed-off-by: Harry van Haaren
> > ---
> > lib/librte_ether/rte_ethdev.c | 38
> > +++---
> > 1 file changed, 23 insertions(+), 15 deletions(-)
> >
> > diff --git a/lib/librte_ether/rte_ethdev.c
> > b/lib/librte_ether/rte_ethdev.c index f593f6e..07f0c26 100644
> > --- a/lib/librte_ether/rte_ethdev.c
> > +++ b/lib/librte_ether/rte_ethdev.c
> > @@ -137,27 +137,30 @@ struct rte_eth_xstats_name_off { };
> >
> > static const struct rte_eth_xstats_name_off rte_stats_strings[] = {
> > - {"rx_packets", offsetof(struct rte_eth_stats, ipackets)},
> > - {"tx_packets", offsetof(struct rte_eth_stats, opackets)},
> > - {"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_good_packets", offsetof(struct rte_eth_stats, ipackets)},
> > + {"tx_good_packets", offsetof(struct rte_eth_stats, opackets)},
> > + {"rx_good_bytes", offsetof(struct rte_eth_stats, ibytes)},
> > + {"tx_good_bytes", offsetof(struct rte_eth_stats, obytes)},
>
> Hi Harry
> If there are any apps today scraping the existing stats this will cause an
> issue.
> I think the "good" description breaks the meaning of the various stats defined
> in the header. For example rx_packets is Total number of successfully
> received packets rather than the "good" packets.
>
> BR
> Maryam
> > {"rx_errors", offsetof(struct rte_eth_stats, ierrors)},
My bad, was quoting the wrong stats register... I'm happy with this change
[dpdk-dev] [PATCH v3 03/11] ethdev: update xstats_get() strings and Q handling
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren > Sent: Thursday, October 22, 2015 4:48 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v3 03/11] ethdev: update xstats_get() strings and > Q handling > > Update the strings used for presenting stats to adhere to the scheme > previously presented. Updated xstats_get() function to handle Q information > only if xstats() is not implemented in the PMD, providing the PMD with the > needed flexibility to expose its extended Q stats. > > Signed-off-by: Harry van Haaren Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH v3 10/11] i40evf: add xstats() implementation
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren > Sent: Thursday, October 22, 2015 4:49 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v3 10/11] i40evf: add xstats() implementation > > Add implementation of xstats() functions in i40evf PMD. > > Signed-off-by: Harry van Haaren > --- Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH v3 08/11] ixgbevf: add xstats() functions to VF
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren > Sent: Thursday, October 22, 2015 4:49 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v3 08/11] ixgbevf: add xstats() functions to VF > > Add xstats() functions and stat strings as necessary to ixgbevf PMD. > > Signed-off-by: Harry van Haaren > --- Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH v3 06/11] igbvf: add xstats() implementation
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren > Sent: Thursday, October 22, 2015 4:48 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v3 06/11] igbvf: add xstats() implementation > > Add xstats functionality to igbvf PMD, adding necessary statistic strings. > > Signed-off-by: Harry van Haaren > --- Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH v3 1/3] rte: add keep alive functionality
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton > Sent: Wednesday, October 28, 2015 8:52 AM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v3 1/3] rte: add keep alive functionality > > Adds functions for detecting and reporting the live-ness of LCores, the > primary requirement of which is minimal overheads for the > core(s) being checked. Core failures are notified via an application defined > callback. > > Signed-off-by: Remy Horton > --- Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH v3 3/3] example: add keep alive sample application
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton > Sent: Wednesday, October 28, 2015 8:52 AM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v3 3/3] example: add keep alive sample > application > > Modification of l2fwd to demonstrate keep-alive functionality. > > Signed-off-by: Remy Horton > --- Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH v3 04/11] virtio: add xstats() implementation
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren > Sent: Thursday, October 22, 2015 4:48 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v3 04/11] virtio: add xstats() implementation > > Add xstats() functions and statistic strings to virtio PMD. > > Signed-off-by: Harry van Haaren > --- Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH v3 11/11] fm10k: add xstats() implementation
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren > Sent: Thursday, October 22, 2015 4:49 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v3 11/11] fm10k: add xstats() implementation > > Add xstats() functions and statistic strings. > > Signed-off-by: Harry van Haaren > --- Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH v4 00/10] Port XStats
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren > Sent: Friday, October 30, 2015 11:36 AM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v4 00/10] Port XStats > > This patchset adds an implementation of the xstats_get() and xstats_reset() > API to the following PMDs: virtio, igb, igbvf, ixgbe, ixgbevf, i40e, i40evf > and > fm10k. > > The xstats API allows DPDK apps to gain access to extended statistics from > each port on a NIC. These statistics are structured as per a scheme detailed > in > the patch for the doc/prog_guide. > > v4: Consistency of names, refactored Q stat code > v3: Added more stats to PMDs > v2: Send correct patchset > > Harry van Haaren (10): > doc: add extended statistics to prog_guide > ethdev: update xstats_get() strings and Q handling > virtio: add xstats() implementation > igb: add xstats() implementation > igbvf: add xstats() implementation > ixgbe: add extended statistic strings > ixgbevf: add xstats() implementation > i40e: add xstats() implementation > i40evf: add xstats() implementation > fm10k: add xstats() implementation > > doc/guides/prog_guide/poll_mode_drv.rst | 51 ++- > doc/guides/rel_notes/release_2_2.rst| 14 ++ > drivers/net/e1000/igb_ethdev.c | 193 ++-- > drivers/net/fm10k/fm10k_ethdev.c| 87 +++ > drivers/net/i40e/i40e_ethdev.c | 217 > ++- > drivers/net/i40e/i40e_ethdev_vf.c | 89 ++- > drivers/net/ixgbe/ixgbe_ethdev.c| 254 > +++- > drivers/net/virtio/virtio_ethdev.c | 98 +++- > drivers/net/virtio/virtio_rxtx.c| 32 > drivers/net/virtio/virtqueue.h | 4 + > lib/librte_ether/rte_ethdev.c | 38 +++-- > 11 files changed, 1006 insertions(+), 71 deletions(-) > > -- > 1.9.1 Series-acked-by: Maryam Tahhan
[dpdk-dev] ixgbe: account more Rx errors Issue
> From: Andriy Berestovskyy [mailto:aber at semihalf.com] > Sent: Friday, September 4, 2015 10:38 AM > To: Tahhan, Maryam; dev at dpdk.org > Subject: ixgbe: account more Rx errors Issue > > Hi, > Updating to DPDK 2.1 I noticed an issue with the ixgbe stats. > > In commit f6bf669b9900 "ixgbe: account more Rx errors" we add XEC > hardware counter (l3_l4_xsum_error) to the ierrors now. The issue is the > UDP packets with zero check sum are counted in XEC and now in ierrors too. > > I've tried to disable hw_ip_checksum in rxmode, but it didn't help. > > I'm not sure we should add XEC to ierrors, because packets counted in XEC > are not dropped by the NIC actually. So in my case ierrors counter is now > greater than actual number of packets received by the NIC, which makes no > sense. > > What's your opinion? Hi Andriy Thanks for flagging this, I'm aware of this phenomenon, unfortunately it means we are hitting 2 hw registers on the NIC. XEC counts the Number of receive IPv4, TCP, UDP or SCTP XSUM errors And general crc errors counts Counts the number of receive packets with CRC errors. In order for a packet to be counted in this register, it must be 64 bytes or greater (from through , inclusively) in length. This register counts all packets received, regardless of L2 filtering and receive enablement So our options are we can: 1. Add only one of these into the error stats. 2. We can introduce some cooking of stats in this scenario, so only add either or if they are equal or one is higher than the other. 3. Add them all which means you can have more errors than the number of received packets, but TBH this is going to be the case if your packets have multiple errors anyway. I'm happy to go with either 1, 2 or 3 but would like some more feedback from the community on this front. Regards Maryam > Regards, > Andriy
[dpdk-dev] ixgbe: account more Rx errors Issue
> From: Andriy Berestovskyy [mailto:aber at semihalf.com] > Sent: Friday, September 4, 2015 5:59 PM > To: Tahhan, Maryam > Cc: dev at dpdk.org; Olivier MATZ > Subject: Re: ixgbe: account more Rx errors Issue > > Hi Maryam, > Please see below. > > > XEC counts the Number of receive IPv4, TCP, UDP or SCTP XSUM errors > > Please note than UDP checksum is optional for IPv4, but UDP packets with > zero checksum hit XEC. > I understand, but this is what the hardware register is picking up and what I included previously is the definitions of the registers from the datasheet. > > And general crc errors counts Counts the number of receive packets with > CRC errors. > > Let me explain you with an example. > > DPDK 2.0 behavior: > host A sends 10M IPv4 UDP packets (no checksum) to host B host B stats: 9M > ipackets + 1M ierrors (missed) = 10M > > DPDK 2.1 behavior: > host A sends 10M IPv4 UDP packets (no checksum) to host B host B stats: 9M > ipackets + 11M in ierrors (1M missed + 10M XEC) = 20M? Because it's hitting the 2 error registers. If you had packets with multiple errors that are added up as part of ierrors you'll still be getting more than 10M errors which is why I asked for feedback on the 3 suggestions below. What I'm saying is the number of errors being > the number of received packets will be seen if you hit multiple error registers on the NIC. > > > So our options are we can: > > 1. Add only one of these into the error stats. > > 2. We can introduce some cooking of stats in this scenario, so only add > either or if they are equal or one is higher than the other. > > 3. Add them all which means you can have more errors than the number of > received packets, but TBH this is going to be the case if your packets have > multiple errors anyway. > > 4. ierrors should reflect NIC drops only. I may have misinterpreted this, but ierrors in rte_ethdev.h ierrors is defined as the Total number of erroneous received packets. Maybe we need a clear definition or a separate drop counter as I see uint64_t q_errors defined as: Total number of queue packets received that are dropped. > XEC does not count drops, so IMO it should be removed from ierrors. While it's picking up the 0 checksum as an error (which it shouldn't necessarily be doing), removing it could mean missing other valid L3/L4 checksum errors... Let me experiment some more with L3/L4 checksum errors and crcerrs to see if we can cook the stats around this register in particular. I would hate to remove it and miss genuine errors > > Please note that we still can access the XEC using rte_eth_xstats_get() > > > Regards, > Andriy
[dpdk-dev] ixgbe: account more Rx errors Issue
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Monday, September 7, 2015 9:30 AM > To: Tahhan, Maryam; Andriy Berestovskyy > Cc: dev at dpdk.org > Subject: Re: ixgbe: account more Rx errors Issue > > Hi, > > On 09/06/2015 07:15 PM, Tahhan, Maryam wrote: > >> From: Andriy Berestovskyy [mailto:aber at semihalf.com] > >> Sent: Friday, September 4, 2015 5:59 PM > >> To: Tahhan, Maryam > >> Cc: dev at dpdk.org; Olivier MATZ > >> Subject: Re: ixgbe: account more Rx errors Issue > >> > >> Hi Maryam, > >> Please see below. > >> > >>> XEC counts the Number of receive IPv4, TCP, UDP or SCTP XSUM errors > >> > >> Please note than UDP checksum is optional for IPv4, but UDP packets > >> with zero checksum hit XEC. > >> > > > > I understand, but this is what the hardware register is picking up and what > > I > included previously is the definitions of the registers from the datasheet. > > > >>> And general crc errors counts Counts the number of receive packets > >>> with > >> CRC errors. > >> > >> Let me explain you with an example. > >> > >> DPDK 2.0 behavior: > >> host A sends 10M IPv4 UDP packets (no checksum) to host B host B > >> stats: 9M ipackets + 1M ierrors (missed) = 10M > >> > >> DPDK 2.1 behavior: > >> host A sends 10M IPv4 UDP packets (no checksum) to host B host B > >> stats: 9M ipackets + 11M in ierrors (1M missed + 10M XEC) = 20M? > > > > Because it's hitting the 2 error registers. If you had packets with multiple > errors that are added up as part of ierrors you'll still be getting more than > 10M errors which is why I asked for feedback on the 3 suggestions below. > What I'm saying is the number of errors being > the number of received > packets will be seen if you hit multiple error registers on the NIC. > > > >> > >>> So our options are we can: > >>> 1. Add only one of these into the error stats. > >>> 2. We can introduce some cooking of stats in this scenario, so only > >>> add > >> either or if they are equal or one is higher than the other. > >>> 3. Add them all which means you can have more errors than the number > >>> of > >> received packets, but TBH this is going to be the case if your > >> packets have multiple errors anyway. > >> > >> 4. ierrors should reflect NIC drops only. > > > > I may have misinterpreted this, but ierrors in rte_ethdev.h ierrors is > > defined > as the Total number of erroneous received packets. > > Maybe we need a clear definition or a separate drop counter as I see > uint64_t q_errors defined as: Total number of queue packets received that > are dropped. > > > >> XEC does not count drops, so IMO it should be removed from ierrors. > > > > While it's picking up the 0 checksum as an error (which it shouldn't > > necessarily be doing), removing it could mean missing other valid > > L3/L4 checksum errors... Let me experiment some more with L3/L4 > > checksum errors and crcerrs to see if we can cook the stats around > > this register in particular. I would hate to remove it and miss > > genuine errors > > For me, the definition that looks the most straightforward is: > > ipackets = packets successfully received by hardware imissed = packets > dropped by hardware because the software does > not poll fast enough (= queue full) > ierrors = packets dropped by hardware (malformed packets, ...) > > These 3 stats never count twice the same packet. > > If we want more statistics, they could go in xstats. For instance, a counter > for > invalid checksum. The definition of these stats would be pmd-specific. > > I agree we should clarify and have a consensus on the definitions before going > further. > > > Regards, > Olivier Hi Olivier I think it's important to distinguish between errors and drops and provide a statistics API that exposes both. This way people have access to as much information as possible when things do go wrong and nothing is missed in terms of errors. My suggestion for the high level registers would be: ipackets = Total number of packets successfully received by hardware imissed = Total number of packets dropped by hardware because the software does not poll fast enough (= queue full) idrops = Total number of packets dropped by hardware (malformed packets, ...) Where the # of drops can ONLY be <= the packets received (without overlap between registers). ierrors = Total number of erroneous received packets. Where the # of errors can be >= the packets received (without overlap between registers), this is because there may be multiple errors associated with a packet. This way people can see how many packets were dropped and why at a high level as well as through the extended stats API rather than using one API or the other. What do you think? Best Regards Maryam > > > > > > >> > >> Please note that we still can access the XEC using > >> rte_eth_xstats_get() > >> > >> > >> Regards, > >> Andriy
[dpdk-dev] ixgbe: account more Rx errors Issue
> From: Kyle Larose [mailto:eomereadig at gmail.com] > Sent: Wednesday, September 9, 2015 6:43 PM > To: Tahhan, Maryam > Cc: Olivier MATZ; Andriy Berestovskyy; dev at dpdk.org > Subject: Re: [dpdk-dev] ixgbe: account more Rx errors Issue > > > On Mon, Sep 7, 2015 at 7:44 AM, Tahhan, Maryam > wrote: > > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > > Sent: Monday, September 7, 2015 9:30 AM > > To: Tahhan, Maryam; Andriy Berestovskyy > > Cc: dev at dpdk.org > > Subject: Re: ixgbe: account more Rx errors Issue > > > > Hi, > > > > On 09/06/2015 07:15 PM, Tahhan, Maryam wrote: > > >> From: Andriy Berestovskyy [mailto:aber at semihalf.com] > > >> Sent: Friday, September 4, 2015 5:59 PM > > >> To: Tahhan, Maryam > > >> Cc: dev at dpdk.org; Olivier MATZ > > >> Subject: Re: ixgbe: account more Rx errors Issue > > >> > > >> Hi Maryam, > > >> Please see below. > > >> > > >>> XEC counts the Number of receive IPv4, TCP, UDP or SCTP XSUM errors > > >> > > >> Please note than UDP checksum is optional for IPv4, but UDP packets > > >> with zero checksum hit XEC. > > >> > > > > > > I understand, but this is what the hardware register is picking up and > > > what I > > included previously is the definitions of the registers from the datasheet. > > > > > >>> And general crc errors counts Counts the number of receive packets > > >>> with > > >> CRC errors. > > >> > > >> Let me explain you with an example. > > >> > > >> DPDK 2.0 behavior: > > >> host A sends 10M IPv4 UDP packets (no checksum) to host B host B > > >> stats: 9M ipackets + 1M ierrors (missed) = 10M > > >> > > >> DPDK 2.1 behavior: > > >> host A sends 10M IPv4 UDP packets (no checksum) to host B host B > > >> stats: 9M ipackets + 11M in ierrors (1M missed + 10M XEC) = 20M? > > > > > > Because it's hitting the 2 error registers. If you had packets with > > > multiple > > errors that are added up as part of ierrors you'll still be getting more > > than > > 10M errors which is why I asked for feedback on the 3 suggestions below. > > What I'm saying is the number of errors being > the number of received > > packets will be seen if you hit multiple error registers on the NIC. > > > > > >> > > >>> So our options are we can: > > >>> 1. Add only one of these into the error stats. > > >>> 2. We can introduce some cooking of stats in this scenario, so only > > >>> add > > >> either or if they are equal or one is higher than the other. > > >>> 3. Add them all which means you can have more errors than the number > > >>> of > > >> received packets, but TBH this is going to be the case if your > > >> packets have multiple errors anyway. > > >> > > >> 4. ierrors should reflect NIC drops only. > > > > > > I may have misinterpreted this, but ierrors in rte_ethdev.h ierrors is > > > defined > > as the Total number of erroneous received packets. > > > Maybe we need a clear definition or a separate drop counter as I see > > uint64_t q_errors defined as: Total number of queue packets received that > > are dropped. > > > > > >> XEC does not count drops, so IMO it should be removed from ierrors. > > > > > > While it's picking up the 0 checksum as an error (which it shouldn't > > > necessarily be doing), removing it could mean missing other valid > > > L3/L4 checksum errors... Let me experiment some more with L3/L4 > > > checksum errors and crcerrs to see if we can cook the stats around > > > this register in particular. I would hate to remove it and miss > > > genuine errors > > > > For me, the definition that looks the most straightforward is: > > >> ipackets = packets successfully received by hardware imissed = packets > > dropped by hardware because the software does > >? ?not poll fast enough (= queue full) > > ierrors = packets dropped by hardware (malformed packets, ...) > > > > These 3 stats never count twice the same packet. > > > > If we want more statistics, they could go in xstats. For instance, a > > counter for > > invalid checksum. The definition of these stats would be pmd-specific. > > > > I agree we should clarify and have a consensus on the de
[dpdk-dev] [PATCH v2 1/1] ethdev: distinguish between drop and error stats
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Tuesday, November 3, 2015 1:56 PM > To: Tahhan, Maryam > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 1/1] ethdev: distinguish between drop and > error stats > > 2015-11-03 13:14, Tahhan, Maryam: > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > There was not a lot of comments on this proposal. > > > If the idea is well received, it needs to be implemented in drivers, > > > at least few of them. > > > > Hi Thomas > > I was waiting to implement it across the drivers when I knew it was going to > be merged to master. It shouldn't break anything that's currently using ethdev > stats and should be relatively straight forward to implement for the drivers. > Does it makes sense to add it into drivers now or wait for the next open DPDK > window? > > It's better to change API only when it is used by some drivers. > We can wait the release 2.3 to integrate it. Ok, I will wait for 2.3 and integrate it with the drivers. Thanks.
[dpdk-dev] [PATCH v4 1/3] rte: add keep alive functionality
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton > Sent: Thursday, November 5, 2015 11:33 AM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v4 1/3] rte: add keep alive functionality > > Adds functions for detecting and reporting the live-ness of LCores, the > primary > requirement of which is minimal overheads for the > core(s) being checked. Core failures are notified via an application defined > callback. > > Signed-off-by: Remy Horton > --- Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH v4 3/3] example: add keep alive sample application
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton > Sent: Thursday, November 5, 2015 11:33 AM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v4 3/3] example: add keep alive sample application > > Modified version of l2fwd to demonstrate keep-alive functionality. > > Signed-off-by: Remy Horton > --- Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH 3/3] i40e: refactor xstats queue handling
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren > Sent: Friday, November 6, 2015 2:13 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH 3/3] i40e: refactor xstats queue handling > > This patch refactors the queue and priority statistic handling. > Generic queue stats are presented by rte_eth_xstats_get(), and the > i40e_xstats_get() exposes only the extra stats. > > Signed-off-by: Harry van Haaren > --- Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH 2/3] ixgbe: refactor xstats queue handling
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren > Sent: Friday, November 6, 2015 2:13 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH 2/3] ixgbe: refactor xstats queue handling > > This patch refactors the queue handling. Generic queue stats are handled by > rte_eth_xstats_get() and the ixgbe_xstats_get() exposes only the extra stats. > > Signed-off-by: Harry van Haaren > --- Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH 1/3] ethdev: xstats generic Q stats refactor
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren > Sent: Friday, November 6, 2015 2:13 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH 1/3] ethdev: xstats generic Q stats refactor > > This patch refactors the generic queue stats to be exposed by > rte_ethdev_xstats_get(). > > Signed-off-by: Harry van Haaren > --- Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH v2] i40e: fix resetting of stats
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Harry van Haaren
> Sent: Thursday, November 5, 2015 12:56 PM
> To: dev at dpdk.org
> Cc: Harry van Haaren
> Subject: [dpdk-dev] [PATCH v2] i40e: fix resetting of stats
>
> This patch fixes a bug where only some of the statistics were being reset when
> calling rte_eth_stats_reset() or rte_eth_xstats_reset().
>
> This patch marks the VSI to update its offset, causing the stats be look like
> they
> are reset.
>
> Fixes: 9aace75fc82e ("i40e: fix statistics")
>
> Signed-off-by: Harry van Haaren
> ---
Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH] ethdev: distinguish between drop and error stats
Hi Jay
Yeah, I will do this to make the distinction clear for the counters I?m
modifying in my patch. But please note that these aren?t counters that are
defined in the NIC datasheets, these are the high level DPDK general counters
for ethdevs. Their descriptions exist in the code and the generated DPDK docs.
Best Regards,
Maryam
From: Jay Rolette [mailto:role...@infiniteio.com]
Sent: Friday, October 2, 2015 2:25 PM
To: Tahhan, Maryam
Cc: DPDK
Subject: Re: [dpdk-dev] [PATCH] ethdev: distinguish between drop and error stats
Can you improve the comments on these counters? If you didn't happen to follow
this thread, there's no way to reasonably figure out what the difference is
from looking at the code without chasing it all the way down and
cross-referencing the NIC datasheet.
Thanks,
Jay
On Fri, Oct 2, 2015 at 7:47 AM, Maryam Tahhan mailto:maryam.tahhan at intel.com>> wrote:
Make a distniction between dropped packets and error statistics to allow
a higher level fault management entity to interact with DPDK and take
appropriate measures when errors are detected. It will also provide
valuable information for any applications that collects/extracts DPDK
stats, such applications include Open vSwitch.
After this patch the distinction is:
ierrors = Total number of packets dropped by hardware (malformed
packets, ...) Where the # of drops can ONLY be <= the packets received
(without overlap between registers).
Rx_pkt_errors = Total number of erroneous received packets. Where the #
of errors can be >= the packets received (without overlap between
registers), this is because there may be multiple errors associated with
a packet.
Signed-off-by: Maryam Tahhan mailto:maryam.tahhan
at intel.com>>
---
lib/librte_ether/rte_ethdev.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 8a8c82b..53dd55d 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -200,8 +200,9 @@ struct rte_eth_stats {
/**< Deprecated; Total of RX packets with CRC error. */
uint64_t ibadlen;
/**< Deprecated; Total of RX packets with bad length. */
- uint64_t ierrors; /**< Total number of erroneous received packets. */
+ uint64_t ierrors; /**< Total number of dropped received packets. */
uint64_t oerrors; /**< Total number of failed transmitted packets. */
+ uint64_t ipkterrors; /**< Total number of erroneous received packets.
*/
uint64_t imcasts;
/**< Deprecated; Total number of multicast received packets. */
uint64_t rx_nombuf; /**< Total number of RX mbuf allocation failures. */
--
2.4.3
[dpdk-dev] Future Direction for rte_eth_stats_get()
Hi David Some of the stats were HW specific rather than generic stats that should be exposed through rte_eth_stats and were migrated to the xstats API. http://dpdk.org/ml/archives/dev/2015-June/019915.html. The naming was also not generic enough to cover some of the drivers and in some cases what was exposed was only a partial view of the relevant stats. They were marked as deprecated to deter people from using them in the future, but haven't been removed from all the driver implementations yet. The Registers that remain undeprecated are those considered to be generic. Which registers are you particularly interested in that have been deprecated? Can you elaborate on what you mean by " scenarios where a static view is expected " Thanks in advance. Best Regards, Maryam > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of David Harton > (dharton) > Sent: Wednesday, January 20, 2016 5:19 PM > To: dev at dpdk.org > Subject: [dpdk-dev] Future Direction for rte_eth_stats_get() > > I see that some of the rte_eth_stats have been marked deprecated in 2.2 > that are returned by rte_eth_stats_get(). Applications that utilize any > number of device types rely on functions like this one to debug I/O > issues. > > Is there a reason the stats have been deprecated? Why not keep the > stats in line with the standard linux practices such as rtnl_link_stats64? > > Note, using rte_eth_xstats_get() does not help for this particular scenario > because a common binary API is needed to communicate through > various layers and also provide a consistent view/meaning to users. The > xstats is excellent for debugging device specific scenarios but can't help > in scenarios where a static view is expected. > > Thanks, > Dave
[dpdk-dev] Future Direction for rte_eth_stats_get()
the missing fields to be added? Best Regards, Maryam > -Original Message- > From: David Harton (dharton) [mailto:dharton at cisco.com] > Sent: Friday, January 22, 2016 1:41 PM > To: Tahhan, Maryam ; dev at dpdk.org > Subject: RE: Future Direction for rte_eth_stats_get() > > Hi Maryam, > > Thanks for the pointer. I'll review the convo's. > > Consider I have an application that uses many(all?) of the DPDK drivers > and their netmap counterparts depending on configuration. The user > interface provides a set of I/O stats to help debug I/O issues and that > set is the same regardless of driver type. The set of stats provided > matches what linux provides today since netmap existed before dpdk. > > What I want to avoid is having an application that is driver independent > having to become driver dependent interpreting a bunch of strings > (from xstats) or worse the layer running at the data plane core that is > advertising the API needed by the application parsing those strings > because the application cannot change the upper layer. > > What if instead of passing strings and values a set of stat ids and value > are returned. At least this way the application can remain driver > agnostic versus having to parse a free form string that likely isn't the > same across driver types. > > Also, why wouldn't rte_eth_stats_get() align with linux which is the > defacto standard? I understand that not every driver may not support > every stat but that's ok. Just return 0 (pause frames, crc errors, etc). > It's not like the list of linux stats is ever growing or advertising an > absurd number of stats that aren't applicable to all drivers. > > Thanks again, > Dave > > > -Original Message- > > From: Tahhan, Maryam [mailto:maryam.tahhan at intel.com] > > Sent: Friday, January 22, 2016 6:08 AM > > To: David Harton (dharton) ; dev at dpdk.org > > Subject: RE: Future Direction for rte_eth_stats_get() > > > > Hi David > > Some of the stats were HW specific rather than generic stats that > > should be exposed through rte_eth_stats and were migrated to the > xstats API. > > http://dpdk.org/ml/archives/dev/2015-June/019915.html. The > naming was > > also not generic enough to cover some of the drivers and in some > cases > > what was exposed was only a partial view of the relevant stats. > > > > They were marked as deprecated to deter people from using them > in the > > future, but haven't been removed from all the driver > implementations yet. > > The Registers that remain undeprecated are those considered to be > generic. > > > > Which registers are you particularly interested in that have been > > deprecated? Can you elaborate on what you mean by " scenarios > where a > > static view is expected " > > > > Thanks in advance. > > > > Best Regards, > > Maryam > > > > > > > -Original Message- > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of David > Harton > > > (dharton) > > > Sent: Wednesday, January 20, 2016 5:19 PM > > > To: dev at dpdk.org > > > Subject: [dpdk-dev] Future Direction for rte_eth_stats_get() > > > > > > I see that some of the rte_eth_stats have been marked > deprecated in > > > 2.2 that are returned by rte_eth_stats_get(). Applications that > > > utilize any number of device types rely on functions like this one > > > to debug I/O issues. > > > > > > Is there a reason the stats have been deprecated? Why not keep > the > > > stats in line with the standard linux practices such as > > rtnl_link_stats64? > > > > > > Note, using rte_eth_xstats_get() does not help for this particular > > scenario > > > because a common binary API is needed to communicate through > various > > > layers and also provide a consistent view/meaning to users. The > > > xstats is excellent for debugging device specific scenarios but > > > can't > > help > > > in scenarios where a static view is expected. > > > > > > Thanks, > > > Dave
[dpdk-dev] Future Direction for rte_eth_stats_get()
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Friday, January 22, 2016 2:44 PM > To: Tahhan, Maryam ; David Harton > (dharton) > Cc: dev at dpdk.org; olivier.matz at 6wind.com; Van Haaren, Harry > > Subject: Re: [dpdk-dev] Future Direction for rte_eth_stats_get() > > 2016-01-22 14:18, Tahhan, Maryam: > > So what can be enabled again in struct rte_eth_stats from what was > already there is the equivalent of: > > * rx_length_errors > > * rx_crc_errors > > * rx_missed_errors - the deprecation notice was removed for this > field. > > * multicast > > > > What should be added in to distinguish between errors and drops. > struct rte_eth_stats : > > * rx_errors > > * tx_errors > > > > As for the detailed rx errors and tx errors I'm open to feedback from > you folks as to what should go in and what is too detailed. These > weren't in struct rte_eth_stats previously, they are available through > xstats and are uniformly named across the drivers. Oliver + Harry any > thoughts? > > > > David I assume you are looking for all the missing fields to be added? > > They are not missing. They just not exactly match ones having a long > history in Linux kernel. > Please let's avoid to blindly mimic others without thinking about > modern needs. > My bad wording - I apologise, but I agree we should consider what makes sense and what doesn't. > > > > From: David Harton > > > > > Is there a reason the stats have been deprecated? Why not > keep > > > > > the stats in line with the standard linux practices such as > > > > > rtnl_link_stats64?
[dpdk-dev] [PATCH] ethdev: fix statistics description
> > Hi, John & Greg > > Would you please give any opinion for this patch ? > > I have looked through all PMDs and found not all statistics items can be > supported by some NIC. > For example, rx_nombuf, q_ipackets, q_opackets, q_ibytes and q_obytes > are not supported by i40e. Queue stats should be supported by i40e as we have access to struct i40e_queue_stats this is a gap. Same for e1000. For me (from a stats perspective), we should be able to report everything that ethtool can report for the different kernel network drivers (as we have the same base driver code in DPDK). In other words, the DPDK stats API should provide the same set of stats as a standard networking interface would to an external monitoring tool in case we want to perform some sort of analytics on it afterwards. At a very minimum the top level stats should include: ipackets, opackets, ibytes, obytes, imissed, ierrors, oerrors. The queue stats in theory could be migrated to the xstats, it would require a lot of clean up in existing drivers which is why we didn't remove them when we did the original cleanup of the struct for the xstats API. > But when the function rte_eth_stats_get(uint8_t port_id, struct > rte_eth_stats *stats) is called for i40e PMD, Above un-supported statistics > item in output stats are zero, this is not real value. Agreed - should not output 0 for these. But should ensure where stats are possible to obtain, we support them in DPDK. > So far, there is no way to know whether an item in struct rte_eth_stats is > supported or not only from this structure definition. > Maybe some structure member can be added to indicate each of statistics > item valid or not. > But this means ABI change. Migrating the queue/nonstandard stats to the xstats API would fix this, the only issue is with the existing drivers that are unsupported fields with 0. > > In following list, I list statistics support details of all PMDs. > Hope it can be displayed in your screen. > Thanks for this, it's very helpful. I'm currently collating a list of the missing stats for e1000, ixgbe and i40e from DPDK. So this is very helpful. > Thanks > /Wei > > NIC ipackets opackets ibytes obytes imissed ierrors oerrors > rx_nombuf q_ipackets q_opacktes q_ibytesq_obytes q_errors > af_packet y y y y nny > n yyy y > y > bnx2x y y y y yyy > y nnn n > n > bnxt y y y y yyy > n yyy y > y > bonding y y y y yyy > y yyy y > y > cxgbe y y y y yyy > n yyy y > y > e1000(igb) y y y y yyy > n nnn n > n > e1000(igbvf) y y y y nnn > n nnn > n n > ena y y y y yyy > y nnn n > n > enic y y y y yyy > y nnn n > n > fm10k y y y y nnn > n yyy y > n > i40e y y y y yyy > n nnn n > n > i40evfy y y y nyy > n nnn n > n > ixgbe y y y y yyy > n yyy y > y > ixgbevf y y y y nnn > n nnn n > n > mlx4 y y y y nyy > y yyy y > y > mlx5 y y y y nyy > y yyy y > y > mpipe y y y y nyy > y yyy y > y > nfp y y y y yyy > y yyy y > n > null y y n n nny > n yyn n > y > pcap y y y y nny > n
[dpdk-dev] [PATCH] eal: avoid unnecessary conflicts over rte_config file
> Hi John,
>
> > Before this patch, DPDK used the file ~/.rte_config as a lock to
> > detect potential interference between multiple DPDK applications
> > running on the same machine. However, if a single user ran DPDK
> > applications concurrently on several different machines, and if the
> > user's home directory was shared between the machines via NFS, DPDK
> > would incorrectly detect conflicts for all but the first application
> > and abort them. This patch fixes the problem by incorporating the
> > machine name into the config file name (e.g., ~/.rte_hostname_config).
> >
> > Signed-off-by: John Ousterhout
> > ---
> > doc/guides/prog_guide/multi_proc_support.rst | 11 +++
> > lib/librte_eal/common/eal_common_proc.c | 8 ++--
> > lib/librte_eal/common/eal_filesystem.h | 15 +--
> > 3 files changed, 22 insertions(+), 12 deletions(-)
> >
> > diff --git a/doc/guides/prog_guide/multi_proc_support.rst
> > b/doc/guides/prog_guide/multi_proc_support.rst
> > index badd102..a54fa1c 100644
> > --- a/doc/guides/prog_guide/multi_proc_support.rst
> > +++ b/doc/guides/prog_guide/multi_proc_support.rst
> > @@ -129,10 +129,13 @@ Support for this usage scenario is provided
> > using the ``--file-prefix`` paramete
> >
> > By default, the EAL creates hugepage files on each hugetlbfs
> > filesystem using the rtemap_X filename, where X is in the range 0 to the
> maximum number of hugepages -1.
> > -Similarly, it creates shared configuration files, memory mapped in
> > each process, using the /var/run/.rte_config filename, -when run as
> > root (or $HOME/.rte_config when run as a non-root user; -if filesystem and
> device permissions are set up to allow this).
> > -The rte part of the filenames of each of the above is configurable using
> > the
> file-prefix parameter.
> > +Similarly, it creates shared configuration files, memory mapped in each
> process.
> > +When run as root, the name of the configuration file will be
> > +/var/run/.rte_*host*_config, where *host* is the name of the machine.
> > +When run as a non-root user, the the name of the configuration file
> > +will be $HOME/.rte_*host*_config (if filesystem and device permissions
> are set up to allow this).
> > +If the ``--file-prefix`` parameter has been specified, its value will
> > +be used in place of "rte" in the file names.
>
> I am not sure that we need to handle all such cases inside EAL.
> User can easily overcome that problem by just adding something like:
> --file-prefix=`uname -n`
> to his command-line.
> Konstantin
>
I agree with Konstantin, there's no need to include the hostname in the rte
config file + I'm not sure this will be backward compatible with existing DPDK
applications that use a secondary processes that use the config file (as in,
multiprocess DPDK applications in use would all need to be updated). What
Konstantin suggests fixes the issue you were encountering without breaking
backward compatibility.
Is addition, the hostname is not unique... you could in theory have 2 hosts
with the same hostname and encounter the issue you were seeing again.
> >
> > In addition to specifying the file-prefix parameter, any DPDK
> > applications that are to be run side-by-side must explicitly limit their
> memory use.
> > diff --git a/lib/librte_eal/common/eal_common_proc.c
> > b/lib/librte_eal/common/eal_common_proc.c
> > index 12e0fca..517aa0c 100644
> > --- a/lib/librte_eal/common/eal_common_proc.c
> > +++ b/lib/librte_eal/common/eal_common_proc.c
> > @@ -45,12 +45,8 @@ rte_eal_primary_proc_alive(const char
> > *config_file_path)
> >
> > if (config_file_path)
> > config_fd = open(config_file_path, O_RDONLY);
> > - else {
> > - char default_path[PATH_MAX+1];
> > - snprintf(default_path, PATH_MAX, RUNTIME_CONFIG_FMT,
> > -default_config_dir, "rte");
> > - config_fd = open(default_path, O_RDONLY);
> > - }
> > + else
> > + config_fd = open(eal_runtime_config_path(), O_RDONLY);
> > if (config_fd < 0)
> > return 0;
> >
> > diff --git a/lib/librte_eal/common/eal_filesystem.h
> > b/lib/librte_eal/common/eal_filesystem.h
> > index fdb4a70..4929aa3 100644
> > --- a/lib/librte_eal/common/eal_filesystem.h
> > +++ b/lib/librte_eal/common/eal_filesystem.h
> > @@ -41,7 +41,7 @@
> > #define EAL_FILESYSTEM_H
> >
> > /** Path of rte config file. */
> > -#define RUNTIME_CONFIG_FMT "%s/.%s_config"
> > +#define RUNTIME_CONFIG_FMT "%s/.%s_%s_config"
> >
> > #include
> > #include
> > @@ -59,11 +59,22 @@ eal_runtime_config_path(void)
> > static char buffer[PATH_MAX]; /* static so auto-zeroed */
> > const char *directory = default_config_dir;
> > const char *home_dir = getenv("HOME");
> > + static char nameBuffer[1000];
> > + int result;
> >
> > if (getuid() != 0 && home_dir != NULL)
> > directory = home_dir;
> > +
> > + /*
> > +* Include the name of the host in the config file
[dpdk-dev] [PATCH] doc: announce xstats api change for 16.07
> From: Van Haaren, Harry > Sent: Tuesday, April 5, 2016 6:58 PM > To: dev at dpdk.org > Cc: Tahhan, Maryam ; Van Haaren, Harry > > Subject: [PATCH] doc: announce xstats api change for 16.07 > > This patch adds a notice that the API for the xstats functionality will be > modified in the 16.07 release, with no backwards compatibility planned > as it would require code duplication in each PMD that supports xstats. > > Signed-off-by: Harry van Haaren > --- Acked-by: Maryam Tahhan
[dpdk-dev] [RFC PATCH v1 0/3] Remove string operations from xstats
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of David Harton > (dharton) > Sent: Wednesday, April 20, 2016 5:04 PM > To: Horton, Remy ; dev at dpdk.org > Subject: Re: [dpdk-dev] [RFC PATCH v1 0/3] Remove string operations > from xstats > > > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton > > Sent: Friday, April 15, 2016 10:44 AM > > To: dev at dpdk.org > > Subject: [dpdk-dev] [RFC PATCH v1 0/3] Remove string operations from > > xstats > > > > The current extended ethernet statistics fetching involve doing > > several string operations, which causes performance issues if there > > are lots of statistics and/or network interfaces. This RFC patchset > > changes the API for xstats to use integer identifiers instead of > > strings and implements this new API for the ixgbe driver. Others > drivers to follow. > > > > -- > > > > Since this will involve API & ABI breakage as previously advertised, > > there are several design assumptions that need consideration: > > > > *) id-name & id-value pairs for both lookup and query Permits > > out-of-order and non-contigious returning of names/ids/values, even > > though expected implmentations would in practice return items in > > sorted order by id. Is this sufficent/desirable future proofing? Idea > > is to allow possibility of drivers returning partial statistics. > > I believe forcing drivers to match to a common id-space will become > burdensome. If the stats id-space isn't common then matching strings is > probably just as sufficient as long as drivers don't add/remove stats ad > hoc between the time the device is initialized and removed. I'm not aware of drivers adding/removing the stats ad hoc? The idea is to have a common-id space otherwise it will be a free for all and we won't have alignment across the drivers. I don't see it being any more burdensome than having a common register naming across the board which is what is there today. The advantage being that you don't have to pull the strings every time. > > > > > *) Bulk name-id mapping lookup only > > At the moment individual lookup is not supported, as this would impose > > extra overheads on drivers. The assumption is that any end user would > > fetch all this data once on startup and then cache the mappings. > > I'm not sure I see the value of looking up a single stat from a user > perspective. I can see where the drivers might say that some stats are > less disruptive/etc but the user doesn't have that knowledge and > wouldn't know how to take advantage. Usually all stats are grabbed > multiple times and the changes noted during debug sessions. > I believe Remy's change doesn't suggest/support individual lookup. It is just a statement that we don't want to burden drivers with individual stats lookups. > > > > *) Replacement or additional API > > This patch replaces the current xstats API, but there is no inherant > > reason beyond maintainability why this funtionality could not be in > > addition rather than a replacement. What is consensus on this? > > I came to the conclusion that replacing the existing API isn't necessary > but rather extending it so backwards compatibility could be maintained > during the previous discussions on this topic. However, if we want to go > forward with cleaning up in order to reduce the support drivers provide > I'm all for it. > > I still believe the API we develop should follow an "ethtool stats like" > format as suggested earlier this year: > > extern int rte_eth_xstats_names_get(uint8_t port_id, > struct rte_eth_xstats_name *names, unsigned n); extern int > rte_eth_xstats_values_get(uint8_t port_id, > uint64_t *values, unsigned n); > > Again, these could be provided alongside the existing API or replace it. I'm struggling a bit here. This is really what Remy has posted http://dpdk.org/dev/patchwork/patch/12094/ or am I missing something obvious? > > I also like the idea you provided of a separate API to obtain the xstats > count rather than deriving the count by calling one of the above > functions with "dummy" values. +1 > > Again, I can provide the patches for the changes I've made that align > with this proposed API. I just never got any feedback on it when > requested previously. I believe time is not in our favour on this front. If you have patches can you post them, otherwise can you please review the patchset that Remy has posted? Thanks in advance. BR Maryam
[dpdk-dev] Future Direction for rte_eth_stats_get()
> From: David Harton (dharton) [mailto:dharton at cisco.com]
> Sent: Friday, February 5, 2016 9:16 PM
> To: Van Haaren, Harry ; Thomas
> Monjalon
> Cc: dev at dpdk.org; Tahhan, Maryam ;
> Mcnamara, John
> Subject: RE: [dpdk-dev] Future Direction for rte_eth_stats_get()
>
>
> > From: Van Haaren, Harry [mailto:harry.van.haaren at intel.com]
> >
> > > From: David Harton
> > > Subject: RE: [dpdk-dev] Future Direction for rte_eth_stats_get()
> > >
> > > Hi folks,
> > >
> > > I didn't see any follow up to this response.
> >
> > I think you may have missed one:
> > http://dpdk.org/ml/archives/dev/2016-January/032211.html
>
> Apologies Harry! I didn't see your original post because the IT gods had
> decided your response was "Junk" mail and it didn't make it to my
> dev_dpdk.org mail folder. :(
>
> A colleague actually pointed me to this post separately today. I've made
> the Junk mailer a little smarter now...hopefully.
>
>
> >
> > I'm looking at the enum thinking it will grow out of control.
> > Have you thought about adding metadata for RX / TX, PF / VF?
>
> Yes, after thinking about it more I think it could get crazy.
>
> >
> > If metadata info is added, it would make retrieving a set of
> > statistics based on a certain mask much easier. Do you think this may
> be of use?
>
> Actually, I put a fair bit of thought into things and then realized, why re-
> invent the wheel?
> Why not follow the ethtool stats model?
>
> struct rte_eth_xstats_name {
> char name[RTE_ETH_XSTATS_NAME_SIZE]; };
>
> extern int rte_eth_xtats_count(uint8_t port_id, unsigned *count); extern
> int rte_eth_xtats_strings(uint8_t port_id, unsigned count, struct
> rte_eth_xtats_name *names); extern int rte_eth_xtats_values(uint8_t
> port_id, unsigned count, uint64_t *values);
>
> The existing API could be left in-place and these could be added for folks
> that don't want to grab the strings all the time.
>
> The cons compared to providing an enum or extending struct
> rte_eth_stats are:
> - you have to perform a query immediately after the device is attached
> - doesn't require conformity...which has pros and cons
>
> I'm actually testing the changes above if folks think this would be a
> reasonable compromise I can patch them up.
>
I think this is a reasonable compromise.
> I still feel the feedback myself and others gave about rte_eth_stats_get()
> being closer to a standard MIB should get some consideration.
+1
> Applications that run with a number of different drivers/device types
> likely want to avoid having to create "xstats mapping tables" every time
> a new device pops out just so they can debug problems.
>
> Thanks,
> Dave
[dpdk-dev] [PATCH v4] eal: add function to check if primary proc alive
> From: Van Haaren, Harry > Sent: Tuesday, February 23, 2016 2:10 PM > To: david.marchand at 6wind.com > Cc: Tahhan, Maryam ; dev at dpdk.org; Van > Haaren, Harry > Subject: [PATCH v4] eal: add function to check if primary proc alive > > This patch adds a new function to the EAL API: > int rte_eal_primary_proc_alive(const char *path); > > The function indicates if a primary process is alive right now. > This functionality is implemented by testing for a write- lock on the > config file, and the function tests for a lock. > > The use case for this functionality is that a secondary process can wait > until a primary process starts by polling the function and waiting. When > the primary is running, the secondary continues to poll to detect if the > primary process has quit unexpectedly, the secondary process can detect > this. > > The RTE_MAGIC number is written to the shared config by the primary > process, this is the signal to the secondary process that the EAL is set up, > and ready to be used. The function > rte_eal_mcfg_complete() writes RTE_MAGIC. This has been delayed in > the EAL init proceedure, as the PCI probing in the primary process can > interfere with the secondary running. > > Signed-off-by: Harry van Haaren Acked-by: Maryam Tahhan
[dpdk-dev] [PATCH] ethdev: don't count missed packets in erroneous packets counter
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Thursday, March 17, 2016 4:41 PM
> To: Igor Ryzhov
> Cc: dev at dpdk.org; Tahhan, Maryam ;
> olivier.matz at 6wind.com
> Subject: Re: [dpdk-dev] [PATCH] ethdev: don't count missed packets in
> erroneous packets counter
>
> CC Maryam and Olivier who had discussions about imissed and other
> stats:
> http://dpdk.org/ml/archives/dev/2015-August/022905.html
> http://dpdk.org/ml/archives/dev/2015-September/023351.html
> http://dpdk.org/ml/archives/dev/2015-September/023612.html
>
> 2016-03-10 16:03, Igor Ryzhov:
> > Comment for "ierrors" counter says that it counts erroneous received
> packets. But for some reason "imissed" counter is added to "ierrors"
> counter in most drivers. It is a mistake, because missed packets are
> obviously not received. This patch fixes it.
>
> According to this patch
> http://dpdk.org/browse/dpdk/commit/?id=70bdb186
> imissed was kept in ierrors because of backward compatibility.
> I'm OK to remove imissed from ierrors.
>
> Fixes: 70bdb18657da ("ethdev: add Rx error counters for missed, badcrc
> and badlen packets")
> Fixes: 6bfe648406b5 ("i40e: add Rx error statistics")
> Fixes: 856505d303f4 ("cxgbe: add port statistics")
>
> Acked-by: Thomas Monjalon
Looks fine, but make sure to add an explicit comment in release notes somewhere
to flag the change. In case any apps were accounting for imissed as part of
ierrors like testpmd was:
- if (((stats->ierrors - stats->imissed) + stats->rx_nombuf) > 0)
{
+ if ((stats->ierrors + stats->rx_nombuf) > 0) {
printf(" RX-error:%"PRIu64"\n", stats->ierrors);
printf(" RX-nombufs: %14"PRIu64"\n",
stats->rx_nombuf);
[dpdk-dev] [RFC PATCH v2 0/3] Remove string operations from xstats
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton > Sent: Friday, May 6, 2016 12:11 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [RFC PATCH v2 0/3] Remove string operations from > xstats > > The current extended ethernet statistics fetching involve doing several > string operations, which causes performance issues if there are lots of > statistics and/or network interfaces. This RFC patchset changes the API > for xstats to use integer identifiers instead of strings and implements this > new API for the ixgbe driver. Others drivers to follow. > > -- > > v2 changes: > * Fetching xstats count now seperate API function > * Added #define constants for some magic numbers > * Fixed bug with virtual function count fetching > * For non-xstats-supporting drivers, queue stats returned > * Some refactoring/cleanups > * Removed index assumption from example > > > Remy Horton (3): > rte: change xstats to use integer keys > drivers/net/ixgbe: change xstats to use integer keys > examples/ethtool: add xstats display command > > drivers/net/ixgbe/ixgbe_ethdev.c | 98 > - > examples/ethtool/ethtool-app/ethapp.c | 57 +++ > lib/librte_ether/rte_ethdev.c | 100 > +- > lib/librte_ether/rte_ethdev.h | 55 +++ > 4 files changed, 284 insertions(+), 26 deletions(-) > > -- > 2.5.5 Looks Great overall. Is there a need to update prog_guide/poll_mode_drv.rst with the new mods? BR Maryam
[dpdk-dev] [RFC PATCH v2 1/3] rte: change xstats to use integer keys
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton > Sent: Friday, May 6, 2016 12:11 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [RFC PATCH v2 1/3] rte: change xstats to use integer > keys > > Signed-off-by: Remy Horton > --- Acked-by: Maryam Tahhan
[dpdk-dev] [RFC PATCH v2 1/3] rte: change xstats to use integer keys
>uint8_t stat_idx,
> @@ -1427,6 +1447,10 @@ struct eth_dev_ops {
> eth_stats_reset_t stats_reset; /**< Reset generic device
> statistics. */
> eth_xstats_get_t xstats_get;/**< Get extended device
> statistics. */
> eth_xstats_reset_t xstats_reset; /**< Reset extended
> device statistics. */
> + eth_xstats_names_t xstats_names;
> + /**< Get names of extended statistics. */
> + eth_xstats_count_t xstats_count;
> + /**< Get number of extended statistics. */
Hi Remy
While reviewing the second patch in this patchset I noticed you aren't actually
using
eth_xstats_count_t xstats_count in the eth_dev_ops to retrieve the count in
the driver.
Do you still need xstats_count?
BR
Maryam
[dpdk-dev] [RFC PATCH v2 2/3] drivers/net/ixgbe: change xstats to use integer id
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton > Sent: Friday, May 6, 2016 12:11 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [RFC PATCH v2 2/3] drivers/net/ixgbe: change xstats > to use integer id > > Signed-off-by: Remy Horton > --- Acked-by: Maryam Tahhan
[dpdk-dev] [RFC PATCH v2 3/3] examples/ethtool: add xstats display command
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Remy Horton > Sent: Friday, May 6, 2016 12:11 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [RFC PATCH v2 3/3] examples/ethtool: add xstats > display command > > Signed-off-by: Remy Horton > --- Acked-by: Maryam Tahhan
