[ovs-dev] [PATCH v2] dpdk: Late initialization.

Aaron Conole aconole at redhat.com
Mon Jan 9 15:14:48 UTC 2017


Daniele Di Proietto <diproiettod at vmware.com> 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 <aconole at redhat.com>
> Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
> ---
> 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?

> +    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],


More information about the dev mailing list