[dpdk-dev] [PATCH] spinlock:move constructor function out of header

[Wiles, Keith]

> On Jul 14, 2016, at 2:59 PM, Thomas Monjalon  
> wrote:
> 
> Thanks Keith for continuing work.
> 
> 2016-07-14 14:31, Keith Wiles:
>> Moving the rte_rtm_init() constructor routine out of the
>> header file and into a new rte_spinlock.c for all archs/platforms.
>> Having constructor routines in a header file is not a good
>> place to keep these types of functions.
>> 
>> The problem is with linking 3rd party libraries when
>> an application is not linked directly to dpdk libraries, which
>> in this case causes a missing symbol in the linking phase.
>> 
>> Fixes: ba7468997ea6 ("spinlock: add HTM lock elision for x86")
>> 
>> Originally submitted by Damjan Marion 
> 
> You should keep the original Signed-off and authorship (From: field)
> with "git am".
> It is also easier to track when using -v2 --in-reply-to='?.

I can try and add that into the patch for v2.
> 
>> Signed-off-by: Keith Wiles 
>> ---
>> lib/librte_eal/bsdapp/eal/Makefile |  1 +
>> lib/librte_eal/common/arch/arm/rte_spinlock.c  | 46 
>> ++
>> lib/librte_eal/common/arch/ppc_64/rte_spinlock.c   | 46 
>> ++
>> lib/librte_eal/common/arch/tile/rte_spinlock.c | 46 
>> ++
>> lib/librte_eal/common/arch/x86/rte_spinlock.c  | 46 
>> ++
>> .../common/include/arch/x86/rte_spinlock.h | 14 ++-
>> lib/librte_eal/linuxapp/eal/Makefile   |  1 +
> 
> I am not sure we should add a .c file for each arch, given it is called only
> from arch/x86/rte_spinlock.h.

I did not like having the .c for everyone, but the previous comment seemed to 
suggest it. I am willing to change it any better method, just let me know what 
you think. I would like just one.

On a side note I have combined the bsdapp and linuxapp into a single directory 
before. It is doable and it eliminates a number of duplicate files or code. 
Plus a also added support for OS X for DPDK, but I do not have access to any 
NICs with that version yet other then virtual ones. I could submit it and may 
be someone will write the kext to make it work. :-)

[dpdk-dev] spinlock: Move constructor function out of header file

[Damjan Marion (damarion)]

> On 15 Jul 2016, at 00:06, Thomas Monjalon  
> wrote:
> 
> 2016-07-14 18:10, Damjan Marion:
>> Dear Jan,
>> 
>> Thank you for your comments. A bit too much overhead to submit simple patch
>> so let?s forget about it. I will just add it as it is to our private
>> collection of patches.
> 
> These are changes trivial to fix when applying.
> I strongly prefer that you upstream patches instead of keeping patches
> in the VPP repository. I will help you in this task.
> Thanks for the effort.

Yeah, I agree with you, unfortunately it is all about time,
for me it is still cheaper to rebase them.

I respect your rules, but I just don?t have enough free cycles
to spend learning all of them, for my occasional patch submission.

[dpdk-dev] [PATCH] net/i40e: add additional prefetch instructions for bulk rx

[Vladyslav Buslov]

Added prefetch of first packet payload cacheline in i40e_rx_scan_hw_ring
Added prefetch of second mbuf cacheline in i40e_rx_alloc_bufs

Signed-off-by: Vladyslav Buslov 
---
 drivers/net/i40e/i40e_rxtx.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index d3cfb98..e493fb4 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1003,6 +1003,7 @@ i40e_rx_scan_hw_ring(struct i40e_rx_queue *rxq)
/* Translate descriptor info to mbuf parameters */
for (j = 0; j < nb_dd; j++) {
mb = rxep[j].mbuf;
+   rte_prefetch0(RTE_PTR_ADD(mb->buf_addr, 
RTE_PKTMBUF_HEADROOM));
qword1 = rte_le_to_cpu_64(\
rxdp[j].wb.qword1.status_error_len);
pkt_len = ((qword1 & I40E_RXD_QW1_LENGTH_PBUF_MASK) >>
@@ -1086,9 +1087,11 @@ i40e_rx_alloc_bufs(struct i40e_rx_queue *rxq)

rxdp = >rx_ring[alloc_idx];
for (i = 0; i < rxq->rx_free_thresh; i++) {
-   if (likely(i < (rxq->rx_free_thresh - 1)))
+   if (likely(i < (rxq->rx_free_thresh - 1))) {
/* Prefetch next mbuf */
-   rte_prefetch0(rxep[i + 1].mbuf);
+   rte_prefetch0([i + 1].mbuf->cacheline0);
+   rte_prefetch0([i + 1].mbuf->cacheline1);
+   }

mb = rxep[i].mbuf;
rte_mbuf_refcnt_set(mb, 1);
-- 
2.8.3

[dpdk-dev] [PATCH] Add missing prefetches to i40e bulk rx path

[Vladyslav Buslov]

Hello,

Recently I tried to use bulk rx function to reduce CPU usage of 
rte_eth_rx_burst.
However application performance with i40e_recv_pkts_bulk_alloc was 
significantly worse than with i40e_recv_pkts. (3m less PPS, 0.5 IPC on 
receiving core)

Quick investigation revealed two problems:
 - First payload cacheline is prefetched in i40e_recv_pkts but not in 
i40e_recv_pkts_bulk_alloc.
 - Only first line of next mbuf is prefetched during mbuf init in 
i40e_rx_alloc_bufs. This causes cache miss at setting 'next' field from mbuf 
cacheline1 to NULL.

Fixing these two small issues significantly reduced CPU time spent in 
rte_eth_rx_burst and improved PPS compared to both original 
i40e_recv_pkts_bulk_alloc and i40e_recv_pkts.

Regards,

Vladyslav Buslov (1):
  net/i40e: add additional prefetch instructions for bulk rx

 drivers/net/i40e/i40e_rxtx.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

-- 
2.8.3

[dpdk-dev] spinlock: Move constructor function out of header file

[Jan Viktorin]

Hello Damjan,

thank you for the patch. It makes sense to me.

Next time, please CC the appropriate maintainers.
(See the MAINTAINERS file in the root of the DPDK source tree.)

In the subject after "spinlock:" you should start with a lower case
letter, so "move constructor..."

On Thu, 14 Jul 2016 15:27:29 +0200
damarion at cisco.com wrote:

> From: Damjan Marion 
> 
> Having constructor function in the header gile is generaly

I'd write:

Having constructor functions in header files is generally a bad idea.

Anyway:

s/gile/file/

> bad idea, as it will eventually be implanted to 3rd party
> library.
> 
> In this case it is causing linking issues with 3rd party

it causes linking issues

> libraries when application is not linked to dpdk, due to missing

an application

to dpdk due to a missing gymbol (no comma)

> symbol called by constructor.

Please include the following line:

Fixes: ba7468997ea6 ("spinlock: add HTM lock elision for x86")

> 
> Signed-off-by: Damjan Marion 
> 
> ---
> lib/librte_eal/common/arch/x86/rte_spinlock.c  | 45 ++
>  .../common/include/arch/x86/rte_spinlock.h | 13 ++-
>  lib/librte_eal/linuxapp/eal/Makefile   |  1 +
>  3 files changed, 49 insertions(+), 10 deletions(-)
>  create mode 100644 lib/librte_eal/common/arch/x86/rte_spinlock.c
> 
> diff --git a/lib/librte_eal/common/arch/x86/rte_spinlock.c 
> b/lib/librte_eal/common/arch/x86/rte_spinlock.c
> new file mode 100644
> index 000..ad8cc5a
> --- /dev/null
> +++ b/lib/librte_eal/common/arch/x86/rte_spinlock.c
> @@ -0,0 +1,45 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2010-2014 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_cpuflags.h"
> +
> +#include 

According to:
http://dpdk.org/doc/guides-16.04/contributing/coding_style.html#coding-style

you should change the order of these includes.

> +
> +uint8_t rte_rtm_supported; /* cache the flag to avoid the overhead
> +   of the rte_cpu_get_flag_enabled function */

The comment should be placed before the declaration or use the /**< */
Doxygen style. I'd prefer to placed it before. Can you fix it with this
patch?

> +
> +static void __attribute__((constructor))
> +rte_rtm_init(void)
> +{
> + rte_rtm_supported = rte_cpu_get_flag_enabled(RTE_CPUFLAG_RTM);
> +}
> diff --git a/lib/librte_eal/common/include/arch/x86/rte_spinlock.h 
> b/lib/librte_eal/common/include/arch/x86/rte_spinlock.h
> index 02f95cb..8e630c2 100644
> --- a/lib/librte_eal/common/include/arch/x86/rte_spinlock.h
> +++ b/lib/librte_eal/common/include/arch/x86/rte_spinlock.h
> @@ -94,24 +94,17 @@ rte_spinlock_trylock (rte_spinlock_t *sl)
>  }
>  #endif
>  
> -static uint8_t rtm_supported; /* cache the flag to avoid the overhead
> -  of the rte_cpu_get_flag_enabled function */
> -
> -static inline void __attribute__((constructor))
> -rte_rtm_init(void)
> -{
> - rtm_supported = rte_cpu_get_flag_enabled(RTE_CPUFLAG_RTM);
> -}
> +extern uint8_t rte_rtm_supported;
>  
>  static inline int rte_tm_supported(void)
>  {
> - return rtm_supported;
> + return rte_rtm_supported;
>  }
>  
>  static inline int
>  rte_try_tm(volatile int *lock)
>  {
> - if (!rtm_supported)
> + if (!rte_rtm_supported)
>   return 0;

[dpdk-dev] [dpdk-dev, 1/2] app/test: rework command registration

[vikto...@rehivetech.com]

On Wed, 13 Jul 2016 23:24:09 +0200
Thomas Monjalon  wrote:

> The tests are registered with their command name by adding a structure
> to a list. The structure of each test was declared in each test file
> and passed to the register macro.
> This rework generate the structure inside the register macro.

There is just this little issue:

WARNING:LONG_LINE: line over 80 characters
#335: FILE: app/test/test_cryptodev_perf.c:2924:
+REGISTER_TEST_COMMAND(cryptodev_sw_snow3g_perftest, 
perftest_sw_snow3g_cryptodev);

WARNING:LONG_LINE: line over 80 characters
#336: FILE: app/test/test_cryptodev_perf.c:2925:
+REGISTER_TEST_COMMAND(cryptodev_qat_snow3g_perftest, 
perftest_qat_snow3g_cryptodev);

total: 0 errors, 2 warnings, 758 lines checked

Otherwise
Reviewed-by: Jan Viktorin 

> 
> Signed-off-by: Thomas Monjalon 
> 
> ---
> app/test/test.h  | 17 +--
>  app/test/test_acl.c  |  6 +-
>  app/test/test_alarm.c|  6 +-
>  app/test/test_atomic.c   |  6 +-
>  app/test/test_byteorder.c|  6 +-
>  app/test/test_cmdline.c  |  6 +-
>  app/test/test_common.c   |  6 +-
>  app/test/test_cpuflags.c |  6 +-
>  app/test/test_cryptodev.c| 41 
> ++--
>  app/test/test_cryptodev_perf.c   | 28 
>  app/test/test_cycles.c   |  6 +-
>  app/test/test_debug.c|  6 +-
>  app/test/test_devargs.c  |  6 +-
>  app/test/test_distributor.c  |  6 +-
>  app/test/test_distributor_perf.c |  6 +-
>  app/test/test_eal_flags.c|  6 +-
>  app/test/test_eal_fs.c   |  6 +-
>  app/test/test_errno.c|  6 +-
>  app/test/test_func_reentrancy.c  |  6 +-
>  app/test/test_hash.c |  6 +-
>  app/test/test_hash_functions.c   |  6 +-
>  app/test/test_hash_multiwriter.c |  8 +--
>  app/test/test_hash_perf.c|  6 +-
>  app/test/test_hash_scaling.c |  7 +-
>  app/test/test_interrupts.c   |  6 +-
>  app/test/test_ivshmem.c  |  6 +-
>  app/test/test_kni.c  |  6 +-
>  app/test/test_kvargs.c   |  6 +-
>  app/test/test_link_bonding.c |  6 +-
>  app/test/test_link_bonding_mode4.c   |  7 +-
>  app/test/test_link_bonding_rssconf.c |  7 +-
>  app/test/test_logs.c |  6 +-
>  app/test/test_lpm.c  |  6 +-
>  app/test/test_lpm6.c |  6 +-
>  app/test/test_lpm6_perf.c|  6 +-
>  app/test/test_lpm_perf.c |  6 +-
>  app/test/test_malloc.c   |  6 +-
>  app/test/test_mbuf.c |  6 +-
>  app/test/test_memcpy.c   |  6 +-
>  app/test/test_memcpy_perf.c  |  6 +-
>  app/test/test_memory.c   |  6 +-
>  app/test/test_mempool.c  |  6 +-
>  app/test/test_mempool_perf.c |  6 +-
>  app/test/test_memzone.c  |  6 +-
>  app/test/test_meter.c|  6 +-
>  app/test/test_mp_secondary.c |  6 +-
>  app/test/test_pci.c  |  6 +-
>  app/test/test_per_lcore.c|  6 +-
>  app/test/test_pmd_perf.c |  6 +-
>  app/test/test_pmd_ring.c |  6 +-
>  app/test/test_pmd_ring_perf.c|  6 +-
>  app/test/test_power.c|  6 +-
>  app/test/test_power_acpi_cpufreq.c   |  6 +-
>  app/test/test_power_kvm_vm.c |  6 +-
>  app/test/test_prefetch.c |  6 +-
>  app/test/test_red.c  | 20 +++---
>  app/test/test_reorder.c  |  6 +-
>  app/test/test_resource.c |  6 +-
>  app/test/test_ring.c |  6 +-
>  app/test/test_ring_perf.c|  6 +-
>  app/test/test_rwlock.c   |  6 +-
>  app/test/test_sched.c|  6 +-
>  app/test/test_spinlock.c |  6 +-
>  app/test/test_string_fns.c   |  6 +-
>  app/test/test_table.c|  6 +-
>  app/test/test_tailq.c|  6 +-
>  app/test/test_thash.c|  6 +-
>  app/test/test_timer.c|  6 +-
>  app/test/test_timer_perf.c   |  6 +-
>  app/test/test_timer_racecond.c   |  6 +-
>  app/test/test_version.c  |  6 +-
>  71 files changed, 91 insertions(+), 422 deletions(-)
> 

[...]

[dpdk-dev] [PATCH v6 02/17] crypto: no need for a crypto pmd type

[vikto...@rehivetech.com]

On Tue, 12 Jul 2016 11:31:07 +0530
Shreyansh Jain  wrote:

> This information is not used and just adds noise.
> 
> Signed-off-by: David Marchand 
> Signed-off-by: Shreyansh Jain 
> ---
Reviewed-by: Jan Viktorin 

[dpdk-dev] [PATCH v6 04/17] eal: remove duplicate function declaration

[vikto...@rehivetech.com]

On Tue, 12 Jul 2016 11:31:09 +0530
Shreyansh Jain  wrote:

> rte_eal_dev_init is declared in both eal_private.h and rte_dev.h since its
> introduction.
> This function has been exported in ABI, so remove it from eal_private.h
> 
> Fixes: e57f20e05177 ("eal: make vdev init path generic for both virtual and 
> pci devices")
> Signed-off-by: David Marchand 
> Signed-off-by: Shreyansh Jain 
> ---
Reviewed-by: Jan Viktorin 

[dpdk-dev] [PATCH v6 03/17] drivers: align pci driver definitions

[vikto...@rehivetech.com]

On Tue, 12 Jul 2016 11:31:08 +0530
Shreyansh Jain  wrote:

> Pure coding style, but it might make it easier later if we want to move
> fields in rte_cryptodev_driver and eth_driver structures.
> 
> Signed-off-by: David Marchand 
> Signed-off-by: Shreyansh Jain 
> ---
Reviewed-by: Jan Viktorin 

[dpdk-dev] [PATCH v6 01/17] pci: no need for dynamic tailq init

[vikto...@rehivetech.com]

On Tue, 12 Jul 2016 11:31:06 +0530
Shreyansh Jain  wrote:

> These lists can be initialized once and for all at build time.
> With this, those lists are only manipulated in a common place
> (and we could even make them private).
> 
> A nice side effect is that pci drivers can now register in constructors.
> 
> Signed-off-by: David Marchand 
> Reviewed-by: Jan Viktorin 
> Signed-off-by: Shreyansh Jain 
> ---
Tested-by: Jan Viktorin 

[dpdk-dev] [PATCH v6 09/17] crypto: get rid of crypto driver register callback

[vikto...@rehivetech.com]

On Tue, 12 Jul 2016 11:31:14 +0530
Shreyansh Jain  wrote:

> Now that all pdev are pci drivers, we don't need to register crypto drivers
> through a dedicated channel.
> 
> Signed-off-by: David Marchand 
> Signed-off-by: Shreyansh Jain 
> ---
Reviewed-by: Jan Viktorin 

[dpdk-dev] [PATCH v6 10/17] ethdev: get rid of eth driver register callback

[vikto...@rehivetech.com]

On Tue, 12 Jul 2016 11:31:15 +0530
Shreyansh Jain  wrote:

> Now that all pdev are pci drivers, we don't need to register ethdev drivers
> through a dedicated channel.
> 
> Signed-off-by: David Marchand 
> Signed-off-by: Shreyansh Jain 
> ---
Reviewed-by: Jan Viktorin 

[dpdk-dev] [PATCH v6 11/17] eal/linux: move back interrupt thread init before setting affinity

[vikto...@rehivetech.com]

On Tue, 12 Jul 2016 11:31:16 +0530
Shreyansh Jain  wrote:

> Now that virtio pci driver is initialized in a constructor, iopl() stuff
> happens early enough so that interrupt thread can be created right after
> plugin loading.
> This way, chelsio driver should be happy again [1].
> 
> [1] http://dpdk.org/ml/archives/dev/2015-November/028289.html
> 
> Signed-off-by: David Marchand 
> Tested-by: Rahul Lakkireddy 
> Signed-off-by: Shreyansh Jain 
> ---
Reviewed-by: Jan Viktorin 

[dpdk-dev] [PATCH v6 07/17] ethdev: export init/uninit common wrappers for pci drivers

[vikto...@rehivetech.com]

On Tue, 12 Jul 2016 11:31:12 +0530
Shreyansh Jain  wrote:

> Preparing for getting rid of eth_drv, here are two wrappers that can be
> used by pci drivers that assume a 1 to 1 association between pci resource and
> upper interface.
> 
> Signed-off-by: David Marchand 
> Signed-off-by: Shreyansh Jain 
> ---
Reviewed-by: Jan Viktorin 

[dpdk-dev] [PATCH v6 06/17] crypto: export init/uninit common wrappers for pci drivers

[vikto...@rehivetech.com]

On Tue, 12 Jul 2016 11:31:11 +0530
Shreyansh Jain  wrote:

> Preparing for getting rid of rte_cryptodev_driver, here are two wrappers
> that can be used by pci drivers that assume a 1 to 1 association between
> pci resource and upper interface.
> 
> Signed-off-by: David Marchand 
> Signed-off-by: Shreyansh Jain 
> ---
Reviewed-by: Jan Viktorin 

[dpdk-dev] [PATCH v6 13/17] pci: add a helper to update a device

[Jan Viktorin]

On Tue, 12 Jul 2016 11:31:18 +0530
Shreyansh Jain  wrote:

> This helper updates a pci device object with latest information it can
> find.
> It will be used mainly for hotplug code.
> 
> Signed-off-by: David Marchand 
> Signed-off-by: Shreyansh Jain 
> ---
>  lib/librte_eal/bsdapp/eal/eal_pci.c | 49 
> +
>  lib/librte_eal/common/eal_common_pci.c  |  2 --
>  lib/librte_eal/common/eal_private.h | 13 +
>  lib/librte_eal/common/include/rte_pci.h |  3 ++
>  lib/librte_eal/linuxapp/eal/eal_pci.c   | 13 +
>  5 files changed, 78 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c 
> b/lib/librte_eal/bsdapp/eal/eal_pci.c
> index a73cbb0..1d91c78 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
> @@ -406,6 +406,55 @@ error:
>   return -1;
>  }
>  
> +int
> +pci_update_device(const struct rte_pci_addr *addr)
> +{
> + int fd;
> + struct pci_conf matches[2];
> + struct pci_match_conf match = {
> + .pc_sel = {
> + .pc_domain = addr->domain,
> + .pc_bus = addr->bus,
> + .pc_dev = addr->devid,
> + .pc_func = addr->function,
> + },
> + };
> + struct pci_conf_io conf_io = {
> + .pat_buf_len = 0,
> + .num_patterns = 1,
> + .patterns = ,
> + .match_buf_len = sizeof(matches),
> + .matches = [0],
> + };
> +
> + fd = open("/dev/pci", O_RDONLY);
> + if (fd < 0) {
> + RTE_LOG(ERR, EAL, "%s(): error opening /dev/pci\n", __func__);
> + goto error;
> + }
> +
> + if (ioctl(fd, PCIOCGETCONF, _io) < 0) {
> + RTE_LOG(ERR, EAL, "%s(): error with ioctl on /dev/pci: %s\n",
> + __func__, strerror(errno));
> + goto error;
> + }
> +
> + if (conf_io.num_matches != 1)
> + goto error;
> +
> + if (pci_scan_one(fd, [0]) < 0)
> + goto error;
> +
> + close(fd);
> +
> + return 0;
> +
> +error:
> + if (fd >= 0)
> + close(fd);
> + return -1;
> +}
> +
>  /* Read PCI config space. */
>  int rte_eal_pci_read_config(const struct rte_pci_device *dev,
>   void *buf, size_t len, off_t offset)
> diff --git a/lib/librte_eal/common/eal_common_pci.c 
> b/lib/librte_eal/common/eal_common_pci.c
> index 6a0f6ac..58f0c74 100644
> --- a/lib/librte_eal/common/eal_common_pci.c
> +++ b/lib/librte_eal/common/eal_common_pci.c
> @@ -87,8 +87,6 @@ struct pci_driver_list pci_driver_list =
>  struct pci_device_list pci_device_list =
>   TAILQ_HEAD_INITIALIZER(pci_device_list);
>  
> -#define SYSFS_PCI_DEVICES "/sys/bus/pci/devices"
> -

Is this delete correct? The SYSFS_PCI_DEVICES was moved here from
rte_pci.h. A relict of some rebase?

>  const char *pci_get_sysfs_path(void)
>  {
>   const char *path = NULL;
> diff --git a/lib/librte_eal/common/eal_private.h 
> b/lib/librte_eal/common/eal_private.h
> index 06a68f6..b8ff9c5 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -152,6 +152,19 @@ struct rte_pci_driver;
>  struct rte_pci_device;
>  
>  /**
> + * Update a pci device object by asking the kernel for the latest 
> information.
> + *
> + * This function is private to EAL.
> + *
> + * @param addr
> + *   The PCI Bus-Device-Function address to look for
> + * @return
> + *   - 0 on success.
> + *   - negative on error.
> + */
> +int pci_update_device(const struct rte_pci_addr *addr);
> +
> +/**
>   * Unbind kernel driver for this device
>   *
>   * This function is private to EAL.
> diff --git a/lib/librte_eal/common/include/rte_pci.h 
> b/lib/librte_eal/common/include/rte_pci.h
> index 06508fa..5c2062c 100644
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -107,6 +107,9 @@ const char *pci_get_sysfs_path(void);
>  /** Nb. of values in PCI resource format. */
>  #define PCI_RESOURCE_FMT_NVAL 3
>  
> +/** Default sysfs path for PCI device search. */
> +#define SYSFS_PCI_DEVICES "/sys/bus/pci/devices"
> +

Why adding SYSFS_PCI_DEVICES here? The pci_get_sysfs_path is used for
this purpose. Again, a rebase issue?

>  /**
>   * A structure describing a PCI resource.
>   */
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c 
> b/lib/librte_eal/linuxapp/eal/eal_pci.c
> index f0215ee..8f3ef20 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> @@ -417,6 +417,19 @@ pci_scan_one(const char *dirname, uint16_t domain, 
> uint8_t bus,
>   return 0;
>  }
>  
> +int
> +pci_update_device(const struct rte_pci_addr *addr)
> +{
> + char filename[PATH_MAX];
> +
> + snprintf(filename, sizeof(filename), "%s/" PCI_PRI_FMT,
> +  SYSFS_PCI_DEVICES, addr->domain, addr->bus, addr->devid,

Use pci_get_sysfs_path here.

> + 

[dpdk-dev] [PATCH v6 12/17] pci: add a helper for device name

[Jan Viktorin]

On Tue, 12 Jul 2016 11:31:17 +0530
Shreyansh Jain  wrote:

> eal is a better place than crypto / ethdev for naming resources.

s/for naming/to name/

What is meant by "resources" here?

> Add a helper in eal and make use of it in crypto / ethdev.
> 
> Signed-off-by: David Marchand 
> Signed-off-by: Shreyansh Jain 
> ---
>  lib/librte_cryptodev/rte_cryptodev.c| 27 ---
>  lib/librte_eal/common/include/rte_pci.h | 25 +
>  lib/librte_ether/rte_ethdev.c   | 24 
>  3 files changed, 33 insertions(+), 43 deletions(-)
> 
> diff --git a/lib/librte_cryptodev/rte_cryptodev.c 
> b/lib/librte_cryptodev/rte_cryptodev.c
> index d7be111..60c6384 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.c
> +++ b/lib/librte_cryptodev/rte_cryptodev.c
> @@ -367,23 +367,6 @@ rte_cryptodev_pmd_allocate(const char *name, int 
> socket_id)
>   return cryptodev;
>  }
>  
> -static inline int
> -rte_cryptodev_create_unique_device_name(char *name, size_t size,
> - struct rte_pci_device *pci_dev)
> -{
> - int ret;
> -
> - if ((name == NULL) || (pci_dev == NULL))
> - return -EINVAL;
> -
> - ret = snprintf(name, size, "%d:%d.%d",
> - pci_dev->addr.bus, pci_dev->addr.devid,
> - pci_dev->addr.function);
> - if (ret < 0)
> - return ret;
> - return 0;
> -}
> -
>  int
>  rte_cryptodev_pmd_release_device(struct rte_cryptodev *cryptodev)
>  {
> @@ -446,9 +429,8 @@ rte_cryptodev_pci_probe(struct rte_pci_driver *pci_drv,
>   if (cryptodrv == NULL)
>   return -ENODEV;
>  
> - /* Create unique Crypto device name using PCI address */
> - rte_cryptodev_create_unique_device_name(cryptodev_name,
> - sizeof(cryptodev_name), pci_dev);
> + rte_eal_pci_device_name(_dev->addr, cryptodev_name,
> + sizeof(cryptodev_name));
>  
>   cryptodev = rte_cryptodev_pmd_allocate(cryptodev_name, rte_socket_id());
>   if (cryptodev == NULL)
> @@ -503,9 +485,8 @@ rte_cryptodev_pci_remove(struct rte_pci_device *pci_dev)
>   if (pci_dev == NULL)
>   return -EINVAL;
>  
> - /* Create unique device name using PCI address */
> - rte_cryptodev_create_unique_device_name(cryptodev_name,
> - sizeof(cryptodev_name), pci_dev);
> + rte_eal_pci_device_name(_dev->addr, cryptodev_name,
> + sizeof(cryptodev_name));
>  
>   cryptodev = rte_cryptodev_pmd_get_named_dev(cryptodev_name);
>   if (cryptodev == NULL)
> diff --git a/lib/librte_eal/common/include/rte_pci.h 
> b/lib/librte_eal/common/include/rte_pci.h
> index 3027adf..06508fa 100644
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -82,6 +82,7 @@ extern "C" {
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  
>  TAILQ_HEAD(pci_device_list, rte_pci_device); /**< PCI devices in D-linked Q. 
> */
> @@ -95,6 +96,7 @@ const char *pci_get_sysfs_path(void);
>  
>  /** Formatting string for PCI device identifier: Ex: :00:01.0 */
>  #define PCI_PRI_FMT "%.4" PRIx16 ":%.2" PRIx8 ":%.2" PRIx8 ".%" PRIx8
> +#define PCI_PRI_STR_SIZE sizeof(":XX:XX.X")
>  
>  /** Short formatting string, without domain, for PCI device: Ex: 00:01.0 */
>  #define PCI_SHORT_PRI_FMT "%.2" PRIx8 ":%.2" PRIx8 ".%" PRIx8
> @@ -308,6 +310,29 @@ eal_parse_pci_DomBDF(const char *input, struct 
> rte_pci_addr *dev_addr)
>  }
>  #undef GET_PCIADDR_FIELD
>  
> +/**
> + * Utility function to write a pci device name, this device name can later be
> + * used to retrieve the corresponding rte_pci_addr using above functions.

What about saying "using functions eal_parse_pci_*BDF"? The
specification "above" is quite uncertain...

> + *
> + * @param addr
> + *   The PCI Bus-Device-Function address
> + * @param output
> + *   The output buffer string
> + * @param size
> + *   The output buffer size
> + * @return
> + *  0 on success, negative on error.
> + */
> +static inline void
> +rte_eal_pci_device_name(const struct rte_pci_addr *addr,
> + char *output, size_t size)
> +{
> + RTE_VERIFY(size >= PCI_PRI_STR_SIZE);
> + RTE_VERIFY(snprintf(output, size, PCI_PRI_FMT,
> + addr->domain, addr->bus,
> + addr->devid, addr->function) >= 0);
> +}
> +
>  /* Compare two PCI device addresses. */
>  /**

[...]

[dpdk-dev] [PATCH v6 16/17] ethdev: convert to eal hotplug

[Jan Viktorin]

On Tue, 12 Jul 2016 11:31:21 +0530
Shreyansh Jain  wrote:

> Remove bus logic from ethdev hotplug by using eal for this.
> 
> Current api is preserved:
> - the last port that has been created is tracked to return it to the
>   application when attaching,
> - the internal device name is reused when detaching.
> 
> We can not get rid of ethdev hotplug yet since we still need some mechanism
> to inform applications of port creation/removal to substitute for ethdev
> hotplug api.
> 
> dev_type field in struct rte_eth_dev and rte_eth_dev_allocate are kept as
> is, but this information is not needed anymore and is removed in the following
> commit.
> 
> Signed-off-by: David Marchand 
> Signed-off-by: Shreyansh Jain 
> ---
>  lib/librte_ether/rte_ethdev.c | 207 
> +++---
>  1 file changed, 33 insertions(+), 174 deletions(-)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index a667012..8d14fd7 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -72,6 +72,7 @@
>  static const char *MZ_RTE_ETH_DEV_DATA = "rte_eth_dev_data";
>  struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS];
>  static struct rte_eth_dev_data *rte_eth_dev_data;
> +static uint8_t eth_dev_last_created_port;
>  static uint8_t nb_ports;
>  

[...]

> -
>  /* attach the new device, then store port_id of the device */
>  int
>  rte_eth_dev_attach(const char *devargs, uint8_t *port_id)
>  {
> - struct rte_pci_addr addr;
>   int ret = -1;
> + int current = eth_dev_last_created_port;
> + char *name = NULL;
> + char *args = NULL;
>  
>   if ((devargs == NULL) || (port_id == NULL)) {
>   ret = -EINVAL;
>   goto err;
>   }
>  
> - if (eal_parse_pci_DomBDF(devargs, ) == 0) {
> - ret = rte_eth_dev_attach_pdev(, port_id);
> - if (ret < 0)
> - goto err;
> - } else {
> - ret = rte_eth_dev_attach_vdev(devargs, port_id);
> - if (ret < 0)
> - goto err;
> + /* parse devargs, then retrieve device name and args */
> + if (rte_eal_parse_devargs_str(devargs, , ))
> + goto err;
> +
> + ret = rte_eal_dev_attach(name, args);
> + if (ret < 0)
> + goto err;
> +
> + /* no point looking at eth_dev_last_created_port if no port exists */

I am not sure about this comment. What is "no point"?

Isn't this also a potential bug? (Like the one below.) How could it
happen there is no port after a successful attach?

> + if (!nb_ports) {
> + ret = -1;
> + goto err;
> + }
> + /* if nothing happened, there is a bug here, since some driver told us
> +  * it did attach a device, but did not create a port */
> + if (current == eth_dev_last_created_port) {
> + ret = -1;
> + goto err;

Should we log this? Or call some kind panic?

>   }
> + *port_id = eth_dev_last_created_port;
> + ret = 0;
>  
> - return 0;
>  err:
> - RTE_LOG(ERR, EAL, "Driver, cannot attach the device\n");
> + free(name);
> + free(args);
>   return ret;
>  }
>  
> @@ -590,7 +464,6 @@ err:
>  int
>  rte_eth_dev_detach(uint8_t port_id, char *name)
>  {
> - struct rte_pci_addr addr;
>   int ret = -1;
>  
>   if (name == NULL) {
> @@ -598,33 +471,19 @@ rte_eth_dev_detach(uint8_t port_id, char *name)
>   goto err;
>   }
>  
> - /* check whether the driver supports detach feature, or not */
> + /* FIXME: move this to eal, once device flags are relocated there */
>   if (rte_eth_dev_is_detachable(port_id))
>   goto err;
>  
> - if (rte_eth_dev_get_device_type(port_id) == RTE_ETH_DEV_PCI) {
> - ret = rte_eth_dev_get_addr_by_port(port_id, );
> - if (ret < 0)
> - goto err;
> -
> - ret = rte_eth_dev_detach_pdev(port_id, );
> - if (ret < 0)
> - goto err;
> -
> - snprintf(name, RTE_ETH_NAME_MAX_LEN,
> - "%04x:%02x:%02x.%d",
> - addr.domain, addr.bus,
> - addr.devid, addr.function);
> - } else {
> - ret = rte_eth_dev_detach_vdev(port_id, name);
> - if (ret < 0)
> - goto err;
> - }
> + snprintf(name, sizeof(rte_eth_devices[port_id].data->name),
> +  "%s", rte_eth_devices[port_id].data->name);
> + ret = rte_eal_dev_detach(name);
> + if (ret < 0)
> + goto err;
>  
>   return 0;
>  
>  err:
> - RTE_LOG(ERR, EAL, "Driver, cannot detach the device\n");

I'd be more specific about the failing device, we have its name.

>   return ret;
>  }
> 

[dpdk-dev] [PATCH v6 15/17] eal: add hotplug operations for pci and vdev

[Jan Viktorin]

On Tue, 12 Jul 2016 11:31:20 +0530
Shreyansh Jain  wrote:

> Hotplug which deals with resources should come from the layer that already
> handles them, i.e. EAL.
> 
> For both attach and detach operations, 'name' is used to select the bus
> that will handle the request.
> 
> Signed-off-by: David Marchand 
> Signed-off-by: Shreyansh Jain 
> ---
>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  2 ++
>  lib/librte_eal/common/eal_common_dev.c  | 47 
> +
>  lib/librte_eal/common/include/rte_dev.h | 25 +
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  2 ++
>  4 files changed, 76 insertions(+)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map 
> b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index 1852c4a..6f9324f 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -159,5 +159,7 @@ DPDK_16.07 {
>   rte_keepalive_mark_sleep;
>   rte_keepalive_register_relay_callback;
>   rte_thread_setname;
> + rte_eal_dev_attach;
> + rte_eal_dev_detach;
>  
>  } DPDK_16.04;
> diff --git a/lib/librte_eal/common/eal_common_dev.c 
> b/lib/librte_eal/common/eal_common_dev.c
> index a8a4146..14c6cf1 100644
> --- a/lib/librte_eal/common/eal_common_dev.c
> +++ b/lib/librte_eal/common/eal_common_dev.c
> @@ -150,3 +150,50 @@ rte_eal_vdev_uninit(const char *name)
>   RTE_LOG(ERR, EAL, "no driver found for %s\n", name);
>   return -EINVAL;
>  }
> +
> +int rte_eal_dev_attach(const char *name, const char *devargs)
> +{
> + struct rte_pci_addr addr;
> + int ret = -1;
> +
> + if (name == NULL || devargs == NULL) {
> + RTE_LOG(ERR, EAL, "Invalid device arguments provided\n");
> + return ret;
> + }
> +
> + if (eal_parse_pci_DomBDF(name, ) == 0) {
> + if (rte_eal_pci_probe_one() < 0)
> + goto err;
> +
> + } else {
> + if (rte_eal_vdev_init(name, devargs))
> + goto err;
> + }
> +
> + return 0;
> +
> +err:
> + RTE_LOG(ERR, EAL, "Driver cannot attach the device\n");

I think this log can be more specific. We have the name of the device.
It might be confusing to the user when the attach fails and there is
no simple way how to determine which one.

> + return ret;
> +}
> +
> +int rte_eal_dev_detach(const char *name)
> +{
> + struct rte_pci_addr addr;
> +
> + if (name == NULL)
> + goto err;
> +
> + if (eal_parse_pci_DomBDF(name, ) == 0) {
> + if (rte_eal_pci_detach() < 0)
> + goto err;
> + } else {
> + if (rte_eal_vdev_uninit(name))
> + goto err;
> + }
> + return 0;
> +
> +err:
> + RTE_LOG(ERR, EAL, "Driver cannot detach the device\n");

Same as for attach.

> + return -1;
> +}
> diff --git a/lib/librte_eal/common/include/rte_dev.h 
> b/lib/librte_eal/common/include/rte_dev.h
> index 994650b..2f0579c 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -178,6 +178,31 @@ int rte_eal_vdev_init(const char *name, const char 
> *args);
>   */
>  int rte_eal_vdev_uninit(const char *name);
>  
> +/**
> + * Attach a resource to a registered driver.

[dpdk-dev] [PATCH 1/2] vfio: fix pci_vfio_map_resource

[Dan Aloni]

On Thu, Jul 14, 2016 at 02:50:51PM +, Burakov, Anatoly wrote:
> > Someone to review this patch please?
> > It can be integrated in RC3 if we are sure it doesn't break anything.
> > 
> > 2016-07-07 15:26, Yong Wang:
> > > The offset of the 2nd mmap when mapping the region after msix_bar
> > > needs to take region address into consideration.  This is exposed when
> > > using vfio-pci to manage vmxnet3 pmd.
> > >
> > > Fixes: 90a1633b2347 ("eal/linux: allow to map BARs with MSI-X tables")
> > >
> > > Signed-off-by: Yong Wang 
> > > Signed-off-by: Ronghua Zhang 
> 
> Hi Thomas,
> 
> The patch makes sense to me, but I don't have any devices that trigger that 
> particular codepath in my immediate vicinity. The original patch was 
> mentioning an NVMe adapter, so that probably should be tested with this 
> change. I'm CC'ing the original author of that patch just in case. Do we know 
> of any other NICs/devices that might be affected by this? Aside from the 
> obvious example of vmxnet3... 

The patch makes sense. To explain further, the reg.offset is
resource-relative and not bar-relative like table_start and table_end.
The mmap() needs to be resource-relative.

I think it would be cleaner to use 'reg.offset' to substruct,
conveying the fact that bar_addr already accounts for it.

-   void *second_addr = RTE_PTR_ADD(bar_addr, 
memreg[1].offset);
+   void *second_addr = RTE_PTR_ADD(bar_addr,
+   
memreg[1].offset - reg.offset);

The work I've done with the Intel NVMe adapter was cut short due to
other tasks I've had to shift to. But it was left in a state where it
almost worked under VFIO, in a sense that the NVME card entered a
fault state and I had not invested enough time to figure out why.

If it's due to this bug, then I would be able to finally close this
one. But that's a bit hopeful, though. Happy to find actual users of
the changes.

-- 
Dan Aloni

[dpdk-dev] [PATCH v6 17/17] ethdev: get rid of device type

[Jan Viktorin]

On Tue, 12 Jul 2016 11:31:22 +0530
Shreyansh Jain  wrote:

> Now that hotplug has been moved to eal, there is no reason to keep the device
> type in this layer.
> 
> Signed-off-by: David Marchand 
> Signed-off-by: Shreyansh Jain 
> ---
>  app/test/virtual_pmd.c|  2 +-
>  drivers/net/af_packet/rte_eth_af_packet.c |  2 +-
>  drivers/net/bonding/rte_eth_bond_api.c|  2 +-
>  drivers/net/cxgbe/cxgbe_main.c|  2 +-
>  drivers/net/mlx4/mlx4.c   |  2 +-
>  drivers/net/mlx5/mlx5.c   |  2 +-
>  drivers/net/mpipe/mpipe_tilegx.c  |  2 +-
>  drivers/net/null/rte_eth_null.c   |  2 +-
>  drivers/net/pcap/rte_eth_pcap.c   |  2 +-
>  drivers/net/ring/rte_eth_ring.c   |  2 +-
>  drivers/net/vhost/rte_eth_vhost.c |  2 +-
>  drivers/net/virtio/virtio_user_ethdev.c   |  2 +-
>  drivers/net/xenvirt/rte_eth_xenvirt.c |  2 +-
>  examples/ip_pipeline/init.c   | 22 --
>  lib/librte_ether/rte_ethdev.c |  5 ++---
>  lib/librte_ether/rte_ethdev.h | 15 +--
>  16 files changed, 16 insertions(+), 52 deletions(-)
> 

[...]

>  
> -static int
> -app_link_is_virtual(struct app_link_params *p)
> -{
> - uint32_t pmd_id = p->pmd_id;
> - struct rte_eth_dev *dev = _eth_devices[pmd_id];
> -
> - if (dev->dev_type == RTE_ETH_DEV_VIRTUAL)
> - return 1;
> -
> - return 0;
> -}
> -
>  void
>  app_link_up_internal(struct app_params *app, struct app_link_params *cp)
>  {
>   uint32_t i;
>   int status;
>  
> - if (app_link_is_virtual(cp)) {
> - cp->state = 1;
> - return;
> - }
> -
>   /* For each link, add filters for IP of current link */
>   if (cp->ip != 0) {
>   for (i = 0; i < app->n_links; i++) {
> @@ -736,11 +719,6 @@ app_link_down_internal(struct app_params *app, struct 
> app_link_params *cp)
>   uint32_t i;
>   int status;
>  
> - if (app_link_is_virtual(cp)) {
> - cp->state = 0;
> - return;
> - }
> -
>   /* PMD link down */

I understand that app_link_is_virtual is useless. However, it changes
semantics. What is the behaviour of the app_link_up_internal after this
change?

>   status = rte_eth_dev_set_link_down(cp->pmd_id);
>   if (status < 0)

[...]

[dpdk-dev] net/virtio-user: Fix missing brackets in if condition

[vikto...@rehivetech.com]

On Tue, 12 Jul 2016 11:30:25 +0200
Maxime Coquelin  wrote:

> The error is reported using test build script:

I recommend to note that the error is reported only by GCC 6+.

> 
> $ scripts/test-build.sh x86_64-native-linuxapp-gcc
> ...
> drivers/net/virtio/virtio_user_ethdev.c: In function 
> ?virtio_user_pmd_devinit?:
> drivers/net/virtio/virtio_user_ethdev.c:345:2: error: this ?if? clause does 
> not guard... [-Werror=misleading-indentation]
>   if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PATH) == 1)
> ^~
> 
> Fixes: 404bd6bfe360 ("net/virtio-user: fix return value not checked")
> 
> Cc: Jianfeng Tan 
> Signed-off-by: Maxime Coquelin 
> Acked-by: Yuanhan Liu 
> 
Reviewed-by: Jan Viktorin 

[dpdk-dev] spinlock: Move constructor function out of header file

[Damjan Marion (damarion)]

Dear Jan,

Thank you for your comments. A bit too much overhead to submit simple patch
so let?s forget about it. I will just add it as it is to our private
collection of patches.

If anybody wants to pick it from here, please do...

Thanks,

Damjan


> On 14 Jul 2016, at 20:03, Jan Viktorin  wrote:
> 
> Hello Damjan,
> 
> thank you for the patch. It makes sense to me.
> 
> Next time, please CC the appropriate maintainers.
> (See the MAINTAINERS file in the root of the DPDK source tree.)
> 
> In the subject after "spinlock:" you should start with a lower case
> letter, so "move constructor..."
> 
> On Thu, 14 Jul 2016 15:27:29 +0200
> damarion at cisco.com wrote:
> 
>> From: Damjan Marion 
>> 
>> Having constructor function in the header gile is generaly
> 
> I'd write:
> 
> Having constructor functions in header files is generally a bad idea.
> 
> Anyway:
> 
> s/gile/file/
> 
>> bad idea, as it will eventually be implanted to 3rd party
>> library.
>> 
>> In this case it is causing linking issues with 3rd party
> 
> it causes linking issues
> 
>> libraries when application is not linked to dpdk, due to missing
> 
> an application
> 
> to dpdk due to a missing gymbol (no comma)
> 
>> symbol called by constructor.
> 
> Please include the following line:
> 
> Fixes: ba7468997ea6 ("spinlock: add HTM lock elision for x86")
> 
>> 
>> Signed-off-by: Damjan Marion 
>> 
>> ---
>> lib/librte_eal/common/arch/x86/rte_spinlock.c  | 45 
>> ++
>> .../common/include/arch/x86/rte_spinlock.h | 13 ++-
>> lib/librte_eal/linuxapp/eal/Makefile   |  1 +
>> 3 files changed, 49 insertions(+), 10 deletions(-)
>> create mode 100644 lib/librte_eal/common/arch/x86/rte_spinlock.c
>> 
>> diff --git a/lib/librte_eal/common/arch/x86/rte_spinlock.c 
>> b/lib/librte_eal/common/arch/x86/rte_spinlock.c
>> new file mode 100644
>> index 000..ad8cc5a
>> --- /dev/null
>> +++ b/lib/librte_eal/common/arch/x86/rte_spinlock.c
>> @@ -0,0 +1,45 @@
>> +/*-
>> + *   BSD LICENSE
>> + *
>> + *   Copyright(c) 2010-2014 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_cpuflags.h"
>> +
>> +#include 
> 
> According to:
> http://dpdk.org/doc/guides-16.04/contributing/coding_style.html#coding-style
> 
> you should change the order of these includes.
> 
>> +
>> +uint8_t rte_rtm_supported; /* cache the flag to avoid the overhead
>> +  of the rte_cpu_get_flag_enabled function */
> 
> The comment should be placed before the declaration or use the /**< */
> Doxygen style. I'd prefer to placed it before. Can you fix it with this
> patch?
> 
>> +
>> +static void __attribute__((constructor))
>> +rte_rtm_init(void)
>> +{
>> +rte_rtm_supported = rte_cpu_get_flag_enabled(RTE_CPUFLAG_RTM);
>> +}
>> diff --git a/lib/librte_eal/common/include/arch/x86/rte_spinlock.h 
>> b/lib/librte_eal/common/include/arch/x86/rte_spinlock.h
>> index 02f95cb..8e630c2 100644
>> --- a/lib/librte_eal/common/include/arch/x86/rte_spinlock.h
>> +++ b/lib/librte_eal/common/include/arch/x86/rte_spinlock.h
>> @@ -94,24 +94,17 @@ rte_spinlock_trylock (rte_spinlock_t *sl)
>> }
>> #endif
>> 
>> -static uint8_t rtm_supported; /* cache the flag to avoid the overhead
>> - of the rte_cpu_get_flag_enabled 

[dpdk-dev] [PATCH v6 05/17] eal: introduce init macros

[Jan Viktorin]

On Thu, 14 Jul 2016 10:57:55 +0530
Shreyansh jain  wrote:

> Hi Jan,
> 
> On Wednesday 13 July 2016 11:04 PM, Jan Viktorin wrote:
> > On Wed, 13 Jul 2016 11:20:43 +0200
> > Jan Viktorin  wrote:
> >   
> >> Hello Shreyansh,
> >>
> >> On Tue, 12 Jul 2016 11:31:10 +0530
> >> Shreyansh Jain  wrote:
> >>  
> >>> Introduce a RTE_INIT macro used to mark an init function as a constructor.
> >>> Current eal macros have been converted to use this (no functional impact).
> >>> DRIVER_REGISTER_PCI is added as a helper for pci drivers.
> >>>
> >>> Suggested-by: Jan Viktorin 
> >>> Signed-off-by: David Marchand 
> >>> Signed-off-by: Shreyansh Jain 
> >>> ---
> >>
> >> [...]
> >>  
> >>> +#define RTE_INIT(func) \
> >>> +static void __attribute__((constructor, used)) func(void)
> >>> +
> >>>  #ifdef __cplusplus
> >>>  }
> >>>  #endif
> >>> diff --git a/lib/librte_eal/common/include/rte_pci.h 
> >>> b/lib/librte_eal/common/include/rte_pci.h
> >>> index fa74962..3027adf 100644
> >>> --- a/lib/librte_eal/common/include/rte_pci.h
> >>> +++ b/lib/librte_eal/common/include/rte_pci.h
> >>> @@ -470,6 +470,14 @@ void rte_eal_pci_dump(FILE *f);
> >>>   */
> >>>  void rte_eal_pci_register(struct rte_pci_driver *driver);
> >>>  
> >>> +/** Helper for PCI device registeration from driver (eth, crypto) 
> >>> instance */
> >>> +#define DRIVER_REGISTER_PCI(nm, drv) \
> >>> +RTE_INIT(pciinitfn_ ##nm); \
> >>> +static void pciinitfn_ ##nm(void) \
> >>> +{ \
> >>
> >> You are missing setting the name here like PMD_REGISTER_DRIVER does
> >> now. Or should I include it in my patch set?
> >>
> >>(drv).name = RTE_STR(nm);  
> 
> That is a miss from my side.
> I will publish v7 with this. You want this right away or should I wait a 
> little while (more reviews, or any pending additions as per Thomas's notes) 
> before publishing?

Please. The time is almost gone. 18/7/2016 is the release (according
to the roadmap)... I have to fix it in my patchset, otherwise it
does not build (after moving the .name from rte_pci_driver to
rte_driver).

> 
> > 
> > Moreover, it should accept the rte_pci_driver *, shouldn't it? Here, it
> > expects a wrapper around it (eth_driver)... I now, my SoC patches were
> > supposing the some... but I think it is wrong.
> > 
> > The original David's patch set contains calls like this:
> > 
> >   RTE_EAL_PCI_REGISTER(bnx2xvf, rte_bnx2xvf_pmd.pci_drv);
> > 
> > So, I think, we should go the original way.  
> 
> I have a slightly different opinion of the above.
> IMO, aim of the helpers is to hide the PCI details and continue to make 
> driver consider itself as a generic ETH driver. In that case, dereferencing 
> pci_drv would be done by macro.

In this case, I'd prefer to see DRIVER_REGISTER_PCI_ETH.

At first, this was also my way of thinking. But I've changed my mind. I
find it to be a bit overdesigned.

> 
> Also, considering that in future pci_drv would also have soc_drv, the helpers 
> can effectively hide the intra-structure naming of these. It would help when 
> more such device types (would there be?) are introduced - in which case, 
> driver framework has a consistent coding convention.

Hide? I am afraid, I don't understand clearly what you mean.

> 
> But, I am ok switching back to David's way as well - I don't have any strong 
> argument against that.

I'd like to preserve the clear semantics. That is DRIVER_REGISTER_PCI
-> give a pci device.

Has anybody a different opinion? David? Thomas?

> 
> > 
> > Jan
> >   
> >>  
> >>> + rte_eal_pci_register(_drv); \
> >>> +}
> >>> +
> >>>  /**
> >>>   * Unregister a PCI driver.
> >>>   *  
> [...]
> 
> -
> Shreyansh
> 



-- 
  Jan ViktorinE-mail: Viktorin at RehiveTech.com
  System ArchitectWeb:www.RehiveTech.com
  RehiveTech
  Brno, Czech Republic

[dpdk-dev] [PATCH] eal: fix rte_intr_dp_is_en() check

[Yong Wang]

When binding a device to igb_uio with intr_conf.rxq set to 1, nb_efd
is 1 (for link event) but rte_intr_dp_is_en() will still return true.
rte_intr_dp_is_en() should also consider intr_handle type in addition
to nb_efd.

Signed-off-by: Yong Wang 
---
 lib/librte_eal/linuxapp/eal/eal_interrupts.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c 
b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 47a3b20..71f63e9 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -1200,7 +1200,8 @@ rte_intr_efd_disable(struct rte_intr_handle *intr_handle)
 int
 rte_intr_dp_is_en(struct rte_intr_handle *intr_handle)
 {
-   return !(!intr_handle->nb_efd);
+   return (!(!intr_handle->nb_efd) &&
+   (intr_handle->type == RTE_INTR_HANDLE_VFIO_MSIX));
 }

 int
-- 
1.9.1

[dpdk-dev] [PATCH v2] vfio: fix pci_vfio_map_resource

[Yong Wang]

The offset of the 2nd mmap() when mapping the region after msix_bar
needs to take region address into consideration as mmap() takes
address that is resource-relative instead of bar-relative.  This is
exposed when binding vmxnet3 to vfio-pci.

Fixes: 90a1633b2347 ("eal/linux: allow to map BARs with MSI-X tables")

Signed-off-by: Yong Wang 
Signed-off-by: Ronghua Zhang 
---
v2:
* Addressed review comment from Dan

 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c 
b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 46cd683..bf7cf61 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -431,7 +431,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
} else {
memreg[0].offset = reg.offset;
memreg[0].size = table_start;
-   memreg[1].offset = table_end;
+   memreg[1].offset = reg.offset + table_end;
memreg[1].size = reg.size - table_end;

RTE_LOG(DEBUG, EAL,
@@ -474,7 +474,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
/* if there's a second part, try to map it */
if (map_addr != MAP_FAILED
&& memreg[1].offset && memreg[1].size) {
-   void *second_addr = RTE_PTR_ADD(bar_addr, 
memreg[1].offset);
+   void *second_addr = RTE_PTR_ADD(bar_addr,
+   
memreg[1].offset - reg.offset);
map_addr = pci_map_resource(second_addr,
vfio_dev_fd, 
memreg[1].offset,
memreg[1].size,
-- 
1.9.1

[dpdk-dev] [PATCH v2 5/5] test: change lpm routes file from header to data file

[Bruce Richardson]

Change the file extension of the test_lpm_routes file from .h to .dat.
This makes the lines-of-code counts for DPDK more realistic as they are not
affected by the huge counts from the lpm data.

Signed-off-by: Bruce Richardson 
---
 app/test/Makefile   | 2 +-
 app/test/{test_lpm_routes.h => test_lpm_routes.dat} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename app/test/{test_lpm_routes.h => test_lpm_routes.dat} (100%)

diff --git a/app/test/Makefile b/app/test/Makefile
index a147db4..cc8c205 100644
--- a/app/test/Makefile
+++ b/app/test/Makefile
@@ -127,7 +127,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_functions.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_scaling.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_multiwriter.c

-$(eval $(call linked_resource,test_lpm_data,test_lpm_routes.h))
+$(eval $(call linked_resource,test_lpm_data,test_lpm_routes.dat))
 SRCS-$(CONFIG_RTE_LIBRTE_LPM) += test_lpm.c
 SRCS-$(CONFIG_RTE_LIBRTE_LPM) += test_lpm_perf.c
 SRCS-$(CONFIG_RTE_LIBRTE_LPM) += test_lpm6.c
diff --git a/app/test/test_lpm_routes.h b/app/test/test_lpm_routes.dat
similarity index 100%
rename from app/test/test_lpm_routes.h
rename to app/test/test_lpm_routes.dat
-- 
2.5.5

[dpdk-dev] [PATCH v2 4/5] test: change lpm test to use routes as resource

[Bruce Richardson]

Change the lpm autotest to use the routes data from the resource data
stored in the binary rather than including it directly into the C file
as a C header. This speeds up compile and link time, without changing
the test results.

Signed-off-by: Bruce Richardson 
---
 app/test/test_lpm_perf.c | 75 +++-
 1 file changed, 74 insertions(+), 1 deletion(-)

diff --git a/app/test/test_lpm_perf.c b/app/test/test_lpm_perf.c
index 0e64919..ad70b02 100644
--- a/app/test/test_lpm_perf.c
+++ b/app/test/test_lpm_perf.c
@@ -34,17 +34,25 @@
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
+#include 
 #include 
 #include 

 #include "test.h"
 #include "resource.h"
-#include "test_lpm_routes.h"
 #include "test_xmmt_ops.h"

+struct route_rule {
+   uint32_t ip;
+   uint8_t depth;
+};
+static struct route_rule *large_route_table;
+static unsigned int NUM_ROUTE_ENTRIES;
+
 REGISTER_LINKED_RESOURCE(test_lpm_data)

 #define TEST_LPM_ASSERT(cond) do {\
@@ -83,6 +91,68 @@ print_route_distribution(const struct route_rule *table, 
uint32_t n)
 }

 static int
+load_large_route_table(void)
+{
+   const struct resource *r;
+   const char *lpm_data;
+
+   r = resource_find("test_lpm_data");
+   TEST_ASSERT_NOT_NULL(r, "No large lpm table data found");
+
+   /* the routing table size is going to be less than the size of the
+* resource, since text extries are more verbose. Allocate this as
+* the max size, and shrink the allocation later
+*/
+   large_route_table = rte_malloc(NULL, resource_size(r), 0);
+   if (large_route_table == NULL)
+   return -1;
+
+   /* parse the lpm table. All entries are of format:
+*  {IP-as-decimal-unsigned, depth}
+* For example:
+*  {1234567U, 24},
+* We use the "U" and "}" characters as format check characters,
+* after parsing each number.
+*/
+   for (lpm_data = r->begin; lpm_data < r->end; lpm_data++) {
+   if (*lpm_data == '{') {
+   char *endptr;
+
+   lpm_data++;
+   large_route_table[NUM_ROUTE_ENTRIES].ip = \
+   strtoul(lpm_data, , 0);
+   if (*endptr != 'U') {
+   if (NUM_ROUTE_ENTRIES > 0) {
+   char *value = strndup(lpm_data, 12);
+   printf("Failed parse of %s\n", value);
+   free(value);
+   }
+   continue;
+   }
+
+   lpm_data = endptr + 2; /* skip U and , */
+   large_route_table[NUM_ROUTE_ENTRIES].depth = \
+   strtoul(lpm_data, , 0);
+   if (*endptr != '}') {
+   if (NUM_ROUTE_ENTRIES > 0) {
+   char *value = strndup(lpm_data, 5);
+   printf("Failed parse of %s\n", value);
+   free(value);
+   }
+   continue;
+   }
+
+   NUM_ROUTE_ENTRIES++;
+   }
+   }
+
+   large_route_table = rte_realloc(large_route_table,
+   sizeof(large_route_table[0]) * NUM_ROUTE_ENTRIES, 0);
+   printf("Read %u route entries\n", NUM_ROUTE_ENTRIES);
+   return 0;
+}
+
+static int
 test_lpm_perf(void)
 {
struct rte_lpm *lpm = NULL;
@@ -98,6 +168,9 @@ test_lpm_perf(void)
uint64_t cache_line_counter = 0;
int64_t count = 0;

+   TEST_ASSERT_SUCCESS(load_large_route_table(),
+   "Error loading lpm table");
+
rte_srand(rte_rdtsc());

printf("No. routes = %u\n", (unsigned) NUM_ROUTE_ENTRIES);
-- 
2.5.5

[dpdk-dev] [PATCH v2 3/5] test: add lpm routes as a linked resource

[Bruce Richardson]

Since we now have the ability to store resource data directly inside
the test binary, take the test_lpm_routes.h header file and make it a
test resource. Later commits will then switch the test C file over to use
this rather than including it directly.

Signed-off-by: Bruce Richardson 
---
 app/test/Makefile| 1 +
 app/test/test_lpm_perf.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/app/test/Makefile b/app/test/Makefile
index 2de8c7a..a147db4 100644
--- a/app/test/Makefile
+++ b/app/test/Makefile
@@ -127,6 +127,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_functions.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_scaling.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_multiwriter.c

+$(eval $(call linked_resource,test_lpm_data,test_lpm_routes.h))
 SRCS-$(CONFIG_RTE_LIBRTE_LPM) += test_lpm.c
 SRCS-$(CONFIG_RTE_LIBRTE_LPM) += test_lpm_perf.c
 SRCS-$(CONFIG_RTE_LIBRTE_LPM) += test_lpm6.c
diff --git a/app/test/test_lpm_perf.c b/app/test/test_lpm_perf.c
index 41da811..0e64919 100644
--- a/app/test/test_lpm_perf.c
+++ b/app/test/test_lpm_perf.c
@@ -41,9 +41,12 @@
 #include 

 #include "test.h"
+#include "resource.h"
 #include "test_lpm_routes.h"
 #include "test_xmmt_ops.h"

+REGISTER_LINKED_RESOURCE(test_lpm_data)
+
 #define TEST_LPM_ASSERT(cond) do {\
if (!(cond)) {\
printf("Error at line %d: \n", __LINE__); \
-- 
2.5.5

[dpdk-dev] [PATCH v2 2/5] test: make all lpm routes be of unsigned type

[Bruce Richardson]

The one route that was different to the others in the test_lpm_routes.h
file was the entry "{0, 8}" which was the only route without a "U" after
the IP part. Add in the extra "U" to that entry so that it can be used
as a check character when parsing routes manually.

Signed-off-by: Bruce Richardson 
---
 app/test/test_lpm_routes.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/test_lpm_routes.h b/app/test/test_lpm_routes.h
index 023b0f9..1decfee 100644
--- a/app/test/test_lpm_routes.h
+++ b/app/test/test_lpm_routes.h
@@ -281825,7 +281825,7 @@ static const struct route_rule large_route_table[] =
{3536977920U, 17},
{3392992768U, 23},
{3341675008U, 23},
-   {0, 8},
+   {0U, 8},
{3326316168U, 30},
{3326316108U, 30},
{3326316060U, 30},
-- 
2.5.5

[dpdk-dev] [PATCH v2 1/5] test: fix unneeded routes table include from lpm test

[Bruce Richardson]

The large routing table header file in only needed by the performance
tests and not by the functional tests to remove the include of
test_lpm_routes.h from test_lpm.c.

Fixes: 62dac65a3265 ("app/test: isolate lpm performance cases")

Signed-off-by: Bruce Richardson 
---
 app/test/test_lpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/test_lpm.c b/app/test/test_lpm.c
index f6930fb..8d1bae9 100644
--- a/app/test/test_lpm.c
+++ b/app/test/test_lpm.c
@@ -36,9 +36,9 @@
 #include 

 #include 
+#include 

 #include "test.h"
-#include "test_lpm_routes.h"
 #include "test_xmmt_ops.h"

 #define TEST_LPM_ASSERT(cond) do {\
-- 
2.5.5

[dpdk-dev] [PATCH v2 0/5] change lpm route table from code to data

[Bruce Richardson]

This patchset takes the existing lpm large routing table information and
converts it from a header file included at compile time to a resource linked
in. This improves things in two ways:
1. Improves DPDK build time
2. Removes approx 1 million lines of code from our LOC counts, as the header
   file no longer counts as code, but more correctly as data.

Before and after approx code stats (with some additional patchsets applied)
are given below.  Notice how the app folder has dropped from being the biggest
component with 6x the drivers code to second place with less code than the
drivers.

Future work is to repeat the same process for lpm6. Additional work for both
lpm and lpm6 is to remove the routing tables entirely and replace them with
auto-generated data that gives the roughly the same results. That would
shrink the overall size of DPDK releases, and is the desired end state.

---

This patchset is based directly on the original RFC submitted:
http://www.dpdk.org/ml/archives/dev/2016-May/038478.html.

Updates in this version compared to RFC:
* rebased on dpdk.org HEAD. The changes are now mostly to test_lpm_perf.c
  rather than test_lpm.c
* removed use of strndupa() as suggest by Nikita Koslov

---

## Before:

SLOCDirectory   SLOC-by-Language (Sorted)
1237464 app ansic=1236324,python=1140
192107  drivers ansic=192107
108109  lib ansic=107968,python=141
65447   examplesansic=65312,sh=135
849 tools   python=444,sh=405
799 scripts sh=799
77  doc python=75,sh=2
0   config  (none)
0   mk  (none)
0   pkg (none)
0   top_dir (none)


Totals grouped by language (dominant language first):
ansic:  1601711 (99.80%)
python:1800 (0.11%)
sh:1341 (0.08%)

---

## After

SLOCDirectory   SLOC-by-Language (Sorted)
192107  drivers ansic=192107
160646  app ansic=159506,python=1140
108109  lib ansic=107968,python=141
65447   examplesansic=65312,sh=135
849 tools   python=444,sh=405
799 scripts sh=799
77  doc python=75,sh=2
0   config  (none)
0   mk  (none)
0   pkg (none)
0   top_dir (none)


Totals grouped by language (dominant language first):
ansic:   524893 (99.41%)
python:1800 (0.34%)
sh:1341 (0.25%)

generated using David A. Wheeler's 'SLOCCount'

Bruce Richardson (5):
  test: fix unneeded routes table include from lpm test
  test: make all lpm routes be of unsigned type
  test: add lpm routes as a linked resource
  test: change lpm test to use routes as resource
  test: change lpm routes file from header to data file

 app/test/Makefile  |  1 +
 app/test/test_lpm.c|  2 +-
 app/test/test_lpm_perf.c   | 78 +-
 .../{test_lpm_routes.h => test_lpm_routes.dat} |  2 +-
 4 files changed, 80 insertions(+), 3 deletions(-)
 rename app/test/{test_lpm_routes.h => test_lpm_routes.dat} (99%)

-- 
2.5.5

[dpdk-dev] [PATCH] doc: update release notes

[Thomas Monjalon]

2016-07-14 13:45, Iremonger, Bernard:
> > 2016-07-01 15:10, Bernard Iremonger:
> > > add release note for live migration of a VM with SRIOV VF
> > [...]
> > > +* **Added support for live migration of a VM with SRIOV VF.**
> > > +
> > > +  Live migration of a VM with Virtio and VF PMD's using the bonding PMD.
> > 
> > Is there some new code for this feature?
> > I am not sure we need to update the release notes for a doc addition.
> 
> The testpmd patches which I submitted and were applied, were made to enable 
> testing of this feature.
> This doc patch is to announce that this feature is available now.

The feature was not available before?

[dpdk-dev] rte_ether: Driver-specific stats getting overwritten

[Igor Ryzhov]

Hello.

How about deleting rx_nombuf from rte_eth_stats?
Do you think this counter is necessary? It just shows enormous numbers in case 
of a lack of processing speed.
But we already have imissed counter which shows real number of packets, dropped 
for the same reason.

> 14  2016 ?., ? 16:37, Thomas Monjalon  
> ???(?):
> 
> 2016-07-14 14:29, Remy Horton:
>> 'noon,
>> 
>> In rte_eth_stats_get() after doing the driver callout to populate struct 
>> rte_eth_stats, the rx_nombuf member is overwritten with 
>> dev->data->rx_mbuf_alloc_failed even though some drivers will have 
>> filled rx_nombuf with a value from elsewhere. This makes assignment of 
>> rx_nombuf from within the driver callout redundant. Is this intentional?
> 
> Yes it is strange and has always been like that.
> Why not moving the assignment before calling the driver callback?

[dpdk-dev] rte_ether: Driver-specific stats getting overwritten

[Remy Horton]

On 14/07/2016 14:51, Igor Ryzhov wrote:
[..]
> How about deleting rx_nombuf from rte_eth_stats? Do you think this
> counter is necessary? It just shows enormous numbers in case of a
> lack of processing speed. But we already have imissed counter which
> shows real number of packets, dropped for the same reason.

Deleting it has API/ABI breakage issues. There is also lack of 
consistency between drivers as to what imissed includes, as some don't 
implement it at all whereas others include filtered packets as well.


>> 14  2016 ?., ? 16:37, Thomas Monjalon
>>  ???(?):
>>
[..]
>> Yes it is strange and has always been like that. Why not moving the
>> assignment before calling the driver callback?

Think I'll do that. Easier than updating all the drivers that don't fill 
it in..

[dpdk-dev] [PATCH] examples/distributor: fix Rx thread logic for zero packets

[Reshma Pattan]

From: Reshma Pattan 

Zero packets can be returned by rte_eth_rx_burst() and
rte_distributor_returned_pkts() inside lcore_rx(), so
for zero packet scenario instead of proceeding to
next operations we should continue to the next iteration of the
loop to avoid unnecessary processing overhead which is causing
rx packets to be dropped and hence distributor failing to forward the
packets.

Fixes: 07db4a97 ("examples/distributor: new sample app")

Signed-off-by: Reshma Pattan 
---
 examples/distributor/main.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/examples/distributor/main.c b/examples/distributor/main.c
index 24857f2..537cee1 100644
--- a/examples/distributor/main.c
+++ b/examples/distributor/main.c
@@ -221,14 +221,22 @@ lcore_rx(struct lcore_params *p)
struct rte_mbuf *bufs[BURST_SIZE*2];
const uint16_t nb_rx = rte_eth_rx_burst(port, 0, bufs,
BURST_SIZE);
+   if (unlikely(nb_rx == 0)) {
+   if (++port == nb_ports)
+   port = 0;
+   continue;
+   }
app_stats.rx.rx_pkts += nb_rx;

rte_distributor_process(d, bufs, nb_rx);
const uint16_t nb_ret = rte_distributor_returned_pkts(d,
bufs, BURST_SIZE*2);
app_stats.rx.returned_pkts += nb_ret;
-   if (unlikely(nb_ret == 0))
+   if (unlikely(nb_ret == 0)) {
+   if (++port == nb_ports)
+   port = 0;
continue;
+   }

uint16_t sent = rte_ring_enqueue_burst(r, (void *)bufs, nb_ret);
app_stats.rx.enqueued_pkts += sent;
-- 
2.5.0

[dpdk-dev] weak functions in some drivers

[Zoltan Kiss]

Hi,

On 01/07/16 11:19, Sergio Gonzalez Monroy wrote:
> On 01/07/2016 11:16, Elo, Matias (Nokia - FI/Espoo) wrote:
>>> -Original Message-
>>> From: Sergio Gonzalez Monroy [mailto:sergio.gonzalez.monroy at intel.com]
>>> Sent: Friday, July 01, 2016 1:05 PM
>>> To: Elo, Matias (Nokia - FI/Espoo) ;
>>> dev at dpdk.org
>>> Cc: ferruh.yigit at intel.com; damarion at cisco.com
>>> Subject: Re: [dpdk-dev] weak functions in some drivers
>>>
>>> On 01/07/2016 10:42, Elo, Matias (Nokia - FI/Espoo) wrote:
> What is not clear to me is motivation to use weak here instead
> of simply
>> using >CONFIG_RTE_I40E_INC_VECTOR
> macro to exclude stubs in i40e_rxtx.c. It will make library
> smaller and
>>> avoid
>> issues like this one
> which are quite hard to troubleshoot.
 Since this issue seen in fd.io, I didn't investigated more, but
 I don't
 want to clock your valid question, this is an attempt to
 resurrect the
 question ...
>>> Hi,
>>>
>>> We are having exactly the same problem. For us the aforementioned
>> workaround doesn't seem to work and vector mode is always disabled
>> with
>>> the
>> i40e drivers. If I modify i40e_rxtx.c and exclude the stub
>> functions using
>> CONFIG_RTE_I40E_INC_VECTOR everything works as expected.
>>> We are building DPDK with the CONFIG_RTE_BUILD_COMBINE_LIBS option
>> enabled and link DPDK library to our application.
>>> Any other ideas how this could be fixed?
>>>
>>> Regards,
>>> Matias
>>>
>> So you have tried to link a combined static lib with --whole-archive
>> -ldpdk --no-whole-archive and still get the wrong/weak function
>> definition?
>>
>> Sergio
> I actually just managed to fix the problem. In our case I had to add
> '-Wl,--whole-archive,-ldpdk,--no-whole-archive'  to the end of
> AM_LDFLAGS.
>
 It turned out that the problem actually wasn't fixed.

 DPDK is built with CONFIG_RTE_BUILD_COMBINE_LIBS=y and
>>> EXTRA_CFLAGS="-fPIC"
 What we are linking originally:
 -l:libdpdk.a

 This works otherwise but vector mode i40e is not enabled.

 When trying:
 -Wl,--whole-archive,-l:libdpdk.a,--no-whole-archive

 Linking fails with ' undefined reference' errors to several dpdk
 functions
>>> (rte_eal_init, rte_cpu_get_flag_enabled, rte_eth_stats_get...).
 Btw. there seems to be a Stack Overflow question related to this:
>>> http://stackoverflow.com/questions/38064021/dpdk-include-libraries-in-dpdk-
>>>
>>> application-compiled-as-shared-library
 -Matias
>>> What DPDK version are you using?
>> v16.04
>
> Ok. I was asking because there is no CONFIG_RTE_BUILD_COMBINE_LIBS in
> 16.04.
>
>
> Could you provide full link/compile command line? I'm not able to
> reproduce the issue so far

I've dug into this issue a bit, let me explain it with some example code:

main.c:
void bar(void);
void foo(void);

int main() {
bar();
//foo();
}
==
weak.c:
#include 

void __attribute__((weak)) foo(void) {
printf("Weak\n");
}

void bar(void) {
foo();
printf("No call\n");
}
==
strong.c:
#include 

void foo(void) {
printf("Strong\n");
}

Compile the second two into a library and link it with main:

gcc -o strong.o -c strong.c
ar rcs libbar.a strong.o weak.o
gcc main.c -L. -lbar -o main

Then look which foo were used:

objdump -x libbar.a | grep foo
 g F .text  0010 foo
  wF .text  0010 foo
0015 R_X86_64_PC32 foo-0x0004

objdump -x main | grep foo
00400538 w F .text 0010 foo

So it got the weak version, simply because bar() was referred, and that 
already pulled the other symbols from that .o file, so ld stop looking 
for the another ones. If you uncomment the foo() call in main(), it will 
do a proper search, and pulls in the strong symbol.

I think there are only workarounds here:
- use --whole-archive, but that pulls in a lot of stuff you don't 
necessarily need. And if you feed that stuff to libtool, it seems to 
misbehave when it sees the -Wl parameter. The Ubuntu 14.04 libtool 
definitely has a bug around that. Matias ran into this with ODP-DPDK: it 
fixed the weak symbol problem in the .so, but then libtool forgot to add 
-ldpdk when doing statical linking
- use only dynamic linking: I guess that's not a viable restriction

The best would be to get rid of these weak functions, as they don't work 
the same with dynamic and static linking, especially the latter is 
problematic. I think Damjan suggested something like that in his 
original message.

I propose something like this:

@@ -3268,10 +3268,40 @@ i40e_set_tx_function(struct rte_eth_dev *dev)
  }

  /* Stubs needed for linkage when CONFIG_RTE_I40E_INC_VECTOR is set to 

[dpdk-dev] [PATCH v3] doc: flow bifurcation guide on Linux

[Jingjing Wu]

Flow bifurcation is a mechanism which depends the advanced
Ethernet device to split traffic between queues. It provides
the capability to let the kernel driver and DPDK driver co-exist
and take their advantages.
It is achieved by using SRIOV and NIC's advanced filtering. This
patch describes it and adds the user guide on ixgbe and i40 NICs.

Signed-off-by: Jingjing Wu 
---
v3 changes:
 - rename bifurcated driver to flow bifurcation
 - move the doc from nics to howto

This patch is based on patch set "[PATCH v3 0/2] doc: live migration procedure"
   http://www.dpdk.org/ml/archives/dev/2016-July/043476.html

 doc/guides/howto/flow_bifurcation.rst  | 288 +++
 doc/guides/howto/img/flow_bifurcation_overview.svg | 544 +
 doc/guides/howto/img/ixgbe_bifu_queue_idx.svg  | 101 
 doc/guides/howto/index.rst |   1 +
 4 files changed, 934 insertions(+)
 create mode 100644 doc/guides/howto/flow_bifurcation.rst
 create mode 100644 doc/guides/howto/img/flow_bifurcation_overview.svg
 create mode 100644 doc/guides/howto/img/ixgbe_bifu_queue_idx.svg

diff --git a/doc/guides/howto/flow_bifurcation.rst 
b/doc/guides/howto/flow_bifurcation.rst
new file mode 100644
index 000..5e10952
--- /dev/null
+++ b/doc/guides/howto/flow_bifurcation.rst
@@ -0,0 +1,288 @@
+..  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.
+
+
+Flow bifurcation guide
+==
+
+Flow bifurcation is a mechanism which depends the advanced Ethernet device to
+split traffic between Linux user space and kernel space. Because it is hardware
+assisted design, this approach can provide the line rate processing capability.
+Other than KNI, the SW is just required to device configuration, no need to
+take care of the packet movement during the traffic split. This can get more
+performance with less CPU overhead.
+
+The Flow bifurcation takes advantage of Ethernet device feature to split the
+incoming data traffic to user space application (Such as DPDK application)
+and/or kernel space program (Linux kernel stack). It can direct some traffic
+(e.g data plane traffic) to DPDK, while direct some other traffic (e.g control
+plane traffic) to the traditional Linux networking stack.
+
+There are a number of technical options to achieve this. A typical example is
+to combine the technology of SR-IOV and packet classification filtering.
+
+SR-IOV is a PCI standard that allows the same physical adapter to split as
+multiple virtual functions. Each virtual function has separated queues with
+physical function. Traffic with a virtual function's destination MAC address
+from network adapter will be directed to it. In a sense, SR-IOV has the
+capability on queue division.
+
+Packet classification filtering is the hardware capability available on most
+network adapters. Filters can be configured to direct specific flows to a given
+receive queue by hardware. Different NIC may have different filter types to
+direct flow to a Virtual Function or a queue belong to it.
+
+Linux network can receive the specific traffic through kernel driver, while
+DPDK can receive the specific traffic bypassing the Linux kernel by using
+drivers like VFIO or DPDK igb_uio module.
+
+.. _figure_flow_bifurcation_overview:
+
+.. figure:: img/flow_bifurcation_overview.*
+
+   Flow Bifucation Overview
+
+
+Use Flow bifurcation on IXGBE in Linux

[dpdk-dev] rte_ether: Driver-specific stats getting overwritten

[Thomas Monjalon]

2016-07-14 14:29, Remy Horton:
> 'noon,
> 
> In rte_eth_stats_get() after doing the driver callout to populate struct 
> rte_eth_stats, the rx_nombuf member is overwritten with 
> dev->data->rx_mbuf_alloc_failed even though some drivers will have 
> filled rx_nombuf with a value from elsewhere. This makes assignment of 
> rx_nombuf from within the driver callout redundant. Is this intentional?

Yes it is strange and has always been like that.
Why not moving the assignment before calling the driver callback?

[dpdk-dev] [PATCH] doc: update release notes

[Iremonger, Bernard]

Hi Thomas,



> Subject: Re: [dpdk-dev] [PATCH] doc: update release notes
> 
> 2016-07-14 13:45, Iremonger, Bernard:
> > > 2016-07-01 15:10, Bernard Iremonger:
> > > > add release note for live migration of a VM with SRIOV VF
> > > [...]
> > > > +* **Added support for live migration of a VM with SRIOV VF.**
> > > > +
> > > > +  Live migration of a VM with Virtio and VF PMD's using the bonding
> PMD.
> > >
> > > Is there some new code for this feature?
> > > I am not sure we need to update the release notes for a doc addition.
> >
> > The testpmd patches which I submitted and were applied, were made to
> enable testing of this feature.
> > This doc patch is to announce that this feature is available now.
> 
> The feature was not available before?

It has not been announced previously.

Regards,

Bernard.

[dpdk-dev] [PATCH] doc: update release notes

[Thomas Monjalon]

2016-07-01 15:10, Bernard Iremonger:
> add release note for live migration of a VM with SRIOV VF
[...]
> +* **Added support for live migration of a VM with SRIOV VF.**
> +
> +  Live migration of a VM with Virtio and VF PMD's using the bonding PMD.

Is there some new code for this feature?
I am not sure we need to update the release notes for a doc addition.

[dpdk-dev] [PATCH] spinlock: Move constructor function out of header file

[damar...@cisco.com]

From: Damjan Marion 

Having constructor function in the header gile is generaly
bad idea, as it will eventually be implanted to 3rd party
library.

In this case it is causing linking issues with 3rd party
libraries when application is not linked to dpdk, due to missing
symbol called by constructor.

Signed-off-by: Damjan Marion 
---
 lib/librte_eal/common/arch/x86/rte_spinlock.c  | 45 ++
 .../common/include/arch/x86/rte_spinlock.h | 13 ++-
 lib/librte_eal/linuxapp/eal/Makefile   |  1 +
 3 files changed, 49 insertions(+), 10 deletions(-)
 create mode 100644 lib/librte_eal/common/arch/x86/rte_spinlock.c

diff --git a/lib/librte_eal/common/arch/x86/rte_spinlock.c 
b/lib/librte_eal/common/arch/x86/rte_spinlock.c
new file mode 100644
index 000..ad8cc5a
--- /dev/null
+++ b/lib/librte_eal/common/arch/x86/rte_spinlock.c
@@ -0,0 +1,45 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2014 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_cpuflags.h"
+
+#include 
+
+uint8_t rte_rtm_supported; /* cache the flag to avoid the overhead
+ of the rte_cpu_get_flag_enabled function */
+
+static void __attribute__((constructor))
+rte_rtm_init(void)
+{
+   rte_rtm_supported = rte_cpu_get_flag_enabled(RTE_CPUFLAG_RTM);
+}
diff --git a/lib/librte_eal/common/include/arch/x86/rte_spinlock.h 
b/lib/librte_eal/common/include/arch/x86/rte_spinlock.h
index 02f95cb..8e630c2 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_spinlock.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_spinlock.h
@@ -94,24 +94,17 @@ rte_spinlock_trylock (rte_spinlock_t *sl)
 }
 #endif

-static uint8_t rtm_supported; /* cache the flag to avoid the overhead
-of the rte_cpu_get_flag_enabled function */
-
-static inline void __attribute__((constructor))
-rte_rtm_init(void)
-{
-   rtm_supported = rte_cpu_get_flag_enabled(RTE_CPUFLAG_RTM);
-}
+extern uint8_t rte_rtm_supported;

 static inline int rte_tm_supported(void)
 {
-   return rtm_supported;
+   return rte_rtm_supported;
 }

 static inline int
 rte_try_tm(volatile int *lock)
 {
-   if (!rtm_supported)
+   if (!rte_rtm_supported)
return 0;

int retries = RTE_RTM_MAX_RETRIES;
diff --git a/lib/librte_eal/linuxapp/eal/Makefile 
b/lib/librte_eal/linuxapp/eal/Makefile
index 1a97693..7287d13 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -106,6 +106,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += rte_keepalive.c

 # from arch dir
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += rte_cpuflags.c
+SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += rte_spinlock.c

 CFLAGS_eal_common_cpuflags.o := $(CPUFLAGS_LIST)

-- 
2.7.4

[dpdk-dev] [PATCH 1/2] vfio: fix pci_vfio_map_resource

[Thomas Monjalon]

Someone to review this patch please?
It can be integrated in RC3 if we are sure it doesn't break anything.

2016-07-07 15:26, Yong Wang:
> The offset of the 2nd mmap when mapping the region after msix_bar
> needs to take region address into consideration.  This is exposed
> when using vfio-pci to manage vmxnet3 pmd.
> 
> Fixes: 90a1633b2347 ("eal/linux: allow to map BARs with MSI-X tables")
> 
> Signed-off-by: Yong Wang 
> Signed-off-by: Ronghua Zhang 
> ---
>  lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c 
> b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> index f91b924..3729c35 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> @@ -896,7 +896,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
>   } else {
>   memreg[0].offset = reg.offset;
>   memreg[0].size = table_start;
> - memreg[1].offset = table_end;
> + memreg[1].offset = reg.offset + table_end;
>   memreg[1].size = reg.size - table_end;
>  
>   RTE_LOG(DEBUG, EAL,
> @@ -939,7 +939,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
>   /* if there's a second part, try to map it */
>   if (map_addr != MAP_FAILED
>   && memreg[1].offset && memreg[1].size) {
> - void *second_addr = RTE_PTR_ADD(bar_addr, 
> memreg[1].offset);
> + void *second_addr = RTE_PTR_ADD(bar_addr,
> + 
> memreg[1].offset - memreg[0].offset);
>   map_addr = pci_map_resource(second_addr,
>   vfio_dev_fd, 
> memreg[1].offset,
>   memreg[1].size,
> 

[dpdk-dev] [PATCH] spinlock:move constructor function out of header

[Thomas Monjalon]

Thanks Keith for continuing work.

2016-07-14 14:31, Keith Wiles:
> Moving the rte_rtm_init() constructor routine out of the
> header file and into a new rte_spinlock.c for all archs/platforms.
> Having constructor routines in a header file is not a good
> place to keep these types of functions.
> 
> The problem is with linking 3rd party libraries when
> an application is not linked directly to dpdk libraries, which
> in this case causes a missing symbol in the linking phase.
> 
> Fixes: ba7468997ea6 ("spinlock: add HTM lock elision for x86")
> 
> Originally submitted by Damjan Marion 

You should keep the original Signed-off and authorship (From: field)
with "git am".
It is also easier to track when using -v2 --in-reply-to=''.

> Signed-off-by: Keith Wiles 
> ---
>  lib/librte_eal/bsdapp/eal/Makefile |  1 +
>  lib/librte_eal/common/arch/arm/rte_spinlock.c  | 46 
> ++
>  lib/librte_eal/common/arch/ppc_64/rte_spinlock.c   | 46 
> ++
>  lib/librte_eal/common/arch/tile/rte_spinlock.c | 46 
> ++
>  lib/librte_eal/common/arch/x86/rte_spinlock.c  | 46 
> ++
>  .../common/include/arch/x86/rte_spinlock.h | 14 ++-
>  lib/librte_eal/linuxapp/eal/Makefile   |  1 +

I am not sure we should add a .c file for each arch, given it is called only
from arch/x86/rte_spinlock.h.

[dpdk-dev] [PATCH 1/2] vfio: fix pci_vfio_map_resource

[Burakov, Anatoly]

> Someone to review this patch please?
> It can be integrated in RC3 if we are sure it doesn't break anything.
> 
> 2016-07-07 15:26, Yong Wang:
> > The offset of the 2nd mmap when mapping the region after msix_bar
> > needs to take region address into consideration.  This is exposed when
> > using vfio-pci to manage vmxnet3 pmd.
> >
> > Fixes: 90a1633b2347 ("eal/linux: allow to map BARs with MSI-X tables")
> >
> > Signed-off-by: Yong Wang 
> > Signed-off-by: Ronghua Zhang 

Hi Thomas,

The patch makes sense to me, but I don't have any devices that trigger that 
particular codepath in my immediate vicinity. The original patch was mentioning 
an NVMe adapter, so that probably should be tested with this change. I'm CC'ing 
the original author of that patch just in case. Do we know of any other 
NICs/devices that might be affected by this? Aside from the obvious example of 
vmxnet3... 

Thanks,
Anatoly

[dpdk-dev] dpdk daily build error on dpdk16.07-rc2

[Thomas Monjalon]

2016-07-14 11:34, Liu, Yu Y:
> Hi Thomas,
> 
> Would you check who can work on "dpdk16.07-rc2 compile error on FC18 & 
> UB1204"?

I think I've already fixed it:
http://dpdk.org/ml/archives/dev/2016-July/043918.html

> For "dpdk16.07-rc2 compile error on suse11", I saw document shows we support 
> suse12+.
> Seems it is not issue for this release.
> Agree?

If there is a fix, I will apply it.
But yes you can stop testing Suse 11.

[dpdk-dev] [PATCH] spinlock:move constructor function out of header

[Keith Wiles]

Moving the rte_rtm_init() constructor routine out of the
header file and into a new rte_spinlock.c for all archs/platforms.
Having constructor routines in a header file is not a good
place to keep these types of functions.

The problem is with linking 3rd party libraries when
an application is not linked directly to dpdk libraries, which
in this case causes a missing symbol in the linking phase.

Fixes: ba7468997ea6 ("spinlock: add HTM lock elision for x86")

Originally submitted by Damjan Marion 

Signed-off-by: Keith Wiles 
---
 lib/librte_eal/bsdapp/eal/Makefile |  1 +
 lib/librte_eal/common/arch/arm/rte_spinlock.c  | 46 ++
 lib/librte_eal/common/arch/ppc_64/rte_spinlock.c   | 46 ++
 lib/librte_eal/common/arch/tile/rte_spinlock.c | 46 ++
 lib/librte_eal/common/arch/x86/rte_spinlock.c  | 46 ++
 .../common/include/arch/x86/rte_spinlock.h | 14 ++-
 lib/librte_eal/linuxapp/eal/Makefile   |  1 +
 7 files changed, 190 insertions(+), 10 deletions(-)
 create mode 100644 lib/librte_eal/common/arch/arm/rte_spinlock.c
 create mode 100644 lib/librte_eal/common/arch/ppc_64/rte_spinlock.c
 create mode 100644 lib/librte_eal/common/arch/tile/rte_spinlock.c
 create mode 100644 lib/librte_eal/common/arch/x86/rte_spinlock.c

diff --git a/lib/librte_eal/bsdapp/eal/Makefile 
b/lib/librte_eal/bsdapp/eal/Makefile
index 698fa0a..cd93f5b 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -89,6 +89,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += rte_keepalive.c

 # from arch dir
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += rte_cpuflags.c
+SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += rte_spinlock.c

 CFLAGS_eal_common_cpuflags.o := $(CPUFLAGS_LIST)

diff --git a/lib/librte_eal/common/arch/arm/rte_spinlock.c 
b/lib/librte_eal/common/arch/arm/rte_spinlock.c
new file mode 100644
index 000..bc924e5
--- /dev/null
+++ b/lib/librte_eal/common/arch/arm/rte_spinlock.c
@@ -0,0 +1,46 @@
+/*-
+ *   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 
+
+#include "rte_cpuflags.h"
+
+/**< cache the flag to avoid the overhead of the function call */
+int rte_rtm_supported;
+
+static void __attribute__((constructor))
+rte_rtm_init(void)
+{
+   rte_rtm_supported = rte_cpu_get_flag_enabled(RTE_CPUFLAG_RTM);
+}
+
diff --git a/lib/librte_eal/common/arch/ppc_64/rte_spinlock.c 
b/lib/librte_eal/common/arch/ppc_64/rte_spinlock.c
new file mode 100644
index 000..bc924e5
--- /dev/null
+++ b/lib/librte_eal/common/arch/ppc_64/rte_spinlock.c
@@ -0,0 +1,46 @@
+/*-
+ *   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 

[dpdk-dev] rte_ether: Driver-specific stats getting overwritten

[Remy Horton]

'noon,

In rte_eth_stats_get() after doing the driver callout to populate struct 
rte_eth_stats, the rx_nombuf member is overwritten with 
dev->data->rx_mbuf_alloc_failed even though some drivers will have 
filled rx_nombuf with a value from elsewhere. This makes assignment of 
rx_nombuf from within the driver callout redundant. Is this intentional?

..Remy

[dpdk-dev] [PATCH] doc: update release notes

[Iremonger, Bernard]

Hi Thomas,



> Subject: Re: [dpdk-dev] [PATCH] doc: update release notes
> 
> 2016-07-01 15:10, Bernard Iremonger:
> > add release note for live migration of a VM with SRIOV VF
> [...]
> > +* **Added support for live migration of a VM with SRIOV VF.**
> > +
> > +  Live migration of a VM with Virtio and VF PMD's using the bonding PMD.
> 
> Is there some new code for this feature?
> I am not sure we need to update the release notes for a doc addition.

The testpmd patches which I submitted and were applied, were made to enable 
testing of this feature.
This doc patch is to announce that this feature is available now.

Regards,

Bernard.  

[dpdk-dev] 16.07-rc2 issue with rte_rtm_init(void) constructor

[Thomas Monjalon]

2016-07-14 09:36, Damjan Marion:
> 
> > On 14 Jul 2016, at 10:20, Thomas Monjalon  
> > wrote:
> > 
> > 2016-07-13 22:58, Damjan Marion:
> >> I have issues with linking application to 16.07-rc2.
> >> 
> >> Looks like reason is constructor function in include file,
> >> so our unit test apps are failing to link as they are not linked with dpdk 
> >> libs.
> >> (and they should not be as they are not calling any dpdk function).
> > 
> > I don't understand:
> > Why are you linking DPDK if you do not use any DPDK function?
> 
> If i simplify it, i have 4 components:
> 
> 1. libdpdk
> 2. libXXX
> 3. app-A
> 4. app-B
> 
> libXXX includes some dpdk headers, mainly because of data structures like 
> rte_mbuf, and some functions are calling
> functions form libdpdk.
> 
> app-A links against libdpdk and libXXX
> app-B links against libXXX only and only uses functions from libXXX which 
> doesn?t have dpdk dependency
> 
> This is working fine in 14.04. In 14.07 rte_rtm_init() implants himself into 
> libXXX as it is defined
> in header file, and that causes linking to fail due to missing 
> rte_cpu_get_flag_enabled().

OK I better understand :)

> >> static inline void __attribute__((constructor))
> >> rte_rtm_init(void)
> >> {
> >>rtm_supported = rte_cpu_get_flag_enabled(RTE_CPUFLAG_RTM);
> >> }
> >> 
> >> linking fails with:
> >> dpdk/include/rte_spinlock.h:103: undefined reference to 
> >> `rte_cpu_get_flag_enabled?
> >> 
> >> Is there any chance that this one is moved to some .c file, so it is loaded
> >> only when it is really needed?
> > 
> > Yes it could be moved to lib/librte_eal/common/arch/x86/.
> 
> Any chance to get this in 16.07 ?

Yes maybe if you submit a patch quickly and it is clean enough.

[dpdk-dev] 16.07-rc2 issue with rte_rtm_init(void) constructor

[Damjan Marion (damarion)]

> On 14 Jul 2016, at 13:30, Thomas Monjalon  
> wrote:
> 
> 2016-07-14 09:36, Damjan Marion:
 
 linking fails with:
 dpdk/include/rte_spinlock.h:103: undefined reference to 
 `rte_cpu_get_flag_enabled?
 
 Is there any chance that this one is moved to some .c file, so it is loaded
 only when it is really needed?
>>> 
>>> Yes it could be moved to lib/librte_eal/common/arch/x86/.
>> 
>> Any chance to get this in 16.07 ?
> 
> Yes maybe if you submit a patch quickly and it is clean enough.

OK, I already have working patch[1], but it is possibly not clean enough.
Basically I moved rte_rtm_init to lib/librte_eal/common/arch/x86/rte_cpuflags.c

Is this the right place?

Should we call ?rtm_supported? different, knowing that now it is not static 
anymore?

Thanks,

Damjan


[1] 
https://github.com/vpp-dev/vpp/blob/dpdk-16.07-rc2/dpdk/dpdk-16.07-rc2_patches/0001-Fix-linking-issue-due-to-constructor-in-header-file.patch

[dpdk-dev] [PATCH] lib: move rte_ring read barrier to correct location

[Ananyev, Konstantin]

Hi Juhamatti,

> 
> Hi Konstantin,
> 
> > > > > > It is quite safe to move the barrier before DEQUEUE because
> > > > > > after the DEQUEUE there is nothing really that we would want
> > > > > > to protect
> > with a
> > > > read barrier.
> > > > >
> > > > > I don't think so.
> > > > > If you remove barrier after DEQUEUE(), that means on systems
> > > > > with relaxed memory ordering cons.tail could be updated before
> > > > > DEQUEUE() will be finished and producer can overwrite queue
> > > > > entries that were
> > not
> > > > yet dequeued.
> > > > > So if cpu can really do such speculative out of order loads,
> > > > > then we do need for  __rte_ring_sc_do_dequeue() something like:
> > > > >
> > > > > rte_smp_rmb();
> > > > > DEQUEUE_PTRS();
> > > > > rte_smp_rmb();
> > >
> > > You have a valid point here, there needs to be a guarantee that
> > > cons_tail
> > cannot
> > > be updated before DEQUEUE is completed. Nevertheless, my point was
> > that it is
> > > not guaranteed with a read barrier anyway. The implementation has
> > > the
> > following
> > > sequence
> > >
> > > DEQUEUE_PTRS(); (i.e. READ/LOAD)
> > > rte_smp_rmb();
> > > ..
> > > r->cons.tail = cons_next; (i.e WRITE/STORE)
> > >
> > > Above read barrier does not guarantee any ordering for the following
> > writes/stores.
> > > As a guarantee is needed, I think we in fact need to change the read
> > > barrier
> > on the
> > > dequeue to a full barrier, which guarantees the read+write order, as
> > follows
> > >
> > > DEQUEUE_PTRS();
> > > rte_smp_mb();
> > > ..
> > > r->cons.tail = cons_next;
> > >
> > > If you agree, I can for sure prepare another patch for this issue.
> >
> > Hmm, I think for __rte_ring_mc_do_dequeue() we are ok with smp_rmb(),
> > as we have to read cons.tail anyway.
> 
> Are you certain that this read creates strong enough dependency between read 
> of cons.tail and the write of it on the mc_do_dequeue()? 

Yes, I believe so.

> I think it does not really create any control dependency there as the next 
> write is not dependent of the result of the read.

I think it is dependent: cons.tail can be updated only if it's current value is 
eual to precomputed before cons_head.
So cpu has to read cons.tail value first.

> The CPU also
> knows already the value that will be written to cons.tail and that value does 
> not depend on the previous read either. The CPU does not
> know we are planning to do a spinlock there, so it might do things 
> out-of-order without proper dependencies.
> 
> > For  __rte_ring_sc_do_dequeue(), I think you right, we might need
> > something stronger.
> > I don't want to put rte_smp_mb() here as it would cause full HW
> > barrier even on machines with strong memory order (IA).
> > I think that rte_smp_wmb() might be enough here:
> > it would force cpu to wait till writes in DEQUEUE_PTRS() are become
> > visible, which means reads have to be completed too.
> 
> In practice I think that rte_smp_wmb() would work fine, even though it is not 
> strictly according to the book. Below solution would be my
> proposal as a fix to the issue of sc dequeueing (and also to mc dequeueing, 
> if we have the problem of CPU completely ignoring the spinlock
> in reality there):
> 
> DEQUEUE_PTRS();
> ..
> rte_smp_wmb();
> r->cons.tail = cons_next;

As I said in previous email - it looks good for me for 
_rte_ring_sc_do_dequeue(),
but I am interested to hear what  ARM and PPC maintainers think about it.
Jan, Jerin do you have any comments on it?
Chao, sorry but I still not sure why PPC is considered as architecture with 
strong memory ordering?
Might be I am missing something obvious here.
Thank
Konstantin

> 
> --
>  Juhamatti
> 
> > Another option would be to define a new macro: rte_weak_mb() or so,
> > that would be expanded into CB on boxes with strong memory model, and
> > to full MB on machines with relaxed ones.
> > Interested to hear what ARM and PPC guys think.
> > Konstantin
> >
> > P.S. Another thing a bit off-topic - for PPC guys:
> > As I can see smp_rmb/smp_wmb are just a complier barriers:
> > find lib/librte_eal/common/include/arch/ppc_64/ -type f | xargs grep
> > smp_ lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h:#define
> > rte_smp_mb() rte_mb()
> > lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h:#define
> > rte_smp_wmb() rte_compiler_barrier()
> > lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h:#define
> > rte_smp_rmb() rte_compiler_barrier()
> > My knowledge about PPC architecture is rudimental, but is that really 
> > enough?
> >
> > >
> > > Thanks,
> > > --
> > >  Juhamatti
> > >
> > > > > Konstantin
> > > > >
> > > > > > The read
> > > > > > barrier is mapped to a compiler barrier on strong memory model
> > > > > > systems and this works fine too as the order of the head,tail
> > > > > > updates is still guaranteed on the new location. Even if the
> > > > > > problem would be theoretical on most systems, it is worth
> > > > > > fixing as the risk for
> > > > problems is 

[dpdk-dev] dpdk daily build error on dpdk16.07-rc2

[Liu, Yu Y]

Hi Thomas,

Would you check who can work on "dpdk16.07-rc2 compile error on FC18 & UB1204"?

For "dpdk16.07-rc2 compile error on suse11", I saw document shows we support 
suse12+. Seems it is not issue for this release.
Agree?

Thanks & Regards,
Yu Liu

From: Gu, YongjieX
Sent: Thursday, July 14, 2016 5:23 PM
To: thomas.monjalon at 6wind.com; Richardson, Bruce ; Yigit, Ferruh ; Gonzalez Monroy, Sergio 
; Jan Medala ; dev at 
dpdk.org
Cc: Cao, Waterman ; Liu, Yu Y ; Chen, WeichunX ; Xu, HuilongX 
; Gu, YongjieX 
Subject: dpdk daily build error on dpdk16.07-rc2

Hi,All,
  There are 4 build errors reported by our dpdk daily report,and can you take a 
look on these issues.
  Jan is working on ena issues,and I will help to test it.

OS, Kernel , GCC , ICC

Affected Targets

Build errors

comments

FC18_64,3.6.10-4,4.7.2,14.0.0

x86_64-ivshmem-linuxapp-gcc

CC test_kvargs.o

UBT124_64,3.8.0-29,4.6.3,14.0.0

  LD test

SUSE11SP2_64,3.0.13-0,4.5.1,14.0.0

/jenkins/workspace/DPDK_AUTO_IDT_VM_FC18_64_BUILD2/DPDK/x86_64-ivshmem-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o):
 In function `eal_alarm_callback':


eal_alarm.c:(.text+0xd7): undefined reference to `clock_gettime'


/jenkins/workspace/DPDK_AUTO_IDT_VM_FC18_64_BUILD2/DPDK/x86_64-ivshmem-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o):
 In function `rte_eal_alarm_set':


eal_alarm.c:(.text+0x20f): undefined reference to `clock_gettime'


/jenkins/workspace/DPDK_AUTO_IDT_VM_FC18_64_BUILD2/DPDK/x86_64-ivshmem-linuxapp-gcc/lib/librte_eal.a(eal_timer.o):
 In function `get_tsc_freq':


eal_timer.c:(.text+0x108): undefined reference to `clock_gettime'


eal_timer.c:(.text+0x146): undefined reference to `clock_gettime'


collect2: error: ld returned 1 exit status


make[5]: *** [test] Error 1


make[4]: *** [test] Error 2


make[3]: *** [app] Error 2


make[2]: *** [all] Error 2


make[1]: *** [pre_install] Error 2


make: *** [install] Error 2



SUSE11SP2_64,3.0.13-0,4.5.1,14.0.0

x86_64-native-linuxapp-gcc-examples

== ipsec-secgw


  CC ipsec.o


  CC esp.o


  CC sp4.o


  CC sp6.o


  CC sa.o


/jenkins/workspace/DPDK_AUTO_IDT_VM_SUSE11SP2_64_BUILD/DPDK/examples/ipsec-secgw/sa.c:56:2:
 error: unknown field 'ip4' specified in initializer


compilation terminated due to -Wfatal-errors.


make[4]: *** [sa.o] Error 1


make[3]: *** [all] Error 2


make[2]: *** [ipsec-secgw] Error 2


make[1]: *** [x86_64-native-linuxapp-gcc_examples] Error 2


make: *** [examples] Error 2



x86_64-native-linuxapp-icc-examples

== ipsec-secgw




x86_64-ivshmem-linuxapp-icc-examples

  CC ipsec.o




  CC esp.o




  CC sp4.o




  CC sp6.o




  CC sa.o




icc: command line warning #10158: ignoring option '-diag-disable'; argument 
must be separate




/jenkins/workspace/DPDK_AUTO_IDT_VM_SUSE11SP2_64_BUILD/DPDK/examples/ipsec-secgw/sa.c(56):
 error: a designator for an anonymous union member can only appear within 
braces corresponding to that anonymous union




.src.ip4 = IPv4(172, 16, 1, 5),




 ^







compilation aborted for 
/jenkins/workspace/DPDK_AUTO_IDT_VM_SUSE11SP2_64_BUILD/DPDK/examples/ipsec-secgw/sa.c
 (code 4)




make[4]: *** [sa.o] Error 4




make[3]: *** [all] Error 2




make[2]: *** [ipsec-secgw] Error 2




make[1]: *** [x86_64-native-linuxapp-icc_examples] Error 2




make: *** [examples] Error 2





UBT144_32,3.13.0-30,4.8.2,14.0.0

i686-native-linuxapp-icc

== Build drivers/net/ena






  CC ena_ethdev.o






  PMDINFO ena_ethdev.o.pmd.c






  CC ena_ethdev.o.pmd.o






  LD ena_ethdev.o






  CC ena_com.o






/jenkins/workspace/DPDK_AUTO_IDT_VM_UBT144_32_BUILD/DPDK/drivers/net/ena/base/ena_com.c(346):
 error #3656: variable "dev_node" may be used before its value is set






ENA_MEM_ALLOC_COHERENT_NODE(ena_dev->dmadev,






^









compilation aborted for 
/jenkins/workspace/DPDK_AUTO_IDT_VM_UBT144_32_BUILD/DPDK/drivers/net/ena/base/ena_com.c
 (code 4)






make[6]: *** [ena_com.o] Error 4






make[5]: *** [ena] Error 2






make[4]: *** [net] Error 2






make[3]: *** [drivers] Error 2






make[2]: *** [all] Error 2






make[1]: *** [pre_install] Error 2






make: *** [install] Error 2






freebsd10.3,10.3-RELEASE,4.8.5

x86_64-native-linuxapp-gcc

sed: 1: "/usr/home/xugang/dpdk-1 ...": extra characters at the end of h command


sed: 1: "/usr/home/xugang/dpdk-1 ...": extra characters at the end of h command


sed: 1: "/usr/home/xugang/dpdk-1 ...": extra characters at the end of h command


sed: 1: "/usr/home/xugang/dpdk-1 ...": extra characters at the end of h command


sed: 1: "/usr/home/xugang/dpdk-1 ...": extra characters at the end of h command


sed: 1: "/usr/home/xugang/dpdk-1 ...": extra characters at the end of h command


sed: 1: "/usr/home/xugang/dpdk-1 ...": extra characters at the end of h command


sed: 1: "/usr/home/xugang/dpdk-1 ...": extra characters at the end of h command


sed: 1: "/usr/home/xugang/dpdk-1 ...": extra characters at the end of h command


sed: 1: "/usr/home/xugang/dpdk-1 ...": extra 

[dpdk-dev] [PATCH v2] vhost: fix segfault on bad descriptor address

[Ilya Maximets]

In current implementation vhost will crash with segmentation fault
if malicious or buggy virtio application breaks addresses of descriptors.

Before commit 0823c1cb0a73 this crash was reproducible even with
normal DPDK application that tries to change number of virtqueues
dynamically inside VM.

Fix that by checking addresses of descriptors before using.

Also fixed return value on error for 'copy_mbuf_to_desc_mergeable()'
from '-1' to '0' because it returns unsigned value and it means
number of used descriptors.

Signed-off-by: Ilya Maximets 
---
Version 2:
* Rebased on top of current master.
* host's address now checked in meargeable case,
  because needed refactoring already done.
* Commit-message changed because old issue with
  virtio reload accidentially fixed by commit
  0823c1cb0a73.

 lib/librte_vhost/vhost_rxtx.c | 28 +---
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
index 15ca956..31e8b58 100644
--- a/lib/librte_vhost/vhost_rxtx.c
+++ b/lib/librte_vhost/vhost_rxtx.c
@@ -147,10 +147,10 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct 
vhost_virtqueue *vq,
struct virtio_net_hdr_mrg_rxbuf virtio_hdr = {{0, 0, 0, 0, 0, 0}, 0};

desc = >desc[desc_idx];
-   if (unlikely(desc->len < dev->vhost_hlen))
+   desc_addr = gpa_to_vva(dev, desc->addr);
+   if (unlikely(desc->len < dev->vhost_hlen || !desc_addr))
return -1;

-   desc_addr = gpa_to_vva(dev, desc->addr);
rte_prefetch0((void *)(uintptr_t)desc_addr);

virtio_enqueue_offload(m, _hdr.hdr);
@@ -182,7 +182,10 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct 
vhost_virtqueue *vq,
return -1;

desc = >desc[desc->next];
-   desc_addr   = gpa_to_vva(dev, desc->addr);
+   desc_addr = gpa_to_vva(dev, desc->addr);
+   if (unlikely(!desc_addr))
+   return -1;
+
desc_offset = 0;
desc_avail  = desc->len;
}
@@ -387,10 +390,10 @@ copy_mbuf_to_desc_mergeable(struct virtio_net *dev, 
struct vhost_virtqueue *vq,
LOG_DEBUG(VHOST_DATA, "(%d) current index %d | end index %d\n",
dev->vid, cur_idx, end_idx);

-   if (buf_vec[vec_idx].buf_len < dev->vhost_hlen)
-   return -1;
-
desc_addr = gpa_to_vva(dev, buf_vec[vec_idx].buf_addr);
+   if (buf_vec[vec_idx].buf_len < dev->vhost_hlen || !desc_addr)
+   return 0;
+
rte_prefetch0((void *)(uintptr_t)desc_addr);

virtio_hdr.num_buffers = end_idx - start_idx;
@@ -425,6 +428,8 @@ copy_mbuf_to_desc_mergeable(struct virtio_net *dev, struct 
vhost_virtqueue *vq,

vec_idx++;
desc_addr = gpa_to_vva(dev, buf_vec[vec_idx].buf_addr);
+   if (unlikely(!desc_addr))
+   return 0;

/* Prefetch buffer address. */
rte_prefetch0((void *)(uintptr_t)desc_addr);
@@ -507,7 +512,7 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t 
queue_id,
*(volatile uint16_t *)>used->idx += nr_used;
vhost_log_used_vring(dev, vq, offsetof(struct vring_used, idx),
sizeof(vq->used->idx));
-   vq->last_used_idx = end;
+   vq->last_used_idx += nr_used;
}

if (likely(pkt_idx)) {
@@ -688,6 +693,9 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct 
vhost_virtqueue *vq,
return -1;

desc_addr = gpa_to_vva(dev, desc->addr);
+   if (unlikely(!desc_addr))
+   return -1;
+
hdr = (struct virtio_net_hdr *)((uintptr_t)desc_addr);
rte_prefetch0(hdr);

@@ -701,6 +709,9 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct 
vhost_virtqueue *vq,
desc = >desc[desc->next];

desc_addr = gpa_to_vva(dev, desc->addr);
+   if (unlikely(!desc_addr))
+   return -1;
+
rte_prefetch0((void *)(uintptr_t)desc_addr);

desc_offset = 0;
@@ -737,6 +748,9 @@ copy_desc_to_mbuf(struct virtio_net *dev, struct 
vhost_virtqueue *vq,
desc = >desc[desc->next];

desc_addr = gpa_to_vva(dev, desc->addr);
+   if (unlikely(!desc_addr))
+   return -1;
+
rte_prefetch0((void *)(uintptr_t)desc_addr);

desc_offset = 0;
-- 
2.7.4

[dpdk-dev] 16.07-rc2 issue with rte_rtm_init(void) constructor

[Thomas Monjalon]

2016-07-13 22:58, Damjan Marion:
> I have issues with linking application to 16.07-rc2.
> 
> Looks like reason is constructor function in include file,
> so our unit test apps are failing to link as they are not linked with dpdk 
> libs.
> (and they should not be as they are not calling any dpdk function).

I don't understand:
Why are you linking DPDK if you do not use any DPDK function?

> static inline void __attribute__((constructor))
> rte_rtm_init(void)
> {
> rtm_supported = rte_cpu_get_flag_enabled(RTE_CPUFLAG_RTM);
> }
> 
> linking fails with:
> dpdk/include/rte_spinlock.h:103: undefined reference to 
> `rte_cpu_get_flag_enabled?
> 
> Is there any chance that this one is moved to some .c file, so it is loaded
> only when it is really needed?

Yes it could be moved to lib/librte_eal/common/arch/x86/.

[dpdk-dev] [PATCH 2/2] bnx2x: fix fw mempool name length

[Rasesh Mody]

This patch fixes following error:
   EAL: Detected 36 lcore(s)
   EAL: Probing VFIO support...
   PMD: bnxt_rte_pmd_init() called for (null)
   EAL: PCI device :08:00.0 on NUMA socket 0
   EAL:   probe driver: 14e4:16a1 rte_bnx2x_pmd
   EAL: PCI device :08:00.1 on NUMA socket 0
   EAL:   probe driver: 14e4:16a1 rte_bnx2x_pmd
   Lcore 0: RX port 0
   Lcore 1: RX port 1
   Initializing port 0... EAL: Error - exiting with code: 1
 Cause: Cannot configure device: err=-6, port=0

Fixes: 540a2110 ("bnx2x: driver core")
Fixes: 85cf0079 ("mem: avoid memzone/mempool/ring name truncation")

Signed-off-by: Rasesh Mody 
---
 drivers/net/bnx2x/bnx2x.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index a059858..a49a07f 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -8886,7 +8886,7 @@ int bnx2x_alloc_hsi_mem(struct bnx2x_softc *sc)
 /***/

if (bnx2x_dma_alloc(sc, FW_BUF_SIZE, >gz_buf_dma,
- "fw_dec_buf", RTE_CACHE_LINE_SIZE) != 0) {
+ "fw_buf", RTE_CACHE_LINE_SIZE) != 0) {
sc->spq = NULL;
sc->sp = NULL;
sc->eq = NULL;
-- 
1.7.10.3

[dpdk-dev] [PATCH 1/2] bnx2x: disable fast path interrupts

[Rasesh Mody]

Disable fastpath interrupts and remove unneeded delay in
bnx2x_interrupt_action(). This patch fixes and prevents performance
degradation for BNX2X PMD.

Fixes: 540a2110 ("bnx2x: driver core")

Signed-off-by: Rasesh Mody 
---
 drivers/net/bnx2x/bnx2x.c|2 +-
 drivers/net/bnx2x/bnx2x_ethdev.c |4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index 95fbad8..a059858 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -4507,7 +4507,7 @@ static void bnx2x_handle_fp_tq(struct bnx2x_fastpath *fp, 
int scan_fp)
}

bnx2x_ack_sb(sc, fp->igu_sb_id, USTORM_ID,
-  le16toh(fp->fp_hc_idx), IGU_INT_ENABLE, 1);
+  le16toh(fp->fp_hc_idx), IGU_INT_DISABLE, 1);
 }

 /*
diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index c8d2bf2..f97dd5b 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -107,8 +107,8 @@ bnx2x_interrupt_action(struct rte_eth_dev *dev)

PMD_DEBUG_PERIODIC_LOG(INFO, "Interrupt handled");

-   if (bnx2x_intr_legacy(sc, 0))
-   DELAY_MS(250);
+   bnx2x_intr_legacy(sc, 0)
+
if (sc->periodic_flags & PERIODIC_GO)
bnx2x_periodic_callout(sc);
link_status = REG_RD(sc, sc->link_params.shmem_base +
-- 
1.7.10.3

[dpdk-dev] [PATCH 0/3] add new command line options and error handling in pdump

[Ananyev, Konstantin]

This patch set contains
1)Error handling fixes in pdump library.
2)Support of server and client socket path command line options in pdump tool.
3)Default socket path name fixes in pdump library doc.

Reshma Pattan (3):
  pdump: fix error handlings
  app/pdump: add new command line options for socket paths
  doc: fix default socket path names

 app/pdump/main.c| 57 +++--
 doc/guides/prog_guide/pdump_lib.rst | 12   
doc/guides/sample_app_ug/pdump.rst  | 31 ++--
 lib/librte_pdump/rte_pdump.c| 26 +
 4 files changed, 97 insertions(+), 29 deletions(-)

Acked-by: Konstantin Ananyev 

--
2.5.0

[dpdk-dev] [PATCH 2/2] i40e: Enable bad checksum flags in i40e vPMD

[Jeff Shaw]

From: Damjan Marion 

Decode the checksum flags from the rx descriptor, setting
the appropriate bit in the mbuf ol_flags field when the flag
indicates a bad checksum.

Signed-off-by: Damjan Marion 
Signed-off-by: Jeff Shaw 
---
 drivers/net/i40e/i40e_rxtx_vec.c | 48 +++-
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/net/i40e/i40e_rxtx_vec.c b/drivers/net/i40e/i40e_rxtx_vec.c
index e78ac63..ace51df 100644
--- a/drivers/net/i40e/i40e_rxtx_vec.c
+++ b/drivers/net/i40e/i40e_rxtx_vec.c
@@ -138,19 +138,14 @@ i40e_rxq_rearm(struct i40e_rx_queue *rxq)
 static inline void
 desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
 {
-   __m128i vlan0, vlan1, rss;
-   union {
-   uint16_t e[4];
-   uint64_t dword;
-   } vol;
+   __m128i vlan0, vlan1, rss, l3_l4e;

/* mask everything except RSS, flow director and VLAN flags
 * bit2 is for VLAN tag, bit11 for flow director indication
 * bit13:12 for RSS indication.
 */
-   const __m128i rss_vlan_msk = _mm_set_epi16(
-   0x, 0x, 0x, 0x,
-   0x3804, 0x3804, 0x3804, 0x3804);
+   const __m128i rss_vlan_msk = _mm_set_epi32(
+   0x1c03004, 0x1c03004, 0x1c03004, 0x1c03004);

/* map rss and vlan type to rss hash and vlan flag */
const __m128i vlan_flags = _mm_set_epi8(0, 0, 0, 0,
@@ -163,23 +158,36 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf 
**rx_pkts)
PKT_RX_RSS_HASH | PKT_RX_FDIR, PKT_RX_RSS_HASH, 0, 0,
0, 0, PKT_RX_FDIR, 0);

-   vlan0 = _mm_unpackhi_epi16(descs[0], descs[1]);
-   vlan1 = _mm_unpackhi_epi16(descs[2], descs[3]);
-   vlan0 = _mm_unpacklo_epi32(vlan0, vlan1);
+   const __m128i l3_l4e_flags = _mm_set_epi8(0, 0, 0, 0, 0, 0, 0, 0,
+   PKT_RX_EIP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD | 
PKT_RX_IP_CKSUM_BAD,
+   PKT_RX_EIP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD,
+   PKT_RX_EIP_CKSUM_BAD | PKT_RX_IP_CKSUM_BAD,
+   PKT_RX_EIP_CKSUM_BAD,
+   PKT_RX_L4_CKSUM_BAD | PKT_RX_IP_CKSUM_BAD,
+   PKT_RX_L4_CKSUM_BAD,
+   PKT_RX_IP_CKSUM_BAD,
+   0);
+
+   vlan0 = _mm_unpackhi_epi32(descs[0], descs[1]);
+   vlan1 = _mm_unpackhi_epi32(descs[2], descs[3]);
+   vlan0 = _mm_unpacklo_epi64(vlan0, vlan1);

vlan1 = _mm_and_si128(vlan0, rss_vlan_msk);
vlan0 = _mm_shuffle_epi8(vlan_flags, vlan1);

-   rss = _mm_srli_epi16(vlan1, 11);
+   rss = _mm_srli_epi32(vlan1, 12);
rss = _mm_shuffle_epi8(rss_flags, rss);

+   l3_l4e = _mm_srli_epi32(vlan1, 22);
+   l3_l4e = _mm_shuffle_epi8(l3_l4e_flags, l3_l4e);
+
vlan0 = _mm_or_si128(vlan0, rss);
-   vol.dword = _mm_cvtsi128_si64(vlan0);
+   vlan0 = _mm_or_si128(vlan0, l3_l4e);

-   rx_pkts[0]->ol_flags = vol.e[0];
-   rx_pkts[1]->ol_flags = vol.e[1];
-   rx_pkts[2]->ol_flags = vol.e[2];
-   rx_pkts[3]->ol_flags = vol.e[3];
+   rx_pkts[0]->ol_flags = _mm_extract_epi16(vlan0, 0);
+   rx_pkts[1]->ol_flags = _mm_extract_epi16(vlan0, 2);
+   rx_pkts[2]->ol_flags = _mm_extract_epi16(vlan0, 4);
+   rx_pkts[3]->ol_flags = _mm_extract_epi16(vlan0, 6);
 }
 #else
 #define desc_to_olflags_v(desc, rx_pkts) do {} while (0)
@@ -754,7 +762,8 @@ i40e_rx_vec_dev_conf_condition_check(struct rte_eth_dev 
*dev)
 #ifndef RTE_LIBRTE_I40E_RX_OLFLAGS_ENABLE
/* whithout rx ol_flags, no VP flag report */
if (rxmode->hw_vlan_strip != 0 ||
-   rxmode->hw_vlan_extend != 0)
+   rxmode->hw_vlan_extend != 0 ||
+   rxmode->hw_ip_checksum != 0)
return -1;
 #endif

@@ -765,8 +774,7 @@ i40e_rx_vec_dev_conf_condition_check(struct rte_eth_dev 
*dev)
 /* - no csum error report support
 * - no header split support
 */
-   if (rxmode->hw_ip_checksum == 1 ||
-   rxmode->header_split == 1)
+   if (rxmode->header_split == 1)
return -1;

return 0;
-- 
2.5.0

[dpdk-dev] [PATCH 1/2] i40e: Add packet_type metadata in the i40e vPMD

[Jeff Shaw]

From: Damjan Marion 

The ptype is decoded from the rx descriptor and stored
in the packet type field in the mbuf using the same function
as the non-vector driver.

Signed-off-by: Damjan Marion 
Signed-off-by: Jeff Shaw 
---
 drivers/net/i40e/i40e_rxtx.c | 566 +--
 drivers/net/i40e/i40e_rxtx.h | 563 ++
 drivers/net/i40e/i40e_rxtx_vec.c |  16 ++
 3 files changed, 581 insertions(+), 564 deletions(-)

diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index d3cfb98..2903347 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -174,569 +174,6 @@ i40e_get_iee15888_flags(struct rte_mbuf *mb, uint64_t 
qword)
 }
 #endif

-/* For each value it means, datasheet of hardware can tell more details
- *
- * @note: fix i40e_dev_supported_ptypes_get() if any change here.
- */
-static inline uint32_t
-i40e_rxd_pkt_type_mapping(uint8_t ptype)
-{
-   static const uint32_t type_table[UINT8_MAX + 1] __rte_cache_aligned = {
-   /* L2 types */
-   /* [0] reserved */
-   [1] = RTE_PTYPE_L2_ETHER,
-   [2] = RTE_PTYPE_L2_ETHER_TIMESYNC,
-   /* [3] - [5] reserved */
-   [6] = RTE_PTYPE_L2_ETHER_LLDP,
-   /* [7] - [10] reserved */
-   [11] = RTE_PTYPE_L2_ETHER_ARP,
-   /* [12] - [21] reserved */
-
-   /* Non tunneled IPv4 */
-   [22] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_L4_FRAG,
-   [23] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_L4_NONFRAG,
-   [24] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_L4_UDP,
-   /* [25] reserved */
-   [26] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_L4_TCP,
-   [27] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_L4_SCTP,
-   [28] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_L4_ICMP,
-
-   /* IPv4 --> IPv4 */
-   [29] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_TUNNEL_IP |
-   RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_INNER_L4_FRAG,
-   [30] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_TUNNEL_IP |
-   RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_INNER_L4_NONFRAG,
-   [31] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_TUNNEL_IP |
-   RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_INNER_L4_UDP,
-   /* [32] reserved */
-   [33] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_TUNNEL_IP |
-   RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_INNER_L4_TCP,
-   [34] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_TUNNEL_IP |
-   RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_INNER_L4_SCTP,
-   [35] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_TUNNEL_IP |
-   RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_INNER_L4_ICMP,
-
-   /* IPv4 --> IPv6 */
-   [36] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_TUNNEL_IP |
-   RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
-   RTE_PTYPE_INNER_L4_FRAG,
-   [37] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_TUNNEL_IP |
-   RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
-   RTE_PTYPE_INNER_L4_NONFRAG,
-   [38] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_TUNNEL_IP |
-   RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
-   RTE_PTYPE_INNER_L4_UDP,
-   /* [39] reserved */
-   [40] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_TUNNEL_IP |
-   RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
-   RTE_PTYPE_INNER_L4_TCP,
-   [41] = RTE_PTYPE_L2_ETHER | RTE_PTYPE_L3_IPV4_EXT_UNKNOWN |
-   RTE_PTYPE_TUNNEL_IP |
-   RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN |
-   RTE_PTYPE_INNER_L4_SCTP,
-   [42] = RTE_PTYPE_L2_ETHER | 

[dpdk-dev] [PATCH 0/2] Add ptype and xsum handling in i40e rx vpmd

[Jeff Shaw]

Our testing suggests minimal (in some cases zero) impact to core-bound
forwarding throughput as measured by testpmd. Throughput increase is
observed in l3fwd as now the vpmd can be used with hw_ip_checksum
enabled and without needing '--parse-ptype'.

The benefits to applications using this functionality is realized when
Ethernet processing and L3/L4 checksum validation can be skipped.

We hope others can also test performance in their applications while
conducting a review of this series.

Damjan Marion (2):
  i40e: Add packet_type metadata in the i40e vPMD
  i40e: Enable bad checksum flags in i40e vPMD

 drivers/net/i40e/i40e_rxtx.c | 566 +--
 drivers/net/i40e/i40e_rxtx.h | 563 ++
 drivers/net/i40e/i40e_rxtx_vec.c |  64 +++--
 3 files changed, 609 insertions(+), 584 deletions(-)

-- 
2.5.0

[dpdk-dev] [PATCH] vhost: fix segfault on bad descriptor address.

[Yuanhan Liu]

On Wed, Jul 13, 2016 at 08:54:08AM -0700, Rich Lane wrote:
> On Wednesday, July 13, 2016, Yuanhan Liu  
> wrote:
> 
> On Wed, Jul 13, 2016 at 10:34:07AM +0300, Ilya Maximets wrote:
> > This scenario fixed somehow, I agree. But this patch still needed to
> protect
> > vhost from untrusted VM, from malicious or buggy virtio application.
> > Maybe we could change the commit-message and resend this patch as a
> > security enhancement? What do you think?
> 
> Indeed, but I'm a bit concerned about the performance regression found
> by Rich, yet I am not quite sure why it happens, though Rich claimed
> that it seems to be a problem related to compiler.
> 
> 
> The workaround I suggested solves the performance regression. But even if it
> hadn't, this is a security fix that should be merged regardless of the
> performance impact.

Good point. Ilya, would you reword the commit log and resend based on
latest code?

--yliu

[dpdk-dev] [PATCH] doc: fix mailing list address typo.

[Yuanhan Liu]

On Wed, Jul 13, 2016 at 04:51:02AM -0700, Jeff Shaw wrote:
> The correct mailing list dev at dpdk.org, not dev at dpkg.org.

Nice catch!

Reviewed-by: Yuanhan Liu 

--yliu

[dpdk-dev] [PATCH] doc: fix mailing list address typo.

[Mcnamara, John]

> -Original Message-
> From: Shaw, Jeffrey B
> Sent: Wednesday, July 13, 2016 12:51 PM
> To: dev at dpdk.org; Butler, Siobhan A ;
> Mcnamara, John 
> Subject: [PATCH] doc: fix mailing list address typo.
> 
> The correct mailing list dev at dpdk.org, not dev at dpkg.org.
> 
> Signed-off-by: Jeff Shaw 


Acked-by: John McNamara 

[dpdk-dev] [PATCH] vhost: fix segfault on bad descriptor address.

[Ilya Maximets]

On 14.07.2016 04:42, Yuanhan Liu wrote:
> On Wed, Jul 13, 2016 at 08:54:08AM -0700, Rich Lane wrote:
>> On Wednesday, July 13, 2016, Yuanhan Liu  
>> wrote:
>>
>> On Wed, Jul 13, 2016 at 10:34:07AM +0300, Ilya Maximets wrote:
>> > This scenario fixed somehow, I agree. But this patch still needed to
>> protect
>> > vhost from untrusted VM, from malicious or buggy virtio application.
>> > Maybe we could change the commit-message and resend this patch as a
>> > security enhancement? What do you think?
>>
>> Indeed, but I'm a bit concerned about the performance regression found
>> by Rich, yet I am not quite sure why it happens, though Rich claimed
>> that it seems to be a problem related to compiler.
>>
>>
>> The workaround I suggested solves the performance regression. But even if it
>> hadn't, this is a security fix that should be merged regardless of the
>> performance impact.
> 
> Good point. Ilya, would you reword the commit log and resend based on
> latest code?

OK.

[dpdk-dev] [PATCH] lib: move rte_ring read barrier to correct location

[Kuusisaari, Juhamatti]

Hi Konstantin,

> > > > > It is quite safe to move the barrier before DEQUEUE because after
> > > > > the DEQUEUE there is nothing really that we would want to protect
> with a
> > > read barrier.
> > > >
> > > > I don't think so.
> > > > If you remove barrier after DEQUEUE(), that means on systems with
> > > > relaxed memory ordering cons.tail could be updated before DEQUEUE()
> > > > will be finished and producer can overwrite queue entries that were
> not
> > > yet dequeued.
> > > > So if cpu can really do such speculative out of order loads, then we
> > > > do need for  __rte_ring_sc_do_dequeue() something like:
> > > >
> > > > rte_smp_rmb();
> > > > DEQUEUE_PTRS();
> > > > rte_smp_rmb();
> >
> > You have a valid point here, there needs to be a guarantee that cons_tail
> cannot
> > be updated before DEQUEUE is completed. Nevertheless, my point was
> that it is
> > not guaranteed with a read barrier anyway. The implementation has the
> following
> > sequence
> >
> > DEQUEUE_PTRS(); (i.e. READ/LOAD)
> > rte_smp_rmb();
> > ..
> > r->cons.tail = cons_next; (i.e WRITE/STORE)
> >
> > Above read barrier does not guarantee any ordering for the following
> writes/stores.
> > As a guarantee is needed, I think we in fact need to change the read barrier
> on the
> > dequeue to a full barrier, which guarantees the read+write order, as
> follows
> >
> > DEQUEUE_PTRS();
> > rte_smp_mb();
> > ..
> > r->cons.tail = cons_next;
> >
> > If you agree, I can for sure prepare another patch for this issue.
> 
> Hmm, I think for __rte_ring_mc_do_dequeue() we are ok with smp_rmb(),
> as we have to read cons.tail anyway.

Are you certain that this read creates strong enough dependency between
read of cons.tail and the write of it on the mc_do_dequeue()? I think it does 
not 
really create any control dependency there as the next write is not dependent 
of the result of the read. The CPU also knows already the value that will be 
written 
to cons.tail and that value does not depend on the previous read either. The 
CPU 
does not know we are planning to do a spinlock there, so it might do things 
out-of-order without proper dependencies.

> For  __rte_ring_sc_do_dequeue(), I think you right, we might need
> something stronger.
> I don't want to put rte_smp_mb() here as it would cause full HW barrier even
> on machines
> with strong memory order (IA).
> I think that rte_smp_wmb() might be enough here:
> it would force cpu to wait till writes in DEQUEUE_PTRS() are become visible,
> which
> means reads have to be completed too.

In practice I think that rte_smp_wmb() would work fine, even though it is not 
strictly according to the book. Below solution would be my proposal as a fix to 
the 
issue of sc dequeueing (and also to mc dequeueing, if we have the problem of 
CPU 
completely ignoring the spinlock in reality there):

DEQUEUE_PTRS();
..
rte_smp_wmb();
r->cons.tail = cons_next;

--
 Juhamatti

> Another option would be to define a new macro: rte_weak_mb() or so,
> that would be expanded into CB on boxes with strong memory model,
> and to full MB on machines with relaxed ones.
> Interested to hear what ARM and PPC guys think.
> Konstantin
> 
> P.S. Another thing a bit off-topic - for PPC guys:
> As I can see smp_rmb/smp_wmb are just a complier barriers:
> find lib/librte_eal/common/include/arch/ppc_64/ -type f | xargs grep smp_
> lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h:#define
> rte_smp_mb() rte_mb()
> lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h:#define
> rte_smp_wmb() rte_compiler_barrier()
> lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h:#define
> rte_smp_rmb() rte_compiler_barrier()
> My knowledge about PPC architecture is rudimental, but is that really enough?
> 
> >
> > Thanks,
> > --
> >  Juhamatti
> >
> > > > Konstantin
> > > >
> > > > > The read
> > > > > barrier is mapped to a compiler barrier on strong memory model
> > > > > systems and this works fine too as the order of the head,tail
> > > > > updates is still guaranteed on the new location. Even if the problem
> > > > > would be theoretical on most systems, it is worth fixing as the risk 
> > > > > for
> > > problems is very low.
> > > > >
> > > > > --
> > > > >  Juhamatti
> > > > >
> > > > > > Konstantin
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > > > > >
> > > > > > > > > Signed-off-by: Juhamatti Kuusisaari
> > > > > > > > > 
> > > > > > > > > ---
> > > > > > > > >  lib/librte_ring/rte_ring.h | 6 --
> > > > > > > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git a/lib/librte_ring/rte_ring.h
> > > > > > > > > b/lib/librte_ring/rte_ring.h index eb45e41..a923e49 100644
> > > > > > > > > --- a/lib/librte_ring/rte_ring.h
> > > > > > > > > +++ b/lib/librte_ring/rte_ring.h
> > > > > > > > > @@ -662,9 +662,10 @@ __rte_ring_mc_do_dequeue(struct
> > > > > > > > > rte_ring *r,
> > > > > > > > void **obj_table,
> > > > > > > > >   

[dpdk-dev] [PATCH 2/2] app/test: filter out unavailable tests

[Thomas Monjalon]

Some tests can fail to run because they are not compiled.
It has been more visible recently when the PCI test became disabled
in the default configuration because of dependency on libarchive:
PCI autotest:Fail [Not found]

The autotest script catch them and do not count them as an error anymore:
PCI autotest:Skipped [Not Available]

The commands dump_ring and dump_mempool are removed from the autotest list
because they are some internal commands but not some registered tests.
Thus they cannot be parsed as available test commands.

Signed-off-by: Thomas Monjalon 
---
 app/test/autotest_data.py   | 12 
 app/test/autotest_runner.py | 12 ++--
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py
index 1e6b422..c69705e 100644
--- a/app/test/autotest_data.py
+++ b/app/test/autotest_data.py
@@ -99,18 +99,6 @@ parallel_test_group_list = [
 "Func" :   default_autotest,
 "Report" : None,
},
-   {
-"Name" :   "Dump rings",
-"Command" :"dump_ring",
-"Func" :   dump_autotest,
-"Report" : None,
-   },
-   {
-"Name" :   "Dump mempools",
-"Command" :"dump_mempool",
-"Func" :   dump_autotest,
-"Report" : None,
-   },
]
 },
 {
diff --git a/app/test/autotest_runner.py b/app/test/autotest_runner.py
index 291a821..bd99e19 100644
--- a/app/test/autotest_runner.py
+++ b/app/test/autotest_runner.py
@@ -33,7 +33,7 @@

 # The main logic behind running autotests in parallel

-import multiprocessing, sys, pexpect, time, os, StringIO, csv
+import multiprocessing, subprocess, sys, pexpect, re, time, os, StringIO, csv

 # wait for prompt
 def wait_prompt(child):
@@ -105,6 +105,11 @@ def run_test_group(cmdline, test_group):
results.append((0, "Success", "Start %s" % test_group["Prefix"],
time.time() - start_time, startuplog.getvalue(), None))

+   # parse the binary for available test commands
+   binary = cmdline.split()[0]
+   symbols = subprocess.check_output(['nm', binary]).decode('utf-8')
+   avail_cmds = re.findall('test_register_(\w+)', symbols)
+
# run all tests in test group
for test in test_group["Tests"]:

@@ -124,7 +129,10 @@ def run_test_group(cmdline, test_group):
print >>logfile, "\n%s %s\n" % ("-"*20, test["Name"])

# run test function associated with the test
-   result = test["Func"](child, test["Command"])
+   if test["Command"] in avail_cmds:
+   result = test["Func"](child, test["Command"])
+   else:
+   result = (0, "Skipped [Not Available]")

# make a note when the test was finished
end_time = time.time()
-- 
2.7.0

[dpdk-dev] [PATCH 1/2] app/test: rework command registration

[Thomas Monjalon]

The tests are registered with their command name by adding a structure
to a list. The structure of each test was declared in each test file
and passed to the register macro.
This rework generate the structure inside the register macro.

Signed-off-by: Thomas Monjalon 
---
 app/test/test.h  | 17 +--
 app/test/test_acl.c  |  6 +-
 app/test/test_alarm.c|  6 +-
 app/test/test_atomic.c   |  6 +-
 app/test/test_byteorder.c|  6 +-
 app/test/test_cmdline.c  |  6 +-
 app/test/test_common.c   |  6 +-
 app/test/test_cpuflags.c |  6 +-
 app/test/test_cryptodev.c| 41 ++--
 app/test/test_cryptodev_perf.c   | 28 
 app/test/test_cycles.c   |  6 +-
 app/test/test_debug.c|  6 +-
 app/test/test_devargs.c  |  6 +-
 app/test/test_distributor.c  |  6 +-
 app/test/test_distributor_perf.c |  6 +-
 app/test/test_eal_flags.c|  6 +-
 app/test/test_eal_fs.c   |  6 +-
 app/test/test_errno.c|  6 +-
 app/test/test_func_reentrancy.c  |  6 +-
 app/test/test_hash.c |  6 +-
 app/test/test_hash_functions.c   |  6 +-
 app/test/test_hash_multiwriter.c |  8 +--
 app/test/test_hash_perf.c|  6 +-
 app/test/test_hash_scaling.c |  7 +-
 app/test/test_interrupts.c   |  6 +-
 app/test/test_ivshmem.c  |  6 +-
 app/test/test_kni.c  |  6 +-
 app/test/test_kvargs.c   |  6 +-
 app/test/test_link_bonding.c |  6 +-
 app/test/test_link_bonding_mode4.c   |  7 +-
 app/test/test_link_bonding_rssconf.c |  7 +-
 app/test/test_logs.c |  6 +-
 app/test/test_lpm.c  |  6 +-
 app/test/test_lpm6.c |  6 +-
 app/test/test_lpm6_perf.c|  6 +-
 app/test/test_lpm_perf.c |  6 +-
 app/test/test_malloc.c   |  6 +-
 app/test/test_mbuf.c |  6 +-
 app/test/test_memcpy.c   |  6 +-
 app/test/test_memcpy_perf.c  |  6 +-
 app/test/test_memory.c   |  6 +-
 app/test/test_mempool.c  |  6 +-
 app/test/test_mempool_perf.c |  6 +-
 app/test/test_memzone.c  |  6 +-
 app/test/test_meter.c|  6 +-
 app/test/test_mp_secondary.c |  6 +-
 app/test/test_pci.c  |  6 +-
 app/test/test_per_lcore.c|  6 +-
 app/test/test_pmd_perf.c |  6 +-
 app/test/test_pmd_ring.c |  6 +-
 app/test/test_pmd_ring_perf.c|  6 +-
 app/test/test_power.c|  6 +-
 app/test/test_power_acpi_cpufreq.c   |  6 +-
 app/test/test_power_kvm_vm.c |  6 +-
 app/test/test_prefetch.c |  6 +-
 app/test/test_red.c  | 20 +++---
 app/test/test_reorder.c  |  6 +-
 app/test/test_resource.c |  6 +-
 app/test/test_ring.c |  6 +-
 app/test/test_ring_perf.c|  6 +-
 app/test/test_rwlock.c   |  6 +-
 app/test/test_sched.c|  6 +-
 app/test/test_spinlock.c |  6 +-
 app/test/test_string_fns.c   |  6 +-
 app/test/test_table.c|  6 +-
 app/test/test_tailq.c|  6 +-
 app/test/test_thash.c|  6 +-
 app/test/test_timer.c|  6 +-
 app/test/test_timer_perf.c   |  6 +-
 app/test/test_timer_racecond.c   |  6 +-
 app/test/test_version.c  |  6 +-
 71 files changed, 91 insertions(+), 422 deletions(-)

diff --git a/app/test/test.h b/app/test/test.h
index 81828be..467b9c0 100644
--- a/app/test/test.h
+++ b/app/test/test.h
@@ -250,11 +250,16 @@ struct test_command {

 void add_test_command(struct test_command *t);

-#define REGISTER_TEST_COMMAND(t) \
-static void __attribute__((used)) testfn_##t(void);\
-void __attribute__((constructor, used)) testfn_##t(void)\
-{\
-   add_test_command();\
-}
+/* Register a test function with its command string */
+#define REGISTER_TEST_COMMAND(cmd, func) \
+   static struct test_command test_struct_##cmd = { \
+   .command = RTE_STR(cmd), \
+   .callback = func, \
+   }; \
+   static void __attribute__((constructor, used)) \
+   test_register_##cmd(void) \
+   { \
+   add_test_command(_struct_##cmd); \
+   }

 #endif
diff --git a/app/test/test_acl.c b/app/test/test_acl.c
index 2b82790..28955f0 100644
--- a/app/test/test_acl.c
+++ b/app/test/test_acl.c
@@ -1682,8 +1682,4 @@ test_acl(void)
return 0;
 }

-static struct test_command