[dpdk-dev] [PATCH] ip_frag: fix missing dependency on librte_hash

[Panu Matilainen]

Not sure what exactly changed and where, but I've started getting
build failures on Fedora rawhide i386:
lib/librte_ip_frag/ip_frag_internal.c:36:23: fatal error:
rte_jhash.h: No such file or directory
 #include 
   ^
Looking at librte_ip_frag, it clearly depends on librte_hash so
its probably more a question of something commonly masking the issue.

Signed-off-by: Panu Matilainen 
---
 lib/librte_ip_frag/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_ip_frag/Makefile b/lib/librte_ip_frag/Makefile
index e97dfbd..43f8b1e 100644
--- a/lib/librte_ip_frag/Makefile
+++ b/lib/librte_ip_frag/Makefile
@@ -54,6 +54,7 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_IP_FRAG)-include += rte_ip_frag.h

 DEPDIRS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += lib/librte_eal
 DEPDIRS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += lib/librte_ether
+DEPDIRS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += lib/librte_hash
 DEPDIRS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += lib/librte_mempool

-- 
2.7.4

[dpdk-dev] [PATCH 3/3] drivers/net:build support for new tap device driver

[Panu Matilainen]

On 09/15/2016 05:10 PM, Keith Wiles wrote:
> Signed-off-by: Keith Wiles 
> ---
>  config/common_linuxapp | 3 +++
>  drivers/net/Makefile   | 1 +
>  mk/rte.app.mk  | 1 +
>  3 files changed, 5 insertions(+)
>
> diff --git a/config/common_linuxapp b/config/common_linuxapp
> index 2483dfa..704c01c 100644
> --- a/config/common_linuxapp
> +++ b/config/common_linuxapp
> @@ -44,3 +44,6 @@ CONFIG_RTE_LIBRTE_PMD_VHOST=y
>  CONFIG_RTE_LIBRTE_PMD_AF_PACKET=y
>  CONFIG_RTE_LIBRTE_POWER=y
>  CONFIG_RTE_VIRTIO_USER=y
> +CONFIG_RTE_LIBRTE_PMD_TAP=y
> +CONFIG_RTE_PMD_TAP_MAX_QUEUES=32
> +
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index bc93230..b4afa98 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -55,6 +55,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD) += thunderx
>  DIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio
>  DIRS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD) += vmxnet3
>  DIRS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += xenvirt
> +DIRS-$(CONFIG_RTE_LIBRTE_PMD_TAP) += tap
>
>  ifeq ($(CONFIG_RTE_LIBRTE_VHOST),y)
>  DIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += vhost
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 1a0095b..bd1d10f 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -129,6 +129,7 @@ ifeq ($(CONFIG_RTE_LIBRTE_VHOST),y)
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_VHOST)  += -lrte_pmd_vhost
>  endif # $(CONFIG_RTE_LIBRTE_VHOST)
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD)+= -lrte_pmd_vmxnet3_uio
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_TAP)+= -lrte_pmd_tap
>
>  ifeq ($(CONFIG_RTE_LIBRTE_CRYPTODEV),y)
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB)   += -lrte_pmd_aesni_mb
>

Splitting the Makefile and config changes into a separate patch makes no 
sense at all in this case. Just do it in the patch introducing the driver.

And actually, ditto for documentation.

- Panu -

[dpdk-dev] [PATCH v3 02/15] eal/soc: add rte_eal_soc_register/unregister logic

[Panu Matilainen]

On 09/15/2016 05:09 PM, Thomas Monjalon wrote:
> 2016-09-15 15:09, Jan Viktorin:
>> On Thu, 15 Sep 2016 14:00:25 +0100
>> "Hunt, David"  wrote:
>>
 new file mode 100644
 index 000..56135ed
 --- /dev/null
 +++ b/lib/librte_eal/common/eal_common_soc.c
 @@ -0,0 +1,56 @@
 +/*-
 + *   BSD LICENSE
 + *
 + *   Copyright(c) 2016 RehiveTech. All rights reserved.
 + *   All rights reserved.
>>>
>>> Duplicate "All rights reserved"
>>
>> This is present in many source files in DPDK... I don't know why.
>>
>> lib/librte_eal/common/eal_common_pci.c
>> lib/librte_eal/common/eal_common_dev.c
>> ...
>
> It would deserve a dedicated thread to discuss legal sense of these things.
> I'm not a lawyer but I think "All rights reserved." has no real sense.
>

 From a layman (such as myself) perspective it indeed seems totally 
ludicrous in the context of this particular license :) Whether it makes 
more sense to lawyers I wouldn't know, but as for the background: it's 
present in both 2- and 3-clause BSD licenses so *one* of them is 
probably best left alone.

According to https://fedoraproject.org/wiki/Licensing:BSD, in the 
3-clause BSD license "All rights reserved" is on a line of its own. In 
the other variants it follows the copyright holder. So that's probably 
where the duplicates originate from.

- Panu -

[dpdk-dev] [PATCH v3 12/15] ether: extract function eth_dev_get_intr_handle

[Panu Matilainen]

On 09/15/2016 05:05 PM, Thomas Monjalon wrote:
> 2016-09-15 14:02, Hunt, David:
>> On 9/9/2016 9:43 AM, Shreyansh Jain wrote:
>>> +static inline
>>> +struct rte_intr_handle *eth_dev_get_intr_handle(struct rte_eth_dev *dev)
>>> +{
>>> +   if (dev->pci_dev) {
>>> +   return >pci_dev->intr_handle;
>>> +   }
>>> +
>>> +   RTE_VERIFY(0);
>>
>> Rather than RTE_VERIFY(0), might I suggest using rte_panic with a more
>> relevant error message?
>
> RTE_ASSERT is preferred.
> We must stop adding some rte_panic calls except for debug.

+1

It wouldn't hurt to make that a hard rule.

- Panu -

[dpdk-dev] [PATCH] dpdk_procinfo: check for primary process

[Panu Matilainen]

On 09/06/2016 08:12 PM, Maryam Tahhan wrote:
> Add a check to see if the primary process is running and exit gracefully if it
> is not.
>
> Suggested-by: Patrick Kutch 
> Signed-off-by: Maryam Tahhan 
> ---
>  app/proc_info/main.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/app/proc_info/main.c b/app/proc_info/main.c
> index 6dc0bbb..ddc8cf8 100644
> --- a/app/proc_info/main.c
> +++ b/app/proc_info/main.c
> @@ -329,6 +329,11 @@ main(int argc, char **argv)
>   argc -= ret;
>   argv += (ret - 3);
>
> +if (!rte_eal_primary_proc_alive(NULL)) {
> +rte_exit(EXIT_FAILURE, "NO PRIMARY DPDK PROCESS IS 
> RUNNING\n");

I don't think there'a a need to YELL THAT MESSAGE.

- Panu -

[dpdk-dev] [PATCH 0/4] provide man pages for binaries provided by DPDK

[Panu Matilainen]

On 08/30/2016 05:51 PM, Mcnamara, John wrote:
>> -Original Message-
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Christian Ehrhardt
>> Sent: Thursday, August 4, 2016 12:17 PM
>> To: christian.ehrhardt at canonical.com; thomas.monjalon at 6wind.com;
>> dev at dpdk.org
>> Subject: [dpdk-dev] [PATCH 0/4] provide man pages for binaries provided by
>> DPDK
>>
>> Hi,
>> this is about providing manpages for the binaries installed by DPDK.
>> Eventually people using commands expect at least something reasonable
>> avalable behind "man command".
>>
>
> Hi Christian,
>
> Thanks for that. It is really useful and the output looks very good.

Seconded, nice work!

- Panu -

[dpdk-dev] Permanently binding NIC ports with DPDK drivers

[Panu Matilainen]

On 08/25/2016 02:01 PM, Ferruh Yigit wrote:
> On 8/25/2016 5:57 AM, Keren Hochman wrote:
>> Hi,
>> I there a way to permanently bind a nic port when using DPDK drier with
>> kernel < 3.6 ? (In this kernel VFIO driver is not supported)?
>> Thanks, Keren
>>
>
> There was a tool from Panu for this purpose:
> http://dpdk.org/ml/archives/dev/2015-December/029500.html

Yup, but driverctl uses the newer driver_override binding feature which 
is only in kernel >= 3.16 so it's of no use with kernel < 3.6.

- Panu -

[dpdk-dev] [PATCH] vhost: add pmd xstats

[Panu Matilainen]

On 08/24/2016 11:44 AM, Thomas Monjalon wrote:
> 2016-08-24 13:46, Yuanhan Liu:
>> On Tue, Aug 23, 2016 at 12:45:54PM +0300, Panu Matilainen wrote:
>>>>>> Since collecting data of vhost_update_packet_xstats will have some
>>>>>> effect on RX/TX performance, so, Setting compiling switch
>>>>>> CONFIG_RTE_LIBRTE_PMD_VHOST_UPDATE_XSTATS=n by default in the
>>>>> file
>>>>>> config/common_base, if needing xstats data, you can enable it(y).
>>>>>
>>>>> NAK, such things need to be switchable at run-time.
>>>>>
>>>>>   - Panu -
>>>>
>>>> Considering the following reasons using the compiler switch, not
>>>> command-line at run-time.
>>>>
>>>> 1.Similar xstats update functions are always collecting stats data in the
>>>> background when rx/tx are running, such as the physical NIC or virtio,
>>>> which have no switch. Compiler switch for vhost pmd xstats is added
>>>> as a option when performance is viewed as critical factor.
>>>>
>>>> 2. No data structure and API in any layer support the xstats update switch
>>>> at run-time. Common data structure (struct rte_eth_dev_data) has no
>>>> device-specific data member, if implementing enable/disable of vhost_update
>>>> _packet_xstats at run-time, must define a flag(device-specific) in it,
>>>> because the definition of struct vhost_queue in the driver code
>>>> (eth_vhost_rx/eth_vhost_tx processing)is not visible from device 
>>>> perspective.
>>>>
>>>> 3. I tested RX/TX with v1 patch (y) as reference based on Intel(R)
>>>> Xeon(R) CPU E5-2699 v3 @ 2.30GHz, for 64byts packets in burst mode, 32 
>>>> packets
>>>> in one RX/TX processing. Overhead of vhost_update_packet_xstats is less 
>>>> than
>>>> 3% for the rx/tx processing. It looks that vhost_update_packet_xstats has a
>>>> limited effect on performance drop.
>>>
>>> Well, either the performance overhead is acceptable and it should always be
>>> on (like with physical NICs I think). Or it is not. In which case it needs
>>> to be turnable on and off, at run-time. Rebuilding is not an option in the
>>> world of distros.
>>
>> I think the less than 3% overhead is acceptable here, that I agree with
>> Panu we should always keep it on. If someone compains it later that even
>> 3% is too big for them, let's consider to make it be switchable at
>> run-time. Either we could introduce a generic eth API for that, Or
>> just introduce a vhost one if that doesn't make too much sense to other
>> eth drivers.
>
> +1
> It may have sense to introduce a generic run-time option for stats.
>

Yup, sounds good.

- Panu -

[dpdk-dev] [PATCH] vhost: add pmd xstats

[Panu Matilainen]

On 08/23/2016 11:04 AM, Yang, Zhiyong wrote:
> Hi,  Panu:
>
>> -Original Message-----
>> From: Panu Matilainen [mailto:pmatilai at redhat.com]
>> Sent: Monday, August 22, 2016 3:53 PM
>> To: Yang, Zhiyong ; dev at dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH] vhost: add pmd xstats
>>
>> On 08/19/2016 03:16 PM, Zhiyong Yang wrote:
>>> This feature adds vhost pmd extended statistics from per queue
>>> perspective for the application such as OVS etc.
>>>
>>> The statistics counters are based on RFC 2819 and 2863 as follows:
>>>
>>> rx/tx_good_packets
>>> rx/tx_total_bytes
>>> rx/tx_dropped_pkts
>>> rx/tx_broadcast_packets
>>> rx/tx_multicast_packets
>>> rx/tx_ucast_packets
>>> rx/tx_undersize_errors
>>> rx/tx_size_64_packets
>>> rx/tx_size_65_to_127_packets;
>>> rx/tx_size_128_to_255_packets;
>>> rx/tx_size_256_to_511_packets;
>>> rx/tx_size_512_to_1023_packets;
>>> rx/tx_size_1024_to_1522_packets;
>>> rx/tx_1523_to_max_packets;
>>> rx/tx_errors
>>> rx_fragmented_errors
>>> rx_jabber_errors
>>> rx_unknown_protos_packets;
>>>
>>> No API is changed or added.
>>> rte_eth_xstats_get_names() to retrieve what kinds of vhost xstats are
>>> supported,
>>> rte_eth_xstats_get() to retrieve vhost extended statistics,
>>> rte_eth_xstats_reset() to reset vhost extended statistics.
>>>
>>> Since collecting data of vhost_update_packet_xstats will have some
>>> effect on RX/TX performance, so, Setting compiling switch
>>> CONFIG_RTE_LIBRTE_PMD_VHOST_UPDATE_XSTATS=n by default in the
>> file
>>> config/common_base, if needing xstats data, you can enable it(y).
>>
>> NAK, such things need to be switchable at run-time.
>>
>>  - Panu -
>
> Considering the following reasons using the compiler switch, not
> command-line at run-time.
>
> 1.Similar xstats update functions are always collecting stats data in the
> background when rx/tx are running, such as the physical NIC or virtio,
> which have no switch. Compiler switch for vhost pmd xstats is added
> as a option when performance is viewed as critical factor.
>
> 2. No data structure and API in any layer support the xstats update switch
> at run-time. Common data structure (struct rte_eth_dev_data) has no
> device-specific data member, if implementing enable/disable of vhost_update
> _packet_xstats at run-time, must define a flag(device-specific) in it,
> because the definition of struct vhost_queue in the driver code
> (eth_vhost_rx/eth_vhost_tx processing)is not visible from device perspective.
>
> 3. I tested RX/TX with v1 patch (y) as reference based on Intel(R)
> Xeon(R) CPU E5-2699 v3 @ 2.30GHz, for 64byts packets in burst mode, 32 packets
> in one RX/TX processing. Overhead of vhost_update_packet_xstats is less than
> 3% for the rx/tx processing. It looks that vhost_update_packet_xstats has a
> limited effect on performance drop.

Well, either the performance overhead is acceptable and it should always 
be on (like with physical NICs I think). Or it is not. In which case it 
needs to be turnable on and off, at run-time. Rebuilding is not an 
option in the world of distros.

- Panu -

>
>-zhiyong-
>

[dpdk-dev] [PATCH] vhost: add pmd xstats

[Panu Matilainen]

On 08/19/2016 03:16 PM, Zhiyong Yang wrote:
> This feature adds vhost pmd extended statistics from per queue perspective
> for the application such as OVS etc.
>
> The statistics counters are based on RFC 2819 and 2863 as follows:
>
> rx/tx_good_packets
> rx/tx_total_bytes
> rx/tx_dropped_pkts
> rx/tx_broadcast_packets
> rx/tx_multicast_packets
> rx/tx_ucast_packets
> rx/tx_undersize_errors
> rx/tx_size_64_packets
> rx/tx_size_65_to_127_packets;
> rx/tx_size_128_to_255_packets;
> rx/tx_size_256_to_511_packets;
> rx/tx_size_512_to_1023_packets;
> rx/tx_size_1024_to_1522_packets;
> rx/tx_1523_to_max_packets;
> rx/tx_errors
> rx_fragmented_errors
> rx_jabber_errors
> rx_unknown_protos_packets;
>
> No API is changed or added.
> rte_eth_xstats_get_names() to retrieve what kinds of vhost xstats are
> supported,
> rte_eth_xstats_get() to retrieve vhost extended statistics,
> rte_eth_xstats_reset() to reset vhost extended statistics.
>
> Since collecting data of vhost_update_packet_xstats will have some effect
> on RX/TX performance, so, Setting compiling switch
> CONFIG_RTE_LIBRTE_PMD_VHOST_UPDATE_XSTATS=n by default in the file
> config/common_base, if needing xstats data, you can enable it(y).

NAK, such things need to be switchable at run-time.

- Panu -

[dpdk-dev] DPDK Stable Releases and Long Term Support

[Panu Matilainen]

[ Yes I'm late to this party, apologies for missing the first round of 
discussion ]

On 07/28/2016 03:33 PM, Mcnamara, John wrote:
>
> This document sets out the guidelines for DPDK Stable Releases and Long Term
> Support releases (LTS) based on the initial RFC and comments:
> http://dpdk.org/ml/archives/dev/2016-June/040256.html.
>
> In particular it incorporates suggestions for a Stable Release structure as
> well as a Long Term Support release.
>
>
> Introduction
> 
>
> The purpose of the DPDK Stable Releases will be to maintain releases of DPDK
> with backported fixes over an extended period of time. This will provide
> downstream consumers of DPDK with a stable target on which to base
> applications or packages.
>
> The Long Term Support release (LTS) will be a designation applied to a Stable
> Release to indicate longer support.
>
>
> Stable Releases
> ---
>
> Any major release of DPDK can be designated as a Stable Release if a
> maintainer volunteers to maintain it.
>
> A Stable Release will be used to backport fixes from a N release back to a N-1
> release, for example, from 16.11 to 16.07.
>
> The duration of a stable release should be one complete release cycle. It can
> be longer, up to 1 year, if a maintainer continues to support the stable
> branch, or if users supply backported fixes, however the explicit commitment
> should be for one release cycle.
>
> The release cadence can be determined by the maintainer based on the number of
> bugfixes and the criticality of the bugs. However, releases should be
> coordinated with the validation engineers to ensure that a tagged release has
> been tested.
>
>
> LTS Release
> ---
>
> A stable release can be designated as an LTS release based on community
> agreement and a commitment from a maintainer. An LTS release will have a
> maintenance duration of 2 years.
>
> It is anticipated that there should be at least 4 releases per year of the LTS
> or approximately 1 every 3 months. However, the cadence can be shorter or
> longer depending on the number and criticality of the backported
> fixes. Releases should be coordinated with the validation engineers to ensure
> that a tagged release has been tested.
>
>
> Initial Stable Release
> --
>
> The initial DPDK Stable Release will be 16.07. It will be viewed as a trial of
> the Stable Release/LTS policy to determine what are the best working practices
> for DPDK.
>
> The maintainer for the initial release will be Yuanhan Liu
> . It is hoped that other community members 
> will
> volunteer as maintainers for other Stable Releases.
>
> The initial targeted release for LTS is proposed to be 16.11 based on the
> results of the work carried out on the 16.07 Stable Release.
>
> A list has been set up for Stable Release/LTS specific discussions:
> . This address can also be used for CCing maintainers on 
> bug
> fix submissions.
>
>
> What changes should be backported
> -
>
> The backporting should be limited to bug fixes.
>
> Features should not be backported to stable releases. It may be acceptable, in
> limited cases, to back port features for the LTS release where:
>
> * There is a justifiable use case (for example a new PMD).
> * The change is non-invasive.

A new PMD also would not touch existing code, which makes it a 
low-to-no-risk thing. Ditto for, say, new command line tool or an example.

> * The work of preparing the backport is done by the proposer.
> * There is support within the community.
>
>
> Testing
> ---
>
> Stable and LTS releases should be tested before release/tagging.
>
> Intel will provide validation engineers to test the 16.07 Stable Release and
> the initial LTS tree. Other community members should provide testing for other
> stable releases.
>
> The validation will consist of compilation testing on the range of OSes
> supported by the master release and functional/performance testing on the
> current major/LTS release of the following OSes:
>
> * Ubuntu
> * RHEL
> * SuSE
> * FreeBSD
>
>
> Releasing
> -
>
> A Stable Release will be released by:
>
> * Tagging the release with YY.MM.nn (year, month, number) or similar.
> * Uploading a tarball of the release to dpdk.org.
> * Sending an announcement to the  list.
>
>
> ABI
> ---
>
> The Stable Release should not be seen as a way of breaking or circumventing
> the DPDK ABI policy.

I find this a strange thing to say about a stable/LTS release ABI. I had 
read the originating thread before seeing this, but it still made me go 
"Huh?" for several seconds. The problem perhaps being, the rest of the 
document addresses stable/LTS releases, but this statement speaks about 
normal development work going on elsewhere.

The earlier version had a mention about ABI/API breakage related to 
things what not to backport but that's entirely gone here. Given how 
important ABI + API stability is for stable/LTS releases, I think it 
deserves a special mention 

[dpdk-dev] [PATCH] ivshmem: remove integration in dpdk

[Panu Matilainen]

On 07/29/2016 03:28 PM, David Marchand wrote:
> Following discussions on the mailing list [1] and since nobody stood up to
> implement the necessary cleanups, here is the ivshmem integration removal.
>
> There is not much to say about this patch, a lot of code is being removed.
> The default configuration file for packet_ordering example is replaced with
> the "native" x86 file.
> The only tricky part is in eal_memory with the memseg index stuff.
>
> More cleanups can be done after this but will come in subsequent patchsets.
>
> [1]: http://dpdk.org/ml/archives/dev/2016-June/040844.html
>
> Signed-off-by: David Marchand 
> ---
>  MAINTAINERS  |   8 -
>  app/test/Makefile|   1 -
>  app/test/autotest_data.py|   6 -
>  app/test/test.c  |   3 -
>  app/test/test.h  |   1 -
>  app/test/test_ivshmem.c  | 433 
>  config/defconfig_arm64-armv8a-linuxapp-gcc   |   1 -
>  config/defconfig_x86_64-ivshmem-linuxapp-gcc |  49 --
>  config/defconfig_x86_64-ivshmem-linuxapp-icc |  49 --
>  doc/api/doxy-api-index.md|   1 -
>  doc/api/doxy-api.conf|   1 -
>  doc/api/examples.dox |   2 -
>  doc/guides/linux_gsg/build_dpdk.rst  |   2 +-
>  doc/guides/linux_gsg/quick_start.rst |  14 +-
>  doc/guides/prog_guide/img/ivshmem.png| Bin 44920 -> 0 bytes
>  doc/guides/prog_guide/index.rst  |   1 -
>  doc/guides/prog_guide/ivshmem_lib.rst| 160 -
>  doc/guides/prog_guide/source_org.rst |   1 -
>  doc/guides/rel_notes/deprecation.rst |   3 -
>  doc/guides/rel_notes/release_16_11.rst   |   3 +
>  examples/Makefile|   1 -
>  examples/l2fwd-ivshmem/Makefile  |  43 --
>  examples/l2fwd-ivshmem/guest/Makefile|  50 --
>  examples/l2fwd-ivshmem/guest/guest.c | 452 -
>  examples/l2fwd-ivshmem/host/Makefile |  50 --
>  examples/l2fwd-ivshmem/host/host.c   | 895 -
>  examples/l2fwd-ivshmem/include/common.h  | 111 
>  examples/packet_ordering/Makefile|   2 +-
>  lib/Makefile |   1 -
>  lib/librte_eal/common/eal_common_memzone.c   |  12 -
>  lib/librte_eal/common/eal_private.h  |  22 -
>  lib/librte_eal/common/include/rte_memory.h   |   3 -
>  lib/librte_eal/common/include/rte_memzone.h  |   7 +-
>  lib/librte_eal/common/malloc_heap.c  |   8 -
>  lib/librte_eal/linuxapp/eal/Makefile |   9 -
>  lib/librte_eal/linuxapp/eal/eal.c|  10 -
>  lib/librte_eal/linuxapp/eal/eal_ivshmem.c| 954 
> ---
>  lib/librte_eal/linuxapp/eal/eal_memory.c |  30 +-
>  lib/librte_ivshmem/Makefile  |  54 --
>  lib/librte_ivshmem/rte_ivshmem.c | 919 --
>  lib/librte_ivshmem/rte_ivshmem.h | 165 -
>  lib/librte_ivshmem/rte_ivshmem_version.map   |  12 -
>  mk/rte.app.mk|   1 -
>  43 files changed, 13 insertions(+), 4537 deletions(-)
>  delete mode 100644 app/test/test_ivshmem.c
>  delete mode 100644 config/defconfig_x86_64-ivshmem-linuxapp-gcc
>  delete mode 100644 config/defconfig_x86_64-ivshmem-linuxapp-icc
>  delete mode 100644 doc/guides/prog_guide/img/ivshmem.png
>  delete mode 100644 doc/guides/prog_guide/ivshmem_lib.rst
>  delete mode 100644 examples/l2fwd-ivshmem/Makefile
>  delete mode 100644 examples/l2fwd-ivshmem/guest/Makefile
>  delete mode 100644 examples/l2fwd-ivshmem/guest/guest.c
>  delete mode 100644 examples/l2fwd-ivshmem/host/Makefile
>  delete mode 100644 examples/l2fwd-ivshmem/host/host.c
>  delete mode 100644 examples/l2fwd-ivshmem/include/common.h
>  delete mode 100644 lib/librte_eal/linuxapp/eal/eal_ivshmem.c
>  delete mode 100644 lib/librte_ivshmem/Makefile
>  delete mode 100644 lib/librte_ivshmem/rte_ivshmem.c
>  delete mode 100644 lib/librte_ivshmem/rte_ivshmem.h
>  delete mode 100644 lib/librte_ivshmem/rte_ivshmem_version.map
>
[...]

Ooh, what a nice "welcome back from vacation" message in my inbox :)
FWIW,

Acked-by: Panu Matilainen 

- Panu -

[dpdk-dev] [PATCH v2] mempool: rename functions with confusing names

[Panu Matilainen]

On 06/30/2016 03:00 PM, Thomas Monjalon wrote:
> 2016-06-29 17:27, Bruce Richardson:
>> Fix this by introducing two new functions to replace the old ones,
>> * rte_mempool_avail_count to replace rte_mempool_count
>> * rte_mempool_in_use_count to replace rte_mempool_free_count
>
> This patch needs to be rebased please.
>
>> --- a/lib/librte_mempool/rte_mempool_version.map
>> +++ b/lib/librte_mempool/rte_mempool_version.map
>> @@ -32,5 +32,6 @@ DPDK_16.07 {
>>  rte_mempool_populate_virt;
>>  rte_mempool_register_ops;
>>  rte_mempool_set_ops_byname;
>> +rte_mempool_avail_count;
>
> The "in_use_count" function is missing.

Its missing because the function is static inline. If you ask me, this 
would be as fine time as any to remove that inlining ;)

- Panu -

> Please keep alphabetical order.
>

[dpdk-dev] [PATCH v3 00/20] vhost ABI/API refactoring

[Panu Matilainen]

On 06/30/2016 10:57 AM, Yuanhan Liu wrote:
> On Thu, Jun 30, 2016 at 10:39:45AM +0300, Panu Matilainen wrote:
>> On 06/07/2016 06:51 AM, Yuanhan Liu wrote:
>>> v3: - adapted the new vhost ABI/API changes to tep_term example, to make
>>>  sure not break build at least.
>>>- bumped the ABI version to 3
>>>
>>> NOTE: I created a branch at dpdk.org [0] for more conveinient testing:
>>>
>>>[0]: git://dpdk.org/next/dpdk-next-virtio for-testing
>>>
>>>
>>> Every time we introduce a new feature to vhost, we are likely to break
>>> ABI. Moreover, some cleanups (such as the one from Ilya to remove vec_buf
>> >from vhost_virtqueue struct) also break ABI.
>>>
>>> This patch set is meant to resolve above issue ultimately, by hiding
>>> virtio_net structure (as well as few others) internaly, and export the
>>> virtio_net dev strut to applications by a number, vid, like the way
>>> kernel exposes an fd to user space.
>>>
>>> Back to the patch set, the first part of this set makes some changes to
>>> vhost example, vhost-pmd and vhost, bit by bit, to remove the dependence
>>> to "virtio_net" struct. And then do the final change to make the current
>>> APIs to adapt to using "vid".
>>>
>>> After that, "vrtio_net_device_ops" is the only left open struct that an
>>> application can acces, therefore, it's the only place that might introduce
>>> potential ABI breakage in future for extension. Hence, I made few more
>>> (5) space reservation, to make sure we will not break ABI for a long time,
>>> and hopefuly, forever.
>>
>> Been intending to say this for a while but seems I never actually got around
>> to do so:
>>
>> This is a really fine example of how to refactor an API against constant ABI
>> breakages, thank you Yuanhan!
>
> Panu, thanks!
>
>> Exported structs are one of the biggest
>> obstacles in keeping a stable ABI while adding new features, and while its
>> not always possible to hide everything to this extent, the damage (erm,
>> exposure) can usually be considerably limited by careful API design.
>
> Agreed.
>
>> Since the first and the foremost objection against doing this in the DPDK
>> context is always "but performance!", I'm curious as to what sort of numbers
>> you're getting with the new API vs the old one? I'm really hoping other
>> libraries would follow suit after seeing that its possible to provide a
>> future-proof API/ABI without sacrificing performance :)
>
> From my (limited) test, nope, I see no performance drop at all, not even a
> little.

Awesome!

With that, hopefully others will see the light and follow its example. 
If nothing else, they ought to get a bit envious when you can add 
features left and right without ever having to wait for API/ABI break 
periods etc ;)

- Panu -

>
>   --yliu
>

[dpdk-dev] [PATCH v3 00/20] vhost ABI/API refactoring

[Panu Matilainen]

On 06/07/2016 06:51 AM, Yuanhan Liu wrote:
> v3: - adapted the new vhost ABI/API changes to tep_term example, to make
>   sure not break build at least.
> - bumped the ABI version to 3
>
> NOTE: I created a branch at dpdk.org [0] for more conveinient testing:
>
> [0]: git://dpdk.org/next/dpdk-next-virtio for-testing
>
>
> Every time we introduce a new feature to vhost, we are likely to break
> ABI. Moreover, some cleanups (such as the one from Ilya to remove vec_buf
> from vhost_virtqueue struct) also break ABI.
>
> This patch set is meant to resolve above issue ultimately, by hiding
> virtio_net structure (as well as few others) internaly, and export the
> virtio_net dev strut to applications by a number, vid, like the way
> kernel exposes an fd to user space.
>
> Back to the patch set, the first part of this set makes some changes to
> vhost example, vhost-pmd and vhost, bit by bit, to remove the dependence
> to "virtio_net" struct. And then do the final change to make the current
> APIs to adapt to using "vid".
>
> After that, "vrtio_net_device_ops" is the only left open struct that an
> application can acces, therefore, it's the only place that might introduce
> potential ABI breakage in future for extension. Hence, I made few more
> (5) space reservation, to make sure we will not break ABI for a long time,
> and hopefuly, forever.

Been intending to say this for a while but seems I never actually got 
around to do so:

This is a really fine example of how to refactor an API against constant 
ABI breakages, thank you Yuanhan! Exported structs are one of the 
biggest obstacles in keeping a stable ABI while adding new features, and 
while its not always possible to hide everything to this extent, the 
damage (erm, exposure) can usually be considerably limited by careful 
API design.

Since the first and the foremost objection against doing this in the 
DPDK context is always "but performance!", I'm curious as to what sort 
of numbers you're getting with the new API vs the old one? I'm really 
hoping other libraries would follow suit after seeing that its possible 
to provide a future-proof API/ABI without sacrificing performance :)

Thanks again,

- Panu -

[dpdk-dev] [PATCH v2 5/7] mk: fix external dependencies of crypto drivers

[Panu Matilainen]

On 06/26/2016 07:42 PM, Thomas Monjalon wrote:
> When linking drivers as shared libraries, the dependencies need
> to be marked as DT_NEEDED entries.
>
> The crypto dependencies (libsso and libIPSec) are static libraries.
> To make them linked in the shared PMDs, the code must relocatable:
> - libIPSec_MB.a must be built with -fPIC
> - libsso_kasumi.a must be built with KASUMI_CFLAGS=-DKASUMI_C
>
> Fixes: 924e84f87306 ("aesni_mb: add driver for multi buffer based crypto")
> Fixes: eec136f3c54f ("aesni_gcm: add driver for AES-GCM crypto operations")
> Fixes: 3aafc423cf4d ("snow3g: add driver for SNOW 3G library")
> Fixes: 2773c86d061a ("crypto/kasumi: add driver for KASUMI library")
>
> Signed-off-by: Thomas Monjalon 
> ---
>  drivers/crypto/aesni_gcm/Makefile | 3 ++-
>  drivers/crypto/aesni_mb/Makefile  | 3 ++-
>  drivers/crypto/kasumi/Makefile| 3 ++-
>  drivers/crypto/snow3g/Makefile| 3 ++-
>  4 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/crypto/aesni_gcm/Makefile 
> b/drivers/crypto/aesni_gcm/Makefile
> index ab5d2ed..5898cae 100644
> --- a/drivers/crypto/aesni_gcm/Makefile
> +++ b/drivers/crypto/aesni_gcm/Makefile
> @@ -49,9 +49,10 @@ LIBABIVER := 1
>  # versioning export map
>  EXPORT_MAP := rte_pmd_aesni_gcm_version.map
>
> -# external library include paths
> +# external library dependencies
>  CFLAGS += -I$(AESNI_MULTI_BUFFER_LIB_PATH)
>  CFLAGS += -I$(AESNI_MULTI_BUFFER_LIB_PATH)/include
> +LDLIBS += -L$(AESNI_MULTI_BUFFER_LIB_PATH) -lIPSec_MB
>  LDLIBS += -lcrypto
>
>  # library source files
> diff --git a/drivers/crypto/aesni_mb/Makefile 
> b/drivers/crypto/aesni_mb/Makefile
> index 348a8bd..d3994cc 100644
> --- a/drivers/crypto/aesni_mb/Makefile
> +++ b/drivers/crypto/aesni_mb/Makefile
> @@ -49,9 +49,10 @@ LIBABIVER := 1
>  # versioning export map
>  EXPORT_MAP := rte_pmd_aesni_version.map
>
> -# external library include paths
> +# external library dependencies
>  CFLAGS += -I$(AESNI_MULTI_BUFFER_LIB_PATH)
>  CFLAGS += -I$(AESNI_MULTI_BUFFER_LIB_PATH)/include
> +LDLIBS += -L$(AESNI_MULTI_BUFFER_LIB_PATH) -lIPSec_MB
>
>  # library source files
>  SRCS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB) += rte_aesni_mb_pmd.c
> diff --git a/drivers/crypto/kasumi/Makefile b/drivers/crypto/kasumi/Makefile
> index 72b1ca4..9fb0be8 100644
> --- a/drivers/crypto/kasumi/Makefile
> +++ b/drivers/crypto/kasumi/Makefile
> @@ -49,10 +49,11 @@ LIBABIVER := 1
>  # versioning export map
>  EXPORT_MAP := rte_pmd_kasumi_version.map
>
> -# external library include paths
> +# external library dependencies
>  CFLAGS += -I$(LIBSSO_KASUMI_PATH)
>  CFLAGS += -I$(LIBSSO_KASUMI_PATH)/include
>  CFLAGS += -I$(LIBSSO_KASUMI_PATH)/build
> +LDLIBS += -L$(LIBSSO_KASUMI_PATH)/build -lsso_kasumi
>
>  # library source files
>  SRCS-$(CONFIG_RTE_LIBRTE_PMD_KASUMI) += rte_kasumi_pmd.c
> diff --git a/drivers/crypto/snow3g/Makefile b/drivers/crypto/snow3g/Makefile
> index 582907f..bea6760 100644
> --- a/drivers/crypto/snow3g/Makefile
> +++ b/drivers/crypto/snow3g/Makefile
> @@ -49,10 +49,11 @@ LIBABIVER := 1
>  # versioning export map
>  EXPORT_MAP := rte_pmd_snow3g_version.map
>
> -# external library include paths
> +# external library dependencies
>  CFLAGS += -I$(LIBSSO_SNOW3G_PATH)
>  CFLAGS += -I$(LIBSSO_SNOW3G_PATH)/include
>  CFLAGS += -I$(LIBSSO_SNOW3G_PATH)/build
> +LDLIBS += -L$(LIBSSO_SNOW3G_PATH)/build -lsso_snow3g
>
>  # library source files
>  SRCS-$(CONFIG_RTE_LIBRTE_PMD_SNOW3G) += rte_snow3g_pmd.c
>

LDLIBS should only contain the actual libraries, everything else 
including -L  should go to LDFLAGS. The rule is not honored in 
dpdk at all currently and nothing depends on that, so it doesn't really 
matter. Just thought to mention it since this adds several new instances 
of mixed usage.

- Panu -

[dpdk-dev] [PATCH] port: fix build when KNI support is not enabled

[Panu Matilainen]

Commit 9fc37d1c071c is missing a conditional in the dependencies,
causing builds to fail when KNI is not enabled:
== Build lib/librte_port
  LD librte_port.so.3
/usr/bin/ld: cannot find -lrte_kni
collect2: error: ld returned 1 exit status

Fixes: 9fc37d1c071c ("port: support KNI")

Signed-off-by: Panu Matilainen 
---
 lib/librte_port/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_port/Makefile b/lib/librte_port/Makefile
index 0fc929b..3d84a0e 100644
--- a/lib/librte_port/Makefile
+++ b/lib/librte_port/Makefile
@@ -82,6 +82,8 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_mempool
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_ip_frag
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_sched
+ifeq ($(CONFIG_RTE_LIBRTE_KNI),y)
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PORT) += lib/librte_kni
+endif

 include $(RTE_SDK)/mk/rte.lib.mk
-- 
2.5.5

[dpdk-dev] [PATCH] mem: skip memory locking on failure

[Panu Matilainen]

On 06/14/2016 05:12 PM, Olivier MATZ wrote:
> Hi Panu,
>
> On 06/14/2016 03:21 PM, Panu Matilainen wrote:
>> On 06/13/2016 01:26 PM, Olivier Matz wrote:
>>> Since recently [1], it is not possible to run the dpdk with user
>>> (non-root) privileges and the --no-huge option. This is because the eal
>>> layer tries to lock the memory. Using locked memory is mandatory for
>>> physical devices because they reference physical addresses.
>>>
>>> But a user may want to start the dpdk without locked memory, because he
>>> does not have the permission to do so, and/or does not have this need.
>>>
>>> Moreover, the option --no-huge is still not functional today since the
>>> physical memory address is not properly filled, so the initial patch is
>>> not really useful.
>>>
>>> This commit fixes this issue by retrying the mmap() wihout the
>>> MAP_LOCKED flag if the first mmap() failed.
>>>
>>> [1] http://www.dpdk.org/ml/archives/dev/2016-May/039404.html
>>>
>>> Fixes: 593a084afc2b ("mem: lock pages when not using hugepages")
>>> Reported-by: Panu Matilainen 
>>> Signed-off-by: Olivier Matz 
>>> ---
>>>  lib/librte_eal/linuxapp/eal/eal_memory.c | 9 +
>>>  1 file changed, 9 insertions(+)
>>>
>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> b/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> index 79d1d2d..08692d1 100644
>>> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> @@ -1075,6 +1075,15 @@ rte_eal_hugepage_init(void)
>>>  if (internal_config.no_hugetlbfs) {
>>>  addr = mmap(NULL, internal_config.memory, PROT_READ |
>>> PROT_WRITE,
>>>  MAP_LOCKED | MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
>>> +/* retry without MAP_LOCKED */
>>> +if (addr == MAP_FAILED && errno == EAGAIN) {
>>> +addr = mmap(NULL, internal_config.memory,
>>> +PROT_READ | PROT_WRITE,
>>> +MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
>>> +if (addr != MAP_FAILED)
>>> +RTE_LOG(NOTICE, EAL,
>>> +"Cannot lock memory: don't use physical
>>> devices\n");
>>> +}
>>>  if (addr == MAP_FAILED) {
>>>  RTE_LOG(ERR, EAL, "%s: mmap() failed: %s\n", __func__,
>>>  strerror(errno));
>>>
>>
>> I'm not really that familiar with dpdk memory usage, but gut feeling
>> says such a thing needs to be explicit - either you explicitly ask for
>> memory that doesn't need to be locked, or this simply fails with no
>> retries. Or something like that. But "maybe I did, maybe I didn't"
>> doesn't seem like very good API semantics to me :)
>
> Yes, you're right. Anyway as this commit is not useful today,
> it would be better to revert it.

I suppose you mean revert the memlock commit, ie this?

commit 593a084afc2b441895aeca78a2c4465e450d0ef5
Author: Olivier Matz 
Date:   Wed May 18 13:04:42 2016 +0200

 mem: lock pages when not using hugepages

Reverting that would help in the sense that then we could make the 
test-suite runnable by regular users (I've some patches for this), and 
once that is in place it would sort of force dealing with the issue one 
way or the other in future work in this area :)

>
>> Are there actual plans to make --no-huge work with real devices?
>
> I think this is something that could be part of the memory
> rework referenced by Thomas:
> http://dpdk.org/ml/archives/dev/2016-April/037444.html
>
> I don't know if it's planified yet.
>
>
>> If not then documenting --no-huge to imply unlocked memory is one
>> option I guess.
>
> There is already some words in the known issues:
> http://dpdk.org/doc/guides/rel_notes/known_issues.html?highlight=known%20issues#pmd-does-not-work-with-no-huge-eal-command-line-parameter

Right, so it wouldn't be a regression at least.

- Panu -

>
> Maybe we could add something somewhere else, but I did not find
> any doc referencing eal options. Only a guide for testpmd here:
> http://dpdk.org/doc/guides/testpmd_app_ug/run_app.html#eal-command-line-options
>
>
> John, maybe you have an idea?
>
> Thanks
> Olivier
>

[dpdk-dev] [PATCH 1/3] mk: fix librte_pipeline dependency list truncation

[Panu Matilainen]

On 06/21/2016 01:58 PM, Dumitrescu, Cristian wrote:
>
>
>> -Original Message-----
>> From: Panu Matilainen [mailto:pmatilai at redhat.com]
>> Sent: Tuesday, June 21, 2016 11:45 AM
>> To: Richardson, Bruce 
>> Cc: Dumitrescu, Cristian ; dev at dpdk.org;
>> christian.ehrhardt at canonical.com; thomas.monjalon at 6wind.com
>> Subject: Re: [dpdk-dev] [PATCH 1/3] mk: fix librte_pipeline dependency list
>> truncation
>>
>> On 06/21/2016 01:31 PM, Bruce Richardson wrote:
>>> On Tue, Jun 21, 2016 at 01:25:52PM +0300, Panu Matilainen wrote:
>>>> On 06/21/2016 01:01 PM, Dumitrescu, Cristian wrote:
>>>>>
>>>>>
>>>>>> -Original Message-
>>>>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Panu
>> Matilainen
>>>>>> Sent: Tuesday, June 21, 2016 9:12 AM
>>>>>> To: dev at dpdk.org
>>>>>> Cc: christian.ehrhardt at canonical.com; thomas.monjalon at 6wind.com
>>>>>> Subject: [dpdk-dev] [PATCH 1/3] mk: fix librte_pipeline dependency list
>>>>>> truncation
>>>>>>
>>>>>> In other libraries, dependency list is always appended to, but
>>>>>> in commit 6cbf4f75e059 it with an assignment. This causes the
>>>>>> librte_eal dependency added in commit 6cbf4f75e059 to get discarded,
>>>>>> resulting in missing dependency on librte_eal.
>>>>>>
>>>>>> Fixes: b3688bee81a8 ("pipeline: new packet framework logic")
>>>>>> Fixes: 6cbf4f75e059 ("mk: fix missing internal dependencies")
>>>>>>
>>>>>> Signed-off-by: Panu Matilainen 
>>>>>> ---
>>>>>> lib/librte_pipeline/Makefile | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/lib/librte_pipeline/Makefile b/lib/librte_pipeline/Makefile
>>>>>> index 95387aa..a8f3128 100644
>>>>>> --- a/lib/librte_pipeline/Makefile
>>>>>> +++ b/lib/librte_pipeline/Makefile
>>>>>> @@ -53,7 +53,7 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_PIPELINE)-
>> include +=
>>>>>> rte_pipeline.h
>>>>>>
>>>>>> # this lib depends upon:
>>>>>> DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_eal
>>>>>> -DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) := lib/librte_table
>>>>>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_table
>>>>>> DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_port
>>>>>>
>>>>>> include $(RTE_SDK)/mk/rte.lib.mk
>>>>>> --
>>>>>> 2.5.5
>>>>>
>>>>>
>>>>> In release 16.4, EAL was missing from the dependency list, now it is
>> added. The librte_pipeline uses rte_malloc, therefore it depends on
>> librte_eal being present.
>>>>>
>>>>> In the Makefile of the other Packet Framework libraries (librte_port,
>> librte_table), it looks like the first dependency in the list is EAL, which 
>> is listed
>> with the assignment operator, followed by others that are listed with the
>> append operator:
>>>>>   DEPDIRS-$(CONFIG_RTE_LIBRTE_XYZ) := lib/librte_eal
>>>>>   DEPDIRS-$(CONFIG_RTE_LIBRTE_XYZ) += lib/librte_some other lib
>>>>>
>>>>> Therefore, at least for cosmetic reasons, we should probably do the
>> same in librte_pipeline, which requires changing both the librte_eal and the
>> librte_table lines as below:
>>>>>   DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) := lib/librte_eal
>>>>>   DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_table
>>>>>   DEPDIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += lib/librte_port
>>>>
>>>> Ah, didn't notice those because the assignment is first of the
>> dependencies.
>>>>
>>>>>
>>>>> However, some other libraries e.g. librte_lpm simply add the EAL
>> dependency using the append operator:
>>>>>   DEPDIRS-$(CONFIG_RTE_LIBRTE_LPM) += lib/librte_eal
>>>>>
>>>>> To be honest, I need to refresh my knowledge on make, I don't
>> remember right now when we should use the assignment and when the
>> append. Do we need to use the assign for first dependency (EAL) and
>> append for others or should we use append everywhere?
>>>>
>>>> At least in automake, you need to as

[dpdk-dev] [PATCH v2] i40e: modify the meaning of single VLAN type

[Panu Matilainen]

On 06/21/2016 01:29 PM, Bruce Richardson wrote:
> On Mon, Jun 13, 2016 at 04:03:32PM +0800, Beilei Xing wrote:
>> In current i40e codebase, if single VLAN header is added in a packet,
>> it's treated as inner VLAN. Generally, a single VLAN header is
>> treated as the outer VLAN header. So change corresponding register
>> for single VLAN.
>> At the meanwhile, change the meanings of inner VLAN and outer VLAN.
>>
>> Signed-off-by: Beilei Xing 
>
> This patch changes the ABI, since an app written to the original API as 
> specified
> e.g. to set a single vlan header, would no longer work with this change.
> Therefore, even though the original behaviour was inconsistent with other 
> drivers
> it may still need to be preserved.
>
> I'm thinking that we may need to provide appropriately versioned copies of the
> vlan_offload_set and vlan_tpid_set functions for backward compatibility with
> the old ABI.
>
> Any other comments or thoughts on this?
> Neil, Thomas, Panu - is this fix something that we need to provide backward
> version-compatibility for, or given that the functions are being called 
> through
> a generic ethdev API mean that this can just go in as a straight bug-fix?

Since it's currently inconsistent with everything else, I'd just call it 
a bug-fix and leave it at that.

Besides, I dont think you could version it via the ordinary means even 
if you wanted to, due to the way its called through eth_dev_ops etc.

- Panu -

[dpdk-dev] [PATCH 2/3] pdump: fix missing dependency on libpthread

[Panu Matilainen]

Fixes: 278f945402c5 ("pdump: add new library for packet capture")

Signed-off-by: Panu Matilainen 
---
 lib/librte_pdump/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_pdump/Makefile b/lib/librte_pdump/Makefile
index af81a28..a506c4d 100644
--- a/lib/librte_pdump/Makefile
+++ b/lib/librte_pdump/Makefile
@@ -36,6 +36,7 @@ LIB = librte_pdump.a

 CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3
 CFLAGS += -D_GNU_SOURCE
+LDLIBS += -lpthread

 EXPORT_MAP := rte_pdump_version.map

-- 
2.5.5

[dpdk-dev] [PATCH] dropping librte_ivshmem - was log: deprecate history dump

[Panu Matilainen]

On 06/10/2016 12:26 AM, Thomas Monjalon wrote:
> Looking a bit more into librte_ivshmem, the documentation says we need
> a Qemu patch but the URL doesn't exist anymore:
>   https://01.org/packet-processing/intel%C2%AE-ovdk
>   -> 404 Oops, we couldn't find that page
>
> I've never understood why we should keep this wart and now I'm going
> to be upset.

Good :)

> To sum up the situation, eal depends on ivshmem which depends on
> ring/mempool which depends... on eal.
> The truth is that eal should not depends on librte_ivshmem.
> And the option CONFIG_RTE_LIBRTE_IVSHMEM should not exist.
>
> There are 3 parts to distinguish:
>
> 1/ The librte_ivshmem API to export some data structures from host.
> No real problem here.
>
> 2/ The scan of the ivshmem devices in the guest init.
> It should be handled as any other PCI device with an appropriate driver.
> The scan is done by rte_eal_pci_init.
>
> 3/ The automatic mapped allocation of DPDK objects in the guest.
> It should not be done in EAL.
> An ivshmem driver would be called by rte_eal_dev_init.
> It would check where are the shared DPDK structures, as currently done
> with the IVSHMEM_MAGIC (0x0BADC0DE), and do the appropriate allocations.
> Thus only the driver would depend on ring and mempool.
>
> The last step of the ivshmem cleanup will be to remove the memory hack
> RTE_EAL_SINGLE_FILE_SEGMENTS. Then CONFIG_RTE_LIBRTE_IVSHMEM could be
> removed.
>
> So this is my proposal:
> Someone start working on the above cleanup now, otherwise the whole
> rte_ivshmem feature will be deprecated in 16.07 and removed in 16.11.
> We already talked about the rte_ivshmem design issues several times
> and nobody declared using it.

+1 (more like +100) to that.

In addition to the technical mess in EAL, there are quite some 
eyebrow-raisers related to IVSHMEM:

That it all starts with "you'll need to build a special version of qemu" 
with this special patch from the 'net, a patch which doesn't even exist 
anymore, is a complete non-starter. Such a situation can occur during 
early development, but its been years by now. Dependencies to 
non-upstreamed features in other projects are not a healthy sign.

Regardless of whether the patch has been integrated to qemu upstream or 
not, the situation is quite telling: nobody cares enough to have updated 
the information. I found a copy of the patch from my laptop, and as far 
as I can tell, the patch has never been proposed upstream, much less 
applied. Certainly the patch would not come even close to applying to 
current qemu. And apparently IVSHMEM is unmaintained in qemu upstream 
too (according to MAINTAINERS).

On DPDK side, that the most obvious (to me at least) user of memnic PMD 
has been unmaintained for two years no, and allowed to fall off the edge 
of the world (witness http://dpdk.org/browse/memnic/) is also quite telling.

Just deprecate it already. If somebody shows up with actual patches to 
clean it all up, the deprecation can be lifted of course, but cleaning 
up this abandonware seems like waste of engineering resources to me.

- Panu -

[dpdk-dev] [PATCHv7 1/6] pmdinfogen: Add buildtools and pmdinfogen utility

[Panu Matilainen]

On 06/16/2016 04:33 PM, Neil Horman wrote:
> On Thu, Jun 16, 2016 at 03:29:57PM +0300, Panu Matilainen wrote:
>> On 06/09/2016 08:46 PM, Neil Horman wrote:
>>> pmdinfogen is a tool used to parse object files and build json strings for
>>> use in later determining hardware support in a dso or application binary.
>>> pmdinfo looks for the non-exported symbol names this_pmd_name and
>>> this_pmd_tbl (where n is a integer counter).  It records the name of
>>> each of these tuples, using the later to find the symbolic name of the
>>> pci_table for physical devices that the object supports.  With this
>>> information, it outputs a C file with a single line of the form:
>>>
>>> static char *_driver_info[] __attribute__((used)) = " \
>>> PMD_DRIVER_INFO=";
>>>
>>> Where  is the arbitrary name of the pmd, and  is the
>>> json encoded string that hold relevant pmd information, including the pmd
>>> name, type and optional array of pci device/vendor ids that the driver
>>> supports.
>>>
>>> This c file is suitable for compiling to object code, then relocatably
>>> linking into the parent file from which the C was generated.  This creates
>>> an entry in the string table of the object that can inform a later tool
>>> about hardware support.
>>>
>>> Signed-off-by: Neil Horman 
>>> CC: Bruce Richardson 
>>> CC: Thomas Monjalon 
>>> CC: Stephen Hemminger 
>>> CC: Panu Matilainen 
>>> ---
>>
>> Unlike earlier versions, pmdinfogen ends up installed in bindir during "make
>> install". Is that intentional, or just a side-effect from using
>> rte.hostapp.mk? If its intentional it probably should be prefixed with dpdk_
>> like the other tools.
>>
> Im not sure what the answer is here.  As you can see, Thomas and I argued at
> length over which makefile to use, and I gave up, so I suppose you can call it
> intentional.  Being in bindir makes a reasonable amount of sense I suppose, as
> 3rd party developers can use it during their independent driver development.

Right, it'd be useful for 3rd party driver developer, so lets consider 
it intentional :)

> I'm not sure I agree with prefixing it though.  Given that the hostapp.mk file
> installs everything there, and nothing that previously used that make file 
> had a
> dpdk_ prefix that I can tell, I'm not sure why this would.  pmdinfogen seems
> like a pretty unique name, and I know of no other project that uses the term 
> pmd
> to describe anything.

I agree about "pmd" being fairly unique as is, but if pmdinfo is dpdk_ 
prefixed then this should be too, or neither should be prefixed. I dont 
personally care which way, but it should be consistent.

- Panu -

>
> Neil
>
>>  - Panu -
>>
>>

[dpdk-dev] [PATCHv7 5/6] pmdinfo.py: Add tool to query binaries for hw and other support information

[Panu Matilainen]

On 06/09/2016 08:47 PM, Neil Horman wrote:
> This tool searches for the primer sting PMD_DRIVER_INFO= in any ELF binary,
> and, if found parses the remainder of the string as a json encoded string,
> outputting the results in either a human readable or raw, script parseable
> format
>
> Note that, in the case of dynamically linked applications, pmdinfo.py will
> scan for implicitly linked PMDs by searching the specified binaries
> .dynamic section for DT_NEEDED entries that contain the substring
> librte_pmd.  The DT_RUNPATH, LD_LIBRARY_PATH, /usr/lib and /lib are
> searched for these libraries, in that order
>
> If a file is specified with no path, it is assumed to be a PMD DSO, and the
> LD_LIBRARY_PATH, /usr/lib[64]/ and /lib[64] is searched for it
>
> Currently the tool can output data in 3 formats:
>
> a) raw, suitable for scripting, where the raw JSON strings are dumped out
> b) table format (default) where hex pci ids are dumped in a table format
> c) pretty, where a user supplied pci.ids file is used to print out vendor
> and device strings
>
> Signed-off-by: Neil Horman 
> CC: Bruce Richardson 
> CC: Thomas Monjalon 
> CC: Stephen Hemminger 
> CC: Panu Matilainen 
> ---
>  mk/rte.sdkinstall.mk |   2 +
>  tools/pmdinfo.py | 629 
> +++
>  2 files changed, 631 insertions(+)
>  create mode 100755 tools/pmdinfo.py
>
> diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk
> index 68e56b6..dc36df5 100644
> --- a/mk/rte.sdkinstall.mk
> +++ b/mk/rte.sdkinstall.mk
> @@ -126,6 +126,8 @@ install-runtime:
>   $(Q)$(call rte_mkdir,  $(DESTDIR)$(sbindir))
>   $(Q)$(call rte_symlink,$(DESTDIR)$(datadir)/tools/dpdk_nic_bind.py, 
> \
>  $(DESTDIR)$(sbindir)/dpdk_nic_bind)
> + $(Q)$(call rte_symlink,$(DESTDIR)$(datadir)/tools/pmdinfo.py, \
> +$(DESTDIR)$(bindir)/dpdk-pmdinfo)

The symlink should be with underscore instead of dash for consistency 
with all the other tools, ie dpdk_pmdinfo.

Neil, I already gave you an ack on the series as per the functionality, 
feel free to include that in any future versions of the patch series. 
Minor nits like these are ... well, minor nits from my POV at least.

- Panu -

[dpdk-dev] [PATCHv7 1/6] pmdinfogen: Add buildtools and pmdinfogen utility

[Panu Matilainen]

On 06/09/2016 08:46 PM, Neil Horman wrote:
> pmdinfogen is a tool used to parse object files and build json strings for
> use in later determining hardware support in a dso or application binary.
> pmdinfo looks for the non-exported symbol names this_pmd_name and
> this_pmd_tbl (where n is a integer counter).  It records the name of
> each of these tuples, using the later to find the symbolic name of the
> pci_table for physical devices that the object supports.  With this
> information, it outputs a C file with a single line of the form:
>
> static char *_driver_info[] __attribute__((used)) = " \
>   PMD_DRIVER_INFO=";
>
> Where  is the arbitrary name of the pmd, and  is the
> json encoded string that hold relevant pmd information, including the pmd
> name, type and optional array of pci device/vendor ids that the driver
> supports.
>
> This c file is suitable for compiling to object code, then relocatably
> linking into the parent file from which the C was generated.  This creates
> an entry in the string table of the object that can inform a later tool
> about hardware support.
>
> Signed-off-by: Neil Horman 
> CC: Bruce Richardson 
> CC: Thomas Monjalon 
> CC: Stephen Hemminger 
> CC: Panu Matilainen 
> ---

Unlike earlier versions, pmdinfogen ends up installed in bindir during 
"make install". Is that intentional, or just a side-effect from using 
rte.hostapp.mk? If its intentional it probably should be prefixed with 
dpdk_ like the other tools.

- Panu -

[dpdk-dev] [PATCH v4] eal: out-of-bounds write

[Panu Matilainen]

On 06/15/2016 04:25 PM, Slawomir Mrozowicz wrote:
> Overrunning array mcfg->memseg of 256 44-byte elements
> at element index 257 using index j.
> Fixed by add condition with message information.
>
> Fixes: af75078fece3 ("first public release")
> Coverity ID 13282
>
> Signed-off-by: Slawomir Mrozowicz 
> ---
>  lib/librte_eal/linuxapp/eal/eal_memory.c | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c 
> b/lib/librte_eal/linuxapp/eal/eal_memory.c
> index 5b9132c..19753b1 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
> @@ -1301,6 +1301,15 @@ rte_eal_hugepage_init(void)
>   break;
>   }
>
> + if (j >= RTE_MAX_MEMSEG) {
> + RTE_LOG(ERR, EAL,
> + "Failed: all memsegs used by ivshmem.\n"
> + "Current %d is not enough.\n"
> + "Please either increase the RTE_MAX_MEMSEG\n",
> + RTE_MAX_MEMSEG);
> + return -ENOMEM;
> + }


The error message is either incomplete or not coherent: "please either 
increase..." or what?

Also no need for that "Failed:" because its already prefixed by 
"Error:". I'm not sure how helpful it is to have an error message 
suggest increasing a value that requires recomplication, but maybe 
something more in the lines of:

("All memory segments exhausted by IVSHMEM. Try recompiling with larger 
RTE_MAX_MEMSEG than current %d?", RTE_MAX_MEMSEG)

- Panu -

[dpdk-dev] random pkt generator PMD

[Panu Matilainen]

On 06/15/2016 03:14 PM, Yerden Zhumabekov wrote:
>
>
> On 15.06.2016 17:25, Panu Matilainen wrote:
>> On 06/15/2016 02:10 PM, Yerden Zhumabekov wrote:
>>>
>>>
>>> On 15.06.2016 16:43, Dumitrescu, Cristian wrote:
>>>>
>>>>>
>>>>> Hello everybody,
>>>>>
>>>>> DPDK already got a number of PMDs for various eth devices, it even has
>>>>> PMD emulations for backends such as pcap, sw rings etc.
>>>>>
>>>>> I've been thinking about the idea of having PMD which would generate
>>>>> mbufs on the fly in some randomized fashion. This would serve goals
>>>>> like, for example:
>>>>>
>>>>> 1) running tests for applications with network processing capabilities
>>>>> without additional software packet generators;
>>>>> 2) making performance measurements with no hw inteference;
>>>>> 3) ability to run without root privileges, --no-pci, --no-huge, for CI
>>>>> build, so on.
>>>>>
>>>>> Maybe there's no such need, and these goals may be achieved by other
>>>>> means and this idea is flawed? Any thoughts?
>>>> How about a Perl/Python script to generate a PCAP file with random
>>>> packets and then feed the PCAP file to the PCAP PMD?
>>>>
>>>> Random can mean different requirements for different
>>>> users/application, I think it is difficult to fit this  under a simple
>>>> generic API. Customizing the script for different requirements if a
>>>> far better option in my opinion.
>>>
>>> AFAIK, the thing about pcap pmd is that one needs to rewind pcap file
>>> once pcap pmd reaches its end. It requires additional (non-generic)
>>> handling in app code.
>>
>> So add a loop-mode to pcap pmd?
>
> It would be nice to have an option like "...,rewind=1,...".

As Cristian points out in 
http://dpdk.org/ml/archives/dev/2016-June/041589.html, the current pmd 
behavior of stopping is the odd man out in the pmd crowd.

Rather than whether to rewind or not, I'd make the number of loops 
configurable, defaulting to forever and 1 being the equal to current 
behavior.

- Panu -

[dpdk-dev] random pkt generator PMD

[Panu Matilainen]

On 06/15/2016 02:10 PM, Yerden Zhumabekov wrote:
>
>
> On 15.06.2016 16:43, Dumitrescu, Cristian wrote:
>>
>>> -Original Message-
>>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Yerden
>>> Zhumabekov
>>> Sent: Wednesday, June 15, 2016 10:44 AM
>>> To: dev at dpdk.org
>>> Subject: [dpdk-dev] random pkt generator PMD
>>>
>>> Hello everybody,
>>>
>>> DPDK already got a number of PMDs for various eth devices, it even has
>>> PMD emulations for backends such as pcap, sw rings etc.
>>>
>>> I've been thinking about the idea of having PMD which would generate
>>> mbufs on the fly in some randomized fashion. This would serve goals
>>> like, for example:
>>>
>>> 1) running tests for applications with network processing capabilities
>>> without additional software packet generators;
>>> 2) making performance measurements with no hw inteference;
>>> 3) ability to run without root privileges, --no-pci, --no-huge, for CI
>>> build, so on.
>>>
>>> Maybe there's no such need, and these goals may be achieved by other
>>> means and this idea is flawed? Any thoughts?
>> How about a Perl/Python script to generate a PCAP file with random
>> packets and then feed the PCAP file to the PCAP PMD?
>>
>> Random can mean different requirements for different
>> users/application, I think it is difficult to fit this  under a simple
>> generic API. Customizing the script for different requirements if a
>> far better option in my opinion.
>
> AFAIK, the thing about pcap pmd is that one needs to rewind pcap file
> once pcap pmd reaches its end. It requires additional (non-generic)
> handling in app code.

So add a loop-mode to pcap pmd?

- Panu -

[dpdk-dev] [PATCH v2 1/1] eal: fix resource leak of mapped memory

[Panu Matilainen]

On 06/15/2016 12:35 PM, Kerlin, MarcinX wrote:
> Hi Sergio,
>
> Thanks for tips, I removed double casting and I leave (void *) casting 
> because pointer hp is const and compiler returns warnings.

If hp is something that needs freeing then it shouldn't be const in the 
first place. Please fix that instead.

- Panu -

[dpdk-dev] [PATCH] mem: skip memory locking on failure

[Panu Matilainen]

On 06/13/2016 01:26 PM, Olivier Matz wrote:
> Since recently [1], it is not possible to run the dpdk with user
> (non-root) privileges and the --no-huge option. This is because the eal
> layer tries to lock the memory. Using locked memory is mandatory for
> physical devices because they reference physical addresses.
>
> But a user may want to start the dpdk without locked memory, because he
> does not have the permission to do so, and/or does not have this need.
>
> Moreover, the option --no-huge is still not functional today since the
> physical memory address is not properly filled, so the initial patch is
> not really useful.
>
> This commit fixes this issue by retrying the mmap() wihout the
> MAP_LOCKED flag if the first mmap() failed.
>
> [1] http://www.dpdk.org/ml/archives/dev/2016-May/039404.html
>
> Fixes: 593a084afc2b ("mem: lock pages when not using hugepages")
> Reported-by: Panu Matilainen 
> Signed-off-by: Olivier Matz 
> ---
>  lib/librte_eal/linuxapp/eal/eal_memory.c | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c 
> b/lib/librte_eal/linuxapp/eal/eal_memory.c
> index 79d1d2d..08692d1 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
> @@ -1075,6 +1075,15 @@ rte_eal_hugepage_init(void)
>   if (internal_config.no_hugetlbfs) {
>   addr = mmap(NULL, internal_config.memory, PROT_READ | 
> PROT_WRITE,
>   MAP_LOCKED | MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
> + /* retry without MAP_LOCKED */
> + if (addr == MAP_FAILED && errno == EAGAIN) {
> + addr = mmap(NULL, internal_config.memory,
> + PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
> + if (addr != MAP_FAILED)
> + RTE_LOG(NOTICE, EAL,
> + "Cannot lock memory: don't use physical 
> devices\n");
> + }
>   if (addr == MAP_FAILED) {
>   RTE_LOG(ERR, EAL, "%s: mmap() failed: %s\n", __func__,
>   strerror(errno));
>

I'm not really that familiar with dpdk memory usage, but gut feeling 
says such a thing needs to be explicit - either you explicitly ask for 
memory that doesn't need to be locked, or this simply fails with no 
retries. Or something like that. But "maybe I did, maybe I didn't" 
doesn't seem like very good API semantics to me :)

Are there actual plans to make --no-huge work with real devices? If not 
then documenting --no-huge to imply unlocked memory is one option I guess.

- Panu -

- Panu -

[dpdk-dev] [PATCH v3 08/20] vhost: introduce new API to export numa node

[Panu Matilainen]

On 06/07/2016 06:51 AM, Yuanhan Liu wrote:
> Introduce a new API rte_vhost_get_numa_node() to get the numa node
> from which the virtio_net struct is allocated.
>
> Signed-off-by: Yuanhan Liu 
> Tested-by: Rich Lane 
> Acked-by: Rich Lane 
> ---
>  drivers/net/vhost/rte_eth_vhost.c  | 13 -
>  lib/librte_vhost/rte_vhost_version.map |  7 +++
>  lib/librte_vhost/rte_virtio_net.h  | 12 
>  lib/librte_vhost/virtio-net.c  | 26 ++
>  4 files changed, 49 insertions(+), 9 deletions(-)
>
[...]
> diff --git a/lib/librte_vhost/rte_vhost_version.map 
> b/lib/librte_vhost/rte_vhost_version.map
> index 3d8709e..bf7b000 100644
> --- a/lib/librte_vhost/rte_vhost_version.map
> +++ b/lib/librte_vhost/rte_vhost_version.map
> @@ -20,3 +20,10 @@ DPDK_2.1 {
>   rte_vhost_driver_unregister;
>
>  } DPDK_2.0;
> +
> +DPDK_16.07 {
> + global:
> +
> + rte_vhost_get_numa_node;
> +
> +} DPDK_16.04;

This fails to compile in shared library configuration:

LD librte_vhost.so.3.1
/usr/bin/ld: unable to find version dependency `DPDK_16.04'
collect2: error: ld returned 1 exit status

Problem obviously being that DPDK_16.04 does not exist, the most recent 
version symbol for librte_vhost is DPDK_2.1 so you need to inherit from 
that instead.

- Panu -

[dpdk-dev] [PATCH] mk: generate internal library dependencies from DEPDIRS-y automatically

[Panu Matilainen]

Up to now dependencies between DPDK internal libraries have been
untracked at shared library level, requiring applications to know
about library internal dependencies and often consequently overlinking.

Since the dependencies are already recorded for build ordering in the
makefiles, we can use that information to generate LDLIBS entries for
internal libraries automatically.

Also revert commit 8180554d82b3 ("vhost: fix linkage of driver with
library") which is made redundant by this change.

Signed-off-by: Panu Matilainen 
---
 drivers/net/vhost/Makefile | 1 -
 mk/rte.lib.mk  | 7 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/vhost/Makefile b/drivers/net/vhost/Makefile
index 30b91a0..f49a69b 100644
--- a/drivers/net/vhost/Makefile
+++ b/drivers/net/vhost/Makefile
@@ -38,7 +38,6 @@ LIB = librte_pmd_vhost.a

 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
-LDLIBS += -lrte_vhost

 EXPORT_MAP := rte_pmd_vhost_version.map

diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index b420280..1ff403f 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -77,6 +77,13 @@ else
 _CPU_LDFLAGS := $(CPU_LDFLAGS)
 endif

+# Translate DEPDIRS-y into LDLIBS
+# Ignore (sub)directory dependencies which do not provide an actual library
+_IGNORE_DIRS = lib/librte_eal/% lib/librte_net lib/librte_compat
+_DEPDIRS = $(filter-out $(_IGNORE_DIRS),$(DEPDIRS-y))
+_LDDIRS = $(subst librte_ether,libethdev,$(_DEPDIRS))
+LDLIBS += $(subst lib/lib,-l,$(_LDDIRS))
+
 O_TO_A = $(AR) crDs $(LIB) $(OBJS-y)
 O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight
 O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)","  AR $(@)")
-- 
2.5.5

[dpdk-dev] [PATCH] mk: fix underlinking issues of most librte libraries

[Panu Matilainen]

On 06/07/2016 11:23 AM, Thomas Monjalon wrote:
> 2016-05-24 12:56, Panu Matilainen:
>> On 05/20/2016 08:08 PM, Thomas Monjalon wrote:
>>> 2016-05-20 18:50, Christian Ehrhardt:
>>>> The individual libraries have various cross dependencies.
>>>> This is already refelcted in the DEPDIR dependency, but not yet in
>>>> proper DT_NEEDED flags in the .so's.
>>>> This adds the -l flags so that is properly stored in the .so's ELF
>>>> headers.
>>>
>>> Why not filling LDLIBS by parsing DEPDIRS-y in rte.lib.mk?
>>>
>>
>> A fair question :) The thought has passed my mind too, but I thought
>> it'd be too messy with differing library vs directory names and other
>> exceptions. But in reality manually maintained separate LDLIBS is only
>> going to get out of sync sooner or later, and turns out its not that bad
>> anyway:
>>
>> diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
>> index b420280..88b4e98 100644
>> --- a/mk/rte.lib.mk
>> +++ b/mk/rte.lib.mk
>> @@ -77,6 +77,12 @@ else
>>   _CPU_LDFLAGS := $(CPU_LDFLAGS)
>>   endif
>>
>> +# Translate DEPDIRS-y into LDLIBS
>> +IGNORE_DEPS = -lrte_eal/% -lrte_net -lrte_compat
>
> We need some comments to explain why they are ignored.

Yup. It'd be more clear if this filtering was done before translating 
directories to -lrte_foo but this was the first variant I came across 
that worked :) The issue is that librte_eal/common, librte_net and 
librte_compat are common directory dependencies but no actual library is 
generated from them so they just need to be weeded out of the equation, 
the revised version makes this more clear.

>
>> +_LDDIRS = $(subst librte_ether,libethdev,$(DEPDIRS-y))
>> +_LDDEPS = $(subst lib/lib,-l,$(_LDDIRS))
>> +LDLIBS += $(filter-out $(IGNORE_DEPS), $(_LDDEPS))
>
>> I'm also sure somebody more familiar with gmake could improve it, but it
>> seems to work quite fine here. I'll wait a bit to see if there are other
>> suggestions before sending it as a proper patch.
>
> I'm familiar with gmake and I do not think it can be improved more ;)
> Please send a patch. Thanks
>

Ok, will do.

- Panu -

[dpdk-dev] [RFC] Yet another option for DPDK options

[Panu Matilainen]

On 06/03/2016 03:01 PM, Arnon Warshavsky wrote:
>
>
> On Fri, Jun 3, 2016 at 2:50 PM, Neil Horman  > wrote:
>
> On Fri, Jun 03, 2016 at 12:01:30PM +0100, Bruce Richardson wrote:
> > On Fri, Jun 03, 2016 at 11:29:43AM +0100, Bruce Richardson wrote:
> > > On Thu, Jun 02, 2016 at 04:08:37PM -0400, Neil Horman wrote:
> > > > On Thu, Jun 02, 2016 at 07:41:10PM +, Wiles, Keith wrote:
> > > > >
> > > > > On 6/2/16, 12:11 PM, "Neil Horman"  > wrote:
> > > > >
> > > > > >
> > > > > >1) The definition of a config structure that can be passed
> to rte_eal_init,
> > > > > >defining the configuration for that running process
> > > > >
> > > > > Having a configuration structure means we have to have an
> ABI change to that structure anytime we add or remove an option. I
> was thinking a very simple DB of some kind would be better. Have the
> code query the DB to obtain the needed information. The APIs used to
> query and set the DB needs to be very easy to use as well.
> > > >
> > > > Thats a fair point.  A decent starting point is likely a
> simple struct that
> > > > looks like this:
> > > >
> > > > struct key_vals {
> > > >   char *key;
> > > >   union {
> > > >   ulong longval;
> > > >   void *ptrval;
> > > >   } value;
> > > > };
> > > >
> > > > struct config {
> > > >   size_t count;
> > > >   struct key_vals kvp[0];
> > > > };
> > > >
> > > > >
> > > > > Maybe each option can define its own structure if needed or
> just a simple variable type can be used for the basic types (int,
> string, bool, ?)
> > > > >
> > > > Well, if you have config sections that require mulitiple
> elements, I'd handle
> > > > that with naming, i.e. if you have a config group that has an
> int and char
> > > > value, I'd name them "group.intval", and "group.charval", so
> they are
> > > > independently searchable, but linked from a nomenclature
> standpoint.
> > > >
> > > > > Would this work better in the long run, does a fixed
> structure still make sense?
> > > > >
> > > > No. I think you're ABI concerns are valid, but the above is
> likely a good
> > > > starting point to address them.
> > > >
> > > > Best
> > > > Neil
> > >
> > > I'll throw out one implementation idea here that I looked at
> previously, for
> > > the reason that it was simple enough implement with existing code.
> > >
> > > We already have the cfgfile library which works with name/value
> pairs read from
> > > ini files on disk. However, it would be easy enough to add
> couple of APIs to
> > > that to allow the user to "set" values inside an ini structure
> as well. With
> > > that done we can then just add a new eal_init api which takes a
> single
> > > "struct rte_cfgfile *" as parameter. For those apps that want to
> just use
> > > inifiles for configuration straight, they can then do:
> > >
> > > cfg = rte_cfgfile_load("my_cfg_file");
> > > rte_eal_newinit(cfg);
> > >
> > > Those who want a different config can instead do:
> > >
> > > cfg = rte_cfgfile_new();
> > > rte_cfgfile_add_section(cfg, "dpdk");
> > > foreach_eal_setting_wanted:
> > > rte_cfgfile_set(cfg, "dpdk", mysetting, myvalue);
> > > rte_eal_newinit(cfg);
> > >
> > From chatting to a couple of other DPDK dev's here I suspect I may
> not have
> > been entirely clear here with this example. What is being shown
> above is building
> > up a "config-file" in memory - or rather a config structure which
> happens to
> > have the idea of sections and values as an ini file has. There is
> no actual
> > file ever being written to disk, and for those using any non-ini
> config file
> > structure for their app, the code overhead of using the APIs above
> should be
> > pretty much the same as building up any other set of key-value
> pairs in
> > memory to pass to an init function.
> >
> > Hope this is a little clearer now.
> >
> I'm fine with the idea of reusing the config file library that
> currently exists,
> or more to the point, modifying it to be usable as a configuration
> API, rather
> than a configuration file parser.  My primary interest is in
> separating the user
> configuration mechanism from the internal library configuration lookup
> mechanism.  What I would really like to be able to see is
> application developers
> have the flexibiilty to choose their own configuration method and
> format, and
> programatically build a configuration for the dpdk on a per-instance
> basis prior
> to calling rte_eal_init
>
> It seems like this approach 

[dpdk-dev] [RFC] Yet another option for DPDK options

[Panu Matilainen]

On 06/03/2016 02:50 PM, Neil Horman wrote:
> On Fri, Jun 03, 2016 at 12:01:30PM +0100, Bruce Richardson wrote:
>> On Fri, Jun 03, 2016 at 11:29:43AM +0100, Bruce Richardson wrote:
>>> On Thu, Jun 02, 2016 at 04:08:37PM -0400, Neil Horman wrote:
 On Thu, Jun 02, 2016 at 07:41:10PM +, Wiles, Keith wrote:
>
> On 6/2/16, 12:11 PM, "Neil Horman"  wrote:
>
>>
>> 1) The definition of a config structure that can be passed to 
>> rte_eal_init,
>> defining the configuration for that running process
>
> Having a configuration structure means we have to have an ABI change to 
> that structure anytime we add or remove an option. I was thinking a very 
> simple DB of some kind would be better. Have the code query the DB to 
> obtain the needed information. The APIs used to query and set the DB 
> needs to be very easy to use as well.

 Thats a fair point.  A decent starting point is likely a simple struct that
 looks like this:

 struct key_vals {
char *key;
union {
ulong longval;
void *ptrval;
} value;
 };

 struct config {
size_t count;
struct key_vals kvp[0];
 };

>
> Maybe each option can define its own structure if needed or just a simple 
> variable type can be used for the basic types (int, string, bool, ?)
>
 Well, if you have config sections that require mulitiple elements, I'd 
 handle
 that with naming, i.e. if you have a config group that has an int and char
 value, I'd name them "group.intval", and "group.charval", so they are
 independently searchable, but linked from a nomenclature standpoint.

> Would this work better in the long run, does a fixed structure still make 
> sense?
>
 No. I think you're ABI concerns are valid, but the above is likely a good
 starting point to address them.

 Best
 Neil
>>>
>>> I'll throw out one implementation idea here that I looked at previously, for
>>> the reason that it was simple enough implement with existing code.
>>>
>>> We already have the cfgfile library which works with name/value pairs read 
>>> from
>>> ini files on disk. However, it would be easy enough to add couple of APIs to
>>> that to allow the user to "set" values inside an ini structure as well. With
>>> that done we can then just add a new eal_init api which takes a single
>>> "struct rte_cfgfile *" as parameter. For those apps that want to just use
>>> inifiles for configuration straight, they can then do:
>>>
>>> cfg = rte_cfgfile_load("my_cfg_file");
>>> rte_eal_newinit(cfg);
>>>
>>> Those who want a different config can instead do:
>>>
>>> cfg = rte_cfgfile_new();
>>> rte_cfgfile_add_section(cfg, "dpdk");
>>> foreach_eal_setting_wanted:
>>> rte_cfgfile_set(cfg, "dpdk", mysetting, myvalue);
>>> rte_eal_newinit(cfg);
>>>
>> From chatting to a couple of other DPDK dev's here I suspect I may not have
>> been entirely clear here with this example. What is being shown above is 
>> building
>> up a "config-file" in memory - or rather a config structure which happens to
>> have the idea of sections and values as an ini file has. There is no actual
>> file ever being written to disk, and for those using any non-ini config file
>> structure for their app, the code overhead of using the APIs above should be
>> pretty much the same as building up any other set of key-value pairs in
>> memory to pass to an init function.

/me nods.

This is pretty much exactly what I suggested (only in much less detail) 
last year :) http://dpdk.org/ml/archives/dev/2015-October/024803.html

>> Hope this is a little clearer now.
> I'm fine with the idea of reusing the config file library that currently 
> exists,
> or more to the point, modifying it to be usable as a configuration API, rather
> than a configuration file parser.  My primary interest is in separating the 
> user
> configuration mechanism from the internal library configuration lookup
> mechanism.  What I would really like to be able to see is application 
> developers
> have the flexibiilty to choose their own configuration method and format, and
> programatically build a configuration for the dpdk on a per-instance basis 
> prior
> to calling rte_eal_init
>
> It seems like this approach satisfies that requirement

/me nods some more.

What the key-value config also can buy us is a direct mapping to cli 
options (which is something Keith has been looking into IIRC), at which 
point I think all the bases are quite nicely covered.

- Panu -

[dpdk-dev] [PATCH v2 1/2] ethdev: add callback to get register size in bytes

[Panu Matilainen]

On 05/30/2016 12:39 PM, zr at semihalf.com wrote:
> From: Zyta Szpak 
>
> Version 2 of fixing the fixed register width assumption.
> rte_eth_dev_get_reg_length and rte_eth_dev_get_reg callbacks
> do not provide register size to the app in any way. It is
> needed to allocate proper number of bytes before retrieving
> registers content with rte_eth_dev_get_reg.
>
> Signed-off-by: Zyta Szpak 
[...]
> diff --git a/lib/librte_ether/rte_ether_version.map 
> b/lib/librte_ether/rte_ether_version.map
> index 214ecc7..288bc63 100644
> --- a/lib/librte_ether/rte_ether_version.map
> +++ b/lib/librte_ether/rte_ether_version.map
> @@ -130,5 +130,6 @@ DPDK_16.04 {
>   rte_eth_tx_buffer_drop_callback;
>   rte_eth_tx_buffer_init;
>   rte_eth_tx_buffer_set_err_callback;
> + rte_eth_dev_get_reg_width;
>
>  } DPDK_2.2;

This symbol did not exist in DPDK 16.04 so it must not be added there. 
Add a new section for 16.07 which inherits from DPDK_16.04.

- Panu -

[dpdk-dev] [PATCHv4 5/5] pmdinfo.py: Add tool to query binaries for hw and other support information

[Panu Matilainen]

On 05/27/2016 02:35 PM, Neil Horman wrote:
> On Fri, May 27, 2016 at 12:16:03PM +0300, Panu Matilainen wrote:
>
>> Yup. But there's also a bit of a gotcha involved with the virtual devices
>> with added api. This is how eg testpmd looks when built in shared library
>> config:
>>
>> [pmatilai at sopuli ~]$ pmdinfo /usr/bin/testpmd
>> Scanning /usr/lib64/librte_pmd_bond.so.1 for pmd information
>> PMD NAME: bonding
>> PMD TYPE: PMD_VDEV
>> PMD PARAMETERS: slave= primary= mode=[0-4] xmit_policy=[l2 | l23 |
>> l34] socket_id= mac= lsc_poll_period_ms= up_delay=
>> down_delay=
>>
>> [pmatilai at sopuli ~]$
>>
>> Having support for bonding driver but nothing else will seem pretty damn
>> confusing unless you happen to be a dpdk developer knowing this fact about
>> some pmds also providing API and being linked directly etc.
>>
> Possibly, but I think its important to remember that we aren't really dealing
> with end users buying this software at best buy.  The target user is more 
> likely
> a sysadmin, that I would like to think has the wherewithal to understand that
> hardware support might be either (a) built-in, getting loaded at runtime by
> virtue of the program itself, or a library that it is explicitly linked to, or
> (b) configured-in, getting resolved an loaded at run time by virtue of a
> configuration file or other site specific element.  They should be able to
> figure out that the latter won't be discoverable by pmdinfo without help.

Sure, but ability to figure something out doesn't necessarily mean they 
should have to, much less want to, do so. Somebody setting up DPDK is 
interested in moving network packets, and the details of how a 
particular piece of software was built and linked together couldn't be 
much more further from away from that task.

I do remember the days when you had to manually load device drivers by 
kernel module names during Linux installation and while I certainly 
could do it, I dont really miss that part a single bit :)

I also realize I'm probably way ahead of things with my these usability 
ramblings when you're only trying to get the feature in, and that's 
clearly been causing some unintended friction here. Sorry about that, 
I'm really just excited about the possibilities this feature opens.

>> Might make sense to have extra cli switch to control which kind of pmds are
>> shown, and maybe default to skipping vdevs since they dont provide any
>> actual hardware support anyway. Showing nothing at all in the above case
>> would likely be a little less confusing. Maybe :)
>>
> So, Thomas and I just finished arguing about the VDEV/PDEV issue.  Since he
> asserted that the ennumeration for device types was being removed I removed 
> any
> notion of differentiation from the tool.  Unless there are plans to keep that
> differentiation, I don't think adding a filter for device types is really
> advisable

Ah yes, sorry. I did see the discussion but the actual message 
apparently failed to register :)

>
> What might be useful is a filter on vendor/device id's.  That is to say only
> show pmd information that matches the specified vendor/device tuple, so that a
> user can search to see if the application supports the hardware they have.

Yeah, makes sense.

>> Anyway, this is "refine later if needed once we gain more experience"
>> material to me.
> Agreed, this isn't an addition that needs to happen now, walk before we run.

Nod.

- Panu -

> Neil
>
>>
>>  - Panu -
>>
>>

[dpdk-dev] [PATCH 1/2] ethdev: add callback to get register size in bytes

[Panu Matilainen]

On 05/25/2016 09:36 AM, zr at semihalf.com wrote:
> From: Zyta Szpak 
>
> Version 2 of fixing the fixed register width assumption.
> rte_eth_dev_get_reg_length and rte_eth_dev_get_reg callbacks
> do not provide register size to the app in any way. It is
> needed to allocate proper number of bytes before retrieving
> registers content with rte_eth_dev_get_reg.
>
> Signed-off-by: Zyta Szpak 
> ---
>  lib/librte_ether/rte_ethdev.c | 12 
>  lib/librte_ether/rte_ethdev.h | 18 ++
>  2 files changed, 30 insertions(+)
>
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index a31018e..e0765f8 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -3231,6 +3231,18 @@ rte_eth_dev_get_reg_length(uint8_t port_id)
>  }
>
>  int
> +rte_eth_dev_get_reg_width(uint8_t port_id)
> +{
> + struct rte_eth_dev *dev;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +
> + dev = _eth_devices[port_id];
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_reg_width, -ENOTSUP);
> + return (*dev->dev_ops->get_reg_width)(dev);
> +}
> +
> +int
>  rte_eth_dev_get_reg_info(uint8_t port_id, struct rte_dev_reg_info *info)
>  {
>   struct rte_eth_dev *dev;
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 2757510..552eaed 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -1292,6 +1292,9 @@ typedef int (*eth_timesync_write_time)(struct 
> rte_eth_dev *dev,
>  typedef int (*eth_get_reg_length_t)(struct rte_eth_dev *dev);
>  /**< @internal Retrieve device register count  */
>
> +typedef int (*eth_get_reg_width_t)(struct rte_eth_dev *dev);
> +/**< @internal Retrieve device register byte number */
> +
>  typedef int (*eth_get_reg_t)(struct rte_eth_dev *dev,
>   struct rte_dev_reg_info *info);
>  /**< @internal Retrieve registers  */
> @@ -1455,6 +1458,8 @@ struct eth_dev_ops {
>
>   eth_get_reg_length_t get_reg_length;
>   /**< Get # of registers */
> + eth_get_reg_width_t get_reg_width;
> + /**< Get # of bytes in register */
>   eth_get_reg_t get_reg;
>   /**< Get registers */
>   eth_get_eeprom_length_t get_eeprom_length;

This is an ABI break, but maybe it is part of that "driver 
implementation API" which is exempt from the ABI/API policies. Thomas?

> @@ -3971,6 +3976,19 @@ int rte_eth_tx_queue_info_get(uint8_t port_id, 
> uint16_t queue_id,
>   */
>  int rte_eth_dev_get_reg_length(uint8_t port_id);
>
> +/*
> + * Retrieve the number of bytes in register for a specific device
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @return
> + *   - (>=0) number of registers if successful.
> + *   - (-ENOTSUP) if hardware doesn't support.
> + *   - (-ENODEV) if *port_id* invalid.
> + *   - others depends on the specific operations implementation.
> + */
> +int rte_eth_dev_get_reg_width(uint8_t port_id);
> +
>  /**
>   * Retrieve device registers and register attributes
>   *

The function needs to be exported via rte_ether_version.map as well.

- Panu -
>

[dpdk-dev] [PATCHv4 5/5] pmdinfo.py: Add tool to query binaries for hw and other support information

[Panu Matilainen]

On 05/25/2016 09:58 PM, Thomas Monjalon wrote:
> 2016-05-25 13:47, Neil Horman:
>> On Wed, May 25, 2016 at 07:22:39PM +0200, Thomas Monjalon wrote:
>>> 2016-05-24 15:41, Neil Horman:
 Note that, in the case of dynamically linked applications, pmdinfo.py will 
 scan
 for implicitly linked PMDs by searching the specified binaries .dynamic 
 section
 for DT_NEEDED entries that contain the substring librte_pmd.
>>>
>>> I don't know any DPDK app dynamically linked with a PMD (with DT_NEEDED).
>> I know lots of them, they're all in the dpdk.  everything under app that 
>> uses a
>> virutal device links at link time to librte_pmd_bonding and librte_pmd_pipe 
>> (and
>> a few others), because they have additional apis that they need to resolve at
>> load time.
>
> Oh yes! you are right.

Also everything linking against the linker script (in its current form) 
such as OVS will pull in absolutely everything, including the pmds:

[pmatilai at sopuli ~]$ ldd /usr/sbin/ovs-vswitchd | awk '/librte_pmd/ 
{print $1}'
librte_pmd_bnx2x.so.1
librte_pmd_virtio.so.1
librte_pmd_cxgbe.so.1
librte_pmd_ring.so.2
librte_pmd_af_packet.so.1
librte_pmd_vhost.so.1
librte_pmd_ixgbe.so.1
librte_pmd_vmxnet3_uio.so.1
librte_pmd_fm10k.so.1
librte_pmd_i40e.so.1
librte_pmd_null.so.1
librte_pmd_pcap.so.1
librte_pmd_null_crypto.so.1
librte_pmd_ena.so.1
librte_pmd_e1000.so.1
librte_pmd_qede.so.1
librte_pmd_bond.so.1
librte_pmd_enic.so.1
[pmatilai at sopuli ~]$

>
>>> However it is a good idea to handle this case.

Yup. But there's also a bit of a gotcha involved with the virtual 
devices with added api. This is how eg testpmd looks when built in 
shared library config:

[pmatilai at sopuli ~]$ pmdinfo /usr/bin/testpmd
Scanning /usr/lib64/librte_pmd_bond.so.1 for pmd information
PMD NAME: bonding
PMD TYPE: PMD_VDEV
PMD PARAMETERS: slave= primary= mode=[0-4] xmit_policy=[l2 | 
l23 | l34] socket_id= mac= lsc_poll_period_ms= 
up_delay= down_delay=

[pmatilai at sopuli ~]$

Having support for bonding driver but nothing else will seem pretty damn 
confusing unless you happen to be a dpdk developer knowing this fact 
about some pmds also providing API and being linked directly etc.

Might make sense to have extra cli switch to control which kind of pmds 
are shown, and maybe default to skipping vdevs since they dont provide 
any actual hardware support anyway. Showing nothing at all in the above 
case would likely be a little less confusing. Maybe :)

Anyway, this is "refine later if needed once we gain more experience" 
material to me.

- Panu -

[dpdk-dev] [PATCH] mk: fix underlinking issues of most librte libraries

[Panu Matilainen]

On 05/26/2016 04:13 PM, Ferruh Yigit wrote:
> On 5/24/2016 11:11 AM, Christian Ehrhardt wrote:
>> Hi Panu,
>> I already agreed to Thomas on IRC but won't have time until next week.
>> Thanks for making a patch that does that already - I'll give it a look and
>> some test on my end next week then.
>>
>>
>>
>> Christian Ehrhardt
>> Software Engineer, Ubuntu Server
>> Canonical Ltd
>>
>> On Tue, May 24, 2016 at 11:56 AM, Panu Matilainen 
>> wrote:
>>
>>> On 05/20/2016 08:08 PM, Thomas Monjalon wrote:
>>>
>>>> 2016-05-20 18:50, Christian Ehrhardt:
>>>>
>>>>> The individual libraries have various cross dependencies.
>>>>> This is already refelcted in the DEPDIR dependency, but not yet in
>>>>> proper DT_NEEDED flags in the .so's.
>>>>> This adds the -l flags so that is properly stored in the .so's ELF
>>>>> headers.
>>>>>
>>>>
>>>> Why not filling LDLIBS by parsing DEPDIRS-y in rte.lib.mk?
>>>>
>>>>
>>> A fair question :) The thought has passed my mind too, but I thought it'd
>>> be too messy with differing library vs directory names and other
>>> exceptions. But in reality manually maintained separate LDLIBS is only
>>> going to get out of sync sooner or later, and turns out its not that bad
>>> anyway:
>>>
>>> diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
>>> index b420280..88b4e98 100644
>>> --- a/mk/rte.lib.mk
>>> +++ b/mk/rte.lib.mk
>>> @@ -77,6 +77,12 @@ else
>>>  _CPU_LDFLAGS := $(CPU_LDFLAGS)
>>>  endif
>>>
>>> +# Translate DEPDIRS-y into LDLIBS
>>> +IGNORE_DEPS = -lrte_eal/% -lrte_net -lrte_compat
>>> +_LDDIRS = $(subst librte_ether,libethdev,$(DEPDIRS-y))
>>> +_LDDEPS = $(subst lib/lib,-l,$(_LDDIRS))
>>> +LDLIBS += $(filter-out $(IGNORE_DEPS), $(_LDDEPS))
>>> +
>>>  O_TO_A = $(AR) crDs $(LIB) $(OBJS-y)
>>>  O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight
>>>  O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)","  AR $(@)")
>>>
>>> I'm also sure somebody more familiar with gmake could improve it, but it
>>> seems to work quite fine here. I'll wait a bit to see if there are other
>>> suggestions before sending it as a proper patch.
>>>
>>> - Panu -
>>>
>
> I did test the patch with minor diff in rte.vars.mk [1], otherwise all
> libraries kept needed in final library.
>
> Patch itself looks good, overlinking problem in final binary seems fixed
> [2] [3].
>
> But this cause a compilation error for cmdline_test [4]. This is because
> of cyclic dependency between librte_eal <-> librte_mempool.
>
> Other applications don't have this compilation error because they use
> librte_mempool, and since it is linked against application,  this error
> not observed. cmdline_test itself doesn't have any direct call to
> librte_mempool.
>
> It is possible to add workaround to cmdline_test [5], (introduce
> overlinking back), but should we attack on cyclic dependency instead?

Cyclic dependencies are nasty, eliminating it would be good.

But in the meanwhile it can be worked around in a generic manner by 
handling it in the libdpdk.so linker script, see 
http://dpdk.org/ml/archives/dev/2016-May/039413.html

- Panu -

>
> Thanks,
> ferruh
>
> ---
>
> [1]
> diff --git a/mk/exec-env/linuxapp/rte.vars.mk
> b/mk/exec-env/linuxapp/rte.vars.mk
> index d51bd17..10d37d5 100644
> --- a/mk/exec-env/linuxapp/rte.vars.mk
> +++ b/mk/exec-env/linuxapp/rte.vars.mk
> @@ -46,7 +46,7 @@ EXECENV_CFLAGS  = -pthread
>  endif
>
>  # Workaround lack of DT_NEEDED entry
> -EXECENV_LDFLAGS = --no-as-needed
> +EXECENV_LDFLAGS = --as-needed
>
>  EXECENV_LDLIBS  =
>  EXECENV_ASFLAGS =
>
>
> [2]
> # ldd build/app/testpmd
> linux-vdso.so.1 (0x7ffcff92e000)
> librte_mbuf.so.2.1 => .../build/lib/librte_mbuf.so.2.1
> libethdev.so.4.1 => .../build/lib/libethdev.so.4.1
> librte_mempool.so.2.1 => .../build/lib/librte_mempool.so.2.1
> librte_ring.so.1.1 => .../build/lib/librte_ring.so.1.1
> librte_eal.so.2.1 => .../build/lib/librte_eal.so.2.1
> librte_cmdline.so.2.1 => .../build/lib/librte_cmdline.so.2.1
> librte_pmd_bond.so.1.1 => .../build/lib/librte_pmd_bond.so.1.1
> libpthread.so.0 => /lib64/libpthread.so.0 (0x7f6879ff1000)
> libc.so.6 => /lib64/libc.so.6 (0x7f6879c3)
> /lib64/ld-linux-x86-64.so.2 (0x0

[dpdk-dev] [PATCHv4 0/5] Implement pmd hardware support exports

[Panu Matilainen]

On 05/24/2016 10:41 PM, Neil Horman wrote:
> Hey all-
>   So heres attempt number 2 at a method for exporting PMD hardware support
> information.  As we discussed previously, the consensus seems to be that pmd
> information should be:
>
> 1) Able to be interrogated on any ELF binary (application binary or individual
> DSO)
> 2) Equally functional on statically linked applications or on DSO's
> 3) Resilient to symbol stripping
> 4) Script friendly
> 5) Show kernel dependencies
> 6) List driver options
> 7) Show driver name
> 8) Offer human readable output
> 9) Show DPDK version
> 10) Show driver version
> 11) Allow for expansion
> 12) Not place additional build environment dependencies on an application
>
[...]
> v4)
>  * Modified the operation of the -p option. As much as I don't like implying
> that autoloaded pmds are guaranteed to be there at run time, I'm having a hard
> time seeing how we can avoid specifying the application file to scan for the
> autoload directory.  Without it we can't determine which library the user 
> means
> in a multiversion installation
>  * Cleaned up the help text
>  * Added a rule for an install target for pmdinfo
>  * Guarded against some tracebacks in pmdinfo
>  * Use DT_NEEDED entries to get versioned libraries in -p mode

Thank you! That's exactly what I've been asking for all along.

>  * Fixed traceback that occurs on lack of input arguments
>  * Fixed some erroneous macro usage in drivers that aren't in the default 
> build
>
> Signed-off-by: Neil Horman 
> CC: Bruce Richardson 
> CC: Thomas Monjalon 
> CC: Stephen Hemminger 
> CC: Panu Matilainen 

/me happy now, so:

Acked-by: Panu Matilainen 

As always there might be some refining to do as we get more experience 
with it but it seems like a fine starting point to me.

- Panu -

[dpdk-dev] [PATCH] qede: add missing external dependency and disable by default

[Panu Matilainen]

The qede driver depends on libz but the LDLIBS entry in makefile
was missing. Also because of the external dependency, make it
disabled in default config as per common DPDK policy on external deps.

Fixes: ec94dbc57362 ("qede: add base driver")

Signed-off-by: Panu Matilainen 
---
 config/common_base| 2 +-
 drivers/net/qede/Makefile | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/config/common_base b/config/common_base
index 3535c6e..47c26f6 100644
--- a/config/common_base
+++ b/config/common_base
@@ -299,7 +299,7 @@ CONFIG_RTE_LIBRTE_BOND_DEBUG_ALB_L1=n

 # QLogic 25G/40G PMD
 #
-CONFIG_RTE_LIBRTE_QEDE_PMD=y
+CONFIG_RTE_LIBRTE_QEDE_PMD=n
 CONFIG_RTE_LIBRTE_QEDE_DEBUG_INIT=n
 CONFIG_RTE_LIBRTE_QEDE_DEBUG_INFO=n
 CONFIG_RTE_LIBRTE_QEDE_DEBUG_DRV=n
diff --git a/drivers/net/qede/Makefile b/drivers/net/qede/Makefile
index 4cc9ee8..c9b3b1c 100644
--- a/drivers/net/qede/Makefile
+++ b/drivers/net/qede/Makefile
@@ -14,6 +14,8 @@ LIB = librte_pmd_qede.a
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)

+LDLIBS += -lz
+
 EXPORT_MAP := rte_pmd_qede_version.map

 LIBABIVER := 1
-- 
2.5.5

[dpdk-dev] [PATCH] mk: fix underlinking issues of most librte libraries

[Panu Matilainen]

On 05/20/2016 08:08 PM, Thomas Monjalon wrote:
> 2016-05-20 18:50, Christian Ehrhardt:
>> The individual libraries have various cross dependencies.
>> This is already refelcted in the DEPDIR dependency, but not yet in
>> proper DT_NEEDED flags in the .so's.
>> This adds the -l flags so that is properly stored in the .so's ELF
>> headers.
>
> Why not filling LDLIBS by parsing DEPDIRS-y in rte.lib.mk?
>

A fair question :) The thought has passed my mind too, but I thought 
it'd be too messy with differing library vs directory names and other 
exceptions. But in reality manually maintained separate LDLIBS is only 
going to get out of sync sooner or later, and turns out its not that bad 
anyway:

diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index b420280..88b4e98 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -77,6 +77,12 @@ else
  _CPU_LDFLAGS := $(CPU_LDFLAGS)
  endif

+# Translate DEPDIRS-y into LDLIBS
+IGNORE_DEPS = -lrte_eal/% -lrte_net -lrte_compat
+_LDDIRS = $(subst librte_ether,libethdev,$(DEPDIRS-y))
+_LDDEPS = $(subst lib/lib,-l,$(_LDDIRS))
+LDLIBS += $(filter-out $(IGNORE_DEPS), $(_LDDEPS))
+
  O_TO_A = $(AR) crDs $(LIB) $(OBJS-y)
  O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight
  O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)","  AR $(@)")

I'm also sure somebody more familiar with gmake could improve it, but it 
seems to work quite fine here. I'll wait a bit to see if there are other 
suggestions before sending it as a proper patch.

- Panu -

[dpdk-dev] [PATCHv3 5/5] pmdinfo.py: Add tool to query binaries for hw and other support information

[Panu Matilainen]

On 05/20/2016 08:24 PM, Neil Horman wrote:
> This tool searches for the primer sting PMD_DRIVER_INFO= in any ELF binary,
> and, if found parses the remainder of the string as a json encoded string,
> outputting the results in either a human readable or raw, script parseable
> format
>
> Note that, in the case of dynamically linked applications, pmdinfo.py will 
> scan
> for implicitly linked PMDs by searching the specified binaries .dynamic 
> section
> for DT_NEEDED entries that contain the substring librte_pmd.  The DT_RUNPATH,
> LD_LIBRARY_PATH, /usr/lib and /lib are searched for these libraries, in that
> order
>
> If a file is specified with no path, it is assumed to be a PMD DSO, and the
> LD_LIBRARY_PATH, /usr/lib[64]/ and /lib[64] is searched for it
>
> Currently the tool can output data in 3 formats:
>
> a) raw, suitable for scripting, where the raw JSON strings are dumped out
> b) table format (default) where hex pci ids are dumped in a table format
> c) pretty, where a user supplied pci.ids file is used to print out vendor and
> device strings
>
> Signed-off-by: Neil Horman 
> CC: Bruce Richardson 
> CC: Thomas Monjalon 
> CC: Stephen Hemminger 
> CC: Panu Matilainen 
> ---
>  tools/pmdinfo.py | 545 
> +++
>  1 file changed, 545 insertions(+)
>  create mode 100755 tools/pmdinfo.py

Oh, one more thing: assuming there'll be a v4, please add something like 
this to make the tool available in $PATH:

diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk
index 68e56b6..5a6a699 100644
--- a/mk/rte.sdkinstall.mk
+++ b/mk/rte.sdkinstall.mk
@@ -126,6 +126,8 @@ install-runtime:
 $(Q)$(call rte_mkdir,  $(DESTDIR)$(sbindir))
 $(Q)$(call rte_symlink, 
$(DESTDIR)$(datadir)/tools/dpdk_nic_bind.py, \
$(DESTDIR)$(sbindir)/dpdk_nic_bind)
+   $(Q)$(call rte_symlink,$(DESTDIR)$(datadir)/tools/pmdinfo.py, \
+  $(DESTDIR)$(bindir)/pmdinfo)

  install-kmod:
  ifneq ($(wildcard $O/kmod/*),)

- Panu -

[dpdk-dev] [PATCHv3 5/5] pmdinfo.py: Add tool to query binaries for hw and other support information

[Panu Matilainen]

On 05/20/2016 08:24 PM, Neil Horman wrote:
> This tool searches for the primer sting PMD_DRIVER_INFO= in any ELF binary,
> and, if found parses the remainder of the string as a json encoded string,
> outputting the results in either a human readable or raw, script parseable
> format
>
> Note that, in the case of dynamically linked applications, pmdinfo.py will 
> scan
> for implicitly linked PMDs by searching the specified binaries .dynamic 
> section
> for DT_NEEDED entries that contain the substring librte_pmd.  The DT_RUNPATH,
> LD_LIBRARY_PATH, /usr/lib and /lib are searched for these libraries, in that
> order
>
> If a file is specified with no path, it is assumed to be a PMD DSO, and the
> LD_LIBRARY_PATH, /usr/lib[64]/ and /lib[64] is searched for it
>
> Currently the tool can output data in 3 formats:
>
> a) raw, suitable for scripting, where the raw JSON strings are dumped out
> b) table format (default) where hex pci ids are dumped in a table format
> c) pretty, where a user supplied pci.ids file is used to print out vendor and
> device strings
>
> Signed-off-by: Neil Horman 
> CC: Bruce Richardson 
> CC: Thomas Monjalon 
> CC: Stephen Hemminger 
> CC: Panu Matilainen 
> ---
>  tools/pmdinfo.py | 545 
> +++
>  1 file changed, 545 insertions(+)
>  create mode 100755 tools/pmdinfo.py
>
> diff --git a/tools/pmdinfo.py b/tools/pmdinfo.py
> new file mode 100755
> index 000..9b4b4a4
> --- /dev/null
> +++ b/tools/pmdinfo.py
> @@ -0,0 +1,545 @@
> +#!/usr/bin/python
> +#---
> +# scripts/pmd_hw_support.py
> +#
> +# Utility to dump PMD_INFO_STRING support from an object file
> +#
> +#---
> +import os, sys
> +from optparse import OptionParser
> +import string
> +import json
> +
> +# For running from development directory. It should take precedence over the
> +# installed pyelftools.
> +sys.path.insert(0, '.')
> +
> +
> +from elftools import __version__
> +from elftools.common.exceptions import ELFError
> +from elftools.common.py3compat import (
> +ifilter, byte2int, bytes2str, itervalues, str2bytes)
> +from elftools.elf.elffile import ELFFile
> +from elftools.elf.dynamic import DynamicSection, DynamicSegment
> +from elftools.elf.enums import ENUM_D_TAG
> +from elftools.elf.segments import InterpSegment
> +from elftools.elf.sections import SymbolTableSection
> +from elftools.elf.gnuversions import (
> +GNUVerSymSection, GNUVerDefSection,
> +GNUVerNeedSection,
> +)
> +from elftools.elf.relocation import RelocationSection
> +from elftools.elf.descriptions import (
> +describe_ei_class, describe_ei_data, describe_ei_version,
> +describe_ei_osabi, describe_e_type, describe_e_machine,
> +describe_e_version_numeric, describe_p_type, describe_p_flags,
> +describe_sh_type, describe_sh_flags,
> +describe_symbol_type, describe_symbol_bind, describe_symbol_visibility,
> +describe_symbol_shndx, describe_reloc_type, describe_dyn_tag,
> +describe_ver_flags,
> +)
> +from elftools.elf.constants import E_FLAGS
> +from elftools.dwarf.dwarfinfo import DWARFInfo
> +from elftools.dwarf.descriptions import (
> +describe_reg_name, describe_attr_value, set_global_machine_arch,
> +describe_CFI_instructions, describe_CFI_register_rule,
> +describe_CFI_CFA_rule,
> +)
> +from elftools.dwarf.constants import (
> +DW_LNS_copy, DW_LNS_set_file, DW_LNE_define_file)
> +from elftools.dwarf.callframe import CIE, FDE
> +
> +raw_output = False
> +pcidb = None
> +
> +#===
> +
> +class Vendor:
> +"""
> +Class for vendors. This is the top level class
> +for the devices belong to a specific vendor.
> +self.devices is the device dictionary
> +subdevices are in each device.
> +"""
> +def __init__(self, vendorStr):
> +"""
> +Class initializes with the raw line from pci.ids
> +Parsing takes place inside __init__
> +"""
> +self.ID = vendorStr.split()[0]
> +self.name = vendorStr.replace("%s " % self.ID,"").rstrip()
> +self.devices = {}
> +
> +def addDevice(self, deviceStr):
> +"""
> +Adds a device to self.devices
> +takes the raw line from pci.ids
> +"""
> +s = deviceStr.strip()
> +devID = s.split()[0]
> +if devID in self.devices:
> + 

[dpdk-dev] [PATCHv3 2/5] drivers: Update driver registration macro usage

[Panu Matilainen]

On 05/20/2016 08:24 PM, Neil Horman wrote:
> Modify the PMD_REGISTER_DRIVER macro, adding a name argument to it.  The
> addition of a name argument creates a token that can be used for subsequent
> macros in the creation of unique symbol names to export additional bits of
> information for use by the pmdinfogen tool.  For example:
>
> PMD_REGISTER_DRIVER(ena_driver, ena);
>
[..]
> diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c 
> b/drivers/net/bnx2x/bnx2x_ethdev.c
> index 071b44f..ee9581b 100644
> --- a/drivers/net/bnx2x/bnx2x_ethdev.c
> +++ b/drivers/net/bnx2x/bnx2x_ethdev.c
> @@ -550,5 +550,7 @@ static struct rte_driver rte_bnx2xvf_driver = {
>   .init = rte_bnx2xvf_pmd_init,
>  };
>
> -PMD_REGISTER_DRIVER(rte_bnx2x_driver);
> -PMD_REGISTER_DRIVER(rte_bnx2xvf_driver);
> +PMD_REGISTER_DRIVER(rte_bnx2x_driver, pci_id_bnx2x_map, bnx2x);
> +DRIVER_EXPORT_PCI_TABLE(bnx2x, pci_id_bnx2x_map);
> +PMD_REGISTER_DRIVER(rte_bnx2xvf_driver, pci_id_bnx2xvf_map, bnx2xvf);
> +DRIVER_EXPORT_PCI_TABLE(bnx2xvf, pci_id_bnx2xvf_map);

There are extra arguments to PMD_REGISTER_DRIVER() here, causing build 
failure. Its easy to miss since bnx2x doesn't get built by default, 
dunno if it was in earlier versions already (if it was, sorry for 
missing it).

- Panu -

[dpdk-dev] [PATCH 4/4] pmd_hw_support.py: Add tool to query binaries for hw support information

[Panu Matilainen]

On 05/23/2016 04:55 PM, Neil Horman wrote:
> On Mon, May 23, 2016 at 02:56:18PM +0300, Panu Matilainen wrote:
>> On 05/20/2016 05:06 PM, Neil Horman wrote:
>> [...]
>>> Look, I think we're simply not going to agree on this issue at all.  What 
>>> about
>>> this in the way of compromise.  I simply am not comfortable with 
>>> automatically
>>> trying to guess what hardware support will exist in an application based on 
>>> the
>>> transient contents of a plugin directory, because of all the reasons we've
>>> already gone over, but I do understand the desire to get information about 
>>> what
>>> _might_ be automatically loaded for an application.  what if we added a 
>>> 'plugin
>>> mode' to pmdinfo. In this mode you would dpecify a dpdk installation 
>>> directory
>>> and an appropriate mode option.  When specified pmdinfo would scan 
>>> librte_eal in
>>> the specified directory, looking for an exported json string that informs 
>>> us of
>>> the configured plugin directory.  If found, we iterate through all the 
>>> libraries
>>> there displaying hw support.  That allows you to query the plugin directory 
>>> for
>>> available hardware support, while not implying that the application is
>>> guaranteed to get it (because you didn't specifically state on the command 
>>> line
>>> that you wanted to know about the applications hardware support).
>>
>> That brings it all to one tiny step away from what I've been asking: have
>> the plugin mode automatically locate librte_eal from an executable. So I'm
>> not quite sure where the compromise is supposed to be here :)
> The compromise is that I'm not willing to quietly assume that a given
> application linked to the dpdk library in /usr/lib64/dpdk-, will get
> hardware support for the cxgb4, mlx5 and ixgbe pmds, because those DSO's are 
> in
> the exported RTE_EAL_PMD_PATH.

Why not? I dont get it.

> With this method, you at least have to tell the
> pmdinfo application that I wish to scan that path for pmds and report on
> hardware support for whatever is found there.  Thats a different operation 
> from
> getting a report on what hardware an application supports.  i.e. its the
> difference between asking the questions:
>
> "What hardware does the application /usr/bin/foo support"
> and
> "What hardware support is available via the plugin DSO's pointed to by the 
> dpdk
> version in /usr/lib64/dpdk-2.2"

Well, for the application to be able to load any PMDs, it will have to 
be linked to some version of librte_eal, which will have to be somewhere 
in the library search path (or use rpath).


> I feel its important for users to understand that autoloading doesn't not
> guarantee support for the hardware that is autoloaded to any application.  My
> compromise is to provide what your asking for, but doing so in a way that
> attempts to make that differentiation clear.

I would think requiring a specific option to enable the plugin scan 
should be quite enough to make that point clear.

>>
>> I do appreciate wanting to differentiate between "physically" linked-in and
>> runtime-loaded hardware support, they obviously *are* different from a
>> technical POV. But its also a difference an average user might not even know
>> about or understand, let alone care about, they just want to know "will it
>> work?"
>>
>
> Which average user are we talking about here?  Keep in mind the DPDK is
> anything but mainstream.  Its a toolkit to implement high speed network
> communications for niche or custom purpose applications.  The users of tools
> like this are going to be people who understand the nuance of how
> applications are built and want to tune them to work at their most efficient
> point, not some person just trying to get firefox to connect to digg.  I think
> its reasonable to assume that people who are running the tool have sufficient
> knoweldge to understand that DSO's and static binaries may embody hardware
> support differently, and that things which are loaded at run time may not be
> reported at scan time (by way of corressponding example, though its not 
> perfect,
> people seem to understand that iscsid supports lots of different HBA's, but 
> the
> specific hardware support for those HBA's isn't codified in /usr/sbin/iscsid,
> but rather in the modules under /lib/modules/ thats
> a lousy comparison, as the notion of static linkage of the kernel doesn't 
> really
> compare to static application linkage, but still, people can figure out whats
> going on there pretty readily, and I think they ca

[dpdk-dev] [PATCH 4/4] pmd_hw_support.py: Add tool to query binaries for hw support information

[Panu Matilainen]

On 05/20/2016 05:06 PM, Neil Horman wrote:
[...]
> Look, I think we're simply not going to agree on this issue at all.  What 
> about
> this in the way of compromise.  I simply am not comfortable with automatically
> trying to guess what hardware support will exist in an application based on 
> the
> transient contents of a plugin directory, because of all the reasons we've
> already gone over, but I do understand the desire to get information about 
> what
> _might_ be automatically loaded for an application.  what if we added a 
> 'plugin
> mode' to pmdinfo. In this mode you would dpecify a dpdk installation directory
> and an appropriate mode option.  When specified pmdinfo would scan librte_eal 
> in
> the specified directory, looking for an exported json string that informs us 
> of
> the configured plugin directory.  If found, we iterate through all the 
> libraries
> there displaying hw support.  That allows you to query the plugin directory 
> for
> available hardware support, while not implying that the application is
> guaranteed to get it (because you didn't specifically state on the command 
> line
> that you wanted to know about the applications hardware support).

That brings it all to one tiny step away from what I've been asking: 
have the plugin mode automatically locate librte_eal from an executable. 
So I'm not quite sure where the compromise is supposed to be here :)

I do appreciate wanting to differentiate between "physically" linked-in 
and runtime-loaded hardware support, they obviously *are* different from 
a technical POV. But its also a difference an average user might not 
even know about or understand, let alone care about, they just want to 
know "will it work?"

- Panu -

[dpdk-dev] [PATCH] mk: Fix cruft getting installed by "make install" with tar >= 1.29

[Panu Matilainen]

--exclude became a positional option in tar 1.29, breaking the
test app filtering in "make install", causing .map files and all test
apps to get installed in bindir. Adjust the tar arguments accordingly,
this is compatible with older versions too since they do not care about
the order.

Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1337864

Signed-off-by: Panu Matilainen 
---
 mk/rte.sdkinstall.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk
index 68e56b6..abdab0f 100644
--- a/mk/rte.sdkinstall.mk
+++ b/mk/rte.sdkinstall.mk
@@ -116,9 +116,9 @@ install-runtime:
$(Q)$(call rte_mkdir, $(DESTDIR)$(libdir))
$(Q)cp -a$O/lib/* $(DESTDIR)$(libdir)
$(Q)$(call rte_mkdir, $(DESTDIR)$(bindir))
-   $(Q)tar -cf -  -C $O app  --exclude 'app/*.map' \
+   $(Q)tar -cf -  -C $O --exclude 'app/*.map' \
--exclude 'app/cmdline*' --exclude app/test \
-   --exclude app/testacl --exclude app/testpipeline | \
+   --exclude app/testacl --exclude app/testpipeline app | \
tar -xf -  -C $(DESTDIR)$(bindir) --strip-components=1 \
--keep-newer-files --warning=no-ignore-newer
$(Q)$(call rte_mkdir,  $(DESTDIR)$(datadir))
-- 
2.5.5

[dpdk-dev] Underlinked libs and overlinked applications - an issue?

[Panu Matilainen]

On 05/19/2016 09:17 PM, Thomas Monjalon wrote:
> 2016-05-19 17:38, Christian Ehrhardt:
>> Hi,
>> I was working on the new 16.04 build system to adapt deb packaging to it.
>> I remember somewhen back in the DPDK 2.2 and shared+combined library days I
>> had some issues with over/underlinking - but it seems those are still
>> existent or came back.
>
> I would say it has always been like that.
> Thanks for raising the issue.
>
>> After a build in almost default config (just disabled the kernel modules)
>> and set RTE_MACHINE to default I find the following.
>>
>> #1
>> The libraries are all only linked against external things - even clearly
>> using internal structures:
>> ldd usr/lib/x86_64-linux-gnu/librte_lpm.so.2
>>linux-vdso.so.1 =>  (0x7fff7e7a5000)
>>libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x7f175d4dd000)
>>/lib64/ld-linux-x86-64.so.2 (0x558d3afbf000)
>
> The DT_NEEDED entries are created only for external dependencies.
> Probably we should create ones for internal dependencies based on the
> variable DEPDIRS-y.

Yes, DT_NEEDED for internal dependencies are needed too, see eg recent 
commits c6417ce61f83 and 8180554d82b3. That was part of Sergios' build 
system improvement patch series last spring or so (but since then 
abandoned), I picked up the work and been doing it in smaller pieces. 
Phase 1 was the external dependencies, phase 2 is internal dependencies 
which I'm working on. Unfortunately health issues have stalled my work a 
lot recently :-/

>> #2
>> The Application then seem to try to make up for that by realizing all that
>> is missing.
>> But looking at the app alone it seems overlinked by that - it is not using
>> all of these on its own.
>> ldd usr/bin/cmdline_test
>>linux-vdso.so.1 =>  (0x7ffeec9ea000)
>>librte_distributor.so.1 => not found
>>librte_reorder.so.1 => not found
>> [...]
>>librte_jobstats.so.1 => not found
>> [...]
>> And for example none of the librte_jobstats.so.1 symbols are used
>> "directly" in there.
>
> Yes every libraries are put for every apps in rte.app.mk.
> Probably that we could better use DEPDIRS-y for apps.
>

Another trick from Sergios' original patch series is to utilize 
AS_NEEDED() in the linker script, so it ends up looking something like 
this for shared library config (static would remain as it is now):

INPUT ( librte_eal librte_mempool librte_ring
 AS_NEEDED ( librte_acl librte_timer librte_vhost <...> )
)

...and then use the linker script for linking the apps, which should 
simplify rte.app.mk considerably. Or so the theory goes. Anyway, the 
above requires DT_NEEDED for the internal libraries to be present first, 
but once all these pieces are in place, dynamically linked apps will 
only link to the libraries they actually need.

- Panu -

- Panu -

[dpdk-dev] [PATCHv2 4/4] pmdinfo.py: Add tool to query binaries for hw and other support information

[Panu Matilainen]

On 05/20/2016 11:55 AM, Thomas Monjalon wrote:
> 2016-05-20 08:22, Panu Matilainen:
>> Ability to query individual DSOs is a building block for other things
>> like automation (you dont expect normal users to go manually loading hw
>> support modules for the OS either), but its not an end-user solution.
>>
>> Thomas said in http://dpdk.org/ml/archives/dev/2016-May/038324.html:
>>
>> "This tool should not behave differently depending of how DPDK was
>> compiled (static or shared)."
>
> I meant the basic tool must be usable on static binary and shared library.

Well, I'm not sure I would interpret that any differently in terms of 
what to expect. But since its all up to interpretation and different 
points of view...

>> Its entirely possible to handle all the three above cases virtually
>> identically (point the tool to the app executable), so I have a hard
>> time understanding the level of resistance to handling the plugin case.
>
> We need first a basic tool. Then we'll check how to build on it for
> end user needs. I think you have some good ideas.
> We could also try (later) to inspect a running DPDK app.
>

...I'll just say fair enough, I've made my points and I'll go shut up 
for now.

- Panu -

[dpdk-dev] [PATCH v3 00/35] mempool: rework memory allocation

[Panu Matilainen]

On 05/19/2016 03:47 PM, Thomas Monjalon wrote:
> 2016-05-18 13:04, Olivier Matz:
>> This series is a rework of mempool. For those who don't want to read
>> all the cover letter, here is a sumary:
>>
>> - it is not possible to allocate large mempools if there is not enough
>>   contiguous memory, this series solves this issue
>> - introduce new APIs with less arguments: "create, populate, obj_init"
>> - allow to free a mempool
>> - split code in smaller functions, will ease the introduction of ext_handler
>> - remove test-pmd anonymous mempool creation
>> - remove most of dom0-specific mempool code
>> - opens the door for a eal_memory rework: we probably don't need large
>>   contiguous memory area anymore, working with pages would work.
>>
>> This breaks the ABI as it was indicated in the deprecation for 16.04.
>> The API stays almost the same, no modification is needed in examples app
>> or in test-pmd. Only kni and mellanox drivers are slightly modified.
>
> Applied with a small change you sent me to fix mlx build in the middle of the 
> patchset
> and update the removed Xen files in MAINTAINERS file.
>
> Thanks for the big rework!
>

Just noticed this series "breaks" --no-huge as a regular user, commit 
593a084afc2b to be exact:

mmap(NULL, 4194304, PROT_READ|PROT_WRITE, 
MAP_PRIVATE|MAP_ANONYMOUS|MAP_LOCKED, 0, 0) = -1 EAGAIN (Resource 
temporarily unavailable)
write(1, "EAL: rte_eal_hugepage_init: mmap"..., 76EAL: 
rte_eal_hugepage_init: mmap() failed: Resource temporarily unavailable

"Breaks" in quotes because I guess it always was broken (as the 
non-locked pages might not be in physical memory) and because its
possible to adjust resourse limits to allow the operation to succeed.
If you're root, that is.

I was just looking into making the test-suite runnable by a regular user 
with no special privileges, primarily to make it possible to run the 
testsuite as part of rpm package builds (in %check), and no special 
setup or extra privileges can be assumed there. Such tests are of course 
of limited coverage but still better than nothing, and --no-huge was my 
ticket there. Talk about bad timing :)

It'd be fine to have limited subset of tests to run when non-privileged 
but since this one lives inside rte_eal_init() it practically prevents 
everything, unless I'm missing some other magic switch or such. Thoughts?

- Panu -

[dpdk-dev] [PATCH 4/4] pmd_hw_support.py: Add tool to query binaries for hw support information

[Panu Matilainen]

On 05/19/2016 04:26 PM, Neil Horman wrote:
> On Thu, May 19, 2016 at 09:08:52AM +0300, Panu Matilainen wrote:
>> On 05/18/2016 04:48 PM, Neil Horman wrote:
>>> On Wed, May 18, 2016 at 03:48:12PM +0300, Panu Matilainen wrote:
>>>> On 05/18/2016 03:03 PM, Neil Horman wrote:
>>>>> On Wed, May 18, 2016 at 02:48:30PM +0300, Panu Matilainen wrote:
>>>>>> On 05/16/2016 11:41 PM, Neil Horman wrote:
>>>>>>> This tool searches for the primer sting PMD_DRIVER_INFO= in any ELF 
>>>>>>> binary,
>>>>>>> and, if found parses the remainder of the string as a json encoded 
>>>>>>> string,
>>>>>>> outputting the results in either a human readable or raw, script 
>>>>>>> parseable
>>>>>>> format
>>>>>>>
>>>>>>> Signed-off-by: Neil Horman 
>>>>>>> CC: Bruce Richardson 
>>>>>>> CC: Thomas Monjalon 
>>>>>>> CC: Stephen Hemminger 
>>>>>>> CC: Panu Matilainen 
>>>>>>> ---
>>>>>>>  tools/pmd_hw_support.py | 174 
>>>>>>> 
>>>>>>>  1 file changed, 174 insertions(+)
>>>>>>>  create mode 100755 tools/pmd_hw_support.py
>>>>>>>
>>>>>>> diff --git a/tools/pmd_hw_support.py b/tools/pmd_hw_support.py
>>>>>>> new file mode 100755
>>>>>>> index 000..0669aca
>>>>>>> --- /dev/null
>>>>>>> +++ b/tools/pmd_hw_support.py
>>>>>>> @@ -0,0 +1,174 @@
>>>>>>> +#!/usr/bin/python3
>>>>>>
>>>>>> I think this should use /usr/bin/python to be consistent with the other
>>>>>> python scripts, and like the others work with python 2 and 3. I only 
>>>>>> tested
>>>>>> it with python2 after changing this and it seemed to work fine so the
>>>>>> compatibility side should be fine as-is.
>>>>>>
>>>>> Sure, I can change the python executable, that makes sense.
>>>>>
>>>>>> On the whole, AFAICT the patch series does what it promises, and works 
>>>>>> for
>>>>>> both static and shared linkage. Using JSON formatted strings in an ELF
>>>>>> section is a sound working technical solution for the storage of the 
>>>>>> data.
>>>>>> But the difference between the two cases makes me wonder about this 
>>>>>> all...
>>>>> You mean the difference between checking static binaries and dynamic 
>>>>> binaries?
>>>>> yes, there is some functional difference there
>>>>>
>>>>>>
>>>>>> For static library build, you'd query the application executable, eg
>>>>> Correct.
>>>>>
>>>>>> testpmd, to get the data out. For a shared library build, that method 
>>>>>> gives
>>>>>> absolutely nothing because the data is scattered around in individual
>>>>>> libraries which might be just about wherever, and you need to somehow
>>>>> Correct, I figured that users would be smart enough to realize that with
>>>>> dynamically linked executables, they would need to look at DSO's, but I 
>>>>> agree,
>>>>> its a glaring diffrence.
>>>>
>>>> Being able to look at DSOs is good, but expecting the user to figure out
>>>> which DSOs might be loaded and not and where to look is going to be well
>>>> above many users. At very least it's not what I would call user-friendly.
>>>>
>>> I disagree, there is no linkage between an application and the dso's it 
>>> opens
>>> via dlopen that is exportable.  The only way to handle that is to have a
>>> standard search path for the pmd_hw_info python script.  Thats just like 
>>> modinfo
>>> works (i.e. "modinfo bnx2" finds the bnx2 module for the running kernel).  
>>> We
>>> can of course do something simmilar, but we have no existing implicit path
>>> information to draw from to do that (because you can have multiple dpdk 
>>> installs
>>> side by side).  The only way around that is to explicitly call out the path 
>>> on
>>> the command line.
>>
>> There's no telling what

[dpdk-dev] [PATCH] scripts: remove unused map files merger

[Panu Matilainen]

On 05/19/2016 09:05 PM, Thomas Monjalon wrote:
> This script was forgotten when dropping the combined library.
>
> Fixes: 948fd64befc3 ("mk: replace the combined library with a linker script")
>
> Signed-off-by: Thomas Monjalon 
> ---

I haven't even noticed there was such a script :) Thanks for spotting.

Acked-by: Panu Matilainen 

- Panu -

[dpdk-dev] [PATCHv2 4/4] pmdinfo.py: Add tool to query binaries for hw and other support information

[Panu Matilainen]

On 05/19/2016 03:00 PM, Neil Horman wrote:
> On Thu, May 19, 2016 at 12:02:27PM +0300, Panu Matilainen wrote:
>> On 05/19/2016 12:08 AM, Neil Horman wrote:
>>> This tool searches for the primer sting PMD_DRIVER_INFO= in any ELF binary,
>>> and, if found parses the remainder of the string as a json encoded string,
>>> outputting the results in either a human readable or raw, script parseable
>>> format
>>>
>>> Note that, in the case of dynamically linked applications, pmdinfo.py will 
>>> scan
>>> for implicitly linked PMDs by searching the specified binaries .dynamic 
>>> section
>>> for DT_NEEDED entries that contain the substring librte_pmd.  The 
>>> DT_RUNPATH,
>>> LD_LIBRARY_PATH, /usr/lib and /lib are searched for these libraries, in that
>>> order
>>
>> Scanning /usr/lib and /lib does little good on systems where /usr/lib64 and
>> /lib64 are the standard path, such as x86_64 Fedora / RHEL and derivates.
>>
> Ah, sorry, forgot the 64 bit variants, I can add those in.
>
>> With the path changed (or LD_LIBRARY_PATH set manually), I can confirm it
>> works for a shared binary which is --whole-archive linked to all of DPDK
>> such as ovs-vswitchd currently is (because it needs to for static DPDK
>> linkage and is not aware of plugin autoloading).
>>
> Right, thats why it works, because DPDK always requires --whole-archive for
> static linking, and likely always will (see commit
> 20afd76a504155e947c770783ef5023e87136ad8)
>
>> It doesn't help testpmd though because its not linked with --whole-archive
>> in the shared case, so its not working for the main DPDK executable...
>>
> This sentence doesn't make sense --whole-archive is only applicable in the
> static binary case, and only when linking archive files.

Okay sorry I was indeed mixing up things here.

1) Testpmd doesn't get linked to those pmds at all, because of this in 
mk/rte.app.mk:

ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
# plugins (link only if static libraries)
...
endif

2) I'm could swear I've seen --whole-archive it make a difference on 
newer toolchains with the linker script, but I can't reproduce that now 
no matter what. Must've been hallucinating ;)

>
>> In any case, using --whole-archive is a sledgehammer solution at best, and
>> against the spirit of shared libs and plugins in particular.
>>
> It may be a sledgehammer solution, but its the one dpdk uses, and will likely
> use in perpituity.
>
>> I think the shared linkage case can be solved by exporting the PMD path from
>> librte_eal (either through an elf section or c-level symbol) and teach the
>> script to detect the case of an executable dynamically linked to librte_eal,
>> fish the path from there and then process everything in that path.
>>
> I really disagree with this, because its a half-measure at best.  Yes, if its
> set, you will definately get all the shared objects in that directory loaded,
> but that is in no way a guarantee that those are the only libraries that get
> loaded (the application may load its own independently).

That is in no way different from statically linked apps loading 
additional drivers with the EAL -d switch.

> So you're left in this
> situation in which you get maybe some of the hardware support an application
> offers.  Its also transient.  That is to say, if you configure a plugin
> directory and search it when you scan an application, its contents may change
> post scan, leading to erroneous results.

Yes, changing system state such as installing or removing packages 
between scanning and running can "do stuff" such as change results. 
People are quite aware of this because that's how a huge number of 
things in the system works, you install plugins for multimedia formats 
you need, maybe remove others you dont to clean up system etc. I fail to 
see how that is a problem when it's the expected behavior with plugins.

> The way I see it, we have 3 cases that we need to handle:
>
> 1) Statically linked application - in this case, all pmds that are statically
> linked in to the application will be reported, so we're good here
>
> 2) Dynamically loaded via DT_NEEDED entries - This is effectively the same as 
> a
> static linking case, in that we have a list of libraries that must be resolved
> at run time, so we are safe to search for and scan the DSO's that the
> application ennumerates
>
> 3) Dynamically loaded via dlopen - In this case, we don't actually know until
> runtime what DSO's are going to get loaded, even if RTE_EAL_PMD_PATH is set,
> because the contents of that path can change at arbitrary times.  In this 
> case,
> its correct to indicate that the application its

[dpdk-dev] [PATCHv2 4/4] pmdinfo.py: Add tool to query binaries for hw and other support information

[Panu Matilainen]

On 05/19/2016 12:08 AM, Neil Horman wrote:
> This tool searches for the primer sting PMD_DRIVER_INFO= in any ELF binary,
> and, if found parses the remainder of the string as a json encoded string,
> outputting the results in either a human readable or raw, script parseable
> format
>
> Note that, in the case of dynamically linked applications, pmdinfo.py will 
> scan
> for implicitly linked PMDs by searching the specified binaries .dynamic 
> section
> for DT_NEEDED entries that contain the substring librte_pmd.  The DT_RUNPATH,
> LD_LIBRARY_PATH, /usr/lib and /lib are searched for these libraries, in that
> order

Scanning /usr/lib and /lib does little good on systems where /usr/lib64 
and /lib64 are the standard path, such as x86_64 Fedora / RHEL and 
derivates.

With the path changed (or LD_LIBRARY_PATH set manually), I can confirm 
it works for a shared binary which is --whole-archive linked to all of 
DPDK such as ovs-vswitchd currently is (because it needs to for static 
DPDK linkage and is not aware of plugin autoloading).

It doesn't help testpmd though because its not linked with 
--whole-archive in the shared case, so its not working for the main DPDK 
executable...

In any case, using --whole-archive is a sledgehammer solution at best, 
and against the spirit of shared libs and plugins in particular.

I think the shared linkage case can be solved by exporting the PMD path 
from librte_eal (either through an elf section or c-level symbol) and 
teach the script to detect the case of an executable dynamically linked 
to librte_eal, fish the path from there and then process everything in 
that path.

>
> If a file is specified with no path, it is assumed to be a PMD DSO, and the
> LD_LIBRARY_PATH, /usr/lib/ and /lib is searched for it

Same as above, /usr/lib/ and /lib is incorrect for a large number of 
systems.

>
> Currently the tool can output data in 3 formats:
>
> a) raw, suitable for scripting, where the raw JSON strings are dumped out
> b) table format (default) where hex pci ids are dumped in a table format
> c) pretty, where a user supplied pci.ids file is used to print out vendor and
> device strings

c) is a nice addition. Would be even nicer if it knew the most common 
pci.ids locations so it doesn't need extra arguments in those cases, ie 
see if /usr/share/hwdata/pci.ids or /usr/share/misc/pci.ids exists and 
use that unless overridden on the cli.

- Panu -

[dpdk-dev] [PATCHv2 2/4] drivers: Update driver registration macro usage

[Panu Matilainen]

On 05/19/2016 12:08 AM, Neil Horman wrote:
> Modify the PMD_REGISTER_DRIVER macro, bifurcating it into two
> (PMD_REGISTER_DRIVER_PDEV and PMD_REGISTER_DRIVER_VDEV.  Both of these do the
> same thing the origional macro did, but both add the definition of a string
> variable that informs interested parties of the name of the pmd, and the 
> former
> also defines an second string that holds the symbol name of the pci table that
> is registered by this pmd.
>
> pmdinfo uses this information to extract hardware support from an object file
> and create a json string to make hardware support info discoverable later.
>
> Signed-off-by: Neil Horman 
> CC: Bruce Richardson 
> CC: Thomas Monjalon 
> CC: Stephen Hemminger 
> CC: Panu Matilainen 
> ---
>  drivers/Makefile   |  2 ++
>  drivers/crypto/aesni_gcm/aesni_gcm_pmd.c   |  4 +++-
>  drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c |  4 +++-
>  drivers/crypto/null/null_crypto_pmd.c  |  4 +++-
>  drivers/crypto/qat/rte_qat_cryptodev.c |  4 +++-
>  drivers/crypto/snow3g/rte_snow3g_pmd.c |  4 +++-
>  drivers/net/af_packet/rte_eth_af_packet.c  |  4 +++-
>  drivers/net/bnx2x/bnx2x_ethdev.c   |  6 --
>  drivers/net/bonding/rte_eth_bond_pmd.c |  7 ++-
>  drivers/net/cxgbe/cxgbe_ethdev.c   |  4 +++-
>  drivers/net/e1000/em_ethdev.c  |  3 ++-
>  drivers/net/e1000/igb_ethdev.c |  6 --
>  drivers/net/ena/ena_ethdev.c   |  3 ++-
>  drivers/net/enic/enic_ethdev.c |  3 ++-
>  drivers/net/fm10k/fm10k_ethdev.c   |  3 ++-
>  drivers/net/i40e/i40e_ethdev.c |  3 ++-
>  drivers/net/i40e/i40e_ethdev_vf.c  |  3 ++-
>  drivers/net/ixgbe/ixgbe_ethdev.c   |  6 --
>  drivers/net/mlx4/mlx4.c|  3 ++-
>  drivers/net/mlx5/mlx5.c|  3 ++-
>  drivers/net/mpipe/mpipe_tilegx.c   |  4 ++--
>  drivers/net/nfp/nfp_net.c  |  3 ++-
>  drivers/net/null/rte_eth_null.c|  3 ++-
>  drivers/net/pcap/rte_eth_pcap.c|  4 +++-
>  drivers/net/ring/rte_eth_ring.c|  3 ++-
>  drivers/net/szedata2/rte_eth_szedata2.c|  3 ++-
>  drivers/net/vhost/rte_eth_vhost.c  |  3 ++-
>  drivers/net/virtio/virtio_ethdev.c |  3 ++-
>  drivers/net/vmxnet3/vmxnet3_ethdev.c   |  3 ++-
>  drivers/net/xenvirt/rte_eth_xenvirt.c  |  2 +-
>  lib/librte_eal/common/include/rte_dev.h| 20 
>  31 files changed, 93 insertions(+), 37 deletions(-)
>

drivers/net/qede is missing and causes a build failure with a fresh config.

It seems to be missing in v1 but I managed to test it, guess it must've 
been an old .config generated before QEDE got merged.

- Panu -

[dpdk-dev] [PATCHv2 1/4] pmdinfogen: Add buildtools and pmdinfogen utility

[Panu Matilainen]

On 05/19/2016 12:08 AM, Neil Horman wrote:
[...]
> + if (strcmp(secname, ".modinfo") == 0) {
> + if (nobits)
> + fprintf(stderr, "%s has NOBITS .modinfo\n", 
> filename);
> + info->modinfo = (void *)hdr + sechdrs[i].sh_offset;
> + info->modinfo_len = sechdrs[i].sh_size;
> + } else if (strcmp(secname, "__ksymtab") == 0)
> + info->export_sec = i;
> + else if (strcmp(secname, "__ksymtab_unused") == 0)
> + info->export_unused_sec = i;
> + else if (strcmp(secname, "__ksymtab_gpl") == 0)
> + info->export_gpl_sec = i;
> + else if (strcmp(secname, "__ksymtab_unused_gpl") == 0)
> + info->export_unused_gpl_sec = i;
> + else if (strcmp(secname, "__ksymtab_gpl_future") == 0)
> + info->export_gpl_future_sec = i;
> +

Looks like a leftover from kernel modpost.c, not needed in DPDK.

- Panu -

[dpdk-dev] [PATCH 4/4] pmd_hw_support.py: Add tool to query binaries for hw support information

[Panu Matilainen]

On 05/18/2016 04:48 PM, Neil Horman wrote:
> On Wed, May 18, 2016 at 03:48:12PM +0300, Panu Matilainen wrote:
>> On 05/18/2016 03:03 PM, Neil Horman wrote:
>>> On Wed, May 18, 2016 at 02:48:30PM +0300, Panu Matilainen wrote:
>>>> On 05/16/2016 11:41 PM, Neil Horman wrote:
>>>>> This tool searches for the primer sting PMD_DRIVER_INFO= in any ELF 
>>>>> binary,
>>>>> and, if found parses the remainder of the string as a json encoded string,
>>>>> outputting the results in either a human readable or raw, script parseable
>>>>> format
>>>>>
>>>>> Signed-off-by: Neil Horman 
>>>>> CC: Bruce Richardson 
>>>>> CC: Thomas Monjalon 
>>>>> CC: Stephen Hemminger 
>>>>> CC: Panu Matilainen 
>>>>> ---
>>>>>  tools/pmd_hw_support.py | 174 
>>>>> 
>>>>>  1 file changed, 174 insertions(+)
>>>>>  create mode 100755 tools/pmd_hw_support.py
>>>>>
>>>>> diff --git a/tools/pmd_hw_support.py b/tools/pmd_hw_support.py
>>>>> new file mode 100755
>>>>> index 000..0669aca
>>>>> --- /dev/null
>>>>> +++ b/tools/pmd_hw_support.py
>>>>> @@ -0,0 +1,174 @@
>>>>> +#!/usr/bin/python3
>>>>
>>>> I think this should use /usr/bin/python to be consistent with the other
>>>> python scripts, and like the others work with python 2 and 3. I only tested
>>>> it with python2 after changing this and it seemed to work fine so the
>>>> compatibility side should be fine as-is.
>>>>
>>> Sure, I can change the python executable, that makes sense.
>>>
>>>> On the whole, AFAICT the patch series does what it promises, and works for
>>>> both static and shared linkage. Using JSON formatted strings in an ELF
>>>> section is a sound working technical solution for the storage of the data.
>>>> But the difference between the two cases makes me wonder about this all...
>>> You mean the difference between checking static binaries and dynamic 
>>> binaries?
>>> yes, there is some functional difference there
>>>
>>>>
>>>> For static library build, you'd query the application executable, eg
>>> Correct.
>>>
>>>> testpmd, to get the data out. For a shared library build, that method gives
>>>> absolutely nothing because the data is scattered around in individual
>>>> libraries which might be just about wherever, and you need to somehow
>>> Correct, I figured that users would be smart enough to realize that with
>>> dynamically linked executables, they would need to look at DSO's, but I 
>>> agree,
>>> its a glaring diffrence.
>>
>> Being able to look at DSOs is good, but expecting the user to figure out
>> which DSOs might be loaded and not and where to look is going to be well
>> above many users. At very least it's not what I would call user-friendly.
>>
> I disagree, there is no linkage between an application and the dso's it opens
> via dlopen that is exportable.  The only way to handle that is to have a
> standard search path for the pmd_hw_info python script.  Thats just like 
> modinfo
> works (i.e. "modinfo bnx2" finds the bnx2 module for the running kernel).  We
> can of course do something simmilar, but we have no existing implicit path
> information to draw from to do that (because you can have multiple dpdk 
> installs
> side by side).  The only way around that is to explicitly call out the path on
> the command line.

There's no telling what libraries user might load at runtime with -D, 
that is true for both static and shared libraries.

When CONFIG_RTE_EAL_PMD_PATH is set, as it is likely to be on distro 
builds, you *know* that everything in that path will be loaded on 
runtime regardless of what commandline options there might be so the 
situation is actually on par with static builds. Of course you still 
dont know about ones added with -D but that's a limitation of any 
solution that works without actually running the app.

>
>>>> discover the location + correct library files to be able to query that. For
>>>> the shared case, perhaps the script could be taught to walk files in
>>>> CONFIG_RTE_EAL_PMD_PATH to give in-the-ballpark correct/identical results
>>> My initial thought would be to run ldd on the executable, and use a 
>>> heuristic to
>>> determine relevant pmd DSO's, and then feed each o

[dpdk-dev] [PATCH 4/4] pmd_hw_support.py: Add tool to query binaries for hw support information

[Panu Matilainen]

On 05/18/2016 03:03 PM, Neil Horman wrote:
> On Wed, May 18, 2016 at 02:48:30PM +0300, Panu Matilainen wrote:
>> On 05/16/2016 11:41 PM, Neil Horman wrote:
>>> This tool searches for the primer sting PMD_DRIVER_INFO= in any ELF binary,
>>> and, if found parses the remainder of the string as a json encoded string,
>>> outputting the results in either a human readable or raw, script parseable
>>> format
>>>
>>> Signed-off-by: Neil Horman 
>>> CC: Bruce Richardson 
>>> CC: Thomas Monjalon 
>>> CC: Stephen Hemminger 
>>> CC: Panu Matilainen 
>>> ---
>>>  tools/pmd_hw_support.py | 174 
>>> 
>>>  1 file changed, 174 insertions(+)
>>>  create mode 100755 tools/pmd_hw_support.py
>>>
>>> diff --git a/tools/pmd_hw_support.py b/tools/pmd_hw_support.py
>>> new file mode 100755
>>> index 000..0669aca
>>> --- /dev/null
>>> +++ b/tools/pmd_hw_support.py
>>> @@ -0,0 +1,174 @@
>>> +#!/usr/bin/python3
>>
>> I think this should use /usr/bin/python to be consistent with the other
>> python scripts, and like the others work with python 2 and 3. I only tested
>> it with python2 after changing this and it seemed to work fine so the
>> compatibility side should be fine as-is.
>>
> Sure, I can change the python executable, that makes sense.
>
>> On the whole, AFAICT the patch series does what it promises, and works for
>> both static and shared linkage. Using JSON formatted strings in an ELF
>> section is a sound working technical solution for the storage of the data.
>> But the difference between the two cases makes me wonder about this all...
> You mean the difference between checking static binaries and dynamic binaries?
> yes, there is some functional difference there
>
>>
>> For static library build, you'd query the application executable, eg
> Correct.
>
>> testpmd, to get the data out. For a shared library build, that method gives
>> absolutely nothing because the data is scattered around in individual
>> libraries which might be just about wherever, and you need to somehow
> Correct, I figured that users would be smart enough to realize that with
> dynamically linked executables, they would need to look at DSO's, but I agree,
> its a glaring diffrence.

Being able to look at DSOs is good, but expecting the user to figure out 
which DSOs might be loaded and not and where to look is going to be well 
above many users. At very least it's not what I would call user-friendly.

>> discover the location + correct library files to be able to query that. For
>> the shared case, perhaps the script could be taught to walk files in
>> CONFIG_RTE_EAL_PMD_PATH to give in-the-ballpark correct/identical results
> My initial thought would be to run ldd on the executable, and use a heuristic 
> to
> determine relevant pmd DSO's, and then feed each of those through the python
> script.  I didn't want to go to that trouble unless there was consensus on it
> though.

Problem is, ldd doesn't know about them either because the pmds are not 
linked to the executables at all anymore. They could be force-linked of 
course, but that means giving up the flexibility of plugins, which IMO 
is a no-go. Except maybe as an option, but then that would be a third 
case to support.


>
>> when querying the executable as with static builds. If identical operation
>> between static and shared versions is a requirement (without running the app
>> in question) then query through the executable itself is practically the
>> only option. Unless some kind of (auto-generated) external config file
>> system ala kernel depmod / modules.dep etc is brought into the picture.
> Yeah, I'm really trying to avoid that, as I think its really not a typical 
> part
> of how user space libraries are interacted with.
>
>>
>> For shared library configurations, having the data in the individual pmds is
>> valuable as one could for example have rpm autogenerate provides from the
>> data to ease/automate installation (in case of split packaging and/or 3rd
>> party drivers). And no doubt other interesting possibilities. With static
>> builds that kind of thing is not possible.
> Right.
>
> Note, this also leaves out PMD's that are loaded dynamically (i.e. via 
> dlopen).
> For those situations I don't think we have any way of 'knowing' that the
> application intends to use them.

Hence my comment about CONFIG_RTE_EAL_PMD_PATH above, it at least 
provides a reasonable heuristic of what would be loaded by the app when 
run. But ultimately the only way to know what hardware

[dpdk-dev] [PATCH 4/4] pmd_hw_support.py: Add tool to query binaries for hw support information

[Panu Matilainen]

On 05/16/2016 11:41 PM, Neil Horman wrote:
> This tool searches for the primer sting PMD_DRIVER_INFO= in any ELF binary,
> and, if found parses the remainder of the string as a json encoded string,
> outputting the results in either a human readable or raw, script parseable
> format
>
> Signed-off-by: Neil Horman 
> CC: Bruce Richardson 
> CC: Thomas Monjalon 
> CC: Stephen Hemminger 
> CC: Panu Matilainen 
> ---
>  tools/pmd_hw_support.py | 174 
> 
>  1 file changed, 174 insertions(+)
>  create mode 100755 tools/pmd_hw_support.py
>
> diff --git a/tools/pmd_hw_support.py b/tools/pmd_hw_support.py
> new file mode 100755
> index 000..0669aca
> --- /dev/null
> +++ b/tools/pmd_hw_support.py
> @@ -0,0 +1,174 @@
> +#!/usr/bin/python3

I think this should use /usr/bin/python to be consistent with the other 
python scripts, and like the others work with python 2 and 3. I only 
tested it with python2 after changing this and it seemed to work fine so 
the compatibility side should be fine as-is.

On the whole, AFAICT the patch series does what it promises, and works 
for both static and shared linkage. Using JSON formatted strings in an 
ELF section is a sound working technical solution for the storage of the 
data. But the difference between the two cases makes me wonder about 
this all...

For static library build, you'd query the application executable, eg 
testpmd, to get the data out. For a shared library build, that method 
gives absolutely nothing because the data is scattered around in 
individual libraries which might be just about wherever, and you need to 
somehow discover the location + correct library files to be able to 
query that. For the shared case, perhaps the script could be taught to 
walk files in CONFIG_RTE_EAL_PMD_PATH to give in-the-ballpark 
correct/identical results when querying the executable as with static 
builds. If identical operation between static and shared versions is a 
requirement (without running the app in question) then query through the 
executable itself is practically the only option. Unless some kind of 
(auto-generated) external config file system ala kernel depmod / 
modules.dep etc is brought into the picture.

For shared library configurations, having the data in the individual 
pmds is valuable as one could for example have rpm autogenerate provides 
from the data to ease/automate installation (in case of split packaging 
and/or 3rd party drivers). And no doubt other interesting possibilities. 
With static builds that kind of thing is not possible.

Calling up on the list of requirements from 
http://dpdk.org/ml/archives/dev/2016-May/038324.html, I see a pile of 
technical requirements but perhaps we should stop for a moment to think 
about the use-cases first?

To name some from the top of my head:
- user wants to know whether the hardware on the system is supported
- user wants to know which package(s) need to be installed to support 
the system hardware
- user wants to list all supported hardware before going shopping
- [what else?]

...and then think how these things would look like from the user 
perspective, in the light of the two quite dramatically differing cases 
of static vs shared linkage.

P.S. Sorry for being late to this party, I'm having some health issues 
so my level of participation is a bit on-and-off at the moment.

- Panu -

[dpdk-dev] [PATCH] qede: fix build with gcc >= 6.0

[Panu Matilainen]

On 05/14/2016 04:06 AM, Rasesh Mody wrote:
>> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
>> Sent: Friday, May 13, 2016 8:38 AM
>>
>> 2016-05-10 13:01, Panu Matilainen:
>>> With gcc >= 6.0, qede base driver fails to build with:
>>> drivers/net/qede/base/ecore_cxt.c: In function 'ecore_cdu_init_common':
>>> cc1: error: left shift of negative value
>>> [-Werror=shift-negative-value]
>>>
>>> Since the base drivers are untouchable, work around by disabling the
>>> warning.
>>
>> Are the qede base driver files untouchable?
>>
>>> Signed-off-by: Panu Matilainen 
>> [...]
>>> +ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
>>> +CFLAGS_BASE_DRIVER += -Wno-shift-negative-value endif
>>
>> Fixes: ec94dbc57362 ("qede: add base driver")
>>
>> Applied, thanks
>>
>> Rasesh, would you mind to fix the base driver and remove this workaround?
>
> Sure Thomas! Will look into this.

Hmm, so the base driver in qede is not holy and untouchable? That's good 
news of course. I simply assumed it was by the tell-tale signs of having 
a "base driver" in the first place, and like with those other 
untouchable base-drivers there were many warning-disablers present already.

- Panu -

[dpdk-dev] [PATCH 2/2] doc: announce ABI change of struct rte_port_sink_params

[Panu Matilainen]

On 05/16/2016 04:18 PM, Fan Zhang wrote:
> The ABI changes are planned for rte_port_sink_params, which will be
> supported from release 16.11. Here announces that ABI changes in detail.
>
> Signed-off-by: Fan Zhang 
> Acked-by: Cristian Dumitrescu 
> ---
>  doc/guides/rel_notes/deprecation.rst | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst 
> b/doc/guides/rel_notes/deprecation.rst
> index d228bae..d2f7306 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -78,3 +78,7 @@ Deprecation Notices
>  * ABI will change for rte_port_source_params struct. The member file_name
>data type will be changed from char * to const char *. This change targets
>release 16.11.
> +
> +* ABI will change for rte_port_sink_params struct. The member file_name
> +  data type will be changed from char * to const char *. This change targets
> +  release 16.11.
>

Surely such a minor change doesn't require two separate announcements or 
patches for that matter. In fact I doubt it's an ABI change at all 
(although it is an API change certainly).

However to me the bigger issue is that a change like this alone doesn't 
seem like worth breaking the ABI. The ABI was just broken in 16.04 to 
introduce these struct members (among other things) and to break the ABI 
again just to fixup a const-correctness issue seems a bit much. Could it 
maybe wait until there's some actually compelling reason to break the ABI?

Note that I'm not against the change as such, const-correctness is a 
good thing.

- Panu -

[dpdk-dev] [PATCH 02/20] thunderx/nicvf: add pmd skeleton

[Panu Matilainen]

On 05/07/2016 06:16 PM, Jerin Jacob wrote:


> diff --git a/drivers/net/thunderx/Makefile b/drivers/net/thunderx/Makefile
> new file mode 100644
> index 000..69bb750
> --- /dev/null
> +++ b/drivers/net/thunderx/Makefile
> @@ -0,0 +1,64 @@
> +#   BSD LICENSE
> +#
> +#   Copyright(c) 2016 Cavium Networks. 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 Cavium Networks nor the names of its
> +#   contributors may be used to endorse or promote products derived
> +#   from this software without specific prior written permission.
> +#
> +#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +#
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +#
> +# library name
> +#
> +LIB = librte_pmd_thunderx_nicvf.a
> +
> +CFLAGS += $(WERROR_FLAGS)
> +
> +EXPORT_MAP := rte_pmd_thunderx_nicvf_version.map
> +
> +LIBABIVER := 1
> +
> +OBJS_BASE_DRIVER=$(patsubst %.c,%.o,$(notdir $(wildcard $(SRCDIR)/base/*.c)))
> +$(foreach obj, $(OBJS_BASE_DRIVER), $(eval 
> CFLAGS_$(obj)+=$(CFLAGS_BASE_DRIVER)))
> +
> +VPATH += $(SRCDIR)/base
> +
> +#
> +# all source are stored in SRCS-y
> +#
> +SRCS-$(CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD) += nicvf_hw.c
> +SRCS-$(CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD) += nicvf_mbox.c
> +SRCS-$(CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD) += nicvf_ethdev.c
> +
> +
> +# this lib depends upon:
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD) += lib/librte_eal 
> lib/librte_ether
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD) += lib/librte_mempool 
> lib/librte_mbuf
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_THUNDERX_NICVF_PMD) += lib/librte_net 
> lib/librte_malloc

librte_malloc no longer exists for almost a year by now (see commits 
2f9d47013e4d and aace9d0bcf58) but seems to be a popular thing to copy 
to new drivers :)

- Panu -

[dpdk-dev] [PATCH 01/40] bnxt: new driver for Broadcom NetXtreme-C devices

[Panu Matilainen]

On 05/06/2016 10:25 PM, Stephen Hurd wrote:
> Initial skeleton simply fails init.
> Add nic guide and tie into build system.
>
> Signed-off-by: Stephen Hurd 
> ---
[...]
> diff --git a/drivers/net/bnxt/Makefile b/drivers/net/bnxt/Makefile
> new file mode 100644
> index 000..89b4a27
> --- /dev/null
> +++ b/drivers/net/bnxt/Makefile
> @@ -0,0 +1,63 @@
> +#   BSD LICENSE
> +#
> +#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> +#   Copyright(c) 2014 6WIND S.A.
> +#   Copyright(c) Broadcom Limited.
> +#   All rights reserved.
> +#
> +#   Redistribution and use in source and binary forms, with or without
> +#   modification, are permitted provided that the following conditions
> +#   are met:
> +#
> +# * Redistributions of source code must retain the above copyright
> +#   notice, this list of conditions and the following disclaimer.
> +# * Redistributions in binary form must reproduce the above copyright
> +#   notice, this list of conditions and the following disclaimer in
> +#   the documentation and/or other materials provided with the
> +#   distribution.
> +# * Neither the name of Intel Corporation nor the names of its
> +#   contributors may be used to endorse or promote products derived
> +#   from this software without specific prior written permission.
> +#
> +#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +#
> +# library name
> +#
> +LIB = librte_pmd_bnxt.a
> +
> +LIBABIVER := 1
> +
> +CFLAGS += -O3
> +CFLAGS += $(WERROR_FLAGS)
> +
> +EXPORT_MAP := rte_pmd_bnxt_version.map
> +
> +#
> +# all source are stored in SRCS-y
> +#
> +SRCS-$(CONFIG_RTE_LIBRTE_BNXT_PMD) += bnxt_ethdev.c
> +
> +#
> +# Export include files
> +#
> +SYMLINK-y-include +=
> +
> +# this lib depends upon:
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_BNXT_PMD) += lib/librte_mbuf
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_BNXT_PMD) += lib/librte_ether
> +DEPDIRS-$(CONFIG_RTE_LIBRTE_BNXT_PMD) += lib/librte_malloc

librte_malloc got merged into librte_eal almost a year ago so it no 
longer exists at all [*], but still this keeps getting copy-pasted to 
new drivers, qede had this too :)

[*] See commits 2f9d47013e4d and aace9d0bcf58

- Panu -

[dpdk-dev] [PATCH] qede: fix build with gcc >= 6.0

[Panu Matilainen]

With gcc >= 6.0, qede base driver fails to build with:
drivers/net/qede/base/ecore_cxt.c: In function 'ecore_cdu_init_common':
cc1: error: left shift of negative value [-Werror=shift-negative-value]

Since the base drivers are untouchable, work around by disabling
the warning.

Signed-off-by: Panu Matilainen 
---
 drivers/net/qede/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/qede/Makefile b/drivers/net/qede/Makefile
index 47e01be..884e173 100644
--- a/drivers/net/qede/Makefile
+++ b/drivers/net/qede/Makefile
@@ -47,6 +47,9 @@ CFLAGS_BASE_DRIVER += -Wno-unused-but-set-variable
 CFLAGS_BASE_DRIVER += -Wno-missing-declarations
 CFLAGS_BASE_DRIVER += -Wno-maybe-uninitialized
 CFLAGS_BASE_DRIVER += -Wno-strict-prototypes
+ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
+CFLAGS_BASE_DRIVER += -Wno-shift-negative-value
+endif
 else ifeq ($(CC), clang)
 CFLAGS_BASE_DRIVER += -Wno-format-extra-args
 CFLAGS_BASE_DRIVER += -Wno-visibility
-- 
2.5.5

[dpdk-dev] [PATCH] mk: cleanup leftover references to librte_malloc

[Panu Matilainen]

librte_malloc was long since merged into librte_eal, mop up the
leftovers from rarer drivers.

Fixes: 2f9d47013e4d ("mem: move librte_malloc to eal/common")

Signed-off-by: Panu Matilainen 
---
 drivers/net/cxgbe/Makefile| 2 +-
 drivers/net/ena/Makefile  | 2 +-
 drivers/net/mpipe/Makefile| 2 +-
 drivers/net/nfp/Makefile  | 2 +-
 drivers/net/szedata2/Makefile | 1 -
 5 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/cxgbe/Makefile b/drivers/net/cxgbe/Makefile
index 0711976..e2ff412 100644
--- a/drivers/net/cxgbe/Makefile
+++ b/drivers/net/cxgbe/Makefile
@@ -82,6 +82,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_CXGBE_PMD) += t4_hw.c
 # this lib depends upon:
 DEPDIRS-$(CONFIG_RTE_LIBRTE_CXGBE_PMD) += lib/librte_eal lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_CXGBE_PMD) += lib/librte_mempool lib/librte_mbuf
-DEPDIRS-$(CONFIG_RTE_LIBRTE_CXGBE_PMD) += lib/librte_net lib/librte_malloc
+DEPDIRS-$(CONFIG_RTE_LIBRTE_CXGBE_PMD) += lib/librte_net

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/ena/Makefile b/drivers/net/ena/Makefile
index ac2b55d..a0d3358 100644
--- a/drivers/net/ena/Makefile
+++ b/drivers/net/ena/Makefile
@@ -54,7 +54,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += ena_eth_com.c
 # this lib depends upon:
 DEPDIRS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += lib/librte_eal lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += lib/librte_mempool lib/librte_mbuf
-DEPDIRS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += lib/librte_net lib/librte_malloc
+DEPDIRS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += lib/librte_net

 CFLAGS += $(INCLUDES)

diff --git a/drivers/net/mpipe/Makefile b/drivers/net/mpipe/Makefile
index 46f046d..846e2e0 100644
--- a/drivers/net/mpipe/Makefile
+++ b/drivers/net/mpipe/Makefile
@@ -42,6 +42,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD) += mpipe_tilegx.c

 DEPDIRS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD) += lib/librte_eal lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD) += lib/librte_mempool lib/librte_mbuf
-DEPDIRS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD) += lib/librte_net lib/librte_malloc
+DEPDIRS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD) += lib/librte_net

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/nfp/Makefile b/drivers/net/nfp/Makefile
index 11f..4cadd13 100644
--- a/drivers/net/nfp/Makefile
+++ b/drivers/net/nfp/Makefile
@@ -53,6 +53,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_NFP_PMD) += nfp_net.c
 # this lib depends upon:
 DEPDIRS-$(CONFIG_RTE_LIBRTE_NFP_PMD) += lib/librte_eal lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_NFP_PMD) += lib/librte_mempool lib/librte_mbuf
-DEPDIRS-$(CONFIG_RTE_LIBRTE_NFP_PMD) += lib/librte_net lib/librte_malloc
+DEPDIRS-$(CONFIG_RTE_LIBRTE_NFP_PMD) += lib/librte_net

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/szedata2/Makefile b/drivers/net/szedata2/Makefile
index 963a8d6..ee4986c 100644
--- a/drivers/net/szedata2/Makefile
+++ b/drivers/net/szedata2/Makefile
@@ -57,7 +57,6 @@ SYMLINK-y-include +=
 # this lib depends upon:
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_SZEDATA2) += lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_SZEDATA2) += lib/librte_ether
-DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_SZEDATA2) += lib/librte_malloc
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_SZEDATA2) += lib/librte_kvargs

 include $(RTE_SDK)/mk/rte.lib.mk
-- 
2.5.5

[dpdk-dev] [PATCH v2] vhost: Fix linkage of vhost PMD

[Panu Matilainen]

On 04/26/2016 08:39 AM, Tetsuya Mukawa wrote:
> Currently, vhost PMD doesn't have linkage for librte_vhost, even though
> it depends on librte_vhost APIs. This causes a linkage error if below
> conditions are fulfilled.
>
>  - DPDK libraries are compiled as shared libraries.
>  - DPDK application doesn't link librte_vhost.
>  - Above application tries to link vhost PMD using '-d' DPDK option.
>
> The patch adds linkage for librte_vhost to vhost PMD not to cause an
> above error.
>
> Signed-off-by: Tetsuya Mukawa 
> Acked-by: Panu Matilainen 
> ---
>  drivers/net/vhost/Makefile | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/vhost/Makefile b/drivers/net/vhost/Makefile
> index f49a69b..30b91a0 100644
> --- a/drivers/net/vhost/Makefile
> +++ b/drivers/net/vhost/Makefile
> @@ -38,6 +38,7 @@ LIB = librte_pmd_vhost.a
>
>  CFLAGS += -O3
>  CFLAGS += $(WERROR_FLAGS)
> +LDLIBS += -lrte_vhost
>
>  EXPORT_MAP := rte_pmd_vhost_version.map
>
>

Hmm, turns out this isn't quite enough, simply because its the first of 
its kind (first internal dependency between libraries), at least I'm 
getting:

== Build drivers/net/vhost
gcc -m64 
-Wl,--version-script=/srv/work/dist/dpdk/dpdk-16.04/drivers/net/vhost/rte_pmd_vhost_version.map
 
  -shared rte_eth_vhost.o -lrte_vhost -Wl,-soname,librte_pmd_vhost.so.1 
-o librte_pmd_vhost.so.1
/usr/bin/ld: cannot find -lrte_vhost
collect2: error: ld returned 1 exit status
/srv/work/dist/dpdk/dpdk-16.04/mk/rte.lib.mk:127: recipe for target 
'librte_pmd_vhost.so.1' failed

So it'll need something like this as a pre-requisite to add the internal 
libraries to the linker path:

diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index 8f7e021..f5d7b04 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -86,7 +86,7 @@ O_TO_A_DO = @set -e; \
 $(O_TO_A) && \
 echo $(O_TO_A_CMD) > $(call exe2cmd,$(@))

-O_TO_S = $(LD) $(_CPU_LDFLAGS) $(EXTRA_LDFLAGS) -shared $(OBJS-y) 
$(LDLIBS) \
+O_TO_S = $(LD) -L$(RTE_OUTPUT)/lib $(_CPU_LDFLAGS) $(EXTRA_LDFLAGS) 
-shared $(OBJS-y) $(LDLIBS) \
  -Wl,-soname,$(LIB) -o $(LIB)
  O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight
  O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)","  LD $(@)")


I can submit an official patch for this later but I'm not exactly 
feeling like the sharpest knife in the drawer today so if somebody beats 
me to it, feel free.

- Panu -

[dpdk-dev] [RFC] Link ibrte_vhost to librte_pmd_vhost

[Panu Matilainen]

On 04/25/2016 12:05 PM, Tetsuya Mukawa wrote:
> Hi Yuanhan,
>
> I want to apply a patch to vhost PMD.
> Before submitting, could you please let me know your guess about the patch?
>
> Here is my problem.
> I am using below shared library configuration to build my application.
> CONFIG_RTE_BUILD_SHARED_LIB=y
> Normally, My application doesn't need vhost facilities, so librte_vhost
> isn't linked while compiling.
>
> Sometimes, I need to use vhost PMD, so I just want to add '-d
> librte_pmd_vhost.so' to DPDK command line to load vhost PMD library.
> But my application doesn't have librte_vhost, then I've got an error
> about it.
> Even if specify like "-d librte_vhost -d librte_pmd_vhost", I still have
> an error.
> Probably this is because above libraries will be dlopen(ed) with
> RTLD_LOCAL option.
>
> Here, I have 2 choices.
> One is linking librte_vhost to my application while compiling, even if I
> don't need it normally.
> This is the way all DPDK examples did. But I am wondering if I should
> follow this.
>
> Another way is applying a below patch.
> --- a/drivers/net/vhost/Makefile
> +++ b/drivers/net/vhost/Makefile
> @@ -38,6 +38,7 @@ LIB = librte_pmd_vhost.a
>
>   CFLAGS += -O3
>   CFLAGS += $(WERROR_FLAGS)
> +LDLIBS += -lrte_vhost
>
>   EXPORT_MAP := rte_pmd_vhost_version.map
>
> This is same way to link libpcap to librte_pmd_pcap.
> What do you think about adding it to vhost PMD?

Yes, this is absolutely the right thing to do.

Ultimately this should be done for all dependencies in all libraries, 
but missing dependencies are even more pronounced in plugins so the 
sooner this goes in, the better.

Acked-by: Panu Matilainen 

- Panu -


>
> Thanks,
> Tetsuya
>

[dpdk-dev] [PATCH 1/1] lib/librte_eal: fix resource leak

[Panu Matilainen]

On 04/21/2016 02:19 PM, Sergio Gonzalez Monroy wrote:
> On 20/04/2016 10:15, David Marchand wrote:
>> On Tue, Apr 19, 2016 at 6:27 PM, Marcin Kerlin
>>  wrote:
>>> Fix issue reported by Coverity.
>>>
>>> Coverity ID 13295, 13296, 13303:
>>> Resource leak: The system resource will not be reclaimed
>>> and reused, reducing the future availability of the resource.
>>> In rte_eal_hugepage_attach: Leak of memory or pointers to system
>>> resources.
>>>
>>> Fixes: af75078fece3 ("first public release")
>>>
>>> Signed-off-by: Marcin Kerlin 
>>> ---
>>>   lib/librte_eal/linuxapp/eal/eal_memory.c | 12 +++-
>>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> b/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> index 5b9132c..6320aa0 100644
>>> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> @@ -1475,13 +1475,17 @@ rte_eal_hugepage_attach(void)
>>>  "and retry running both
>>> primary "
>>>  "and secondary processes\n");
>>>  }
>>> +
>>> +   if (base_addr != MAP_FAILED)
>>> +   munmap((void *)(uintptr_t)base_addr,
>>> mcfg->memseg[s].len);
>>> +
>> What is the point of this casting ?
>> Idem for the rest of the patch.
>
> I don't see the point either.
> Marcin?
>
>>
>> I can't see cleanup for previously mapped segments when mapping one
>> fails.
>> Do we want this cleanup as well ?
>
> Good point.
>
> We are not really leaking resources because we panic if we fail to
> initialize eal memory.

FWIW, the panic-attack mode is something I'd like to see eliminated 
eventually and hopefully will be submitting patches for sooner or later. 
Aborting from library code is rather antisocial behavior, even if its on 
just initialization code that usually runs fairly early in process lifetime.

>
> Now, if we are going to do the cleanup, I think that as David points out
> we should be
> cleaning up all previous mappings too.

+1

Even if the current code just panics, it doesn't mean it always will.

- Panu -

>
> Sergio
>> CC Sergio.
>>
>>
>

[dpdk-dev] [dpdk-users] DPDK 16.04 link changes cause PMD drivers to not be loaded

[Panu Matilainen]

On 04/21/2016 06:24 PM, Aurojit Panda wrote:
>
>
> Thomas Monjalon wrote:
>> 2016-04-21 08:01, Aurojit Panda:
>>> Panu Matilainen wrote:
>> [...]
>>>> Again, PMDs are *plugins* that are *meant* to be loaded at runtime.
>>>> That allows for all sorts of flexibility especially
>>>> for packaging and shipping, at some extra cost in setup complexity.
>>> I am all for a plugin architecture, I was merely suggesting that you
>>> embed some path infromation at the beginning. Also please note:
>>> (a) This behavior changed recently.
>>
>> What changed recently?
>>
> Change 948fd64befc3726 moved DPDK from building a shared library to
> using LD linker scripts.
>
>>> (b) This change is entirely undocumented, which is why I was reporting
>>> it in the first place.
>>> (c) It is actually quite unintutive, because previously ensuring
>>> LD_LIBRARY_PATH was correct was all that was required
>>> to get any DPDK application to interact with ports.
>>
>> ?
>> Are you talking about combined library?
> Yes, if you build a shared combined library, PMD drivers are not
> automatically loaded anymore. This implies that programs do not
> enumerate the set of ports on the machine. (Unless CONFIG_EAL_PMD_PATH
> is correctly set to the absolute directory where the built DPDK will live).
>>
>>>> For your own purposes, you can of course tweak the linking settings
>>>> as much as you like. Look for "plugins" in mk/rte.app.mk and change
>>>> the shared lib condition on the line above to "y" and there you have
>>>> it.
>>>> But that's not the way plugins are meant to be used.
>>> That is not a reasonable solution given that it makes it very hard to
>>> track future changes to DPDK without merges.
>>> My alternatives neither break people's abilities to use plugins,
>>> nor do they impact behavior.
>>
>> Please do not hesitate to send some patch to show your solution.
>
> My initial e-mail was sent because I don't entirely understand LD linker
> scripts (the PMD libraries are listed within the built libdpdk.so file),
> so the only patch I can suggest is undoing 948fd64befc3726 in which case
> the behavior as I describe.

Reverting would be counter-productive, the change was introduced for a 
reason. Many of them in fact.

Any application wanting to force linkage to plugins always needed to 
link them inside --whole-archive section with static or individual 
shared libraries, now that is true for the combined shared library as well.

Within DPDK itself it would be easy enough to add an option to force 
link-in of the plugins (see the comment about mk/rte.app.mk change), but 
not sure we want to have yet another build option. And anything else 
still would need to use --whole-archive if they want that behavior.

So yes it's a behavior change for that particular corner which nobody 
thought of or brought up during review, and so ended up being 
undocumented too.

Like said there are always ways to improve things, once the use-case is 
understood, actual examples of how you are deploying DPDK would perhaps 
help. One possibility could be attempting to load known plugins from 
LD_LIBRARY_PATH, but that requires inserting that information somewhere 
in the binaries where it currently is not needed.

- Panu -

[dpdk-dev] On DPDK ABI policy

[Panu Matilainen]

[ change of subject since this is about ABI policy, not namespacing ]

On 04/07/2016 01:16 PM, Marc Sune wrote:
>
>
> 2016-04-07 11:33 GMT+02:00 Panu Matilainen  <mailto:pmatilai at redhat.com>>:
>
> On 04/07/2016 12:18 PM, Thomas Monjalon wrote:
>
> Thank you everyone for the feedbacks.
>
> 2016-04-05 15:56, Thomas Monjalon:
>
> The goal of this email is to get some feedback on how
> important it is
> to fix the DPDK namespace.
>
>
> Everybody agree every symbols must be prefixed. Checking and
> fixing the
> namespace consistency will be in the roadmap.
>
> It seems most of you agree renaming would be a nice improvement
> but not
> so important.
> The main drawback is the induced backporting pain, even if we have
> some scripts to convert the patches to the old namespace.
> Note: the backports can be in DPDK itself or in the applications.
>
> If there is enough agreement that we should do something, I
> suggest to
> introduce the "dpdk_" prefix slowly and live with both
> "rte_" and "dpdk_"
> during some time.
> We could start using the new prefix for the new APIs
> (example: crypto)
> or when there is a significant API break (example: mempool).
>
>
> The slow change has been clearly rejected in favor of a complete
> change
> in one patch.
> The timing was also discussed as it could impact the pending
> patches.
> So it would be done at the end or the beginning of a release.
> Marc suggests to do it for 16.04 as the numbering scheme has
> changed.
>
>
> Just noting that it cannot be done in 16.04 because the ABI policy
> requires a deprecation cycle of at least one major release for every
> breakage. And we're discussing a total 100% breakage of everything
> here, even if its just a simple rename.
>
>
> I keep not understanding the ABI policy, and particularly why ABI
> changes have to be announced once cycle before _if_ there is already at
> least one ABI change proposed. DPDK applications will have to recompile
> anyway.

The point is to allow API/ABI consumers to assess in advance what sort 
of pains can they expect when moving their applications from one version 
to another. Otherwise all sorts of massive changes could ride the wave 
of whatever "change 16bit struct member to 32bit" trivialities that are 
nevertheless ABI breaks.

There have already been quite a few exceptions to the rule when the ABI 
is already being broken, so its not entirely rigid. Another point that 
migth warrant some tweaking to the policy is the "core" libraries 
depending on each other so an ABI break in any one of them forces 
recompile of everything anyway.

> This aspect of the policy only slows down DPDK development and it

One could also think that slowing down development and forcing people to 
think ahead are not entirely unintentional or unwanted side-effects :)

Look at the latest librte_vhost initiative to remove unnecessarily 
exposed structs to avoid having to deal with ABI breakages all the time: 
the policy is effectively encouraging people into better library design.

> pollutes the repository with commits announcing ABI changes that are
> irrelevant after 2 cycles, as (code) diffs show that already (not
> mentioning NEXT_ABI complexity and extra LOCs).

Fully agreed on NEXT_ABI, I never liked it at all.

> Maintaining LTS releases, and enforcing bug fixing in old LTS first,
> upstreaming bugfixes is to me a much better approach to solve backwards
> compatibility issues.

LTS releases could help the situation somewhat, but then again people 
tend to still want those new fancy things backported (you know, have the 
cake and eat it too) but that can't be done because of ABI breakage, so 
they're forced to run the latest version anyway.

> But this is probably another discussion.

Yup, changed subject to avoid mixing it up with the namespace discussion 
too much.

- Panu -

[dpdk-dev] DPDK namespace

[Panu Matilainen]

On 04/07/2016 12:18 PM, Thomas Monjalon wrote:
> Thank you everyone for the feedbacks.
>
> 2016-04-05 15:56, Thomas Monjalon:
>> The goal of this email is to get some feedback on how important it is
>> to fix the DPDK namespace.
>
> Everybody agree every symbols must be prefixed. Checking and fixing the
> namespace consistency will be in the roadmap.
>
> It seems most of you agree renaming would be a nice improvement but not
> so important.
> The main drawback is the induced backporting pain, even if we have
> some scripts to convert the patches to the old namespace.
> Note: the backports can be in DPDK itself or in the applications.
>
>> If there is enough agreement that we should do something, I suggest to
>> introduce the "dpdk_" prefix slowly and live with both "rte_" and "dpdk_"
>> during some time.
>> We could start using the new prefix for the new APIs (example: crypto)
>> or when there is a significant API break (example: mempool).
>
> The slow change has been clearly rejected in favor of a complete change
> in one patch.
> The timing was also discussed as it could impact the pending patches.
> So it would be done at the end or the beginning of a release.
> Marc suggests to do it for 16.04 as the numbering scheme has changed.

Just noting that it cannot be done in 16.04 because the ABI policy 
requires a deprecation cycle of at least one major release for every 
breakage. And we're discussing a total 100% breakage of everything here, 
even if its just a simple rename.

- Panu -

> There is no strong conclusion at this point because we need to decide
> wether the renaming deserves to be done or never.
> I suggest to take the inputs from the technical board.
>
> Do not hesitate to comment. Thanks
>

[dpdk-dev] [PATCH] vhost: ABI/API change announcement due to refactor

[Panu Matilainen]

On 04/06/2016 09:53 AM, Yuanhan Liu wrote:
> We currently exposed way too many fields (or even structures) than
> necessary. For example, vhost_virtqueue struct should NOT be exposed
> to user at all: application just need to tell the right queue id to
> locate a specific queue, and that's all. Instead, the structure should
> be defined in an internal header file. With that, we could do any changes
> to it we want, without worrying about that we may offense the painful
> ABI rules.
>
> Similar changes could be done to virtio_net struct as well, just exposing
> very few fields that are necessary and moving all others to an internal
> structure.
>
> Huawei then suggested a more radical yet much cleaner one: just exposing
> a virtio_net handle to application, just like the way kernel exposes an
> fd to user for locating a specific file, and exposing some new functions
> to access those old fields, such as flags, virt_qp_nb.
>
> With this change, we're likely to be free from ABI violations forever
> (well, except when we have to extend the virtio_net_device_ops struct).
> For example, following nice cleanup would not be a blocking one then:
>
>  http://dpdk.org/ml/archives/dev/2016-February/033528.html
>
> Suggested-by: Huawei Xie 
> Cc: Ilya Maximets 
> Signed-off-by: Yuanhan Liu 
> ---
>   doc/guides/rel_notes/deprecation.rst | 7 +++
>   1 file changed, 7 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst 
> b/doc/guides/rel_notes/deprecation.rst
> index ad31355..7d16d86 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -40,3 +40,10 @@ Deprecation Notices
> The existing API will be backward compatible, but there will be new API
> functions added to facilitate the creation of mempools using an external
> handler. The 16.07 release will contain these changes.
> +
> +* A librte_vhost public structures refactor is planned for DPDK 16.07
> +  that requires both ABI and API change.
> +  The proposed refactor would expose DPDK vhost dev to applications as
> +  a handle, like the way kernel exposes an fd to user for locating a
> +  specific file, and to keep all major structures internally, so that
> +  we are likely to be free from ABI violations in future.
>

Acked-by: Panu Matilainen 

I applaud the initiative, public structs are by far the worst offender 
when trying to maintain a stable ABI because they're so hard to 
correctly version that hardly anybody besides glibc bothers.

- Panu -

[dpdk-dev] DPDK namespace

[Panu Matilainen]

On 04/06/2016 08:26 AM, Yuanhan Liu wrote:
> On Tue, Apr 05, 2016 at 05:31:22PM +0300, Arnon Warshavsky wrote:
>> On Tue, Apr 5, 2016 at 5:13 PM, Trahe, Fiona  
>> wrote:
>>
>>>
>>>
 -Original Message-
 From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
 Sent: Tuesday, April 05, 2016 2:57 PM
 To: dev at dpdk.org
 Subject: [dpdk-dev] DPDK namespace

 DPDK is going to be more popular in Linux distributions.
 It means people will have some DPDK files in their /usr/include and some
>>> DPDK
 libraries on their system.

 Let's imagine someone trying to compile an application which needs
 rte_ethdev.h. He has to figure out that this "rte header" is provided by
>>> the DPDK.
 Hopefully it will be explained on StackOverflow that RTE stands for DPDK.
 Then someone else will try to run a binary without having installed the
>>> DPDK
 libraries. The linker will require libethdev.so (no prefix here).
 StackOverflow will probably have another good answer (among wrong ones):
 "Hey Sherlock Holmes, have you tried to install the DPDK library?"
 Followed by an insight: "You know, the DPDK naming is weird..."
 And we could continue the story with developers having some naming clash
 because of some identifiers not prefixed at all.

 The goal of this email is to get some feedback on how important it is to
>>> fix the
 DPDK namespace.

 If there is enough agreement that we should do something, I suggest to
 introduce the "dpdk_" prefix slowly and live with both "rte_" and "dpdk_"
 during some time.
 We could start using the new prefix for the new APIs (example: crypto)
>>> or when
 there is a significant API break (example: mempool).

 Opinions welcome!
>>> I don't have an opinion on how important it is to fix the namespace,
>>> though it does seem like a good idea.
>>> However if it's to be done, in my opinion it should be completed quickly
>>> or will just cause more confusion.
>>> So if rte_cryptoxxx becomes dpdk_cryptoxxx all other libraries should
>>> follow in next release or two, with
>>> the resulting ABI compatibility handling. Maybe with dual naming handled
>>> for several releases, but a
>>> clear end date when all are converted.
>>> Else there will be many years with a mix of rte_ and dpdk_
>>>
>>>
>>
>> Googling rte functions or error codes usually takes you to dpdk dev email
>> archive so I don't think it is that much difficult to figure out where rte
>> comes from.
>> Other than that , except for my own refactoring pains when replacing a dpdk
>> version, I do not see a major reason why not.
>> If Going for dpdk_ prefix, I agree with the quick death approach.
>
> +1: it's a bit weird to keep both, especially for a long while, that
> every time we turn a rte_ prefix to dpdk_ prefix, we break applications.
> Instead of breaking applications many times, I'd prefer to break once.
> Therefore, applications could do a simple global rte_ -> dpdk_ substitute:
> it doesn't sound that painful then.

I concur. If (and I think that should be a pretty big IF) the prefix is 
to be changed then its better done in one fast sweep than gradually.

Gratuitious (or nearly so) change is always extremely annoying, and the 
longer it takes the more painful it is. Application developers wont much 
care what the prefix is as long as its consistent, but if they're forced 
to track prefix changes across several releases with different libraries 
moving at different pace, they WILL be calling for bloody murder :)

As for rte_ being strange for DPDK - yes it is, but it takes like 5 
minutes to get over it. It would help to have it explained on dpdk.org 
FAQ: "Due to historical reasons, DPDK libraries are prefixed rte_ 
instead of dpdk_ because  and changing it is unnecessarily painful."

>
> And here are few more comments:
>
> - we should add rte_/dpdk_ prefix to all public structures as well.
>
>I'm thinking we are doing well here. I'm just aware that vhost lib
>does a bad job, which is something I proposed to fix in next release.

Yup, all public symbols should be prefixed. What the exact prefix is 
isn't that important really.

>
> - If we do the whole change once, I'd suggest to do it ASAP when this
>release is over.
>
>It should be a HUGE change that touches a lot of code, if we do it
>later, at a stage that a lot of patches for new features have been
>made or sent out, all of them need rebase. That'd be painful.

Nod, that's yet another aspect to consider.

So to summarize, I'm not strongly opposed to doing a one-time mass rte_ 
-> dpdk_ prefix change, but it needs to be one big sweep all at once, or 
not do it at all. Gradual change is a suicide.

Keeping rte_ is not the end of the world by any means, especially when 
applied consistently and explained someplace.

- Panu -

[dpdk-dev] [PATCH v2 0/7] Various fixes to compile with gcc6

[Panu Matilainen]

On 03/31/2016 06:02 PM, Thomas Monjalon wrote:
> 2016-03-22 17:37, Aaron Conole:
>> This series brings a number of code cleanups to allow building using gcc6,
>> with various legitimate warnings being fixed.
>>
>> Some of these fixes are to the drivers area, making this series a bit
>> atypical. However, the fixes identified in patches 2 and 3 are actual
>> bugs and should be fixed.
>>
>> The first patch from the original series has been dropped. It is no
>> longer needed, after commit 5ecdeba601d1 ("lpm: merge tbl24 and tbl8
>> structures").
>>
>>
>> Aaron Conole (7):
>>app/test/test: Fix missing brackets
>>drivers/net/e1000: Fix missing brackets
>>drivers/net/e1000: Fix missing lsc interrupt check brackets
>>drivers/net/ixgbe: Fix vlan filter missing brackets
>>drivers/net/e1000/igb: Signed left shift operator
>>drivers/net/ixgbe: Signed left shift operator
>>drivers/net/ixgbe: Fix uninitialized warning
>
> Applied with v3 of patch 2, thanks
>

...and so we finally have a warning-free build with gcc 6.

Thanks,

- Panu -

[dpdk-dev] [PATCH v2 2/7] drivers/net/e1000: Fix missing brackets

[Panu Matilainen]

On 03/24/2016 02:35 AM, Lu, Wenzhuo wrote:
> Hi Thomas, Aaron,
>>> Wenzhuo, do you agree to fix the base driver here?
> Honestly I find the logic has some problem and maybe more changes needed. I'm 
> talking with our kernel driver contactors to make sure they have no concern 
> about  my idea.
> And Aaron, do we really need to fix this code? For I find this function is 
> not called. So it has no real impact to us. Could we just wait until kernel 
> driver fixes it and leverage the new version? Thanks.

It breaks the build with gcc >= 6.0 due to -Werror so yes we need to fix 
it, one way or the other.

As spelled out in an earlier mail 
(http://dpdk.org/ml/archives/dev/2016-March/034290.html), for somebody 
building with gcc >= 6.0 the options basically are:
1) disable the driver entirely
2) build with -Wno-error
3) paper over it with local warning disablers
4) patch the thing locally

If the offending function really is not used at all in DPDK context then 
3) is an entirely valid option for an upstream solution, ie something 
like (untested)

--- a/drivers/net/e1000/Makefile
+++ b/drivers/net/e1000/Makefile
@@ -54,6 +54,9 @@ else
  #
  CFLAGS_BASE_DRIVER = -Wno-uninitialized -Wno-unused-parameter
  CFLAGS_BASE_DRIVER += -Wno-unused-variable
+ifeq ($(shell test $(GCC_VERSION) -ge 60 && echo 1), 1)
+CFLAGS_BASE_DRIVER += -Wno-misleading-indentation
+endif
  endif

  #


- Panu -

[dpdk-dev] [PATCH v2] mk: fix eal shared library dependencies -lrt

[Panu Matilainen]

On 03/22/2016 11:58 AM, Daniel Mrzyglod wrote:
> For GLIBC < 2.17 it is necessery to add -lrt for linker
> from glibc > 2.17 The `clock_*' suite of functions (declared in ) is 
> now
> available directly in the main C library. This affect Ubuntu 12.04 in i686
> and other older Linux Distros).
>
> Fixes: 4758404a3084 ("mk: fix eal shared library dependencies")
>
> Signed-off-by: Daniel Mrzyglod 
> ---
>   app/test/Makefile| 1 +
>   examples/ptpclient/Makefile  | 1 +
>   lib/librte_eal/linuxapp/eal/Makefile | 1 +
>   3 files changed, 3 insertions(+)
>
> diff --git a/app/test/Makefile b/app/test/Makefile
> index 00e4df2..707930f 100644
> --- a/app/test/Makefile
> +++ b/app/test/Makefile
> @@ -114,6 +114,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += test_cmdline_string.c
>   SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += test_cmdline_lib.c
>
>   ifeq ($(CONFIG_RTE_LIBRTE_SCHED),y)
> +LDFLAGS += -lrt
>   SRCS-y += test_red.c
>   SRCS-y += test_sched.c
>   endif
> diff --git a/examples/ptpclient/Makefile b/examples/ptpclient/Makefile
> index b77cf71..082c690 100644
> --- a/examples/ptpclient/Makefile
> +++ b/examples/ptpclient/Makefile
> @@ -46,6 +46,7 @@ SRCS-y := ptpclient.c
>
>   CFLAGS += -O3
>   CFLAGS += $(WERROR_FLAGS)
> +LDFLAGS += -lrt

These two should be LDLIBS instead. LDFLAGS is for passing in other 
arguments to the linker, such as path, LDLIBS exists solely for passing 
in the actual libraries.

- Panu -

[dpdk-dev] [PATCH 2/2] mk: fix two more missing libm dependencies

[Panu Matilainen]

Commit e86a699cf6b1 missed two further libm dependencies: ceil() used by
librte_meter is typically inlined so the missing dependency does not
actually cause failures, and librte_pmd_nfp is not built by default
so its easy to miss.

This causes duplicates in LDLIBS in many configurations so its vital
they are removed before passing to linker.

Fixes: e86a699cf6b1 ("mk: fix shared library dependencies on libm and librt")

Reported-by: Ferruh Yigit 
Signed-off-by: Panu Matilainen 
---
 drivers/net/nfp/Makefile  | 2 ++
 lib/librte_meter/Makefile | 2 ++
 mk/rte.app.mk | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/drivers/net/nfp/Makefile b/drivers/net/nfp/Makefile
index ef7a13d..11f 100644
--- a/drivers/net/nfp/Makefile
+++ b/drivers/net/nfp/Makefile
@@ -39,6 +39,8 @@ LIB = librte_pmd_nfp.a
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)

+LDLIBS += -lm
+
 EXPORT_MAP := rte_pmd_nfp_version.map

 LIBABIVER := 1
diff --git a/lib/librte_meter/Makefile b/lib/librte_meter/Makefile
index 8765881..f07fced 100644
--- a/lib/librte_meter/Makefile
+++ b/lib/librte_meter/Makefile
@@ -39,6 +39,8 @@ LIB = librte_meter.a
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)

+LDLIBS += -lm
+
 EXPORT_MAP := rte_meter_version.map

 LIBABIVER := 1
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index f4eb5e8..2f9bafe 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -85,6 +85,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)  += -lrte_vhost
 ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
 _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lm
 _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lrt
+_LDLIBS-$(CONFIG_RTE_LIBRTE_METER)  += -lm
 ifeq ($(CONFIG_RTE_LIBRTE_VHOST_NUMA),y)
 _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)  += -lnuma
 endif
@@ -101,6 +102,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD)   += -libverbs
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SZEDATA2)   += -lsze2
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)+= -lxenstore
 _LDLIBS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD)  += -lgxio
+_LDLIBS-$(CONFIG_RTE_LIBRTE_NFP_PMD)+= -lm
 # QAT / AESNI GCM PMDs are dependent on libcrypto (from openssl)
 # for calculating HMAC precomputes
 ifeq ($(CONFIG_RTE_LIBRTE_PMD_QAT),y)
-- 
2.5.0

[dpdk-dev] [PATCH 1/2] mk: Eliminate possible duplicates from LDLIBS

[Panu Matilainen]

Duplicates in LDLIBS can cause link failures from multiply defined
symbols, ensure all libraries are only mentioned once. Can't use
sorting for duplicate elimination as order is critical so awk one-liner
is used.

Signed-off-by: Panu Matilainen 
---
 mk/rte.app.mk | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index a1cd9a3..f4eb5e8 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -174,6 +174,9 @@ _LDLIBS-y += --no-whole-archive

 LDLIBS += $(_LDLIBS-y) $(CPU_LDLIBS) $(EXTRA_LDLIBS)

+# Eliminate duplicates without sorting
+LDLIBS := $(shell echo $(LDLIBS) | awk '{for (i = 1; i <= NF; i++) { if 
(!seen[$$i]++) print $$i }}')
+
 .PHONY: all
 all: install

-- 
2.5.0

[dpdk-dev] [PATCH] mk: fix linker script when re-building

[Panu Matilainen]

On 03/17/2016 01:22 AM, Sergio Gonzalez Monroy wrote:
> The linker script is generated by simply finding all libraries in
> RTE_OUTPUT/lib.
>
> The issue shows up when re-building the DPDK, hence already having a
> linker script in that directory, resulting in the linker script
> including itself.
>
> That does not play well with the linker.
>
> Simply filtering the linker script from all the found libraries solves
> the problem.
>
> Fixes: 948fd64befc3 ("mk: replace the combined library with a linker script")
>
> Signed-off-by: Sergio Gonzalez Monroy 
> ---
>   mk/rte.combinedlib.mk | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mk/rte.combinedlib.mk b/mk/rte.combinedlib.mk
> index fe4817b..449358b 100644
> --- a/mk/rte.combinedlib.mk
> +++ b/mk/rte.combinedlib.mk
> @@ -42,7 +42,7 @@ endif
>   RTE_LIBNAME := dpdk
>   COMBINEDLIB := lib$(RTE_LIBNAME)$(EXT)
>
> -LIBS := $(notdir $(wildcard $(RTE_OUTPUT)/lib/*$(EXT)))
> +LIBS := $(filter-out $(COMBINEDLIB), $(notdir $(wildcard 
> $(RTE_OUTPUT)/lib/*$(EXT
>
>   all: FORCE
>   $(Q)echo "GROUP ( $(LIBS) )" > $(RTE_OUTPUT)/lib/$(COMBINEDLIB)
>

Oops, thanks for spotting.

Acked-by: Panu Matilainen 

- Panu -

[dpdk-dev] [PATCH v3 0/2] slow data path communication between DPDK port and Linux

[Panu Matilainen]

On 03/16/2016 03:58 PM, Thomas Monjalon wrote:
> 2016-03-16 15:15, Panu Matilainen:
>> What I really would like to see is a clear policy regarding kernel
>> modules in DPDK. I certainly am in no position to dictate one, and
>> that's why I've been asking questions and throwing around crazy (or not)
>> ideas around the topic.
>
> I think the consensus is to avoid new kernel module,
> but allow them in a staging directory while being discussed upstream.

To me the more interesting question is: what happens after that?
As in, if upstream says no, does it mean axe from dpdk, no ifs and buts? 
If accepted upstream, does a version of the module still live within 
dpdk codebase (for example to provide the version for older kernel 
versions, I dont see that as unreasonable at all)?


> About the existing out-of-tree kernel modules, we must continue trying
> to obsolete them with upstream work.

Agreed.

>
> If you feel the consensus must be clearly stated and acked,
> please send a patch for doc/guides/contributing/design.rst.

I'll be happy to, once we have a clear consensus on what the policy 
actually is.

- Panu -

[dpdk-dev] [PATCH v3 0/2] slow data path communication between DPDK port and Linux

[Panu Matilainen]

On 03/16/2016 01:13 PM, Ferruh Yigit wrote:
> On 3/16/2016 10:45 AM, Thomas Monjalon wrote:
>> 2016-03-16 10:26, Ferruh Yigit:
>>> On 3/16/2016 8:22 AM, Panu Matilainen wrote:
>>>> On 03/16/2016 10:19 AM, Ferruh Yigit wrote:
>>>>> On 3/16/2016 7:26 AM, Panu Matilainen wrote:
>>>>>> On 03/14/2016 05:32 PM, Ferruh Yigit wrote:
>>>>>>> On 3/9/2016 11:17 AM, Ferruh Yigit wrote:
>>>>>>>> This patch sent to keep record of latest status of the work.
>>>>>>>>
>>>>>>>>
>>>>>>>> This is slow data path communication implementation based on existing 
>>>>>>>> KNI.
>>>>>>>>
>>>>>>>> Difference is: librte_kni converted into a PMD, kdp kernel module is 
>>>>>>>> almost
>>>>>>>> same except all control path functionality removed and some 
>>>>>>>> simplification done.
>>>>>>>>
>>>>>>>> Motivation is to simplify slow path data communication.
>>>>>>>> Now any application can use this new PMD to send/get data to Linux 
>>>>>>>> kernel.
>>>>>>>>
>>>>>>>> PMD supports two communication methods:
>>>>>>>>
>>>>>>>> 1) KDP kernel module
>>>>>>>> PMD initialization functions handles creating virtual interfaces (with 
>>>>>>>> help of
>>>>>>>> kdp kernel module) and created FIFO. FIFO is used to share data between
>>>>>>>> userspace and kernelspace. This is default method.
>>>>>>>>
>>>>>>>> 2) tun/tap module
>>>>>>>> When KDP module is not inserted, PMD creates tap interface and 
>>>>>>>> transfers
>>>>>>>> packets using tap interface.
>>>>>>>>
>>>>>>>> In long term this patch intends to replace the KNI and KNI will be
>>>>>>>> depreciated.
>>>>>>>>
>>>>>>>
>>>>>>> Self-NACK: Will work on another option that does not introduce new
>>>>>>> kernel module.
>>>>>>>
>>>>>>
>>>>>> Hmm, care to elaborate a bit? The second mode of this PMD already was
>>>>>> free of external kernel modules. Do you mean you'll be just removing
>>>>>> mode 1) from the PMD or looking at something completely different?
>>>>>>
>>>>>> Just thinking that tun/tap PMD sounds like a useful thing to have, I
>>>>>> hope you're not abandoning that.
>>>>>>
>>>>>
>>>>> It will be KNI PMD.
>>>>> Plan is to have something like KDP, but with existing KNI kernel module.
>>>>> There will be tun/tap support as fallback.
>>>>
>>>> Hum, now I'm confused. I was under the impression everybody hated KNI
>>>> and wanted to get rid of it, and certainly not build future solutions on
>>>> top of it?
>>>
>>> We can't remove it.
>>
>> Why?
>>
>>> We can't replace/improve it -you were one of the major opposition to this.
>>> This doesn't leave more option other than using it.
>>
>> Why cannot we replace it by something upstream?
>>
> I doubt KDP is upstream-able to Linux community. If somebody can, that
> is great.
>
> Even for KCP, upstreaming task is still under discussion, and as a heads
> up, it is likely to be dropped.

If KCP/KDP are not upstreamable then the solution is to find another way 
that is.

Easier said than done, no doubt.

- Panu -

[dpdk-dev] [PATCH v3 0/2] slow data path communication between DPDK port and Linux

[Panu Matilainen]

On 03/16/2016 12:26 PM, Ferruh Yigit wrote:
> On 3/16/2016 8:22 AM, Panu Matilainen wrote:
>> On 03/16/2016 10:19 AM, Ferruh Yigit wrote:
>>> On 3/16/2016 7:26 AM, Panu Matilainen wrote:
>>>> On 03/14/2016 05:32 PM, Ferruh Yigit wrote:
>>>>> On 3/9/2016 11:17 AM, Ferruh Yigit wrote:
>>>>>> This patch sent to keep record of latest status of the work.
>>>>>>
>>>>>>
>>>>>> This is slow data path communication implementation based on existing 
>>>>>> KNI.
>>>>>>
>>>>>> Difference is: librte_kni converted into a PMD, kdp kernel module is 
>>>>>> almost
>>>>>> same except all control path functionality removed and some 
>>>>>> simplification done.
>>>>>>
>>>>>> Motivation is to simplify slow path data communication.
>>>>>> Now any application can use this new PMD to send/get data to Linux 
>>>>>> kernel.
>>>>>>
>>>>>> PMD supports two communication methods:
>>>>>>
>>>>>> 1) KDP kernel module
>>>>>> PMD initialization functions handles creating virtual interfaces (with 
>>>>>> help of
>>>>>> kdp kernel module) and created FIFO. FIFO is used to share data between
>>>>>> userspace and kernelspace. This is default method.
>>>>>>
>>>>>> 2) tun/tap module
>>>>>> When KDP module is not inserted, PMD creates tap interface and transfers
>>>>>> packets using tap interface.
>>>>>>
>>>>>> In long term this patch intends to replace the KNI and KNI will be
>>>>>> depreciated.
>>>>>>
>>>>>
>>>>> Self-NACK: Will work on another option that does not introduce new
>>>>> kernel module.
>>>>>
>>>>
>>>> Hmm, care to elaborate a bit? The second mode of this PMD already was
>>>> free of external kernel modules. Do you mean you'll be just removing
>>>> mode 1) from the PMD or looking at something completely different?
>>>>
>>>> Just thinking that tun/tap PMD sounds like a useful thing to have, I
>>>> hope you're not abandoning that.
>>>>
>>>
>>> It will be KNI PMD.
>>> Plan is to have something like KDP, but with existing KNI kernel module.
>>> There will be tun/tap support as fallback.
>>
>> Hum, now I'm confused. I was under the impression everybody hated KNI
>> and wanted to get rid of it, and certainly not build future solutions on
>> top of it?
>>
>
> We can't remove it.
> We can't replace/improve it -you were one of the major opposition to this.

No no no. There's a misunderstanding somewhere in there.

I understand the functionality provided by KNI is important. I'd LOVE to 
see the it replaced. With something that does not require out-of-tree 
kernel modules.

As long as out-of-tree kernel modules are in the picture, the feature 
might as well not exist at all for the audience I'm dealing with. To 
that audience, replacing KNI with out-of-tree KCP/KDP or whatever is 
just irrelevant, there's no progress being made.

I also understand there are lot of users to whom out-of-tree kernel 
modules are not a problem at all, and I'm in no position to tell them 
that's somehow wrong. If KCP/KDP is better than KNI for that audience 
then more power to them.

But I dont see why such modules would *have* to be within the dpdk 
source - as suggested several times around this thread/topic such work 
could live in a separate repository or such.

What I really would like to see is a clear policy regarding kernel 
modules in DPDK. I certainly am in no position to dictate one, and 
that's why I've been asking questions and throwing around crazy (or not) 
ideas around the topic.

- Panu -

[dpdk-dev] [PATCH v3 0/2] slow data path communication between DPDK port and Linux

[Panu Matilainen]

On 03/16/2016 10:19 AM, Ferruh Yigit wrote:
> On 3/16/2016 7:26 AM, Panu Matilainen wrote:
>> On 03/14/2016 05:32 PM, Ferruh Yigit wrote:
>>> On 3/9/2016 11:17 AM, Ferruh Yigit wrote:
>>>> This patch sent to keep record of latest status of the work.
>>>>
>>>>
>>>> This is slow data path communication implementation based on existing KNI.
>>>>
>>>> Difference is: librte_kni converted into a PMD, kdp kernel module is almost
>>>> same except all control path functionality removed and some simplification 
>>>> done.
>>>>
>>>> Motivation is to simplify slow path data communication.
>>>> Now any application can use this new PMD to send/get data to Linux kernel.
>>>>
>>>> PMD supports two communication methods:
>>>>
>>>> 1) KDP kernel module
>>>> PMD initialization functions handles creating virtual interfaces (with 
>>>> help of
>>>> kdp kernel module) and created FIFO. FIFO is used to share data between
>>>> userspace and kernelspace. This is default method.
>>>>
>>>> 2) tun/tap module
>>>> When KDP module is not inserted, PMD creates tap interface and transfers
>>>> packets using tap interface.
>>>>
>>>> In long term this patch intends to replace the KNI and KNI will be
>>>> depreciated.
>>>>
>>>
>>> Self-NACK: Will work on another option that does not introduce new
>>> kernel module.
>>>
>>
>> Hmm, care to elaborate a bit? The second mode of this PMD already was
>> free of external kernel modules. Do you mean you'll be just removing
>> mode 1) from the PMD or looking at something completely different?
>>
>> Just thinking that tun/tap PMD sounds like a useful thing to have, I
>> hope you're not abandoning that.
>>
>
> It will be KNI PMD.
> Plan is to have something like KDP, but with existing KNI kernel module.
> There will be tun/tap support as fallback.

Hum, now I'm confused. I was under the impression everybody hated KNI 
and wanted to get rid of it, and certainly not build future solutions on 
top of it?

- Panu -

>
> Regards,
> ferruh
>

[dpdk-dev] [PATCH v3 0/2] slow data path communication between DPDK port and Linux

[Panu Matilainen]

On 03/14/2016 05:32 PM, Ferruh Yigit wrote:
> On 3/9/2016 11:17 AM, Ferruh Yigit wrote:
>> This patch sent to keep record of latest status of the work.
>>
>>
>> This is slow data path communication implementation based on existing KNI.
>>
>> Difference is: librte_kni converted into a PMD, kdp kernel module is almost
>> same except all control path functionality removed and some simplification 
>> done.
>>
>> Motivation is to simplify slow path data communication.
>> Now any application can use this new PMD to send/get data to Linux kernel.
>>
>> PMD supports two communication methods:
>>
>> 1) KDP kernel module
>> PMD initialization functions handles creating virtual interfaces (with help 
>> of
>> kdp kernel module) and created FIFO. FIFO is used to share data between
>> userspace and kernelspace. This is default method.
>>
>> 2) tun/tap module
>> When KDP module is not inserted, PMD creates tap interface and transfers
>> packets using tap interface.
>>
>> In long term this patch intends to replace the KNI and KNI will be
>> depreciated.
>>
>
> Self-NACK: Will work on another option that does not introduce new
> kernel module.
>

Hmm, care to elaborate a bit? The second mode of this PMD already was 
free of external kernel modules. Do you mean you'll be just removing 
mode 1) from the PMD or looking at something completely different?

Just thinking that tun/tap PMD sounds like a useful thing to have, I 
hope you're not abandoning that.

- Panu -

[dpdk-dev] [PATCH v3 1/3] mk: clear up libm and librt linkage confusion

[Panu Matilainen]

On 03/14/2016 06:44 PM, Ferruh Yigit wrote:
> On 3/10/2016 1:15 PM, Panu Matilainen wrote:
>> There are two places that need -lm (test app and librte_sched) and
>> exactly one that needs -lrt (librte_sched). Add the relevant
>> DT_NEEDED entries to both, and eliminate the bogus discrepancy
>> between Linux and BSD EXECENV_LDLIBS wrt these libs.
>>
>> Signed-off-by: Panu Matilainen 
>> ---
>>   app/test/Makefile| 2 ++
>>   lib/librte_sched/Makefile| 3 +++
>>   mk/exec-env/linuxapp/rte.vars.mk | 2 +-
>>   mk/rte.app.mk| 6 ++
>>   4 files changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/app/test/Makefile b/app/test/Makefile
>> index ec33e1a..00e4df2 100644
>> --- a/app/test/Makefile
>> +++ b/app/test/Makefile
>> @@ -160,6 +160,8 @@ CFLAGS += $(WERROR_FLAGS)
>>
>>   CFLAGS += -D_GNU_SOURCE
>>
>> +LDLIBS += -lm
>> +
>>   # Disable VTA for memcpy test
>>   ifeq ($(CC), gcc)
>>   ifeq ($(shell test $(GCC_VERSION) -ge 44 && echo 1), 1)
>> diff --git a/lib/librte_sched/Makefile b/lib/librte_sched/Makefile
>> index b1cb285..4d631f6 100644
>> --- a/lib/librte_sched/Makefile
>> +++ b/lib/librte_sched/Makefile
>> @@ -41,6 +41,9 @@ CFLAGS += $(WERROR_FLAGS)
>>
>>   CFLAGS_rte_red.o := -D_GNU_SOURCE
>>
>> +LDLIBS += -lm
>> +LDLIBS += -lrt
>> +
>>   EXPORT_MAP := rte_sched_version.map
>>
>>   LIBABIVER := 1
>> diff --git a/mk/exec-env/linuxapp/rte.vars.mk 
>> b/mk/exec-env/linuxapp/rte.vars.mk
>> index 5fd7d85..d51bd17 100644
>> --- a/mk/exec-env/linuxapp/rte.vars.mk
>> +++ b/mk/exec-env/linuxapp/rte.vars.mk
>> @@ -48,7 +48,7 @@ endif
>>   # Workaround lack of DT_NEEDED entry
>>   EXECENV_LDFLAGS = --no-as-needed
>>
>> -EXECENV_LDLIBS  = -lrt -lm
>> +EXECENV_LDLIBS  =
>>   EXECENV_ASFLAGS =
>>
>>   ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
>> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
>> index daac09f..cadc7ab 100644
>> --- a/mk/rte.app.mk
>> +++ b/mk/rte.app.mk
>> @@ -77,11 +77,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM)+= -lrte_lpm
>>   _LDLIBS-$(CONFIG_RTE_LIBRTE_POWER)  += -lrte_power
>>   _LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)+= -lrte_acl
>>   _LDLIBS-$(CONFIG_RTE_LIBRTE_METER)  += -lrte_meter
>> -
>>   _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lrte_sched
>> -_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lm
>> -_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lrt
>> -
>>   _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)  += -lrte_vhost
>>
>>   ifeq ($(CONFIG_RTE_LIBRTE_VHOST_NUMA),y)
>> @@ -104,6 +100,8 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)+= -lxenstore
>>   _LDLIBS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD)  += -lgxio
>>   # QAT PMD has a dependency on libcrypto (from openssl) for calculating 
>> HMAC precomputes
>>   _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_QAT)+= -lcrypto
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lm
>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)  += -lrt
>>   endif # !CONFIG_RTE_BUILD_SHARED_LIBS
>>
>>   _LDLIBS-y += --start-group
>>
> This is causing a shared library compilation error with gcc:
>
> "
> == Build app/test-acl
>LD testacl
> /root/dpdk/build/lib/librte_meter.so: error: undefined reference to 'ceil'
> collect2: error: ld returned 1 exit status
> "
>
> There is an indirect libm dependency from test-acl. Adding -lm fixes the
> issue.
>
> But this issue not seen by everybody, not sure why I am getting this but
> not others.
>
> Also clang compiles fine, only fails with gcc.
> I am using Fedora 23, gcc version:
> gcc (GCC) 5.3.1 20151207 (Red Hat 5.3.1-2)
>
> I will dig some more.

Right, librte_meter indeed has a dependency on libm that I've missed.
The curious thing is why is it not failing everywhere - I cannot 
reproduce that at all on Fedora 23. Poking around in math.h leads to 
bits/mathlinline.h where the answer probably lies: ceil() is typically 
provided by inline code, but for one reason or another that is not 
available in your setup.

Anyway, I'll send a patch to add the missing dependency, thanks for 
spotting and reporting!

- Panu -

> Regards,
> ferruh
>
>
>
>

[dpdk-dev] [PATCH v4 1/2] ethdev: add vlan type for setting ether type

[Panu Matilainen]

On 03/11/2016 10:49 AM, Helin Zhang wrote:
> In order to set ether type of VLAN for single VLAN, inner
> and outer VLAN, the VLAN type as an input parameter is added
> to 'rte_eth_dev_set_vlan_ether_type()'.
> In addition, corresponding changes in e1000, ixgbe and i40e
> are also added.
>
> Signed-off-by: Helin Zhang 
> Acked-by: Wenzhuo Lu 
> ---
>   app/test-pmd/cmdline.c  | 30 +-
>   app/test-pmd/config.c   |  9 +++--
>   app/test-pmd/testpmd.h  |  3 +-
>   doc/guides/rel_notes/release_16_04.rst  |  4 ++
>   doc/guides/testpmd_app_ug/testpmd_funcs.rst | 11 +
>   drivers/net/e1000/igb_ethdev.c  | 21 +++---
>   drivers/net/i40e/i40e_ethdev.c  | 63 
> +++--
>   drivers/net/ixgbe/ixgbe_ethdev.c| 19 +++--
>   lib/librte_ether/rte_ethdev.c   | 25 +++-
>   lib/librte_ether/rte_ethdev.h   | 23 ++-
>   lib/librte_ether/rte_ether_version.map  |  7 
>   11 files changed, 175 insertions(+), 40 deletions(-)
>
> v4:
>   - Updated the doc of testpmd guide.
>
> v3:
>   - Used versioning mechanism to avoid ABI issue.
>   - Re-organized the patch set.
>
> v2:
>   - Used RTE_NEXT_ABI to avoid ABI change issue.
>   - Reworked the announcement of ABI change for release 16.07.
>
[...]
> @@ -1077,7 +1088,7 @@ typedef int (*vlan_filter_set_t)(struct rte_eth_dev 
> *dev,
>   /**< @internal filtering of a VLAN Tag Identifier by an Ethernet device. */
>
>   typedef void (*vlan_tpid_set_t)(struct rte_eth_dev *dev,
> -   uint16_t tpid);
> + enum rte_vlan_type type, uint16_t tpid);
>   /**< @internal set the outer VLAN-TPID by an Ethernet device. */
>
>   typedef void (*vlan_offload_set_t)(struct rte_eth_dev *dev, int mask);
> @@ -2346,6 +2357,8 @@ int rte_eth_dev_set_vlan_strip_on_queue(uint8_t 
> port_id, uint16_t rx_queue_id,
>*
>* @param port_id
>*   The port identifier of the Ethernet device.
> + * @vlan_type
> + *   The vlan type.
>* @param tag_type
>*   The Tag Protocol ID
>* @return
> @@ -2353,7 +2366,13 @@ int rte_eth_dev_set_vlan_strip_on_queue(uint8_t 
> port_id, uint16_t rx_queue_id,
>*   - (-ENOSUP) if hardware-assisted VLAN TPID setup is not supported.
>*   - (-ENODEV) if *port_id* invalid.
>*/
> -int rte_eth_dev_set_vlan_ether_type(uint8_t port_id, uint16_t tag_type);
> +int rte_eth_dev_set_vlan_ether_type(uint8_t port_id,
> + enum rte_vlan_type vlan_type,
> + uint16_t tag_type);
> +int rte_eth_dev_set_vlan_ether_type_v22(uint8_t port_id, uint16_t tag_type);
> +int rte_eth_dev_set_vlan_ether_type_v1604(uint8_t port_id,
> +   enum rte_vlan_type vlan_type,
> +   uint16_t tag_type);
>
>   /**
>* Set VLAN offload configuration on an Ethernet device

Its nice to see people actually trying to be compatible on occasion :)

However in this case there's not much point in doing so, because 
libethdev ABI has already been broken in this cycle:
http://dpdk.org/browse/dpdk/commit/?id=cfd2279ea6299826fe992028f1dffaf9fa7e7d0a

In other words, the compatibility versions can never get invoked because 
all software built against libethdev needs to be rebuilt anyway because 
of the soname bump. Just drop the compat versions, no point carrying 
around something that cannot possibly get used.

- Panu -

[dpdk-dev] [PATCH] mk: crypto pmds can only be built if librte_cryptodev is enabled

[Panu Matilainen]

On 03/11/2016 11:28 AM, Thomas Monjalon wrote:
> 2016-03-11 11:13, Panu Matilainen:
>> If the experimental CONFIG_RTE_LIBRTE_CRYPTODEV is disabled,
>> build of any crypto pmds will fail because of the missing dependency.
>> This has been present for a while now but hidden until the addition
>> of null_crypto since all the other crypto pmds have been disabled
>> by default.
>
> Good catch, thanks.
>
>> +ifeq ($(CONFIG_RTE_LIBRTE_CRYPTODEV),y)
>>  DIRS-y += crypto
>> +endif
>
> Why not
> DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto ?

Just because old habits ... and ... you know :)

Should I send a new version or can you fix that up when committing?

- Panu -

[dpdk-dev] [PATCH] mk: crypto pmds can only be built if librte_cryptodev is enabled

[Panu Matilainen]

If the experimental CONFIG_RTE_LIBRTE_CRYPTODEV is disabled,
build of any crypto pmds will fail because of the missing dependency.
This has been present for a while now but hidden until the addition
of null_crypto since all the other crypto pmds have been disabled
by default.

Conditionalize the entire drivers/crypto directory on
CONFIG_RTE_LIBRTE_CRYPTODEV to fix.

Fixes: 1703e94ac5ce ("qat: add driver for QuickAssist devices")

Signed-off-by: Panu Matilainen 
---
 drivers/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/Makefile b/drivers/Makefile
index 6ec67f6..c6758a1 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -32,6 +32,8 @@
 include $(RTE_SDK)/mk/rte.vars.mk

 DIRS-y += net
+ifeq ($(CONFIG_RTE_LIBRTE_CRYPTODEV),y)
 DIRS-y += crypto
+endif

 include $(RTE_SDK)/mk/rte.subdir.mk
-- 
2.5.0

[dpdk-dev] [PATCH 8/8] drivers/net/ixgbe: Fix uninitialized warning

[Panu Matilainen]

On 03/10/2016 04:45 PM, Remy Horton wrote:
>
> On 10/03/2016 13:42, Panu Matilainen wrote:
>> On 02/25/2016 08:48 PM, Aaron Conole wrote:
>>> Silence a compiler warning that this variable may be used uninitialized.
>>>
>>> Signed-off-by: Aaron Conole 
> [..]
>>
>> The patch looks ok as such, but then again warning looks like a false
>> positive to me: assignment and dereferencing depend on the same value of
>> eop, which cannot change between the two.
>
> In two minds about this. It is a logical impossibility, but these days
> optimising compilers are getting very aggressive. For instance GCC has a
> delightfully-named -fdelete-null-pointer-checks option, which caused
> security holes..

Indeed, that's why silencing a false positive (assuming it actually is 
one) by throwing some more NULL-checks for the allegedly impossible 
makes me a bit nervous. Besides compiler optimizations going crazy, I've 
seen such extra NULL-checks turn into actual bugs when surroundings 
subtly change.

- Panu -

[dpdk-dev] [RFC 35/35] mempool: update copyright

[Panu Matilainen]

On 03/09/2016 08:52 PM, Stephen Hemminger wrote:
> I understand that 6Wind has made major contributions to DPDK in many places.
>
> I would prefer that each file not get copyright additions from each
> contributor,
> otherwise this starts a bad precedent where the source gets cluttered with
> every contributor.

That, and they also add rather useless noise to patches and commit 
history just because people feel compelled to update the copyright years.

Many projects have a separate credits file where contributors get noted, 
but I guess those tend to be under copyleft licenses, the BSD license 
expects somebody to claim copyright.

Anyway, I'd much rather see one toplevel license where all such updates 
go. It'd make life easier for packagers whose distros require including 
a license file in packages, and it'd also help fix the first impression 
of dpdk being under [L]GPL (which easily happens if you just glimpse at 
the toplevel source directory)

This is of course getting a bit side-tracked for this patch...

- Panu -

[dpdk-dev] [PATCH] pipeline: use unsigned constants for left shift operations

[Panu Matilainen]

Tell the compiler to use unsigned constants for left shift ops,
otherwise building with gcc >= 6.0 fails due to multiple warnings like:
warning: left shift of negative value [-Wshift-negative-value]

Signed-off-by: Panu Matilainen 
---
 examples/ip_pipeline/pipeline/pipeline_common_fe.c | 4 ++--
 examples/ip_pipeline/pipeline/pipeline_firewall.c  | 4 ++--
 examples/ip_pipeline/pipeline/pipeline_routing.c   | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/examples/ip_pipeline/pipeline/pipeline_common_fe.c 
b/examples/ip_pipeline/pipeline/pipeline_common_fe.c
index bffc9a4..a691d42 100644
--- a/examples/ip_pipeline/pipeline/pipeline_common_fe.c
+++ b/examples/ip_pipeline/pipeline/pipeline_common_fe.c
@@ -337,7 +337,7 @@ app_link_config(struct app_params *app,
return -1;
}

-   netmask = (~0) << (32 - depth);
+   netmask = (~0U) << (32 - depth);
host = ip & netmask;
bcast = host | (~netmask);

@@ -889,7 +889,7 @@ print_link_info(struct app_link_params *p)
 {
struct rte_eth_stats stats;
struct ether_addr *mac_addr;
-   uint32_t netmask = (~0) << (32 - p->depth);
+   uint32_t netmask = (~0U) << (32 - p->depth);
uint32_t host = p->ip & netmask;
uint32_t bcast = host | (~netmask);

diff --git a/examples/ip_pipeline/pipeline/pipeline_firewall.c 
b/examples/ip_pipeline/pipeline/pipeline_firewall.c
index 3d7ea7a..320b25d 100644
--- a/examples/ip_pipeline/pipeline/pipeline_firewall.c
+++ b/examples/ip_pipeline/pipeline/pipeline_firewall.c
@@ -256,10 +256,10 @@ app_pipeline_firewall_key_check_and_normalize(struct 
pipeline_firewall_key *key)
return -1;

if (src_ip_depth)
-   src_ip_netmask = (~0) << (32 - src_ip_depth);
+   src_ip_netmask = (~0U) << (32 - src_ip_depth);

if (dst_ip_depth)
-   dst_ip_netmask = ((~0) << (32 - dst_ip_depth));
+   dst_ip_netmask = ((~0U) << (32 - dst_ip_depth));

key->key.ipv4_5tuple.src_ip &= src_ip_netmask;
key->key.ipv4_5tuple.dst_ip &= dst_ip_netmask;
diff --git a/examples/ip_pipeline/pipeline/pipeline_routing.c 
b/examples/ip_pipeline/pipeline/pipeline_routing.c
index 6354730..62a5eec 100644
--- a/examples/ip_pipeline/pipeline/pipeline_routing.c
+++ b/examples/ip_pipeline/pipeline/pipeline_routing.c
@@ -319,7 +319,7 @@ app_pipeline_routing_add_route(struct app_params *app,
if ((depth == 0) || (depth > 32))
return -1;

-   netmask = (~0) << (32 - depth);
+   netmask = (~U0) << (32 - depth);
key->key.ipv4.ip &= netmask;

/* data */
@@ -421,7 +421,7 @@ app_pipeline_routing_delete_route(struct app_params *app,
if ((depth == 0) || (depth > 32))
return -1;

-   netmask = (~0) << (32 - depth);
+   netmask = (~0U) << (32 - depth);
key->key.ipv4.ip &= netmask;
}
break;
-- 
2.5.0

[dpdk-dev] [PATCH 8/8] drivers/net/ixgbe: Fix uninitialized warning

[Panu Matilainen]

On 02/25/2016 08:48 PM, Aaron Conole wrote:
> Silence a compiler warning that this variable may be used uninitialized.
>
> Signed-off-by: Aaron Conole 
> ---
>   drivers/net/ixgbe/ixgbe_rxtx.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index e95e6b7..775edc7 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -1563,7 +1563,7 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct rte_mbuf 
> **rx_pkts, uint16_t nb_pkts,
>   struct ixgbe_rx_entry *rxe;
>   struct ixgbe_scattered_rx_entry *sc_entry;
>   struct ixgbe_scattered_rx_entry *next_sc_entry;
> - struct ixgbe_rx_entry *next_rxe;
> + struct ixgbe_rx_entry *next_rxe = NULL;
>   struct rte_mbuf *first_seg;
>   struct rte_mbuf *rxm;
>   struct rte_mbuf *nmb;
> @@ -1740,7 +1740,7 @@ next_desc:
>* the pointer to the first mbuf at the NEXTP entry in the
>* sw_sc_ring and continue to parse the RX ring.
>*/
> - if (!eop) {
> + if (!eop && next_rxe) {
>   rxm->next = next_rxe->mbuf;
>   next_sc_entry->fbuf = first_seg;
>   goto next_desc;
>

The patch looks ok as such, but then again warning looks like a false 
positive to me: assignment and dereferencing depend on the same value of 
eop, which cannot change between the two.

CC'ing the maintainers for attention...

- Panu -

[dpdk-dev] [PATCH 7/8] drivers/net/ixgbe: Signed left shift operator

[Panu Matilainen]

On 02/25/2016 08:48 PM, Aaron Conole wrote:
> Tell the compiler to use an unsigned constant for the config shifts.
>
> Signed-off-by: Aaron Conole 
> ---
>   drivers/net/ixgbe/ixgbe_pf.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_pf.c b/drivers/net/ixgbe/ixgbe_pf.c
> index 2ffbd1f..8b5119f 100644
> --- a/drivers/net/ixgbe/ixgbe_pf.c
> +++ b/drivers/net/ixgbe/ixgbe_pf.c
> @@ -236,9 +236,9 @@ int ixgbe_pf_host_configure(struct rte_eth_dev *eth_dev)
>   vfre_slot = (vf_num >> VFRE_SHIFT) > 0 ? 1 : 0;
>
>   /* Enable pools reserved to PF only */
> - IXGBE_WRITE_REG(hw, IXGBE_VFRE(vfre_slot), (~0) << vfre_offset);
> + IXGBE_WRITE_REG(hw, IXGBE_VFRE(vfre_slot), (~0U) << vfre_offset);
>   IXGBE_WRITE_REG(hw, IXGBE_VFRE(vfre_slot ^ 1), vfre_slot - 1);
> - IXGBE_WRITE_REG(hw, IXGBE_VFTE(vfre_slot), (~0) << vfre_offset);
> + IXGBE_WRITE_REG(hw, IXGBE_VFTE(vfre_slot), (~0U) << vfre_offset);
>   IXGBE_WRITE_REG(hw, IXGBE_VFTE(vfre_slot ^ 1), vfre_slot - 1);
>
>   /* PFDMA Tx General Switch Control Enables VMDQ loopback */
>
Acked-by: Panu Matilainen 

CC'd the ixgbe maintainers...

- Panu -

[dpdk-dev] [PATCH 6/8] drivers/net/e1000/igb: Signed left shift operator

[Panu Matilainen]

On 02/25/2016 08:48 PM, Aaron Conole wrote:
> Tell the compiler to use an unsigned constant for the config shifts.
>
> Signed-off-by: Aaron Conole 
> ---
>   drivers/net/e1000/igb_pf.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/e1000/igb_pf.c b/drivers/net/e1000/igb_pf.c
> index 1d00dda..afe80f5 100644
> --- a/drivers/net/e1000/igb_pf.c
> +++ b/drivers/net/e1000/igb_pf.c
> @@ -172,8 +172,8 @@ int igb_pf_host_configure(struct rte_eth_dev *eth_dev)
>   E1000_WRITE_REG(hw, E1000_VT_CTL, vtctl);
>
>   /* Enable pools reserved to PF only */
> - E1000_WRITE_REG(hw, E1000_VFRE, (~0) << vf_num);
> - E1000_WRITE_REG(hw, E1000_VFTE, (~0) << vf_num);
> + E1000_WRITE_REG(hw, E1000_VFRE, (~0U) << vf_num);
> + E1000_WRITE_REG(hw, E1000_VFTE, (~0U) << vf_num);
>
>   /* PFDMA Tx General Switch Control Enables VMDQ loopback */
>   if (hw->mac.type == e1000_i350)
>
Acked-by: Panu Matilainen 

CC'd the e1000 maintainer too.

- Panu -

[dpdk-dev] [PATCH 5/8] drivers/net/ixgbe: Fix vlan filter missing brackets

[Panu Matilainen]

On 02/25/2016 08:48 PM, Aaron Conole wrote:
> The ixgbe vlan filter code has an if check with an incorrect whitespace.
>
> Signed-off-by: Aaron Conole 
> ---
>   drivers/net/ixgbe/ixgbe_ethdev.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c 
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 3e6fe86..2e1c3ad 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -4258,10 +4258,11 @@ ixgbe_set_pool_vlan_filter(struct rte_eth_dev *dev, 
> uint16_t vlan,
>   if (ixgbe_vmdq_mode_check(hw) < 0)
>   return -ENOTSUP;
>   for (pool_idx = 0; pool_idx < ETH_64_POOLS; pool_idx++) {
> - if (pool_mask & ((uint64_t)(1ULL << pool_idx)))
> + if (pool_mask & ((uint64_t)(1ULL << pool_idx))) {
>   ret = hw->mac.ops.set_vfta(hw,vlan,pool_idx,vlan_on);
>   if (ret < 0)
>   return ret;
> + }
>   }
>
>   return ret;
>

Acked-by: Panu Matilainen 

Seems really obvious but cc'd the ixgbe maintainers too.

- Panu -

[dpdk-dev] [PATCH 4/8] drivers/net/e1000: Fix missing lsc interrupt check brackets

[Panu Matilainen]

On 02/25/2016 08:48 PM, Aaron Conole wrote:
> The device lsc interupt check has a misleading whitespace around it which
> can be improved by adding appropriate braces to the check. Since the ret
> variable was checked after previous assignment, this introduces no functional
> change.
>
> Signed-off-by: Aaron Conole 
> ---
>   drivers/net/e1000/em_ethdev.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
> index 4a843fe..1d86091 100644
> --- a/drivers/net/e1000/em_ethdev.c
> +++ b/drivers/net/e1000/em_ethdev.c
> @@ -637,13 +637,14 @@ eth_em_start(struct rte_eth_dev *dev)
>
>   if (rte_intr_allow_others(intr_handle)) {
>   /* check if lsc interrupt is enabled */
> - if (dev->data->dev_conf.intr_conf.lsc != 0)
> + if (dev->data->dev_conf.intr_conf.lsc != 0) {
>   ret = eth_em_interrupt_setup(dev);
>   if (ret) {
>   PMD_INIT_LOG(ERR, "Unable to setup interrupts");
>   em_dev_clear_queues(dev);
>   return ret;
>   }
> + }
>   } else {
>   rte_intr_callback_unregister(intr_handle,
>   eth_em_interrupt_handler,
>

Acked-by: Panu Matilainen 

Seems really obvious but cc'd the e1000 maintainer too.

- Panu -

[dpdk-dev] [PATCH 2/8] app/test/test: Fix missing brackets

[Panu Matilainen]

On 02/25/2016 08:48 PM, Aaron Conole wrote:
> The test application calls printf(...) with the suite->suite_name argument.
> The intent (based on whitespace) in the printf is to check suite->suite_name
> first and then apply the printf. This doesn't happen due to missing brackets.
>
> Signed-off-by: Aaron Conole 
> ---
>   app/test/test.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/app/test/test.c b/app/test/test.c
> index f35b304..ccad0e3 100644
> --- a/app/test/test.c
> +++ b/app/test/test.c
> @@ -162,9 +162,10 @@ unit_test_suite_runner(struct unit_test_suite *suite)
>   int test_success;
>   unsigned total = 0, executed = 0, skipped = 0, succeeded = 0, failed = 
> 0;
>
> - if (suite->suite_name)
> + if (suite->suite_name) {
>   printf(" + 
> --- +\n");
>   printf(" + Test Suite : %s\n", suite->suite_name);
> + }
>
>   if (suite->setup)
>   if (suite->setup() != 0)
>

Acked-by: Panu Matilainen 

This is just about as obvious as they get...

- Panu -

[dpdk-dev] [PATCH v3 3/3] mk: add DT_NEEDED entries for librte_eal external dependencies

[Panu Matilainen]

Details between the platforms differ somewhat, and for static
builds they need to be handled from mk/exec-env still.

Signed-off-by: Panu Matilainen 
---
 lib/librte_eal/bsdapp/eal/Makefile   | 4 
 lib/librte_eal/linuxapp/eal/Makefile | 4 
 2 files changed, 8 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/Makefile 
b/lib/librte_eal/bsdapp/eal/Makefile
index 9ecf429..349b0d0 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -44,6 +44,10 @@ CFLAGS += -I$(RTE_SDK)/lib/librte_ring
 CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
 CFLAGS += $(WERROR_FLAGS) -O3

+LDLIBS += -lexecinfo
+LDLIBS += -lpthread
+LDLIBS += -lgcc_s
+
 EXPORT_MAP := rte_eal_version.map

 LIBABIVER := 2
diff --git a/lib/librte_eal/linuxapp/eal/Makefile 
b/lib/librte_eal/linuxapp/eal/Makefile
index d72f035..25b3a8e 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -49,6 +49,10 @@ CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
 CFLAGS += -I$(RTE_SDK)/lib/librte_ivshmem
 CFLAGS += $(WERROR_FLAGS) -O3

+LDLIBS += -ldl
+LDLIBS += -lpthread
+LDLIBS += -lgcc_s
+
 # specific to linuxapp exec-env
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) := eal.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_hugepage_info.c
-- 
2.5.0