[ovs-dev] dpcls: enable cpu feature detection
diff mbox series

Message ID 20200224160330.3682-1-harry.van.haaren@intel.com
State New
Headers show
Series
  • [ovs-dev] dpcls: enable cpu feature detection
Related show

Commit Message

Harry van Haaren Feb. 24, 2020, 4:03 p.m. UTC
This commit implements a method to retrieve the
CPU ISA capabilities, and allows each DPCLS implementation
to use these ISA capabilities to decide if the specific
implemenation is a valid candidate for the CPU.

Please note this is a POC patch only, the actual generic
code here can always run. Future DPCLS implementations may
require specific ISA, which is what this patch enables.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 lib/dpdk-stub.c                  | 11 +++++++++++
 lib/dpdk.c                       | 16 ++++++++++++++++
 lib/dpdk.h                       |  7 +++++++
 lib/dpif-netdev-lookup-generic.c |  7 +++++++
 4 files changed, 41 insertions(+)

Comments

0-day Robot Feb. 24, 2020, 5:03 p.m. UTC | #1
Bleep bloop.  Greetings Harry van Haaren, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 109 characters long (recommended limit is 79)
#36 FILE: lib/dpdk-stub.c:87:
        VLOG_ERR("DPDK not supported in this copy of Open vSwitch, cannot use CPU flag based optimizations");

ERROR: Inappropriate bracing around statement
#65 FILE: lib/dpdk.c:533:
    if (strncmp(arch, "x86_64", 6))

ERROR: Inappropriate bracing around statement
#68 FILE: lib/dpdk.c:536:
    if (strncmp(feature, "avx512f", 7) == 0)

ERROR: Inappropriate bracing around statement
#107 FILE: lib/dpif-netdev-lookup-generic.c:308:
    if (!avx512f_available)

Lines checked: 115, Warnings: 1, Errors: 3


build:
depbase=`echo lib/route-table.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT lib/route-table.lo -MD -MP -MF $depbase.Tpo -c -o lib/route-table.lo lib/route-table.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/route-table.lo -MD -MP -MF lib/.deps/route-table.Tpo -c lib/route-table.c -o lib/route-table.o
depbase=`echo lib/tc.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT lib/tc.lo -MD -MP -MF $depbase.Tpo -c -o lib/tc.lo lib/tc.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/tc.lo -MD -MP -MF lib/.deps/tc.Tpo -c lib/tc.c -o lib/tc.o
depbase=`echo lib/dpdk-stub.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT lib/dpdk-stub.lo -MD -MP -MF $depbase.Tpo -c -o lib/dpdk-stub.lo lib/dpdk-stub.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/dpdk-stub.lo -MD -MP -MF lib/.deps/dpdk-stub.Tpo -c lib/dpdk-stub.c -o lib/dpdk-stub.o
lib/dpdk-stub.c: In function 'dpdk_get_cpu_has_isa':
lib/dpdk-stub.c:83:34: error: unused parameter 'arch' [-Werror=unused-parameter]
 dpdk_get_cpu_has_isa(const char *arch, const char *feature)
                                  ^
lib/dpdk-stub.c:83:52: error: unused parameter 'feature' [-Werror=unused-parameter]
 dpdk_get_cpu_has_isa(const char *arch, const char *feature)
                                                    ^
cc1: all warnings being treated as errors
make[2]: *** [lib/dpdk-stub.lo] Error 1
make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make: *** [all] Error 2


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot

Patch
diff mbox series

diff --git a/lib/dpdk-stub.c b/lib/dpdk-stub.c
index c332c217c..0aaccd983 100644
--- a/lib/dpdk-stub.c
+++ b/lib/dpdk-stub.c
@@ -79,6 +79,17 @@  print_dpdk_version(void)
 {
 }
 
+int
+dpdk_get_cpu_has_isa(const char *arch, const char *feature)
+{
+    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
+    if (ovsthread_once_start(&once)) {
+        VLOG_ERR("DPDK not supported in this copy 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 37ea2973c..927dc6867 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>
@@ -525,6 +526,21 @@  print_dpdk_version(void)
     puts(rte_version());
 }
 
+int
+dpdk_get_cpu_has_isa(const char *arch, const char *feature)
+{
+    /* Ensure Arch is x86_64 */
+    if (strncmp(arch, "x86_64", 6))
+        return 0;
+
+    if (strncmp(feature, "avx512f", 7) == 0)
+        return rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F) > 0;
+
+    VLOG_WARN("%s does not know the %s, %s combo, returning not supported.\n",
+              __func__, 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..6c2144624 100644
--- a/lib/dpdk.h
+++ b/lib/dpdk.h
@@ -44,4 +44,11 @@  bool dpdk_per_port_memory(void);
 bool dpdk_available(void);
 void print_dpdk_version(void);
 void dpdk_status(const struct ovsrec_open_vswitch *);
+
+/* Returns 1 if the CPU has support for the requested architecture specific
+ * ISA capability. Eg: "x86_64", "avx512f". The strings to use here are as
+ * shown in "lscpu" Architecture: and Flags: fields.
+ */
+int dpdk_get_cpu_has_isa(const char * arch, const char *feature);
+
 #endif /* dpdk.h */
diff --git a/lib/dpif-netdev-lookup-generic.c b/lib/dpif-netdev-lookup-generic.c
index 89c8be0fa..dbc830512 100644
--- a/lib/dpif-netdev-lookup-generic.c
+++ b/lib/dpif-netdev-lookup-generic.c
@@ -301,6 +301,13 @@  DECLARE_OPTIMIZED_LOOKUP_FUNCTION(4, 0)
 dpcls_subtable_lookup_func
 dpcls_subtable_generic_probe(uint32_t u0_bits, uint32_t u1_bits)
 {
+    /* POC only: check if we can use CPU based info */
+    int avx512f_available = dpdk_get_cpu_has_isa("x86_64", "avx512f");
+    printf("avx512f available? %d\n", avx512f_available);
+
+    if (!avx512f_available)
+        return 0;
+
     dpcls_subtable_lookup_func f = NULL;
 
     CHECK_LOOKUP_FUNCTION(5, 1);