[ovs-dev,1/2] dpdk: allow init to fail
diff mbox series

Message ID 20180503190801.20925-2-aconole@redhat.com
State Accepted
Delegated to: Ian Stokes
Headers show
Series
  • dpdk: refactor the initialization step
Related show

Commit Message

Aaron Conole May 3, 2018, 7:08 p.m. UTC
It's possible for dpdk initialization to fail either due to an internal
error or an invalid configuration.  When that happens, it's rather
impolite to immediately abort without any details.

With this change, a failed dpdk initialization attempt will continue to
trigger a SIGABRT.  However, the failure details will be logged, and a
user or administrator may have more information to correct the issue.
A restart of OvS would still be required to re-attempt initialization.

The refactor to propagate the init error will be used in an upcoming
commit.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 lib/dpdk.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

Comments

Kevin Traynor May 17, 2018, 8:44 p.m. UTC | #1
On 05/03/2018 08:08 PM, Aaron Conole wrote:
> It's possible for dpdk initialization to fail either due to an internal
> error or an invalid configuration.  When that happens, it's rather
> impolite to immediately abort without any details.
> 
> With this change, a failed dpdk initialization attempt will continue to
> trigger a SIGABRT.  However, the failure details will be logged, and a
> user or administrator may have more information to correct the issue.
> A restart of OvS would still be required to re-attempt initialization.
> 
> The refactor to propagate the init error will be used in an upcoming
> commit.
> 

LGTM
Acked-by: Kevin Traynor <ktraynor@redhat.com>

> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  lib/dpdk.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/dpdk.c b/lib/dpdk.c
> index 00dd97470..8bb686c43 100644
> --- a/lib/dpdk.c
> +++ b/lib/dpdk.c
> @@ -22,6 +22,7 @@
>  #include <sys/stat.h>
>  #include <getopt.h>
>  
> +#include <rte_errno.h>
>  #include <rte_log.h>
>  #include <rte_memzone.h>
>  #include <rte_version.h>
> @@ -306,7 +307,7 @@ static cookie_io_functions_t dpdk_log_func = {
>      .write = dpdk_log_write,
>  };
>  
> -static void
> +static bool
>  dpdk_init__(const struct smap *ovs_other_config)
>  {
>      char **argv = NULL, **argv_to_release = NULL;
> @@ -422,9 +423,6 @@ dpdk_init__(const struct smap *ovs_other_config)
>  
>      /* Make sure things are initialized ... */
>      result = rte_eal_init(argc, argv);
> -    if (result < 0) {
> -        ovs_abort(result, "Cannot init EAL");
> -    }
>      argv_release(argv, argv_to_release, argc);
>  
>      /* Set the main thread affinity back to pre rte_eal_init() value */
> @@ -436,6 +434,11 @@ dpdk_init__(const struct smap *ovs_other_config)
>          }
>      }
>  
> +    if (result < 0) {
> +        VLOG_EMER("Unable to initialize DPDK: %s", ovs_strerror(rte_errno));
> +        return false;
> +    }
> +
>      rte_memzone_dump(stdout);
>  
>      /* We are called from the main thread here */
> @@ -459,6 +462,7 @@ dpdk_init__(const struct smap *ovs_other_config)
>  
>      /* Finally, register the dpdk classes */
>      netdev_dpdk_register();
> +    return true;
>  }
>  
>  void
> @@ -476,10 +480,15 @@ dpdk_init(const struct smap *ovs_other_config)
>          if (ovsthread_once_start(&once_enable)) {
>              VLOG_INFO("Using %s", rte_version());
>              VLOG_INFO("DPDK Enabled - initializing...");
> -            dpdk_init__(ovs_other_config);
> -            enabled = true;
> -            VLOG_INFO("DPDK Enabled - initialized");
> +            enabled = dpdk_init__(ovs_other_config);
> +            if (enabled) {
> +                VLOG_INFO("DPDK Enabled - initialized");
> +            } else {
> +                ovs_abort(rte_errno, "Cannot init EAL");
> +            }
>              ovsthread_once_done(&once_enable);
> +        } else {
> +            VLOG_ERR_ONCE("DPDK Initialization Failed.");
>          }
>      } else {
>          VLOG_INFO_ONCE("DPDK Disabled - Use other_config:dpdk-init to enable");
>
Stokes, Ian May 22, 2018, 2:44 p.m. UTC | #2
> It's possible for dpdk initialization to fail either due to an internal
> error or an invalid configuration.  When that happens, it's rather
> impolite to immediately abort without any details.
> 
> With this change, a failed dpdk initialization attempt will continue to
> trigger a SIGABRT.  However, the failure details will be logged, and a
> user or administrator may have more information to correct the issue.
> A restart of OvS would still be required to re-attempt initialization.
> 
> The refactor to propagate the init error will be used in an upcoming
> commit.
> 

Thanks for this Aaron, I'll apply this to pull request this week.

Regards
Ian

> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  lib/dpdk.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/dpdk.c b/lib/dpdk.c
> index 00dd97470..8bb686c43 100644
> --- a/lib/dpdk.c
> +++ b/lib/dpdk.c
> @@ -22,6 +22,7 @@
>  #include <sys/stat.h>
>  #include <getopt.h>
> 
> +#include <rte_errno.h>
>  #include <rte_log.h>
>  #include <rte_memzone.h>
>  #include <rte_version.h>
> @@ -306,7 +307,7 @@ static cookie_io_functions_t dpdk_log_func = {
>      .write = dpdk_log_write,
>  };
> 
> -static void
> +static bool
>  dpdk_init__(const struct smap *ovs_other_config)  {
>      char **argv = NULL, **argv_to_release = NULL; @@ -422,9 +423,6 @@
> dpdk_init__(const struct smap *ovs_other_config)
> 
>      /* Make sure things are initialized ... */
>      result = rte_eal_init(argc, argv);
> -    if (result < 0) {
> -        ovs_abort(result, "Cannot init EAL");
> -    }
>      argv_release(argv, argv_to_release, argc);
> 
>      /* Set the main thread affinity back to pre rte_eal_init() value */
> @@ -436,6 +434,11 @@ dpdk_init__(const struct smap *ovs_other_config)
>          }
>      }
> 
> +    if (result < 0) {
> +        VLOG_EMER("Unable to initialize DPDK: %s",
> ovs_strerror(rte_errno));
> +        return false;
> +    }
> +
>      rte_memzone_dump(stdout);
> 
>      /* We are called from the main thread here */ @@ -459,6 +462,7 @@
> dpdk_init__(const struct smap *ovs_other_config)
> 
>      /* Finally, register the dpdk classes */
>      netdev_dpdk_register();
> +    return true;
>  }
> 
>  void
> @@ -476,10 +480,15 @@ dpdk_init(const struct smap *ovs_other_config)
>          if (ovsthread_once_start(&once_enable)) {
>              VLOG_INFO("Using %s", rte_version());
>              VLOG_INFO("DPDK Enabled - initializing...");
> -            dpdk_init__(ovs_other_config);
> -            enabled = true;
> -            VLOG_INFO("DPDK Enabled - initialized");
> +            enabled = dpdk_init__(ovs_other_config);
> +            if (enabled) {
> +                VLOG_INFO("DPDK Enabled - initialized");
> +            } else {
> +                ovs_abort(rte_errno, "Cannot init EAL");
> +            }
>              ovsthread_once_done(&once_enable);
> +        } else {
> +            VLOG_ERR_ONCE("DPDK Initialization Failed.");
>          }
>      } else {
>          VLOG_INFO_ONCE("DPDK Disabled - Use other_config:dpdk-init to
> enable");
> --
> 2.14.3

Patch
diff mbox series

diff --git a/lib/dpdk.c b/lib/dpdk.c
index 00dd97470..8bb686c43 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -22,6 +22,7 @@ 
 #include <sys/stat.h>
 #include <getopt.h>
 
+#include <rte_errno.h>
 #include <rte_log.h>
 #include <rte_memzone.h>
 #include <rte_version.h>
@@ -306,7 +307,7 @@  static cookie_io_functions_t dpdk_log_func = {
     .write = dpdk_log_write,
 };
 
-static void
+static bool
 dpdk_init__(const struct smap *ovs_other_config)
 {
     char **argv = NULL, **argv_to_release = NULL;
@@ -422,9 +423,6 @@  dpdk_init__(const struct smap *ovs_other_config)
 
     /* Make sure things are initialized ... */
     result = rte_eal_init(argc, argv);
-    if (result < 0) {
-        ovs_abort(result, "Cannot init EAL");
-    }
     argv_release(argv, argv_to_release, argc);
 
     /* Set the main thread affinity back to pre rte_eal_init() value */
@@ -436,6 +434,11 @@  dpdk_init__(const struct smap *ovs_other_config)
         }
     }
 
+    if (result < 0) {
+        VLOG_EMER("Unable to initialize DPDK: %s", ovs_strerror(rte_errno));
+        return false;
+    }
+
     rte_memzone_dump(stdout);
 
     /* We are called from the main thread here */
@@ -459,6 +462,7 @@  dpdk_init__(const struct smap *ovs_other_config)
 
     /* Finally, register the dpdk classes */
     netdev_dpdk_register();
+    return true;
 }
 
 void
@@ -476,10 +480,15 @@  dpdk_init(const struct smap *ovs_other_config)
         if (ovsthread_once_start(&once_enable)) {
             VLOG_INFO("Using %s", rte_version());
             VLOG_INFO("DPDK Enabled - initializing...");
-            dpdk_init__(ovs_other_config);
-            enabled = true;
-            VLOG_INFO("DPDK Enabled - initialized");
+            enabled = dpdk_init__(ovs_other_config);
+            if (enabled) {
+                VLOG_INFO("DPDK Enabled - initialized");
+            } else {
+                ovs_abort(rte_errno, "Cannot init EAL");
+            }
             ovsthread_once_done(&once_enable);
+        } else {
+            VLOG_ERR_ONCE("DPDK Initialization Failed.");
         }
     } else {
         VLOG_INFO_ONCE("DPDK Disabled - Use other_config:dpdk-init to enable");