| Message ID | 20180418183023.2627-2-aconole@redhat.com |
|---|---|
| State | Superseded |
| Delegated to: | Ian Stokes |
| Headers | show |
| Series | dpdk: minor refactor of the initialization step | expand |
On 04/18/2018 07:30 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. > > Signed-off-by: Aaron Conole <aconole@redhat.com> > --- > lib/dpdk.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/lib/dpdk.c b/lib/dpdk.c > index 00dd97470..641474cde 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,10 +423,11 @@ dpdk_init__(const struct smap *ovs_other_config) > > /* Make sure things are initialized ... */ > result = rte_eal_init(argc, argv); > + argv_release(argv, argv_to_release, argc); > if (result < 0) { > - ovs_abort(result, "Cannot init EAL"); > + VLOG_EMER("Unable to initialize DPDK: %s", ovs_strerror(rte_errno)); > + return false; > } > - argv_release(argv, argv_to_release, argc); > > /* Set the main thread affinity back to pre rte_eal_init() value */ > if (auto_determine && !err) { On eal init failure, I think you should still execute this code block before returning to (possibly) restore the threads > @@ -459,6 +461,7 @@ dpdk_init__(const struct smap *ovs_other_config) > > /* Finally, register the dpdk classes */ > netdev_dpdk_register(); > + return true; > } > > void > @@ -476,10 +479,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"); >
Kevin Traynor <ktraynor@redhat.com> writes: Thanks, Kevin! > On 04/18/2018 07:30 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. >> >> Signed-off-by: Aaron Conole <aconole@redhat.com> >> --- >> lib/dpdk.c | 20 ++++++++++++++------ >> 1 file changed, 14 insertions(+), 6 deletions(-) >> >> diff --git a/lib/dpdk.c b/lib/dpdk.c >> index 00dd97470..641474cde 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,10 +423,11 @@ dpdk_init__(const struct smap *ovs_other_config) >> >> /* Make sure things are initialized ... */ >> result = rte_eal_init(argc, argv); >> + argv_release(argv, argv_to_release, argc); >> if (result < 0) { >> - ovs_abort(result, "Cannot init EAL"); >> + VLOG_EMER("Unable to initialize DPDK: %s", ovs_strerror(rte_errno)); >> + return false; >> } >> - argv_release(argv, argv_to_release, argc); >> >> /* Set the main thread affinity back to pre rte_eal_init() value */ >> if (auto_determine && !err) { > > On eal init failure, I think you should still execute this code block > before returning to (possibly) restore the threads Good point. >> @@ -459,6 +461,7 @@ dpdk_init__(const struct smap *ovs_other_config) >> >> /* Finally, register the dpdk classes */ >> netdev_dpdk_register(); >> + return true; >> } >> >> void >> @@ -476,10 +479,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"); >>
diff --git a/lib/dpdk.c b/lib/dpdk.c index 00dd97470..641474cde 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,10 +423,11 @@ dpdk_init__(const struct smap *ovs_other_config) /* Make sure things are initialized ... */ result = rte_eal_init(argc, argv); + argv_release(argv, argv_to_release, argc); if (result < 0) { - ovs_abort(result, "Cannot init EAL"); + VLOG_EMER("Unable to initialize DPDK: %s", ovs_strerror(rte_errno)); + return false; } - argv_release(argv, argv_to_release, argc); /* Set the main thread affinity back to pre rte_eal_init() value */ if (auto_determine && !err) { @@ -459,6 +461,7 @@ dpdk_init__(const struct smap *ovs_other_config) /* Finally, register the dpdk classes */ netdev_dpdk_register(); + return true; } void @@ -476,10 +479,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");
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 | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)