[dpdk-dev,v4,06/26] eal-common: introduce a way to query cpu support
Checks
Commit Message
This adds a new API to check for the eal cpu versions.
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
lib/librte_eal/common/eal_common_cpuflags.c | 13 +++++++++++--
lib/librte_eal/common/include/generic/rte_cpuflags.h | 9 +++++++++
2 files changed, 20 insertions(+), 2 deletions(-)
Comments
On Sat, Feb 25, 2017 at 11:02:49AM -0500, Aaron Conole wrote:
> This adds a new API to check for the eal cpu versions.
>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
> lib/librte_eal/common/eal_common_cpuflags.c | 13 +++++++++++--
> lib/librte_eal/common/include/generic/rte_cpuflags.h | 9 +++++++++
> 2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_cpuflags.c b/lib/librte_eal/common/eal_common_cpuflags.c
> index b5f76f7..2c2127b 100644
> --- a/lib/librte_eal/common/eal_common_cpuflags.c
> +++ b/lib/librte_eal/common/eal_common_cpuflags.c
> @@ -43,6 +43,13 @@
> void
> rte_cpu_check_supported(void)
> {
> + if (!rte_cpu_is_supported())
> + exit(1);
> +}
> +
> +bool
> +rte_cpu_is_supported(void)
> +{
> /* This is generated at compile-time by the build system */
> static const enum rte_cpu_flag_t compile_time_flags[] = {
> RTE_COMPILE_TIME_CPUFLAGS
> @@ -57,14 +64,16 @@ rte_cpu_check_supported(void)
> fprintf(stderr,
> "ERROR: CPU feature flag lookup failed with error %d\n",
> ret);
> - exit(1);
> + return false;
> }
> if (!ret) {
> fprintf(stderr,
> "ERROR: This system does not support \"%s\".\n"
> "Please check that RTE_MACHINE is set correctly.\n",
> rte_cpu_get_flag_name(compile_time_flags[i]));
> - exit(1);
> + return false;
> }
> }
> +
> + return true;
> }
> diff --git a/lib/librte_eal/common/include/generic/rte_cpuflags.h b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> index 71321f3..e4342ad 100644
> --- a/lib/librte_eal/common/include/generic/rte_cpuflags.h
> +++ b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> @@ -40,6 +40,7 @@
> */
>
> #include <errno.h>
> +#include <stdbool.h>
>
The addition of this include is causing all sorts of compilation errors
inside the PMDs, as many of them seem to be defining their own bools
types. :-(
For safety sake, probably best to have the function return int rather
than bool.
/Bruce
Bruce Richardson <bruce.richardson@intel.com> writes:
> On Sat, Feb 25, 2017 at 11:02:49AM -0500, Aaron Conole wrote:
>> This adds a new API to check for the eal cpu versions.
>>
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>> lib/librte_eal/common/eal_common_cpuflags.c | 13 +++++++++++--
>> lib/librte_eal/common/include/generic/rte_cpuflags.h | 9 +++++++++
>> 2 files changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/librte_eal/common/eal_common_cpuflags.c b/lib/librte_eal/common/eal_common_cpuflags.c
>> index b5f76f7..2c2127b 100644
>> --- a/lib/librte_eal/common/eal_common_cpuflags.c
>> +++ b/lib/librte_eal/common/eal_common_cpuflags.c
>> @@ -43,6 +43,13 @@
>> void
>> rte_cpu_check_supported(void)
>> {
>> + if (!rte_cpu_is_supported())
>> + exit(1);
>> +}
>> +
>> +bool
>> +rte_cpu_is_supported(void)
>> +{
>> /* This is generated at compile-time by the build system */
>> static const enum rte_cpu_flag_t compile_time_flags[] = {
>> RTE_COMPILE_TIME_CPUFLAGS
>> @@ -57,14 +64,16 @@ rte_cpu_check_supported(void)
>> fprintf(stderr,
>> "ERROR: CPU feature flag lookup failed with error %d\n",
>> ret);
>> - exit(1);
>> + return false;
>> }
>> if (!ret) {
>> fprintf(stderr,
>> "ERROR: This system does not support \"%s\".\n"
>> "Please check that RTE_MACHINE is set correctly.\n",
>> rte_cpu_get_flag_name(compile_time_flags[i]));
>> - exit(1);
>> + return false;
>> }
>> }
>> +
>> + return true;
>> }
>> diff --git a/lib/librte_eal/common/include/generic/rte_cpuflags.h b/lib/librte_eal/common/include/generic/rte_cpuflags.h
>> index 71321f3..e4342ad 100644
>> --- a/lib/librte_eal/common/include/generic/rte_cpuflags.h
>> +++ b/lib/librte_eal/common/include/generic/rte_cpuflags.h
>> @@ -40,6 +40,7 @@
>> */
>>
>> #include <errno.h>
>> +#include <stdbool.h>
>>
>
> The addition of this include is causing all sorts of compilation errors
> inside the PMDs, as many of them seem to be defining their own bools
> types. :-(
>
> For safety sake, probably best to have the function return int rather
> than bool.
Will do - I never saw the issue, but perhaps I was excluding the PMDs
in question.
Thanks for the review, Bruce!
On Mon, Feb 27, 2017 at 09:33:19AM -0500, Aaron Conole wrote:
> Bruce Richardson <bruce.richardson@intel.com> writes:
>
> > On Sat, Feb 25, 2017 at 11:02:49AM -0500, Aaron Conole wrote:
> >> This adds a new API to check for the eal cpu versions.
> >>
> >> Signed-off-by: Aaron Conole <aconole@redhat.com>
> >> ---
> >> lib/librte_eal/common/eal_common_cpuflags.c | 13 +++++++++++--
> >> lib/librte_eal/common/include/generic/rte_cpuflags.h | 9 +++++++++
> >> 2 files changed, 20 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/lib/librte_eal/common/eal_common_cpuflags.c b/lib/librte_eal/common/eal_common_cpuflags.c
> >> index b5f76f7..2c2127b 100644
> >> --- a/lib/librte_eal/common/eal_common_cpuflags.c
> >> +++ b/lib/librte_eal/common/eal_common_cpuflags.c
> >> @@ -43,6 +43,13 @@
> >> void
> >> rte_cpu_check_supported(void)
> >> {
> >> + if (!rte_cpu_is_supported())
> >> + exit(1);
> >> +}
> >> +
> >> +bool
> >> +rte_cpu_is_supported(void)
> >> +{
> >> /* This is generated at compile-time by the build system */
> >> static const enum rte_cpu_flag_t compile_time_flags[] = {
> >> RTE_COMPILE_TIME_CPUFLAGS
> >> @@ -57,14 +64,16 @@ rte_cpu_check_supported(void)
> >> fprintf(stderr,
> >> "ERROR: CPU feature flag lookup failed with error %d\n",
> >> ret);
> >> - exit(1);
> >> + return false;
> >> }
> >> if (!ret) {
> >> fprintf(stderr,
> >> "ERROR: This system does not support \"%s\".\n"
> >> "Please check that RTE_MACHINE is set correctly.\n",
> >> rte_cpu_get_flag_name(compile_time_flags[i]));
> >> - exit(1);
> >> + return false;
> >> }
> >> }
> >> +
> >> + return true;
> >> }
> >> diff --git a/lib/librte_eal/common/include/generic/rte_cpuflags.h b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> >> index 71321f3..e4342ad 100644
> >> --- a/lib/librte_eal/common/include/generic/rte_cpuflags.h
> >> +++ b/lib/librte_eal/common/include/generic/rte_cpuflags.h
> >> @@ -40,6 +40,7 @@
> >> */
> >>
> >> #include <errno.h>
> >> +#include <stdbool.h>
> >>
> >
> > The addition of this include is causing all sorts of compilation errors
> > inside the PMDs, as many of them seem to be defining their own bools
> > types. :-(
> >
> > For safety sake, probably best to have the function return int rather
> > than bool.
>
> Will do - I never saw the issue, but perhaps I was excluding the PMDs
> in question.
>
> Thanks for the review, Bruce!
No problem.
FYI, the drivers I saw the errors in with this patch are:
* qede
* i40e
* ixgbe
* cxgbe
Regards,
/Bruce
@@ -43,6 +43,13 @@
void
rte_cpu_check_supported(void)
{
+ if (!rte_cpu_is_supported())
+ exit(1);
+}
+
+bool
+rte_cpu_is_supported(void)
+{
/* This is generated at compile-time by the build system */
static const enum rte_cpu_flag_t compile_time_flags[] = {
RTE_COMPILE_TIME_CPUFLAGS
@@ -57,14 +64,16 @@ rte_cpu_check_supported(void)
fprintf(stderr,
"ERROR: CPU feature flag lookup failed with error %d\n",
ret);
- exit(1);
+ return false;
}
if (!ret) {
fprintf(stderr,
"ERROR: This system does not support \"%s\".\n"
"Please check that RTE_MACHINE is set correctly.\n",
rte_cpu_get_flag_name(compile_time_flags[i]));
- exit(1);
+ return false;
}
}
+
+ return true;
}
@@ -40,6 +40,7 @@
*/
#include <errno.h>
+#include <stdbool.h>
/**
* Enumeration of all CPU features supported
@@ -82,4 +83,12 @@ rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature);
void
rte_cpu_check_supported(void);
+/**
+ * This function checks that the currently used CPU supports the CPU features
+ * that were specified at compile time. It is called automatically within the
+ * EAL, so does not need to be used by applications. This version returns a
+ * result so that decisions may be made (for instance, graceful shutdowns).
+ */
+bool
+rte_cpu_is_supported(void);
#endif /* _RTE_CPUFLAGS_H_ */