[ovs-dev] [PATCH v7 0/5] Convert DPDK configuration from command line to DB based
Aaron Conole
aconole at redhat.com
Thu Feb 4 16:45:50 UTC 2016
"Wojciechowicz, RobertX" <robertx.wojciechowicz at intel.com> writes:
> Hi,
>
> one question to the "vhost-sock-dir" database entry.
> We received requirement to make sure that this entry will be available
> in the database
> even if there will be used the default vhost socket directory.
> In my patch "dpdk_set_config" function was supposed to do that.
> Is it possible to somehow meet this requirement?
Okay, it is. However, will it hold up this series? I have a follow up
patch to do additional vhost-user configuration changes, and will
happily fold it into that, if it makes sense.
Thanks,
Aaron
> Br,
> Robert
>
> -----Original Message-----
> From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Wojciechowicz, RobertX
> Sent: Tuesday, February 2, 2016 11:32 AM
> To: Aaron Conole <aconole at redhat.com>; Christian Ehrhardt
> <christian.ehrhardt at canonical.com>
> Cc: <dev at openvswitch.org> <dev at openvswitch.org>; Flavio Leitner <fbl at sysclose.org>
> Subject: Re: [ovs-dev] [PATCH v7 0/5] Convert DPDK configuration from
> command line to DB based
>
> 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.
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
> --------------------------------------------------------------
> 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