| Message ID | 20200610104839.54608-5-harry.van.haaren@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | DPCLS Subtable ISA Optimization | expand |
btw, remember to add "." at the end of the commit title. so "dpcls: enable cpu feature detection." On Wed, Jun 10, 2020 at 3:47 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 select a function > implementation that uses the best ISA available on the CPU being used. > > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com> > --- > 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) > +{ > + 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); why are "avx512f" and "bmi2" hard-coded here? I thought this function "dpdk_get_cpu_has_isa" allows you to check any cpu feature. Regards, William > + > + 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 */ > -- > 2.17.1 >
> -----Original Message----- > From: William Tu <u9012063@gmail.com> > Sent: Tuesday, June 16, 2020 4:41 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 v3 4/7] dpcls: enable cpu feature detection > > btw, remember to add "." at the end of the commit title. > so > "dpcls: enable cpu feature detection." Ah thanks, will do. <snip patch details> > > + CHECK_CPU_FEATURE(feature, "avx512f", RTE_CPUFLAG_AVX512F); > > + CHECK_CPU_FEATURE(feature, "bmi2", RTE_CPUFLAG_BMI2); > > why are "avx512f" and "bmi2" hard-coded here? > I thought this function "dpdk_get_cpu_has_isa" allows you to check any > cpu feature. The strings are hard-coded here to abstract the OVS APIs from DPDK rte_cpu_flag_t. Indeed the implementation here can be extended to check any CPU flags, but would require some additional CHECK_CPU_FEATURE() macros. The number of ISA combinations actually used is generally limited to < 10, so I see no issue with implementing as individual checks. It is possible to change the API to directly use RTE_CPUFLAG_*, however I'm not sure that's a clean API. DPDK uses an "__extension__ enum rte_cpu_flag_t" to define CPU flags, which feels like it would better be contained in a .c file inside OVS. To contain the rte_cpu_flag_t in a .c I wrapped it into a "string arch, string cpu_feature" combo, which I believe is much more flexible, and a more generic API for OVS. /* An alternative API, but IMO it is a worse solution due to DPDK datatype in an OVS API. */ int dpdk_get_cpu_has_isa(const char *arch, enum rte_cpu_flag_t cpuflag);
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 select a function implementation that uses the best ISA available on the CPU being used. Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com> --- lib/dpdk-stub.c | 13 +++++++++++++ lib/dpdk.c | 27 +++++++++++++++++++++++++++ lib/dpdk.h | 2 ++ 3 files changed, 42 insertions(+)