[dpdk-dev] disable hugepages
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
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
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
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
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
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
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
*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
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
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
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
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
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
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
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
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
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
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
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
*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
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
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
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
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
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
*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
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
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
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
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
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
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
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
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
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
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
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
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
*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
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
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
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
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
*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
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
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
*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
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
*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
*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
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
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
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
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
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
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
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
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
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
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
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
_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
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
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
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
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
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
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
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!
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
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
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
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?
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
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?
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
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
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?
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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*
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*
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
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
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