* Re: [PATCH v3 1/3] lib: add information metrics library
From: Pattan, Reshma @ 2016-11-07 15:25 UTC (permalink / raw)
To: Horton, Remy; +Cc: 'dev@dpdk.org'
In-Reply-To: <3AEA2BF9852C6F48A459DA490692831F010C15B0@IRSMSX109.ger.corp.intel.com>
Hi,
> -----Original Message-----
> From: Pattan, Reshma
> Sent: Friday, November 4, 2016 4:43 PM
> To: Remy Horton <remy.horton@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v3 1/3] lib: add information metrics library
>
> Hi,
>
> Few comments below.
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Remy Horton
> > Sent: Friday, November 4, 2016 3:36 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH v3 1/3] lib: add information metrics
> > library
> >
> > This patch adds a new information metric library that allows other
> > modules to register named metrics and update their values. It is
> > intended to be independent of ethdev, rather than mixing ethdev and
> > non-ethdev information in xstats.
> >
> > Signed-off-by: Remy Horton <remy.horton@intel.com>
> > ---
> > +int
> > +
> > +int
> > +rte_metrics_get_values(int port_id,
> > + struct rte_stat_value *values,
> > + uint16_t capacity)
> > +{
> > + struct rte_metrics_meta_s *entry;
> > + struct rte_metrics_data_s *stats;
> > + const struct rte_memzone *memzone;
> > + uint16_t idx_name;
> > + int return_value;
> > +
> > + memzone = rte_memzone_lookup(RTE_METRICS_MEMZONE_NAME);
> > + /* If not allocated, fail silently */
> > + if (memzone == NULL)
> > + return 0;
> > + stats = memzone->addr;
> > + rte_spinlock_lock(&stats->lock);
> > +
> > + if (values != NULL) {
> > + if (capacity < stats->cnt_stats) {
> > + rte_spinlock_unlock(&stats->lock);
> > + return -ERANGE;
> > + }
> > + for (idx_name = 0; idx_name < stats->cnt_stats; idx_name++)
> > {
> > + entry = &stats->metadata[idx_name];
> > + values[idx_name].key = idx_name;
> > + values[idx_name].value = entry->value[port_id];
> > + }
>
> Here you also need to include logic to return values for port independent
> metrics.
>
> > diff --git a/lib/librte_metrics/rte_metrics.h
> > b/lib/librte_metrics/rte_metrics.h
> > new file mode 100644
> > index 0000000..6b75404
> > --- /dev/null
> > +++ b/lib/librte_metrics/rte_metrics.h
> > @@ -0,0 +1,204 @@
> > +/**
> > + * Statistic name
> > + */
> > +struct rte_metric_name {
> > + /** String describing statistic */
> > + char name[RTE_METRICS_MAX_NAME_LEN]; };
> > +
> > +
> > +/**
> > + * Statistic name.
> > + */
>
> Need to correct the description to "stats values" or "metric values."
>
> > +struct rte_stat_value {
>
> Need to change the name to rte_metric_value.
>
> > + /** Numeric identifier of statistic */
> > + uint16_t key;
> > + /** Value for statistic */
> > + uint64_t value;
> > +};
> > +
> > +
> > +/**
> > + * Initialises statistic module. This only has to be explicitly
> > +called
>
> Typo < Initialises>
To avoid confusion, here I mean to say "Initializes" should be used (i.e. US English) .
Also another non-related comment, you may need to add a comment about EMWA
near the mean bit rate calculation code.
>
> Thanks,
> Reshma
^ permalink raw reply
* Re: [PATCH] ixgbe: fix wrong VF TX registers
From: Thomas Monjalon @ 2016-11-07 15:22 UTC (permalink / raw)
To: Wenzhuo Lu; +Cc: dev
In-Reply-To: <1478451310-104316-1-git-send-email-wenzhuo.lu@intel.com>
2016-11-06 11:55, Wenzhuo Lu:
> The VF TX registers are using PF's addresses by mistake,
> correct them.
>
> Fixes: 0198848a47f5 ("ixgbe: add access to specific device info")
>
> Reported-by: Xuekun Hu <xuekun.hu@intel.com>
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Applied with other similar patches squashed, thanks
^ permalink raw reply
* [PATCH 1/3 v3] bnxt: use appropriate data type in bnxt_alloc_vnic_attributes
From: Ajit Khaparde @ 2016-11-07 15:12 UTC (permalink / raw)
To: dev; +Cc: Ferruh Yigit
In-Reply-To: <a5c6b4f5-f50e-fccc-d33d-ec50f182e36c@intel.com>
Prevent the arithmetic in bnxt_alloc_vnic_attributes from causing
any unintentional havoc because of the usage of a signed variable.
Coverity: 137874
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
--
v1: attempt did not seem complete.
v2: simplify the fix by redoing the fix for Coverity 127557
---
drivers/net/bnxt/bnxt_vnic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/bnxt/bnxt_vnic.c b/drivers/net/bnxt/bnxt_vnic.c
index 205a940..33fdde2 100644
--- a/drivers/net/bnxt/bnxt_vnic.c
+++ b/drivers/net/bnxt/bnxt_vnic.c
@@ -175,7 +175,7 @@ int bnxt_alloc_vnic_attributes(struct bnxt *bp)
struct rte_pci_device *pdev = bp->pdev;
const struct rte_memzone *mz;
char mz_name[RTE_MEMZONE_NAMESIZE];
- uint16_t entry_length = RTE_CACHE_LINE_ROUNDUP(
+ uint32_t entry_length = RTE_CACHE_LINE_ROUNDUP(
HW_HASH_INDEX_SIZE * sizeof(*vnic->rss_table) +
HW_HASH_KEY_SIZE);
uint16_t max_vnics;
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/3] bnxt: add a FALLTHROUGH comment in the cascading switch statement
From: Ajit Khaparde @ 2016-11-07 15:11 UTC (permalink / raw)
To: dev; +Cc: Ferruh Yigit
The cascading switch statement in bnxt_hwrm.c is missing the FALLTHROUGH
comment. Adding that.
Coverity: 127552
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
drivers/net/bnxt/bnxt_hwrm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index cf79fc6..07e7124 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -606,6 +606,7 @@ int bnxt_hwrm_ring_alloc(struct bnxt *bp,
switch (ring_type) {
case HWRM_RING_ALLOC_INPUT_RING_TYPE_TX:
req.queue_id = bp->cos_queue[0].id;
+ /* FALLTHROUGH */
case HWRM_RING_ALLOC_INPUT_RING_TYPE_RX:
req.ring_type = ring_type;
req.cmpl_ring_id =
--
1.8.3.1
^ permalink raw reply related
* [PATCH 3/3 v2] bnxt: remove support for few PCI IDs
From: Ajit Khaparde @ 2016-11-07 15:11 UTC (permalink / raw)
To: dev; +Cc: Ferruh Yigit
Some of the production parts will arrive after the 16.11 release.
Back off support for those devices. We will add these IDs again
at an appropriate time.
Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
--
v2: Adding Signed-off-by which seems to have fallen off in the previous attempt.
---
drivers/net/bnxt/bnxt_ethdev.c | 30 ------------------------------
1 file changed, 30 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 2a95031..035fe07 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -63,34 +63,19 @@ static const char bnxt_version[] =
#define BROADCOM_DEV_ID_57302 0x16c9
#define BROADCOM_DEV_ID_57304_PF 0x16ca
#define BROADCOM_DEV_ID_57304_VF 0x16cb
-#define BROADCOM_DEV_ID_57417_MF 0x16cc
#define BROADCOM_DEV_ID_NS2 0x16cd
-#define BROADCOM_DEV_ID_57311 0x16ce
-#define BROADCOM_DEV_ID_57312 0x16cf
#define BROADCOM_DEV_ID_57402 0x16d0
#define BROADCOM_DEV_ID_57404 0x16d1
#define BROADCOM_DEV_ID_57406_PF 0x16d2
#define BROADCOM_DEV_ID_57406_VF 0x16d3
#define BROADCOM_DEV_ID_57402_MF 0x16d4
#define BROADCOM_DEV_ID_57407_RJ45 0x16d5
-#define BROADCOM_DEV_ID_57412 0x16d6
-#define BROADCOM_DEV_ID_57414 0x16d7
-#define BROADCOM_DEV_ID_57416_RJ45 0x16d8
-#define BROADCOM_DEV_ID_57417_RJ45 0x16d9
#define BROADCOM_DEV_ID_5741X_VF 0x16dc
-#define BROADCOM_DEV_ID_57412_MF 0x16de
-#define BROADCOM_DEV_ID_57314 0x16df
-#define BROADCOM_DEV_ID_57317_RJ45 0x16e0
#define BROADCOM_DEV_ID_5731X_VF 0x16e1
-#define BROADCOM_DEV_ID_57417_SFP 0x16e2
-#define BROADCOM_DEV_ID_57416_SFP 0x16e3
-#define BROADCOM_DEV_ID_57317_SFP 0x16e4
#define BROADCOM_DEV_ID_57404_MF 0x16e7
#define BROADCOM_DEV_ID_57406_MF 0x16e8
#define BROADCOM_DEV_ID_57407_SFP 0x16e9
#define BROADCOM_DEV_ID_57407_MF 0x16ea
-#define BROADCOM_DEV_ID_57414_MF 0x16ec
-#define BROADCOM_DEV_ID_57416_MF 0x16ee
static struct rte_pci_id bnxt_pci_id_map[] = {
{ RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57301) },
@@ -103,28 +88,13 @@ static struct rte_pci_id bnxt_pci_id_map[] = {
{ RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57406_PF) },
{ RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57406_VF) },
{ RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57402_MF) },
- { RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57314) },
{ RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57407_RJ45) },
{ RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57404_MF) },
{ RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57406_MF) },
{ RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57407_SFP) },
{ RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57407_MF) },
- { RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57417_MF) },
- { RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57311) },
- { RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57312) },
- { RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57412) },
- { RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57414) },
- { RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57416_RJ45) },
- { RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57417_RJ45) },
{ RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_5741X_VF) },
- { RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57412_MF) },
- { RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57317_RJ45) },
{ RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_5731X_VF) },
- { RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57417_SFP) },
- { RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57416_SFP) },
- { RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57317_SFP) },
- { RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57414_MF) },
- { RTE_PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, BROADCOM_DEV_ID_57416_MF) },
{ .vendor_id = 0, /* sentinel */ },
};
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2] E1000: fix for forced speed/duplex config
From: Thomas Monjalon @ 2016-11-07 15:07 UTC (permalink / raw)
To: Ananda Sathyanarayana; +Cc: dev, Lu, Wenzhuo
In-Reply-To: <6A0DE07E22DDAD4C9103DF62FEBC090939340B93@shsmsx102.ccr.corp.intel.com>
> > From the code, it looks like, hw->mac.autoneg, variable is used to switch
> > between calling either autoneg function or forcing speed/duplex function. But
> > this variable is not modified in eth_em_start/eth_igb_start routines (it is always
> > set to 1) even while forcing the link speed.
> >
> > Following discussion thread has some more information on this
> >
> > http://dpdk.org/ml/archives/dev/2016-October/049272.html
> >
> > Signed-off-by: Ananda Sathyanarayana <ananda@versa-networks.com>
> Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Applied, thanks
^ permalink raw reply
* Re: [PATCH v2 00/10] net/virtio: fix queue reconfigure issue
From: Yao, Lei A @ 2016-11-07 15:05 UTC (permalink / raw)
To: Yuanhan Liu, dev@dpdk.org
Cc: Thomas Monjalon, Tan, Jianfeng, Kevin Traynor, Ilya Maximets,
Kyle Larose, Maxime Coquelin
In-Reply-To: <1478338865-26126-1-git-send-email-yuanhan.liu@linux.intel.com>
Tested-by: Lei Yao <lei.a.yao@intel.com>
- Apply patch to v16.11-rc2
- Compile: Pass
- OS: Ubuntu16.04 4.4.0-45-generic
- GCC: 5.4.0
Most of the basic Virtio related test cases are tested with this patch. No function issue found and no obvious performance drop. The following is the pass case list:
TC1: vhost/virtio PVP vector performance
TC2: vhost/virtio PVP normal path performance
TC3: vhost/virtio PVP mergeable path performance
TC7: vhost/virtio-net PVP ipv4 fwd normal path performance
TC8: vhost/virtio-net PVP ipv4 fwd mergeable path performance
TC9: vhost/virtio-net VM2VM iperf with TSO enabled performance
TC11: vhost/virtio-pmd PVP with 2q 2c vector performance
TC12: vhost/virtio-pmd PVP with 2q 1c vector performance
TC16: vhost/virtio1.0 PVP normal performance
TC17: vhost/virtio 1.0 PVP mergeable performance
TC18: vhost/virtio 1.0 PVP vector performance(should be same as normal)
TC19: dpdk vhost + virtio-pmd PVP vector performance
TC20: dpdk vhost + virtio-pmd PVP non-vector performance
TC21: dpdk vhost + virtio-pmd PVP mergeable performance
TC25: Test Vhost/virtio-pmd PVP vector performance with qemu2.5
TC26: Test Vhost/virtio-pmd PVP vector performance with qemu2.6
TC27: Test Vhost/virtio-pmd PVP vector performance with qemu2.7
test vhost-user reconnect with virtio-pmd
test virtio-pmd reconnect with vhost-user
test vhost-user reconnect with multi virtio-pmd
multi test virtio-pmd reconnect with vhost-user
-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Yuanhan Liu
Sent: Saturday, November 5, 2016 5:41 PM
To: dev@dpdk.org
Cc: Thomas Monjalon <thomas.monjalon@6wind.com>; Tan, Jianfeng <jianfeng.tan@intel.com>; Kevin Traynor <ktraynor@redhat.com>; Ilya Maximets <i.maximets@samsung.com>; Kyle Larose <klarose@sandvine.com>; Maxime Coquelin <maxime.coquelin@redhat.com>; Yuanhan Liu <yuanhan.liu@linux.intel.com>
Subject: [dpdk-dev] [PATCH v2 00/10] net/virtio: fix queue reconfigure issue
This patchset fixes few issues related to virtio queue reconfigure:
increase or shrink the queue number. The major issue and the reason behind is described with length details in patch 4 "net/virtio: allocate queue at init stage".
Those bugs can not be fixed by few lines of code, it's because the current driver init logic is quite wrong, that I need change quite many places to make it right. Meanwhile, I have already done my best to keep the changes being as minimal as possible, so that we could have fewer changes to break something else; also, it's would be easier for review.
v2: - fix two more minor issues regarding to queue enabling; see patch 9
and 10.
- refined commit log a bit.
Thanks.
--yliu
---
Yuanhan Liu (10):
net/virtio: revert fix restart
net/virtio: simplify queue memzone name
net/virtio: simplify queue allocation
net/virtio: allocate queue at init stage
net/virtio: initiate vring at init stage
net/virtio: move queue configure code to proper place
net/virtio: complete init stage at the right place
net/virtio: remove started field
net/virtio: fix less queues being enabled issue
net/virtio: fix multiple queue enabling
drivers/net/virtio/virtio_ethdev.c | 248 +++++++++++++++++-------------- drivers/net/virtio/virtio_ethdev.h | 16 --
drivers/net/virtio/virtio_pci.h | 3 +-
drivers/net/virtio/virtio_rxtx.c | 291 ++++++++++++-------------------------
drivers/net/virtio/virtqueue.h | 7 +
5 files changed, 237 insertions(+), 328 deletions(-)
--
1.9.0
^ permalink raw reply
* Re: [PATCH 0/3] vhost: comments and doc update due to vhost-cuse removal
From: Thomas Monjalon @ 2016-11-07 15:00 UTC (permalink / raw)
To: Yuanhan Liu; +Cc: dev
In-Reply-To: <1478056501-9321-1-git-send-email-yuanhan.liu@linux.intel.com>
2016-11-02 11:14, Yuanhan Liu:
> Here is a small patchset of updating vhost programming and sample guide
> and comments, due to the removal of vhost-cuse.
Applied, thanks
^ permalink raw reply
* Re: [PATCH v2 00/10] net/virtio: fix queue reconfigure issue
From: Thomas Monjalon @ 2016-11-07 14:44 UTC (permalink / raw)
To: Yuanhan Liu
Cc: dev, Tan Jianfeng, Kevin Traynor, Ilya Maximets, Kyle Larose,
Maxime Coquelin
In-Reply-To: <1478338865-26126-1-git-send-email-yuanhan.liu@linux.intel.com>
2016-11-05 17:40, Yuanhan Liu:
> This patchset fixes few issues related to virtio queue reconfigure:
> increase or shrink the queue number. The major issue and the reason
> behind is described with length details in patch 4 "net/virtio: allocate
> queue at init stage".
>
> Those bugs can not be fixed by few lines of code, it's because the
> current driver init logic is quite wrong, that I need change quite many
> places to make it right. Meanwhile, I have already done my best to keep
> the changes being as minimal as possible, so that we could have fewer
> changes to break something else; also, it's would be easier for review.
Applied with a small fix and v3 of last patch, thanks
^ permalink raw reply
* Re: [PATCH v2 04/10] net/virtio: allocate queue at init stage
From: Thomas Monjalon @ 2016-11-07 14:23 UTC (permalink / raw)
To: Yuanhan Liu
Cc: dev, Tan Jianfeng, Kevin Traynor, Ilya Maximets, Kyle Larose,
Maxime Coquelin
In-Reply-To: <1478338865-26126-5-git-send-email-yuanhan.liu@linux.intel.com>
2016-11-05 17:40, Yuanhan Liu:
> -int virtio_dev_queue_setup(struct rte_eth_dev *dev,
> - int queue_type,
> - uint16_t queue_idx,
> - uint16_t vtpci_queue_idx,
> - uint16_t nb_desc,
> - unsigned int socket_id,
> - void **pvq)
> +static int
> +virtio_init_queue(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx)
> {
I've fixed an error with debug enabled:
drivers/net/virtio/virtio_ethdev.c:335:57: error:
‘nb_desc’ undeclared (first use in this function)
PMD_INIT_LOG(DEBUG, "vq_size: %u nb_desc:%u", vq_size, nb_desc);
^
^ permalink raw reply
* [PATCH v2] examples/ipsec-secgw: fix pointer to local outside scope
From: Fan Zhang @ 2016-11-07 14:22 UTC (permalink / raw)
To: dev; +Cc: sergio.gonzalez.monroy
In-Reply-To: <1478175163-229116-4-git-send-email-roy.fan.zhang@intel.com>
Coverity issue: 137871
Fixes: 0d547ed03717 ("examples/ipsec-secgw: support configuration file")
Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---
examples/ipsec-secgw/parser.c | 120 ++++++++++++++++++++----------------------
1 file changed, 58 insertions(+), 62 deletions(-)
diff --git a/examples/ipsec-secgw/parser.c b/examples/ipsec-secgw/parser.c
index ede08d8..f1afdb7 100644
--- a/examples/ipsec-secgw/parser.c
+++ b/examples/ipsec-secgw/parser.c
@@ -503,86 +503,82 @@ parse_cfg_file(const char *cfg_filename)
do {
char oneline[1024];
-
+ char *pos;
get_s = fgets(oneline, 1024, f);
- if (get_s) {
- char *pos;
- line_num++;
+ if (!get_s)
+ break;
- if (strlen(oneline) > 1022) {
- rte_panic("%s:%u: error: the line "
- "contains more characters the "
- "parser can handle\n",
- cfg_filename, line_num);
- goto error_exit;
- }
+ line_num++;
- /* process comment char '#' */
- if (oneline[0] == '#')
- continue;
+ if (strlen(oneline) > 1022) {
+ rte_panic("%s:%u: error: the line "
+ "contains more characters the "
+ "parser can handle\n",
+ cfg_filename, line_num);
+ goto error_exit;
+ }
- pos = strchr(oneline, '#');
- if (pos != NULL)
- *pos = '\0';
-
- /* process line concatenator '\' */
- pos = strchr(oneline, 92);
- if (pos != NULL) {
- if (pos != oneline+strlen(oneline) - 2) {
- rte_panic("%s:%u: error: no "
- "character should exist "
- "after '\\' symbol\n",
- cfg_filename, line_num);
- goto error_exit;
- }
-
- *pos = '\0';
-
- if (strlen(oneline) + strlen(str) > 1022) {
- rte_panic("%s:%u: error: the "
- "concatenated line "
- "contains more characters "
- "the parser can handle\n",
- cfg_filename, line_num);
- goto error_exit;
- }
-
- strncpy(str + strlen(str), oneline,
- strlen(oneline));
+ /* process comment char '#' */
+ if (oneline[0] == '#')
+ continue;
- continue;
+ pos = strchr(oneline, '#');
+ if (pos != NULL)
+ *pos = '\0';
+
+ /* process line concatenator '\' */
+ pos = strchr(oneline, 92);
+ if (pos != NULL) {
+ if (pos != oneline+strlen(oneline) - 2) {
+ rte_panic("%s:%u: error: no character "
+ "should exist after '\\'\n",
+ cfg_filename, line_num);
+ goto error_exit;
}
- /* copy the line to str and process */
+ *pos = '\0';
+
if (strlen(oneline) + strlen(str) > 1022) {
- rte_panic("%s:%u: error: the line "
- "contains more characters the "
- "parser can handle\n",
+ rte_panic("%s:%u: error: the "
+ "concatenated line contains more "
+ "characters the parser can "
+ "handle\n",
cfg_filename, line_num);
goto error_exit;
}
+
strncpy(str + strlen(str), oneline,
strlen(oneline));
- str[strlen(str)] = '\n';
- if (cmdline_parse(cl, str) < 0) {
- rte_panic("%s:%u: error: parsing \"%s\" "
- "failed\n", cfg_filename,
- line_num, str);
- goto error_exit;
- }
+ continue;
+ }
- if (status.status < 0) {
- rte_panic("%s:%u: error: %s",
- cfg_filename, line_num,
- status.parse_msg);
- goto error_exit;
- }
+ /* copy the line to str and process */
+ if (strlen(oneline) + strlen(str) > 1022) {
+ rte_panic("%s:%u: error: the line contains more "
+ "characters the parser can handle\n",
+ cfg_filename, line_num);
+ goto error_exit;
+ }
+ strncpy(str + strlen(str), oneline,
+ strlen(oneline));
+
+ str[strlen(str)] = '\n';
+ if (cmdline_parse(cl, str) < 0) {
+ rte_panic("%s:%u: error: parsing \"%s\" failed\n",
+ cfg_filename, line_num, str);
+ goto error_exit;
+ }
- memset(str, 0, 1024);
+ if (status.status < 0) {
+ rte_panic("%s:%u: error: %s", cfg_filename,
+ line_num, status.parse_msg);
+ goto error_exit;
}
- } while (get_s != NULL);
+
+ memset(str, 0, 1024);
+ } while (1);
cmdline_stdin_exit(cl);
fclose(f);
--
2.5.5
^ permalink raw reply related
* [PATCH v2] examples/ipsec-secgw: fix copy into fixed size buffer issue
From: Fan Zhang @ 2016-11-07 14:21 UTC (permalink / raw)
To: dev; +Cc: sergio.gonzalez.monroy
In-Reply-To: <1478175163-229116-3-git-send-email-roy.fan.zhang@intel.com>
Fixes: 0d547ed0 ("examples/ipsec-secgw: support configuration
file")
Coverity issue: 137875
Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---
examples/ipsec-secgw/sa.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index 9e2c8a9..8c4406c 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -170,15 +170,18 @@ static uint32_t
parse_key_string(const char *key_str, uint8_t *key)
{
const char *pt_start = key_str, *pt_end = key_str;
- char sub_str[3];
uint32_t nb_bytes = 0;
while (pt_end != NULL) {
+ char sub_str[3] = {0};
+
pt_end = strchr(pt_start, ':');
- if (pt_end == NULL)
- strncpy(sub_str, pt_start, strlen(pt_start));
- else {
+ if (pt_end == NULL) {
+ if (strlen(pt_start) > 2)
+ return 0;
+ strncpy(sub_str, pt_start, 2);
+ } else {
if (pt_end - pt_start > 2)
return 0;
--
2.5.5
^ permalink raw reply related
* Re: [PATCH] pci: fix one device probing
From: Thomas Monjalon @ 2016-11-07 13:49 UTC (permalink / raw)
To: Igor Ryzhov; +Cc: dev, David Marchand
In-Reply-To: <CALwxeUvPNv4HV+xNaRXkUeC3jidnzf6TYEVts6JHwT68W=82-A@mail.gmail.com>
2016-11-07 14:26, David Marchand:
> On Thu, Aug 4, 2016 at 1:50 PM, Igor Ryzhov <iryzhov@nfware.com> wrote:
> > The rte_eal_pci_probe_one function could return false positive result if
> > no driver is found for the device.
> >
> > Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
>
> Acked-by: David Marchand <david.marchand@6wind.com>
Applied, thanks
^ permalink raw reply
* Re: [PATCH] pci: do not mark device as taken when probe fails
From: Thomas Monjalon @ 2016-11-07 13:42 UTC (permalink / raw)
To: David Marchand; +Cc: dev, benjamin.walker
In-Reply-To: <1478509409-25508-1-git-send-email-david.marchand@6wind.com>
2016-11-07 10:03, David Marchand:
> dev->driver should be set only if a driver did take the device.
>
> Signed-off-by: David Marchand <david.marchand@6wind.com>
Applied, thanks
^ permalink raw reply
* Re: [PATCH] doc: fix typos
From: Mcnamara, John @ 2016-11-07 13:36 UTC (permalink / raw)
To: Alain Leon, dev@dpdk.org
In-Reply-To: <1478488819-15050-1-git-send-email-xerebz@gmail.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Alain Leon
> Sent: Monday, November 7, 2016 3:20 AM
> To: dev@dpdk.org
> Cc: Alain Leon <xerebz@gmail.com>
> Subject: [dpdk-dev] [PATCH] doc: fix typos
>
> Fixes typos present in the documentation and code comments.
>
> Signed-off-by: Alain Leon <xerebz@gmail.com>
Acked-by: John McNamara <john.mcnamara@intel.com>
^ permalink raw reply
* Re: [PATCH] pci: fix one device probing
From: David Marchand @ 2016-11-07 13:26 UTC (permalink / raw)
To: Igor Ryzhov; +Cc: dev@dpdk.org
In-Reply-To: <1470311406-38841-1-git-send-email-iryzhov@nfware.com>
On Thu, Aug 4, 2016 at 1:50 PM, Igor Ryzhov <iryzhov@nfware.com> wrote:
> The rte_eal_pci_probe_one function could return false positive result if
> no driver is found for the device.
>
> Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
> ---
> lib/librte_eal/common/eal_common_pci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
> index 7248c38..bfb6fd2 100644
> --- a/lib/librte_eal/common/eal_common_pci.c
> +++ b/lib/librte_eal/common/eal_common_pci.c
> @@ -344,7 +344,7 @@ rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
> continue;
>
> ret = pci_probe_all_drivers(dev);
> - if (ret < 0)
> + if (ret)
> goto err_return;
> return 0;
> }
Acked-by: David Marchand <david.marchand@6wind.com>
--
David Marchand
^ permalink raw reply
* [PATCH v4] latencystats: added new library for latency stats
From: Reshma Pattan @ 2016-11-07 13:14 UTC (permalink / raw)
To: dev; +Cc: Reshma Pattan
In-Reply-To: <1476711584-25377-1-git-send-email-reshma.pattan@intel.com>
Library is designed to calculate latency stats and report them to the
application when queried. Library measures minimum, average, maximum
latencies and jitter in nano seconds.
Current implementation supports global latency stats, i.e. per application
stats.
Added new field to mbuf struct to mark the packet arrival time on Rx and
use the timestamp to measure the latency on Tx.
Modified testpmd code to initialize/uninitialize latency stats calulation.
Modified dpdk-procinfo process to display the newly added metrics info.
This pacth is dependent on http://dpdk.org/dev/patchwork/patch/16927/ .
APIs:
Added APIs to initialize and un initialize latency stats
calculation.
Added API to retrieve latency stats names and values.
Functionality:
*Library will register ethdev Rx/Tx callbacks for each active port,
queue combinations.
*Library will register latency stats names with new metrics library.
http://dpdk.org/dev/patchwork/patch/16927/
*Rx packets will be marked with time stamp on each sampling interval.
*On Tx side, packets with time stamp will be considered for calculating
the minimum, maximum, average latencies and jitter.
*Average latency is calculated using exponential weighted moving average
method.
*Minimum and maximum latencies will be low and high latency values observed
so far.
*Jitter calculation is done based on inter packet delay variation.
*Measured stats are reported to the metrics library in a separate pthread.
*Measured stats can be retrieved via get API of the libray (or)
by calling generic get API of the new metrics library.
documents yet to be updated.
Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
MAINTAINERS | 4 +
app/proc_info/main.c | 70 ++++
app/test-pmd/testpmd.c | 10 +
config/common_base | 5 +
lib/Makefile | 1 +
lib/librte_latencystats/Makefile | 57 ++++
lib/librte_latencystats/rte_latencystats.c | 380 +++++++++++++++++++++
lib/librte_latencystats/rte_latencystats.h | 141 ++++++++
.../rte_latencystats_version.map | 10 +
lib/librte_mbuf/rte_mbuf.h | 3 +
mk/rte.app.mk | 2 +
11 files changed, 683 insertions(+)
create mode 100644 lib/librte_latencystats/Makefile
create mode 100644 lib/librte_latencystats/rte_latencystats.c
create mode 100644 lib/librte_latencystats/rte_latencystats.h
create mode 100644 lib/librte_latencystats/rte_latencystats_version.map
diff --git a/MAINTAINERS b/MAINTAINERS
index ba12d1b..2567448 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -702,3 +702,7 @@ F: examples/tep_termination/
F: examples/vmdq/
F: examples/vmdq_dcb/
F: doc/guides/sample_app_ug/vmdq_dcb_forwarding.rst
+
+Latency Stats
+M: Reshma Pattan <reshma.pattan@intel.com>
+F: lib/librte_latencystats/
diff --git a/app/proc_info/main.c b/app/proc_info/main.c
index 2c56d10..37d5ae4 100644
--- a/app/proc_info/main.c
+++ b/app/proc_info/main.c
@@ -57,6 +57,7 @@
#include <rte_atomic.h>
#include <rte_branch_prediction.h>
#include <rte_string_fns.h>
+#include <rte_metrics.h>
/* Maximum long option length for option parsing. */
#define MAX_LONG_OPT_SZ 64
@@ -68,6 +69,8 @@ static uint32_t enabled_port_mask;
static uint32_t enable_stats;
/**< Enable xstats. */
static uint32_t enable_xstats;
+/**< Enable metrics. */
+static uint32_t enable_metrics;
/**< Enable stats reset. */
static uint32_t reset_stats;
/**< Enable xstats reset. */
@@ -85,6 +88,8 @@ proc_info_usage(const char *prgname)
" --stats: to display port statistics, enabled by default\n"
" --xstats: to display extended port statistics, disabled by "
"default\n"
+ " --metrics: to display derived metrics of the ports, disabled by "
+ "default\n"
" --stats-reset: to reset port statistics\n"
" --xstats-reset: to reset port extended statistics\n",
prgname);
@@ -127,6 +132,7 @@ proc_info_parse_args(int argc, char **argv)
{"stats", 0, NULL, 0},
{"stats-reset", 0, NULL, 0},
{"xstats", 0, NULL, 0},
+ {"metrics", 0, NULL, 0},
{"xstats-reset", 0, NULL, 0},
{NULL, 0, 0, 0}
};
@@ -159,6 +165,10 @@ proc_info_parse_args(int argc, char **argv)
else if (!strncmp(long_option[option_index].name, "xstats",
MAX_LONG_OPT_SZ))
enable_xstats = 1;
+ else if (!strncmp(long_option[option_index].name,
+ "metrics",
+ MAX_LONG_OPT_SZ))
+ enable_metrics = 1;
/* Reset stats */
if (!strncmp(long_option[option_index].name, "stats-reset",
MAX_LONG_OPT_SZ))
@@ -301,6 +311,60 @@ nic_xstats_clear(uint8_t port_id)
printf("\n NIC extended statistics for port %d cleared\n", port_id);
}
+static void
+metrics_display(int port_id)
+{
+ struct rte_stat_value *stats;
+ struct rte_metric_name *names;
+ int len, ret;
+ static const char *nic_stats_border = "########################";
+
+ memset(&stats, 0, sizeof(struct rte_stat_value));
+ len = rte_metrics_get_names(NULL, 0);
+ if (len < 0) {
+ printf("Cannot get metrics count\n");
+ return;
+ }
+
+ stats = malloc(sizeof(struct rte_stat_value) * len);
+ if (stats == NULL) {
+ printf("Cannot allocate memory for metrics\n");
+ return;
+ }
+
+ names = malloc(sizeof(struct rte_metric_name) * len);
+ if (names == NULL) {
+ printf("Cannot allocate memory for metrcis names\n");
+ free(stats);
+ return;
+ }
+
+ if (len != rte_metrics_get_names(names, len)) {
+ printf("Cannot get metrics names\n");
+ free(stats);
+ free(names);
+ return;
+ }
+
+ printf("###### metrics for port %-2d #########\n", port_id);
+ printf("%s############################\n", nic_stats_border);
+ ret = rte_metrics_get_values(port_id, stats, len);
+ if (ret < 0 || ret > len) {
+ printf("Cannot get metrics values\n");
+ free(stats);
+ free(names);
+ return;
+ }
+
+ int i;
+ for (i = 0; i < len; i++)
+ printf("%s: %"PRIu64"\n", names[i].name, stats[i].value);
+
+ printf("%s############################\n", nic_stats_border);
+ free(stats);
+ free(names);
+}
+
int
main(int argc, char **argv)
{
@@ -360,8 +424,14 @@ main(int argc, char **argv)
nic_stats_clear(i);
else if (reset_xstats)
nic_xstats_clear(i);
+ else if (enable_metrics)
+ metrics_display(i);
}
}
+ /* print port independent stats */
+ if (enable_metrics)
+ metrics_display(RTE_METRICS_NONPORT);
+
return 0;
}
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 6185be6..0efab4e 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -78,6 +78,10 @@
#ifdef RTE_LIBRTE_PDUMP
#include <rte_pdump.h>
#endif
+#include <rte_metrics.h>
+#ifdef RTE_LIBRTE_LATENCY_STATS
+#include <rte_latencystats.h>
+#endif
#include "testpmd.h"
@@ -2070,6 +2074,9 @@ signal_handler(int signum)
/* uninitialize packet capture framework */
rte_pdump_uninit();
#endif
+#ifdef RTE_LIBRTE_LATENCY_STATS
+ rte_latencystats_uninit();
+#endif
force_quit();
/* exit with the expected status */
signal(signum, SIG_DFL);
@@ -2127,6 +2134,9 @@ main(int argc, char** argv)
/* set all ports to promiscuous mode by default */
FOREACH_PORT(port_id, ports)
rte_eth_promiscuous_enable(port_id);
+#ifdef RTE_LIBRTE_LATENCY_STATS
+ rte_latencystats_init(1, NULL);
+#endif
#ifdef RTE_LIBRTE_CMDLINE
if (interactive == 1) {
diff --git a/config/common_base b/config/common_base
index 21d18f8..d9002b8 100644
--- a/config/common_base
+++ b/config/common_base
@@ -589,3 +589,8 @@ CONFIG_RTE_APP_TEST_RESOURCE_TAR=n
CONFIG_RTE_TEST_PMD=y
CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES=n
CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS=n
+
+#
+# Compile the latency statistics library
+#
+CONFIG_RTE_LIBRTE_LATENCY_STATS=y
diff --git a/lib/Makefile b/lib/Makefile
index 990f23a..2111349 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -58,6 +58,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_TABLE) += librte_table
DIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += librte_pipeline
DIRS-$(CONFIG_RTE_LIBRTE_REORDER) += librte_reorder
DIRS-$(CONFIG_RTE_LIBRTE_PDUMP) += librte_pdump
+DIRS-$(CONFIG_RTE_LIBRTE_LATENCY_STATS) += librte_latencystats
ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
diff --git a/lib/librte_latencystats/Makefile b/lib/librte_latencystats/Makefile
new file mode 100644
index 0000000..f744da6
--- /dev/null
+++ b/lib/librte_latencystats/Makefile
@@ -0,0 +1,57 @@
+# BSD LICENSE
+#
+# Copyright(c) 2016 Intel Corporation. All rights reserved.
+# 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.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+# library name
+LIB = librte_latencystats.a
+
+CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3
+LDLIBS += -lm
+LDLIBS += -lpthread
+
+EXPORT_MAP := rte_latencystats_version.map
+
+LIBABIVER := 1
+
+# all source are stored in SRCS-y
+SRCS-$(CONFIG_RTE_LIBRTE_LATENCY_STATS) := rte_latencystats.c
+
+# install this header file
+SYMLINK-$(CONFIG_RTE_LIBRTE_LATENCY_STATS)-include := rte_latencystats.h
+
+# this lib depends upon:
+DEPDIRS-$(CONFIG_RTE_LIBRTE_LATENCY_STATS) += lib/librte_mbuf
+DEPDIRS-$(CONFIG_RTE_LIBRTE_LATENCY_STATS) += lib/librte_eal
+DEPDIRS-$(CONFIG_RTE_LIBRTE_LATENCY_STATS) += lib/librte_ether
+DEPDIRS-$(CONFIG_RTE_LIBRTE_LATENCY_STATS) += lib/librte_metrics
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_latencystats/rte_latencystats.c b/lib/librte_latencystats/rte_latencystats.c
new file mode 100644
index 0000000..0bcb09f
--- /dev/null
+++ b/lib/librte_latencystats/rte_latencystats.c
@@ -0,0 +1,380 @@
+/*-
+ * BSD LICENSE
+ *
+ * Copyright(c) 2016 Intel Corporation. All rights reserved.
+ * 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.
+ */
+
+#include <unistd.h>
+#include <sys/types.h>
+#include <stdbool.h>
+#include <math.h>
+#include <pthread.h>
+
+#include <rte_mbuf.h>
+#include <rte_log.h>
+#include <rte_cycles.h>
+#include <rte_ethdev.h>
+#include <rte_metrics.h>
+#include <rte_memzone.h>
+#include <rte_lcore.h>
+#include <rte_timer.h>
+
+#include "rte_latencystats.h"
+
+/** Nano seconds per second */
+#define NS_PER_SEC 1E9
+
+/** Clock cycles per nano second */
+#define CYCLES_PER_NS (rte_get_timer_hz() / NS_PER_SEC)
+
+/* Macros for printing using RTE_LOG */
+#define RTE_LOGTYPE_LATENCY_STATS RTE_LOGTYPE_USER1
+
+static pthread_t latency_stats_thread;
+static const char *MZ_RTE_LATENCY_STATS = "rte_latencystats";
+static int latency_stats_index;
+static uint64_t sampIntvl;
+static uint64_t timer_tsc;
+static uint64_t prev_tsc;
+
+struct rte_latency_stats {
+ float min_latency; /**< Minimum latency in nano seconds */
+ float avg_latency; /**< Average latency in nano seconds */
+ float max_latency; /**< Maximum latency in nano seconds */
+ float jitter; /** Latency variation between to packets */
+ uint64_t total_sampl_pkts;
+} *glob_stats;
+
+static struct rxtx_cbs {
+ struct rte_eth_rxtx_callback *cb;
+} rx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT],
+ tx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
+
+struct latency_stats_nameoff {
+ char name[RTE_ETH_XSTATS_NAME_SIZE];
+ unsigned int offset;
+};
+
+static const struct latency_stats_nameoff lat_stats_strings[] = {
+ {"min_latency_ns", offsetof(struct rte_latency_stats, min_latency)},
+ {"avg_latency_ns", offsetof(struct rte_latency_stats, avg_latency)},
+ {"max_latency_ns", offsetof(struct rte_latency_stats, max_latency)},
+ {"jitter_ns", offsetof(struct rte_latency_stats, jitter)},
+};
+
+#define NUM_LATENCY_STATS (sizeof(lat_stats_strings) / \
+ sizeof(lat_stats_strings[0]))
+
+static __attribute__((noreturn)) void *
+report_latency_stats(__rte_unused void *arg)
+{
+ for (;;) {
+ unsigned int i;
+ float *stats_ptr = NULL;
+ uint64_t values[NUM_LATENCY_STATS] = {0};
+ int ret;
+
+ for (i = 0; i < NUM_LATENCY_STATS; i++) {
+ stats_ptr = RTE_PTR_ADD(glob_stats,
+ lat_stats_strings[i].offset);
+ values[i] = (uint64_t)floor((*stats_ptr)/
+ CYCLES_PER_NS);
+ }
+
+ ret = rte_metrics_update_metrics(RTE_METRICS_NONPORT,
+ latency_stats_index,
+ values, NUM_LATENCY_STATS);
+ if (ret < 0)
+ RTE_LOG(INFO, LATENCY_STATS,
+ "Failed to push the stats\n");
+ }
+}
+
+static void
+rte_latencystats_fill_values(struct rte_stat_value *values)
+{
+ unsigned int i;
+ float *stats_ptr = NULL;
+
+ for (i = 0; i < NUM_LATENCY_STATS; i++) {
+ stats_ptr = RTE_PTR_ADD(glob_stats,
+ lat_stats_strings[i].offset);
+ values[i].key = i;
+ values[i].value = (uint64_t)floor((*stats_ptr)/
+ CYCLES_PER_NS);
+ }
+}
+
+static uint16_t
+add_time_stamps(uint8_t pid __rte_unused,
+ uint16_t qid __rte_unused,
+ struct rte_mbuf **pkts,
+ uint16_t nb_pkts,
+ uint16_t max_pkts __rte_unused,
+ void *user_cb __rte_unused)
+{
+ unsigned int i;
+ uint64_t diff_tsc, now;
+
+ /* for every sample interval,
+ * time stamp is marked on one received packet
+ */
+ now = rte_rdtsc();
+ for (i = 0; i < nb_pkts; i++) {
+ diff_tsc = now - prev_tsc;
+ timer_tsc += diff_tsc;
+ if (timer_tsc >= sampIntvl) {
+ pkts[i]->timestamp = now;
+ timer_tsc = 0;
+ }
+ prev_tsc = now;
+ now = rte_rdtsc();
+ }
+
+ return nb_pkts;
+}
+
+static uint16_t
+calc_latency(uint8_t pid __rte_unused,
+ uint16_t qid __rte_unused,
+ struct rte_mbuf **pkts,
+ uint16_t nb_pkts,
+ void *_ __rte_unused)
+{
+ unsigned int i, cnt = 0;
+ uint64_t now;
+ float latency[nb_pkts];
+ static float prev_latency;
+ const float alpha = 0.2;
+
+ now = rte_rdtsc();
+ for (i = 0; i < nb_pkts; i++) {
+ if (pkts[i]->timestamp)
+ latency[cnt++] = now - pkts[i]->timestamp;
+ }
+
+ for (i = 0; i < cnt; i++) {
+ /**
+ *The jitter is calculated as statistical mean of interpacket
+ *delay variation. The "jitter estimate" is computed by taking
+ *the absolute values of the ipdv sequence and applying an
+ *exponential filter with parameter 1/16 to generate the
+ *estimate. i.e J=J+(|D(i-1,i)|-J)/16. Where J is jitter,
+ *D(i-1,i) is difference in latency of two consecutive packets
+ *i-1 and i.
+ *Reference: Calculated as per RFC 5481, sec 4.1,
+ *RFC 3393 sec 4.5, RFC 1889 sec.
+ */
+ glob_stats->jitter += (abs(prev_latency - latency[i])
+ - glob_stats->jitter)/16;
+ if (glob_stats->min_latency == 0)
+ glob_stats->min_latency = latency[i];
+ else if (latency[i] < glob_stats->min_latency)
+ glob_stats->min_latency = latency[i];
+ else if (latency[i] > glob_stats->max_latency)
+ glob_stats->max_latency = latency[i];
+ /**
+ *The average latency is measured using exponential moving
+ *average, i.e. using EWMA
+ *https://en.wikipedia.org/wiki/Moving_average
+ */
+ glob_stats->avg_latency +=
+ alpha * (latency[i] - glob_stats->avg_latency);
+ glob_stats->total_sampl_pkts++;
+ prev_latency = latency[i];
+ }
+
+ return nb_pkts;
+}
+
+int
+rte_latencystats_init(uint64_t samp_intvl,
+ rte_latency_stats_flow_type_fn user_cb)
+{
+ unsigned int i;
+ uint8_t pid;
+ uint16_t qid;
+ struct rxtx_cbs *cbs = NULL;
+ const uint8_t nb_ports = rte_eth_dev_count();
+ const char *ptr_strings[NUM_LATENCY_STATS] = {0};
+ const struct rte_memzone *mz = NULL;
+ const unsigned int flags = 0;
+
+ /** Allocate stats in shared memory fo muliti process support */
+ mz = rte_memzone_reserve(MZ_RTE_LATENCY_STATS, sizeof(*glob_stats),
+ rte_socket_id(), flags);
+ if (mz == NULL) {
+ RTE_LOG(ERR, LATENCY_STATS, "Cannot reserve memory: %s:%d\n",
+ __func__, __LINE__);
+ return -ENOMEM;
+ }
+
+ glob_stats = mz->addr;
+ samp_intvl *= CYCLES_PER_NS;
+
+ /** Register latency stats with stats library */
+ for (i = 0; i < NUM_LATENCY_STATS; i++)
+ ptr_strings[i] = lat_stats_strings[i].name;
+
+ latency_stats_index = rte_metrics_reg_metrics(ptr_strings,
+ NUM_LATENCY_STATS);
+ if (latency_stats_index < 0) {
+ RTE_LOG(DEBUG, LATENCY_STATS,
+ "Failed to register latency stats names\n");
+ return -1;
+ }
+
+ /** Register Rx/Tx callbacks */
+ for (pid = 0; pid < nb_ports; pid++) {
+ struct rte_eth_dev_info dev_info;
+ rte_eth_dev_info_get(pid, &dev_info);
+ for (qid = 0; qid < dev_info.nb_rx_queues; qid++) {
+ cbs = &rx_cbs[pid][qid];
+ cbs->cb = rte_eth_add_first_rx_callback(pid, qid,
+ add_time_stamps, user_cb);
+ if (!cbs->cb)
+ RTE_LOG(INFO, LATENCY_STATS, "Failed to "
+ "register Rx callback for pid=%d, "
+ "qid=%d\n", pid, qid);
+ }
+ for (qid = 0; qid < dev_info.nb_tx_queues; qid++) {
+ cbs = &tx_cbs[pid][qid];
+ cbs->cb = rte_eth_add_tx_callback(pid, qid,
+ calc_latency, user_cb);
+ if (!cbs->cb)
+ RTE_LOG(INFO, LATENCY_STATS, "Failed to "
+ "register Tx callback for pid=%d, "
+ "qid=%d\n", pid, qid);
+ }
+ }
+
+ int ret = 0;
+ char thread_name[RTE_MAX_THREAD_NAME_LEN];
+
+ /** Create the host thread to update latency stats to stats library */
+ ret = pthread_create(&latency_stats_thread, NULL, report_latency_stats,
+ NULL);
+ if (ret != 0) {
+ RTE_LOG(ERR, LATENCY_STATS,
+ "Failed to create the latency stats thread:%s, %s:%d\n",
+ strerror(errno), __func__, __LINE__);
+ return -1;
+ }
+ /** Set thread_name for aid in debugging */
+ snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, "latency-stats-thread");
+ ret = rte_thread_setname(latency_stats_thread, thread_name);
+ if (ret != 0)
+ RTE_LOG(DEBUG, LATENCY_STATS,
+ "Failed to set thread name for latency stats handling\n");
+
+ return 0;
+}
+
+int
+rte_latencystats_uninit(void)
+{
+ uint8_t pid;
+ uint16_t qid;
+ int ret = 0;
+ struct rxtx_cbs *cbs = NULL;
+ const uint8_t nb_ports = rte_eth_dev_count();
+
+ /** De register Rx/Tx callbacks */
+ for (pid = 0; pid < nb_ports; pid++) {
+ struct rte_eth_dev_info dev_info;
+ rte_eth_dev_info_get(pid, &dev_info);
+ for (qid = 0; qid < dev_info.nb_rx_queues; qid++) {
+ cbs = &rx_cbs[pid][qid];
+ ret = rte_eth_remove_rx_callback(pid, qid, cbs->cb);
+ if (ret)
+ RTE_LOG(INFO, LATENCY_STATS, "failed to "
+ "remove Rx callback for pid=%d, "
+ "qid=%d\n", pid, qid);
+ }
+ for (qid = 0; qid < dev_info.nb_tx_queues; qid++) {
+ cbs = &tx_cbs[pid][qid];
+ ret = rte_eth_remove_tx_callback(pid, qid, cbs->cb);
+ if (ret)
+ RTE_LOG(INFO, LATENCY_STATS, "failed to "
+ "remove Tx callback for pid=%d, "
+ "qid=%d\n", pid, qid);
+ }
+ }
+
+ /** Cancel the thread */
+ ret = pthread_cancel(latency_stats_thread);
+ if (ret != 0) {
+ RTE_LOG(ERR, LATENCY_STATS,
+ "Failed to cancel latency stats update thread:"
+ "%s,%s:%d\n",
+ strerror(errno), __func__, __LINE__);
+ return -1;
+ }
+
+ return 0;
+}
+
+int
+rte_latencystats_get_names(struct rte_metric_name *names, uint16_t size)
+{
+ unsigned int i;
+
+ if (names == NULL || size < NUM_LATENCY_STATS)
+ return NUM_LATENCY_STATS;
+
+ for (i = 0; i < NUM_LATENCY_STATS; i++)
+ snprintf(names[i].name, sizeof(names[i].name),
+ "%s", lat_stats_strings[i].name);
+
+ return NUM_LATENCY_STATS;
+}
+
+int
+rte_latencystats_get(struct rte_stat_value *values, uint16_t size)
+{
+ if (size < NUM_LATENCY_STATS || values == NULL)
+ return NUM_LATENCY_STATS;
+
+ if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+ const struct rte_memzone *mz;
+ mz = rte_memzone_lookup(MZ_RTE_LATENCY_STATS);
+ if (mz == NULL) {
+ RTE_LOG(ERR, LATENCY_STATS,
+ "Latency stats memzone not found\n");
+ return -ENOMEM;
+ }
+ glob_stats = mz->addr;
+ }
+
+ /* Retrieve latency stats */
+ rte_latencystats_fill_values(values);
+
+ return NUM_LATENCY_STATS;
+}
diff --git a/lib/librte_latencystats/rte_latencystats.h b/lib/librte_latencystats/rte_latencystats.h
new file mode 100644
index 0000000..7b1e72a
--- /dev/null
+++ b/lib/librte_latencystats/rte_latencystats.h
@@ -0,0 +1,141 @@
+/*-
+ * BSD LICENSE
+ *
+ * Copyright(c) 2016 Intel Corporation. All rights reserved.
+ * 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.
+ */
+
+#ifndef _RTE_LATENCYSTATS_H_
+#define _RTE_LATENCYSTATS_H_
+
+/**
+ * @file
+ * RTE latency stats
+ *
+ * library to provide application and flow based latency stats.
+ */
+
+#include <rte_metrics.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Function type used for identifting flow types of a Rx packet.
+ *
+ * The callback function is called on Rx for each packet.
+ * This function is used for flow based latency calculations.
+ *
+ * @param pkt
+ * Packet that has to be identified with its flow types.
+ * @param user_param
+ * The arbitrary user parameter passed in by the application when
+ * the callback was originally configured.
+ * @return
+ * The flow_mask, representing the multiple flow types of a packet.
+ */
+typedef uint16_t (*rte_latency_stats_flow_type_fn)(struct rte_mbuf *pkt,
+ void *user_param);
+
+/**
+ * @internal
+ * Registers Rx/Tx callbacks for each active port, queue.
+ *
+ * @param sampIntvl
+ * Sampling time period in nano seconds, at which packet
+ * should be marked with time stamp.
+ * @param user_cb
+ * User callback to be called to get flow types of a packet.
+ * Used for flow based latency calculation.
+ * If the value is NULL, global stats will be calculated,
+ * else flow based stats will be calculated.
+ * @return
+ * -1 : On error
+ * -ENOMEM: On error
+ * 0 : On success
+ */
+int rte_latencystats_init(uint64_t samp_intvl,
+ rte_latency_stats_flow_type_fn user_cb);
+
+/**
+ * @internal
+ * Removes registered Rx/Tx callbacks for each active port, queue.
+ * @return
+ * -1: On error
+ * 0: On suces
+ */
+int rte_latencystats_uninit(void);
+
+/**
+ * Retrieve names of latency statistics
+ *
+ * @param names
+ * Block of memory to insert names into. Must be at least size in capacity.
+ * If set to NULL, function returns required capacity.
+ * @param size
+ * Capacity of latency stats names (number of names).
+ * @return
+ * - positive value lower or equal to size: success. The return value
+ * is the number of entries filled in the stats table.
+ * - positive value higher than size: error, the given statistics table
+ * is too small. The return value corresponds to the size that should
+ * be given to succeed. The entries in the table are not valid and
+ * shall not be used by the caller.
+ */
+int rte_latencystats_get_names(struct rte_metric_name *names,
+ uint16_t size);
+
+/**
+ * Retrieve latency statistics.
+ *
+ * @param values
+ * A pointer to a table of structure of type *rte_stat_value*
+ * to be filled with latency statistics ids and values.
+ * This parameter can be set to NULL if size is 0.
+ * @param size
+ * The size of the stats table, which should be large enough to store
+ * all the latency stats.
+ * @return
+ * - positive value lower or equal to size: success. The return value
+ * is the number of entries filled in the stats table.
+ * - positive value higher than size: error, the given statistics table
+ * is too small. The return value corresponds to the size that should
+ * be given to succeed. The entries in the table are not valid and
+ * shall not be used by the caller.
+ * -ENOMEM: On failure.
+ */
+int rte_latencystats_get(struct rte_stat_value *values,
+ uint16_t size);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_LATENCYSTATS_H_ */
diff --git a/lib/librte_latencystats/rte_latencystats_version.map b/lib/librte_latencystats/rte_latencystats_version.map
new file mode 100644
index 0000000..82dc5a7
--- /dev/null
+++ b/lib/librte_latencystats/rte_latencystats_version.map
@@ -0,0 +1,10 @@
+DPDK_16.11 {
+ global:
+
+ rte_latencystats_get;
+ rte_latencystats_get_names;
+ rte_latencystats_init;
+ rte_latencystats_uninit;
+
+ local: *;
+};
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 109e666..cc3bf65 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -493,6 +493,9 @@ struct rte_mbuf {
/** Timesync flags for use with IEEE1588. */
uint16_t timesync;
+
+ /** Timestamp for measuring latency. */
+ uint64_t timestamp;
} __rte_cache_aligned;
/**
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index f75f0e2..4e5289a 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -98,6 +98,8 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_RING) += -lrte_ring
_LDLIBS-$(CONFIG_RTE_LIBRTE_EAL) += -lrte_eal
_LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE) += -lrte_cmdline
_LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE) += -lrte_cfgfile
+_LDLIBS-$(CONFIG_RTE_LIBRTE_LATENCY_STATS) += -lrte_latencystats
+
_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += -lrte_pmd_bond
_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lrte_pmd_xenvirt -lxenstore
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v3] net/ring: remove unnecessary NULL check
From: Thomas Monjalon @ 2016-11-07 13:08 UTC (permalink / raw)
To: Mauricio Vasquez B; +Cc: dev, Ferruh Yigit, bruce.richardson
In-Reply-To: <162c0602-5fff-fa80-98c5-3725fcb96e69@intel.com>
2016-11-02 14:05, Ferruh Yigit:
> On 11/2/2016 1:46 PM, Mauricio Vasquez B wrote:
> > Coverity detected this as an issue because internals->data will never be NULL,
> > then the check is not necessary.
> >
> > Fixes: d082c0395bf6 ("ring: fix memory leak when detaching")
> > Coverity issue: 137873
> >
> > Signed-off-by: Mauricio Vasquez B <mauricio.vasquez@polito.it>
>
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
Applied, thanks
^ permalink raw reply
* Re: [PATCH v3 2/2] mempool: pktmbuf pool default fallback for mempool ops error
From: Hemant Agrawal @ 2016-11-07 12:30 UTC (permalink / raw)
To: Olivier Matz
Cc: dev@dpdk.org, jerin.jacob@caviumnetworks.com,
david.hunt@intel.com
In-Reply-To: <412b3d71-f6df-7ee5-6189-ed7af1268fed@6wind.com>
Hi Olivier,
> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Friday, October 14, 2016 5:41 PM
> > On 9/22/2016 6:42 PM, Hemant Agrawal wrote:
> >> Hi Olivier
> >>
> >> On 9/19/2016 7:27 PM, Olivier Matz wrote:
> >>> Hi Hemant,
> >>>
> >>> On 09/16/2016 06:46 PM, Hemant Agrawal wrote:
> >>>> In the rte_pktmbuf_pool_create, if the default external mempool is
> >>>> not available, the implementation can default to "ring_mp_mc",
> >>>> which is an software implementation.
> >>>>
> >>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> >>>> ---
> >>>> Changes in V3:
> >>>> * adding warning message to say that falling back to default sw
> >>>> pool
> >>>> ---
> >>>> lib/librte_mbuf/rte_mbuf.c | 8 ++++++++
> >>>> 1 file changed, 8 insertions(+)
> >>>>
> >>>> diff --git a/lib/librte_mbuf/rte_mbuf.c
> >>>> b/lib/librte_mbuf/rte_mbuf.c index 4846b89..8ab0eb1 100644
> >>>> --- a/lib/librte_mbuf/rte_mbuf.c
> >>>> +++ b/lib/librte_mbuf/rte_mbuf.c
> >>>> @@ -176,6 +176,14 @@ rte_pktmbuf_pool_create(const char *name,
> >>>> unsigned n,
> >>>>
> >>>> rte_errno = rte_mempool_set_ops_byname(mp,
> >>>> RTE_MBUF_DEFAULT_MEMPOOL_OPS, NULL);
> >>>> +
> >>>> + /* on error, try falling back to the software based default
> >>>> pool */
> >>>> + if (rte_errno == -EOPNOTSUPP) {
> >>>> + RTE_LOG(WARNING, MBUF, "Default HW Mempool not supported. "
> >>>> + "falling back to sw mempool \"ring_mp_mc\"");
> >>>> + rte_errno = rte_mempool_set_ops_byname(mp, "ring_mp_mc",
> >>>> NULL);
> >>>> + }
> >>>> +
> >>>> if (rte_errno != 0) {
> >>>> RTE_LOG(ERR, MBUF, "error setting mempool handler\n");
> >>>> return NULL;
> >>>>
> >>>
> >>> Without adding a new method ".supported()", the first call to
> >>> rte_mempool_populate() could return the same error ENOTSUP. In this
> >>> case, it is still possible to fallback.
> >>>
> >> It will be bit late.
> >>
> >> On failure, than we have to set the default ops and do a goto before
> >> rte_pktmbuf_pool_init(mp, &mbp_priv);
>
> I still think we can do the job without adding the .supported() method.
> The following code is just an (untested) example:
>
> struct rte_mempool *
> rte_pktmbuf_pool_create(const char *name, unsigned n,
> unsigned cache_size, uint16_t priv_size, uint16_t data_room_size,
> int socket_id)
> {
> struct rte_mempool *mp;
> struct rte_pktmbuf_pool_private mbp_priv;
> unsigned elt_size;
> int ret;
> const char *ops[] = {
> RTE_MBUF_DEFAULT_MEMPOOL_OPS, "ring_mp_mc", NULL,
> };
> const char **op;
>
> if (RTE_ALIGN(priv_size, RTE_MBUF_PRIV_ALIGN) != priv_size) {
> RTE_LOG(ERR, MBUF, "mbuf priv_size=%u is not aligned\n",
> priv_size);
> rte_errno = EINVAL;
> return NULL;
> }
> elt_size = sizeof(struct rte_mbuf) + (unsigned)priv_size +
> (unsigned)data_room_size;
> mbp_priv.mbuf_data_room_size = data_room_size;
> mbp_priv.mbuf_priv_size = priv_size;
>
> for (op = &ops[0]; *op != NULL; op++) {
> mp = rte_mempool_create_empty(name, n, elt_size, cache_size,
> sizeof(struct rte_pktmbuf_pool_private), socket_id, 0);
> if (mp == NULL)
> return NULL;
>
> ret = rte_mempool_set_ops_byname(mp, *op, NULL);
> if (ret != 0) {
> RTE_LOG(ERR, MBUF, "error setting mempool handler\n");
> rte_mempool_free(mp);
> if (ret == -ENOTSUP)
> continue;
> rte_errno = -ret;
> return NULL;
> }
> rte_pktmbuf_pool_init(mp, &mbp_priv);
>
> ret = rte_mempool_populate_default(mp);
> if (ret < 0) {
> rte_mempool_free(mp);
> if (ret == -ENOTSUP)
> continue;
> rte_errno = -ret;
> return NULL;
> }
> }
>
> rte_mempool_obj_iter(mp, rte_pktmbuf_init, NULL);
>
> return mp;
> }
>
>
[Hemant] This look fine to me. Please submit a patch for the same.
> >>> I've just submitted an RFC, which I think is quite linked:
> >>> http://dpdk.org/ml/archives/dev/2016-September/046974.html
> >>> Assuming a new parameter "mempool_ops" is added to
> >>> rte_pktmbuf_pool_create(), would it make sense to fallback to
> >>> "ring_mp_mc"? What about just returning ENOTSUP? The application
> >>> could do the job and decide which sw fallback to use.
> >>
> >> We ran into this issue when trying to run the standard DPDK examples
> >> (l3fwd) in VM. Do you think, is it practical to add fallback handling
> >> in each of the DPDK examples?
>
> OK. What is still unclear for me, is how the software is aware of the different
> hardware-assisted handlers. Moreover, we could imagine more software
> handlers, which could be used depending on the use case.
>
> I think this choice has to be made by the user or the application:
>
> - the application may want to use a specific (sw or hw) handler: in
> this case, it want to be notified if it fails, instead of having
> a quiet fallback to ring_mp_mc
> - if several handlers are available, the application may want to
> try them in a specific order
> - maybe some handlers will have some limitations with some
> configurations or driver? The application could decide to use
> a different handler according the configuration.
>
> So that's why I think this is an application decision.
>
[Hemant] We should simplify it: if the application has supplied the handler, it is application's responsibility to take care of failure. Only if the application want to use the default handler, the implementation can fallback. The fallback handler can again be configurable.
> >> Typically when someone is writing a application on host, he need not
> >> worry non-availability of the hw offloaded mempool. He may also want
> >> to run the same binary in virtual machine. In VM, it is not
> >> guaranteed that hw offloaded mempools will be available.
>
> Running the same binary is of course a need. But if your VM does not provide
> the same virtualized hardware than the host, I think the command line option
> makes sense.
>
> AFAIU, on the host, you can use a hw mempool handler or a sw one, and on the
> guest, only a sw one. So you need an option to select the behavior you want on
> the host, without recompiling.
>
> I understand that modifying all the applications is not a good option either. I'm
> thinking a a EAL parameter that would allow to configure a library (mbuf in this
> case). Something like kernel boot options.
> Example: testpmd -l 2,4 --opt mbuf.handler="ring_mp_mc" -- [app args]
>
> I don't know if this is feasible, this has to be discussed first, but what do you
> think on the principle? The value could also be an ordered list if we want a
> fallback, and this option could be overriden by the application before creating
> the mbuf pool. There was some discussions about a kind of dpdk global
> configuration some time ago.
>
[Hemant] I agree that command line option provide a better control in this case.
On the flipside, We need to be careful that we do not end up having too many command line options.
>
> Regards,
> Olivier
^ permalink raw reply
* Re: [PATCH v2 10/10] net/virtio: fix multiple queue enabling
From: Yuanhan Liu @ 2016-11-07 9:25 UTC (permalink / raw)
To: dev
Cc: Thomas Monjalon, Tan Jianfeng, Kevin Traynor, Ilya Maximets,
Kyle Larose, Maxime Coquelin
In-Reply-To: <1478338865-26126-11-git-send-email-yuanhan.liu@linux.intel.com>
On Sat, Nov 05, 2016 at 05:41:05PM +0800, Yuanhan Liu wrote:
> When queue number shrinks to 1 from X, the following code stops us
> sending the multiple queue ctrl message:
>
> if (nb_queues > 1) {
> if (virtio_set_multiple_queues(dev, nb_queues) != 0)
> return -EINVAL;
> }
>
> This ends up with still X queues being enabled, which is obviously
> wrong. Fix it by removing the check.
>
> Fixes: 823ad647950a ("virtio: support multiple queues")
>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
This breaks the virtio-user case, where ctrl-queue is not enabled by
defeault.
Here is an update patch would fix this.
--yliu
---
>From 22502943764a99b1398d40f0110f8ce28323323a Mon Sep 17 00:00:00 2001
From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Date: Sat, 5 Nov 2016 16:53:27 +0800
Subject: [PATCH v3 10/10] net/virtio: fix multiple queue enabling
When queue number shrinks to 1 from X, the following code stops us
sending the multiple queue ctrl message:
if (nb_queues > 1) {
if (virtio_set_multiple_queues(dev, nb_queues) != 0)
return -EINVAL;
}
This ends up with still X queues being enabled, which is obviously
wrong. Fix it by replacing the check with a multiple queue enabled
or not check.
Fixes: 823ad647950a ("virtio: support multiple queues")
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
v3: - fix the virtio-user case, which is default with ctrl-queue
being disabled. Thus virtio_set_multiple_queues fails.
---
drivers/net/virtio/virtio_ethdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d70bd00..18da98f 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1472,6 +1472,7 @@ virtio_dev_start(struct rte_eth_dev *dev)
uint16_t nb_queues, i;
struct virtnet_rx *rxvq;
struct virtnet_tx *txvq __rte_unused;
+ struct virtio_hw *hw = dev->data->dev_private;
/* check if lsc interrupt feature is enabled */
if (dev->data->dev_conf.intr_conf.lsc) {
@@ -1494,7 +1495,7 @@ virtio_dev_start(struct rte_eth_dev *dev)
*vhost backend will have no chance to be waked up
*/
nb_queues = RTE_MAX(dev->data->nb_rx_queues, dev->data->nb_tx_queues);
- if (nb_queues > 1) {
+ if (hw->max_queue_pairs > 1) {
if (virtio_set_multiple_queues(dev, nb_queues) != 0)
return -EINVAL;
}
--
1.9.0
^ permalink raw reply related
* [PATCH] pci: do not mark device as taken when probe fails
From: David Marchand @ 2016-11-07 9:03 UTC (permalink / raw)
To: thomas.monjalon; +Cc: dev, benjamin.walker
In-Reply-To: <CALwxeUtOT1TCaevHwx8kSdEqHd4b5VO1ER2n0Xym94mWvObm_Q@mail.gmail.com>
dev->driver should be set only if a driver did take the device.
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
lib/librte_eal/common/eal_common_pci.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 971ad20..6163b09 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -214,7 +214,11 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
dev->driver = dr;
/* call the driver probe() function */
- return dr->probe(dr, dev);
+ ret = dr->probe(dr, dev);
+ if (ret)
+ dev->driver = NULL;
+
+ return ret;
}
/* return positive value if driver doesn't support this device */
return 1;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] pci: Don't call probe callback if driver already loaded.
From: David Marchand @ 2016-11-07 9:02 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: Ben Walker, dev@dpdk.org
In-Reply-To: <20538590.K61FAlGikS@xps13>
Hello Thomas,
On Sun, Nov 6, 2016 at 10:59 PM, Thomas Monjalon
<thomas.monjalon@6wind.com> wrote:
> 2016-10-25 14:50, Ben Walker:
>> If the user asks to probe multiple times, the probe
>> callback should only be called on devices that don't have
>> a driver already loaded.
>>
>> This is useful if a driver is registered after the
>> execution of a program has started and the list of devices
>> needs to be re-scanned.
>>
>> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
>
> Applied, thanks
Can you add a follow up patch I will send shortly ?
Thanks.
--
David Marchand
^ permalink raw reply
* [PATCH] maintainers: claim responsability for xen
From: Jianfeng Tan @ 2016-11-07 7:38 UTC (permalink / raw)
To: dev; +Cc: thomas.monjalon, bruce.richardson, yuanhan.liu, Jianfeng Tan
As some users are still using xen as the hypervisor, I suggest to
continue support for xen in DPDK. And from 16.11, I will be the
maintainer of all xen-related files.
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index ba12d1b..d2fa2db 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -190,6 +190,7 @@ M: Anatoly Burakov <anatoly.burakov@intel.com>
F: lib/librte_eal/linuxapp/eal/*vfio*
Linux Xen
+M: Jianfeng Tan <jianfeng.tan@intel.com>
F: lib/librte_eal/linuxapp/xen_dom0/
F: lib/librte_eal/linuxapp/eal/*xen*
F: lib/librte_eal/linuxapp/eal/include/exec-env/rte_dom0_common.h
--
2.7.4
^ permalink raw reply related
* [PATCH] doc: fix typos
From: Alain Leon @ 2016-11-07 3:20 UTC (permalink / raw)
To: dev; +Cc: Alain Leon
Fixes typos present in the documentation and code comments.
Signed-off-by: Alain Leon <xerebz@gmail.com>
---
doc/guides/prog_guide/overview.rst | 2 +-
lib/librte_kni/rte_kni.h | 2 +-
lib/librte_mbuf/rte_mbuf.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/doc/guides/prog_guide/overview.rst b/doc/guides/prog_guide/overview.rst
index 68cc75c..9986e3c 100644
--- a/doc/guides/prog_guide/overview.rst
+++ b/doc/guides/prog_guide/overview.rst
@@ -157,7 +157,7 @@ The mbuf library provides the facility to create and destroy buffers
that may be used by the DPDK application to store message buffers.
The message buffers are created at startup time and stored in a mempool, using the DPDK mempool library.
-This library provide an API to allocate/free mbufs, manipulate control message buffers (ctrlmbuf) which are generic message buffers,
+This library provides an API to allocate/free mbufs, manipulate control message buffers (ctrlmbuf) which are generic message buffers,
and packet buffers (pktmbuf) which are used to carry network packets.
Network Packet Buffer Management is described in :ref:`Mbuf Library <Mbuf_Library>`.
diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h
index 5f6f9e4..37deb47 100644
--- a/lib/librte_kni/rte_kni.h
+++ b/lib/librte_kni/rte_kni.h
@@ -42,7 +42,7 @@
* interfaces that may be used by the RTE application to receive/transmit
* packets from/to Linux kernel net interfaces.
*
- * This library provide two APIs to burst receive packets from KNI interfaces,
+ * This library provides two APIs to burst receive packets from KNI interfaces,
* and burst transmit packets to KNI interfaces.
*/
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 109e666..ead7c6e 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -44,7 +44,7 @@
* buffers. The message buffers are stored in a mempool, using the
* RTE mempool library.
*
- * This library provide an API to allocate/free packet mbufs, which are
+ * This library provides an API to allocate/free packet mbufs, which are
* used to carry network packets.
*
* To understand the concepts of packet buffers or mbufs, you
--
2.7.4
^ permalink raw reply related
* [PATCH] lib/ip_frag: fix IP reassembly not working issue
From: Wenzhuo Lu @ 2016-11-06 17:16 UTC (permalink / raw)
To: dev; +Cc: adrien.mazarguil, konstantin.ananyev, Wenzhuo Lu
After changing pkt[0] to pkt[], the example IP reassembly is not
working.
It's weird because this change is fine. There should be no
difference between them.
As a workaround, revert this change.
Fixes: 347a1e037fd3 (lib: use C99 syntax for zero-size arrays)
Reported-by: Huilong Xu <huilongx.xu@intel.com>
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
lib/librte_ip_frag/rte_ip_frag.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/librte_ip_frag/rte_ip_frag.h b/lib/librte_ip_frag/rte_ip_frag.h
index 69596ab..6708906 100644
--- a/lib/librte_ip_frag/rte_ip_frag.h
+++ b/lib/librte_ip_frag/rte_ip_frag.h
@@ -124,7 +124,7 @@ struct rte_ip_frag_tbl {
struct ip_frag_pkt *last; /**< last used entry. */
struct ip_pkt_list lru; /**< LRU list for table entries. */
struct ip_frag_tbl_stat stat; /**< statistics counters. */
- struct ip_frag_pkt pkt[]; /**< hash table. */
+ __extension__ struct ip_frag_pkt pkt[0]; /**< hash table. */
};
/** IPv6 fragment extension header */
--
1.9.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox