[ovs-dev] [PATCH RFC] automake: fix rte_memcopy for gcc versions <=4.8.2

Kavanagh, Mark B mark.b.kavanagh at intel.com
Thu Apr 2 16:38:13 UTC 2015


>On 04/01/15 at 07:45am, Kavanagh, Mark B wrote:
>>
>> >What's wrong with setting CFLAGS on the "configure" or "make" command
>> >line?  This is the standard way to do this with Automake and Autoconf.
>> >
>>
>> Sure. However, setting CFLAGS on the command line overwrites any values CFLAGS has
>attained via the 'configure' step. The most obvious symptom of this is significantly
>degraded performance, due to the fact that the optimization flags passed to CFLAGS during
>'configure' are overwritten by the command line value of CFLAGS.
>
>That should not be the case at all. 9562639 fixed this and stores
>the flags collected by configure in OVS_CFLAGS while the user can
>specify his own CFLAGS.
>
>If there is a bug, let's fix the bug.

Hi Thomas,

I have little experience of automake, so apologies for any shortcomings in my understanding in this response.

I've done quite a bit of testing on this, and have observed that whenever CFLAGS are passed to 'make' on the command line (building against DPDK), the performance degrades by ~3x; this leads me to believe that the existing value of CFLAGS (namely -O2 -g) has been overwritten by its command line counterpart.

With respect to 9562639, I don't see where OVS_CFLAGS obtains its initial value (i.e. existing CFLAGS). I've made a slight modification, and found that it resolves the issue - maybe you could comment as to the validity of the changes?

diff --git a/acinclude.m4 b/acinclude.m4
index 479da2e..bfdbc88 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -204,8 +204,8 @@ AC_DEFUN([OVS_CHECK_DPDK], [
     fi
     CFLAGS="$ovs_save_CFLAGS"
     LDFLAGS="$ovs_save_LDFLAGS"
-    OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR"
-    OVS_CFLAGS="$OVS_CFLAGS -I$DPDK_INCLUDE"
+    OVS_LDFLAGS="$LDFLAGS $OVS_LDFLAGS -L$DPDK_LIB_DIR"
+    OVS_CFLAGS="$CFLAGS $OVS_CFLAGS -I$DPDK_INCLUDE"

     # DPDK pmd drivers are not linked unless --whole-archive is used.
     #

>
>> >We should certainly not introduce a configure option for every single
>> >possible compiler flag.
>>
>> Agreed; given the precedent set by OVS_ENABLE_WERROR, I figured that this was the
>convention.
>
>The only reason this still exists is for backwards compatibility
>to not break existing build scripts. Specifying CFLAGS=-Werror has
>the same effect and f.e. travis builds are doing exactly this.



More information about the dev mailing list