| Message ID | 20200224160330.3682-1-harry.van.haaren@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] dpcls: enable cpu feature detection | expand |
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
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);
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(+)