On 09/01/2017 07:14, "Aaron Conole" <[email protected]> wrote:
>Daniele Di Proietto <[email protected]> writes:
>
>> With this commit, we allow the user to set other_config:dpdk-init=true
>> after the process is started. This makes it easier to start Open
>> vSwitch with DPDK using standard init scripts without restarting the
>> service.
>>
>> This is still far from ideal, because initializing DPDK might still
>> abort the process (e.g. if there not enough memory), so the user must
>> check the status of the process after setting dpdk-init to true.
>>
>> Nonetheless, I think this is an improvement, because it doesn't require
>> restarting the whole unit.
>>
>> CC: Aaron Conole <[email protected]>
>> Signed-off-by: Daniele Di Proietto <[email protected]>
>> ---
>> v1->v2: No change, first non-RFC post.
>> ---
>
>Looks good - just one minor detail below
>
>> lib/dpdk-stub.c | 8 ++++----
>> lib/dpdk.c | 30 ++++++++++++++++++++----------
>> tests/ofproto-macros.at | 2 +-
>> 3 files changed, 25 insertions(+), 15 deletions(-)
>>
>> diff --git a/lib/dpdk-stub.c b/lib/dpdk-stub.c
>> index bd981bb90..daef7291f 100644
>> --- a/lib/dpdk-stub.c
>> +++ b/lib/dpdk-stub.c
>> @@ -27,13 +27,13 @@ VLOG_DEFINE_THIS_MODULE(dpdk);
>> void
>> dpdk_init(const struct smap *ovs_other_config)
>> {
>> - static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>> + if (smap_get_bool(ovs_other_config, "dpdk-init", false)) {
>> + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>>
>> - if (ovsthread_once_start(&once)) {
>> - if (smap_get_bool(ovs_other_config, "dpdk-init", false)) {
>> + if (ovsthread_once_start(&once)) {
>> VLOG_ERR("DPDK not supported in this copy of Open vSwitch.");
>> + ovsthread_once_done(&once);
>> }
>> - ovsthread_once_done(&once);
>> }
>> }
>>
>> diff --git a/lib/dpdk.c b/lib/dpdk.c
>> index ee4360b22..008c6c06d 100644
>> --- a/lib/dpdk.c
>> +++ b/lib/dpdk.c
>> @@ -273,12 +273,6 @@ dpdk_init__(const struct smap *ovs_other_config)
>> cpu_set_t cpuset;
>> char *sock_dir_subcomponent;
>>
>> - if (!smap_get_bool(ovs_other_config, "dpdk-init", false)) {
>> - VLOG_INFO("DPDK Disabled - to change this requires a restart.\n");
>> - return;
>> - }
>> -
>> - VLOG_INFO("DPDK Enabled, initializing");
>> if (process_vhost_flags("vhost-sock-dir", xstrdup(ovs_rundir()),
>> NAME_MAX, ovs_other_config,
>> &sock_dir_subcomponent)) {
>> @@ -413,11 +407,27 @@ dpdk_init__(const struct smap *ovs_other_config)
>> void
>> dpdk_init(const struct smap *ovs_other_config)
>> {
>> - static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>> + static bool enabled = false;
>
>This doesn't appear to be used, apart from the first test of the
>following conditional (where it will always pass to the second). Did I
>miss something?
Oops, it should be used.
I need to set it to true after calling dpdk_init__(), otherwise the following
scenario
might happen:
1) other_config:dpdk-init is set to "true"
2) vswitchd is started and dpdk_init__() is called
3) other_config:dpdk-init is set to "false"
4) The log message "DPDK Disabled - Use other_config:dpdk-init to enable" is
printed, giving the illusion that DPDK was disabled.
I'll add 'enable=true' in the next version.
Thanks,
Daniele
>
>> + if (enabled || !ovs_other_config) {
>> + return;
>> + }
>> +
>> + if (smap_get_bool(ovs_other_config, "dpdk-init", false)) {
>> + static struct ovsthread_once once_enable =
>> OVSTHREAD_ONCE_INITIALIZER;
>>
>> - if (ovs_other_config && ovsthread_once_start(&once)) {
>> - dpdk_init__(ovs_other_config);
>> - ovsthread_once_done(&once);
>> + if (ovsthread_once_start(&once_enable)) {
>> + VLOG_INFO("DPDK Enabled - initializing...");
>> + dpdk_init__(ovs_other_config);
>> + VLOG_INFO("DPDK Enabled - initialized");
>> + ovsthread_once_done(&once_enable);
>> + }
>> + } else {
>> + static struct ovsthread_once once_disable =
>> OVSTHREAD_ONCE_INITIALIZER;
>> + if (ovsthread_once_start(&once_disable)) {
>> + VLOG_INFO("DPDK Disabled - Use other_config:dpdk-init to
>> enable");
>> + ovsthread_once_done(&once_disable);
>> + }
>> }
>> }
>>
>> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
>> index 5477777b8..faff5b0a8 100644
>> --- a/tests/ofproto-macros.at
>> +++ b/tests/ofproto-macros.at
>> @@ -331,7 +331,7 @@ m4_define([_OVS_VSWITCHD_START],
>> /ofproto|INFO|using datapath ID/d
>> /netdev_linux|INFO|.*device has unknown hardware address family/d
>> /ofproto|INFO|datapath ID changed to fedcba9876543210/d
>> -/dpdk|INFO|DPDK Disabled - to change this requires a restart./d']])
>> +/dpdk|INFO|DPDK Disabled - Use other_config:dpdk-init to enable/d']])
>> ])
>>
>> # OVS_VSWITCHD_START([vsctl-args], [vsctl-output], [=override],
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
