[ovs-dev] [PATCH v7 0/5] Convert DPDK configuration from command line to DB based

Wojciechowicz, RobertX robertx.wojciechowicz at intel.com
Tue Feb 2 10:31:57 UTC 2016


Hi,

in the meantime I tested this patch in my environment: "patches/ovs-dev-v7-2-5-netdev-dpdk-Convert-initialization-from-cmdline-to-db.patch"
and indeed it solves also the issue I tried to address in my patch: "http://openvswitch.org/pipermail/dev/2016-January/065186.html".
This patch works for me perfectly and it makes my patch obsolete at this point.

I have just one minor remark to this patch:

--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2207,7 +2207,7 @@ process_vhost_flags(char *flag, char *default_val, int size,
     val = smap_get(&ovs_cfg->other_config, flag);

     /* Depending on which version of vhost is in use, process the vhost-specific
-     * flag if it is provided on the vswitchd command line, otherwise resort to
+     * flag if it is provided in the Open vSwitch database, otherwise resort to
      * a default value.
      *
      * For vhost-user: Process "-vhost_sock_dir" to set the custom location of

Br,
Robert

-----Original Message-----
From: Aaron Conole [mailto:aconole at redhat.com] 
Sent: Thursday, January 28, 2016 3:52 PM
To: Christian Ehrhardt <christian.ehrhardt at canonical.com>
Cc: <dev at openvswitch.org> <dev at openvswitch.org>; Flavio Leitner <fbl at sysclose.org>; Panu Matilainen <pmatilai at redhat.com>; Traynor, Kevin <kevin.traynor at intel.com>; Zoltan Kiss <zoltan.kiss at linaro.org>; Wojciechowicz, RobertX <robertx.wojciechowicz at intel.com>
Subject: Re: [PATCH v7 0/5] Convert DPDK configuration from command line to DB based

Christian Ehrhardt <christian.ehrhardt at canonical.com> writes:
> Hi Aaron
>
> I refreshed my stack that combines your current series with the vhost-user
> permission/ownership work of december.
>
> quilt applied
> patches/ovs-dev-v7-1-5-netdev-dpdk-Restore-thread-affinity-after-DPDK-init.patch
> patches/ovs-dev-v7-2-5-netdev-dpdk-Convert-initialization-from-cmdline-to-db.patch
> patches/ovs-dev-v7-3-5-netdev-dpdk-Autofill-lcore-coremask-if-absent.patch
> patches/ovs-dev-v7-4-5-netdev-dpdk-Allow-arbitrary-eal-arguments.patch
> patches/ovs-dev-v7-5-5-NEWS-Announce-the-DPDK-EAL-configuration-change.patch
> patches/ovs-dev-4-5-lib-daemon-Move-the-user-group-code-up-one-level.patch
> patches/ovs-dev-5-5-netdev-dpdk-Support-user-cfg-vhost-socket-perms.patch
>
> It passes all the tests I had yesterday

That's great to know! :) 

> dpdk|INFO|User-provided vhost-sock-dir in use: /usr/local/var/run/openvswitch-vhost
> dpdk|INFO|User-provided vhost_sock_owners in use: :kvm
> dpdk|INFO|User-provided vhost_sock_permissions in use: 0660
> dpdk|INFO|Socket /usr/local/var/run/openvswitch-vhost/vhost-user-1 created for vhost-user
> port vhost-user-1
> dpdk|INFO|Socket /usr/local/var/run/openvswitch-vhost/vhost-user-1 changed permissions to
> 0660
> dpdk|INFO|Socket /usr/local/var/run/openvswitch-vhost/vhost-user-1 changed ownership to
> :kvm
>
> So the issue with dpdk-alloc-mem is fixed - thanks.

I guess I only use hugepages, so I never properly tested this. I've
added a few scripts, and will start work on a separate series of tests
to add to the testdir to do these kinds of tests.

> The remaining issue with the range check for the permissions doesn't belong to this series
> :-)
> While I agree to the communities assessment in December that they should be discussed
> split, so the second series doesn't stall the former one.
> I'd consider it great to submit them always together, but as two series. So one 0/5 and one
> 0/2.

It's a bit of a chicken-egg problem, and there's really no good way of
doing it. I'd like to just prevent folks from trying to apply the second
series without the first (plus many times it's considered inappropriate
to submit a patch series which depends on an unaccepted series). I agree
though, it's confusing.

> That way we keep the people aware and avoid people doing the same work over and over
> again (me and just today e.g. by Robert Wojciechowicz)
> It would simplify testing both, but still keep discussions as separate as needed.

It's not a problem for me. I have updated the second series to include
your note on the default permissions and the parsing, so I can post them
as RFC or with some other tag, if you want.

> I might seem to miss something obvious (feels that way), but fyi with the series various
> parts of "make check" fail.
> After realizing this I more or less blindly ran a loop of "quilt push; make -j12 && make check
> TESTSUITEFLAGS=-j12" to see which of the patches triggers this behavior.
> It is #2 "ovs-dev-v7-2-5-netdev-dpdk-Convert-initialization-from-cmdline-to-db.patch"
> A lot of tests report fail, and it keeps "hanging" since some seem no more to end correctly.
> Here a log of my "make check TESTSUITEFLAGS=-j12"
> http://paste.ubuntu.com/14688004/

D'oh! I've got some fixes. Looks like my jenkins setup has been running make
check without dpdk and I never noticed :/ I've fixed it, and will have a
v8 posted today.

> I didn't look into it in detail yet - is that a real issue, or am I missing just some sort of DB
> upgrade or so?

Some are actual issues (possibly with ovsthread_once_start? - I'm
investigating),  and some are stupid non-issues (I added a log to the
vswitchd without properly accounting for the check in OVS_VSWITCHD_START)

If you want to get past the errors, you can edit tests/ofproto-macros.at
and change the _OVS_VSWITCHD_START macro to include
/dpdk|INFO|DPDK Disabled - to change this requires a restart./d
in the sed line.

Thanks so much for the feedback and testing, Christian!

> Christian Ehrhardt
> Software Engineer, Ubuntu Server
> Canonical Ltd
>
> On Wed, Jan 27, 2016 at 10:33 PM, Aaron Conole <aconole at redhat.com> wrote:
>
>
>  Currently, configuration of DPDK parameters is done via the command line
>  through a --dpdk **OPTIONS** -- command line argument. This has a number of
>  challenges, including:
>  * It must be the first option passed to ovs-vswitchd
>  * It breaks from the way most other things are configured in OVS
>  * It doesn't allow an easy way to populate defaults
>
>  This series brings the following changes to openvswitch:
>  * All DPDK options are taken from the ovs database rather than the
>  command line
>  * DPDK lcores are optionally auto-assigned to a single core based on the
>  bridge coremask.
>  * Updated documentation
>
>  v2:
>  * Dropped the vhost-user socket configuration options. Those can be re-added
>  as an extension
>  * Incorporated feedback from Kevin Traynor.
>
>  v3:
>  * Went back to a global dpdk-init
>  * Language cleanup and various minor fixes
>
>  v4:
>  * Added a way to pass arbitrary eal arguments
>
>  v5:
>  * Restore the socket-mem default, and fix up the ovs-dev.py script, along
>  with the manpage for ovsdb-server
>
>  v6:
>  * Correct a documentation issue with INSTALL.DPDK.md
>  * Correct a non-dpdk enabled OVS incorrect warning variable
>  * Remove an excess whitespace
>
>  v7:
>  * After testing by Christian with dpdk-alloc-mem
>
>  Aaron Conole (5):
>  netdev-dpdk: Restore thread affinity after DPDK init
>  netdev-dpdk: Convert initialization from cmdline to db
>  netdev-dpdk: Autofill lcore coremask if absent
>  netdev-dpdk: Allow arbitrary eal arguments
>  NEWS: Announce the DPDK EAL configuration change
>
>  FAQ.md | 6 +-
>  INSTALL.DPDK.md | 90 ++++++++++---
>  NEWS | 5 +
>  lib/netdev-dpdk.c | 327
>  ++++++++++++++++++++++++++++++++++++++-------
>  lib/netdev-dpdk.h | 22 ++-
>  utilities/ovs-dev.py | 7 +-
>  vswitchd/bridge.c | 3 +
>  vswitchd/ovs-vswitchd.8.in | 5 +-
>  vswitchd/ovs-vswitchd.c | 25 +---
>  vswitchd/vswitch.xml | 128 +++++++++++++++++-
>  10 files changed, 513 insertions(+), 105 deletions(-)
>
>  --
>  2.5.0
>
>
<#secure method=pgpmime mode=encrypt>
--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.




More information about the dev mailing list