| Message ID | 1610732335-3775-1-git-send-email-ian.stokes@intel.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v1,branch-2.13,1/2] acinclude: Strip out -march provided by DPDK. | expand |
On 1/15/21 6:38 PM, Ian Stokes wrote: > DPDK forces '-mno-avx512f' flag for the application if the toolchain > used to build DPDK had broken AVX512 support. > > DPDK forces '-mno-avx512f' flag for the application if the toolchain > used to build DPDK had broken AVX512 support. But OVS could be built > with a completely different or fixed toolchain with correct avx512 > support. > > Fix that by stripping out `-mno-avx512f` as we already do for '-march'. > This will allow the OVS to decide if the AVX512 can be used. > > Reordering of CFLAGS (i.e. adding DPDK flags before OVS ones) is not an > option since autotools might reorder them back later and it's very > unpredictable. > > Reported-at: openvswitch/ovs-issues#201 I think, it's better to have a URL here. Could be changed on commit. Otherwise, LGTM. > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > Co-authored-by: Ilya Maximets <i.maximets@ovn.org> > Signed-off-by: Ian Stokes <ian.stokes@intel.com> > --- > acinclude.m4 | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/acinclude.m4 b/acinclude.m4 > index 9922c69b0..4033e28eb 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -433,7 +433,13 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > fi > # Force in pkg-config since this could override user-specified options. > # It's enough to have -mssse3 to build with DPDK headers. > - DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ ]]*//g') > + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ ]]*//g' > + # Also stripping out '-mno-avx512f'. Support for AVX512 will be disabled > + # if OVS will detect that it's broken. OVS could be built with a > + # completely different toolchain that correctly supports AVX512, flags > + # forced by DPDK only breaks our feature detection mechanism and leads to > + # build failures: https://github.com/openvswitch/ovs-issues/issues/201 > + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-mno-avx512f//g')) > OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE" > OVS_ENABLE_OPTION([-mssse3]) > >
> On 1/15/21 6:38 PM, Ian Stokes wrote: > > DPDK forces '-mno-avx512f' flag for the application if the toolchain > > used to build DPDK had broken AVX512 support. > > > > DPDK forces '-mno-avx512f' flag for the application if the toolchain > > used to build DPDK had broken AVX512 support. But OVS could be built > > with a completely different or fixed toolchain with correct avx512 > > support. > > > > Fix that by stripping out `-mno-avx512f` as we already do for '-march'. > > This will allow the OVS to decide if the AVX512 can be used. > > > > Reordering of CFLAGS (i.e. adding DPDK flags before OVS ones) is not an > > option since autotools might reorder them back later and it's very > > unpredictable. > > > > Reported-at: openvswitch/ovs-issues#201 > > I think, it's better to have a URL here. Could be changed on commit. > Otherwise, LGTM. Sure, will add URL on commit. I haven't seen any other comments from packagers so I'll look to merge today. Thanks Ian > > > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > > Co-authored-by: Ilya Maximets <i.maximets@ovn.org> > > Signed-off-by: Ian Stokes <ian.stokes@intel.com> > > --- > > acinclude.m4 | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/acinclude.m4 b/acinclude.m4 > > index 9922c69b0..4033e28eb 100644 > > --- a/acinclude.m4 > > +++ b/acinclude.m4 > > @@ -433,7 +433,13 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > > fi > > # Force in pkg-config since this could override user-specified options. > > # It's enough to have -mssse3 to build with DPDK headers. > > - DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ ]]*//g') > > + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ ]]*//g' > > + # Also stripping out '-mno-avx512f'. Support for AVX512 will be disabled > > + # if OVS will detect that it's broken. OVS could be built with a > > + # completely different toolchain that correctly supports AVX512, flags > > + # forced by DPDK only breaks our feature detection mechanism and > leads to > > + # build failures: https://github.com/openvswitch/ovs-issues/issues/201 > > + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-mno-avx512f//g')) > > OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE" > > OVS_ENABLE_OPTION([-mssse3]) > > > >
diff --git a/acinclude.m4 b/acinclude.m4 index 9922c69b0..4033e28eb 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -433,7 +433,13 @@ AC_DEFUN([OVS_CHECK_DPDK], [ fi # Force in pkg-config since this could override user-specified options. # It's enough to have -mssse3 to build with DPDK headers. - DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ ]]*//g') + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ ]]*//g' + # Also stripping out '-mno-avx512f'. Support for AVX512 will be disabled + # if OVS will detect that it's broken. OVS could be built with a + # completely different toolchain that correctly supports AVX512, flags + # forced by DPDK only breaks our feature detection mechanism and leads to + # build failures: https://github.com/openvswitch/ovs-issues/issues/201 + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-mno-avx512f//g')) OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE" OVS_ENABLE_OPTION([-mssse3])