[ovs-dev] [PATCH 3/3] acinclude: Use SSE4.2 instruction set

[Bodireddy, Bhanuprakash]

Thanks for looking in to this Daniele, My comments inline.

>-----Original Message-----
>From: Daniele Di Proietto [mailto:diproiettod at ovn.org]
>Sent: Sunday, August 14, 2016 9:13 PM
>To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy at intel.com>
>Cc: dev at openvswitch.org
>Subject: Re: [ovs-dev] [PATCH 3/3] acinclude: Use SSE4.2 instruction set
>
>By default we do not want to build Open vSwitch for a particular CPU, we
>want to create a build that can be distributed to every CPU of the architecture
>(e.g. amd64).
>Unfortunately, this not entirely possible for with DPDK, because rte_memcpy
>(included by the headers in netdev-dpdk) requires SSSE3.  The 'default'
>machine in DPDK (which, if I'm not mistaken, should produce the most
>distributable build) does not work on every amd64, but requires -
>march=core2, i.e. -mssse3.
I got the reason behind using mssse3 from the previous discussions on the Mailing list.

>We shouldn't change this unless OvS+DPDK cannot work without sse4.2.  I
>don't think this is the case, right?
With out sse4.2, OVS DPDK uses murmur hash for hash computation. I found that performance
is better using crc32 intrinsics instead of murmurhash3. OVS DPDK is configured with SSSE3 by 
default and thought to override this with the patch. But I agree with your suggestion. 
Anyways we have a note in INSTALL.md on  msse4.2 and that should help users looking to leverage
built-in Intrinsics.

Regards,
Bhanu Prakash.

>Thanks,
>Daniele
>
>2016-08-14 11:35 GMT-07:00 Bhanuprakash Bodireddy
><bhanuprakash.bodireddy at intel.com>:
>On processors with SSE4.2 instruction set support, CRC32 intrinsics can
>be used for efficient hash computation.
>
>Update the m4_translit to convert '.' to '_' that otherwise cause 'bad
>substitution' error when configuring OVS DPDK with msse4.2 support.
>
>  ./configure: line 21027: ${ovs_cv__msse4.2+:}: bad substitution
>
>Signed-off-by: Bhanuprakash Bodireddy
><bhanuprakash.bodireddy at intel.com>
>---
> acinclude.m4 | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/acinclude.m4 b/acinclude.m4
>index aa57b47..93c7a5a 100644
>--- a/acinclude.m4
>+++ b/acinclude.m4
>@@ -280,7 +280,7 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>       OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR"
>     fi
>     OVS_CFLAGS="$OVS_CFLAGS -I$DPDK_INCLUDE"
>-    OVS_ENABLE_OPTION([-mssse3])
>+    OVS_ENABLE_OPTION([-msse4.2])
>
>     # DPDK pmd drivers are not linked unless --whole-archive is used.
>     #
>@@ -776,7 +776,7 @@ dnl gives unlimited permission to copy and/or
>distribute it,
> dnl with or without modifications, as long as this notice is preserved.
>
> AC_DEFUN([_OVS_CHECK_CC_OPTION], [dnl
>-  m4_define([ovs_cv_name], [ovs_cv_[]m4_translit([$1], [-=], [__])])dnl
>+  m4_define([ovs_cv_name], [ovs_cv_[]m4_translit([$1], [-=.], [___])])dnl
>   AC_CACHE_CHECK([whether $CC accepts $1], [ovs_cv_name],
>     [ovs_save_CFLAGS="$CFLAGS"
>      dnl Include -Werror in the compiler options, because without -Werror
>--
>2.4.11
>
>_______________________________________________
>dev mailing list
>dev at openvswitch.org
>http://openvswitch.org/mailman/listinfo/dev