[dpdk-dev] disable hugepages

2016-11-09 Thread Christian Ehrhardt
On Wed, Nov 9, 2016 at 1:55 PM, Keren Hochman 
wrote:

> how can I create mempool without hugepages?My application is running on a
> pcap file so no huge pages is needed ?
>

Not sure if that is what you really want (Debug use only), but in general
no-huge is available as EAL arg

>From http://pktgen.readthedocs.io/en/latest/usage_eal.html :

EAL options for DEBUG use only:
  --no-huge   : Use malloc instead of hugetlbfs


[dpdk-dev] [PATCH] mk: add missing *CPPFLAGS to rte.compile-pre.mk

2016-09-14 Thread Christian Ehrhardt
On Tue, Aug 30, 2016 at 7:25 PM, Luca Boccassi  wrote:

> Some targets in mk/internal/rte.compile-pre.mk are calling CC or
> HOSTCC without passing CPPFLAGS, EXTRA_CPPFLAGS or HOST_CPPFLAGS,
> HOST_EXTRA_CPPFLAGS.
> On Debian/Ubuntu builds this means that preprocessor flags set by the
> dpkg-buildpackage environment, like hardening flags, are not
> correctly passed to all objects builds.
>
> Signed-off-by: Luca Boccassi 
>

Acked-by: Christian Ehrhardt 


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] [PATCH] mk: use ?= instead of := for RTE_DEVEL_BUILD

2016-09-14 Thread Christian Ehrhardt
On Wed, Sep 14, 2016 at 12:50 PM, Luca Boccassi 
wrote:

> RTE_DEVEL_BUILD is set to := y in mk/rte.vars.mk, which makes it
> impossible to override via an environment variable, and forces users
> to pass it inline in the make call.
> Use ?= instead to have it pick up the environment variable as well.
>
> Cc: 
> Signed-off-by: Luca Boccassi 
>

Acked-by: Christian Ehrhardt 


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] [PATCH v2 4/4] doc: add basic invocation info for dpdk-devbind

2016-08-31 Thread Christian Ehrhardt
This summarizes the "how to call dpdk-pmdinfo" in one place to be
picked up by html/pdf/man-page docs.

That knowledge was available before but spread in various docs along
examples (which are great and have to be kept) as well as in the
--usage/--help option of the tool itself.

As a root only program in sbin it should belong to section 8
"8   System administration commands (usually only for root)"

Signed-off-by: Christian Ehrhardt 
---
 doc/guides/conf.py   |   4 +-
 doc/guides/tools/devbind.rst | 143 +++
 doc/guides/tools/index.rst   |   1 +
 mk/rte.sdkinstall.mk |   5 ++
 4 files changed, 152 insertions(+), 1 deletion(-)
 create mode 100644 doc/guides/tools/devbind.rst

diff --git a/doc/guides/conf.py b/doc/guides/conf.py
index c45c4be..149bcdb 100644
--- a/doc/guides/conf.py
+++ b/doc/guides/conf.py
@@ -113,7 +113,9 @@ man_pages = [("testpmd_app_ug/run_app", "testpmd",
  ("tools/proc_info", "dpdk-procinfo",
   "access dpdk port stats and memory info", "", 1),
  ("tools/pmdinfo", "dpdk-pmdinfo",
-  "dump a PMDs hardware support info", "", 1)]
+  "dump a PMDs hardware support info", "", 1),
+ ("tools/devbind", "dpdk-devbind",
+  "check device status and bind/unbind them from drivers", "", 8)]

  :numref: fallback 
 # The following hook functions add some simple handling for the :numref:
diff --git a/doc/guides/tools/devbind.rst b/doc/guides/tools/devbind.rst
new file mode 100644
index 000..18a8059
--- /dev/null
+++ b/doc/guides/tools/devbind.rst
@@ -0,0 +1,143 @@
+
+..  BSD LICENSE
+Copyright(c) 2016 Canonical 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.
+
+
+dpdk-devbind Application
+
+
+The ``dpdk-devbind`` tool is a Data Plane Development Kit (DPDK) tool that 
helps binding and unbinding devices from specific drivers.
+As well as checking their status in that regard.
+
+
+Running the Application
+---
+
+The tool has a number of command line options:
+
+.. code-block:: console
+
+   dpdk-devbind [options] DEVICE1 DEVICE2 
+
+OPTIONS
+---
+
+* ``--help, --usage``
+
+Display usage information and quit
+
+* ``-s, --status``
+
+Print the current status of all known network interfaces.
+For each device, it displays the PCI domain, bus, slot and function,
+along with a text description of the device. Depending upon whether the
+device is being used by a kernel driver, the ``igb_uio`` driver, or no
+driver, other relevant information will be displayed:
+- the Linux interface name e.g. ``if=eth0``
+- the driver being used e.g. ``drv=igb_uio``
+- any suitable drivers not currently using that device e.g. 
``unused=igb_uio``
+NOTE: if this flag is passed along with a bind/unbind option, the
+status display will always occur after the other operations have taken
+place.
+
+* ``-b driver, --bind=driver``
+
+Select the driver to use or "none" to unbind the device
+
+* ``-u, --unbind``
+
+Unbind a device (Equivalent to ``-b none``)
+
+* ``--force``
+
+By default, devices which are used by Linu

[dpdk-dev] [PATCH v2 3/4] doc: add basic invocation info for dpdk-pmdinfo

2016-08-31 Thread Christian Ehrhardt
This summarizes the "how to call dpdk-pmdinfo" in one place to be picked
up by html/pdf/man-page docs.

Signed-off-by: Christian Ehrhardt 
---
 doc/guides/conf.py   |  4 +++-
 doc/guides/tools/index.rst   |  1 +
 doc/guides/tools/pmdinfo.rst | 57 
 3 files changed, 61 insertions(+), 1 deletion(-)
 create mode 100644 doc/guides/tools/pmdinfo.rst

diff --git a/doc/guides/conf.py b/doc/guides/conf.py
index 55b6b2f..c45c4be 100644
--- a/doc/guides/conf.py
+++ b/doc/guides/conf.py
@@ -111,7 +111,9 @@ man_pages = [("testpmd_app_ug/run_app", "testpmd",
  ("tools/pdump", "dpdk-pdump",
   "enable packet capture on dpdk ports", "", 1),
  ("tools/proc_info", "dpdk-procinfo",
-  "access dpdk port stats and memory info", "", 1)]
+  "access dpdk port stats and memory info", "", 1),
+ ("tools/pmdinfo", "dpdk-pmdinfo",
+  "dump a PMDs hardware support info", "", 1)]

  :numref: fallback 
 # The following hook functions add some simple handling for the :numref:
diff --git a/doc/guides/tools/index.rst b/doc/guides/tools/index.rst
index d7654a2..80f2115 100644
--- a/doc/guides/tools/index.rst
+++ b/doc/guides/tools/index.rst
@@ -37,4 +37,5 @@ Tool User Guides

 proc_info
 pdump
+pmdinfo

diff --git a/doc/guides/tools/pmdinfo.rst b/doc/guides/tools/pmdinfo.rst
new file mode 100644
index 000..a90c59f
--- /dev/null
+++ b/doc/guides/tools/pmdinfo.rst
@@ -0,0 +1,57 @@
+
+..  BSD LICENSE
+Copyright(c) 2016 Canonical 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.
+
+
+dpdk-pmdinfo Application
+
+
+The ``dpdk-pmdinfo`` tool is a Data Plane Development Kit (DPDK) utility that
+can dump a PMDs hardware support info.
+
+
+Running the Application
+---
+
+The tool has a number of command line options:
+
+.. code-block:: console
+
+
+   dpdk-pmdinfo [-hrtp] [-d 
+
+   -h, --helpShow a short help message and exit
+   -r, --raw Dump as raw json strings
+   -d FILE, --pcidb=FILE Specify a pci database to get vendor names from
+   -t, --table   Output information on hw support as a hex table
+   -p, --plugindir   Scan dpdk for autoload plugins
+
+.. Note::
+
+   * Parameters inside the square brackets represents optional parameters.
-- 
2.7.4



[dpdk-dev] [PATCH v2 2/4] doc: rendering and installation of man pages

2016-08-31 Thread Christian Ehrhardt
This enables the rendering of rst into man pages as well as installing
them (if built) along the binaries. To do so there is a new make target
"doc-guides-man" which will render the rst files into man format.

Currently these three tools had docs that were compatible "enough" to
make up for a reasonable manpage.
- testpmd
- dpdk-pdump
- dpdk-procinfo

Since a man page should be installed along the binary they are not
installed in install-doc but install-runtime insteade. If not explicitly
built by the "doc-guides-man" target before calling install-runtime
there is no change to the old behaviour.

Signed-off-by: Christian Ehrhardt 
---
 doc/guides/conf.py   | 8 
 mk/rte.sdkdoc.mk | 2 +-
 mk/rte.sdkinstall.mk | 6 ++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/doc/guides/conf.py b/doc/guides/conf.py
index cd6a4f7..55b6b2f 100644
--- a/doc/guides/conf.py
+++ b/doc/guides/conf.py
@@ -105,6 +105,14 @@ class CustomLatexFormatter(LatexFormatter):
 # Replace the default latex formatter.
 PygmentsBridge.latex_formatter = CustomLatexFormatter

+# Configuration for man pages
+man_pages = [("testpmd_app_ug/run_app", "testpmd",
+  "tests for dpdk pmds", "", 1),
+ ("tools/pdump", "dpdk-pdump",
+  "enable packet capture on dpdk ports", "", 1),
+ ("tools/proc_info", "dpdk-procinfo",
+  "access dpdk port stats and memory info", "", 1)]
+
  :numref: fallback 
 # The following hook functions add some simple handling for the :numref:
 # directive for Sphinx versions prior to 1.3.1. The functions replace the
diff --git a/mk/rte.sdkdoc.mk b/mk/rte.sdkdoc.mk
index 9952f25..21d9bdf 100644
--- a/mk/rte.sdkdoc.mk
+++ b/mk/rte.sdkdoc.mk
@@ -63,7 +63,7 @@ help:
 all: api-html guides-html guides-pdf

 .PHONY: clean
-clean: api-html-clean guides-html-clean guides-pdf-clean
+clean: api-html-clean guides-html-clean guides-pdf-clean guides-man-clean

 .PHONY: api-html
 api-html: api-html-clean
diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk
index 5217063..533d369 100644
--- a/mk/rte.sdkinstall.mk
+++ b/mk/rte.sdkinstall.mk
@@ -66,6 +66,7 @@ includedir  ?=  $(prefix)/include/dpdk
 datarootdir ?=  $(prefix)/share
 docdir  ?=   $(datarootdir)/doc/dpdk
 datadir ?=   $(datarootdir)/dpdk
+mandir  ?=   $(datarootdir)/man
 sdkdir  ?=$(datadir)
 targetdir   ?=$(datadir)/$(RTE_TARGET)

@@ -133,6 +134,11 @@ install-runtime:
   $(DESTDIR)$(sbindir)/dpdk-devbind)
$(Q)$(call rte_symlink,$(DESTDIR)$(datadir)/tools/dpdk-pmdinfo.py, \
   $(DESTDIR)$(bindir)/dpdk-pmdinfo)
+ifneq ($(wildcard $O/doc/man/*/*.1),)
+   $(Q)$(call rte_mkdir, $(DESTDIR)$(mandir))
+   $(Q)$(call rte_mkdir, $(DESTDIR)$(mandir)/man1)
+   $(Q)cp -a $O/doc/man/*/*.1 $(DESTDIR)$(mandir)/man1
+endif

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



[dpdk-dev] [PATCH v2 1/4] doc: move tool guides in their own subdirectory

2016-08-31 Thread Christian Ehrhardt
This is to clarify the scope of these documents that are more tools than
sample applications.
Also this is a preparation step to add more tools and generate man pages
off of their rst files.

Signed-off-by: Christian Ehrhardt 
---
 doc/guides/index.rst   |   1 +
 doc/guides/sample_app_ug/index.rst |   2 -
 doc/guides/sample_app_ug/pdump.rst | 144 -
 doc/guides/sample_app_ug/proc_info.rst |  71 
 doc/guides/tools/index.rst |  40 +
 doc/guides/tools/pdump.rst | 144 +
 doc/guides/tools/proc_info.rst |  71 
 7 files changed, 256 insertions(+), 217 deletions(-)
 delete mode 100644 doc/guides/sample_app_ug/pdump.rst
 delete mode 100644 doc/guides/sample_app_ug/proc_info.rst
 create mode 100644 doc/guides/tools/index.rst
 create mode 100644 doc/guides/tools/pdump.rst
 create mode 100644 doc/guides/tools/proc_info.rst

diff --git a/doc/guides/index.rst b/doc/guides/index.rst
index 0441859..57570f6 100644
--- a/doc/guides/index.rst
+++ b/doc/guides/index.rst
@@ -41,6 +41,7 @@ DPDK documentation
nics/index
cryptodevs/index
sample_app_ug/index
+   tools/index
testpmd_app_ug/index
faq/index
howto/index
diff --git a/doc/guides/sample_app_ug/index.rst 
b/doc/guides/sample_app_ug/index.rst
index 96bb317..6573452 100644
--- a/doc/guides/sample_app_ug/index.rst
+++ b/doc/guides/sample_app_ug/index.rst
@@ -72,11 +72,9 @@ Sample Applications User Guide
 dist_app
 vm_power_management
 tep_termination
-proc_info
 ptpclient
 performance_thread
 ipsec_secgw
-pdump

 **Figures**

diff --git a/doc/guides/sample_app_ug/pdump.rst 
b/doc/guides/sample_app_ug/pdump.rst
deleted file mode 100644
index ac0e7c9..000
--- a/doc/guides/sample_app_ug/pdump.rst
+++ /dev/null
@@ -1,144 +0,0 @@
-
-..  BSD LICENSE
-Copyright(c) 2016 Intel Corporation. All rights reserved.
-All rights reserved.
-
-Redistribution and use in source and binary forms, with or without
-modification, are permitted provided that the following conditions
-are met:
-
-* Redistributions of source code must retain the above copyright
-notice, this list of conditions and the following disclaimer.
-* Redistributions in binary form must reproduce the above copyright
-notice, this list of conditions and the following disclaimer in
-the documentation and/or other materials provided with the
-distribution.
-* Neither the name of Intel Corporation nor the names of its
-contributors may be used to endorse or promote products derived
-from this software without specific prior written permission.
-
-THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-
-dpdk-pdump Application
-==
-
-The ``dpdk-pdump`` tool is a Data Plane Development Kit (DPDK) tool that runs 
as
-a DPDK secondary process and is capable of enabling packet capture on dpdk 
ports.
-
-   .. Note::
-
-  * The ``dpdk-pdump`` tool depends on libpcap based PMD which is disabled
-by default in the build configuration files,
-owing to an external dependency on the libpcap development files
-which must be installed on the board.
-Once the libpcap development files are installed, the libpcap based PMD
-can be enabled by setting CONFIG_RTE_LIBRTE_PMD_PCAP=y and recompiling 
the DPDK.
-
-
-Running the Application

-
-The tool has a number of command line options:
-
-.. code-block:: console
-
-   ./build/app/dpdk-pdump --
-  --pdump '(port= | device_id=),
-   (queue=),
-   (rx-dev= |
-tx-dev=),
-   [ring-size=],
-   [mbuf-size=],
-   [total-num-mbufs=]'
-  [--server-socket-path=]
-  [--client-socket-path=]
-
-The ``--pdump`` command line option is mandatory and it takes various sub 
arguments which are described in
-below section.
-
-

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

2016-08-31 Thread Christian Ehrhardt
*Updates in v2*
- rebased to latest upstream
- moved tools to their own guide section
- fixed some wording and indents
- properly marked fixed-width text elements
- fixed some rst issues in devbind doc

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".

Still it is a try to stick to the rst/sphinx based doc creation.
I found that for three of the 5 binaries that are usually installed the current
rst files are sufficient to make a meaningful man page:
- testpmd
- dpdk-pdump
- dpd-procinfo

To be clear, this is only meant for the binaries installed by DPDK,
there is no reason to render all the guides and howto's as one huge manpage.
Also this series doesn't strive to render the api doc as man pages,
while this certainly might be possible and even reasonable for section
"3   Library calls (functions within program libraries)".

Finally I must beg a pardon - I'm no makefile magician and sometimes even prefer
things that work compared to long cryptic lines with many special chars.
Yet if someone has something reasonable to unify the copy in patch #4
please let me know.

Christian Ehrhardt (4):
  doc: move tool guides in their own subdirectory
  doc: rendering and installation of man pages
  doc: add basic invocation info for dpdk-pmdinfo
  doc: add basic invocation info for dpdk-devbind

 doc/guides/conf.py |  12 +++
 doc/guides/index.rst   |   1 +
 doc/guides/sample_app_ug/index.rst |   2 -
 doc/guides/sample_app_ug/pdump.rst | 144 -
 doc/guides/sample_app_ug/proc_info.rst |  71 
 doc/guides/tools/devbind.rst   | 143 
 doc/guides/tools/index.rst |  42 ++
 doc/guides/tools/pdump.rst | 144 +
 doc/guides/tools/pmdinfo.rst   |  57 +
 doc/guides/tools/proc_info.rst |  71 
 mk/rte.sdkdoc.mk   |   2 +-
 mk/rte.sdkinstall.mk   |  11 +++
 12 files changed, 482 insertions(+), 218 deletions(-)
 delete mode 100644 doc/guides/sample_app_ug/pdump.rst
 delete mode 100644 doc/guides/sample_app_ug/proc_info.rst
 create mode 100644 doc/guides/tools/devbind.rst
 create mode 100644 doc/guides/tools/index.rst
 create mode 100644 doc/guides/tools/pdump.rst
 create mode 100644 doc/guides/tools/pmdinfo.rst
 create mode 100644 doc/guides/tools/proc_info.rst

-- 
2.7.4



[dpdk-dev] [PATCH 3/4] doc: add basic invocation info for dpdk-devbind

2016-08-31 Thread Christian Ehrhardt
Hi,
thanks for sharing your RST experience - I didn't see the rst issues before.
Fixed them all and checked html/man output again - looking good to me.

On Tue, Aug 30, 2016 at 5:05 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 3/4] doc: add basic invocation info for dpdk-
> > devbind
> >
> > +
> > +OPTIONS
> > +---
> > +
> > +* ``--help, --usage``
> > +
> > +Display usage information and quit
> > +
> > +* ``-s, --status``
> > +
> > +Print the current status of all known network interfaces.
> > +For each device, it displays the PCI domain, bus, slot and
> > function,
> > +along with a text description of the device. Depending upon
> > whether the
> > +device is being used by a kernel driver, the igb_uio driver, or
> > no
> > +driver, other relevant information will be displayed:
> > +* the Linux interface name e.g. if=eth0
> > +* the driver being used e.g. drv=igb_uio
> > +* any suitable drivers not currently using that device
> > +e.g. unused=igb_uio
> > +NOTE: if this flag is passed along with a bind/unbind option,
> the
> > +status display will always occur after the other operations have
> > taken
> > +place.
>
> There are a few RST errors in this file. One of theme relates to the second
> level bullet list above. There should be a blank line before and after the
> list.
>
> Also, "e.g. unused=igb_uio" should be joined to, or aligned with, the
> previous
> line.
>
> Also, it would be better to quote any fixed width strings in the docs with
> 
> quotes, like ``unused=igb_uio``. This could be applied to any of the
> ``--options``
> in the text as well.
> > +Examples
> > +
> > +
> > +To display current device status:
> > +.. code-block:: console
> > +
> > +   dpdk-devbind --status
> > +
>
> All the "code-block" directives should have a blank line before them.
> However it is probably better to use the simpler :: directive, like:
>
> To display current device status::
>
> dpdk-devbind --status
>
>


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] [PATCH 2/4] doc: add basic invocation info for dpdk-pmdinfo

2016-08-31 Thread Christian Ehrhardt
On Tue, Aug 30, 2016 at 4:59 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 2/4] doc: add basic invocation info for dpdk-
> > pmdinfo
> >

[...]

> > --- a/doc/guides/sample_app_ug/index.rst
> > +++ b/doc/guides/sample_app_ug/index.rst
> > @@ -77,6 +77,7 @@ Sample Applications User Guide
>
> I think these docs would be better in a "doc/guides/tools" directory.
> That would be clearer in terms to the documentation structure and
> also in terms of their functionality
>

I agree that it represents the scope better then, but I was aligning myself
to the pdump and procinfo tools that are already there.
testpmd is a bit of an outlier, as that already has its own dir.
But I assume given the suggestion to move the new ones we would want to
move those other two as well then?
I thank that would serve consistency then - I'll prep a patch with that as
part of the series.


[...]

free-of-discussion ack to all other suggestions


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

2016-08-31 Thread Christian Ehrhardt
Thanks for the review and the feedback.
I'm trying to integrate the feedback and send a v2 today.

On Wed, Aug 31, 2016 at 7:54 AM, Panu Matilainen 
wrote:

> 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 -
>



-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] [PATCH 4/4] doc: make the devbind man page be part of section 8

2016-08-31 Thread Christian Ehrhardt
Damn, I'm payed by # of commits :-P
That can of course  be squashed with the actual devbind patch.
I already thought about it before and hearing another voice asking for it
is enough to convince me.

On Tue, Aug 30, 2016 at 5:12 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 4/4] doc: make the devbind man page be part of
> > section 8
> >
> > As a root only program in sbin it should belong to section 8
> > "8   System administration commands (usually only for root)"
> >
> > Signed-off-by: Christian Ehrhardt 
> > ---
> >  doc/guides/conf.py   | 2 +-
> >  mk/rte.sdkinstall.mk | 5 +
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/doc/guides/conf.py b/doc/guides/conf.py index
> > ad8e8b3..52e2acf 100644
> > --- a/doc/guides/conf.py
> > +++ b/doc/guides/conf.py
> > @@ -102,7 +102,7 @@ man_pages = [("testpmd_app_ug/run_app", "testpmd",
> >   ("sample_app_ug/pmdinfo", "dpdk-pmdinfo",
> >"dump a PMDs hardware support info", "", 1),
> >   ("sample_app_ug/devbind", "dpdk-devbind",
> > -      "check device status and bind/unbind them from drivers",
> > "", 1)]
> > +  "check device status and bind/unbind them from drivers",
> > + "", 8)]
>
>
> Could this be rolled into one of the earlier patches?
>
>
>


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] [PATCH 0/3] fix virtio_user issues

2016-08-29 Thread Christian Ehrhardt
On Fri, Aug 5, 2016 at 1:36 PM, Jianfeng Tan  wrote:

> Patch 1: fix issue when using virtio_user with OVS-DPDK.
> Patch 2: fix issue when using virtio_user with VPP.
> Patch 3: fix issue when failing to start virtio_user devices.
>
> Signed-off-by: Jianfeng Tan 
>
> Jianfeng Tan (3):
>   net/virtio_user: fix queue pair not enabled
>   net/virtio_user: fix wrong sequence of messages
>   net/virtio_user: fix dev not freed after init error
>
>
Hi,
I think those were post 16.07 - would they be reasonable for the stable
tree?


[dpdk-dev] [PATCH 0/3] Hash library fixes

2016-08-29 Thread Christian Ehrhardt
On Fri, Aug 26, 2016 at 11:30 PM, Pablo de Lara <
pablo.de.lara.guarch at intel.com> wrote:

> Pablo de Lara (3):
>   hash: fix ring size
>   hash: fix false zero signature key hit lookup
>   hash: check if slot is empty with key index
>

Thanks Pablo,
I'd suggest to include #1 and #2 for stable as well once discussed and
applied to master - opinions?.


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] [PATCH 4/4] doc: make the devbind man page be part of section 8

2016-08-04 Thread Christian Ehrhardt
As a root only program in sbin it should belong to section 8
"8   System administration commands (usually only for root)"

Signed-off-by: Christian Ehrhardt 
---
 doc/guides/conf.py   | 2 +-
 mk/rte.sdkinstall.mk | 5 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/doc/guides/conf.py b/doc/guides/conf.py
index ad8e8b3..52e2acf 100644
--- a/doc/guides/conf.py
+++ b/doc/guides/conf.py
@@ -102,7 +102,7 @@ man_pages = [("testpmd_app_ug/run_app", "testpmd",
  ("sample_app_ug/pmdinfo", "dpdk-pmdinfo",
   "dump a PMDs hardware support info", "", 1),
  ("sample_app_ug/devbind", "dpdk-devbind",
-  "check device status and bind/unbind them from drivers", "", 1)]
+  "check device status and bind/unbind them from drivers", "", 8)]

  :numref: fallback 
 # The following hook functions add some simple handling for the :numref:
diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk
index 533d369..b1faf28 100644
--- a/mk/rte.sdkinstall.mk
+++ b/mk/rte.sdkinstall.mk
@@ -139,6 +139,11 @@ ifneq ($(wildcard $O/doc/man/*/*.1),)
$(Q)$(call rte_mkdir, $(DESTDIR)$(mandir)/man1)
$(Q)cp -a $O/doc/man/*/*.1 $(DESTDIR)$(mandir)/man1
 endif
+ifneq ($(wildcard $O/doc/man/*/*.8),)
+   $(Q)$(call rte_mkdir, $(DESTDIR)$(mandir))
+   $(Q)$(call rte_mkdir, $(DESTDIR)$(mandir)/man8)
+   $(Q)cp -a $O/doc/man/*/*.8 $(DESTDIR)$(mandir)/man8
+endif

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



[dpdk-dev] [PATCH 3/4] doc: add basic invocation info for dpdk-devbind

2016-08-04 Thread Christian Ehrhardt
This summarizes the "how to call dpdk-pmdinfo" in one place to be
picked up by html/pdf/man-page docs.

That knowledge was available before but spread in various docs along
examples (which are great and have to be kept) as well as in the
--usage/--help option of the tool itself.

Signed-off-by: Christian Ehrhardt 
---
 doc/guides/conf.py   |   4 +-
 doc/guides/sample_app_ug/devbind.rst | 150 +++
 doc/guides/sample_app_ug/index.rst   |   1 +
 3 files changed, 154 insertions(+), 1 deletion(-)
 create mode 100644 doc/guides/sample_app_ug/devbind.rst

diff --git a/doc/guides/conf.py b/doc/guides/conf.py
index 48fe890..ad8e8b3 100644
--- a/doc/guides/conf.py
+++ b/doc/guides/conf.py
@@ -100,7 +100,9 @@ man_pages = [("testpmd_app_ug/run_app", "testpmd",
  ("sample_app_ug/proc_info", "dpdk-procinfo",
   "access dpdk port stats and memory info", "", 1),
  ("sample_app_ug/pmdinfo", "dpdk-pmdinfo",
-  "dump a PMDs hardware support info", "", 1)]
+  "dump a PMDs hardware support info", "", 1),
+ ("sample_app_ug/devbind", "dpdk-devbind",
+  "check device status and bind/unbind them from drivers", "", 1)]

  :numref: fallback 
 # The following hook functions add some simple handling for the :numref:
diff --git a/doc/guides/sample_app_ug/devbind.rst 
b/doc/guides/sample_app_ug/devbind.rst
new file mode 100644
index 000..297e1b7
--- /dev/null
+++ b/doc/guides/sample_app_ug/devbind.rst
@@ -0,0 +1,150 @@
+
+..  BSD LICENSE
+Copyright(c) 2016 Canonical 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.
+
+
+dpdk-devbind Application
+
+
+The ``dpdk-devbind`` tool is a Data Plane Development Kit (DPDK) tool that 
helps binding and unbinding devices from specific drivers.
+As well as checking their status in that regard.
+
+
+Running the Application
+---
+
+The tool has a number of command line options:
+
+.. code-block:: console
+
+   dpdk-devbind [options] DEVICE1 DEVICE2 
+
+OPTIONS
+---
+
+* ``--help, --usage``
+
+Display usage information and quit
+
+* ``-s, --status``
+
+Print the current status of all known network interfaces.
+For each device, it displays the PCI domain, bus, slot and function,
+along with a text description of the device. Depending upon whether the
+device is being used by a kernel driver, the igb_uio driver, or no
+driver, other relevant information will be displayed:
+* the Linux interface name e.g. if=eth0
+* the driver being used e.g. drv=igb_uio
+* any suitable drivers not currently using that device
+e.g. unused=igb_uio
+NOTE: if this flag is passed along with a bind/unbind option, the
+status display will always occur after the other operations have taken
+place.
+
+* ``-b driver, --bind=driver``
+
+Select the driver to use or "none" to unbind the device
+
+* ``-u, --unbind``
+
+Unbind a device (Equivalent to "-b none")
+
+* ``--force``
+
+By default, devices which are used by Linux - as indicated by having
+routes in the routing table - cannot be mo

[dpdk-dev] [PATCH 2/4] doc: add basic invocation info for dpdk-pmdinfo

2016-08-04 Thread Christian Ehrhardt
This summarizes the "how to call dpdk-pmdinfo" in one place to be picked
up by html/pdf/man-page docs.

Signed-off-by: Christian Ehrhardt 
---
 doc/guides/conf.py   |  4 ++-
 doc/guides/sample_app_ug/index.rst   |  1 +
 doc/guides/sample_app_ug/pmdinfo.rst | 62 
 3 files changed, 66 insertions(+), 1 deletion(-)
 create mode 100644 doc/guides/sample_app_ug/pmdinfo.rst

diff --git a/doc/guides/conf.py b/doc/guides/conf.py
index 4435974..48fe890 100644
--- a/doc/guides/conf.py
+++ b/doc/guides/conf.py
@@ -98,7 +98,9 @@ man_pages = [("testpmd_app_ug/run_app", "testpmd",
  ("sample_app_ug/pdump", "dpdk-pdump",
   "enable packet capture on dpdk ports", "", 1),
  ("sample_app_ug/proc_info", "dpdk-procinfo",
-  "access dpdk port stats and memory info", "", 1)]
+  "access dpdk port stats and memory info", "", 1),
+ ("sample_app_ug/pmdinfo", "dpdk-pmdinfo",
+  "dump a PMDs hardware support info", "", 1)]

  :numref: fallback 
 # The following hook functions add some simple handling for the :numref:
diff --git a/doc/guides/sample_app_ug/index.rst 
b/doc/guides/sample_app_ug/index.rst
index 96bb317..7801688 100644
--- a/doc/guides/sample_app_ug/index.rst
+++ b/doc/guides/sample_app_ug/index.rst
@@ -77,6 +77,7 @@ Sample Applications User Guide
 performance_thread
 ipsec_secgw
 pdump
+pmdinfo

 **Figures**

diff --git a/doc/guides/sample_app_ug/pmdinfo.rst 
b/doc/guides/sample_app_ug/pmdinfo.rst
new file mode 100644
index 000..6bbf7e2
--- /dev/null
+++ b/doc/guides/sample_app_ug/pmdinfo.rst
@@ -0,0 +1,62 @@
+
+..  BSD LICENSE
+Copyright(c) 2016 Canonical 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.
+
+
+dpdk-pmdinfo Application
+
+
+The ``dpdk-pmdinfo`` tool is a Data Plane Development Kit (DPDK) tool that can
+dump a PMDs hardware support info.
+
+   .. Note::
+
+  * The actual data is stored in the object files as PMD_INFO_STRING
+
+
+Running the Application
+---
+
+The tool has a number of command line options:
+
+.. code-block:: console
+
+
+   dpdk-pmdinfo [-hrtp] [-d 
+
+   -h, --helpshow a short help message and exit
+   -r, --raw Dump as raw json strings
+   -d FILE, --pcidb=FILE
+ specify a pci database to get vendor names from
+   -t, --table   output information on hw support as a hex table
+   -p, --plugindir   scan dpdk for autoload plugins
+
+.. Note::
+
+   * Parameters inside the square brackets represents optional parameters.
-- 
2.7.4



[dpdk-dev] [PATCH 1/4] doc: rendering and installation of man pages

2016-08-04 Thread Christian Ehrhardt
This enables the rendering of rst into man pages as well as installing
them (if built) along the binaries. To do so there is a new make target
"doc-guides-man" which will render the rst files into man format.

Currently these three tools had docs that were compatible "enough" to
make up for a reasonable manpage.
- testpmd
- dpdk-pdump
- dpdk-procinfo

Since a man page should be installed along the binary they are not
installed in install-doc but install-runtime insteade. If not explicitly
built by the "doc-guides-man" target before calling install-runtime there no
change to the old behaviour.

Signed-off-by: Christian Ehrhardt 
---
 doc/guides/conf.py   | 8 
 mk/rte.sdkdoc.mk | 2 +-
 mk/rte.sdkinstall.mk | 6 ++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/doc/guides/conf.py b/doc/guides/conf.py
index 2c5610f..4435974 100644
--- a/doc/guides/conf.py
+++ b/doc/guides/conf.py
@@ -92,6 +92,14 @@ class CustomLatexFormatter(LatexFormatter):
 # Replace the default latex formatter.
 PygmentsBridge.latex_formatter = CustomLatexFormatter

+# Configuration for man pages
+man_pages = [("testpmd_app_ug/run_app", "testpmd",
+  "tests for dpdk pmds", "", 1),
+ ("sample_app_ug/pdump", "dpdk-pdump",
+  "enable packet capture on dpdk ports", "", 1),
+ ("sample_app_ug/proc_info", "dpdk-procinfo",
+  "access dpdk port stats and memory info", "", 1)]
+
  :numref: fallback 
 # The following hook functions add some simple handling for the :numref:
 # directive for Sphinx versions prior to 1.3.1. The functions replace the
diff --git a/mk/rte.sdkdoc.mk b/mk/rte.sdkdoc.mk
index 9952f25..21d9bdf 100644
--- a/mk/rte.sdkdoc.mk
+++ b/mk/rte.sdkdoc.mk
@@ -63,7 +63,7 @@ help:
 all: api-html guides-html guides-pdf

 .PHONY: clean
-clean: api-html-clean guides-html-clean guides-pdf-clean
+clean: api-html-clean guides-html-clean guides-pdf-clean guides-man-clean

 .PHONY: api-html
 api-html: api-html-clean
diff --git a/mk/rte.sdkinstall.mk b/mk/rte.sdkinstall.mk
index 5217063..533d369 100644
--- a/mk/rte.sdkinstall.mk
+++ b/mk/rte.sdkinstall.mk
@@ -66,6 +66,7 @@ includedir  ?=  $(prefix)/include/dpdk
 datarootdir ?=  $(prefix)/share
 docdir  ?=   $(datarootdir)/doc/dpdk
 datadir ?=   $(datarootdir)/dpdk
+mandir  ?=   $(datarootdir)/man
 sdkdir  ?=$(datadir)
 targetdir   ?=$(datadir)/$(RTE_TARGET)

@@ -133,6 +134,11 @@ install-runtime:
   $(DESTDIR)$(sbindir)/dpdk-devbind)
$(Q)$(call rte_symlink,$(DESTDIR)$(datadir)/tools/dpdk-pmdinfo.py, \
   $(DESTDIR)$(bindir)/dpdk-pmdinfo)
+ifneq ($(wildcard $O/doc/man/*/*.1),)
+   $(Q)$(call rte_mkdir, $(DESTDIR)$(mandir))
+   $(Q)$(call rte_mkdir, $(DESTDIR)$(mandir)/man1)
+   $(Q)cp -a $O/doc/man/*/*.1 $(DESTDIR)$(mandir)/man1
+endif

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



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

2016-08-04 Thread Christian Ehrhardt
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".

Still it is a try to stick to the rst/sphinx based doc creation.
I found that for three of the 5 binaries that are usually installed the current
rst files are sufficient to make a meaningful man page:
- testpmd
- dpdk-pdump
- dpd-procinfo

To be clear, this is only meant for the binaries installed by DPDK,
there is no reason to render all the guides and howto's as one huge manpage.
Also this series doesn't strive to render the api doc as man pages,
while this certainly might be possible and even reasonable for section
"3   Library calls (functions within program libraries)".

Finally I must beg a pardon - I'm no makefile magician and sometimes even prefer
things that work compared to long cryptic lines with many special chars.
Yet if someone has something reasonable to unify the copy in patch #4
please let me know.

Christian Ehrhardt (4):
  doc: rendering and installation of man pages
  doc: add basic invocation info for dpdk-pmdinfo
  doc: add basic invocation info for dpdk-devbind
  doc: make the devbind man page be part of section 8

 doc/guides/conf.py   |  12 +++
 doc/guides/sample_app_ug/devbind.rst | 150 +++
 doc/guides/sample_app_ug/index.rst   |   2 +
 doc/guides/sample_app_ug/pmdinfo.rst |  62 +++
 mk/rte.sdkdoc.mk |   2 +-
 mk/rte.sdkinstall.mk |  11 +++
 6 files changed, 238 insertions(+), 1 deletion(-)
 create mode 100644 doc/guides/sample_app_ug/devbind.rst
 create mode 100644 doc/guides/sample_app_ug/pmdinfo.rst

-- 
2.7.4



[dpdk-dev] [PATCH v2] scripts: make load-devel-config not to appear as executable

2016-08-03 Thread Christian Ehrhardt
*Updates in v2*
- drop the #!/bin/echo now that it is no more executable

Quoting the first line of the script: "#! /bin/echo must be loaded with ."
Given that we should drop the .sh file ending as well as the executable
flag - both are not needed to source the file.

Signed-off-by: Christian Ehrhardt 
---
 MAINTAINERS  |  2 +-
 scripts/checkpatches.sh  |  2 +-
 scripts/load-devel-config| 12 
 scripts/load-devel-config.sh | 14 --
 scripts/test-build.sh|  4 ++--
 5 files changed, 16 insertions(+), 18 deletions(-)
 create mode 100644 scripts/load-devel-config
 delete mode 100755 scripts/load-devel-config.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index 6536c6b..f8b99ee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27,7 +27,7 @@ F: MAINTAINERS
 F: scripts/check-maintainers.sh
 F: scripts/check-git-log.sh
 F: scripts/checkpatches.sh
-F: scripts/load-devel-config.sh
+F: scripts/load-devel-config
 F: scripts/test-build.sh

 Stable Branches
diff --git a/scripts/checkpatches.sh b/scripts/checkpatches.sh
index 7111558..b596b4e 100755
--- a/scripts/checkpatches.sh
+++ b/scripts/checkpatches.sh
@@ -33,7 +33,7 @@
 # Load config options:
 # - DPDK_CHECKPATCH_PATH
 # - DPDK_CHECKPATCH_LINE_LENGTH
-. $(dirname $(readlink -e $0))/load-devel-config.sh
+. $(dirname $(readlink -e $0))/load-devel-config

 length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}

diff --git a/scripts/load-devel-config b/scripts/load-devel-config
new file mode 100644
index 000..4f43cb3
--- /dev/null
+++ b/scripts/load-devel-config
@@ -0,0 +1,12 @@
+# Load DPDK devel config and allow override
+# from system file
+test ! -r /etc/dpdk/devel.config ||
+. /etc/dpdk/devel.config
+# from user file
+test ! -r ~/.config/dpdk/devel.config ||
+. ~/.config/dpdk/devel.config
+# from local file
+test ! -r $(dirname $(readlink -m $0))/../.develconfig ||
+. $(dirname $(readlink -m $0))/../.develconfig
+
+# The config files must export variables in the shell style
diff --git a/scripts/load-devel-config.sh b/scripts/load-devel-config.sh
deleted file mode 100755
index 489f007..000
--- a/scripts/load-devel-config.sh
+++ /dev/null
@@ -1,14 +0,0 @@
-#! /bin/echo must be loaded with .
-
-# Load DPDK devel config and allow override
-# from system file
-test ! -r /etc/dpdk/devel.config ||
-. /etc/dpdk/devel.config
-# from user file
-test ! -r ~/.config/dpdk/devel.config ||
-. ~/.config/dpdk/devel.config
-# from local file
-test ! -r $(dirname $(readlink -m $0))/../.develconfig ||
-. $(dirname $(readlink -m $0))/../.develconfig
-
-# The config files must export variables in the shell style
diff --git a/scripts/test-build.sh b/scripts/test-build.sh
index d2cafc1..e8971fe 100755
--- a/scripts/test-build.sh
+++ b/scripts/test-build.sh
@@ -48,7 +48,7 @@ default_path=$PATH
 # - DPDK_NOTIFY (notify-send)
 # - LIBSSO_SNOW3G_PATH
 # - LIBSSO_KASUMI_PATH
-. $(dirname $(readlink -e $0))/load-devel-config.sh
+. $(dirname $(readlink -e $0))/load-devel-config

 print_usage () {
echo "usage: $(basename $0) [-h] [-jX] [-s] [config1 [config2] ...]]"
@@ -211,7 +211,7 @@ for conf in $configs ; do
# reload config with DPDK_TARGET set
DPDK_TARGET=$target
reset_env
-   . $(dirname $(readlink -e $0))/load-devel-config.sh
+   . $(dirname $(readlink -e $0))/load-devel-config

options=$(echo $conf | sed 's,[^~+]*,,')
dir=$conf
-- 
2.7.4



[dpdk-dev] [RFC] scripts: make load-devel-config not to appear as executable

2016-08-03 Thread Christian Ehrhardt
On Wed, Aug 3, 2016 at 3:19 PM, Thomas Monjalon 
wrote:

> > Do you want me to resubmit as-is but without RFC tag or would you accept
> it
> > as already submitted?
>
> I will apply it as is.


As just discussed on #dpdk now that it is no more executable I'd even drop
the #!/bin/echo.
That is to avoid the
https://lintian.debian.org/tags/unusual-interpreter.html warning that still
is emitted.

Will submit a v2 soon.


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] [PATCH] doc: fix old dpdk-nic-bind.py references

2016-08-03 Thread Christian Ehrhardt
On Fri, Jul 29, 2016 at 8:20 PM, Pablo de Lara <
pablo.de.lara.guarch at intel.com> wrote:

> dpdk-nic-bind.py script has been renamed to dpdk-devbind.py,
> but some references to the old script have remained.
> This commit completes the renaming.
>
> Fixes: a5d7a3f77ddc ("unify tools naming")
>
> Signed-off-by: Pablo de Lara 
>

I like the cleanups for consistency!

Acked-by: Christian Ehrhardt 


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] [RFC] scripts: make load-devel-config not to appear as executable

2016-08-03 Thread Christian Ehrhardt
On Wed, Aug 3, 2016 at 11:26 AM, Bruce Richardson <
bruce.richardson at intel.com> wrote:

> Definitely no objection on the file mode change.
>
> For the dropping of the .sh extension, I don't think it matters much.
> However,
> given that .sh files are generally scripts to be executed, I think
> dropping the
> extension will reduce confusion.
>
> Acked-by: Bruce Richardson 
>

Thank you Bruce

@Thomas - given we are 2:1 now, the patch would stay as-is.
Do you want me to resubmit as-is but without RFC tag or would you accept it
as already submitted?


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] [RFC] scripts: make load-devel-config not to appear as executable

2016-08-03 Thread Christian Ehrhardt
sorry, I accidentally dropped dev list in one of my replies, readding.

On Tue, Aug 2, 2016 at 11:29 PM, Thomas Monjalon 
wrote:

> > > > Given that we should drop the .sh file ending as well as the
> executable
> > > > flag - both are not needed to source the file.
> > >
> > > Hmmm, it is still a file containing some shell commands, right?
> > > So why removing the .sh extension?
> > >
> >
> > I wanted to discuss on #dpdk today, but everyone seemed busy today.
> > So I expected the discussion on file extension to come up on the patch
> > submission - which is fine and just as it should be.
> >
> > My reasoning was primarily to discourage people to think to call it.
>
> I think it is the contrary: the executable files for users have no
> extension.


I totally understand that for commands in the path, but that doesn't count
here.
Could we have anybodies opinion as a tie breaker so I can submit a v2
without RFC then?

P.S. I understand there was no objection on changing the file mode - which
might be quite unobvious in the diff?


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] [RFC] scripts: make load-devel-config not to appear as executable

2016-08-02 Thread Christian Ehrhardt
Quoting the first line of the script: "#! /bin/echo must be loaded with ."
Given that we should drop the .sh file ending as well as the executable
flag - both are not needed to source the file.

Signed-off-by: Christian Ehrhardt 
---
 MAINTAINERS  |  2 +-
 scripts/checkpatches.sh  |  2 +-
 scripts/load-devel-config| 14 ++
 scripts/load-devel-config.sh | 14 --
 scripts/test-build.sh|  4 ++--
 5 files changed, 18 insertions(+), 18 deletions(-)
 create mode 100644 scripts/load-devel-config
 delete mode 100755 scripts/load-devel-config.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index 6536c6b..f8b99ee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27,7 +27,7 @@ F: MAINTAINERS
 F: scripts/check-maintainers.sh
 F: scripts/check-git-log.sh
 F: scripts/checkpatches.sh
-F: scripts/load-devel-config.sh
+F: scripts/load-devel-config
 F: scripts/test-build.sh

 Stable Branches
diff --git a/scripts/checkpatches.sh b/scripts/checkpatches.sh
index 7111558..b596b4e 100755
--- a/scripts/checkpatches.sh
+++ b/scripts/checkpatches.sh
@@ -33,7 +33,7 @@
 # Load config options:
 # - DPDK_CHECKPATCH_PATH
 # - DPDK_CHECKPATCH_LINE_LENGTH
-. $(dirname $(readlink -e $0))/load-devel-config.sh
+. $(dirname $(readlink -e $0))/load-devel-config

 length=${DPDK_CHECKPATCH_LINE_LENGTH:-80}

diff --git a/scripts/load-devel-config b/scripts/load-devel-config
new file mode 100644
index 000..489f007
--- /dev/null
+++ b/scripts/load-devel-config
@@ -0,0 +1,14 @@
+#! /bin/echo must be loaded with .
+
+# Load DPDK devel config and allow override
+# from system file
+test ! -r /etc/dpdk/devel.config ||
+. /etc/dpdk/devel.config
+# from user file
+test ! -r ~/.config/dpdk/devel.config ||
+. ~/.config/dpdk/devel.config
+# from local file
+test ! -r $(dirname $(readlink -m $0))/../.develconfig ||
+. $(dirname $(readlink -m $0))/../.develconfig
+
+# The config files must export variables in the shell style
diff --git a/scripts/load-devel-config.sh b/scripts/load-devel-config.sh
deleted file mode 100755
index 489f007..000
--- a/scripts/load-devel-config.sh
+++ /dev/null
@@ -1,14 +0,0 @@
-#! /bin/echo must be loaded with .
-
-# Load DPDK devel config and allow override
-# from system file
-test ! -r /etc/dpdk/devel.config ||
-. /etc/dpdk/devel.config
-# from user file
-test ! -r ~/.config/dpdk/devel.config ||
-. ~/.config/dpdk/devel.config
-# from local file
-test ! -r $(dirname $(readlink -m $0))/../.develconfig ||
-. $(dirname $(readlink -m $0))/../.develconfig
-
-# The config files must export variables in the shell style
diff --git a/scripts/test-build.sh b/scripts/test-build.sh
index d2cafc1..e8971fe 100755
--- a/scripts/test-build.sh
+++ b/scripts/test-build.sh
@@ -48,7 +48,7 @@ default_path=$PATH
 # - DPDK_NOTIFY (notify-send)
 # - LIBSSO_SNOW3G_PATH
 # - LIBSSO_KASUMI_PATH
-. $(dirname $(readlink -e $0))/load-devel-config.sh
+. $(dirname $(readlink -e $0))/load-devel-config

 print_usage () {
echo "usage: $(basename $0) [-h] [-jX] [-s] [config1 [config2] ...]]"
@@ -211,7 +211,7 @@ for conf in $configs ; do
# reload config with DPDK_TARGET set
DPDK_TARGET=$target
reset_env
-   . $(dirname $(readlink -e $0))/load-devel-config.sh
+   . $(dirname $(readlink -e $0))/load-devel-config

options=$(echo $conf | sed 's,[^~+]*,,')
dir=$conf
-- 
2.7.4



[dpdk-dev] [PATCH v2] examples: fix unusual-interpreter

2016-08-02 Thread Christian Ehrhardt
*update in v2*
- use #!/usr/bin/env python as usually recommended and suggested in the
  discussion

Due to regular lintian checks in Debian packaging it surfaced that these
two scripts had a space in their #! statement which renders it to be
human, but not shell readable.

Fixes: 8673a3e8 ("examples/ip_pipeline: add config diagram generator")
Fixes: fa667b46 ("examples/ip_pipeline: add core mappings script")

Signed-off-by: Christian Ehrhardt 
---
 examples/ip_pipeline/config/diagram-generator.py| 2 +-
 examples/ip_pipeline/config/pipeline-to-core-mapping.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/examples/ip_pipeline/config/diagram-generator.py 
b/examples/ip_pipeline/config/diagram-generator.py
index f20cbcb..6b7170b 100755
--- a/examples/ip_pipeline/config/diagram-generator.py
+++ b/examples/ip_pipeline/config/diagram-generator.py
@@ -1,4 +1,4 @@
-#! /usr/bin/python2
+#!/usr/bin/env python

 #   BSD LICENSE
 #
diff --git a/examples/ip_pipeline/config/pipeline-to-core-mapping.py 
b/examples/ip_pipeline/config/pipeline-to-core-mapping.py
index 37b131c..c2050b8 100755
--- a/examples/ip_pipeline/config/pipeline-to-core-mapping.py
+++ b/examples/ip_pipeline/config/pipeline-to-core-mapping.py
@@ -1,4 +1,4 @@
-#! /usr/bin/python2
+#!/usr/bin/env python

 #   BSD LICENSE
 #
-- 
2.7.4



[dpdk-dev] [PATCH] examples: fix unusual-interpreter

2016-08-01 Thread Christian Ehrhardt
Hi Thomas,
I agree on both changes you suggested, but not being the scripts author I
wanted to change as few as possible.
Also thanks for taking it into consideration even if just for lintian :-)

If acceptable to you I'd ask to accept this as-is and consider the patch a
head-up for all script owners to change their headers.



Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Mon, Aug 1, 2016 at 2:50 PM, Thomas Monjalon 
wrote:

> 2016-08-01 14:28, Christian Ehrhardt:
> > Due to regular lintian checks in Debian packaging it surfaced that these
> > two scripts had a space in their #! statement which renders it to be
> > human, but not shell readable.
> [...]
> > -#! /usr/bin/python2
> > +#!/usr/bin/python2
>
> I think we can have a space in the shebang (it works with shells I know).
> But maybe lintian do not like it (and it is a sufficient reason to accept
> this trivial patch).
>
> However, a better fix would be to run something else than python2,
> like /usr/bin/env python.
>
> Some other python scripts in tools dir may be fixed.
>


[dpdk-dev] [PATCH] examples: fix unusual-interpreter

2016-08-01 Thread Christian Ehrhardt
Due to regular lintian checks in Debian packaging it surfaced that these
two scripts had a space in their #! statement which renders it to be
human, but not shell readable.

Fixes: 8673a3e8 ("examples/ip_pipeline: add config diagram generator")
Fixes: fa667b46 ("examples/ip_pipeline: add core mappings script")

Signed-off-by: Christian Ehrhardt 
---
 examples/ip_pipeline/config/diagram-generator.py| 2 +-
 examples/ip_pipeline/config/pipeline-to-core-mapping.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/examples/ip_pipeline/config/diagram-generator.py 
b/examples/ip_pipeline/config/diagram-generator.py
index f20cbcb..7b1f8d6 100755
--- a/examples/ip_pipeline/config/diagram-generator.py
+++ b/examples/ip_pipeline/config/diagram-generator.py
@@ -1,4 +1,4 @@
-#! /usr/bin/python2
+#!/usr/bin/python2

 #   BSD LICENSE
 #
diff --git a/examples/ip_pipeline/config/pipeline-to-core-mapping.py 
b/examples/ip_pipeline/config/pipeline-to-core-mapping.py
index 37b131c..5ffc632 100755
--- a/examples/ip_pipeline/config/pipeline-to-core-mapping.py
+++ b/examples/ip_pipeline/config/pipeline-to-core-mapping.py
@@ -1,4 +1,4 @@
-#! /usr/bin/python2
+#!/usr/bin/python2

 #   BSD LICENSE
 #
-- 
2.7.4



[dpdk-dev] [PATCH] ethtool: remove triple license information

2016-08-01 Thread Christian Ehrhardt
License information is already in LICENSE.GPL.
Remove two extra copies and change referred filename in the files.

Signed-off-by: Christian Ehrhardt 
---
 lib/librte_eal/linuxapp/kni/ethtool/igb/COPYING| 339 -
 .../linuxapp/kni/ethtool/igb/e1000_82575.c |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_82575.h |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_api.c   |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_api.h   |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_defines.h   |   2 +-
 lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_hw.h |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_i210.c  |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_i210.h  |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_mac.c   |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_mac.h   |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_manage.c|   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_manage.h|   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_mbx.c   |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_mbx.h   |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_nvm.c   |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_nvm.h   |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_osdep.h |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_phy.c   |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_phy.h   |   2 +-
 .../linuxapp/kni/ethtool/igb/e1000_regs.h  |   2 +-
 lib/librte_eal/linuxapp/kni/ethtool/igb/igb.h  |   2 +-
 .../linuxapp/kni/ethtool/igb/igb_debugfs.c |   2 +-
 .../linuxapp/kni/ethtool/igb/igb_ethtool.c |   2 +-
 .../linuxapp/kni/ethtool/igb/igb_hwmon.c   |   2 +-
 lib/librte_eal/linuxapp/kni/ethtool/igb/igb_main.c |   2 +-
 .../linuxapp/kni/ethtool/igb/igb_param.c   |   2 +-
 .../linuxapp/kni/ethtool/igb/igb_procfs.c  |   2 +-
 lib/librte_eal/linuxapp/kni/ethtool/igb/igb_ptp.c  |   2 +-
 .../linuxapp/kni/ethtool/igb/igb_regtest.h |   2 +-
 lib/librte_eal/linuxapp/kni/ethtool/igb/igb_vmdq.c |   2 +-
 lib/librte_eal/linuxapp/kni/ethtool/igb/igb_vmdq.h |   2 +-
 lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.c  |   2 +-
 lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.h  |   2 +-
 .../linuxapp/kni/ethtool/igb/kcompat_ethtool.c |   2 +-
 lib/librte_eal/linuxapp/kni/ethtool/ixgbe/COPYING  | 339 -
 lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe.h  |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_82598.c   |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_82598.h   |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c   |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_82599.h   |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_api.c |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_api.h |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_common.c  |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_common.h  |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_dcb.h |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_ethtool.c |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_fcoe.h|   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_main.c|   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_mbx.h |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_osdep.h   |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_phy.c |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_phy.h |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_sriov.h   |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_type.h|   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_x540.c|   2 +-
 .../linuxapp/kni/ethtool/ixgbe/ixgbe_x540.h|   2 +-
 .../linuxapp/kni/ethtool/ixgbe/kcompat.c   |   2 +-
 .../linuxapp/kni/ethtool/ixgbe/kcompat.h   |   2 +-
 59 files changed, 57 insertions(+), 735 deletions(-)
 delete mode 100644 lib/librte_eal/linuxapp/kni/ethtool/igb/COPYING
 delete mode 100644 lib/librte_eal/linuxapp/kni/ethtool/ixgbe/COPYING

diff --git a/lib/librte_eal/linuxapp/kni/ethtool/igb/COPYING 
b/lib/librte_eal/linuxapp/kni/ethtool/igb/COPYING
deleted file mode 100644
index 5f297e5..000
--- a/lib/librte_eal/linuxapp/kni/ethtool/igb/COPYING
+++ /dev/null
@@ -1,339 +0,0 @@
-
-"This software program is licensed subject to the GNU General Public License 
-(GPL). Version 2, June 1991, available at 
-<http://www.fsf.org/copyleft/gpl.html>"
-
-GNU General Public License 
-
-Version 2, June 1991
-
-Copyright (C) 1989, 1991 Free Software Foundation, Inc.  
-59 Temple Place - Suite 330, Boston, MA  02111-1307, USA
-
-Everyone is permitted to copy and distribute verbatim copies of this license
-document, but changing it is not allowed.
-
-Preamble
-
-The licenses for most software are designed to take away your freedom to 
-share and change it. By contrast, the GNU General Public License is intended
-to guarantee your freedom to share and change free software--to make sure 
-the software is free for all its users. Thi

[dpdk-dev] doc: announce renaming of ethdev library

2016-07-28 Thread Christian Ehrhardt
Hi Thomas,
just my two cents as Ubuntu DPDK maintainer (and part of the Debian Team
that does the same).
(Yeah I really could reuse it three times :-) )

It will be a bit of effort to adapt, but should be no rocket-science.
I like that eventually the namespace will be cleaner.

Just curious, do we already know by looking ahead if ethdev will get an ABI
bump anyway?
So will the transition be:
a) libethdev4 -> librte_ethdev5
b)libethdev4 -> librte_ethdev4
If it is b) would/should one provide a compat symlink then in your Opinion?

Anyway, for now I think it is fair to say:
Acked-by: Christian Ehrhardt 

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Wed, Jul 27, 2016 at 6:33 PM, Jan Viktorin 
wrote:

> On Tue, 26 Jul 2016 18:22:21 +0200
> Thomas Monjalon  wrote:
>
> > The right name of ethdev should be dpdk_netdev. However:
> > 1/ We are using rte_ prefix in the code and library names.
> > 2/ The API uses rte_ethdev
> > That's why 16.11 will just have the rte_ prefix prepended to
> > the library filename as every other libraries.
> >
> > Signed-off-by: Thomas Monjalon 
> >
> Acked-by: Jan Viktorin 
>


[dpdk-dev] doc: announce ivshmem support removal

2016-07-28 Thread Christian Ehrhardt
Hi Thomas,
just my two cents as Ubuntu DPDK maintainer (and part of the Debian Team
that does the same).
(It seems I can reuse that line for all posts about the deprecation notices
:-) )

While IVSHMEM was enabled (as it was the default) I never heard of any
users of what we provided so far.
But that is expected considering that not all qemu bits are landed either.
Since it will follow the process of "deprecation notice -> grace period ->
ABI bump" on removal, I think packaging and consuming applications should
be fine.

I'd agree to Hiroshi that, if really needed a clean re-implementation more
properly separated from EAL likely is the best way to go.

I think it is a good change to drop rather complex code in favor of
stabilizing the main paths:
Acked-by: Christian Ehrhardt 


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Wed, Jul 27, 2016 at 9:08 PM, Jan Viktorin 
wrote:

> On Wed, 20 Jul 2016 18:35:46 +0200
> Thomas Monjalon  wrote:
>
> > There was a prior call with an explanation of what needs to be done:
> >   http://dpdk.org/ml/archives/dev/2016-June/040844.html
> > - Qemu patch upstreamed
> > - IVSHMEM PCI device managed by a PCI driver
> > - No DPDK objects (ring/mempool) allocated by EAL
> >
> > As nobody seems interested, it is time to remove this code which
> > makes EAL improvements harder.
> >
> > Signed-off-by: Thomas Monjalon 
> > Acked-by: David Marchand 
> > Acked-by: Maxime Coquelin 
>
> Acked-by: Jan Viktorin 


[dpdk-dev] doc: deprecate vhost-cuse

2016-07-28 Thread Christian Ehrhardt
Hi Thomas,
just my two cents as Ubuntu DPDK maintainer (and part of the Debian Team
that does the same).

We never used vhost-cuse in any of our exampled, documentations or tests
(It seems we started "late enough").

So again I think it is a good change to drop rather unmaintained parts to
make the rest more stable:
Acked-by: Christian Ehrhardt 

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Wed, Jul 27, 2016 at 8:59 PM, Jan Viktorin 
wrote:

> On Fri, 15 Jul 2016 20:28:33 +0800
> Yuanhan Liu  wrote:
>
> > Vhost-cuse was invented before vhost-user exist. The both are actually
> > doing the same thing: a vhost-net implementation in user space. But they
> > are not exactly the same thing.
> >
> > Firstly, vhost-cuse is harder for use; no one seems to care it, either.
> > Furthermore, since v2.1, a large majority of development effort has gone
> > to vhost-user. For example, we extended the vhost-user spec to add the
> > multiple queue support. We also added the vhost-user live migration at
> > v16.04 and the latest one, vhost-user reconnect that allows vhost app
> > restart without restarting the guest. Both of them are very important
> > features for product usage and none of them works for vhost-cuse.
> >
> > You now see that the difference between vhost-user and vhost-cuse is
> > big (and will be bigger and bigger as time moves forward), that you
> > should never use vhost-cuse, that we should drop it completely.
> >
> > The remove would also result to a much cleaner code base, allowing us
> > to do all kinds of extending easier.
> >
> > So here to mark vhost-cuse as deprecated in this release and will be
> > removed in the next release (v16.11).
> >
> > Signed-off-by: Yuanhan Liu 
> > Acked-by: Ciara Loftus 
> > Acked-by: Thomas Monjalon 
> > Acked-by: Rich Lane 
>
> Acked-by: Jan Viktorin 
>


[dpdk-dev] [PATCH] doc: announce removal of Xen Dom0 support

2016-07-28 Thread Christian Ehrhardt
Hi Thomas,
just my two cents as Ubuntu DPDK maintainer (and part of the Debian Team
that does the same).

We never enabled CONFIG_RTE_LIBRTE_XEN_DOM0, we only did LIBRTE_PMD_XENVIRT
based on a user request we had.
That said we won't "take away" anything from anybody and so far had no
request to provide it.
The person that asked for the XEN_PMD was Thiago (set to CC as he might
have an opinion on that).

For me I think it is a good change to drop rather unmaintained parts to
make the rest more stable:
Acked-by: Christian Ehrhardt 

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Thu, Jul 28, 2016 at 10:20 AM, Thomas Monjalon  wrote:

> There were some efforts to fix Xen Dom0 support in 16.07:
> http://dpdk.org/ml/archives/dev/2016-July/043823.html
> But there is still at least one bug:
> http://dpdk.org/ml/archives/dev/2016-July/044207.html
> And nobody cares:
> http://dpdk.org/ml/archives/dev/2016-July/044376.html
>
> Signed-off-by: Thomas Monjalon 
> ---
>  doc/guides/rel_notes/deprecation.rst | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index f502f86..6b8e9af 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -41,3 +41,6 @@ Deprecation Notices
>  * The mempool functions for single/multi producer/consumer are deprecated
> and
>will be removed in 16.11.
>It is replaced by rte_mempool_generic_get/put functions.
> +
> +* The support for Xen Dom0 is broken and seems not to be used.
> +  It will be removed in 16.11.
> --
> 2.7.0
>
>


[dpdk-dev] [PATCH v3] mk: fix FreeBSD build

2016-07-19 Thread Christian Ehrhardt
Hi,
the order changed, but IMHO actually it improved.
Things are now at the place they were before, but with the overwriting
config value that came later - that is the best of both worlds.

Tested a few config runs pre/post patch and compared them sorted (equal)
and unsorted - now configs slotted in where they belong.
Thanks for updating Sergio

Acked-by: Christian Ehrhardt 

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Tue, Jul 19, 2016 at 3:40 PM, Sergio Gonzalez Monroy <
sergio.gonzalez.monroy at intel.com> wrote:

> The sed syntax of '0,/regexp/' is GNU specific and fails with
> non GNU sed in FreeBSD.
>
> To solve the issue we can use awk instead to remove duplicates.
>
> The awk script basically keeps the last config value, while
> maintaining order and comments from original config file.
>
> Fixes: b2063f104db7 ("mk: filter duplicate configuration entries")
>
> Signed-off-by: Sergio Gonzalez Monroy 
> ---
>  mk/rte.sdkconfig.mk | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
> index e93237f..5c94edf 100644
> --- a/mk/rte.sdkconfig.mk
> +++ b/mk/rte.sdkconfig.mk
> @@ -88,11 +88,13 @@ $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE |
> $(RTE_OUTPUT)
> $(CPP) -undef -P -x assembler-with-cpp \
> -ffreestanding \
> -o $(RTE_OUTPUT)/.config_tmp $(RTE_CONFIG_TEMPLATE) ; \
> -   for config in $$(grep -v "^#" $(RTE_OUTPUT)/.config_tmp |
> cut -d"=" -f1 | sort | uniq -d); do \
> -   while [ $$(grep "^$${config}="
> $(RTE_OUTPUT)/.config_tmp -c ) -gt 1 ]; do \
> -   sed -i "0,/^$${config}=/{//d}"
> $(RTE_OUTPUT)/.config_tmp; \
> -   done; \
> -   done; \
> +   config=$$(cat $(RTE_OUTPUT)/.config_tmp) ; \
> +   echo "$$config" | awk -F '=' 'BEGIN {i=1} \
> +   /^#/ {pos[i++]=$$0} \
> +   !/^#/ {if (!s[$$1]) {pos[i]=$$0; s[$$1]=i++} \
> +   else {pos[s[$$1]]=$$0}} END \
> +   {for (j=1; j<i; j++) print pos[j]}' \
> +   > $(RTE_OUTPUT)/.config_tmp ; \
> if ! cmp -s $(RTE_OUTPUT)/.config_tmp
> $(RTE_OUTPUT)/.config; then \
> cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config
> ; \
> cp $(RTE_OUTPUT)/.config_tmp
> $(RTE_OUTPUT)/.config.orig ; \
> --
> 2.4.11
>
>


[dpdk-dev] [PATCH v2] Memory leak when adding/removing vhost_user ports

2016-07-19 Thread Christian Ehrhardt
Hi,
thanks for evaluating.
I'll need a few days until I'm ready for OVS again and until OVS is ready
for DPDK 16.07.
Then I can run an adapted version of my old testcase to be sure.

In the worst cases it will be in 16.11 and I'll backport to 16.07 as
distributed.
I'll let you know then.


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Tue, Jul 12, 2016 at 2:08 PM, Yuanhan Liu 
wrote:

> On Wed, Jul 06, 2016 at 09:08:12PM +0800, Yuanhan Liu wrote:
> > On Wed, Jul 06, 2016 at 02:24:57PM +0200, Christian Ehrhardt wrote:
> > > Hi,
> > > while checking for dpdk 16.07 what backports are accepted in the
> meantime so I
> > > can drop them I found this particular discussion has been silently
> forgotten by
> > > all of us.
> >
> > Not really. As later I found that my patch was actually wrong (besides
> > the issue you already found). I will revisit this issue thoroughly when
> > get time, hopefully, next week.
>
> Here it is: vhost_destroy() will be invoked when:
>
> - QEMU quits gracefully
> - QEMU terminates unexpectedly
>
> Meaning, there should be no memory leak. I think we are fine here (I
> maybe wrong though, feeling a bit dizzy now :(
>
> --yliu
>


[dpdk-dev] [PATCH v2] mk: fix FreeBSD build

2016-07-19 Thread Christian Ehrhardt
Hi,
I haven't tested the new suggested way, just went into explaining what
formerly were the reasons.
But I'd strongly vote against reordering and dropping comments.

Sergio - v3 had still awk for some parts.
It doesn't have the "0,..." you mentioned.
Could you check if that is already using GNU-sed only syntax -
http://dpdk.org/dev/patchwork/patch/14592/ ?

If this would be ok - AND - if it creates the same .config as the current
code I'd think that is the way to go.


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Tue, Jul 19, 2016 at 12:32 PM, Ferruh Yigit 
wrote:

> On 7/18/2016 5:06 PM, Sergio Gonzalez Monroy wrote:
> > The sed syntax of '0,/regexp/' is GNU specific and fails with
> > non GNU sed in FreeBSD.
> >
> > To solve the issue we can use awk instead to remove duplicates.
> >
> > Fixes: b2063f104db7 ("mk: filter duplicate configuration entries")
> >
> > Signed-off-by: Sergio Gonzalez Monroy 
> > ---
> >
> > v2:
> > - Use temp var instead of temp file
> >
> >  mk/rte.sdkconfig.mk | 7 ++-
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
> > index e93237f..c2b6e13 100644
> > --- a/mk/rte.sdkconfig.mk
> > +++ b/mk/rte.sdkconfig.mk
> > @@ -88,11 +88,8 @@ $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE |
> $(RTE_OUTPUT)
> >   $(CPP) -undef -P -x assembler-with-cpp \
> >   -ffreestanding \
> >   -o $(RTE_OUTPUT)/.config_tmp $(RTE_CONFIG_TEMPLATE) ; \
> > - for config in $$(grep -v "^#" $(RTE_OUTPUT)/.config_tmp |
> cut -d"=" -f1 | sort | uniq -d); do \
> > - while [ $$(grep "^$${config}="
> $(RTE_OUTPUT)/.config_tmp -c ) -gt 1 ]; do \
> > - sed -i "0,/^$${config}=/{//d}"
> $(RTE_OUTPUT)/.config_tmp; \
> > - done; \
> > - done; \
> > + config=$$(grep -v "^#" $(RTE_OUTPUT)/.config_tmp) ; \
> > + echo "$$config" | awk -F'=' '{a[$$1]=$$0} END {for (i in
> a) print a[i]}' > $(RTE_OUTPUT)/.config_tmp ; \
> This is another nice awk command.
>
> A few comments about new command:
> - Removes all comments from final config
> - Spreads config option all over the file, logical grouping of options
> removed.
>
> When both happens at the same time, I have a concern that this may lead
> missing some config options when somebody wants to update local config
> file, but I am OK if everybody is OK.
>
>
> >   if ! cmp -s $(RTE_OUTPUT)/.config_tmp
> $(RTE_OUTPUT)/.config; then \
> >   cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config
> ; \
> >   cp $(RTE_OUTPUT)/.config_tmp
> $(RTE_OUTPUT)/.config.orig ; \
> >
>
>


[dpdk-dev] [PATCH] mk: fix FreeBSD build

2016-07-18 Thread Christian Ehrhardt
Hi Sergio,
you might have seen that I had a similar version with awk in v2 IIRC. I
also had the secondary tmp file just like you now.
So, since it is so close to my old submission I wont object :-)

Back then the discussion went for reduced build time dependencies and
avoiding a second temp file, which was ok for me - so sed was chosen.

I see that breaking on BSD causes us to rework it again, sorry that I was
unable to test there.

If you could come up with a Solution "sed + no-temp2 + noGNUspecifics" that
would be great and solve everybodies needs.
If not, it is a call up to the participants of the old discussion if not
working on BSD outweighs their old feedback (I guess so).

Most active in the discussion back then was Ferruh IIRC - setting to CC.



Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Mon, Jul 18, 2016 at 3:39 PM, Sergio Gonzalez Monroy <
sergio.gonzalez.monroy at intel.com> wrote:

> On 18/07/2016 14:25, Thomas Monjalon wrote:
>
>> 2016-07-18 14:11, Sergio Gonzalez Monroy:
>>
>>> The sed syntax of '0,/regexp/' is GNU specific and fails with
>>> non GNU sed in FreeBSD.
>>>
>>> To solve the issue we can use awk instead to remove duplicates.
>>>
>> Christian, an opinion please?
>>
>
> Sorry, forgot to CC him.
>
> Fixes: b2063f104db7 ("mk: filter duplicate configuration entries")
>>>
>>> Signed-off-by: Sergio Gonzalez Monroy 
>>>
>> [...]
>>
>>> -   for config in $$(grep -v "^#" $(RTE_OUTPUT)/.config_tmp
>>> | cut -d"=" -f1 | sort | uniq -d); do \
>>> -   while [ $$(grep "^$${config}="
>>> $(RTE_OUTPUT)/.config_tmp -c ) -gt 1 ]; do \
>>> -   sed -i "0,/^$${config}=/{//d}"
>>> $(RTE_OUTPUT)/.config_tmp; \
>>> -   done; \
>>> -   done; \
>>> +   grep -v "^#" $(RTE_OUTPUT)/.config_tmp | awk -F'='
>>> '{a[$$1]=$$0} END {for (i in a) print a[i]}' > $(RTE_OUTPUT)/.config_tmp2 ;
>>> \
>>> +   mv $(RTE_OUTPUT)/.config_tmp2 $(RTE_OUTPUT)/.config_tmp
>>> ; \
>>> +   rm -f $(RTE_OUTPUT)/.config_tmp2 ; \
>>>
>> You can avoid creating/deleting the file .config_tmp2 by using a variable:
>> config=$(grep -v '^#' $(RTE_OUTPUT)/.config_tmp)
>> echo "$config" | awk ... > $(RTE_OUTPUT)/.config_tmp
>>
>
> Sure.
>
> Sergio
>


[dpdk-dev] Compiling DPDK is not working on Red Hat 6.7

2016-07-12 Thread Christian Ehrhardt
Hi,
checking "man clock_gettime" I see: "Link with -lrt (only for glibc
versions before 2.17)."

RH 6.7 is at glibc 2.12, I haven't check the build, but you might easily
run the make with V=1 and see the call.
Check if it contains -lrt for the linking step.




Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Tue, Jul 12, 2016 at 12:02 PM, Raslan Darawsheh 
wrote:

> Hi,
>
> When trying to compile DPDK on Red Hat Enterprise Linux Server release 6.7
> (Santiago) it fails to compile.
>
> This is the compilation error that is being seen:
> LD test
> /download/dpdk/x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_timer.o):
> In function `get_tsc_freq':
> eal_timer.c:(.text+0x152): undefined reference to `clock_gettime'
> eal_timer.c:(.text+0x190): undefined reference to `clock_gettime'
> /download/dpdk/x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o):
> In function `rte_eal_alarm_set':
> eal_alarm.c:(.text+0x382): undefined reference to `clock_gettime'
> /download/dpdk/x86_64-native-linuxapp-gcc/lib/librte_eal.a(eal_alarm.o):
> In function `eal_alarm_callback':
> eal_alarm.c:(.text+0x5e2): undefined reference to `clock_gettime'
> collect2: ld returned 1 exit status
> make[5]: *** [test] Error 1
> make[4]: *** [test] Error 2
> make[3]: *** [app] Error 2
> make[2]: *** [all] Error 2
> make[1]: *** [pre_install] Error 2
> make: *** [install] Error 2
>
> Kindest regards
> Raslan Darawsheh
>


[dpdk-dev] [PATCH v2] librte_pmd_bond: fix exported symbol versioning

2016-07-11 Thread Christian Ehrhardt
*update in v2*
- add missing changes in rte_eth_bond_8023ad.h

The older versions of rte_eth_bond_8023ad_conf_get and
rte_eth_bond_8023ad_setup were available in the old way since 2.0 - at
least according to the map file.

But versioning in the code was set to 16.04.
That breaks compatibility checks for 2.0 on that library.

For example with the dpdk abi checker:
http://people.canonical.com/~paelzer/compat_report.html

To fix, version the old symbols on the 2.0 version as they were
initially added to the map file.

See http://people.canonical.com/~paelzer/compat_report.html

Fixes: dc40f17a ("net/bonding: allow external state machine in mode 4")

Signed-off-by: Christian Ehrhardt 
---
 drivers/net/bonding/rte_eth_bond_8023ad.c | 12 ++--
 drivers/net/bonding/rte_eth_bond_8023ad.h |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c 
b/drivers/net/bonding/rte_eth_bond_8023ad.c
index 48a50e4..2f7ae70 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -1068,7 +1068,7 @@ bond_mode_8023ad_conf_assign(struct mode8023ad_private 
*mode4,
 }

 static void
-bond_mode_8023ad_setup_v1604(struct rte_eth_dev *dev,
+bond_mode_8023ad_setup_v20(struct rte_eth_dev *dev,
struct rte_eth_bond_8023ad_conf *conf)
 {
struct rte_eth_bond_8023ad_conf def_conf;
@@ -1214,7 +1214,7 @@ free_out:
 }

 int
-rte_eth_bond_8023ad_conf_get_v1604(uint8_t port_id,
+rte_eth_bond_8023ad_conf_get_v20(uint8_t port_id,
struct rte_eth_bond_8023ad_conf *conf)
 {
struct rte_eth_dev *bond_dev;
@@ -1229,7 +1229,7 @@ rte_eth_bond_8023ad_conf_get_v1604(uint8_t port_id,
bond_mode_8023ad_conf_get(bond_dev, conf);
return 0;
 }
-VERSION_SYMBOL(rte_eth_bond_8023ad_conf_get, _v1604, 16.04);
+VERSION_SYMBOL(rte_eth_bond_8023ad_conf_get, _v20, 2.0);

 int
 rte_eth_bond_8023ad_conf_get_v1607(uint8_t port_id,
@@ -1278,7 +1278,7 @@ bond_8023ad_setup_validate(uint8_t port_id,
 }

 int
-rte_eth_bond_8023ad_setup_v1604(uint8_t port_id,
+rte_eth_bond_8023ad_setup_v20(uint8_t port_id,
struct rte_eth_bond_8023ad_conf *conf)
 {
struct rte_eth_dev *bond_dev;
@@ -1289,11 +1289,11 @@ rte_eth_bond_8023ad_setup_v1604(uint8_t port_id,
return err;

bond_dev = _eth_devices[port_id];
-   bond_mode_8023ad_setup_v1604(bond_dev, conf);
+   bond_mode_8023ad_setup_v20(bond_dev, conf);

return 0;
 }
-VERSION_SYMBOL(rte_eth_bond_8023ad_setup, _v1604, 16.04);
+VERSION_SYMBOL(rte_eth_bond_8023ad_setup, _v20, 2.0);

 int
 rte_eth_bond_8023ad_setup_v1607(uint8_t port_id,
diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.h 
b/drivers/net/bonding/rte_eth_bond_8023ad.h
index 1de34bc..6b8ff57 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.h
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.h
@@ -188,7 +188,7 @@ int
 rte_eth_bond_8023ad_conf_get(uint8_t port_id,
struct rte_eth_bond_8023ad_conf *conf);
 int
-rte_eth_bond_8023ad_conf_get_v1604(uint8_t port_id,
+rte_eth_bond_8023ad_conf_get_v20(uint8_t port_id,
struct rte_eth_bond_8023ad_conf *conf);
 int
 rte_eth_bond_8023ad_conf_get_v1607(uint8_t port_id,
@@ -209,7 +209,7 @@ int
 rte_eth_bond_8023ad_setup(uint8_t port_id,
struct rte_eth_bond_8023ad_conf *conf);
 int
-rte_eth_bond_8023ad_setup_v1604(uint8_t port_id,
+rte_eth_bond_8023ad_setup_v20(uint8_t port_id,
struct rte_eth_bond_8023ad_conf *conf);
 int
 rte_eth_bond_8023ad_setup_v1607(uint8_t port_id,
-- 
2.7.4



[dpdk-dev] [PATCH] doc: announce driver name changes

2016-07-11 Thread Christian Ehrhardt
Hi,
I'm all in for consistent naming and this "only" is the deprecation notice
ahead of time which is great.
Looking ahead I wanted to ask for opinions if more than just me would
consider it useful to keep aliases on the old names when the rename happens.

That way any sort of old config would continue to work.
Of course I could do that in the scope of the scripts that consume the
names and apply the driver loads.
But then that is true for everybody out there who has some kind of
config/tooling around the old names - so I thought it is worth to ask for
opinions.

Kind Regards,
Christian



Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Sat, Jul 9, 2016 at 6:56 PM, Pablo de Lara <
pablo.de.lara.guarch at intel.com> wrote:

> Driver names for all the supported devices in DPDK do not have
> a naming convention. Some are using a prefix, some are not
> and some have long names. Driver names are used when creating
> virtual devices, so it is useful to have consistency in the names.
>
> Signed-off-by: Pablo de Lara 
> ---
>  doc/guides/rel_notes/deprecation.rst | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index f502f86..37d65c8 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -41,3 +41,8 @@ Deprecation Notices
>  * The mempool functions for single/multi producer/consumer are deprecated
> and
>will be removed in 16.11.
>It is replaced by rte_mempool_generic_get/put functions.
> +
> +* Driver names are quite inconsistent among each others and they will be
> +  renamed to something more consistent (net_ prefix for net drivers and
> +  crypto_ for crypto drivers) in 16.11. Some of these driver names are
> used
> +  publicly, to create virtual devices, so a deprecation notice is
> necessary.
> --
> 2.7.4
>
>


[dpdk-dev] [PATCH v2] Memory leak when adding/removing vhost_user ports

2016-07-06 Thread Christian Ehrhardt
Now let us really get to this old discussion.
Once again - sorry for mixing this up :-/
Found too much at once today.

## Ok - refocus ##

Back then in
http://dpdk.org/dev/patchwork/patch/12103/
http://dpdk.org/dev/patchwork/patch/12118/

We came up with a nice solution for the leak.
But it was never picked up upstream.

There were a lot of changes to all of that code, especially vhost
client/server.
Lacking an openvswitch for DPDK 16.07 I can't test if the issue still shows
up, but looking at the code suggests it still does.

Unfortunately the internal structures and scopes are slightly different so
that I couldn't come up with a working forward port of the old patch.
Attached is a patch as close as I got (obviously not working).
I'm convinced that Yuanhan Liu and Xie Huawei with their deep context
knowledge of dpdk's vhost_user will quickly know:
- if the root cause still applies
- what the best new way of fixing this would be

As this makes long term usage of dpdk aborting by a leak I hope we have a
chance to get this into 16.07 still.

Kind Regards,
Christian


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Wed, Jul 6, 2016 at 2:30 PM, Christian Ehrhardt <
christian.ehrhardt at canonical.com> wrote:

> This is the series this really belongs to
> http://dpdk.org/dev/patchwork/patch/11581/
> Message ID <
> 1458292380-9258-1-git-send-email-patrik.r.andersson at ericsson.com>
>
> Should I wait for a v2 to point the patch at the right ID or do you prefer
> a fixed resubmit right away?
>
> Christian Ehrhardt
> Software Engineer, Ubuntu Server
> Canonical Ltd
>
> On Wed, Jul 6, 2016 at 2:26 PM, Christian Ehrhardt <
> christian.ehrhardt at canonical.com> wrote:
>
>> Sorry,
>> please ignore the two links, the cover letter has - they belong to a
>> different issue I have to bring up again.
>> Everything else still applies.
>>
>> Christian Ehrhardt
>> Software Engineer, Ubuntu Server
>> Canonical Ltd
>>
>> On Wed, Jul 6, 2016 at 2:24 PM, Christian Ehrhardt <
>> christian.ehrhardt at canonical.com> wrote:
>>
>>> Hi,
>>> while checking for dpdk 16.07 what backports are accepted in the
>>> meantime so I
>>> can drop them I found this particular discussion has been silently
>>> forgotten by
>>> all of us.
>>>
>>> Back then we had the patch and discussion first in
>>> http://dpdk.org/dev/patchwork/patch/12103/
>>> and then
>>> http://dpdk.org/dev/patchwork/patch/12118/
>>>
>>> Things worked fine as I reported and I integrated the patch in our
>>> packaging as
>>> it fixed a severe issue. Since it was reported by someone else I do not
>>> seem to
>>> be the only one :-)
>>>
>>> So today I rebased the patch including my updates I made based on our
>>> discussion
>>> and I think it would make as much sense as it made back then to fix this.
>>>
>>> Christian Ehrhardt (1):
>>>   vhost_user: avoid crash when exeeding file descriptors
>>>
>>>  lib/librte_vhost/vhost_user/fd_man.c | 11 ++-
>>>  lib/librte_vhost/vhost_user/vhost-net-user.c | 19 +--
>>>  2 files changed, 23 insertions(+), 7 deletions(-)
>>>
>>> --
>>> 2.7.4
>>>
>>>
>>
>


[dpdk-dev] [PATCH v2] Memory leak when adding/removing vhost_user ports

2016-07-06 Thread Christian Ehrhardt
This is the series this really belongs to
http://dpdk.org/dev/patchwork/patch/11581/
Message ID <1458292380-9258-1-git-send-email-patrik.r.andersson at ericsson.com
>

Should I wait for a v2 to point the patch at the right ID or do you prefer
a fixed resubmit right away?

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Wed, Jul 6, 2016 at 2:26 PM, Christian Ehrhardt <
christian.ehrhardt at canonical.com> wrote:

> Sorry,
> please ignore the two links, the cover letter has - they belong to a
> different issue I have to bring up again.
> Everything else still applies.
>
> Christian Ehrhardt
> Software Engineer, Ubuntu Server
> Canonical Ltd
>
> On Wed, Jul 6, 2016 at 2:24 PM, Christian Ehrhardt <
> christian.ehrhardt at canonical.com> wrote:
>
>> Hi,
>> while checking for dpdk 16.07 what backports are accepted in the meantime
>> so I
>> can drop them I found this particular discussion has been silently
>> forgotten by
>> all of us.
>>
>> Back then we had the patch and discussion first in
>> http://dpdk.org/dev/patchwork/patch/12103/
>> and then
>> http://dpdk.org/dev/patchwork/patch/12118/
>>
>> Things worked fine as I reported and I integrated the patch in our
>> packaging as
>> it fixed a severe issue. Since it was reported by someone else I do not
>> seem to
>> be the only one :-)
>>
>> So today I rebased the patch including my updates I made based on our
>> discussion
>> and I think it would make as much sense as it made back then to fix this.
>>
>> Christian Ehrhardt (1):
>>   vhost_user: avoid crash when exeeding file descriptors
>>
>>  lib/librte_vhost/vhost_user/fd_man.c | 11 ++-
>>  lib/librte_vhost/vhost_user/vhost-net-user.c | 19 +--
>>  2 files changed, 23 insertions(+), 7 deletions(-)
>>
>> --
>> 2.7.4
>>
>>
>


[dpdk-dev] [PATCH v2] Memory leak when adding/removing vhost_user ports

2016-07-06 Thread Christian Ehrhardt
Sorry,
please ignore the two links, the cover letter has - they belong to a
different issue I have to bring up again.
Everything else still applies.

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Wed, Jul 6, 2016 at 2:24 PM, Christian Ehrhardt <
christian.ehrhardt at canonical.com> wrote:

> Hi,
> while checking for dpdk 16.07 what backports are accepted in the meantime
> so I
> can drop them I found this particular discussion has been silently
> forgotten by
> all of us.
>
> Back then we had the patch and discussion first in
> http://dpdk.org/dev/patchwork/patch/12103/
> and then
> http://dpdk.org/dev/patchwork/patch/12118/
>
> Things worked fine as I reported and I integrated the patch in our
> packaging as
> it fixed a severe issue. Since it was reported by someone else I do not
> seem to
> be the only one :-)
>
> So today I rebased the patch including my updates I made based on our
> discussion
> and I think it would make as much sense as it made back then to fix this.
>
> Christian Ehrhardt (1):
>   vhost_user: avoid crash when exeeding file descriptors
>
>  lib/librte_vhost/vhost_user/fd_man.c | 11 ++-
>  lib/librte_vhost/vhost_user/vhost-net-user.c | 19 +--
>  2 files changed, 23 insertions(+), 7 deletions(-)
>
> --
> 2.7.4
>
>


[dpdk-dev] [PATCH v2] vhost_user: avoid crash when exeeding file descriptors

2016-07-06 Thread Christian Ehrhardt
*update in v2*
- refreshing for DPDK 16.07
- Close fd on vserver->listenfd as suggested in discussion

Original From:
From: Patrik Andersson <patrik.r.anders...@ericsson.com>

Protect against DPDK crash when allocation of listen fd >= 1023.
For events on fd:s >1023, the current implementation will trigger
an abort due to access outside of allocated bit mask.

Corrections would include:

  * Match fdset_add() signature in fd_man.c to fd_man.h
  * Handling of return codes from fdset_add()
  * Addition of check of fd number in fdset_add_fd()

The rationale behind the suggested code change is that,
fdset_event_dispatch() could attempt access outside of the FD_SET
bitmask if there is an event on a file descriptor that in turn
looks up a virtio file descriptor with a value > 1023.
Such an attempt will lead to an abort() and a restart of any
vswitch using DPDK.

A discussion topic exist in the ovs-discuss mailing list that can
provide a little more background:
http://openvswitch.org/pipermail/discuss/2016-February/020243.html

Fixes: 8f972312 ("vhost: support vhost-user")

Signed-off-by: Patrik Andersson 
Signed-off-by: Christian Ehrhardt 
---
 lib/librte_vhost/vhost_user/fd_man.c | 11 ++-
 lib/librte_vhost/vhost_user/vhost-net-user.c | 19 +--
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/lib/librte_vhost/vhost_user/fd_man.c 
b/lib/librte_vhost/vhost_user/fd_man.c
index 087aaed..c691339 100644
--- a/lib/librte_vhost/vhost_user/fd_man.c
+++ b/lib/librte_vhost/vhost_user/fd_man.c
@@ -71,20 +71,22 @@ fdset_find_free_slot(struct fdset *pfdset)
return fdset_find_fd(pfdset, -1);
 }

-static void
+static int
 fdset_add_fd(struct fdset  *pfdset, int idx, int fd,
fd_cb rcb, fd_cb wcb, void *dat)
 {
struct fdentry *pfdentry;

-   if (pfdset == NULL || idx >= MAX_FDS)
-   return;
+   if (pfdset == NULL || idx >= MAX_FDS || fd >= FD_SETSIZE)
+   return -1;

pfdentry = >fd[idx];
pfdentry->fd = fd;
pfdentry->rcb = rcb;
pfdentry->wcb = wcb;
pfdentry->dat = dat;
+
+   return 0;
 }

 /**
@@ -150,12 +152,11 @@ fdset_add(struct fdset *pfdset, int fd, fd_cb rcb, fd_cb 
wcb, void *dat)

/* Find a free slot in the list. */
i = fdset_find_free_slot(pfdset);
-   if (i == -1) {
+   if (i == -1 || fdset_add_fd(pfdset, i, fd, rcb, wcb, dat) < 0) {
pthread_mutex_unlock(>fd_mutex);
return -2;
}

-   fdset_add_fd(pfdset, i, fd, rcb, wcb, dat);
pfdset->num++;

pthread_mutex_unlock(>fd_mutex);
diff --git a/lib/librte_vhost/vhost_user/vhost-net-user.c 
b/lib/librte_vhost/vhost_user/vhost-net-user.c
index 94f1b92..5743f52 100644
--- a/lib/librte_vhost/vhost_user/vhost-net-user.c
+++ b/lib/librte_vhost/vhost_user/vhost-net-user.c
@@ -257,6 +257,7 @@ vhost_user_add_connection(int fd, struct vhost_user_socket 
*vsocket)
int vid;
size_t size;
struct vhost_user_connection *conn;
+   int ret;

conn = malloc(sizeof(*conn));
if (conn == NULL) {
@@ -278,7 +279,15 @@ vhost_user_add_connection(int fd, struct vhost_user_socket 
*vsocket)

conn->vsocket = vsocket;
conn->vid = vid;
-   fdset_add(_user.fdset, fd, vhost_user_msg_handler, NULL, conn);
+   ret = fdset_add(_user.fdset, fd, vhost_user_msg_handler,
+   NULL, conn);
+   if (ret < 0) {
+   free(conn);
+   close(fd);
+   RTE_LOG(ERR, VHOST_CONFIG,
+   "failed to add fd %d into vhost server fdset\n",
+   fd);
+   }
 }

 /* call back when there is new vhost-user connection from client  */
@@ -469,8 +478,14 @@ vhost_user_create_server(struct vhost_user_socket *vsocket)
goto err;

vsocket->listenfd = fd;
-   fdset_add(_user.fdset, fd, vhost_user_server_new_connection,
+   ret = fdset_add(_user.fdset, fd, vhost_user_server_new_connection,
  NULL, vsocket);
+   if (ret < 0) {
+   RTE_LOG(ERR, VHOST_CONFIG,
+   "failed to add listen fd %d to vhost server fdset\n",
+   fd);
+   goto err;
+   }

return 0;

-- 
2.7.4



[dpdk-dev] [PATCH v2] Memory leak when adding/removing vhost_user ports

2016-07-06 Thread Christian Ehrhardt
Hi,
while checking for dpdk 16.07 what backports are accepted in the meantime so I
can drop them I found this particular discussion has been silently forgotten by
all of us.

Back then we had the patch and discussion first in
http://dpdk.org/dev/patchwork/patch/12103/
and then
http://dpdk.org/dev/patchwork/patch/12118/

Things worked fine as I reported and I integrated the patch in our packaging as
it fixed a severe issue. Since it was reported by someone else I do not seem to
be the only one :-)

So today I rebased the patch including my updates I made based on our discussion
and I think it would make as much sense as it made back then to fix this.

Christian Ehrhardt (1):
  vhost_user: avoid crash when exeeding file descriptors

 lib/librte_vhost/vhost_user/fd_man.c | 11 ++-
 lib/librte_vhost/vhost_user/vhost-net-user.c | 19 +--
 2 files changed, 23 insertions(+), 7 deletions(-)

-- 
2.7.4



[dpdk-dev] [PATCH] librte_pmd_bond: fix exported symbol versioning

2016-07-06 Thread Christian Ehrhardt
The older versions of rte_eth_bond_8023ad_conf_get and
rte_eth_bond_8023ad_setup were available in the old way since 2.0 - at
least according to the map file.

But versioning in the code was set to 16.04.
That breaks compatibility checks for 2.0 on that library.

For example with the dpdk abi checker:
http://people.canonical.com/~paelzer/compat_report.html

To fix, version the old symbols on the 2.0 version as they were
initially added to the map file.

See http://people.canonical.com/~paelzer/compat_report.html

Fixes: dc40f17a ("net/bonding: allow external state machine in mode 4")

Signed-off-by: Christian Ehrhardt 
---
 drivers/net/bonding/rte_eth_bond_8023ad.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c 
b/drivers/net/bonding/rte_eth_bond_8023ad.c
index 48a50e4..2f7ae70 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -1068,7 +1068,7 @@ bond_mode_8023ad_conf_assign(struct mode8023ad_private 
*mode4,
 }

 static void
-bond_mode_8023ad_setup_v1604(struct rte_eth_dev *dev,
+bond_mode_8023ad_setup_v20(struct rte_eth_dev *dev,
struct rte_eth_bond_8023ad_conf *conf)
 {
struct rte_eth_bond_8023ad_conf def_conf;
@@ -1214,7 +1214,7 @@ free_out:
 }

 int
-rte_eth_bond_8023ad_conf_get_v1604(uint8_t port_id,
+rte_eth_bond_8023ad_conf_get_v20(uint8_t port_id,
struct rte_eth_bond_8023ad_conf *conf)
 {
struct rte_eth_dev *bond_dev;
@@ -1229,7 +1229,7 @@ rte_eth_bond_8023ad_conf_get_v1604(uint8_t port_id,
bond_mode_8023ad_conf_get(bond_dev, conf);
return 0;
 }
-VERSION_SYMBOL(rte_eth_bond_8023ad_conf_get, _v1604, 16.04);
+VERSION_SYMBOL(rte_eth_bond_8023ad_conf_get, _v20, 2.0);

 int
 rte_eth_bond_8023ad_conf_get_v1607(uint8_t port_id,
@@ -1278,7 +1278,7 @@ bond_8023ad_setup_validate(uint8_t port_id,
 }

 int
-rte_eth_bond_8023ad_setup_v1604(uint8_t port_id,
+rte_eth_bond_8023ad_setup_v20(uint8_t port_id,
struct rte_eth_bond_8023ad_conf *conf)
 {
struct rte_eth_dev *bond_dev;
@@ -1289,11 +1289,11 @@ rte_eth_bond_8023ad_setup_v1604(uint8_t port_id,
return err;

bond_dev = _eth_devices[port_id];
-   bond_mode_8023ad_setup_v1604(bond_dev, conf);
+   bond_mode_8023ad_setup_v20(bond_dev, conf);

return 0;
 }
-VERSION_SYMBOL(rte_eth_bond_8023ad_setup, _v1604, 16.04);
+VERSION_SYMBOL(rte_eth_bond_8023ad_setup, _v20, 2.0);

 int
 rte_eth_bond_8023ad_setup_v1607(uint8_t port_id,
-- 
2.7.4



[dpdk-dev] [PATCH v4] mk: filter duplicate configuration entries

2016-07-06 Thread Christian Ehrhardt
*updates in v4*
- replace awk usage with sed
- re-add the old loop to be able to get rid of awk
- add more explanation to the header of the makefile section

*updates in v3*
- replace tac with sed '1!G;h;$!d' to avoid build time dependency

*updates in v2*
- move to .config target
- fix usage order of tac
- simplify inner section by only using awk (instead of awk+loop+bash+sed)

Due to the hierarchy and the demand to keep the base config showing all
options, some config keys end up multiple times in the .config file.

Due to the way the actual config is sourced only the last entry is
important. That can confuse people changing values in .config which
are then ignored.

A suggested solution was to filter for duplicates at the end of the
actual config step which is implemented here.

Signed-off-by: Christian Ehrhardt 
---
 mk/rte.sdkconfig.mk | 9 +
 1 file changed, 9 insertions(+)

diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
index a3acfe6..70c59d6 100644
--- a/mk/rte.sdkconfig.mk
+++ b/mk/rte.sdkconfig.mk
@@ -79,11 +79,20 @@ $(RTE_OUTPUT):
 ifdef NODOTCONF
 $(RTE_OUTPUT)/.config: ;
 else
+# Generate config from template, if there are duplicates keep only the last.
+# To do so the temp config is checked for duplicate keys with cut/sort/uniq
+# Then for each of those identified duplicates as long as there are more than
+# just one left the last match is removed.
 $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE | $(RTE_OUTPUT)
$(Q)if [ "$(RTE_CONFIG_TEMPLATE)" != "" -a -f "$(RTE_CONFIG_TEMPLATE)" 
]; then \
$(CPP) -undef -P -x assembler-with-cpp \
-ffreestanding \
-o $(RTE_OUTPUT)/.config_tmp $(RTE_CONFIG_TEMPLATE) ; \
+   for config in $$(grep -v "^#" $(RTE_OUTPUT)/.config_tmp | cut 
-d"=" -f1 | sort | uniq -d); do \
+   while [ $$(grep "^$${config}=" 
$(RTE_OUTPUT)/.config_tmp -c ) -gt 1 ]; do \
+   sed -i "0,/^$${config}=/{//d}" 
$(RTE_OUTPUT)/.config_tmp; \
+   done; \
+   done; \
if ! cmp -s $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config; 
then \
cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config ; \
cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config.orig 
; \
-- 
2.7.4



[dpdk-dev] [PATCH v3] mk: filter duplicate configuration entries

2016-07-06 Thread Christian Ehrhardt
Thanks Ferruh,
I'm personally more an awk guy so that was my natural choice.
But I in general agree, the less tools used the better for dependencies and
stability.

I checked your suggestion and works like a charm.
I'll still follow Bruce guidance to add more explanation.

v4 should show up any minute ...



Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Wed, Jul 6, 2016 at 10:57 AM, Ferruh Yigit 
wrote:

> On 7/6/2016 9:12 AM, Bruce Richardson wrote:
> > On Wed, Jul 06, 2016 at 07:37:45AM +0200, Christian Ehrhardt wrote:
> >> *updates in v3*
> >> - replace tac with sed '1!G;h;$!d' to avoid build time dependency
> >>
> >> *updates in v2*
> >> - move to .config target
> >> - fix usage order of tac
> >> - simplify inner section by only using awk (instead of
> awk+loop+bash+sed)
> >>
> >> Due to the hierarchy and the demand to keep the base config showing all
> >> options, some config keys end up multiple times in the .config file.
> >>
> >> Due to the way the actual config is sourced only the last entry is
> >> important. That can confuse people changing values in .config which
> >> are then ignored.
> >>
> >> A suggested solution was to filter for duplicates at the end of the
> >> actual config step which is implemented here.
> >>
> >> Signed-off-by: Christian Ehrhardt 
> >> ---
> >>  mk/rte.sdkconfig.mk | 6 ++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
> >> index a3acfe6..d031bf4 100644
> >> --- a/mk/rte.sdkconfig.mk
> >> +++ b/mk/rte.sdkconfig.mk
> >> @@ -79,11 +79,17 @@ $(RTE_OUTPUT):
> >>  ifdef NODOTCONF
> >>  $(RTE_OUTPUT)/.config: ;
> >>  else
> >> +# Generate config from template, if there are duplicates keep only the
> last
> >>  $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE | $(RTE_OUTPUT)
> >>  $(Q)if [ "$(RTE_CONFIG_TEMPLATE)" != "" -a -f
> "$(RTE_CONFIG_TEMPLATE)" ]; then \
> >>  $(CPP) -undef -P -x assembler-with-cpp \
> >>  -ffreestanding \
> >>  -o $(RTE_OUTPUT)/.config_tmp $(RTE_CONFIG_TEMPLATE) ; \
> >> +sed '1!G;h;$$!d' $(RTE_OUTPUT)/.config_tmp >
> $(RTE_OUTPUT)/.config_tmp_reverse ; \
> >> +awk --field-separator '=' '!/^#/ {if (!seen[$$1]) {print
> ($$0)}; seen[$$1]=1;} \
> >> +/^#/ {print($$0)}'
> $(RTE_OUTPUT)/.config_tmp_reverse \
> >> +| sed '1!G;h;$$!d' > $(RTE_OUTPUT)/.config_tmp ; \
> >> +rm $(RTE_OUTPUT)/.config_tmp_reverse ; \
> >>  if ! cmp -s $(RTE_OUTPUT)/.config_tmp
> $(RTE_OUTPUT)/.config; then \
> >>  cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config
> ; \
> >>  cp $(RTE_OUTPUT)/.config_tmp
> $(RTE_OUTPUT)/.config.orig ; \
> >> --
> >
> > Given the length and complexity of the work being done here, using some
> pretty
> > fancy sed and awk features, I feel that the comment at the top should be
> > expanded to actually explain what is being done and how. I would also
> include
> > in that explanation how sed is being used to reverse a file. Personally,
> I
> > would have preferred to keep the dependency on tac for a readability
> perspective.
> >
>
> By using sed, I didn't really mean using sed instead of tac, but
> something close to first version of this patch [1], these are just
> different ways of doing same thing.
>
> I don't know how common "tac" is, but even a box breaks build because
> off missing tac, that is problem, specially this change is not a must
> but a good to have.
>
> [1]
> for config in $$(grep -v "^#" $(RTE_OUTPUT)/.config_tmp | cut -d"=" -f1
> | sort | uniq -d); do \
> while [ $$(grep "^$${config}=" $(RTE_OUTPUT)/.config_tmp -c ) -gt 1
> ]; do \
> sed -i "0,/^$${config}=/{//d}" $(RTE_OUTPUT)/.config_tmp; \
> done; \
> done; \
>
>
>
>


[dpdk-dev] [PATCH v3] mk: filter duplicate configuration entries

2016-07-06 Thread Christian Ehrhardt
*updates in v3*
- replace tac with sed '1!G;h;$!d' to avoid build time dependency

*updates in v2*
- move to .config target
- fix usage order of tac
- simplify inner section by only using awk (instead of awk+loop+bash+sed)

Due to the hierarchy and the demand to keep the base config showing all
options, some config keys end up multiple times in the .config file.

Due to the way the actual config is sourced only the last entry is
important. That can confuse people changing values in .config which
are then ignored.

A suggested solution was to filter for duplicates at the end of the
actual config step which is implemented here.

Signed-off-by: Christian Ehrhardt 
---
 mk/rte.sdkconfig.mk | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
index a3acfe6..d031bf4 100644
--- a/mk/rte.sdkconfig.mk
+++ b/mk/rte.sdkconfig.mk
@@ -79,11 +79,17 @@ $(RTE_OUTPUT):
 ifdef NODOTCONF
 $(RTE_OUTPUT)/.config: ;
 else
+# Generate config from template, if there are duplicates keep only the last
 $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE | $(RTE_OUTPUT)
$(Q)if [ "$(RTE_CONFIG_TEMPLATE)" != "" -a -f "$(RTE_CONFIG_TEMPLATE)" 
]; then \
$(CPP) -undef -P -x assembler-with-cpp \
-ffreestanding \
-o $(RTE_OUTPUT)/.config_tmp $(RTE_CONFIG_TEMPLATE) ; \
+   sed '1!G;h;$$!d' $(RTE_OUTPUT)/.config_tmp > 
$(RTE_OUTPUT)/.config_tmp_reverse ; \
+   awk --field-separator '=' '!/^#/ {if (!seen[$$1]) {print 
($$0)}; seen[$$1]=1;} \
+   /^#/ {print($$0)}' $(RTE_OUTPUT)/.config_tmp_reverse \
+   | sed '1!G;h;$$!d' > $(RTE_OUTPUT)/.config_tmp ; \
+   rm $(RTE_OUTPUT)/.config_tmp_reverse ; \
if ! cmp -s $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config; 
then \
cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config ; \
cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config.orig 
; \
-- 
2.7.4



[dpdk-dev] [PATCH v2] mk: filter duplicate configuration entries

2016-06-30 Thread Christian Ehrhardt
*updates in v2*
- move to .config target
- fix usage order of tac
- simplify inner section by only using awk (instead of awk+loop+bash+sed)

Due to the hierarchy and the demand to keep the base config showing all
options, some config keys end up multiple times in the .config file.

Due to the way the actual config is sourced only the last entry is
important. That can confuse people changing values in .config which
are then ignored.

A suggested solution was to filter for duplicates at the end of the
actual config step which is implemented here.

Signed-off-by: Christian Ehrhardt 
---
 mk/rte.sdkconfig.mk | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
index a3acfe6..6c46122 100644
--- a/mk/rte.sdkconfig.mk
+++ b/mk/rte.sdkconfig.mk
@@ -79,11 +79,17 @@ $(RTE_OUTPUT):
 ifdef NODOTCONF
 $(RTE_OUTPUT)/.config: ;
 else
+# Generate config from template, if there are duplicates keep only the last
 $(RTE_OUTPUT)/.config: $(RTE_CONFIG_TEMPLATE) FORCE | $(RTE_OUTPUT)
$(Q)if [ "$(RTE_CONFIG_TEMPLATE)" != "" -a -f "$(RTE_CONFIG_TEMPLATE)" 
]; then \
$(CPP) -undef -P -x assembler-with-cpp \
-ffreestanding \
-o $(RTE_OUTPUT)/.config_tmp $(RTE_CONFIG_TEMPLATE) ; \
+   tac $(RTE_OUTPUT)/.config_tmp > 
$(RTE_OUTPUT)/.config_tmp_reverse ; \
+   awk --field-separator '=' '!/^#/ {if (!seen[$$1]) {print 
($$0)}; seen[$$1]=1;} \
+   /^#/ {print($$0)}' $(RTE_OUTPUT)/.config_tmp_reverse \
+   | tac > $(RTE_OUTPUT)/.config_tmp ; \
+   rm $(RTE_OUTPUT)/.config_tmp_reverse ; \
if ! cmp -s $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config; 
then \
cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config ; \
cp $(RTE_OUTPUT)/.config_tmp $(RTE_OUTPUT)/.config.orig 
; \
-- 
2.7.4



[dpdk-dev] [RFC] mk: filter duplicate configuration entries

2016-06-30 Thread Christian Ehrhardt
Hi,
thanks a lot for your feedback.
I looked at it once more and found some issues - the tac was correct, but
at the wrong place.
Also I could simplify the inner section at least a bit.
I agree to the move to the .config target.

I'll submit a v2 now to this thread.
Here is the diff it generates for a:  "make V=1
T=x86_64-native-linuxapp-gcc config"
http://paste.ubuntu.com/18163566/



Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Tue, Jun 28, 2016 at 6:48 PM, Ferruh Yigit 
wrote:

> On 6/28/2016 5:38 PM, Christian Ehrhardt wrote:
> > On Tue, Jun 28, 2016 at 6:11 PM, Ferruh Yigit  > <mailto:ferruh.yigit at intel.com>> wrote:
> >
> >     On 6/13/2016 4:10 PM, Christian Ehrhardt wrote:
> > > Due to the hierarchy and the demand to keep the base config shoing
> all
> > > options some options end up multiple times in the .config file.
> > >
> > > A suggested solution was to filter for duplicates at the end of the
> > > actual config step which is implemented here.
> > >
> > > Signed-off-by: Christian Ehrhardt <
> christian.ehrhardt at canonical.com
> > <mailto:christian.ehrhardt at canonical.com>>
> > > ---
> > >  mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk> | 5 +
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk> b/mk/
> rte.sdkconfig.mk
> > <http://rte.sdkconfig.mk>
> > > index a3acfe6..734aa06 100644
> > > --- a/mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk>
> > > +++ b/mk/rte.sdkconfig.mk <http://rte.sdkconfig.mk>
> > > @@ -70,6 +70,11 @@ config: notemplate
> > >  else
> > >  config: $(RTE_OUTPUT)/include/rte_config.h $(RTE_OUTPUT)/Makefile
> > Not sure if this should go under this rule, or
> "$(RTE_OUTPUT)/.config:"
> > and should work with ".config_tmp".
> >
> > >   $(Q)$(MAKE) depdirs
> > > + tac $(RTE_OUTPUT)/.config | awk --field-separator '=' '!/^#/
> {print $$1}' | while read config; do \
> > Why reversing file since already checking all lines one by one in
> > original file?
> >
> >
> > Hi,
> > every other comment is ok I'll rebase and resubmit once I find some time
> > again.
> > But for this (tac) the reason is simple - to keep behaviour.
> > Currently the last one wins.
>
> Correct, but if I am not missing something, reversing doesn't help to this,
> how lines deleted taking care of this:
> sed -i "0,/$${config}/{//d}" $(RTE_OUTPUT)/.config;
>
> sed works on original file, and deletes first occurrence, independent
> from lines from bottom to up, or up to bottom fed into it.
>
> > So if you have
> > CONFIG_A=n
> > CONFIG_A=y
> >
> > Essentially you have
> > CONFIG_A=y
> >
> > By the tac and keeping the first occurrence we maintain behavior.
> > It is interestingly hard to "keep the last occurrence" without such
> > tricks, but I'm open to suggestions.
> >
> >
> > And instead of checking each line, it is possible to get list of
> > duplicates via "sort | uniq -d".
> >
> >
> > That would fail for the reasons outlined above.
> >
> > Although less important, file comments also tripled in final .config.
> >
> > > + if [ $$(grep "^$${config}=" $(RTE_OUTPUT)/.config |
> wc -l) -gt 1 ]; then \
> > "grep -c" can be used instead of "grep | wc -l"
> >
> > > + sed -i "0,/$${config}/{//d}"
> > $(RTE_OUTPUT)/.config; \
> > > + fi; \
> > > + done
> > >   @echo "Configuration done"
> > >  endif
> > >
> > >
> >
> >
>
>


[dpdk-dev] [RFC] mk: filter duplicate configuration entries

2016-06-28 Thread Christian Ehrhardt
On Tue, Jun 28, 2016 at 6:11 PM, Ferruh Yigit 
wrote:

> On 6/13/2016 4:10 PM, Christian Ehrhardt wrote:
> > Due to the hierarchy and the demand to keep the base config shoing all
> > options some options end up multiple times in the .config file.
> >
> > A suggested solution was to filter for duplicates at the end of the
> > actual config step which is implemented here.
> >
> > Signed-off-by: Christian Ehrhardt 
> > ---
> >  mk/rte.sdkconfig.mk | 5 +
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
> > index a3acfe6..734aa06 100644
> > --- a/mk/rte.sdkconfig.mk
> > +++ b/mk/rte.sdkconfig.mk
> > @@ -70,6 +70,11 @@ config: notemplate
> >  else
> >  config: $(RTE_OUTPUT)/include/rte_config.h $(RTE_OUTPUT)/Makefile
> Not sure if this should go under this rule, or "$(RTE_OUTPUT)/.config:"
> and should work with ".config_tmp".
>
> >   $(Q)$(MAKE) depdirs
> > + tac $(RTE_OUTPUT)/.config | awk --field-separator '=' '!/^#/
> {print $$1}' | while read config; do \
> Why reversing file since already checking all lines one by one in
> original file?
>

Hi,
every other comment is ok I'll rebase and resubmit once I find some time
again.
But for this (tac) the reason is simple - to keep behaviour.
Currently the last one wins.
So if you have
CONFIG_A=n
CONFIG_A=y

Essentially you have
CONFIG_A=y

By the tac and keeping the first occurrence we maintain behavior.
It is interestingly hard to "keep the last occurrence" without such tricks,
but I'm open to suggestions.


> And instead of checking each line, it is possible to get list of
> duplicates via "sort | uniq -d".
>

That would fail for the reasons outlined above.

Although less important, file comments also tripled in final .config.
>
> > + if [ $$(grep "^$${config}=" $(RTE_OUTPUT)/.config | wc -l)
> -gt 1 ]; then \
> "grep -c" can be used instead of "grep | wc -l"
>
> > + sed -i "0,/$${config}/{//d}"
> $(RTE_OUTPUT)/.config; \
> > + fi; \
> > + done
> >   @echo "Configuration done"
> >  endif
> >
> >
>
>


[dpdk-dev] [RFC] mk: filter duplicate configuration entries

2016-06-13 Thread Christian Ehrhardt
Due to the hierarchy and the demand to keep the base config shoing all
options some options end up multiple times in the .config file.

A suggested solution was to filter for duplicates at the end of the
actual config step which is implemented here.

Signed-off-by: Christian Ehrhardt 
---
 mk/rte.sdkconfig.mk | 5 +
 1 file changed, 5 insertions(+)

diff --git a/mk/rte.sdkconfig.mk b/mk/rte.sdkconfig.mk
index a3acfe6..734aa06 100644
--- a/mk/rte.sdkconfig.mk
+++ b/mk/rte.sdkconfig.mk
@@ -70,6 +70,11 @@ config: notemplate
 else
 config: $(RTE_OUTPUT)/include/rte_config.h $(RTE_OUTPUT)/Makefile
$(Q)$(MAKE) depdirs
+   tac $(RTE_OUTPUT)/.config | awk --field-separator '=' '!/^#/ {print 
$$1}' | while read config; do \
+   if [ $$(grep "^$${config}=" $(RTE_OUTPUT)/.config | wc -l) -gt 
1 ]; then \
+   sed -i "0,/$${config}/{//d}" $(RTE_OUTPUT)/.config; \
+   fi; \
+   done
@echo "Configuration done"
 endif

-- 
2.7.4



[dpdk-dev] Duplicate config symbols

2016-06-13 Thread Christian Ehrhardt
On Mon, Jun 13, 2016 at 3:47 PM, Thomas Monjalon 
wrote:

> 2016-06-13 13:50, Christian Ehrhardt:
> > I wondered multiple times now when changing a config symbol that some of
> > them are in the .config file multiple times.
> > I totally feel like I'm overlooking something, but still it might be
> worth
> > to ask.
> [...]
> > Is there any reason to do so or is this an issue in make config?
>
> It is an issue in "make config" which has never been considered important.
>

I didn't want to make it more important :-)
I'm fine with the second occurrence overwriting as it did a while now and
knowing it is not a totally unknown issue.

I had seen the old argument for not moving them out completely in the old
thread - thanks for the link - that was important to understand why they
are still there.
Also found related patches from Keith about that now.


> We could remove the first - overridden - occurences.
> I think it can be fixed in mk/rte.sdkconfig.mk.
>

You mean just filtering them eventually while keeping them where they are
so that the old request to have "the base config shows all config options"
still fulfilled - yeah that could be an approach to make everybody happy.
But then it would make for some evil unreadable sed or such, I could live
with it as is knowing it is accepted as-is.
I'll submit an RFC, but hope for someone with more dark magic to make it
nicer.

Kind Regards,
Christian


[dpdk-dev] [PATCH v2] xenvirt: fix compilation after mempool changes

2016-06-13 Thread Christian Ehrhardt
Yeah, working now - thanks for the fast update!

Kind Regards,
Christian

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Mon, Jun 13, 2016 at 1:24 PM, Olivier Matz 
wrote:

> The field elt_va_start has been removed from the mempool structure,
> and it was not replaced in xenvirt.
>
> Fix this by getting the mempool objects address by using the address of
> the first memory chunk list.
>
> Note that it won't work with mempool composed of several chunks,
> but it was already the case before.
>
> Fixes: 84121f197187 ("mempool: store memory chunks in a list")
> Reported-by: Christian Ehrhardt 
> Signed-off-by: Olivier Matz 
> Acked-by: Christian Ehrhardt 
> ---
>
> v1->v2:
> - fix mempool variable name
> - fix typo in Reported-by
>
>  drivers/net/xenvirt/rte_xen_lib.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/xenvirt/rte_xen_lib.c
> b/drivers/net/xenvirt/rte_xen_lib.c
> index de63cd3..6c9a1d4 100644
> --- a/drivers/net/xenvirt/rte_xen_lib.c
> +++ b/drivers/net/xenvirt/rte_xen_lib.c
> @@ -423,6 +423,7 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool,
> uint32_t pg_num, uint32_t *g
>  {
> char key_str[PATH_MAX] = {0};
> char val_str[PATH_MAX] = {0};
> +   void *mempool_obj_va;
>
> if (grant_node_create(pg_num, gref_arr, pa_arr, val_str,
> sizeof(val_str))) {
> return -1;
> @@ -437,7 +438,14 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool,
> uint32_t pg_num, uint32_t *g
> if (snprintf(key_str, sizeof(key_str),
> DPDK_XENSTORE_PATH"%d"MEMPOOL_VA_XENSTORE_STR,
> mempool_idx) == -1)
> return -1;
> -   if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
> (uintptr_t)mpool->elt_va_start) == -1)
> +   if (mpool->nb_mem_chunks != 1) {
> +   RTE_LOG(ERR, PMD,
> +   "mempool with more than 1 chunk is not
> supported\n");
> +   return -1;
> +   }
> +   mempool_obj_va = STAILQ_FIRST(>mem_list)->addr;
> +   if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
> +   (uintptr_t)mempool_obj_va) == -1)
> return -1;
> if (xenstore_write(key_str, val_str) == -1)
> return -1;
> --
> 2.8.0.rc3
>
>


[dpdk-dev] Duplicate config symbols

2016-06-13 Thread Christian Ehrhardt
Hi,
I wondered multiple times now when changing a config symbol that some of
them are in the .config file multiple times.
I totally feel like I'm overlooking something, but still it might be worth
to ask.

In particular:
awk -F "=" '/=/ {print $1}' debian/build/static-root/.config | sort | uniq
-c | sort -n | grep -v 1
 2 CONFIG_RTE_ARCH
 2 CONFIG_RTE_EAL_IGB_UIO
 2 CONFIG_RTE_EAL_VFIO
 2 CONFIG_RTE_EXEC_ENV
 2 CONFIG_RTE_KNI_KMOD
 2 CONFIG_RTE_LIBRTE_KNI
 2 CONFIG_RTE_LIBRTE_PMD_AF_PACKET
 2 CONFIG_RTE_LIBRTE_PMD_VHOST
 2 CONFIG_RTE_LIBRTE_POWER
 2 CONFIG_RTE_LIBRTE_VHOST
 2 CONFIG_RTE_MACHINE
 2 CONFIG_RTE_TOOLCHAIN

Is there any reason to do so or is this an issue in make config?

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] [PATCH v2 2/2] mk: fix missing DEPDIRS to avoid libarary underlinking

2016-06-13 Thread Christian Ehrhardt
This patch adds missing DEPDIRS to avoid any library referring to
symbols they are not linked against.

Signed-off-by: Christian Ehrhardt 
---
 drivers/crypto/null/Makefile   | 2 ++
 drivers/net/af_packet/Makefile | 1 +
 drivers/net/bonding/Makefile   | 2 ++
 drivers/net/fm10k/Makefile | 1 +
 drivers/net/null/Makefile  | 1 +
 drivers/net/pcap/Makefile  | 1 +
 drivers/net/vhost/Makefile | 1 +
 lib/librte_ip_frag/Makefile| 1 +
 lib/librte_pipeline/Makefile   | 1 +
 lib/librte_port/Makefile   | 1 +
 lib/librte_sched/Makefile  | 1 +
 11 files changed, 13 insertions(+)

diff --git a/drivers/crypto/null/Makefile b/drivers/crypto/null/Makefile
index 2173277..573894f 100644
--- a/drivers/crypto/null/Makefile
+++ b/drivers/crypto/null/Makefile
@@ -55,5 +55,7 @@ SYMLINK-y-include +=
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO) += lib/librte_eal
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO) += lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO) += lib/librte_cryptodev
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO) += lib/librte_ring
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO) += lib/librte_kvargs

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/af_packet/Makefile b/drivers/net/af_packet/Makefile
index cb1a7ae..c6de1c6 100644
--- a/drivers/net/af_packet/Makefile
+++ b/drivers/net/af_packet/Makefile
@@ -51,6 +51,7 @@ CFLAGS += $(WERROR_FLAGS)
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += rte_eth_af_packet.c

 # this lib depends upon:
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += lib/librte_eal
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += lib/librte_kvargs
diff --git a/drivers/net/bonding/Makefile b/drivers/net/bonding/Makefile
index 10c794c..504f2e8 100644
--- a/drivers/net/bonding/Makefile
+++ b/drivers/net/bonding/Makefile
@@ -64,5 +64,7 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += lib/librte_eal
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += lib/librte_kvargs
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += lib/librte_cmdline
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += lib/librte_mempool
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += lib/librte_ring

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/fm10k/Makefile b/drivers/net/fm10k/Makefile
index cf141ae..afcbd1d 100644
--- a/drivers/net/fm10k/Makefile
+++ b/drivers/net/fm10k/Makefile
@@ -100,5 +100,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_FM10K_INC_VECTOR) += 
fm10k_rxtx_vec.c
 DEPDIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += lib/librte_eal lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += lib/librte_mempool lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += lib/librte_net
+DEPDIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += lib/librte_kvargs

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/null/Makefile b/drivers/net/null/Makefile
index 2202389..2d115de 100644
--- a/drivers/net/null/Makefile
+++ b/drivers/net/null/Makefile
@@ -54,6 +54,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += rte_eth_null.c
 SYMLINK-y-include += rte_eth_null.h

 # this lib depends upon:
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += lib/librte_eal
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += lib/librte_kvargs
diff --git a/drivers/net/pcap/Makefile b/drivers/net/pcap/Makefile
index b41d8a2..b2071e8 100644
--- a/drivers/net/pcap/Makefile
+++ b/drivers/net/pcap/Makefile
@@ -56,6 +56,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += rte_eth_pcap.c
 SYMLINK-y-include +=

 # this lib depends upon:
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_eal
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_kvargs
diff --git a/drivers/net/vhost/Makefile b/drivers/net/vhost/Makefile
index 545d998..d3d3d05 100644
--- a/drivers/net/vhost/Makefile
+++ b/drivers/net/vhost/Makefile
@@ -56,6 +56,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += rte_eth_vhost.c
 SYMLINK-y-include += rte_eth_vhost.h

 # this lib depends upon:
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += lib/librte_eal
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += lib/librte_kvargs
diff --git a/lib/librte_ip_frag/Makefile b/lib/librte_ip_frag/Makefile
index 9d06780..a375222 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


 # this library depends on rte_ether
+DEPDIRS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += lib/librte_eal
 DEPDIRS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += lib/librte_mempool lib/librte_ether

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_pipeline/Makefile b/lib/librte_pipeline

[dpdk-dev] [PATCH v2 1/2] mk: fix missing vhost dependency to pthread to avoid libarary underlinking

2016-06-13 Thread Christian Ehrhardt
Add the missing external dependency to pthread to avoid referring to
symbols the library is not linked against.

Signed-off-by: Christian Ehrhardt 
---
 drivers/net/vhost/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/vhost/Makefile b/drivers/net/vhost/Makefile
index f49a69b..545d998 100644
--- a/drivers/net/vhost/Makefile
+++ b/drivers/net/vhost/Makefile
@@ -36,6 +36,8 @@ include $(RTE_SDK)/mk/rte.vars.mk
 #
 LIB = librte_pmd_vhost.a

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

-- 
2.7.4



[dpdk-dev] [PATCH v2 0/2] mk: fix more library underlinking

2016-06-13 Thread Christian Ehrhardt
After several cleanups libraries are now linked against the libs they
refer as DEPDIR which is great.
But that now also made it visible that some references where still missing.

The following two patches try to fix that:
[PATCH v2 1/2] mk: fix missing vhost dependency to pthread to avoid
[PATCH v2 2/2] mk: fix missing DEPDIRS to avoid libarary underlinking


[dpdk-dev] [PATCH] mk: fix missing DEPDIRS to avoid libarary underlinking

2016-06-13 Thread Christian Ehrhardt
On Mon, Jun 13, 2016 at 12:08 PM, Thomas Monjalon  wrote:

> 2016-06-13 11:48, Christian Ehrhardt:
> > --- a/drivers/net/af_packet/Makefile
> > +++ b/drivers/net/af_packet/Makefile
> > @@ -54,5 +54,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) +=
> rte_eth_af_packet.c
> >  DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += lib/librte_mbuf
> >  DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += lib/librte_ether
> >  DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += lib/librte_kvargs
> > +DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += lib/librte_eal
>
> It looks more logical to have eal as first DEPDIR in these lists.


Yeah, thought about alphanumeric order, but eal first should be most
appropriate.


> > --- a/drivers/net/vhost/Makefile
> > +++ b/drivers/net/vhost/Makefile
> > @@ -36,6 +36,8 @@ include $(RTE_SDK)/mk/rte.vars.mk
> >  #
> >  LIB = librte_pmd_vhost.a
> >
> > +LDLIBS += -lpthread
> > +
>
> It is not a DEPDIR but an external dependency.

It deserves a separate patch.
>

Yes, sorry for just wrapping it in.
Will send a v2 with both changes later.


> Do we need it in rte.app.mk? It is an EAL dependency as well.
>

Not IMHO:
EAL depends on it so librte_eal.so depends on it as it should (just
reverified by ldd).
I don't see a reason that any app created should depend on pthread other
than if it actually uses libpthread.


[dpdk-dev] [PATCH] mk: fix missing DEPDIRS to avoid libarary underlinking

2016-06-13 Thread Christian Ehrhardt
After several cleanups libraries are now linked against the libs the
refer as DEPDIR which is great. But that now made it visible that some DEPDIRS
where still missing.

This patch is adding the missing depdirs to avoid any library referring to
symbols they are not linked against (found with dpkg-shlibs).

Signed-off-by: Christian Ehrhardt 
---
 drivers/crypto/null/Makefile   | 2 ++
 drivers/net/af_packet/Makefile | 1 +
 drivers/net/bonding/Makefile   | 2 ++
 drivers/net/fm10k/Makefile | 1 +
 drivers/net/null/Makefile  | 1 +
 drivers/net/pcap/Makefile  | 1 +
 drivers/net/vhost/Makefile | 3 +++
 lib/librte_ip_frag/Makefile| 1 +
 lib/librte_pipeline/Makefile   | 1 +
 lib/librte_port/Makefile   | 1 +
 lib/librte_sched/Makefile  | 1 +
 11 files changed, 15 insertions(+)

diff --git a/drivers/crypto/null/Makefile b/drivers/crypto/null/Makefile
index 2173277..573894f 100644
--- a/drivers/crypto/null/Makefile
+++ b/drivers/crypto/null/Makefile
@@ -55,5 +55,7 @@ SYMLINK-y-include +=
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO) += lib/librte_eal
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO) += lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO) += lib/librte_cryptodev
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO) += lib/librte_ring
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO) += lib/librte_kvargs

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/af_packet/Makefile b/drivers/net/af_packet/Makefile
index cb1a7ae..132f1d9 100644
--- a/drivers/net/af_packet/Makefile
+++ b/drivers/net/af_packet/Makefile
@@ -54,5 +54,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += rte_eth_af_packet.c
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += lib/librte_kvargs
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += lib/librte_eal

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/bonding/Makefile b/drivers/net/bonding/Makefile
index 10c794c..504f2e8 100644
--- a/drivers/net/bonding/Makefile
+++ b/drivers/net/bonding/Makefile
@@ -64,5 +64,7 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += lib/librte_eal
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += lib/librte_kvargs
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += lib/librte_cmdline
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += lib/librte_mempool
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += lib/librte_ring

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/fm10k/Makefile b/drivers/net/fm10k/Makefile
index cf141ae..afcbd1d 100644
--- a/drivers/net/fm10k/Makefile
+++ b/drivers/net/fm10k/Makefile
@@ -100,5 +100,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_FM10K_INC_VECTOR) += 
fm10k_rxtx_vec.c
 DEPDIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += lib/librte_eal lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += lib/librte_mempool lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += lib/librte_net
+DEPDIRS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += lib/librte_kvargs

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/null/Makefile b/drivers/net/null/Makefile
index 2202389..3502095 100644
--- a/drivers/net/null/Makefile
+++ b/drivers/net/null/Makefile
@@ -57,5 +57,6 @@ SYMLINK-y-include += rte_eth_null.h
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += lib/librte_kvargs
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += lib/librte_eal

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/pcap/Makefile b/drivers/net/pcap/Makefile
index b41d8a2..85179ad 100644
--- a/drivers/net/pcap/Makefile
+++ b/drivers/net/pcap/Makefile
@@ -59,5 +59,6 @@ SYMLINK-y-include +=
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_kvargs
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += lib/librte_eal

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/net/vhost/Makefile b/drivers/net/vhost/Makefile
index f49a69b..dd318a8 100644
--- a/drivers/net/vhost/Makefile
+++ b/drivers/net/vhost/Makefile
@@ -36,6 +36,8 @@ include $(RTE_SDK)/mk/rte.vars.mk
 #
 LIB = librte_pmd_vhost.a

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

@@ -58,5 +60,6 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += lib/librte_kvargs
 DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += lib/librte_vhost
+DEPDIRS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += lib/librte_eal

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_ip_frag/Makefile b/lib/librte_ip_frag/Makefile
index 9d06780..2b91f73 100644
--- a/lib/librte_ip_frag/Makefile
+++ b/lib/librte_ip_frag/Makefile
@@ -55,5 +55,6 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_IP_FRAG)-include += rte_ip_frag.h

 # this library depends on rte_ether
 DEPDIRS

[dpdk-dev] [PATCH v3 6/6] mk: prevent overlinking in applications

2016-06-13 Thread Christian Ehrhardt
_af_packet.so.1
dpkg-shlibdeps: warning: binaries to analyze should already be installed in
their package's directory
dpkg-shlibdeps: warning: binaries to analyze should already be installed in
their package's directory
dpkg-shlibdeps: warning: symbol rte_eal_has_hugepages used by
debian/build/shared-root/lib/librte_pmd_af_packet.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_zmalloc_socket used by
debian/build/shared-root/lib/librte_pmd_af_packet.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_free used by
debian/build/shared-root/lib/librte_pmd_af_packet.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_socket_id used by
debian/build/shared-root/lib/librte_pmd_af_packet.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_mem_virt2phy used by
debian/build/shared-root/lib/librte_pmd_af_packet.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_eal_driver_register used by
debian/build/shared-root/lib/librte_pmd_af_packet.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol per_lcore__lcore_id used by
debian/build/shared-root/lib/librte_pmd_af_packet.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_log used by
debian/build/shared-root/lib/librte_pmd_af_packet.so.1 found in none of the
libraries

Testing debian/build/shared-root/lib/librte_port.so.2
dpkg-shlibdeps: warning: binaries to analyze should already be installed in
their package's directory
dpkg-shlibdeps: warning: symbol rte_sched_port_enqueue used by
debian/build/shared-root/lib/librte_port.so.2 found in none of the libraries
dpkg-shlibdeps: warning: symbol rte_sched_port_dequeue used by
debian/build/shared-root/lib/librte_port.so.2 found in none of the libraries
dpkg-shlibdeps: warning: binaries to analyze should already be installed in
their package's directory

Testing debian/build/shared-root/lib/librte_pmd_null.so.1
dpkg-shlibdeps: warning: binaries to analyze should already be installed in
their package's directory
dpkg-shlibdeps: warning: symbol rte_zmalloc_socket used by
debian/build/shared-root/lib/librte_pmd_null.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_mem_virt2phy used by
debian/build/shared-root/lib/librte_pmd_null.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_socket_id used by
debian/build/shared-root/lib/librte_pmd_null.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_cpu_get_flag_enabled used by
debian/build/shared-root/lib/librte_pmd_null.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_log used by
debian/build/shared-root/lib/librte_pmd_null.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_eal_has_hugepages used by
debian/build/shared-root/lib/librte_pmd_null.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_free used by
debian/build/shared-root/lib/librte_pmd_null.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol per_lcore__lcore_id used by
debian/build/shared-root/lib/librte_pmd_null.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_eal_driver_register used by
debian/build/shared-root/lib/librte_pmd_null.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: binaries to analyze should already be installed in
their package's directory

Testing debian/build/shared-root/lib/librte_ip_frag.so.1
dpkg-shlibdeps: warning: binaries to analyze should already be installed in
their package's directory
dpkg-shlibdeps: warning: symbol rte_eal_has_hugepages used by
debian/build/shared-root/lib/librte_ip_frag.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_log used by
debian/build/shared-root/lib/librte_ip_frag.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol per_lcore__lcore_id used by
debian/build/shared-root/lib/librte_ip_frag.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_zmalloc_socket used by
debian/build/shared-root/lib/librte_ip_frag.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_cpu_get_flag_enabled used by
debian/build/shared-root/lib/librte_ip_frag.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: symbol rte_mem_virt2phy used by
debian/build/shared-root/lib/librte_ip_frag.so.1 found in none of the
libraries
dpkg-shlibdeps: warning: binaries to analyze should already be installed in
their package's directory


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Mon, Jun 13, 2016 at 11:05 AM, Ferruh Yigit 
wrote:

> On 6/11/2016 7:34 AM, Thomas Monjalon wrote:
> > Hi Ferruh,
> >
> > 2016-06-10 19:32, Ferruh Yigit:
> >> --- a/mk/rte.app.mk
> >> +++ b/mk/rte.app.mk
> >> @@ -50,6 +50,14 @@ ifeq ($(NO_LDSCRIPT),)
> >>  LDSCRIPT = $(RTE_LDSCRIPT)
> >>  endif
> >>
> >> +ifeq ($(CONFIG_RTE_BUILD_SHARED_L

[dpdk-dev] [PATCH] xenvirt: fix compilation after mempool changes

2016-06-13 Thread Christian Ehrhardt
Hmm,
Hi again Oliver.

I was too fast saying yes.
I don't know what is different now but I clearly tested it wrong the first
time.
Now I get:

  CC rte_xen_lib.o
/mnt/nvme/dpdk-16-07-pre-linking/drivers/net/xenvirt/rte_xen_lib.c: In
function ?grant_gntalloc_mbuf_pool?:
/mnt/nvme/dpdk-16-07-pre-linking/drivers/net/xenvirt/rte_xen_lib.c:441:6:
error:
?mp? undeclared (first use in this function)
 if (mp->nb_mem_chunks != 1) {
 ^
/mnt/nvme/dpdk-16-07-pre-linking/drivers/net/xenvirt/rte_xen_lib.c:441:6: note:
each undeclared identifier is reported only once for each function it
appears in
/mnt/nvme/dpdk-16-07-pre-linking/drivers/net/xenvirt/rte_xen_lib.c:422:46:
error:
unused parameter ?mpool? [-Werror=unused-parameter]
grant_gntalloc_mbuf_pool(struct rte_mempool *mpool, uint32_t pg_num,
uint32_t *gref_arr, phys_addr_t *pa_arr, int mempool_idx)
 ^
cc1: all warnings being treated as errors


Not too hard, changing the mp to mpool on the two places the patch has
inserted it gets it working.




Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Mon, Jun 13, 2016 at 10:51 AM, Christian Ehrhardt <
christian.ehrhardt at canonical.com> wrote:

> Hi Oliver,
> thanks for the fast response!
>
> It fixes the compilation issue and I totally agree to your argument of the
> multi-chunk issues being out of scope for this as they never worked.
> Unfortunately I lack an environment to actually test this in real-life if
> we need any more follow up than this.
>
> Acked-by: Christian Ehrhardt 
>
>
>
> Christian Ehrhardt
> Software Engineer, Ubuntu Server
> Canonical Ltd
>
> On Mon, Jun 13, 2016 at 10:22 AM, Olivier Matz 
> wrote:
>
>> The field elt_va_start has been removed from the mempool structure,
>> and it was not replaced in xenvirt.
>>
>> Fix this by getting the mempool objects address by using the address of
>> the first memory chunk list.
>>
>> Note that it won't work with mempool composed of several chunks,
>> but it was already the case before.
>>
>> Fixes: 84121f197187 ("mempool: store memory chunks in a list")
>> Reported-by: Christian Ehrhard 
>> Signed-off-by: Olivier Matz 
>> ---
>>  drivers/net/xenvirt/rte_xen_lib.c | 10 +-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/xenvirt/rte_xen_lib.c
>> b/drivers/net/xenvirt/rte_xen_lib.c
>> index de63cd3..997e56e 100644
>> --- a/drivers/net/xenvirt/rte_xen_lib.c
>> +++ b/drivers/net/xenvirt/rte_xen_lib.c
>> @@ -423,6 +423,7 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool,
>> uint32_t pg_num, uint32_t *g
>>  {
>> char key_str[PATH_MAX] = {0};
>> char val_str[PATH_MAX] = {0};
>> +   void *mempool_obj_va;
>>
>> if (grant_node_create(pg_num, gref_arr, pa_arr, val_str,
>> sizeof(val_str))) {
>> return -1;
>> @@ -437,7 +438,14 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool,
>> uint32_t pg_num, uint32_t *g
>> if (snprintf(key_str, sizeof(key_str),
>> DPDK_XENSTORE_PATH"%d"MEMPOOL_VA_XENSTORE_STR,
>> mempool_idx) == -1)
>> return -1;
>> -   if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
>> (uintptr_t)mpool->elt_va_start) == -1)
>> +   if (mp->nb_mem_chunks != 1) {
>> +   RTE_LOG(ERR, PMD,
>> +   "mempool with more than 1 chunk is not
>> supported\n");
>> +   return -1;
>> +   }
>> +   mempool_obj_va = STAILQ_FIRST(>mem_list)->addr;
>> +   if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
>> +   (uintptr_t)mempool_obj_va) == -1)
>> return -1;
>> if (xenstore_write(key_str, val_str) == -1)
>> return -1;
>> --
>> 2.8.0.rc3
>>
>>
>


[dpdk-dev] [PATCH] xenvirt: fix compilation after mempool changes

2016-06-13 Thread Christian Ehrhardt
Hi Oliver,
thanks for the fast response!

It fixes the compilation issue and I totally agree to your argument of the
multi-chunk issues being out of scope for this as they never worked.
Unfortunately I lack an environment to actually test this in real-life if
we need any more follow up than this.

Acked-by: Christian Ehrhardt 



Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Mon, Jun 13, 2016 at 10:22 AM, Olivier Matz 
wrote:

> The field elt_va_start has been removed from the mempool structure,
> and it was not replaced in xenvirt.
>
> Fix this by getting the mempool objects address by using the address of
> the first memory chunk list.
>
> Note that it won't work with mempool composed of several chunks,
> but it was already the case before.
>
> Fixes: 84121f197187 ("mempool: store memory chunks in a list")
> Reported-by: Christian Ehrhard 
> Signed-off-by: Olivier Matz 
> ---
>  drivers/net/xenvirt/rte_xen_lib.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/xenvirt/rte_xen_lib.c
> b/drivers/net/xenvirt/rte_xen_lib.c
> index de63cd3..997e56e 100644
> --- a/drivers/net/xenvirt/rte_xen_lib.c
> +++ b/drivers/net/xenvirt/rte_xen_lib.c
> @@ -423,6 +423,7 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool,
> uint32_t pg_num, uint32_t *g
>  {
> char key_str[PATH_MAX] = {0};
> char val_str[PATH_MAX] = {0};
> +   void *mempool_obj_va;
>
> if (grant_node_create(pg_num, gref_arr, pa_arr, val_str,
> sizeof(val_str))) {
> return -1;
> @@ -437,7 +438,14 @@ grant_gntalloc_mbuf_pool(struct rte_mempool *mpool,
> uint32_t pg_num, uint32_t *g
> if (snprintf(key_str, sizeof(key_str),
> DPDK_XENSTORE_PATH"%d"MEMPOOL_VA_XENSTORE_STR,
> mempool_idx) == -1)
> return -1;
> -   if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
> (uintptr_t)mpool->elt_va_start) == -1)
> +   if (mp->nb_mem_chunks != 1) {
> +   RTE_LOG(ERR, PMD,
> +   "mempool with more than 1 chunk is not
> supported\n");
> +   return -1;
> +   }
> +   mempool_obj_va = STAILQ_FIRST(>mem_list)->addr;
> +   if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
> +   (uintptr_t)mempool_obj_va) == -1)
> return -1;
> if (xenstore_write(key_str, val_str) == -1)
> return -1;
> --
> 2.8.0.rc3
>
>


[dpdk-dev] Question regarding mempool changes impact to XEN PMD

2016-06-13 Thread Christian Ehrhardt
On Mon, Jun 13, 2016 at 10:14 AM, Olivier Matz 
wrote:

> Hi Christian,
>
> On 06/13/2016 09:34 AM, Christian Ehrhardt wrote:
> > Hi David,
>
> I guess this mail is for me, not for David :)
>

Absolutely yes, sorry to both of you to - probably read too much patch
headers this morning :-)


> > it seems to be the first time I compiled with
> > CONFIG_RTE_LIBRTE_PMD_XENVIRT=y sinec the bigger mempool changes around
> > "587d684d doc: update release notes about mempool allocation".
> >
> > I've seen related patch to mempool / xen in that regard "c042ba20
> mempool:
> > rework support of Xen dom0"
> >
> > But with above config symbol enabled I got:
> > drivers/net/xenvirt/rte_xen_lib.c: In function
> ?grant_gntalloc_mbuf_pool?:
> > drivers/net/xenvirt/rte_xen_lib.c:440:69: error: ?struct rte_mempool? has
> > no member named ?elt_va_start?
> >   if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
> > (uintptr_t)mpool->elt_va_start) == -1)
> >  ^
> >   SYMLINK-FILE include/rte_eth_bond.h
> > mk/internal/rte.compile-pre.mk:126: recipe for target 'rte_xen_lib.o'
> failed
> > make[4]: *** [rte_xen_lib.o] Error 1
> > make[4]: *** Waiting for unfinished jobs
> >
> > The change around the mempools is complex, so I don't see on the first
> look
> > if that needs a minor or major rework in the xen sources.
> > I mean I don't want it to compile, but to work and that could be more
> than
> > just fixing that changed structure :-)
> >
> > So I wanted to ask if you as author would see if it is a trivial change
> > that has to be made?
>
> Sorry, I missed this reference to elt_va_start in my patches.
>
> I'm not very familiar with the xen code in dpdk, but from what I see:
>
> - in the PMD, grant_gntalloc_mbuf_pool() stores the mempool virtual
>   address in the xen key/value database
> - in examples/vhost_xen, the function parse_mpool_va() retrieves it
> - this address is used in new_device()
>
> I think the patch would be almost similar to what I did in mlx
> drivers in this commit:
>
> http://dpdk.org/browse/dpdk/commit/?id=84121f1971873c9f45b2939c316c66126d8754a1
>
> or in librte_kni in this commit:
>
> http://dpdk.org/browse/dpdk/commit?id=d1d914ebbc2514f334a3ed24057e63c8bb76363d
>
> To give more precisions:
>
> - before the patchset, mp->elt_va_start was the virtual address of the
>   mempool objects table. It was always virtually contiguous
>
> - now, a mempool can be fragmented in several virtually contiguous
>   chunks. In case there is only one chunk, it can be safely replaced
>   by STAILQ_FIRST(>mem_list)->addr (= the virtual address of the
>   first chunk).
>
> In case there are more chunks in the mempool, it would require deeper
> modifications I think. But we should keep in mind that having a
> virtually fragmented mempool was not possible before the patchset
> (it would have fail at init). If if fails later in xen code because
> xen does not support fragmented mempools, there is no regression
> compared to what we had before.
>

Ack to that, I only cared about the regression and that I think you covered
excellently.
To make fragmented pools work would be a task for one who "cares" to use it.


>
> I'll send a draft patch, if you could give it a try, it would be great!
>

I can compile and review the patch, but I neither have a setup to actually
run it.
Maybe someone else on the list have, please feel encouraged to do so.


> Thanks for reporting,
> Olivier
>
>


[dpdk-dev] Question regarding mempool changes impact to XEN PMD

2016-06-13 Thread Christian Ehrhardt
Hi David,
it seems to be the first time I compiled with
CONFIG_RTE_LIBRTE_PMD_XENVIRT=y sinec the bigger mempool changes around
"587d684d doc: update release notes about mempool allocation".

I've seen related patch to mempool / xen in that regard "c042ba20 mempool:
rework support of Xen dom0"

But with above config symbol enabled I got:
drivers/net/xenvirt/rte_xen_lib.c: In function ?grant_gntalloc_mbuf_pool?:
drivers/net/xenvirt/rte_xen_lib.c:440:69: error: ?struct rte_mempool? has
no member named ?elt_va_start?
  if (snprintf(val_str, sizeof(val_str), "%"PRIxPTR,
(uintptr_t)mpool->elt_va_start) == -1)
 ^
  SYMLINK-FILE include/rte_eth_bond.h
mk/internal/rte.compile-pre.mk:126: recipe for target 'rte_xen_lib.o' failed
make[4]: *** [rte_xen_lib.o] Error 1
make[4]: *** Waiting for unfinished jobs

The change around the mempools is complex, so I don't see on the first look
if that needs a minor or major rework in the xen sources.
I mean I don't want it to compile, but to work and that could be more than
just fixing that changed structure :-)

So I wanted to ask if you as author would see if it is a trivial change
that has to be made?


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] [PATCH] log: deprecate history dump

2016-06-09 Thread Christian Ehrhardt
Hi,
in I totally like it - thanks Thomas for picking that up.

I just wanted to mention that the Makefile still refers to mempool, but
David beat me in time and Detail a lot.

I'll certainly try to follow and help the bit I can.



Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Thu, Jun 9, 2016 at 4:45 PM, David Marchand 
wrote:

> Thomas,
>
> On Thu, Jun 9, 2016 at 4:09 PM, Thomas Monjalon
>  wrote:
> > The log history uses rte_mempool. In order to remove the mempool
> > dependency in EAL (and improve the build), this feature is deprecated.
> > The ABI is kept but the behaviour is now voided because it seems this
> > function was not used. The history can be read from syslog.
>
> It does look like it is not really used.
> I am for this change unless someone complains.
>
> Comments below.
>
> > Signed-off-by: Thomas Monjalon 
> > ---
> >  app/test-pmd/cmdline.c   |   3 -
> >  app/test/autotest_test_funcs.py  |   5 --
> >  app/test/commands.c  |   4 +-
> >  app/test/test_logs.c |   3 -
> >  doc/guides/rel_notes/deprecation.rst |   3 +
> >  lib/librte_eal/bsdapp/eal/eal_debug.c|   6 --
> >  lib/librte_eal/common/eal_common_log.c   | 128
> +--
> >  lib/librte_eal/common/include/rte_log.h  |   8 ++
> >  lib/librte_eal/linuxapp/eal/eal_debug.c  |   6 --
> >  lib/librte_eal/linuxapp/eal/eal_interrupts.c |   1 -
> >  lib/librte_eal/linuxapp/eal/eal_ivshmem.c|   1 -
> >  lib/librte_eal/linuxapp/eal/eal_log.c|   3 -
> >  lib/librte_mempool/rte_mempool.c |   4 -
> >  13 files changed, 16 insertions(+), 159 deletions(-)
>
> - You missed autotest_data.py.
>
> $ git grep dump_log_history
> app/test/autotest_data.py:   "Command" :"dump_log_history",
>
> - eal Makefile still refers to librte_mempool.
>
> - Since you are looking at this, what keeps us from removing the
> dependency on librte_ring ?
> I would say it was mainly because of mempool.
> Maybe ivshmem ?
>
>
> [snip]
>
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> > index ad05eba..f11df93 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -8,6 +8,9 @@ API and ABI deprecation notices are to be posted here.
> >  Deprecation Notices
> >  ---
> >
> > +* The log history is deprecated in release 16.07.
> > +  It is voided and will be completely removed in release 16.11.
> > +
>
> It is voided in 16.07 and will be completely removed in release 16.11.
>
>
> >  * The ethdev hotplug API is going to be moved to EAL with a notification
> >mechanism added to crypto and ethdev libraries so that hotplug is now
> >available to both of them. This API will be stripped of the device
> arguments
>
> > diff --git a/lib/librte_eal/common/eal_common_log.c
> b/lib/librte_eal/common/eal_common_log.c
> > index b5e37bb..94ecdd2 100644
> > --- a/lib/librte_eal/common/eal_common_log.c
> > +++ b/lib/librte_eal/common/eal_common_log.c
> > @@ -56,29 +56,11 @@
> >  #include 
> >  #include 
> >  #include 
> > -#include 
> >
> >  #include "eal_private.h"
> >
> >  #define LOG_ELT_SIZE 2048
>
> We don't need LOG_ELT_SIZE.
>
> >
> > -#define LOG_HISTORY_MP_NAME "log_history"
> > -
> > -STAILQ_HEAD(log_history_list, log_history);
> > -
> > -/**
> > - * The structure of a message log in the log history.
> > - */
> > -struct log_history {
> > -   STAILQ_ENTRY(log_history) next;
> > -   unsigned size;
> > -   char buf[0];
> > -};
> > -
> > -static struct rte_mempool *log_history_mp = NULL;
> > -static unsigned log_history_size = 0;
> > -static struct log_history_list log_history;
> > -
> >  /* global log structure */
> >  struct rte_logs rte_logs = {
> > .type = ~0,
> > @@ -86,10 +68,7 @@ struct rte_logs rte_logs = {
> > .file = NULL,
> >  };
> >
> > -static rte_spinlock_t log_dump_lock = RTE_SPINLOCK_INITIALIZER;
> > -static rte_spinlock_t log_list_lock = RTE_SPINLOCK_INITIALIZER;
> >  static FILE *default_log_stream;
> > -static int history_enabled = 1;
> >
> >  /**
> >   * This global structure stores some informations about the message
> > @@ -106,59 +85,14 @@ static RTE_DEFINE_PER_LCORE(struct log_cur_msg,
> log_cur_msg

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

2016-06-07 Thread Christian Ehrhardt
Hi Panu,
thanks for refreshing that - I'm much more happy with ldd output of most
libraries now after build with that.
And it comes much slimmer than my initial approach that touched all lib
makefiles.

Acked-by: Christian Ehrhardt 

FYI - for anyone else also trying this in a backported way to 16.04 it
needs c6417ce6 and aace9d0b as prereq.

This being done is a nice step taken towards saner linking.
But I still struggle to see how to fix the circular dependency between
librte_eal and librte_mempool.
Maybe now is a time to look at this part of the original threads again to
eventually get apps less overlinked?
=> http://www.dpdk.org/ml/archives/dev/2016-May/039441.html
My naive suggestions in generalized form can be found there (no answer yet):
=>
http://stackoverflow.com/questions/37351699/how-to-create-both-so-files-for-two-circular-depending-libraries


Kind Regards,
Christian



Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Tue, Jun 7, 2016 at 12:01 PM, Panu Matilainen 
wrote:

> 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] RFC: DPDK Long Term Support

2016-06-07 Thread Christian Ehrhardt
On Fri, Jun 3, 2016 at 5:07 PM, Mcnamara, John 
wrote:
[...]
>
> LTS Version
> 
>
> The proposed initial LTS version will be DPDK 16.07. The next versions,
> based
> on a 2 year cycle, will be DPDK 18.08, 20.08, etc.
>

I can see on the discussions that much more things around this have to be
discussed and agreed, but to some extend we will also just "have to wait
and see" how things work out.
I fully agree to the API change argument to start with 16.07 and the 2 year
cycle (more would be nice, but this means effort and after a while almost
nothing is "easily" backportable).

Never the less I have to ask - as I'd be personally much more happy if it
would be the odd years autumn release that would make the LTS as it would
match our LTS releases much much better.
Otherwise we (Ubuntu) will always "just miss" the LTS by a few months.
First I'd have thought on xx.02 releases, but consuming applications might
need time to adapt and while there are the nice API/ABI guarantees
experience tells me to better leave some kind of time-buffer.

Also this would give all of us a first shot with a shorter (not so long as
in L) LTS to see if the process we defined works out before jumping on a
full 2 year cadence.

So while I see that this is kind of "my problem" I would at least try to
personally ask and vote for LTS being: 16.7, 17.11, 19.11, 21.11, ...

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] If 1 KVM Guest loads the virtio-pci, on top of dpdkvhostuser OVS socket interface, it slows down everything!

2016-05-25 Thread Christian Ehrhardt
Hi again,
another forgotten case.

I currently I lack the HW to fully reproduce this, but the video summary is
pretty good and shows the issue in an impressive way.

Also the description is good and here as well I wonder if anybody else
could reproduce this.
Any hints / insights are welcome.

P.S. and also again - two list cross posting, but here as well it is yet
unclear which it belongs to so I'll keep it as well

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Sun, May 22, 2016 at 6:35 PM, Martinx - ? 
wrote:

> Guys,
>
>  I'm seeing a strange problem here, in my OVS+DPDK deployment, on top of
> Ubuntu 16.04 (DPDK 2.2 and OVS 2.5).
>
>  Here is what I'm trying to do: run OVS with DPDK at the host, for KVM
> Guests that also, will be running more DPDK Apps.
>
>  The host have 2 x 10G NICs, for OVS+DPDK and each KVM Guest receives its
> own VLAN tagged traffic (or all tags).
>
>  There is an IXIA Traffic Generator sending 10G of traffic on both
> directions (20G total).
>
>  Exemplifying, the problem is, lets say that I already have 2 VMs (or 10)
> running DPDK Apps (on top of dpdkvhostuser), everything is working as
> expected, then, if I boot the 3rd (or 11) KVM Guest, the OVS+DPDK bridge at
> the host, slows down, a lot! The 3rd (or 11) VM affects not only the host,
> but also, all the other neighbors VMs!!!
>
>  NOTE: This problem appear since the boot of VM 1.
>
>  Soon as you, inside of the 3rd VM, bind the VirtIO NIC to the
> DPDK-Compative Drivers, the speed comes back to normal. If you bind it back
> to "virtio-pci", boom! The OVS+DPDK at the host and all VMs loses too much
> speed.
>
>  This problem is detailed at the following bug report:
>
> --
> The OVS+DPDK dpdkvhostuser socket bridge, only works as expected, if the
> KVM Guest also have DPDK drivers loaded:
>
> https://bugs.launchpad.net/ubuntu/+source/openvswitch/+bug/1577256
> --
>
>  Also, I've recorded a ~15 min screen cast video about this problem, so,
> you guys can see exactly what is happening here.
>
> https://www.youtube.com/v/yHnaSikd9XY?version=3=hd720=1
>
>  * At 5:25, I'm starting a VM that will boot up and load a DPDK App;
>
>  * At 5:33, OVS+DPDK is messed up, it loses speed;
>The KVM running with virtio-pci drivers breaks OVS+DPDK at the host;
>
>  * At 6:50, DPDK inside of the KVM guest loads up its drivers, kicking
> "virtio-pci", speed back to normal at the host;
>
>  * At 7:43, started another KVM Guest, now, while virtio-pci driver is
> running, the OVS+DPDK at the host and the other VM, are very, very slow;
>
>  * At 8:52, the second VM loads up DPDK Drivers, kicking virtio-pci, the
> speed is back to normal at the host, and on the other VM too;
>
>  * At 10:00, the Ubuntu VM loads up virtio-pci drivers on its boot, the
> speed dropped at the hosts and on the other VMs;
>
>  * 11:57, I'm starting "service dpdk start" inside of the Ubuntu guest, to
> kick up virtio-pci, and bang! Speed is back to normal everywhere;
>
>  * 12:51, I'm trying to unbind the DPDK Drivers and return the virtio-pci,
> I forgot the syntax while recording the video, which is: "dpdk_nic_bind -b
>  virtio-pci", so, I just rebooted it. But both "reboot" or "rebind to
> virtio-pci" triggers the bug.
>
>
> NOTE: I tried to subscriber to qemu-devel but, it is not working, I'm not
> receiving the confirmation e-mail, while qemu-stable worked. I don't know
> if it worth sending it to Linux Kernel too...
>
>
> Regards,
> Thiago
>


[dpdk-dev] Crashing OVS+DPDK at the host, from inside of a KVM Guest

2016-05-25 Thread Christian Ehrhardt
Hi,
ping ...

Later on I want to look at it again once we upgraded to more recent
releases of the software components involved, but those have to be made
ready to use first :-/

But the description is good and I wonder if anybody else could reproduce
this and/or would have a hint on where this might come from or already
existing related fixes.

I mean in general nothing should be able to crash the host right?


P.S. yeah two list cross posting, but it is yet unclear which it belongs to
so I'll keep it

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Sun, May 15, 2016 at 7:08 AM, Martinx - ? 
wrote:

> Guys,
>
>  If using OVS 2.5 with DPDK 2.2, on Ubuntu Xenial, it is possible to crash
> the OVS running at the host, from inside of a KVM Guest.
>
>  Basically, what I'm trying to do, is to run OVS+DPDK at the host, and
> also, inside of a KVM Guest, with multi-queue, but it doesn't work and
> crashes.
>
>  Soon as you enable multi-queue at the guest, it crashes the OVS of the
> host!
>
> OVS+DPDK segfault at the host, after running "ovs-vsctl set Open_vSwitch .
> other_config:n-dpdk-rxqs=4" within a KVM Guest:
>
> https://bugs.launchpad.net/ubuntu/+source/openvswitch/+bug/1577088
>
> Thanks!
> Thiago
>


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

2016-05-24 Thread Christian Ehrhardt
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 -
>


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

2016-05-20 Thread 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.

Signed-off-by: Christian Ehrhardt 
---
 lib/librte_acl/Makefile | 1 +
 lib/librte_cfgfile/Makefile | 1 +
 lib/librte_cmdline/Makefile | 1 +
 lib/librte_cryptodev/Makefile   | 1 +
 lib/librte_distributor/Makefile | 1 +
 lib/librte_ether/Makefile   | 1 +
 lib/librte_hash/Makefile| 1 +
 lib/librte_ip_frag/Makefile | 1 +
 lib/librte_ivshmem/Makefile | 1 +
 lib/librte_jobstats/Makefile| 1 +
 lib/librte_kni/Makefile | 1 +
 lib/librte_kvargs/Makefile  | 1 +
 lib/librte_lpm/Makefile | 1 +
 lib/librte_mbuf/Makefile| 1 +
 lib/librte_mempool/Makefile | 1 +
 lib/librte_meter/Makefile   | 1 +
 lib/librte_pipeline/Makefile| 1 +
 lib/librte_port/Makefile| 1 +
 lib/librte_power/Makefile   | 1 +
 lib/librte_reorder/Makefile | 1 +
 lib/librte_ring/Makefile| 1 +
 lib/librte_sched/Makefile   | 1 +
 lib/librte_table/Makefile   | 3 +++
 lib/librte_timer/Makefile   | 1 +
 lib/librte_vhost/Makefile   | 1 +
 25 files changed, 27 insertions(+)

diff --git a/lib/librte_acl/Makefile b/lib/librte_acl/Makefile
index 2e394c9..7ca0da4 100644
--- a/lib/librte_acl/Makefile
+++ b/lib/librte_acl/Makefile
@@ -91,6 +91,7 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_ACL)-include := rte_acl_osdep.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_ACL)-include += rte_acl.h

 # this lib needs eal
+LDLIBS += -lrte_eal
 DEPDIRS-$(CONFIG_RTE_LIBRTE_ACL) += lib/librte_eal

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_cfgfile/Makefile b/lib/librte_cfgfile/Makefile
index 616aef0..668aadc 100644
--- a/lib/librte_cfgfile/Makefile
+++ b/lib/librte_cfgfile/Makefile
@@ -52,6 +52,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_CFGFILE) += rte_cfgfile.c
 SYMLINK-$(CONFIG_RTE_LIBRTE_CFGFILE)-include += rte_cfgfile.h

 # this lib needs eal
+LDLIBS += -lrte_eal
 DEPDIRS-$(CONFIG_RTE_LIBRTE_CFGFILE) += lib/librte_eal

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_cmdline/Makefile b/lib/librte_cmdline/Makefile
index 7d2d148..d134e18 100644
--- a/lib/librte_cmdline/Makefile
+++ b/lib/librte_cmdline/Makefile
@@ -62,6 +62,7 @@ INCS += cmdline_vt100.h cmdline_socket.h cmdline_cirbuf.h 
cmdline_parse_portlist
 SYMLINK-$(CONFIG_RTE_LIBRTE_CMDLINE)-include := $(INCS)

 # this lib needs eal
+LDLIBS += -lrte_eal
 DEPDIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) += lib/librte_eal

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_cryptodev/Makefile b/lib/librte_cryptodev/Makefile
index 314a046..af69d37 100644
--- a/lib/librte_cryptodev/Makefile
+++ b/lib/librte_cryptodev/Makefile
@@ -53,6 +53,7 @@ SYMLINK-y-include += rte_cryptodev_pmd.h
 EXPORT_MAP := rte_cryptodev_version.map

 # library dependencies
+LDLIBS += -lrte_eal -lrte_mempool -lrte_ring -lrte_mbuf -lrte_kvargs
 DEPDIRS-y += lib/librte_eal
 DEPDIRS-y += lib/librte_mempool
 DEPDIRS-y += lib/librte_ring
diff --git a/lib/librte_distributor/Makefile b/lib/librte_distributor/Makefile
index 4c9af17..fb651e1 100644
--- a/lib/librte_distributor/Makefile
+++ b/lib/librte_distributor/Makefile
@@ -48,6 +48,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) := rte_distributor.c
 SYMLINK-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)-include := rte_distributor.h

 # this lib needs eal
+LDLIBS += -lrte_eal -lrte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += lib/librte_eal
 DEPDIRS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += lib/librte_mbuf

diff --git a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile
index 0bb5dc9..31766e2 100644
--- a/lib/librte_ether/Makefile
+++ b/lib/librte_ether/Makefile
@@ -54,6 +54,7 @@ SYMLINK-y-include += rte_eth_ctrl.h
 SYMLINK-y-include += rte_dev_info.h

 # this lib depends upon:
+LDLIBS += -lrte_eal -lrte_mempool -lrte_ring -lrte_mbuf
 DEPDIRS-y += lib/librte_eal lib/librte_mempool lib/librte_ring lib/librte_mbuf

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_hash/Makefile b/lib/librte_hash/Makefile
index bb1ea99..6d94fe0 100644
--- a/lib/librte_hash/Makefile
+++ b/lib/librte_hash/Makefile
@@ -56,6 +56,7 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_thash.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include += rte_fbk_hash.h

 # this lib needs eal and ring
+LDLIBS += -lrte_eal -lrte_ring
 DEPDIRS-$(CONFIG_RTE_LIBRTE_HASH) += lib/librte_eal lib/librte_ring

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_ip_frag/Makefile b/lib/librte_ip_frag/Makefile
index 9d06780..42a7052 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


 # this library depends on rte_ether
+LDLIBS += -lrte_mempool -lethdev
 DEPDIRS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += lib/librte_mempool lib/librte_ether

 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_ivshmem/Makefile b/lib/librte_ivshmem/Makefile

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

2016-05-20 Thread Christian Ehrhardt
ok,
- little baby steps first
  I'll submit a trivial patch in reply to this fixing the underlinking in
all .so's, except librte_eal.

- discussion on the remaining open issue second.
  For the remaining issue I think that needs a special understanding of
linkers I still need to grasp.
  But since I failed to find a good explanation for the exact case we face
here I put it in a generalized form on StackOverflow so eventually more
than just us can benefit from the answer once/if we find an answer.

So to all of you - if you are not scared of the latter pages of ld/gcc man
pages :-) feel free to take a look at
http://stackoverflow.com/questions/37351699/how-to-create-both-so-files-for-two-circular-depending-libraries

For us, but not important for the question in general:
 liba = librte_eal, libb = librte_mempool
(and ring, ivshmem, but once it is solved once it is solved)

Time to stip for now, have a great Weekend
Christian



Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Fri, May 20, 2016 at 2:41 PM, Christian Ehrhardt <
christian.ehrhardt at canonical.com> wrote:

> On Fri, May 20, 2016 at 1:01 PM, Panu Matilainen 
> wrote:
>
>> 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
>> :-/
>
>
> Since I work on 16.04 as released currently I almost missed those, but I
> at least remember seeing the first on the list.
> I had almost the same change - adopting yours since it is accepted.
> Maybe I can provide a few more baby steps on that path - or at least a few
> more pieces for discussion and review.
> I hope you get well soon Panu!
>
>
>> #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 link

[dpdk-dev] Suggestions for the dpdk stable tree

2016-05-20 Thread Christian Ehrhardt
Hi,
it would be great to have an actively maintained LTS release.

Clearly all us "Distribution People" like Panu, Me and a few others should
- whenever bugs are identified as potential backports - start the
discussion.
And having an stable-maintainer on the other end sounds great.

The active maintainer you provide would do the usual auto-backport of fixes
once they come up upstream.
While us others would provide input from real exposure to users that flows
in via bug tracking tools.

Looking forward for you coming back to the list once you have found one.


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Fri, May 20, 2016 at 4:49 PM, Mcnamara, John 
wrote:

> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Christian Ehrhardt
> > Sent: Friday, May 20, 2016 9:07 AM
> > To: dev ; Stephen Hemminger 
> > Subject: Re: [dpdk-dev] Suggestions for the dpdk stable tree
> >
> > Hi,
> > I guess over time/releases less people mind the 2.2-stable.
> > But I still see a lot of people referring to 2.2 - so why not giving this
> > thread a ping again.
> >
> > ack / nack / opinions ?
>
> Hi Christian,
>
> We are interested in having a LTS/Stable tree.
>
> We have been looking at identifying a maintainer and validation engineer
> internally to support the effort but haven't be able to finalize that. Once
> we do we will come back to the mailing list with a proposal and a request
> for comments.
>
> We would probably be looking at 16.04 or even 16.07 as the basis for the
> LTS at this stage.
>
> It would be great if we could get support from you or others as well.
>
> John.
> --
>
>


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

2016-05-20 Thread Christian Ehrhardt
On Fri, May 20, 2016 at 1:01 PM, Panu Matilainen 
wrote:

> 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
> :-/


Since I work on 16.04 as released currently I almost missed those, but I at
least remember seeing the first on the list.
I had almost the same change - adopting yours since it is accepted.
Maybe I can provide a few more baby steps on that path - or at least a few
more pieces for discussion and review.
I hope you get well soon Panu!


> #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.
>

Interesting approach, I wasn't even working on DPDK back then - that could
really simplify the mk there.
I was experimenting with just dropping the "no-as-needed" in
mk/exec-env/linuxapp/rte.vars.mk, but that seems to shoot too far - haven't
had the time to look deeper.
Interested if your approach could help.

But the other part of it - getting the proper DT_NEEDED into the libs (I
also consider underlinking worse than overlinking) seems to be just missing
a bunch of -l - my last test build looked fine.
There is the underlinking issue of librte_eal left that Sergio mentioned
back in 6d25d90c, but it seems to be the next baby step to solve most else
of the underlinking.
I'll adapt it to git level and at least throw it out for discussion later
on.


[dpdk-dev] Compilation failed on dpdk-16.04

2016-05-20 Thread Christian Ehrhardt
Hi,
as Thomas said that is a dep for pcap.
Note that dpdk is part of Ubuntu 15.10 and later - due to that even if
building on your own on these releases you would be able to run "sudo
apt-get build-dep dpdk" to get all usual build dependencies resolved
automatically.

But since you are on an older release you can just do what this woudl do
for you which would be (including pcap enabled and all kind of docu
building things) :
sudo apt-get install doxygen graphviz inkscape libcap-dev libpcap-dev
libxen-dev libxenstore3.0 python python-sphinx texlive-fonts-recommended
texlive-latex-extra





Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Fri, May 20, 2016 at 2:03 PM, Sheroo Pratap <
sheroopratapresearch at gmail.com> wrote:

> Hi All,
>
>   I m trying to install dpdk-16.04 on my VM ubuntu.
>   I tried to make the build but it failed with below error.
>
>   anyone can help on this.
>
>  ubuntu version is "Ubuntu 14.04.3 (64bit)"
>  kernel version is 4.6.
>
> * SYMLINK-FILE include/rte_eth_null.h*
> *  INSTALL-LIB librte_pmd_null.a*
> *== Build drivers/net/pcap*
> *  CC rte_eth_pcap.o*
> */home/osboxes/Downloads/dpdk-16.04/drivers/net/pcap/rte_eth_pcap.c:47:18:
> fatal error: pcap.h: No such file or directory*
> *compilation terminated.*
> *make[4]: *** [rte_eth_pcap.o] Error 1*
> *make[3]: *** [pcap] Error 2*
> *make[2]: *** [net] Error 2*
> *make[1]: *** [drivers] Error 2*
> *make: *** [all] Error 2*
> *root at osboxes:/home/osboxes/Downloads/dpdk-16.04#*
>
> Thanks and Regards
>  Sheroo Pratap
>


[dpdk-dev] Suggestions for the dpdk stable tree

2016-05-20 Thread Christian Ehrhardt
Hi,
I guess over time/releases less people mind the 2.2-stable.
But I still see a lot of people referring to 2.2 - so why not giving this
thread a ping again.

ack / nack / opinions ?

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Thu, Mar 24, 2016 at 8:14 AM, Christian Ehrhardt <
christian.ehrhardt at canonical.com> wrote:

> Hi,
> there is a stable tree for dpdk 2.2 at http://dpdk.org/browse/dpdk-stable/
> But it is still at the 2.2 Release status.
>
> I was working on testing 2.2 and needed to pull in some patches from post
> 2.2 release to fix issues.
> After that I started to "scan" the git log what else might apply to a
> clean and uncritical addition to a dpdp-2.2 as "stability backports".
>
> I had a focus on identifying:
> - only patches to components that are enabled in our packaging (well that
> is close to the default conf so it should be ok for you as well)
> - issues that have a real chance to occur
> - patches that are small, so review is not overly complex
> - apply without manual adaption (offset ok to some extend, no fuzz, no
> manual fix)
> I realized that I should suggest that list of patches to you to consider
> adding it to the stable tree.
> So therefore for you to consider:
>
>
> commit d3a274ce9dee28118b8647e0db72ef0f4b6a6323
>
> app/testpmd: handle SIGINT and SIGTERM
>
> commit 308df2bfba3d238fc1d2d16cc10c84681803b408
>
> Handle SIGINT and SIGTERM in l3fwd.
>
> commit da82ee17e6ea99bf2931383ac33b0caccaaaefce
>
> tools: fix unbinding failure handling
>
> commit 16c1814c802c205f6d3c32e3d3d10de9f87e7f22
>
> tools: support Python 3 in bind script
>
> commit bb9f408550d13af6c1da104b0d9d9b9837f69bde
>
> tools: support binding to built-in kernel modules
>
>
> commit 6e7caa1ad9d597fed0a49468af25ae6e68b8c443
>
> eal/linux: support built-in kernel modules
>
>
> commit 86f36ff9578b5f3d697c8fcf6072dcb70e2b246f
>
> mempool: fix leak when creation fails
>
>
> commit ca67ed289a76f38d6c4a4021985a36eaf1d77e28
>
> vhost: fix leak of fds and mmaps
>
>
> commit fa11a8a7251e14eca0a4190128732386f13551bd
>
> port: fix crash for ring writer nodrop
>
> commit 04f366906ab395c8047bebfc1ddea244dfcb40f5
>
> port: fix crash for ethdev writer nodrop
>
>
> commit c7a4ff80722e9237a4c504106d21ba5ca27d8df2
>
> i40e: fix overflow
>
> commit 097e920c32bf19cf918cc071525f33b0abdeebaf
>
> i40e: fix inverted check for no refcount
>
> commit 330aa319382aec9ea595f9ebcb9a3c44647ad66c
>
> i40e: fix VLAN filtering
>
> commit 9f44dd3d8ad447c7f797a9564d30a15e5ab7f72b
>
> i40e/base: fix missing check for stopped admin queue
>
> commit 8a8807369ffafef90c410279b4b2645d2d7a7483
>
> i40e/base: fix driver load failure
>
>
> commit 7656a546c0609f3205558a5d48352c82607d38d3
>
> fm10k: fix VLAN flag in scattered Rx
>
>
> commit c6fb0e55585206a89f6db396de860e6e844cad06
>
> pcap: fix captured frame length
>
>
> commit 6e02723754fb2b341701ac438486b2dfea98b523
>
> bonding: fix detach of bonded device
>
> commit df3e8ad73f4c92b4eb8f49ff33271d4a09e6a04a
>
> bonding: fix detach of slave devices
>
> commit 786c990a11e6e6592dfdee02840877070aa3a79a
>
> bonding: copy entire config structure in mode 4
>
> commit 6698820b5f6d955b6af2b916e1074db236d4f5a2
>
> bonding: do not ignore multicast in mode 4
>
> commit 8997a10bfcad789d000debaac4cd85cd3db57997
>
> bonding: fix active slaves with no primary
>
> commit 7a7122edf1c8d63e516d1b2c2eff6fa9b54e0f82
>
> bonding: do not activate slave twice
>
> commit 2186fff3675d4e774cecc8f918c05063e0367d28
>
> bonding: fix crash when no slave device
>
>
> commit 3b1e3e4e362453df8cecbc6d481444be8b84326e
>
> virtio: fix descriptors pointing to the same buffer
>
> commit c680a4a88c4312068f60937a7ba51eac8211c9a6
>
> virtio: fix crash in statistics functions
>
> commit 9a0615af7746485d73d10561cc0743bc2fcd4bf7
>
> virtio: fix restart
>
>
> Christian Ehrhardt
> Software Engineer, Ubuntu Server
> Canonical Ltd
>


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

2016-05-19 Thread 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.

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)

#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.


I'm still digging into that concept of using a linker script for all of
that and some of the new implications by that. And eventually thing "work",
but this linking at least feels wrong to me.


So I wanted to ask - is that intentional - or should that be fixed?
If it should be fixed are there obvious suggestions where/how?
And if it is intentional - could one be so nice to elaborate it a bit for
me - thanks in advance.


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] [RFC] eal: provide option to set vhost_user socket owner/permissions

2016-04-26 Thread Christian Ehrhardt
Thanks,
great that you added more on CC for a wider discussion - I think that is
the only right way to go.

Just to "defend" a bit - solution a) was created under the special
circumstance that I wanted a workaround that would work today.
But that is/was special to what I package with DPDK 2.2 + OVS 2.5 as of
today - and therefore was the right place for a fast interim fix for me.
I totally agree that the A in EAL was meant for abstraction and we might
want to avoid vhost specific things in there that in the long run.

I like your suggestion of a new API as a proper long term solution, but I
don't feel deeply enough involved yet on the API level to give it any
judgement.
So I look forward for more opinions on it.

P.S. the patch bot hit me hard with 2 pages of space/bracket issues, sorry
for that - but it was only meant as RFC after all :-)


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Tue, Apr 26, 2016 at 6:16 AM, Yuanhan Liu 
wrote:

> On Mon, Apr 25, 2016 at 11:18:16AM +0200, Christian Ehrhardt wrote:
> > The API doesn't hold a way to specify a owner/permission set for
> vhost_user
> > created sockets.
>
> Yes, it's kind of like a known issue. So, thanks for bringing it, with
> a solution, for dicussion (cc'ed more people).
>
> > I don't even think an API change would make that much sense.
> >
> > Projects consuming DPDK start to do 'their own workarounds' like
> openvswitch
> > https://patchwork.ozlabs.org/patch/559043/
> > https://patchwork.ozlabs.org/patch/559045/
> > But for this specific example they are blocked/stalled behind a bigger
> > rework (https://patchwork.ozlabs.org/patch/604898/).
> > Also one could ask why each project would need their own workaround.
> >
> > At the same time - as I want it for existing code linking against DPDK I
> > wanted to avoid changing API/ABI. That way I want to provide something
> existing
> > users could utilize. So I created a DPDK EAL commandline option based
> ideas in
> > the former patches.
> >
> > For myself I consider this a nice interim solution for existing released
> > Openvswitch+DPDK solution. And I intend to put it as delta into the DPDK
> 2.2
> > currently packaged in Ubuntu to get it working more smoothly with
> > openvswitch 2.5.
> >
> > But I'd be interested if DPDK in general would be interested in:
> > a) an approach like this?
>
> You were trying to add a vhost specific stuff as EAL command option,
> which is something we might should try to avoid.
>
> > b) would prefer a change of the API?
>
> Adding a new option to the current register API might will not work well,
> either. It gives you no ability to do a dynamic change later. I mean,
> taking OVS as an example, OVS provides you the flexible ability to do all
> kinds of configuration in a dynamic way, say number of rx queues. If we
> do the permissions setup in the register time, there would be no way to
> change it later, right?
>
> So, I'm thinking that we may could add a new API for that? It then would
> allow applications to change it at anytime.
>
> > c) consider it an issue of consuming projects and let them take care?
>
> It's not exactly an issue of consuming projects; we created the socket
> file after all.
>
> And I'd like to hear what others would say.
>
> Thanks.
>
> --yliu
>


[dpdk-dev] Git repository might be down

2016-04-25 Thread Christian Ehrhardt
Hi,
we were discussing on #DPDK about the same.

So far it seems via git:// it fails, but http works for now - thanks to
David Marchland for identifying this fallback.


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Mon, Apr 25, 2016 at 11:59 AM, Tetsuya Mukawa  wrote:

> Hi,
>
> I've faced below error while connecting to DPDK git repository.
>
> $ git clone git://dpdk.org/dpdk
> Cloning into 'dpdk'...
> fatal: read error: Connection reset by peer
>
> It might be caused by my environment, but report it just in case.
>
> Thanks,
> Tetsuya
>


[dpdk-dev] [RFC] eal: provide option to set vhost_user socket owner/permissions

2016-04-25 Thread Christian Ehrhardt
The API doesn't hold a way to specify a owner/permission set for vhost_user
created sockets. I don't even think an API change would make that much sense.

Projects consuming DPDK start to do 'their own workarounds' like openvswitch
https://patchwork.ozlabs.org/patch/559043/
https://patchwork.ozlabs.org/patch/559045/
But for this specific example they are blocked/stalled behind a bigger
rework (https://patchwork.ozlabs.org/patch/604898/).
Also one could ask why each project would need their own workaround.

At the same time - as I want it for existing code linking against DPDK I
wanted to avoid changing API/ABI. That way I want to provide something existing
users could utilize. So I created a DPDK EAL commandline option based ideas in
the former patches.

For myself I consider this a nice interim solution for existing released
Openvswitch+DPDK solution. And I intend to put it as delta into the DPDK 2.2
currently packaged in Ubuntu to get it working more smoothly with
openvswitch 2.5.

But I'd be interested if DPDK in general would be interested in:
a) an approach like this?
b) would prefer a change of the API?
c) consider it an issue of consuming projects and let them take care?

Signed-off-by: Christian Ehrhardt 
---
 doc/guides/testpmd_app_ug/run_app.rst|  19 +++
 lib/librte_eal/common/eal_common_options.c   |   4 +
 lib/librte_eal/common/eal_internal_cfg.h |   2 +
 lib/librte_eal/common/eal_options.h  |   4 +
 lib/librte_eal/common/include/rte_eal.h  |   5 +
 lib/librte_eal/linuxapp/eal/eal.c| 182 +++
 lib/librte_vhost/vhost_user/vhost-net-user.c |   4 +
 7 files changed, 220 insertions(+)

diff --git a/doc/guides/testpmd_app_ug/run_app.rst 
b/doc/guides/testpmd_app_ug/run_app.rst
index f605564..24c9c01 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -156,6 +156,25 @@ See the DPDK Getting Started Guides for more information 
on these options.

 Use malloc instead of hugetlbfs.

+*   ``--vhost-owner``
+
+ When creating vhost_user sockets change owner and group to the specified 
value.
+ This can be given as ``user:group``, but also only ``user`` or ``:group`` 
are supported.
+
+Examples::
+
+   --vhost-owner 'libvirt-qemu:kvm'
+   --vhost-owner 'libvirt-qemu'
+   --vhost-owner ':kvm'
+
+*   ``--vhost-perm``
+
+When creating vhost_user sockets set them up with these permissions.
+
+For example::
+
+   --vhost-perm '0664'
+

 Testpmd Command-line Options
 
diff --git a/lib/librte_eal/common/eal_common_options.c 
b/lib/librte_eal/common/eal_common_options.c
index 2b418d5..073198b 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -95,6 +95,8 @@ eal_long_options[] = {
{OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM},
{OPT_VMWARE_TSC_MAP,0, NULL, OPT_VMWARE_TSC_MAP_NUM   },
{OPT_XEN_DOM0,  0, NULL, OPT_XEN_DOM0_NUM },
+   {OPT_VHOST_OWNER,   1, NULL, OPT_VHOST_OWNER_NUM  },
+   {OPT_VHOST_PERM,1, NULL, OPT_VHOST_PERM_NUM   },
{0, 0, NULL, 0}
 };

@@ -153,6 +155,8 @@ eal_reset_internal_config(struct internal_config 
*internal_cfg)
 #endif
internal_cfg->vmware_tsc_map = 0;
internal_cfg->create_uio_dev = 0;
+   internal_cfg->vhost_sock_owner = NULL;
+   internal_cfg->vhost_sock_perm = NULL;
 }

 static int
diff --git a/lib/librte_eal/common/eal_internal_cfg.h 
b/lib/librte_eal/common/eal_internal_cfg.h
index 5f1367e..bdf34e3 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -83,6 +83,8 @@ struct internal_config {
volatile enum rte_intr_mode vfio_intr_mode;
const char *hugefile_prefix;  /**< the base filename of hugetlbfs 
files */
const char *hugepage_dir; /**< specific hugetlbfs directory to 
use */
+   const char *vhost_sock_owner; /**< owner:group of vhost_user 
sockets */
+   const char *vhost_sock_perm;  /**< permissions of vhost_user 
sockets */

unsigned num_hugepage_sizes;  /**< how many sizes on this system */
struct hugepage_info hugepage_info[MAX_HUGEPAGE_SIZES];
diff --git a/lib/librte_eal/common/eal_options.h 
b/lib/librte_eal/common/eal_options.h
index a881c62..1161083 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -83,6 +83,10 @@ enum {
OPT_VMWARE_TSC_MAP_NUM,
 #define OPT_XEN_DOM0  "xen-dom0"
OPT_XEN_DOM0_NUM,
+#define OPT_VHOST_OWNER   "vhost-owner"
+   OPT_VHOST_OWNER_NUM,
+#define OPT_VHOST_PERM"vhost-perm"
+   OPT_VHOST_PERM_NUM,
OPT_LONG_MAX_NUM
 };

diff --git a/lib/librte_eal/common/include/rte_eal.h 
b/lib/librte_eal/common/

[dpdk-dev] Memory leak when adding/removing vhost_user ports

2016-04-21 Thread Christian Ehrhardt
Thanks Ilya,
yeah we usually wait for the point releases as they undergo some extra
testing and verification.
.1 shouldn't be too much into the future I guess.
Thanks a lot for identifying.

That said, I'd still go on with Yuanhan to finalize the dpdk side leak fix
we identified, so we eventually get it committed.
So Yuanhan, what do you think of my last revised version of your patch for
upstream DPDK (there with the vhost_destroy_device then)?
I mean it is essentially your patch plus a bit of polishing, not mine so I
don't feel entitled to submit it as mine :-)

Kind Regards,
Christian


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Thu, Apr 21, 2016 at 1:01 PM, Ilya Maximets 
wrote:

> Hi, Christian.
> You're, likely, using tar archive with openvswitch from openvswitch.org.
> It doesn't contain many bug fixes from git/branch-2.5 unfortunately.
>
> The problem that you are facing has been solved in branch-2.5 by
>
> commit d9df7b9206831631ddbd90f9cbeef1b4fc5a8e89
> Author: Ilya Maximets 
> Date:   Thu Mar 3 11:30:06 2016 +0300
>
> netdev-dpdk: Fix memory leak in netdev_dpdk_vhost_destruct().
>
> Fixes: 4573fbd38fa1 ("netdev-dpdk: Add vhost-user multiqueue support")
> Signed-off-by: Ilya Maximets 
> Acked-by: Flavio Leitner 
> Acked-by: Daniele Di Proietto 
>
> Best regards, Ilya Maximets.
>
> > I assume there is a leak somewhere on adding/removing vhost_user ports.
> > Although it could also be "only" a fragmentation issue.
> >
> > Reproduction is easy:
> > I set up a pair of nicely working OVS-DPDK connected KVM Guests.
> > Then in a loop I
> >- add up to more 512 ports
> >- test connectivity between the two guests
> >- remove up to 512 ports
> >
> > Depending on memory and the amount of multiqueue/rxq I use it seems to
> > slightly change when exactly it breaks. But for my default setup of 4
> > queues and 5G Hugepages initialized by DPDK it always breaks at the sixth
> > iteration.
> > Here a link to the stack trace indicating a memory shortage (TBC):
> > https://launchpadlibrarian.net/253916410/apport-retrace.log
> >
> > Known Todos:
> > - I want to track it down more, and will try to come up with a non
> > openvswitch based looping testcase that might show it as well to simplify
> > debugging.
> > - in use were Openvswitch-dpdk 2.5 and DPDK 2.2; Retest with DPDK 16.04
> and
> > Openvswitch master is planned.
> >
> > I will go on debugging this and let you know, but I wanted to give a
> heads
> > up to everyone.
> > In case this is a known issue for some of you please let me know.
> >
> > Kind Regards,
> > Christian Ehrhardt
> > Software Engineer, Ubuntu Server
> > Canonical Ltd
> >
> > P.S. I think it is a dpdk issue, but adding Daniele on CC to represent
> > ovs-dpdk as well.
>


[dpdk-dev] Memory leak when adding/removing vhost_user ports

2016-04-21 Thread Christian Ehrhardt
Hi,
I can follow your argument that - and agree that in this case the leak
can't be solved your patch.
Still I found it useful to revise it along our discussion as eventually it
will still be a good patch to have.
I followed your suggestion and found:
- rte_vhost_driver_register callocs vserver (implies fh=0)
- later when on init when getting the callback to vserver_new_vq_conn it
would get set by ops->new_device(vdev_ctx);
- but as you pointed out that could be fh = 0 for the first
- so I initialized vserver->fh with -1 in rte_vhost_driver_register - that
won't ever be a real fh.
- later on get_config_ll_entry won't find a device with that then on the
call by destroy_device.
- so the revised patch currently in use (still for DPDK 2.2) can be found
here http://paste.ubuntu.com/15961394/

Also as you requested I tried with no guest attached at all - that way I
can still reproduce it.
Here is a new stacktrace, but to me it looks the same
http://paste.ubuntu.com/15961185/
Also as you asked before a log of the vswitch, but it is 895MB since a lot
of messages repeat on port add/remove.
Even compressed still 27MB - I need to do something about verbosity there.
Also the system journal of the same time.
Therefore I only added links to bz2 files.
The crash is at "2016-04-21T07:54:47.782Z" in the logs.
=>
http://people.canonical.com/~paelzer/ovs-dpdk-vhost-add-remove-leak/mem-leak-addremove.journal.bz2
=>
http://people.canonical.com/~paelzer/ovs-dpdk-vhost-add-remove-leak/ovs-vswitchd.log.bz2

Kind Regards,
Christian Ehrhardt


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Thu, Apr 21, 2016 at 7:54 AM, Yuanhan Liu 
wrote:

> On Wed, Apr 20, 2016 at 08:18:49AM +0200, Christian Ehrhardt wrote:
> > On Wed, Apr 20, 2016 at 7:04 AM, Yuanhan Liu <
> yuanhan.liu at linux.intel.com>
> > wrote:
> >
> > On Tue, Apr 19, 2016 at 06:33:50PM +0200, Christian Ehrhardt wrote:
> >
> > [...]
> >
> > > With that applied one (and only one) of my two guests looses
> connectivity
> > after
> > > removing the ports the first time.
> >
> > Yeah, that's should be because I invoked the "->destroy_device()"
> > callback.
> >
> >
> > Shouldn't that not only destroy the particular vhost_user device I
> remove?
>
> I assume the "not" is typed wrong here, then yes. Well, it turned
> out that I accidentally destroyed the first guest (with id 0) with
> following code:
>
> ctx.fh = g_vhost_server.server[i]->fh;
> vhost_destroy_device(ctx);
>
> server[i]->fh is initialized with 0 when no connection is established
> (check below for more info), and the first id is started with 0. Anyway,
> this could be fixed easily.
>
> > See below for some better details on the test to clarify that.
> >
> >
> > BTW, I'm curious how do you do the test? I saw you added 256 ports,
> but
> > with 2 guests only? So, 254 of them are idle, just for testing the
> > memory leak bug?
> >
> >
> > Maybe I should describe it better:
> > 1. Spawn some vhost-user ports (40 in my case)
> > 2. Spawn a pair of guests that connect via four of those ports per guest
> > 3. Guests only intialize one of that vhost_user based NICs
> > 4. check connectivity between guests via the vhost_user based connection
> > (working at this stage)
> > LOOP 5-7:
> >5. add ports 41-512
> >6. remove  ports 41-512
> >7. check connectivity between guests via the vhost_user based
> connection
>
> Yes, it's much clearer now. Thanks.
>
> I then don't see it's a leak from DPDK vhost-user, at least not the leak
> on "struct virtio_net" I have mentioned before. "struct virito_net" will
> not even be allocated for those ports never used (ports 41-512 in your
> case),
> as it will be allocated only when there is a connection established, aka,
> a guest is connected.
>
> BTW, will you be able to reproduce it without any connections? Say, all
> 512 ports are added, and then deleted.
>
> Thanks.
>
> --yliu
>
> >
> > So the vhost_user ports the guests are using are never deleted.
> > Only some extra (not even used) ports are added in the loop to
> search
> > for potential leaks over a longer lifetime of an openvswitch-dpdk based
> > solution.
> >
>


[dpdk-dev] Memory leak when adding/removing vhost_user ports

2016-04-20 Thread Christian Ehrhardt
On Wed, Apr 20, 2016 at 7:04 AM, Yuanhan Liu 
wrote:

> On Tue, Apr 19, 2016 at 06:33:50PM +0200, Christian Ehrhardt wrote:
>
[...]

> > With that applied one (and only one) of my two guests looses
> connectivity after
> > removing the ports the first time.
>
> Yeah, that's should be because I invoked the "->destroy_device()"
> callback.
>

Shouldn't that not only destroy the particular vhost_user device I remove?
See below for some better details on the test to clarify that.

BTW, I'm curious how do you do the test? I saw you added 256 ports, but
> with 2 guests only? So, 254 of them are idle, just for testing the
> memory leak bug?
>

Maybe I should describe it better:
1. Spawn some vhost-user ports (40 in my case)
2. Spawn a pair of guests that connect via four of those ports per guest
3. Guests only intialize one of that vhost_user based NICs
4. check connectivity between guests via the vhost_user based connection
(working at this stage)
LOOP 5-7:
   5. add ports 41-512
   6. remove  ports 41-512
   7. check connectivity between guests via the vhost_user based connection

So the vhost_user ports the guests are using are never deleted.
Only some extra (not even used) ports are added in the loop to
search for potential leaks over a longer lifetime of an openvswitch-dpdk
based solution.


[dpdk-dev] basic forwarding example not working

2016-04-20 Thread Christian Ehrhardt
Hi,
I think your log is cut short.
At least in what you posted I don't see the "card skipped" issue anymore.
Also I don't see the segfault at the end anymore.
Actually there is no error left in this second log.

I never used basicfwd so far - maybe it is something special to it that
breaks your case.
If it follows the normal EAL commandline I'd recommend to also add
something for memory like --socket-mem 2048.
Well without it should just try to grab everything, but I prefer to specify
things.

Could you try to run the testpmd or l2fwd programs instead - IMHO these are
more commonly used (=usually work better).


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Tue, Apr 19, 2016 at 7:30 PM, Subbu CS 
wrote:

> Hi Christian,
>
> I unbind two interfaces and then bound them to uio_pci_generic driver.
>
> [root at localhost tools]# ./dpdk_nic_bind.py --status
>
> Network devices using DPDK-compatible driver
> 
> :00:0a.0 'Virtio network device' drv=uio_pci_generic unused=igb_uio
> :00:0b.0 'Virtio network device' drv=uio_pci_generic unused=igb_uio
>
> Network devices using kernel driver
> ===
> :00:03.0 'RTL-8100/8101L/8139 PCI Fast Ethernet Adapter' if=ens3
> drv=8139cp unused=igb_uio,uio_pci_generic *Active*
> :00:08.0 'Virtio network device' if= drv=virtio-pci
> unused=igb_uio,uio_pci_generic
> :00:09.0 'Virtio network device' if= drv=virtio-pci
> unused=igb_uio,uio_pci_generic
>
> Other network devices
> =
> 
>
>
>
> But Still I get the same error
>
>
> [root at localhost build]# ./basicfwd -c 1 -n 2
> EAL: Detected lcore 0 as core 0 on socket 0
> EAL: Support maximum 128 logical core(s) by configuration.
> EAL: Detected 1 lcore(s)
> EAL: Probing VFIO support...
> EAL: Module /sys/module/vfio_pci not found! error 2 (No such file or
> directory)
> EAL: VFIO modules not loaded, skipping VFIO support...
> EAL: Setting up physically contiguous memory...
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f189d60 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f189d20 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f189ce0 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f189ca0 (size = 0x20)
> EAL: Ask a virtual area of 0x2740 bytes
> EAL: Virtual area found at 0x7f187540 (size = 0x2740)
> EAL: Ask a virtual area of 0x4a0 bytes
> EAL: Virtual area found at 0x7f187080 (size = 0x4a0)
> EAL: Ask a virtual area of 0x120 bytes
> EAL: Virtual area found at 0x7f186f40 (size = 0x120)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f186f00 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f186ec0 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f186e80 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f186e40 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f186e00 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f186dc0 (size = 0x20)
> EAL: Requesting 370 pages of size 2MB from socket 0
> EAL: TSC frequency is ~1696076 KHz
> EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using
> unreliable clock cycles !
> EAL: Master lcore 0 is ready (tid=9e7a58c0;cpuset=[0])
> EAL: PCI device :00:08.0 on NUMA socket -1
> EAL:   probe driver: 1af4:1000 rte_virtio_pmd
> EAL:   Not managed by a supported kernel driver, skipped
> Segmentation fault
>
>
>
> On Tue, Apr 19, 2016 at 9:53 PM, Christian Ehrhardt <
> christian.ehrhardt at canonical.com> wrote:
>
>> Hi,
>> for virtio-pci devices you should either just unbind them or bind them to
>> one of the generic drivers like uio_pci_generic.
>> At least I never saw igb_uio on virtio-pci devices.
>>
>> There might be more complete answers, but I think that should get you
>> going.
>>
>>
>> Christian Ehrhardt
>> Software Engineer, Ubuntu Server
>> Canonical Ltd
>>
>> On Tue, Apr 19, 2016 at 6:00 PM, Subbu CS 
>> wrote:
>>
>>> I am using two virtio network adapter on VM on KVM host.
>>>
>>> As  in following output, I have bound both interface with igb_uio driver
>>>
>>> ./dpdk_nic_bind.py --bind=igb_uio 00:0a.0
>>> ./dpdk_nic_bind.py --bind=igb_uio 00:0b

[dpdk-dev] Memory leak when adding/removing vhost_user ports

2016-04-19 Thread Christian Ehrhardt
Hi,
thanks for the patch.
I backported it this way to my DPDK 2.2 based environment for now (see
below):

With that applied one (and only one) of my two guests looses connectivity
after removing the ports the first time.
No traffic seems to pass, setting the device in the guest down/up doesn't
get anything.
But It isn't totally broken - stopping/starting the guest gets it working
again.
So openvswitch/dpdk is still somewhat working - it just seems the guest
lost something, after tapping on that vhost_user interface again it works.

I will check tomorrow and let you know:
- if I'm more lucky with that patch on top of 16.04
- if it looses connectivity after the first or a certain amount of port
removes

If you find issues with my backport adaption let me know.


---

Backport and reasoning:

new fix relies on a lot of new code, vhost_destroy_device looks totally
different from the former destroy_device.
History on todays function content:
  4796ad63 - original code moved from examples to lib
  a90ca1a1 - this replaces ops->destroy_device with vhost_destroy_device
  71dc571e - simple check against null pointers
  45ca9c6f - this changed the code from linked list to arrays

  New code cleans with:
  notify_ops->destroy_device (callback into the parent)
  cleanup_device was existing before even in 2.2 code
  free_device as well existing before even in 2.2 code
  Old code cleans with:
  notify_ops->destroy_device - still there
  rm_config_ll_entry -> eventually calls cleanup_device and free_device
(just in the more complex linked list way)

So the "only" adaption for backporting needed is to replace
vhost_destroy_device
with ops->destroy_device(ctx)

Index: dpdk/lib/librte_vhost/vhost_user/vhost-net-user.c
===
--- dpdk.orig/lib/librte_vhost/vhost_user/vhost-net-user.c
+++ dpdk/lib/librte_vhost/vhost_user/vhost-net-user.c
@@ -310,6 +310,7 @@ vserver_new_vq_conn(int fd, void *dat, _
  }

  vdev_ctx.fh = fh;
+ vserver->fh = fh;
  size = strnlen(vserver->path, PATH_MAX);
  ops->set_ifname(vdev_ctx, vserver->path,
  size);
@@ -516,6 +517,11 @@ rte_vhost_driver_unregister(const char *

  for (i = 0; i < g_vhost_server.vserver_cnt; i++) {
  if (!strcmp(g_vhost_server.server[i]->path, path)) {
+ struct vhost_device_ctx ctx;
+
+ ctx.fh = g_vhost_server.server[i]->fh;
+ ops->destroy_device(ctx);
+
  fdset_del(_vhost_server.fdset,
  g_vhost_server.server[i]->listenfd);

Index: dpdk/lib/librte_vhost/vhost_user/vhost-net-user.h
===
--- dpdk.orig/lib/librte_vhost/vhost_user/vhost-net-user.h
+++ dpdk/lib/librte_vhost/vhost_user/vhost-net-user.h
@@ -43,6 +43,7 @@
 struct vhost_server {
  char *path; /**< The path the uds is bind to. */
  int listenfd; /**< The listener sockfd. */
+ uint32_t fh;
 };

 /* refer to hw/virtio/vhost-user.c */




Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Mon, Apr 18, 2016 at 8:14 PM, Yuanhan Liu 
wrote:

> On Mon, Apr 18, 2016 at 10:46:50AM -0700, Yuanhan Liu wrote:
> > On Mon, Apr 18, 2016 at 07:18:05PM +0200, Christian Ehrhardt wrote:
> > > I assume there is a leak somewhere on adding/removing vhost_user ports.
> > > Although it could also be "only" a fragmentation issue.
> > >
> > > Reproduction is easy:
> > > I set up a pair of nicely working OVS-DPDK connected KVM Guests.
> > > Then in a loop I
> > >- add up to more 512 ports
> > >- test connectivity between the two guests
> > >- remove up to 512 ports
> > >
> > > Depending on memory and the amount of multiqueue/rxq I use it seems to
> > > slightly change when exactly it breaks. But for my default setup of 4
> > > queues and 5G Hugepages initialized by DPDK it always breaks at the
> sixth
> > > iteration.
> > > Here a link to the stack trace indicating a memory shortage (TBC):
> > > https://launchpadlibrarian.net/253916410/apport-retrace.log
> > >
> > > Known Todos:
> > > - I want to track it down more, and will try to come up with a non
> > > openvswitch based looping testcase that might show it as well to
> simplify
> > > debugging.
> > > - in use were Openvswitch-dpdk 2.5 and DPDK 2.2; Retest with DPDK
> 16.04 and
> > > Openvswitch master is planned.
> > >
> > > I will go on debugging this and let you know, but I wanted to give a
> heads
> > > up to everyone.
> >
> > Thanks for the report.
> >
> > > In case this is a known issue for some of you please let me know.
> >
> > Yeah, it might be. I'm wondering that virtio_net struct is not freed.
> > It will be freed only (if I'm not mis

[dpdk-dev] basic forwarding example not working

2016-04-19 Thread Christian Ehrhardt
Hi,
for virtio-pci devices you should either just unbind them or bind them to
one of the generic drivers like uio_pci_generic.
At least I never saw igb_uio on virtio-pci devices.

There might be more complete answers, but I think that should get you going.


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Tue, Apr 19, 2016 at 6:00 PM, Subbu CS 
wrote:

> I am using two virtio network adapter on VM on KVM host.
>
> As  in following output, I have bound both interface with igb_uio driver
>
> ./dpdk_nic_bind.py --bind=igb_uio 00:0a.0
> ./dpdk_nic_bind.py --bind=igb_uio 00:0b.0
>
> [root at localhost tools]# ./dpdk_nic_bind.py --status
>
> Network devices using DPDK-compatible driver
> 
> :00:0a.0 'Virtio network device' drv=igb_uio unused=
> :00:0b.0 'Virtio network device' drv=igb_uio unused=
>
> Network devices using kernel driver
> ===
> :00:03.0 'RTL-8100/8101L/8139 PCI Fast Ethernet Adapter' if=ens3
> drv=8139cp unused=igb_uio *Active*
> :00:08.0 'Virtio network device' if= drv=virtio-pci unused=igb_uio
> :00:09.0 'Virtio network device' if= drv=virtio-pci unused=igb_uio
>
> Other network devices
> =
> 
>
>
> But when I run basic fowarding example from skeleton, I get following error
>
> ./basicfwd -c 1 -n 2
> EAL: Detected lcore 0 as core 0 on socket 0
> EAL: Support maximum 128 logical core(s) by configuration.
> EAL: Detected 1 lcore(s)
> EAL: Probing VFIO support...
> EAL: Module /sys/module/vfio_pci not found! error 2 (No such file or
> directory)
> EAL: VFIO modules not loaded, skipping VFIO support...
> EAL: Setting up physically contiguous memory...
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f75de40 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f75de00 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f75ddc0 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f75dd80 (size = 0x20)
> EAL: Ask a virtual area of 0x2740 bytes
> EAL: Virtual area found at 0x7f75b620 (size = 0x2740)
> EAL: Ask a virtual area of 0x4a0 bytes
> EAL: Virtual area found at 0x7f75b160 (size = 0x4a0)
> EAL: Ask a virtual area of 0x120 bytes
> EAL: Virtual area found at 0x7f75b020 (size = 0x120)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f75afe0 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f75afa0 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f75af60 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f75af20 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f75aee0 (size = 0x20)
> EAL: Ask a virtual area of 0x20 bytes
> EAL: Virtual area found at 0x7f75aea0 (size = 0x20)
> EAL: Requesting 370 pages of size 2MB from socket 0
> EAL: TSC frequency is ~1696081 KHz
> EAL: WARNING: cpu flags constant_tsc=yes nonstop_tsc=no -> using unreliable
> clock cycles !
> EAL: Master lcore 0 is ready (tid=df5298c0;cpuset=[0])
> EAL: PCI device :00:08.0 on NUMA socket -1
> EAL:   probe driver: 1af4:1000 rte_virtio_pmd
> EAL:   Not managed by a supported kernel driver, skipped
> Segmentation fault
>
> Please let me know what is the correct method to run the basic forwarding
> application.
>


[dpdk-dev] Memory leak when adding/removing vhost_user ports

2016-04-18 Thread Christian Ehrhardt
I assume there is a leak somewhere on adding/removing vhost_user ports.
Although it could also be "only" a fragmentation issue.

Reproduction is easy:
I set up a pair of nicely working OVS-DPDK connected KVM Guests.
Then in a loop I
   - add up to more 512 ports
   - test connectivity between the two guests
   - remove up to 512 ports

Depending on memory and the amount of multiqueue/rxq I use it seems to
slightly change when exactly it breaks. But for my default setup of 4
queues and 5G Hugepages initialized by DPDK it always breaks at the sixth
iteration.
Here a link to the stack trace indicating a memory shortage (TBC):
https://launchpadlibrarian.net/253916410/apport-retrace.log

Known Todos:
- I want to track it down more, and will try to come up with a non
openvswitch based looping testcase that might show it as well to simplify
debugging.
- in use were Openvswitch-dpdk 2.5 and DPDK 2.2; Retest with DPDK 16.04 and
Openvswitch master is planned.

I will go on debugging this and let you know, but I wanted to give a heads
up to everyone.
In case this is a known issue for some of you please let me know.

Kind Regards,
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

P.S. I think it is a dpdk issue, but adding Daniele on CC to represent
ovs-dpdk as well.


[dpdk-dev] compile error on ubuntu 14.4.4 kernel 4.2.0-27-generic in qemu

2016-04-18 Thread Christian Ehrhardt
It is hard for me to spot what exactly is missing, but while it was never
intended for trusty the version we have for xenial should get you going.

# all kind of dependencies
sudo apt-get install build-essential ubuntu-dev-tools debhelper dh-python
dh-systemd doxygen graphviz inkscape libcap-dev libpcap-dev libxen-dev
libxenstore3.0 python python-sphinx texlive-fonts-recommended
texlive-latex-extra
# get the version we currently package for xenial
pull-lp-source dpdk xenial
# run the packaged build which takes care of most enabling/disabling and
such
cd dpdk-2.2.0
./debian/rules build

Also as you need at least some SSE (level depends on your config) which
isn't in the guest by default.
Take a look at this - although I'm not 100% sure how mouch of it works back
on trusty.
https://help.ubuntu.com/16.04/serverguide/DPDK.html#dpdk-in-guest

>From there you can derive into whatever you need.
Or just take the build process as your starting point for your own.
On the other hand I encourage you to go to 16.04 and just use the packaged
version if that would suit your needs.


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Sun, Apr 17, 2016 at 8:41 PM, Masaru OKI <oki+dpdk at chocot.jp> wrote:

> On 2016/04/18 3:27, Sharath wrote:
>
>> I am facing feew compile errors while compiling dpdk. The env is ubuntu
>> running as an VM, VM is started by qemu. How do I fix the compile errors?
>>
>
> Default qemu virtual cpu does not support SSE4.2.
> Try qemu -cpu help, and specify your reasonable cpu.
>


[dpdk-dev] [PATCH] lpm: fix freeing of rules_tbl in rte_lpm_free_v20

2016-04-12 Thread Christian Ehrhardt
Back then when we fixed the missing free lpm I was to quickly to say yes
if it applies not only to the lpm6 but also to all of the lpm code.

It turned out to not apply to all of them. In rte_lpm_create_v20 there
is an unexpected fused allocation:
mem_size = sizeof(*lpm) + (sizeof(lpm->rules_tbl[0]) * max_rules);
[...]
lpm = (struct rte_lpm_v20 *)rte_zmalloc_socket(mem_name,mem_size,
   RTE_CACHE_LINE_SIZE, socket_id);

That causes lpm->rules_tbl not to have an own struct malloc_elem that
can be derived via RTE_PTR_SUB(data, MALLOC_ELEM_HEADER_LEN) in
malloc_elem_from_data.
Due to that the rte_lpm_free_v20 accidentially misderives the elem and
assumes it is ELEM_FREE triggering in malloc_elem_free
if (!malloc_elem_cookies_ok(elem) || elem->state !=
return -1;

While it seems counter-intuitive the way to properly remove rules_tbl in
the old fused allocation style of rte_lpm_free_v20 is to not remove it.

The newer rte_lpm_free_v1604 is safe because in rte_lpm_create_v1604
rules_tbl is a separate allocation.

Fixes: d4c18f0a1d5d ("lpm: fix missing free")

Signed-off-by: Christian Ehrhardt 
---
 lib/librte_lpm/rte_lpm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
index 8bdf606..6f65d1c 100644
--- a/lib/librte_lpm/rte_lpm.c
+++ b/lib/librte_lpm/rte_lpm.c
@@ -373,7 +373,6 @@ rte_lpm_free_v20(struct rte_lpm_v20 *lpm)

rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);

-   rte_free(lpm->rules_tbl);
rte_free(lpm);
rte_free(te);
 }
-- 
2.7.4



[dpdk-dev] [RFC] vhost user: add error handling for fd > 1023

2016-04-11 Thread Christian Ehrhardt
I like the approach as well to go for the fix for robustness first.

I was accidentally able to find another testcase to hit the same root cause.
Adding guests with 15 vhost_user based NICs each while having rxq for
openvswitch-dpdk set to 4 and multiqueue for the guest devices at 4 already
breaks when adding the thirds such guests.
That is way earlier than I would have expected the fd's to be exhausted but
still the same root cause, so just another test for the same.

In prep to the wider check to the patch a minor review question from me:
On the section of rte_vhost_driver_register that now detects if there were
issues we might want to close the socketfd as well when bailing out.
Otherwise we would just have another source of fd leaks or would that be
reused later on even now that we have freed vserver-path and vserver itself?

   ret = fdset_add(_vhost_server.fdset, vserver->listenfd,
   vserver_new_vq_conn, NULL, vserver);
   if (ret < 0) {
   pthread_mutex_unlock(_vhost_server.server_mutex);
   RTE_LOG(ERR, VHOST_CONFIG,
   "failed to add listen fd %d to vhost server
fdset\n",
   vserver->listenfd);
   free(vserver->path);
+  close(vserver->listenfd);
   free(vserver);
   return -1;
   }


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Mon, Apr 11, 2016 at 8:06 AM, Patrik Andersson R <
patrik.r.andersson at ericsson.com> wrote:

> I fully agree with this course of action.
>
> Thank you,
>
> Patrik
>
>
>
> On 04/08/2016 08:47 AM, Xie, Huawei wrote:
>
>> On 4/7/2016 10:52 PM, Christian Ehrhardt wrote:
>>
>>> I totally agree to that there is no deterministic rule what to expect.
>>> The only rule is that #fd certainly always is > #vhost_user devices.
>>> In various setup variants I've crossed fd 1024 anywhere between 475
>>> and 970 vhost_user ports.
>>>
>>> Once the discussion continues and we have an updates version of the
>>> patch with some more agreement I hope I can help to test it.
>>>
>> Thanks. Let us first temporarily fix this problem for robustness, then
>> we consider whether upgrade to (e)poll.
>> Will check the patch in detail later. Basically it should work but need
>> check whether we need extra fixes elsewhere.
>>
>
>


[dpdk-dev] [RFC] vhost user: add error handling for fd > 1023

2016-04-07 Thread Christian Ehrhardt
Hi Patrick,

On Tue, Apr 5, 2016 at 10:40 AM, Patrik Andersson R <
patrik.r.andersson at ericsson.com> wrote:
>
> The described fault situation arises due to the fact that there is a bug
> in an OpenStack component, Neutron or Nova, that fails to release ports
> on VM deletion. This typically leads to an accumulation of 1-2 file
> descriptors per unreleased port. It could also arise when allocating a
> large
> number (~500?) of vhost user ports and connecting them all to VMs.
>

I can confirm that I'm able to trigger this without Openstack.
Using DPDK 2.2 and OpenVswitch 2.5.
Initially I had at least 2 guests attached to the first two ports, but it
seems not necessary which makes it as easy as:
ovs-vsctl add-br ovsdpdkbr0 -- set bridge ovsdpdkbr0 datapath_type=netdev
ovs-vsctl add-port ovsdpdkbr0 dpdk0 -- set Interface dpdk0 type=dpdk
for idx in {1..1023}; do ovs-vsctl add-port ovsdpdkbr0 vhost-user-${idx} --
set Interface vhost-user-${idx} type=dpdkvhostuser; done

=> as soon as the associated fd is >1023 the vhost_user socket gets
created, but just afterwards I see the crash mentioned by Patrick

#0  0x7f51cb187518 in __GI_raise (sig=sig at entry=6) at
../sysdeps/unix/sysv/linux/raise.c:54
#1  0x7f51cb1890ea in __GI_abort () at abort.c:89
#2  0x7f51cb1c98c4 in __libc_message (do_abort=do_abort at entry=2,
fmt=fmt at entry=0x7f51cb2e1584 "*** %s ***: %s terminated\n") at
../sysdeps/posix/libc_fatal.c:175
#3  0x7f51cb26af94 in __GI___fortify_fail (msg=,
msg at entry=0x7f51cb2e1515 "buffer overflow detected") at fortify_fail.c:37
#4  0x7f51cb268fa0 in __GI___chk_fail () at chk_fail.c:28
#5  0x7f51cb26aee7 in __fdelt_chk (d=) at fdelt_chk.c:25
#6  0x7f51cbd6d665 in fdset_fill (pfdset=0x7f51cc03dfa0
<g_vhost_server+8192>, wfset=0x7f51c78e4a30, rfset=0x7f51c78e49b0)
at
/build/dpdk-3lQdSB/dpdk-2.2.0/lib/librte_vhost/vhost_user/fd_man.c:110
#7  fdset_event_dispatch (pfdset=pfdset at entry=0x7f51cc03dfa0
<g_vhost_server+8192>) at
/build/dpdk-3lQdSB/dpdk-2.2.0/lib/librte_vhost/vhost_user/fd_man.c:243
#8  0x7f51cbdc1b00 in rte_vhost_driver_session_start () at
/build/dpdk-3lQdSB/dpdk-2.2.0/lib/librte_vhost/vhost_user/vhost-net-user.c:525
#9  0x005061ab in start_vhost_loop (dummy=) at
../lib/netdev-dpdk.c:2047
#10 0x004c2c64 in ovsthread_wrapper (aux_=) at
../lib/ovs-thread.c:340
#11 0x7f51cba346fa in start_thread (arg=0x7f51c78e5700) at
pthread_create.c:333
#12 0x7f51cb2592dd in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:109

As Patrick I don't have a "pure" DPDK test yet, but at least OpenStack is
our of the scope now which should help.

[...]


> The key point, I think, is that more than one file descriptor is used per
> vhost user device. This means that there is no real relation between the
> number of devices and the number of file descriptors in use.


Well it is "one per vhost_user device" as far as I've seen, but those are
not the only fd's used overall.

[...]


> In my opinion the problem is that the assumption: number of vhost
> user device == number of file descriptors does not hold. What the actual
> relation might be hard to determine with any certainty.
>

I totally agree to that there is no deterministic rule what to expect.
The only rule is that #fd certainly always is > #vhost_user devices.
In various setup variants I've crossed fd 1024 anywhere between 475 and 970
vhost_user ports.

Once the discussion continues and we have an updates version of the patch
with some more agreement I hope I can help to test it.

Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] Updating bnx2x firmware

2016-03-29 Thread Christian Ehrhardt
Hi Thiago and Harish,
Ubuntus linux-firmware package has the latest versions of mid and end of
2015 (no newer yet as of today).
And as Harish pointed out any older will likely be untested/unsupported.

I like that you want to stick to the packaged content and I think your FW
in the linux-firmware package is up to date as of now.
The referred FW in the DPDK code is quite old (2012 according to the git).

But I think just changing the defines in the dpdk code to whatever is
current as of today won't solve it in the long term.
Neither for the package to carry a delta just for it (and bnx2 being a
disabled feature).
Nor for upstream-dpdk as it will just change over time and be incorrect
again.

What about a patch to upstream DPDK that detects the latest available FW
and creates a header with matching defines?
If the dpdk project would support such an approach you could create one and
bring it upstream.
Then later on (next dpdk release and it being packaged) it would always
detect and use the latest available - wouldn't that be what everybody would
want?


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

On Fri, Mar 25, 2016 at 2:31 AM, Martinx - ? 
wrote:

> On 24 March 2016 at 19:03, Harish Patil  wrote:
>
> > >
> > >Guys,
> > >
> > > Currently, the bnx2x.c driver looks for the following firmware files
> > >(when
> > >PMD is enabled for it):
> > >
> > >---
> > >$ ~/sources/dpdk/dpdk-2.2.0/drivers/net/bnx2x$ grep lib\/firmware *
> > >bnx2x.c:#define FW_NAME_57711
> "/lib/firmware/bnx2x/bnx2x-e1h-7.2.51.0.fw"
> > >bnx2x.c:#define FW_NAME_57810 "/lib/firmware/bnx2x/bnx2x-e2-7.2.51.0.fw"
> > >---
> > >
> > > Files bnx2x-e1h-7.2.51.0.fw and bnx2x-e2-7.2.51.0.fw.
> > >
> > >
> > > However, on Ubuntu 16.04, the package linux-firmware comes with:
> > >
> > >---
> > >$ dpkg -L linux-firmware | grep -i bnx2x
> > >/lib/firmware/bnx2x/bnx2x-e1h-7.12.30.0.fw
> > >/lib/firmware/bnx2x/bnx2x-e1-7.12.30.0.fw
> > >/lib/firmware/bnx2x/bnx2x-e1-7.13.1.0.fw
> > >/lib/firmware/bnx2x/bnx2x-e2-7.12.30.0.fw
> > >/lib/firmware/bnx2x/bnx2x-e1h-7.13.1.0.fw
> > >/lib/firmware/bnx2x/bnx2x-e2-7.13.1.0.fw
> > >---
> > >
> > >
> > > Is it okay to just point bnx2x.c to a new version and rebuild it ?
> > >
> > > For example: bnx2x-e1h-7.13.1.0.fw and bnx2x-e2-7.13.1.0.fw ?
> > >
> > >
> > > I would prefer to not manually download old firmware from Github:
> > >
> > > https://github.com/cernekee/linux-firmware/tree/master/bnx2x
> > >
> > >
> > >Thanks,
> > >Thiago
> > >
> >
> > Hi Thiago
> > Any reason why you don?t prefer to get the required FW file from other
> > source?
> >
> > You can certainly download it from:
> >
> http://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/tre
> > e/bnx2x
> >
> > Any other FW is an untested combination.
> >
> > Thanks,
> > Harish
> >
>
> Hi Harish,
>
>  Sure, I can download the firmware files, no problem... I just prefer to
> stick with the packages/files from the distribution that I'm using (Ubuntu
> 16.04) but, on this case, I'll follow your recommendation.
>
>  I don't know why Ubuntu removed the previous firmware files out from
> "linux-firmware" package...
>
>  Thanks for posting the git.kernel.org link!
>
> Best,
> Thiago
>


[dpdk-dev] Suggestions for the dpdk stable tree

2016-03-24 Thread Christian Ehrhardt
Hi,
there is a stable tree for dpdk 2.2 at http://dpdk.org/browse/dpdk-stable/
But it is still at the 2.2 Release status.

I was working on testing 2.2 and needed to pull in some patches from post
2.2 release to fix issues.
After that I started to "scan" the git log what else might apply to a clean
and uncritical addition to a dpdp-2.2 as "stability backports".

I had a focus on identifying:
- only patches to components that are enabled in our packaging (well that
is close to the default conf so it should be ok for you as well)
- issues that have a real chance to occur
- patches that are small, so review is not overly complex
- apply without manual adaption (offset ok to some extend, no fuzz, no
manual fix)
I realized that I should suggest that list of patches to you to consider
adding it to the stable tree.
So therefore for you to consider:


commit d3a274ce9dee28118b8647e0db72ef0f4b6a6323

app/testpmd: handle SIGINT and SIGTERM

commit 308df2bfba3d238fc1d2d16cc10c84681803b408

Handle SIGINT and SIGTERM in l3fwd.

commit da82ee17e6ea99bf2931383ac33b0caccaaaefce

tools: fix unbinding failure handling

commit 16c1814c802c205f6d3c32e3d3d10de9f87e7f22

tools: support Python 3 in bind script

commit bb9f408550d13af6c1da104b0d9d9b9837f69bde

tools: support binding to built-in kernel modules


commit 6e7caa1ad9d597fed0a49468af25ae6e68b8c443

eal/linux: support built-in kernel modules


commit 86f36ff9578b5f3d697c8fcf6072dcb70e2b246f

mempool: fix leak when creation fails


commit ca67ed289a76f38d6c4a4021985a36eaf1d77e28

vhost: fix leak of fds and mmaps


commit fa11a8a7251e14eca0a4190128732386f13551bd

port: fix crash for ring writer nodrop

commit 04f366906ab395c8047bebfc1ddea244dfcb40f5

port: fix crash for ethdev writer nodrop


commit c7a4ff80722e9237a4c504106d21ba5ca27d8df2

i40e: fix overflow

commit 097e920c32bf19cf918cc071525f33b0abdeebaf

i40e: fix inverted check for no refcount

commit 330aa319382aec9ea595f9ebcb9a3c44647ad66c

i40e: fix VLAN filtering

commit 9f44dd3d8ad447c7f797a9564d30a15e5ab7f72b

i40e/base: fix missing check for stopped admin queue

commit 8a8807369ffafef90c410279b4b2645d2d7a7483

i40e/base: fix driver load failure


commit 7656a546c0609f3205558a5d48352c82607d38d3

fm10k: fix VLAN flag in scattered Rx


commit c6fb0e55585206a89f6db396de860e6e844cad06

pcap: fix captured frame length


commit 6e02723754fb2b341701ac438486b2dfea98b523

bonding: fix detach of bonded device

commit df3e8ad73f4c92b4eb8f49ff33271d4a09e6a04a

bonding: fix detach of slave devices

commit 786c990a11e6e6592dfdee02840877070aa3a79a

bonding: copy entire config structure in mode 4

commit 6698820b5f6d955b6af2b916e1074db236d4f5a2

bonding: do not ignore multicast in mode 4

commit 8997a10bfcad789d000debaac4cd85cd3db57997

bonding: fix active slaves with no primary

commit 7a7122edf1c8d63e516d1b2c2eff6fa9b54e0f82

bonding: do not activate slave twice

commit 2186fff3675d4e774cecc8f918c05063e0367d28

bonding: fix crash when no slave device


commit 3b1e3e4e362453df8cecbc6d481444be8b84326e

virtio: fix descriptors pointing to the same buffer

commit c680a4a88c4312068f60937a7ba51eac8211c9a6

virtio: fix crash in statistics functions

commit 9a0615af7746485d73d10561cc0743bc2fcd4bf7

virtio: fix restart


Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


[dpdk-dev] Issues with openvswitch2.5+dpdk2.2+kvm/virtio-pci

2016-03-23 Thread Christian Ehrhardt
On Tue, Mar 22, 2016 at 9:47 PM, Daniele Di Proietto  wrote:

> Hi Christian,
>
> thanks for the report.  I've managed to reproduce the problem and I
> observed two separate issues:
>
> 1) short version: it appears to be a problem in DPDK 2.2 and it should be
> fixed by 9a0615af7746("virtio: fix restart"), now on master.
>
[...]

> It appears that simply backporting 9a0615af7746("virtio: fix restart")
> fixes the issue.
>

Yeah debugging it I came to the double configuration as cause as well
yesterday.
Eventually I saw that virtqueue->vq_ring structs got reallocated but stayed
zeroed out by the second call.

I didn't realize there was a fix for it already - thanks so much for
pointing it out.
I'll give a backport a try and let you know if it worked.



> 2) When ovs-vswitchd is started with --monitor, there will be a parent
> process (the monitor) that simply restarts the child if it crashes, while
> the child does the actual ovs-vswitchd job.
>
> It appears that the monitor feature is broken with DPDK, because the DPDK
> library is wrongly initialized in the parent process.  If the child
> crashes, all the DPDK memzones are preserved, meaning that new allocations
> will likely fail.
>
> The fix for this is calling rte_eal_init() in the child process, after
> parsing other ovs-vswitchd command line options. This is done (among other
> things) in Aaron's series currently under review:
>
> http://openvswitch.org/pipermail/dev/2016-March/067422.html
>
> I think we need a separate fix for branch-2.5.
>

Yeah I was on discussing and testing that series already as the series was
also related to a socket ownership/permission issue (back in time).
http://openvswitch.org/pipermail/dev/2015-December/063567.html
I think eventually we need separate fixes for both on the branch-2.5

[...]


[dpdk-dev] [PATCH v4 5/5] lpm: fix missing free of rules_tbl and lpm in rte_lpm_free*

2016-03-21 Thread Christian Ehrhardt
As found in rte_lpm6_free the two lpm interfaces rte_lpm_free_v20 and
rte_lpm_free_v1604 had a leak.

rte_lpm_free_v20 might have missed to free rules_tbl
rte_lpm_free_v1604 due to an early exit might have missed to free
rules_tbl and lpm itself.

Acked-by: Olivier Matz 
Signed-off-by: Christian Ehrhardt 
---
 lib/librte_lpm/rte_lpm.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
index 59ce5a7..af5811c 100644
--- a/lib/librte_lpm/rte_lpm.c
+++ b/lib/librte_lpm/rte_lpm.c
@@ -367,6 +367,7 @@ rte_lpm_free_v20(struct rte_lpm_v20 *lpm)

rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);

+   rte_free(lpm->rules_tbl);
rte_free(lpm);
rte_free(te);
 }
@@ -391,15 +392,12 @@ rte_lpm_free_v1604(struct rte_lpm *lpm)
if (te->data == (void *) lpm)
break;
}
-   if (te == NULL) {
-   rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
-   return;
-   }
-
-   TAILQ_REMOVE(lpm_list, te, next);
+   if (te != NULL)
+   TAILQ_REMOVE(lpm_list, te, next);

rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);

+   rte_free(lpm->rules_tbl);
rte_free(lpm);
rte_free(te);
 }
-- 
2.7.3



[dpdk-dev] [PATCH v4 4/5] lpm: fix use after free of lpm in rte_lpm_create*

2016-03-21 Thread Christian Ehrhardt
There were further chances for a use after free by returning an already freed
pointer in rte_lpm_create for v20 and v1604.
Along that is also makes the RTE_LOG messages of the failed allocations unique.

Acked-by: Olivier Matz 
Signed-off-by: Christian Ehrhardt 
---
 lib/librte_lpm/rte_lpm.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
index d5fa1f8..59ce5a7 100644
--- a/lib/librte_lpm/rte_lpm.c
+++ b/lib/librte_lpm/rte_lpm.c
@@ -303,8 +303,9 @@ rte_lpm_create_v1604(const char *name, int socket_id,
(size_t)rules_size, RTE_CACHE_LINE_SIZE, socket_id);

if (lpm->rules_tbl == NULL) {
-   RTE_LOG(ERR, LPM, "LPM memory allocation failed\n");
+   RTE_LOG(ERR, LPM, "LPM rules_tbl memory allocation failed\n");
rte_free(lpm);
+   lpm = NULL;
rte_free(te);
goto exit;
}
@@ -313,8 +314,9 @@ rte_lpm_create_v1604(const char *name, int socket_id,
(size_t)tbl8s_size, RTE_CACHE_LINE_SIZE, socket_id);

if (lpm->tbl8 == NULL) {
-   RTE_LOG(ERR, LPM, "LPM memory allocation failed\n");
+   RTE_LOG(ERR, LPM, "LPM tbl8 memory allocation failed\n");
rte_free(lpm);
+   lpm = NULL;
rte_free(te);
goto exit;
}
-- 
2.7.3



[dpdk-dev] [PATCH v4 3/5] lpm: fix missing free of lpm

2016-03-21 Thread Christian Ehrhardt
Fixing lpm6 regarding a similar issue showed that that in
rte_lpm_free lpm might not be freed if it didn't find a te (early return)

Acked-by: Bruce Richardson 
Acked-by: Olivier Matz 
Signed-off-by: Christian Ehrhardt 
---
 lib/librte_lpm/rte_lpm.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
index ccaaa2a..d5fa1f8 100644
--- a/lib/librte_lpm/rte_lpm.c
+++ b/lib/librte_lpm/rte_lpm.c
@@ -360,12 +360,8 @@ rte_lpm_free_v20(struct rte_lpm_v20 *lpm)
if (te->data == (void *) lpm)
break;
}
-   if (te == NULL) {
-   rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
-   return;
-   }
-
-   TAILQ_REMOVE(lpm_list, te, next);
+   if (te != NULL)
+   TAILQ_REMOVE(lpm_list, te, next);

rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);

-- 
2.7.3



[dpdk-dev] [PATCH v4 2/5] lpm6: fix missing free of rules_tbl and lpm

2016-03-21 Thread Christian Ehrhardt
lpm6 autotests failed with the default alloc of 512M Memory.
While >=2500M was a workaround it became clear while debugging that it
had a leak.
One could see a lot of output like:
  LPM Test tests6[i]: FAIL
  LPM: LPM memory allocation failed

It turned out that in rte_lpm6_free
- lpm might not be freed if it didn't find a te (early return)
- lpm->rules_tbl was not freed ever

Acked-by: Bruce Richardson 
Acked-by: Olivier Matz 
Signed-off-by: Christian Ehrhardt 
---
 lib/librte_lpm/rte_lpm6.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c
index 48931cc..4c44cd7 100644
--- a/lib/librte_lpm/rte_lpm6.c
+++ b/lib/librte_lpm/rte_lpm6.c
@@ -278,15 +278,13 @@ rte_lpm6_free(struct rte_lpm6 *lpm)
if (te->data == (void *) lpm)
break;
}
-   if (te == NULL) {
-   rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
-   return;
-   }

-   TAILQ_REMOVE(lpm_list, te, next);
+   if (te != NULL)
+   TAILQ_REMOVE(lpm_list, te, next);

rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);

+   rte_free(lpm->rules_tbl);
rte_free(lpm);
rte_free(te);
 }
-- 
2.7.3



  1   2   >