| Message ID | 20200618165354.87787-5-harry.van.haaren@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | DPCLS Subtable ISA Optimization | expand |
On Thu, Jun 18, 2020 at 9:53 AM Harry van Haaren <harry.van.haaren@intel.com> wrote: > > This commit implements a method to retrieve the CPU ISA capabilities. > These ISA capabilities can be used in OVS to at runtime select a function > implementation to make the best use of the available ISA on the CPU. > > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com> > > --- > > v4: > - Improve commit title and message > --- > lib/dpdk-stub.c | 13 +++++++++++++ > lib/dpdk.c | 27 +++++++++++++++++++++++++++ > lib/dpdk.h | 2 ++ > 3 files changed, 42 insertions(+) > > diff --git a/lib/dpdk-stub.c b/lib/dpdk-stub.c > index c332c217c..9935f3d2b 100644 > --- a/lib/dpdk-stub.c > +++ b/lib/dpdk-stub.c > @@ -79,6 +79,19 @@ print_dpdk_version(void) > { > } > > +int > +dpdk_get_cpu_has_isa(const char *arch OVS_UNUSED, > + const char *feature OVS_UNUSED) should we just have bool as a return type? > +{ > + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; > + if (ovsthread_once_start(&once)) { > + VLOG_ERR("DPDK not supported in this version of Open vSwitch, " > + "cannot use CPU flag based optimizations"); > + ovsthread_once_done(&once); How about using VLOG_ERR_ONCE()? > + } > + return 0; and just return false here. The rest looks good to me, thanks William
> -----Original Message----- > From: William Tu <u9012063@gmail.com> > Sent: Saturday, June 27, 2020 7:27 PM > To: Van Haaren, Harry <harry.van.haaren@intel.com> > Cc: ovs-dev <ovs-dev@openvswitch.org>; Stokes, Ian <ian.stokes@intel.com>; > Ilya Maximets <i.maximets@ovn.org>; Federico Iezzi <fiezzi@redhat.com> > Subject: Re: [PATCH v4 4/7] dpdk: enable cpu feature detection. > > On Thu, Jun 18, 2020 at 9:53 AM Harry van Haaren > <harry.van.haaren@intel.com> wrote: > > > > This commit implements a method to retrieve the CPU ISA capabilities. > > These ISA capabilities can be used in OVS to at runtime select a function > > implementation to make the best use of the available ISA on the CPU. > > > > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com> > > > > --- > > > > v4: > > - Improve commit title and message > > --- > > lib/dpdk-stub.c | 13 +++++++++++++ > > lib/dpdk.c | 27 +++++++++++++++++++++++++++ > > lib/dpdk.h | 2 ++ > > 3 files changed, 42 insertions(+) > > > > diff --git a/lib/dpdk-stub.c b/lib/dpdk-stub.c > > index c332c217c..9935f3d2b 100644 > > --- a/lib/dpdk-stub.c > > +++ b/lib/dpdk-stub.c > > @@ -79,6 +79,19 @@ print_dpdk_version(void) > > { > > } > > > > +int > > +dpdk_get_cpu_has_isa(const char *arch OVS_UNUSED, > > + const char *feature OVS_UNUSED) > > should we just have bool as a return type? Sure yes, will fix. > > +{ > > + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; > > + if (ovsthread_once_start(&once)) { > > + VLOG_ERR("DPDK not supported in this version of Open vSwitch, " > > + "cannot use CPU flag based optimizations"); > > + ovsthread_once_done(&once); > > How about using VLOG_ERR_ONCE()? Ah nice - indeed manually doing the ovsthread_once_start() etc felt a little clunky. Updated in v5. > > + } > > + return 0; > and just return false here. Yep, will do. > The rest looks good to me, thanks > > William Cheers!
diff --git a/lib/dpdk-stub.c b/lib/dpdk-stub.c index c332c217c..9935f3d2b 100644 --- a/lib/dpdk-stub.c +++ b/lib/dpdk-stub.c @@ -79,6 +79,19 @@ print_dpdk_version(void) { } +int +dpdk_get_cpu_has_isa(const char *arch OVS_UNUSED, + const char *feature OVS_UNUSED) +{ + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; + if (ovsthread_once_start(&once)) { + VLOG_ERR("DPDK not supported in this version of Open vSwitch, " + "cannot use CPU flag based optimizations"); + ovsthread_once_done(&once); + } + return 0; +} + void dpdk_status(const struct ovsrec_open_vswitch *cfg) { diff --git a/lib/dpdk.c b/lib/dpdk.c index 31450d470..3bea65859 100644 --- a/lib/dpdk.c +++ b/lib/dpdk.c @@ -22,6 +22,7 @@ #include <sys/stat.h> #include <getopt.h> +#include <rte_cpuflags.h> #include <rte_errno.h> #include <rte_log.h> #include <rte_memzone.h> @@ -513,6 +514,32 @@ print_dpdk_version(void) puts(rte_version()); } +#define CHECK_CPU_FEATURE(feature, name_str, RTE_CPUFLAG) \ + do { \ + if (strncmp(feature, name_str, strlen(name_str)) == 0) { \ + int has_isa = rte_cpu_get_flag_enabled(RTE_CPUFLAG); \ + VLOG_DBG("CPU flag %s, available %s\n", name_str, \ + has_isa ? "yes" : "no"); \ + return has_isa; \ + } \ + } while (0) + +int +dpdk_get_cpu_has_isa(const char *arch, const char *feature) +{ + /* Ensure Arch is x86_64 */ + if (strncmp(arch, "x86_64", 6) != 0) { + return 0; + } + + CHECK_CPU_FEATURE(feature, "avx512f", RTE_CPUFLAG_AVX512F); + CHECK_CPU_FEATURE(feature, "bmi2", RTE_CPUFLAG_BMI2); + + VLOG_WARN("Unknown CPU arch,feature: %s,%s. Returning not supported.\n", + arch, feature); + return 0; +} + void dpdk_status(const struct ovsrec_open_vswitch *cfg) { diff --git a/lib/dpdk.h b/lib/dpdk.h index 736a64279..818dfcbba 100644 --- a/lib/dpdk.h +++ b/lib/dpdk.h @@ -44,4 +44,6 @@ bool dpdk_per_port_memory(void); bool dpdk_available(void); void print_dpdk_version(void); void dpdk_status(const struct ovsrec_open_vswitch *); +int dpdk_get_cpu_has_isa(const char * arch, const char *feature); + #endif /* dpdk.h */
This commit implements a method to retrieve the CPU ISA capabilities. These ISA capabilities can be used in OVS to at runtime select a function implementation to make the best use of the available ISA on the CPU. Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com> --- v4: - Improve commit title and message --- lib/dpdk-stub.c | 13 +++++++++++++ lib/dpdk.c | 27 +++++++++++++++++++++++++++ lib/dpdk.h | 2 ++ 3 files changed, 42 insertions(+)