> -----Original Message-----
> From: Aaron Conole [mailto:acon...@redhat.com]
> Sent: Monday, January 18, 2016 8:29 PM
> To: dev@openvswitch.org
> Cc: Flavio Leitner; Traynor, Kevin; Panu Matilainen; Zoltan Kiss
> Subject: [PATCH v5 2/4] netdev-dpdk: Convert initialization from cmdline to
> db
>
> 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 <acon...@redhat.com>
> ---
> 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
Hi Aaron. I've only one real comment below on this patch. The other patches
look fine to me but I didn't get a chance to test.
thanks,
Kevin.
>
> FAQ.md | 6 +-
> INSTALL.DPDK.md | 81 ++++++++++++++-----
> lib/netdev-dpdk.c | 191 ++++++++++++++++++++++++++++++++-----------
> --
> lib/netdev-dpdk.h | 22 ++++--
> utilities/ovs-dev.py | 11 ++-
> vswitchd/bridge.c | 3 +
> vswitchd/ovs-vswitchd.8.in | 5 +-
> vswitchd/ovs-vswitchd.c | 25 +-----
> vswitchd/vswitch.xml | 118 +++++++++++++++++++++++++++-
> 9 files changed, 346 insertions(+), 116 deletions(-)
>
> diff --git a/FAQ.md b/FAQ.md
> index 29b2e19..c233118 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 96b686c..46bd1a8 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.
I'm assuming you've renamed from 'dpdk' to 'dpdk-init', so it could be =false
and there is still the possibility of adding a lazy init at a later time?
If so, I think it is good idea because we wouldn't want a situation in the
future where 'dpdk'=false but an introduced lazy init means DPDK is used.
> +
> + * 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 threads to float.
> +
> + * dpdk-mem-channels
> + This sets the number of memory spread channels per CPU socket. It is
> purely
> + an optimization flag.
> +
> + * dpdk-alloc-mem
> + This sets the total memory to preallocate from hugepages regardless of
> + processor socket. It is recommended to use dpdk-socket-mem instead.
> +
> + * dpdk-socket-mem
> + Comma separated list of memory to pre-allocate from hugepages on specific
> + sockets.
> +
> + * dpdk-hugepage-dir
> + Directory where hugetlbfs is mounted
> +
> + * cuse-dev-name
> + Option to set the vhost_cuse character device name.
> +
> + * vhost-sock-dir
> + Option to set the path to the vhost_user unix socket files.
> +
> + NOTE: Changing any of these options requires restarting the ovs-vswitchd
> + application.
> +
> + Open vSwitch can be started as normal. DPDK will not be initialized until
> + the first DPDK-enabled port is added to the bridge.
lazy init has been removed, so this is not true anymore.
>
> ```
> export DB_SOCK=/usr/local/var/run/openvswitch/db.sock
> - ovs-vswitchd --dpdk -c 0x1 -n 4 -- unix:$DB_SOCK --pidfile --detach
> + ovs-vswitchd unix:$DB_SOCK --pidfile --detach
> ```
>
> If allocated more than one GB hugepage (as for IVSHMEM), set amount and
> use NUMA node 0 memory:
>
> ```
> - ovs-vswitchd --dpdk -c 0x1 -n 4 --socket-mem 1024,0 \
> - -- unix:$DB_SOCK --pidfile --detach
> + ovs-vsctl --no-wait set Open_vSwitch .
> other_config:dpdk_socket_mem="1024,0"
> + ovs-vswitchd unix:$DB_SOCK --pidfile --detach
> ```
>
> 6. Add bridge & ports
> @@ -521,11 +563,13 @@ have arbitrary names.
> `/usr/local/var/run/openvswitch/vhost-user-1`, which you must provide
> to your VM on the QEMU command line. More instructions on this can be
> found in the next section "DPDK vhost-user VM configuration"
> - Note: If you wish for the vhost-user sockets to be created in a
> - directory other than `/usr/local/var/run/openvswitch`, you may specify
> - another location on the ovs-vswitchd command line like so:
>
> - `./vswitchd/ovs-vswitchd --dpdk -vhost_sock_dir /my-dir -c 0x1 ...`
> + - If you wish for the vhost-user sockets to be created in a directory
> other
> + than `/usr/local/var/run/openvswitch`, you may specify another location
> + in the ovsdb like:
> +
> + `./vswitchd/ovs-vsctl --no-wait \
> + set Open_vSwitch . other_config:vhost-sock-dir=path`
>
> DPDK vhost-user VM configuration:
> ---------------------------------
> @@ -638,14 +682,13 @@ DPDK vhost-cuse VM configuration:
>
> 1. This step is only needed if using an alternative character device.
>
> - The new character device filename must be specified on the vswitchd
> - commandline:
> + The new character device filename must be specified in the ovsdb:
>
> - `./vswitchd/ovs-vswitchd --dpdk --cuse_dev_name my-vhost-net -c 0x1
> ...`
> + `./utilities/ovs-vsctl --no-wait set Open_vSwitch . \
> + other_config:cuse-dev-name=my-vhost-net`
>
> - Note that the `--cuse_dev_name` argument and associated string must be
> the first
> - arguments after `--dpdk` and come before the EAL arguments. In the
> example
> - above, the character device to be used will be `/dev/my-vhost-net`.
> + In the example above, the character device to be used will be
> + `/dev/my-vhost-net`.
>
> 2. This step is only needed if reusing the standard character device. It
> will
> conflict with the kernel vhost character device so the user must first
> @@ -758,8 +801,8 @@ steps.
>
> <my-vhost-device> refers to "vhost-net" if using the `/dev/vhost-
> net`
> device. If you have specificed a different name on the ovs-vswitchd
> - commandline using the "--cuse_dev_name" parameter, please specify
> that
> - filename instead.
> + database using the "other_config:cuse-dev-name" parameter, please
> + specify that filename instead.
>
> 2. Disable SELinux or set to permissive mode
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 78f013d..b7bb3eb 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -29,6 +29,7 @@
> #include <stdio.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> +#include <getopt.h>
>
> #include "dirs.h"
> #include "dp-packet.h"
> @@ -49,6 +50,8 @@
> #include "timeval.h"
> #include "unixctl.h"
> #include "openvswitch/vlog.h"
> +#include "smap.h"
> +#include "vswitch-idl.h"
>
> #include "rte_config.h"
> #include "rte_mbuf.h"
> @@ -551,8 +554,15 @@ netdev_dpdk_cast(const struct netdev *netdev)
> static struct netdev *
> netdev_dpdk_alloc(void)
> {
> - struct netdev_dpdk *netdev = dpdk_rte_mzalloc(sizeof *netdev);
> - return &netdev->up;
> + struct netdev_dpdk *netdev;
> +
> + if (!rte_eal_init_ret) { /* Only after successful initialization */
> + netdev = dpdk_rte_mzalloc(sizeof *netdev);
> + if (netdev) {
> + return &netdev->up;
> + }
> + }
> + return NULL;
> }
>
> static void
> @@ -670,6 +680,10 @@ netdev_dpdk_vhost_cuse_construct(struct netdev *netdev_)
> struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
> int err;
>
> + if (rte_eal_init_ret) {
> + return rte_eal_init_ret;
> + }
> +
> ovs_mutex_lock(&dpdk_mutex);
> strncpy(netdev->vhost_id, netdev->up.name, sizeof(netdev->vhost_id));
> err = vhost_construct_helper(netdev_);
> @@ -683,6 +697,10 @@ netdev_dpdk_vhost_user_construct(struct netdev *netdev_)
> struct netdev_dpdk *netdev = netdev_dpdk_cast(netdev_);
> int err;
>
> + if (rte_eal_init_ret) {
> + return rte_eal_init_ret;
> + }
> +
> ovs_mutex_lock(&dpdk_mutex);
> /* Take the name of the vhost-user port and append it to the location
> where
> * the socket is to be created, then register the socket.
> @@ -1776,6 +1794,8 @@ new_device(struct virtio_net *dev)
> struct netdev_dpdk *netdev;
> bool exists = false;
>
> +
> +
added whitespace
<snip>
> diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h
> index 646d3e2..cde8956 100644
> --- a/lib/netdev-dpdk.h
> +++ b/lib/netdev-dpdk.h
> @@ -2,6 +2,9 @@
> #define NETDEV_DPDK_H
>
> #include <config.h>
> +#include "smap.h"
> +#include "ovs-thread.h"
> +#include "vswitch-idl.h"
>
> struct dp_packet;
>
> @@ -22,7 +25,9 @@ struct dp_packet;
>
> #define NON_PMD_CORE_ID LCORE_ID_ANY
>
> -int dpdk_init(int argc, char **argv);
> +struct ovsrec_open_vswitch;
> +
> +void dpdk_init(const struct ovsrec_open_vswitch *ovs_cfg);
> void netdev_dpdk_register(void);
> void free_dpdk_buf(struct dp_packet *);
> int pmd_thread_setaffinity_cpu(unsigned cpu);
> @@ -30,16 +35,19 @@ int pmd_thread_setaffinity_cpu(unsigned cpu);
> #else
>
> #define NON_PMD_CORE_ID UINT32_MAX
> -
> #include "util.h"
>
> -static inline int
> -dpdk_init(int argc, char **argv)
> +static inline void
> +dpdk_init(const struct ovsrec_open_vswitch *ovs_cfg)
> {
> - if (argc >= 2 && !strcmp(argv[1], "--dpdk")) {
> - ovs_fatal(0, "DPDK support not built into this copy of Open
> vSwitch.");
> + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> +
> + if (ovs_cfg && ovsthread_once_start(&once)) {
> + if(smap_get_bool(&ovs_cfg->other_config, "dpdk", false)) {
> + ovs_fatal(0, "DPDK not supported in this copy of Open
> vSwitch.");
The db entry has changed from 'dpdk' to 'dpdk-init' in this patchset
> + }
> + ovsthread_once_done(&once);
> }
> - return 0;
> }
>
<snip>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
