Re: [ovs-dev] [PATCH v10 2/6] netdev-dpdk: Convert initialization from cmdline to db

2016-03-28 Thread Daniele Di Proietto 

I still have some comment:

dpdk-mem-channels: This is not required by DPDK anymore, so I still
think that's not necessary and could be removed.  If someone want

I think we shouldn't abort if we fail something during the initialization.
I know that rte_eal_init() can still abort, but I want to avoid it as
much as possible in OVS (It's still ok to abort for memory
allocation failure, as we do in the rest of OVS).  I've commented inline
when ovs_abort() is used.

Since we only use the "other_config" member of "struct
ovsrec_open_vswitch",
I would avoid passing the whole structure to dpdk_init().  We can avoid
including "vswitch-idl.h".

On 09/03/2016 09:38, "Aaron Conole"  wrote:

>Existing DPDK integration is provided by use of command line options which
>must be split out and passed to librte in a special manner. However, this
>forces any configuration to be passed by way of a special DPDK flag, and
>interferes with ovs+dpdk packaging solutions.
>
>This commit delays dpdk initialization until after the OVS database
>connection is established, at which point ovs initializes librte. It
>pulls all of the config data from the OVS database, and assembles a
>new argv/argc pair to be passed along.
>
>Signed-off-by: Aaron Conole 
>Tested-by: Sean K Mooney 
>Tested-by: RobertX Wojciechowicz 
>Tested-by: Kevin Traynor 
>Acked-by: Panu Matilainen 
>Acked-by: Kevin Traynor 
>Acked-by: Flavio Leitner 
>---
>v2:
>* Removed trailing whitespace
>* Followed for() loop brace coding style
>* Automatically enable DPDK when adding a DPDK enabled port
>* Fixed an issue on startup when DPDK enabled ports are present
>* Updated the documentation (including vswitch.xml) and documented all
>  new parameters
>* Dropped the premature initialization test
>
>v3:
>* Improved description language in INSTALL.DPDK.md
>* Fixed the ovs-vsctl examples for DPDK
>* Returned to the global dpdk-init (bullet 3 from v2)
>* Fixed a build error when compiling without dpdk support enabled
>* converted to xstrdup, for consistency after rebasing
>
>v4:
>* No change
>
>v5:
>* Adjust the ovs-dev script to account for the new dpdk configuration
>* Update the ovs-vswitchd.8.in pointing to INSTALL.DPDK.md
>
>v6:
>* Remove excess whitespace addition
>* Correct INSTALL.DPDK.md regarding when DPDK is initialized
>* Used incorrect variable in the non-dpdk case for testing against
>  dpdk
>
>v7:
>* Account for mutually exclusive options;
>
>v8:
>* ``make check`` testing revealed a number of flaws in the initialization
>  which resulted in memory corruption
>* Fixed the ovs-vswitchd startup during testing
>
>v9:
>* Re-arrange the added headers in netdev-dpdk.c to try and be alphabetical
>* Convert '-c' and '-n' options to be default non-inserted
>
>v10:
>* Documentation adjustment in vswitch.xml explicitly stating these values
>  are not runtime configurable.
>* Wrapped vhost_cuse_dev in #ifdef
>
> FAQ.md |   6 +-
> INSTALL.DPDK.md|  81 +---
> lib/netdev-dpdk.c  | 308
>+
> lib/netdev-dpdk.h  |  22 ++--
> tests/ofproto-macros.at|   3 +-
> utilities/ovs-dev.py   |  11 +-
> vswitchd/bridge.c  |   3 +
> vswitchd/ovs-vswitchd.8.in |   5 +-
> vswitchd/ovs-vswitchd.c|  25 +---
> vswitchd/vswitch.xml   | 131 ++-
> 10 files changed, 454 insertions(+), 141 deletions(-)
>
>diff --git a/FAQ.md b/FAQ.md
>index 8bd7ab9..018e6ae 100644
>--- a/FAQ.md
>+++ b/FAQ.md
>@@ -431,9 +431,9 @@ A: Yes.  How you configure it depends on what you
>mean by "promiscuous
> 
> A: Firstly, you must have a DPDK-enabled version of Open vSwitch.
> 
>-   If your version is DPDK-enabled it will support the --dpdk
>-   argument on the command line and will display lines with
>-   "EAL:..." during startup when --dpdk is supplied.
>+   If your version is DPDK-enabled it will support the
>other_config:dpdk-init
>+   configuration in the database and will display lines with
>+   "EAL:..." during startup when other_config:dpdk-init is set to 'true'.
> 
>Secondly, when adding a DPDK port, unlike a system port, the
>type for the interface must be specified. For example;
>diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
>index 1fc1b66..613764f 100644
>--- a/INSTALL.DPDK.md
>+++ b/INSTALL.DPDK.md
>@@ -143,22 +143,64 @@ Using the DPDK with ovs-vswitchd:
> 
> 5. Start vswitchd:
> 
>-   DPDK configuration arguments can be passed to vswitchd via `--dpdk`
>-   argument. This needs to be first argument passed to vswitchd process.
>-   dpdk arg -c is ignored by ovs-dpdk, but it is a required parameter
>-   for dpdk initialization.
>+   DPDK configuration arguments can be passed to vswitchd via
>Open_vSwitch
>+   other_config database. The recognized configuration options are

[ovs-dev] [PATCH v10 2/6] netdev-dpdk: Convert initialization from cmdline to db

2016-03-09 Thread Aaron Conole 
Existing DPDK integration is provided by use of command line options which
must be split out and passed to librte in a special manner. However, this
forces any configuration to be passed by way of a special DPDK flag, and
interferes with ovs+dpdk packaging solutions.

This commit delays dpdk initialization until after the OVS database
connection is established, at which point ovs initializes librte. It
pulls all of the config data from the OVS database, and assembles a
new argv/argc pair to be passed along.

Signed-off-by: Aaron Conole 
Tested-by: Sean K Mooney 
Tested-by: RobertX Wojciechowicz 
Tested-by: Kevin Traynor 
Acked-by: Panu Matilainen 
Acked-by: Kevin Traynor 
Acked-by: Flavio Leitner 
---
v2:
* Removed trailing whitespace
* Followed for() loop brace coding style
* Automatically enable DPDK when adding a DPDK enabled port
* Fixed an issue on startup when DPDK enabled ports are present
* Updated the documentation (including vswitch.xml) and documented all
  new parameters
* Dropped the premature initialization test

v3:
* Improved description language in INSTALL.DPDK.md
* Fixed the ovs-vsctl examples for DPDK
* Returned to the global dpdk-init (bullet 3 from v2)
* Fixed a build error when compiling without dpdk support enabled
* converted to xstrdup, for consistency after rebasing

v4:
* No change

v5:
* Adjust the ovs-dev script to account for the new dpdk configuration
* Update the ovs-vswitchd.8.in pointing to INSTALL.DPDK.md

v6:
* Remove excess whitespace addition
* Correct INSTALL.DPDK.md regarding when DPDK is initialized
* Used incorrect variable in the non-dpdk case for testing against
  dpdk

v7:
* Account for mutually exclusive options;

v8:
* ``make check`` testing revealed a number of flaws in the initialization
  which resulted in memory corruption
* Fixed the ovs-vswitchd startup during testing

v9:
* Re-arrange the added headers in netdev-dpdk.c to try and be alphabetical
* Convert '-c' and '-n' options to be default non-inserted

v10:
* Documentation adjustment in vswitch.xml explicitly stating these values
  are not runtime configurable.
* Wrapped vhost_cuse_dev in #ifdef

 FAQ.md |   6 +-
 INSTALL.DPDK.md|  81 +---
 lib/netdev-dpdk.c  | 308 +
 lib/netdev-dpdk.h  |  22 ++--
 tests/ofproto-macros.at|   3 +-
 utilities/ovs-dev.py   |  11 +-
 vswitchd/bridge.c  |   3 +
 vswitchd/ovs-vswitchd.8.in |   5 +-
 vswitchd/ovs-vswitchd.c|  25 +---
 vswitchd/vswitch.xml   | 131 ++-
 10 files changed, 454 insertions(+), 141 deletions(-)

diff --git a/FAQ.md b/FAQ.md
index 8bd7ab9..018e6ae 100644
--- a/FAQ.md
+++ b/FAQ.md
@@ -431,9 +431,9 @@ A: Yes.  How you configure it depends on what you mean by 
"promiscuous
 
 A: Firstly, you must have a DPDK-enabled version of Open vSwitch.
 
-   If your version is DPDK-enabled it will support the --dpdk
-   argument on the command line and will display lines with
-   "EAL:..." during startup when --dpdk is supplied.
+   If your version is DPDK-enabled it will support the other_config:dpdk-init
+   configuration in the database and will display lines with
+   "EAL:..." during startup when other_config:dpdk-init is set to 'true'.
 
Secondly, when adding a DPDK port, unlike a system port, the
type for the interface must be specified. For example;
diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index 1fc1b66..613764f 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -143,22 +143,64 @@ Using the DPDK with ovs-vswitchd:
 
 5. Start vswitchd:
 
-   DPDK configuration arguments can be passed to vswitchd via `--dpdk`
-   argument. This needs to be first argument passed to vswitchd process.
-   dpdk arg -c is ignored by ovs-dpdk, but it is a required parameter
-   for dpdk initialization.
+   DPDK configuration arguments can be passed to vswitchd via Open_vSwitch
+   other_config database. The recognized configuration options are listed.
+   Defaults will be provided for all values not explicitly set.
+
+   * dpdk-init
+   Specifies whether OVS should initialize and support DPDK ports. This is
+   a boolean, and defaults to false.
+
+   * dpdk-lcore-mask
+   Specifies the CPU cores on which dpdk lcore threads should be spawned.
+   The DPDK lcore threads are used for DPDK library tasks, such as
+   library internal message processing, logging, etc. Value should be in
+   the form of a hex string (so '0x123') similar to the 'taskset' mask
+   input.
+   If not specified, the value will be determined by choosing the lowest
+   CPU core from initial cpu affinity list. Otherwise, the value will be
+   passed directly to the DPDK library.
+   For performance reasons, it is best to set this to a single core on
+   the system, rather than allow lcore