* Re: [PATCH] doc: add limitation for L3fwd-power app
From: De Lara Guarch, Pablo @ 2016-11-04 18:27 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev@dpdk.org
In-Reply-To: <3103408.LezOKEn1tR@xps13>
Hi Thomas,
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, November 04, 2016 5:00 PM
> To: De Lara Guarch, Pablo
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] doc: add limitation for L3fwd-power app
>
> 2016-11-04 12:48, Pablo de Lara:
> > L3fwd-power requires the mbuf to have correct packet type,
> > but in some drivers, the vector mode must be disabled for this.
>
> This is a severe bug.
> We must really fix packet type handling to have a constant support
> whatever the driver or options.
This is already documented in previous release (2.2).
Actually, the main problem for L3fwd-power app is not the packet type,
but the descriptor_done for the Rx vector function, that works differently
compared to the scalar function.
Thanks,
Pablo
^ permalink raw reply
* Re: [PATCH 1/2 v2] bnxt: use appropriate data type in bnxt_alloc_vnic_attributes
From: Ajit Khaparde @ 2016-11-04 17:35 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: dev, John W. Linville
In-Reply-To: <a5c6b4f5-f50e-fccc-d33d-ec50f182e36c@intel.com>
On Fri, Nov 4, 2016 at 5:52 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> On 11/3/2016 6:58 PM, Ajit Khaparde wrote:
> > 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>
> >
> > --
> > v2: Previous attempt did not seem complete.
> > ---
> > drivers/net/bnxt/bnxt_vnic.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/bnxt/bnxt_vnic.c b/drivers/net/bnxt/bnxt_vnic.c
> > index 205a940..23c85af 100644
> > --- a/drivers/net/bnxt/bnxt_vnic.c
> > +++ b/drivers/net/bnxt/bnxt_vnic.c
> > @@ -179,7 +179,7 @@ int bnxt_alloc_vnic_attributes(struct bnxt *bp)
> > HW_HASH_INDEX_SIZE *
> sizeof(*vnic->rss_table) +
> > HW_HASH_KEY_SIZE);
> > uint16_t max_vnics;
> > - int i;
> > + uint16_t i;
> >
> > if (BNXT_PF(bp)) {
> > struct bnxt_pf_info *pf = &bp->pf;
> > @@ -197,7 +197,7 @@ int bnxt_alloc_vnic_attributes(struct bnxt *bp)
> > mz = rte_memzone_lookup(mz_name);
> > if (!mz) {
> > mz = rte_memzone_reserve(mz_name,
> > - entry_length * max_vnics,
> > + (uint32_t) entry_length *
> max_vnics,
> > SOCKET_ID_ANY,
> > RTE_MEMZONE_2MB |
> > RTE_MEMZONE_SIZE_HINT_ONLY);
> > @@ -210,10 +210,11 @@ int bnxt_alloc_vnic_attributes(struct bnxt *bp)
> >
> > /* Allocate rss table and hash key */
> > vnic->rss_table =
> > - (void *)((char *)mz->addr + (entry_length * i));
> > + (void *)((char *)mz->addr + ((uint32_t) entry_length * i));
> > memset(vnic->rss_table, -1, entry_length);
> >
> > - vnic->rss_table_dma_addr = mz->phys_addr + (entry_length *
> i);
> > + vnic->rss_table_dma_addr =
> > + mz->phys_addr + ((uint32_t) entry_length * i);
>
> ((uint32_t) entry_length * i)
> casting entry_length to uint32_t will prevent the integer promotion and
> fix the coverity issue, but if this is the case, why not make
> entry_length uint32_t at first place?
>
> It seems "entry_length" converted from "int" to "uint16_t" to prevent
> integer promotion (e8a197d2aa9a), but
> "entry_length * max_vnics" => "uint16_t * uint16_t" or
> "entry_length * i" => "uint16_t * int"
> still causing the promotion.
>
> I guess converting "entry_length" to uint32_t (instead of uint16_t) will
> fix both triaged 127557 and new introduced 137874 coverity issues, and
> it is simpler modification, - if I don't miss anything J
>
Sounds reasonable to me. I will send a patch in a little while.
Thanks
>
> Thanks,
> ferruh
>
>
> > vnic->rss_hash_key = (void *)((char *)vnic->rss_table +
> > HW_HASH_INDEX_SIZE *
> sizeof(*vnic->rss_table));
> >
> >
>
>
^ permalink raw reply
* Re: [PATCH] doc: add limitation for L3fwd-power app
From: Thomas Monjalon @ 2016-11-04 17:00 UTC (permalink / raw)
To: Pablo de Lara; +Cc: dev
In-Reply-To: <1478263697-57575-1-git-send-email-pablo.de.lara.guarch@intel.com>
2016-11-04 12:48, Pablo de Lara:
> L3fwd-power requires the mbuf to have correct packet type,
> but in some drivers, the vector mode must be disabled for this.
This is a severe bug.
We must really fix packet type handling to have a constant support
whatever the driver or options.
^ permalink raw reply
* Re: [dpdk-users] rte_zmalloc() returning non-zeroed memory on FreeBSD
From: Lewis Donzis @ 2016-11-04 16:43 UTC (permalink / raw)
To: Sergio Gonzalez Monroy; +Cc: dev
In-Reply-To: <112cd7ac-2dde-3e1b-0fc1-f21f3c253982@intel.com>
> On Nov 4, 2016, at 11:38 AM, Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> wrote:
>
> On 03/11/2016 20:04, Lewis Donzis wrote:
>> I’m curious about how/whether this got resolved. The 16.07.1 code doesn’t appear to have this fixed.
>>
>> Is it still forthcoming?
>>
>> Thanks,
>> lew
>
> It should have been fixed in 16.07.1.
> http://dpdk.org/browse/dpdk-stable/commit/?id=82f931805506efbb8b5046e9045bec8f04bbabf6
>
> Do you have an easy test to reproduce? can you reproduce it with any of the app/examples?
Oh, sorry, I was looking at the rte_zmalloc() code, assuming it would be changed there, but it looks like the commit was to change contigmem instead, which seems perfectly logical and reasonable.
It’s entirely possible that the folks here who tested 16.07.1 forgot to replace the contigmem driver, so perhaps that is the problem.
We’ll check it out and report back.
Thanks!
lew
^ permalink raw reply
* Re: [PATCH v3 1/3] lib: add information metrics library
From: Pattan, Reshma @ 2016-11-04 16:42 UTC (permalink / raw)
To: Horton, Remy; +Cc: dev@dpdk.org
In-Reply-To: <1478230579-4689-2-git-send-email-remy.horton@intel.com>
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>
Thanks,
Reshma
^ permalink raw reply
* Re: [dpdk-users] rte_zmalloc() returning non-zeroed memory on FreeBSD
From: Sergio Gonzalez Monroy @ 2016-11-04 16:38 UTC (permalink / raw)
To: Lewis Donzis, dev
In-Reply-To: <44ECBB6D-0B65-46D0-AAE5-E96E3F8CB59A@perftech.com>
On 03/11/2016 20:04, Lewis Donzis wrote:
> I’m curious about how/whether this got resolved. The 16.07.1 code doesn’t appear to have this fixed.
>
> Is it still forthcoming?
>
> Thanks,
> lew
It should have been fixed in 16.07.1.
http://dpdk.org/browse/dpdk-stable/commit/?id=82f931805506efbb8b5046e9045bec8f04bbabf6
Do you have an easy test to reproduce? can you reproduce it with any of
the app/examples?
Sergio
^ permalink raw reply
* FOSDEM Call for Content
From: Thomas Monjalon @ 2016-11-04 15:51 UTC (permalink / raw)
To: dev
There will be a SDN/NFV room on Saturday, February 4, 2017 at Brussels (Belgium).
Areas of interest can be low-level networking or higher level management.
You can submit an abstract before November 16:
https://blogs.gnome.org/bolsh/2016/10/19/fosdem-sdn-nfv-devroom-call-for-content/
^ permalink raw reply
* Re: [PATCH 2/2] bnxt: add a FALLTHROUGH comment in the cascading switch statement
From: Ferruh Yigit @ 2016-11-04 15:41 UTC (permalink / raw)
To: Ajit Khaparde, dev
In-Reply-To: <20161103185842.GA88409@C02PT1RBG8WP.wifi.aus.avagotech.net>
On 11/3/2016 6:58 PM, Ajit Khaparde wrote:
> 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>
^ permalink raw reply
* Re: [PATCH 4/8] net/virtio: allocate queue at init stage
From: Kevin Traynor @ 2016-11-04 15:21 UTC (permalink / raw)
To: Yuanhan Liu, dev
Cc: Thomas Monjalon, Tan Jianfeng, Ilya Maximets, Kyle Larose
In-Reply-To: <1478189400-14606-5-git-send-email-yuanhan.liu@linux.intel.com>
On 11/03/2016 04:09 PM, Yuanhan Liu wrote:
> Queue allocation should be done once, since the queue related info (such
> as vring addreess) will only be informed to the vhost-user backend once
> without virtio device reset.
>
> That means, if you allocate queues again after the vhost-user negotiation,
> the vhost-user backend will not be informed any more. Leading to a state
> that the vring info mismatches between virtio PMD driver and vhost-backend:
> the driver switches to the new address has just been allocated, while the
> vhost-backend still sticks to the old address has been assigned in the init
> stage.
>
> Unfortunately, that is exactly how the virtio driver is coded so far: queue
> allocation is done at queue_setup stage (when rte_eth_tx/rx_queue_setup is
> invoked). This is wrong, because queue_setup can be invoked several times.
> For example,
>
> $ start_testpmd.sh ... --txq=1 --rxq=1 ...
> > port stop 0
> > port config all txq 1 # just trigger the queue_setup callback again
> > port config all rxq 1
> > port start 0
>
> The right way to do is allocate the queues in the init stage, so that the
> vring info could be persistent with the vhost-user backend.
>
> Besides that, we should allocate max_queue pairs the device supports, but
> not nr queue pairs firstly configured, to make following case work.
>
> $ start_testpmd.sh ... --txq=1 --rxq=1 ...
> > port stop 0
> > port config all txq 2
> > port config all rxq 2
> > port start 0
hi Yuanhan, firstly - thanks for this patchset. It is certainly needed
to fix the silent failure after increase num q's.
I tried a few tests and I'm seeing an issue. I can stop the port,
increase the number of queues and traffic is ok, but if I try to
decrease the number of queues it hangs on port start. I'm running head
of the master with your patches in the guest and 16.07 in the host.
$ testpmd -c 0x5f -n 4 --socket-mem 1024 -- --burst=64 -i
--disable-hw-vlan --rxq=2 --txq=2 --rxd=256 --txd=256 --forward-mode=io
> port stop all
> port config all rxq 1
> port config all txq 1
> port start all
Configuring Port 0 (socket 0)
(hang here)
I've tested a few different scenarios and anytime the queues are
decreased from the previous number the hang occurs.
I can debug further but wanted to report early as maybe issue is an
obvious one?
thanks,
Kevin.
>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> ---
> drivers/net/virtio/virtio_ethdev.c | 105 +++++++++++++++++++++++--------------
> drivers/net/virtio/virtio_ethdev.h | 8 ---
> drivers/net/virtio/virtio_pci.h | 2 +
> drivers/net/virtio/virtio_rxtx.c | 38 +++++++-------
> 4 files changed, 85 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 5a2c14b..253bcb5 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -295,13 +295,19 @@ virtio_dev_queue_release(struct virtqueue *vq)
> }
> }
>
> -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_get_queue_type(struct virtio_hw *hw, uint16_t vtpci_queue_idx)
> +{
> + if (vtpci_queue_idx == hw->max_queue_pairs * 2)
> + return VTNET_CQ;
> + else if (vtpci_queue_idx % 2 == 0)
> + return VTNET_RQ;
> + else
> + return VTNET_TQ;
> +}
> +
> +static int
> +virtio_init_queue(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx)
> {
> char vq_name[VIRTQUEUE_MAX_NAME_SZ];
> char vq_hdr_name[VIRTQUEUE_MAX_NAME_SZ];
> @@ -314,6 +320,7 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
> struct virtqueue *vq;
> size_t sz_hdr_mz = 0;
> void *sw_ring = NULL;
> + int queue_type = virtio_get_queue_type(hw, vtpci_queue_idx);
> int ret;
>
> PMD_INIT_LOG(DEBUG, "setting up queue: %u", vtpci_queue_idx);
> @@ -351,18 +358,18 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
> sz_hdr_mz = PAGE_SIZE;
> }
>
> - vq = rte_zmalloc_socket(vq_name, size, RTE_CACHE_LINE_SIZE, socket_id);
> + vq = rte_zmalloc_socket(vq_name, size, RTE_CACHE_LINE_SIZE,
> + SOCKET_ID_ANY);
> if (vq == NULL) {
> PMD_INIT_LOG(ERR, "can not allocate vq");
> return -ENOMEM;
> }
> + hw->vqs[vtpci_queue_idx] = vq;
> +
> vq->hw = hw;
> vq->vq_queue_index = vtpci_queue_idx;
> vq->vq_nentries = vq_size;
> -
> - if (nb_desc == 0 || nb_desc > vq_size)
> - nb_desc = vq_size;
> - vq->vq_free_cnt = nb_desc;
> + vq->vq_free_cnt = vq_size;
>
> /*
> * Reserve a memzone for vring elements
> @@ -372,7 +379,8 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
> PMD_INIT_LOG(DEBUG, "vring_size: %d, rounded_vring_size: %d",
> size, vq->vq_ring_size);
>
> - mz = rte_memzone_reserve_aligned(vq_name, vq->vq_ring_size, socket_id,
> + mz = rte_memzone_reserve_aligned(vq_name, vq->vq_ring_size,
> + SOCKET_ID_ANY,
> 0, VIRTIO_PCI_VRING_ALIGN);
> if (mz == NULL) {
> if (rte_errno == EEXIST)
> @@ -396,7 +404,7 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
> snprintf(vq_hdr_name, sizeof(vq_hdr_name), "port%d_vq%d_hdr",
> dev->data->port_id, vtpci_queue_idx);
> hdr_mz = rte_memzone_reserve_aligned(vq_hdr_name, sz_hdr_mz,
> - socket_id, 0,
> + SOCKET_ID_ANY, 0,
> RTE_CACHE_LINE_SIZE);
> if (hdr_mz == NULL) {
> if (rte_errno == EEXIST)
> @@ -413,7 +421,7 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
> sizeof(vq->sw_ring[0]);
>
> sw_ring = rte_zmalloc_socket("sw_ring", sz_sw,
> - RTE_CACHE_LINE_SIZE, socket_id);
> + RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY);
> if (!sw_ring) {
> PMD_INIT_LOG(ERR, "can not allocate RX soft ring");
> ret = -ENOMEM;
> @@ -424,19 +432,14 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
> rxvq = &vq->rxq;
> rxvq->vq = vq;
> rxvq->port_id = dev->data->port_id;
> - rxvq->queue_id = queue_idx;
> rxvq->mz = mz;
> - *pvq = rxvq;
> } else if (queue_type == VTNET_TQ) {
> txvq = &vq->txq;
> txvq->vq = vq;
> txvq->port_id = dev->data->port_id;
> - txvq->queue_id = queue_idx;
> txvq->mz = mz;
> txvq->virtio_net_hdr_mz = hdr_mz;
> txvq->virtio_net_hdr_mem = hdr_mz->phys_addr;
> -
> - *pvq = txvq;
> } else if (queue_type == VTNET_CQ) {
> cvq = &vq->cq;
> cvq->vq = vq;
> @@ -444,7 +447,8 @@ int virtio_dev_queue_setup(struct rte_eth_dev *dev,
> cvq->virtio_net_hdr_mz = hdr_mz;
> cvq->virtio_net_hdr_mem = hdr_mz->phys_addr;
> memset(cvq->virtio_net_hdr_mz->addr, 0, PAGE_SIZE);
> - *pvq = cvq;
> +
> + hw->cvq = cvq;
> }
>
> /* For virtio_user case (that is when dev->pci_dev is NULL), we use
> @@ -502,23 +506,45 @@ fail_q_alloc:
> }
>
> static int
> -virtio_dev_cq_queue_setup(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx,
> - uint32_t socket_id)
> +virtio_alloc_queues(struct rte_eth_dev *dev)
> {
> - struct virtnet_ctl *cvq;
> - int ret;
> struct virtio_hw *hw = dev->data->dev_private;
> + uint16_t nr_vq = hw->max_queue_pairs * 2;
> + uint16_t i;
> + int ret;
>
> - PMD_INIT_FUNC_TRACE();
> - ret = virtio_dev_queue_setup(dev, VTNET_CQ, VTNET_SQ_CQ_QUEUE_IDX,
> - vtpci_queue_idx, 0, socket_id, (void **)&cvq);
> - if (ret < 0) {
> - PMD_INIT_LOG(ERR, "control vq initialization failed");
> - return ret;
> + if (vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VQ))
> + nr_vq += 1;
> +
> + hw->vqs = rte_zmalloc(NULL, sizeof(struct virtqueue *) * nr_vq, 0);
> + if (!hw->vqs) {
> + PMD_INIT_LOG(ERR, "failed to allocate vqs");
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < nr_vq; i++) {
> + ret = virtio_init_queue(dev, i);
> + if (ret < 0)
> + goto cleanup;
> }
>
> - hw->cvq = cvq;
> return 0;
> +
> +cleanup:
> + /*
> + * ctrl queue is the last queue; if we go here, it means the ctrl
> + * queue is not allocated, that we can do no cleanup for it here.
> + */
> + while (i > 0) {
> + i--;
> + if (i % 2 == 0)
> + virtio_dev_rx_queue_release(&hw->vqs[i]->rxq);
> + else
> + virtio_dev_tx_queue_release(&hw->vqs[i]->txq);
> + }
> + rte_free(hw->vqs);
> +
> + return ret;
> }
>
> static void
> @@ -1141,6 +1167,7 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
> struct virtio_net_config *config;
> struct virtio_net_config local_config;
> struct rte_pci_device *pci_dev = eth_dev->pci_dev;
> + int ret;
>
> /* Reset the device although not necessary at startup */
> vtpci_reset(hw);
> @@ -1222,6 +1249,10 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
> hw->max_queue_pairs = 1;
> }
>
> + ret = virtio_alloc_queues(eth_dev);
> + if (ret < 0)
> + return ret;
> +
> if (pci_dev)
> PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x",
> eth_dev->data->port_id, pci_dev->id.vendor_id,
> @@ -1390,15 +1421,9 @@ virtio_dev_configure(struct rte_eth_dev *dev)
> return -ENOTSUP;
> }
>
> - /* Setup and start control queue */
> - if (vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VQ)) {
> - ret = virtio_dev_cq_queue_setup(dev,
> - hw->max_queue_pairs * 2,
> - SOCKET_ID_ANY);
> - if (ret < 0)
> - return ret;
> + /* start control queue */
> + if (vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_VQ))
> virtio_dev_cq_start(dev);
> - }
>
> hw->vlan_strip = rxmode->hw_vlan_strip;
>
> diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
> index de33b32..5db8632 100644
> --- a/drivers/net/virtio/virtio_ethdev.h
> +++ b/drivers/net/virtio/virtio_ethdev.h
> @@ -80,14 +80,6 @@ void virtio_dev_cq_start(struct rte_eth_dev *dev);
> */
> void virtio_dev_rxtx_start(struct rte_eth_dev *dev);
>
> -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);
> -
> void virtio_dev_queue_release(struct virtqueue *vq);
>
> int virtio_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id,
> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index 0c5ed31..f63f76c 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -264,6 +264,8 @@ struct virtio_hw {
> struct virtio_net_config *dev_cfg;
> const struct virtio_pci_ops *vtpci_ops;
> void *virtio_user_dev;
> +
> + struct virtqueue **vqs;
> };
>
> /*
> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> index b4c4aa4..fb703d2 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -530,24 +530,24 @@ int
> virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
> uint16_t queue_idx,
> uint16_t nb_desc,
> - unsigned int socket_id,
> + unsigned int socket_id __rte_unused,
> __rte_unused const struct rte_eth_rxconf *rx_conf,
> struct rte_mempool *mp)
> {
> uint16_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_RQ_QUEUE_IDX;
> + struct virtio_hw *hw = dev->data->dev_private;
> + struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
> struct virtnet_rx *rxvq;
> - int ret;
>
> PMD_INIT_FUNC_TRACE();
> - ret = virtio_dev_queue_setup(dev, VTNET_RQ, queue_idx, vtpci_queue_idx,
> - nb_desc, socket_id, (void **)&rxvq);
> - if (ret < 0) {
> - PMD_INIT_LOG(ERR, "rvq initialization failed");
> - return ret;
> - }
>
> - /* Create mempool for rx mbuf allocation */
> + if (nb_desc == 0 || nb_desc > vq->vq_nentries)
> + nb_desc = vq->vq_nentries;
> + vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
> +
> + rxvq = &vq->rxq;
> rxvq->mpool = mp;
> + rxvq->queue_id = queue_idx;
>
> dev->data->rx_queues[queue_idx] = rxvq;
>
> @@ -613,27 +613,25 @@ int
> virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
> uint16_t queue_idx,
> uint16_t nb_desc,
> - unsigned int socket_id,
> + unsigned int socket_id __rte_unused,
> const struct rte_eth_txconf *tx_conf)
> {
> uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
> + struct virtio_hw *hw = dev->data->dev_private;
> + struct virtqueue *vq = hw->vqs[vtpci_queue_idx];
> struct virtnet_tx *txvq;
> - struct virtqueue *vq;
> uint16_t tx_free_thresh;
> - int ret;
>
> PMD_INIT_FUNC_TRACE();
>
> -
> virtio_update_rxtx_handler(dev, tx_conf);
>
> - ret = virtio_dev_queue_setup(dev, VTNET_TQ, queue_idx, vtpci_queue_idx,
> - nb_desc, socket_id, (void **)&txvq);
> - if (ret < 0) {
> - PMD_INIT_LOG(ERR, "tvq initialization failed");
> - return ret;
> - }
> - vq = txvq->vq;
> + if (nb_desc == 0 || nb_desc > vq->vq_nentries)
> + nb_desc = vq->vq_nentries;
> + vq->vq_free_cnt = RTE_MIN(vq->vq_free_cnt, nb_desc);
> +
> + txvq = &vq->txq;
> + txvq->queue_id = queue_idx;
>
> tx_free_thresh = tx_conf->tx_free_thresh;
> if (tx_free_thresh == 0)
>
^ permalink raw reply
* [PATCH v2] net/virtio: cache Rx/Tx offload ability check
From: Yuanhan Liu @ 2016-11-04 14:29 UTC (permalink / raw)
To: dev; +Cc: Yuanhan Liu, Olivier Matz
In-Reply-To: <1478251718-7464-1-git-send-email-yuanhan.liu@linux.intel.com>
It's not a good idea to do the check of whether Rx/Tx offload is
enabled at the data path. Instead, we could do the check at init
stage and store the result, so that we could avoid the check again
and again at the critical datapath.
Cc: Olivier Matz <olivier.matz@6wind.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
v2: - rebase on top of the bug fix patches
- define rx/tx_offload as uint8_t instead of int
drivers/net/virtio/virtio_ethdev.c | 19 +++++++++++++++++++
drivers/net/virtio/virtio_pci.h | 2 ++
drivers/net/virtio/virtio_rxtx.c | 31 +++++--------------------------
3 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 1505f67..2adae58 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1188,6 +1188,22 @@ rx_func_get(struct rte_eth_dev *eth_dev)
eth_dev->rx_pkt_burst = &virtio_recv_pkts;
}
+static inline int
+rx_offload_enabled(struct virtio_hw *hw)
+{
+ return vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_CSUM) ||
+ vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_TSO4) ||
+ vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_TSO6);
+}
+
+static inline int
+tx_offload_enabled(struct virtio_hw *hw)
+{
+ return vtpci_with_feature(hw, VIRTIO_NET_F_CSUM) ||
+ vtpci_with_feature(hw, VIRTIO_NET_F_HOST_TSO4) ||
+ vtpci_with_feature(hw, VIRTIO_NET_F_HOST_TSO6);
+}
+
/* reset device and renegotiate features if needed */
static int
virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
@@ -1209,6 +1225,9 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
if (virtio_negotiate_features(hw, req_features) < 0)
return -1;
+ hw->tx_offload = tx_offload_enabled(hw);
+ hw->rx_offload = rx_offload_enabled(hw);
+
/* If host does not support status then disable LSC */
if (!vtpci_with_feature(hw, VIRTIO_NET_F_STATUS))
eth_dev->data->dev_flags &= ~RTE_ETH_DEV_INTR_LSC;
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index de271bf..7d1dd9b 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -254,6 +254,8 @@ struct virtio_hw {
uint8_t use_msix;
uint8_t modern;
uint8_t use_simple_rxtx;
+ uint8_t tx_offload;
+ uint8_t rx_offload;
uint8_t mac_addr[ETHER_ADDR_LEN];
uint32_t notify_off_multiplier;
uint8_t *isr;
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 4cb2ce7..7571191 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -250,14 +250,6 @@ virtio_tso_fix_cksum(struct rte_mbuf *m)
}
}
-static inline int
-tx_offload_enabled(struct virtio_hw *hw)
-{
- return vtpci_with_feature(hw, VIRTIO_NET_F_CSUM) ||
- vtpci_with_feature(hw, VIRTIO_NET_F_HOST_TSO4) ||
- vtpci_with_feature(hw, VIRTIO_NET_F_HOST_TSO6);
-}
-
static inline void
virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
uint16_t needed, int use_indirect, int can_push)
@@ -270,9 +262,7 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
uint16_t head_idx, idx;
uint16_t head_size = vq->hw->vtnet_hdr_size;
struct virtio_net_hdr *hdr;
- int offload;
- offload = tx_offload_enabled(vq->hw);
head_idx = vq->vq_desc_head_idx;
idx = head_idx;
dxp = &vq->vq_descx[idx];
@@ -286,7 +276,7 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
hdr = (struct virtio_net_hdr *)
rte_pktmbuf_prepend(cookie, head_size);
/* if offload disabled, it is not zeroed below, do it now */
- if (offload == 0)
+ if (vq->hw->tx_offload == 0)
memset(hdr, 0, head_size);
} else if (use_indirect) {
/* setup tx ring slot to point to indirect
@@ -318,7 +308,7 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
}
/* Checksum Offload / TSO */
- if (offload) {
+ if (vq->hw->tx_offload) {
if (cookie->ol_flags & PKT_TX_TCP_SEG)
cookie->ol_flags |= PKT_TX_TCP_CKSUM;
@@ -735,14 +725,6 @@ virtio_rx_offload(struct rte_mbuf *m, struct virtio_net_hdr *hdr)
return 0;
}
-static inline int
-rx_offload_enabled(struct virtio_hw *hw)
-{
- return vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_CSUM) ||
- vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_TSO4) ||
- vtpci_with_feature(hw, VIRTIO_NET_F_GUEST_TSO6);
-}
-
#define VIRTIO_MBUF_BURST_SZ 64
#define DESC_PER_CACHELINE (RTE_CACHE_LINE_SIZE / sizeof(struct vring_desc))
uint16_t
@@ -758,7 +740,6 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
int error;
uint32_t i, nb_enqueued;
uint32_t hdr_size;
- int offload;
struct virtio_net_hdr *hdr;
nb_used = VIRTQUEUE_NUSED(vq);
@@ -777,7 +758,6 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
nb_rx = 0;
nb_enqueued = 0;
hdr_size = hw->vtnet_hdr_size;
- offload = rx_offload_enabled(hw);
for (i = 0; i < num ; i++) {
rxm = rcv_pkts[i];
@@ -808,7 +788,7 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
if (hw->vlan_strip)
rte_vlan_strip(rxm);
- if (offload && virtio_rx_offload(rxm, hdr) < 0) {
+ if (hw->rx_offload && virtio_rx_offload(rxm, hdr) < 0) {
virtio_discard_rxbuf(vq, rxm);
rxvq->stats.errors++;
continue;
@@ -873,7 +853,6 @@ virtio_recv_mergeable_pkts(void *rx_queue,
uint16_t extra_idx;
uint32_t seg_res;
uint32_t hdr_size;
- int offload;
nb_used = VIRTQUEUE_NUSED(vq);
@@ -889,7 +868,6 @@ virtio_recv_mergeable_pkts(void *rx_queue,
extra_idx = 0;
seg_res = 0;
hdr_size = hw->vtnet_hdr_size;
- offload = rx_offload_enabled(hw);
while (i < nb_used) {
struct virtio_net_hdr_mrg_rxbuf *header;
@@ -935,7 +913,8 @@ virtio_recv_mergeable_pkts(void *rx_queue,
rx_pkts[nb_rx] = rxm;
prev = rxm;
- if (offload && virtio_rx_offload(rxm, &header->hdr) < 0) {
+ if (hw->rx_offload &&
+ virtio_rx_offload(rxm, &header->hdr) < 0) {
virtio_discard_rxbuf(vq, rxm);
rxvq->stats.errors++;
continue;
--
1.9.0
^ permalink raw reply related
* Re: [PATCH 1/8] net/virtio: revert "virtio: fix restart"
From: Yuanhan Liu @ 2016-11-04 14:28 UTC (permalink / raw)
To: Maxime Coquelin; +Cc: dev, stable, Ilya Maximets
In-Reply-To: <4dec2204-4da9-da61-fbbf-d17748f2f9bc@redhat.com>
On Fri, Nov 04, 2016 at 09:09:22AM +0100, Maxime Coquelin wrote:
>
>
> On 11/04/2016 03:00 AM, Yuanhan Liu wrote:
> >On Thu, Nov 03, 2016 at 09:36:52PM +0100, Maxime Coquelin wrote:
> >>Hi Yuanhan,
> >>
> >>On 11/03/2016 05:09 PM, Yuanhan Liu wrote:
> >>>This reverts commit 9a0615af7746 ("virtio: fix restart"); conflict is
> >>>manually addressed.
> >>>
> >>>Kyle reported an issue with above commit
> >>>
> >>> qemu-kvm: Guest moved used index from 5 to 1
> >>>
> >>>with following steps,
> >>>
> >>> 1) Start my virtio interfaces
> >>> 2) Send some traffic into/out of the interfaces
> >>> 3) Stop the interfaces
> >>> 4) Start the interfaces
> >>> 5) Send some more traffic
> >>>
> >>>And here are some quotes from Kyle's analysis,
> >>>
> >>> Prior to the patch, if an interface were stopped then started, without
> >>> restarting the application, the queues would be left as-is, because
> >>> hw->started would be set to 1. Now, calling stop sets hw->started to 0,
> >>> which means the next call to start will "touch the queues". This is the
> >>> unintended side-effect that causes the problem.
> >>
> >>Maybe a good idea to explain what is the problem the revert aims to fix.
> >
> >It aims to fix the issue, by "not touching the queues" on restart.
> >
> >>It does not seem to be clearly stated in the commit message.
> >
> >I was thinking the quote from Kyle is enough. How about following supplement:
> >
> > We should not touch the queues once the init is done, otherwise, the
> > vring state of virtio PMD driver and vhost-user would be inconsistent,
> > leading some issue like above.
> >
> > Thus this patch is reverted.
> >
> >Better now?
> Yes, this is much clearer from my PoV.
Fixed, and series applied to dpdk-next-virtio.
Thanks for the review!
--yliu
^ permalink raw reply
* Re: [PATCH] examples/ipsec-secgw: fix buffer not null terminated
From: Ferruh Yigit @ 2016-11-04 14:18 UTC (permalink / raw)
To: Fan Zhang, dev
In-Reply-To: <1478175163-229116-1-git-send-email-roy.fan.zhang@intel.com>
On 11/3/2016 12:12 PM, Fan Zhang wrote:
> Fixes: 0d547ed03717 ("examples/ipsec-secgw: support configuration file")
> Coverity issue: 137854
>
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
Minor nit, for all coverity fixes, defined commit log format is:
Coverity issue: xxxxx
Fixes: .....
Basically two lines should be swapped, but I guess this can be fixed
while applying instead of sending a new version for this.
^ permalink raw reply
* Re: [PATCH] examples/ipsec-secgw: fix buffer not terminated issue
From: Ferruh Yigit @ 2016-11-04 14:16 UTC (permalink / raw)
To: Fan Zhang, dev
In-Reply-To: <1478175163-229116-2-git-send-email-roy.fan.zhang@intel.com>
On 11/3/2016 12:12 PM, Fan Zhang wrote:
> Fixes: 0d547ed03717 ("examples/ipsec-secgw: support configuration file")
> Coverity issue: 137855
>
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
^ permalink raw reply
* Re: [PATCH] examples/ipsec-secgw: fix copy into fixed size buffer issue
From: Ferruh Yigit @ 2016-11-04 14:15 UTC (permalink / raw)
To: Fan Zhang, dev
In-Reply-To: <1478175163-229116-3-git-send-email-roy.fan.zhang@intel.com>
On 11/3/2016 12:12 PM, Fan Zhang wrote:
> Coverity issue: 137875
> Fixes: 0d547ed0 ("examples/ipsec-secgw: support configuration
> file")
>
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
> examples/ipsec-secgw/sa.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> index 9e2c8a9..c891be2 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -177,7 +177,7 @@ parse_key_string(const char *key_str, uint8_t *key)
> pt_end = strchr(pt_start, ':');
>
> if (pt_end == NULL)
> - strncpy(sub_str, pt_start, strlen(pt_start));
> + strncpy(sub_str, pt_start, strlen(sub_str) - 1);
sub_str initial value is not known, "strlen(sub_str) - 1" leaves last
byte of the string random, instead of intended NULL.
Also sub_str has hardcoded length of 3, it can be good to confirm NULL
terminated byte taken into account, and number actually can be maximum
two digits long.
> else {
> if (pt_end - pt_start > 2)
> return 0;
>
^ permalink raw reply
* Re: [PATCH] examples/ipsec-secgw: fix pointer to local outside scope
From: Ferruh Yigit @ 2016-11-04 14:15 UTC (permalink / raw)
To: Fan Zhang, dev
In-Reply-To: <1478175163-229116-4-git-send-email-roy.fan.zhang@intel.com>
On 11/3/2016 12:12 PM, Fan Zhang wrote:
> Fixes: 0d547ed03717 ("examples/ipsec-secgw: support configuration file")
> Coverity issue: 137871
>
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
Coverity issue fix looks good to me, but since indentation level reduced
one level, many line wrappings can be re-arranged.
Apart from line wrappings,
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
^ permalink raw reply
* Re: [PATCH 3/3] vhost: update comments
From: Yuanhan Liu @ 2016-11-04 14:15 UTC (permalink / raw)
To: Mcnamara, John; +Cc: dev@dpdk.org
In-Reply-To: <B27915DBBA3421428155699D51E4CFE20264A8CB@IRSMSX103.ger.corp.intel.com>
On Fri, Nov 04, 2016 at 01:28:45PM +0000, Mcnamara, John wrote:
>
>
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > Sent: Wednesday, November 2, 2016 3:15 AM
> > To: dev@dpdk.org
> > Cc: Mcnamara, John <john.mcnamara@intel.com>; Yuanhan Liu
> > <yuanhan.liu@linux.intel.com>
> > Subject: [PATCH 3/3] vhost: update comments
> >
> > vhost-cuse is removed, update corresponding comments that are still
> > referencing it.
> >
> > Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> >
> > ...
> >
> > /*
> > - * Backend-specific cleanup. Defined by vhost-cuse and vhost-user.
> > + * Backend-specific cleanup.
> > + *
> > + * TODO: fix it; we have one backend now
> > */
> > void vhost_backend_cleanup(struct virtio_net *dev);
>
> In general it is not a good idea to leave TODOs/Fixmes around unless they
> that a better explanation of why they are there.
Agreed.
In this case, if you look at the delete line, you will find we have such
function implemented both in vhost-user and vhost-cuse. Now we get one
only. So, we may could just rename it to vhost_user_cleanup(). Or even,
remove it completely by doing some small refactors.
It's obvious that we need do that in another patch. Thus, the TODO is
been added.
--yliu
>
> Apart from that:
>
>
> Acked-by: John McNamara <john.mcnamara@intel.com>
>
^ permalink raw reply
* Re: [PATCH v2] doc: arm64: document DPDK application profiling methods
From: Mcnamara, John @ 2016-11-04 14:00 UTC (permalink / raw)
To: Jerin Jacob, dev@dpdk.org
Cc: thomas.monjalon@6wind.com, jianbo.liu@linaro.org,
viktorin@rehivetech.com
In-Reply-To: <1475657009-11139-1-git-send-email-jerin.jacob@caviumnetworks.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jerin Jacob
> Sent: Wednesday, October 5, 2016 9:43 AM
> To: dev@dpdk.org
> Cc: thomas.monjalon@6wind.com; jianbo.liu@linaro.org;
> viktorin@rehivetech.com; Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Subject: [dpdk-dev] [PATCH v2] doc: arm64: document DPDK application
> profiling methods
>
Hi Jerin,
Thanks for this. It is probably something that should be extended from
the Intel side as well.
...
> Profile Your Application
> ========================
>
> +Introduction
> +------------
> +
> +The following sections describe the methods to profile DPDK
> +applications on different architectures.
You could probably just skip the introduction heading and put this
text under the main heading.
As Thomas pointed out there are a few other RST formatting issues
as well. I'll forward some suggested changes.
John
^ permalink raw reply
* Re: [PATCH] net/i40e: fix dcb configure failure
From: Xing, Beilei @ 2016-11-04 13:44 UTC (permalink / raw)
To: Wu, Jingjing, dev@dpdk.org; +Cc: Zhang, Helin
In-Reply-To: <1478238153-92544-1-git-send-email-jingjing.wu@intel.com>
> -----Original Message-----
> From: Wu, Jingjing
> Sent: Friday, November 4, 2016 1:43 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>;
> Zhang, Helin <helin.zhang@intel.com>
> Subject: [PATCH] net/i40e: fix dcb configure failure
>
> Removing stopping LLDP in firmware is a workaround for a known errata which
> can cause Rx hang. But the changing will cause DCB configuration fails. That is
> because when LLDP is enabled, the return value of i40e_init_dcb is success. But
> following check just considered the case when LLDP agent is disabled.
> This patch fixes this issue.
>
> Fixes: fcbd40d4327b ("net/i40e: fix Rx hang when disable LLDP")
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
Acked-by: Beilei Xing<Beilei.xing@intel.com>
^ permalink raw reply
* Re: [PATCH v1] doc: announce API and ABI change for librte_ether
From: Mcnamara, John @ 2016-11-04 13:39 UTC (permalink / raw)
To: Iremonger, Bernard, dev@dpdk.org
In-Reply-To: <1476797888-29698-1-git-send-email-bernard.iremonger@intel.com>
> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Tuesday, October 18, 2016 2:38 PM
> To: dev@dpdk.org; Mcnamara, John <john.mcnamara@intel.com>
> Cc: Iremonger, Bernard <bernard.iremonger@intel.com>
> Subject: [PATCH v1] doc: announce API and ABI change for librte_ether
>
> In 17.02 five rte_eth_dev_set_vf_*** functions will be removed from
> librte_ether, renamed and moved to the ixgbe PMD.
>
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
Acked-by: John McNamara <john.mcnamara@intel.com>
^ permalink raw reply
* Re: [PATCH v1] doc: announce API change for ethdev function
From: Mcnamara, John @ 2016-11-04 13:39 UTC (permalink / raw)
To: Iremonger, Bernard, dev@dpdk.org
In-Reply-To: <1476797888-29698-2-git-send-email-bernard.iremonger@intel.com>
> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Tuesday, October 18, 2016 2:38 PM
> To: dev@dpdk.org; Mcnamara, John <john.mcnamara@intel.com>
> Cc: Iremonger, Bernard <bernard.iremonger@intel.com>
> Subject: [PATCH v1] doc: announce API change for ethdev function
>
> The _rte_eth_dev_call_process function will change to return "int"
> and a fourth parameter "void* ret_param" will be added. This change
> targets release 17.02.
>
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
Acked-by: John McNamara <john.mcnamara@intel.com>
^ permalink raw reply
* Re: [PATCH 2/3] doc: update the vhost sample guide
From: Mcnamara, John @ 2016-11-04 13:38 UTC (permalink / raw)
To: Yuanhan Liu, dev@dpdk.org
In-Reply-To: <1478056501-9321-3-git-send-email-yuanhan.liu@linux.intel.com>
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, November 2, 2016 3:15 AM
> To: dev@dpdk.org
> Cc: Mcnamara, John <john.mcnamara@intel.com>; Yuanhan Liu
> <yuanhan.liu@linux.intel.com>
> Subject: [PATCH 2/3] doc: update the vhost sample guide
>
> For vhost-switch sample, the old guide takes too many words on
> vhost-cuse, which is mainly due to vhost-cuse is invented before
> vhost-user.
>
> Now vhost-cuse is removed, meaning the best part of the doc is useless.
> Instead of amending one piece here and there, this patch simply removes
> the most part of the doc and replace it with a simple test guide.
>
> For tep_term sample, mainly for removing the part has "vhost-cuse".
>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Good cleanup.
Acked-by: John McNamara <john.mcnamara@intel.com>
^ permalink raw reply
* Re: [PATCH 3/3] vhost: update comments
From: Mcnamara, John @ 2016-11-04 13:28 UTC (permalink / raw)
To: Yuanhan Liu, dev@dpdk.org
In-Reply-To: <1478056501-9321-4-git-send-email-yuanhan.liu@linux.intel.com>
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, November 2, 2016 3:15 AM
> To: dev@dpdk.org
> Cc: Mcnamara, John <john.mcnamara@intel.com>; Yuanhan Liu
> <yuanhan.liu@linux.intel.com>
> Subject: [PATCH 3/3] vhost: update comments
>
> vhost-cuse is removed, update corresponding comments that are still
> referencing it.
>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>
> ...
>
> /*
> - * Backend-specific cleanup. Defined by vhost-cuse and vhost-user.
> + * Backend-specific cleanup.
> + *
> + * TODO: fix it; we have one backend now
> */
> void vhost_backend_cleanup(struct virtio_net *dev);
In general it is not a good idea to leave TODOs/Fixmes around unless they
that a better explanation of why they are there.
Apart from that:
Acked-by: John McNamara <john.mcnamara@intel.com>
^ permalink raw reply
* Re: [PATCH 1/3] doc: update vhost programming guide
From: Mcnamara, John @ 2016-11-04 13:22 UTC (permalink / raw)
To: Yuanhan Liu, dev@dpdk.org
In-Reply-To: <1478056501-9321-2-git-send-email-yuanhan.liu@linux.intel.com>
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, November 2, 2016 3:15 AM
> To: dev@dpdk.org
> Cc: Mcnamara, John <john.mcnamara@intel.com>; Yuanhan Liu
> <yuanhan.liu@linux.intel.com>
> Subject: [PATCH 1/3] doc: update vhost programming guide
>
> vhost-cuse has been removed in this release. Update the doc, with the
> vhost-cuse part being removed.
>
> Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Acked-by: John McNamara <john.mcnamara@intel.com>
^ permalink raw reply
* Re: [PATCH v4] vhost: Add indirect descriptors support to the TX path
From: Wang, Zhihong @ 2016-11-04 13:09 UTC (permalink / raw)
To: Maxime Coquelin, Yuanhan Liu
Cc: stephen@networkplumber.org, Pierre Pfister (ppfister),
Xie, Huawei, dev@dpdk.org, vkaplans@redhat.com, mst@redhat.com
In-Reply-To: <75d2c094-11e8-99aa-ccb5-60ded04047d7@redhat.com>
> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Friday, November 4, 2016 8:54 PM
> To: Wang, Zhihong <zhihong.wang@intel.com>; Yuanhan Liu
> <yuanhan.liu@linux.intel.com>
> Cc: stephen@networkplumber.org; Pierre Pfister (ppfister)
> <ppfister@cisco.com>; Xie, Huawei <huawei.xie@intel.com>; dev@dpdk.org;
> vkaplans@redhat.com; mst@redhat.com
> Subject: Re: [dpdk-dev] [PATCH v4] vhost: Add indirect descriptors support
> to the TX path
>
>
>
> On 11/04/2016 01:30 PM, Wang, Zhihong wrote:
> >
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> >> Sent: Friday, November 4, 2016 7:23 PM
> >> To: Wang, Zhihong <zhihong.wang@intel.com>; Yuanhan Liu
> >> <yuanhan.liu@linux.intel.com>
> >> Cc: stephen@networkplumber.org; Pierre Pfister (ppfister)
> >> <ppfister@cisco.com>; Xie, Huawei <huawei.xie@intel.com>;
> dev@dpdk.org;
> >> vkaplans@redhat.com; mst@redhat.com
> >> Subject: Re: [dpdk-dev] [PATCH v4] vhost: Add indirect descriptors
> support to the
> >> TX path
> >>
> >>
> >>
> >>>>>> Hi Maxime,
> >>>>>>
> >>>>>> I did a little more macswap test and found out more stuff here:
> >>>>> Thanks for doing more tests.
> >>>>>
> >>>>>>
> >>>>>> 1. I did loopback test on another HSW machine with the same H/W,
> >>>>>> and indirect_desc on and off seems have close perf
> >>>>>>
> >>>>>> 2. So I checked the gcc version:
> >>>>>>
> >>>>>> * Previous: gcc version 6.2.1 20160916 (Fedora 24)
> >>>>>>
> >>>>>> * New: gcc version 5.4.0 20160609 (Ubuntu 16.04.1 LTS)
> >>>>>
> >>>>> On my side, I tested with RHEL7.3:
> >>>>> - gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-11)
> >>>>>
> >>>>> It certainly contains some backports from newer GCC versions.
> >>>>>
> >>>>>>
> >>>>>> On previous one indirect_desc has 20% drop
> >>>>>>
> >>>>>> 3. Then I compiled binary on Ubuntu and scp to Fedora, and as
> >>>>>> expected I got the same perf as on Ubuntu, and the perf gap
> >>>>>> disappeared, so gcc is definitely one factor here
> >>>>>>
> >>>>>> 4. Then I use the Ubuntu binary on Fedora for PVP test, then the
> >>>>>> perf gap comes back again and the same with the Fedora binary
> >>>>>> results, indirect_desc causes about 20% drop
> >>>>>
> >>>>> Let me know if I understand correctly:
> >>>
> >>> Yes, and it's hard to breakdown further at this time.
> >>>
> >>> Also we may need to check whether it's caused by certain NIC
> >>> model. Unfortunately I don't have the right setup right now.
> >>>
> >>>>> Loopback test with macswap:
> >>>>> - gcc version 6.2.1 : 20% perf drop
> >>>>> - gcc version 5.4.0 : No drop
> >>>>>
> >>>>> PVP test with macswap:
> >>>>> - gcc version 6.2.1 : 20% perf drop
> >>>>> - gcc version 5.4.0 : 20% perf drop
> >>>>
> >>>> I forgot to ask, did you recompile only host, or both host and guest
> >>>> testmpd's in your test?
> >>
> >>> Both.
> >>
> >> I recompiled testpmd on a Fedora 24 machine using GCC6:
> >> gcc (GCC) 6.1.1 20160621 (Red Hat 6.1.1-3)
> >> Testing loopback with macswap on my Haswell RHEL7.3 machine gives me
> the
> >> following results:
> >> - indirect on: 7.75Mpps
> >> - indirect off: 7.35Mpps
> >>
> >> Surprisingly, I get better results with indirect on my setup (I
> >> reproduced the tests multiple times).
> >>
> >> Do you have a document explaining the tuning/config you apply to both
> >> the host and the guest (isolation, HT, hugepage size, ...) in your
> >> setup?
> >
> >
> > The setup where it goes wrong:
> > 1. Xeon E5-2699, HT on, turbo off, 1GB hugepage for both host and guest
> On the Haswell machine (on which I don't have BIOS access), HT is on,
> but I unplug siblings at runtime.
> I also have 1G pages on both sides, and I isolate the cores used by both
> testpmd and vCPUS.
>
> > 2. Fortville 40G
> > 3. Fedora 4.7.5-200.fc24.x86_64
> > 4. gcc version 6.2.1
> > 5. 16.11 RC2 for both host and guest
> > 6. PVP, testpmd macswap for both host and guest
> >
> > BTW, I do see indirect_desc gives slightly better performance for loopback
> > in tests on other platforms, but don't know how PVP performs yet.
> Interesting, other platforms are also Haswell/Broadwell?
Yes, but with different OS.
If you don't have the setup I can do more detailed profiling for the
root cause next week, since my platform is the only one right now that
reporting the drop.
>
> For PVP benchmarks, are your figures with 0% pkt loss?
No, for testpmd perf analysis it's not necessary in my opinion.
I do tried low rate though, the result is the same.
>
> Thanks,
> Maxime
>
> >
> >
> >>
> >> Regards,
> >> Maxime
^ permalink raw reply
* Re: [PATCH] doc: add limitation for L3fwd-power app
From: Mcnamara, John @ 2016-11-04 13:04 UTC (permalink / raw)
To: De Lara Guarch, Pablo, dev@dpdk.org; +Cc: De Lara Guarch, Pablo
In-Reply-To: <1478263697-57575-1-git-send-email-pablo.de.lara.guarch@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pablo de Lara
> Sent: Friday, November 4, 2016 12:48 PM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Subject: [dpdk-dev] [PATCH] doc: add limitation for L3fwd-power app
>
> L3fwd-power app needs vector mode to be disabled in order to work
> properly. The app used to work previously, because it was using Rx scalar
> function, but now it uses vector function.
>
> Vector mode needs to be disabled to make the app works, which has been
> documented in release notes.
>
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Acked-by: John McNamara <john.mcnamara@intel.com>
^ permalink raw reply
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