| Message ID | 1610732270-2666-1-git-send-email-ian.stokes@intel.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v1,branch-2.14,1/2] acinclude: Strip out -march provided by DPDK. | expand |
On 1/15/21 6:37 PM, Ian Stokes wrote: > DPDK flags may include -march. Forcing -march could be > considered too heavy a requirement when users compile OVS from > source and could override user provided options. > > Resolve this by stripping -march from provided DPDK flags. > > Signed-off-by: Ian Stokes <ian.stokes@intel.com> > --- > acinclude.m4 | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/acinclude.m4 b/acinclude.m4 > index 857067a88..14fe4218a 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -436,6 +436,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > if test "$DPDK_AUTO_DISCOVER" = "false"; then > OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR" > fi > + # Force in pkg-config since this could override user-specified options. Seems like some word is missing in this sentence. > + # It's enough to have -mssse3 to build with DPDK headers. > + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ ]]*//g') > OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE" > OVS_ENABLE_OPTION([-mssse3]) > >
> On 1/15/21 6:37 PM, Ian Stokes wrote: > > DPDK flags may include -march. Forcing -march could be > > considered too heavy a requirement when users compile OVS from > > source and could override user provided options. > > > > Resolve this by stripping -march from provided DPDK flags. > > > > Signed-off-by: Ian Stokes <ian.stokes@intel.com> > > --- > > acinclude.m4 | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/acinclude.m4 b/acinclude.m4 > > index 857067a88..14fe4218a 100644 > > --- a/acinclude.m4 > > +++ b/acinclude.m4 > > @@ -436,6 +436,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > > if test "$DPDK_AUTO_DISCOVER" = "false"; then > > OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR" > > fi > > + # Force in pkg-config since this could override user-specified options. > > Seems like some word is missing in this sentence. Yes took this from the original patch but not crazy about it either. How about "Strip -march from DPDK flags since this could override user-specified option." BR Ian > > > + # It's enough to have -mssse3 to build with DPDK headers. > > + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ ]]*//g') > > OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE" > > OVS_ENABLE_OPTION([-mssse3]) > > > >
On 1/20/21 12:24 PM, Stokes, Ian wrote: >> On 1/15/21 6:37 PM, Ian Stokes wrote: >>> DPDK flags may include -march. Forcing -march could be >>> considered too heavy a requirement when users compile OVS from >>> source and could override user provided options. >>> >>> Resolve this by stripping -march from provided DPDK flags. >>> >>> Signed-off-by: Ian Stokes <ian.stokes@intel.com> >>> --- >>> acinclude.m4 | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/acinclude.m4 b/acinclude.m4 >>> index 857067a88..14fe4218a 100644 >>> --- a/acinclude.m4 >>> +++ b/acinclude.m4 >>> @@ -436,6 +436,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ >>> if test "$DPDK_AUTO_DISCOVER" = "false"; then >>> OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR" >>> fi >>> + # Force in pkg-config since this could override user-specified options. >> >> Seems like some word is missing in this sentence. > > Yes took this from the original patch but not crazy about it either. > > How about > > "Strip -march from DPDK flags since this could override user-specified option." I guess, Felix just didn't copy the first line of the comment from master. I think, we should just take it from there: # Stripping out possible instruction set specific configuration that DPDK # forces in pkg-config since this could override user-specified options. # It's enough to have -mssse3 to build with DPDK headers. > > BR > Ian >> >>> + # It's enough to have -mssse3 to build with DPDK headers. >>> + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ ]]*//g') >>> OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE" >>> OVS_ENABLE_OPTION([-mssse3]) >>> >>>
> On 1/20/21 12:24 PM, Stokes, Ian wrote: > >> On 1/15/21 6:37 PM, Ian Stokes wrote: > >>> DPDK flags may include -march. Forcing -march could be > >>> considered too heavy a requirement when users compile OVS from > >>> source and could override user provided options. > >>> > >>> Resolve this by stripping -march from provided DPDK flags. > >>> > >>> Signed-off-by: Ian Stokes <ian.stokes@intel.com> > >>> --- > >>> acinclude.m4 | 3 +++ > >>> 1 file changed, 3 insertions(+) > >>> > >>> diff --git a/acinclude.m4 b/acinclude.m4 > >>> index 857067a88..14fe4218a 100644 > >>> --- a/acinclude.m4 > >>> +++ b/acinclude.m4 > >>> @@ -436,6 +436,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > >>> if test "$DPDK_AUTO_DISCOVER" = "false"; then > >>> OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR" > >>> fi > >>> + # Force in pkg-config since this could override user-specified options. > >> > >> Seems like some word is missing in this sentence. > > > > Yes took this from the original patch but not crazy about it either. > > > > How about > > > > "Strip -march from DPDK flags since this could override user-specified > option." > > I guess, Felix just didn't copy the first line of the comment from master. > I think, we should just take it from there: > Ah, I get you, sure will use the original comment from master on commit so. Thanks Ian > # Stripping out possible instruction set specific configuration that DPDK > # forces in pkg-config since this could override user-specified options. > # It's enough to have -mssse3 to build with DPDK headers. > > > > > BR > > Ian > >> > >>> + # It's enough to have -mssse3 to build with DPDK headers. > >>> + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ > ]]*//g') > >>> OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE" > >>> OVS_ENABLE_OPTION([-mssse3]) > >>> > >>>
On 1/20/21 2:28 PM, Stokes, Ian wrote: >> On 1/20/21 12:24 PM, Stokes, Ian wrote: >>>> On 1/15/21 6:37 PM, Ian Stokes wrote: >>>>> DPDK flags may include -march. Forcing -march could be >>>>> considered too heavy a requirement when users compile OVS from >>>>> source and could override user provided options. >>>>> >>>>> Resolve this by stripping -march from provided DPDK flags. >>>>> >>>>> Signed-off-by: Ian Stokes <ian.stokes@intel.com> >>>>> --- >>>>> acinclude.m4 | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>> >>>>> diff --git a/acinclude.m4 b/acinclude.m4 >>>>> index 857067a88..14fe4218a 100644 >>>>> --- a/acinclude.m4 >>>>> +++ b/acinclude.m4 >>>>> @@ -436,6 +436,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ >>>>> if test "$DPDK_AUTO_DISCOVER" = "false"; then >>>>> OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR" >>>>> fi >>>>> + # Force in pkg-config since this could override user-specified options. >>>> >>>> Seems like some word is missing in this sentence. >>> >>> Yes took this from the original patch but not crazy about it either. >>> >>> How about >>> >>> "Strip -march from DPDK flags since this could override user-specified >> option." >> >> I guess, Felix just didn't copy the first line of the comment from master. >> I think, we should just take it from there: >> > > Ah, I get you, sure will use the original comment from master on commit so. Sounds good. With this change: Acked-by: Ilya Maximets <i.maximets@ovn.org> Same for branch-2.13 patch. > > Thanks > Ian > >> # Stripping out possible instruction set specific configuration that DPDK >> # forces in pkg-config since this could override user-specified options. >> # It's enough to have -mssse3 to build with DPDK headers. >> >>> >>> BR >>> Ian >>>> >>>>> + # It's enough to have -mssse3 to build with DPDK headers. >>>>> + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ >> ]]*//g') >>>>> OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE" >>>>> OVS_ENABLE_OPTION([-mssse3]) >>>>> >>>>> >
> On 1/20/21 2:28 PM, Stokes, Ian wrote: > >> On 1/20/21 12:24 PM, Stokes, Ian wrote: > >>>> On 1/15/21 6:37 PM, Ian Stokes wrote: > >>>>> DPDK flags may include -march. Forcing -march could be > >>>>> considered too heavy a requirement when users compile OVS from > >>>>> source and could override user provided options. > >>>>> > >>>>> Resolve this by stripping -march from provided DPDK flags. > >>>>> > >>>>> Signed-off-by: Ian Stokes <ian.stokes@intel.com> > >>>>> --- > >>>>> acinclude.m4 | 3 +++ > >>>>> 1 file changed, 3 insertions(+) > >>>>> > >>>>> diff --git a/acinclude.m4 b/acinclude.m4 > >>>>> index 857067a88..14fe4218a 100644 > >>>>> --- a/acinclude.m4 > >>>>> +++ b/acinclude.m4 > >>>>> @@ -436,6 +436,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > >>>>> if test "$DPDK_AUTO_DISCOVER" = "false"; then > >>>>> OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR" > >>>>> fi > >>>>> + # Force in pkg-config since this could override user-specified options. > >>>> > >>>> Seems like some word is missing in this sentence. > >>> > >>> Yes took this from the original patch but not crazy about it either. > >>> > >>> How about > >>> > >>> "Strip -march from DPDK flags since this could override user-specified > >> option." > >> > >> I guess, Felix just didn't copy the first line of the comment from master. > >> I think, we should just take it from there: > >> > > > > Ah, I get you, sure will use the original comment from master on commit so. > > Sounds good. > > With this change: > Acked-by: Ilya Maximets <i.maximets@ovn.org> > > Same for branch-2.13 patch. Thanks. Pushed the series to both 2.13 and 2.14 Regard Ian > > > > > Thanks > > Ian > > > >> # Stripping out possible instruction set specific configuration that DPDK > >> # forces in pkg-config since this could override user-specified options. > >> # It's enough to have -mssse3 to build with DPDK headers. > >> > >>> > >>> BR > >>> Ian > >>>> > >>>>> + # It's enough to have -mssse3 to build with DPDK headers. > >>>>> + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ > >> ]]*//g') > >>>>> OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE" > >>>>> OVS_ENABLE_OPTION([-mssse3]) > >>>>> > >>>>> > >
On 1/20/21 4:29 PM, Stokes, Ian wrote: >> On 1/20/21 2:28 PM, Stokes, Ian wrote: >>>> On 1/20/21 12:24 PM, Stokes, Ian wrote: >>>>>> On 1/15/21 6:37 PM, Ian Stokes wrote: >>>>>>> DPDK flags may include -march. Forcing -march could be >>>>>>> considered too heavy a requirement when users compile OVS from >>>>>>> source and could override user provided options. >>>>>>> >>>>>>> Resolve this by stripping -march from provided DPDK flags. >>>>>>> >>>>>>> Signed-off-by: Ian Stokes <ian.stokes@intel.com> >>>>>>> --- >>>>>>> acinclude.m4 | 3 +++ >>>>>>> 1 file changed, 3 insertions(+) >>>>>>> >>>>>>> diff --git a/acinclude.m4 b/acinclude.m4 >>>>>>> index 857067a88..14fe4218a 100644 >>>>>>> --- a/acinclude.m4 >>>>>>> +++ b/acinclude.m4 >>>>>>> @@ -436,6 +436,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ >>>>>>> if test "$DPDK_AUTO_DISCOVER" = "false"; then >>>>>>> OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR" >>>>>>> fi >>>>>>> + # Force in pkg-config since this could override user-specified options. >>>>>> >>>>>> Seems like some word is missing in this sentence. >>>>> >>>>> Yes took this from the original patch but not crazy about it either. >>>>> >>>>> How about >>>>> >>>>> "Strip -march from DPDK flags since this could override user-specified >>>> option." >>>> >>>> I guess, Felix just didn't copy the first line of the comment from master. >>>> I think, we should just take it from there: >>>> >>> >>> Ah, I get you, sure will use the original comment from master on commit so. >> >> Sounds good. >> >> With this change: >> Acked-by: Ilya Maximets <i.maximets@ovn.org> >> >> Same for branch-2.13 patch. > > Thanks. > > Pushed the series to both 2.13 and 2.14 It looks like you moved ')' while applying the second patch: - DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ ]]*//g') + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ ]]*//g' I don't think that it will work correctly. Force-push could solve this. :) > > Regard > Ian >> >>> >>> Thanks >>> Ian >>> >>>> # Stripping out possible instruction set specific configuration that DPDK >>>> # forces in pkg-config since this could override user-specified options. >>>> # It's enough to have -mssse3 to build with DPDK headers. >>>> >>>>> >>>>> BR >>>>> Ian >>>>>> >>>>>>> + # It's enough to have -mssse3 to build with DPDK headers. >>>>>>> + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ >>>> ]]*//g') >>>>>>> OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE" >>>>>>> OVS_ENABLE_OPTION([-mssse3]) >>>>>>> >>>>>>> >>> >
> On 1/20/21 4:29 PM, Stokes, Ian wrote: > >> On 1/20/21 2:28 PM, Stokes, Ian wrote: > >>>> On 1/20/21 12:24 PM, Stokes, Ian wrote: > >>>>>> On 1/15/21 6:37 PM, Ian Stokes wrote: > >>>>>>> DPDK flags may include -march. Forcing -march could be > >>>>>>> considered too heavy a requirement when users compile OVS from > >>>>>>> source and could override user provided options. > >>>>>>> > >>>>>>> Resolve this by stripping -march from provided DPDK flags. > >>>>>>> > >>>>>>> Signed-off-by: Ian Stokes <ian.stokes@intel.com> > >>>>>>> --- > >>>>>>> acinclude.m4 | 3 +++ > >>>>>>> 1 file changed, 3 insertions(+) > >>>>>>> > >>>>>>> diff --git a/acinclude.m4 b/acinclude.m4 > >>>>>>> index 857067a88..14fe4218a 100644 > >>>>>>> --- a/acinclude.m4 > >>>>>>> +++ b/acinclude.m4 > >>>>>>> @@ -436,6 +436,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > >>>>>>> if test "$DPDK_AUTO_DISCOVER" = "false"; then > >>>>>>> OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR" > >>>>>>> fi > >>>>>>> + # Force in pkg-config since this could override user-specified > options. > >>>>>> > >>>>>> Seems like some word is missing in this sentence. > >>>>> > >>>>> Yes took this from the original patch but not crazy about it either. > >>>>> > >>>>> How about > >>>>> > >>>>> "Strip -march from DPDK flags since this could override user-specified > >>>> option." > >>>> > >>>> I guess, Felix just didn't copy the first line of the comment from master. > >>>> I think, we should just take it from there: > >>>> > >>> > >>> Ah, I get you, sure will use the original comment from master on commit > so. > >> > >> Sounds good. > >> > >> With this change: > >> Acked-by: Ilya Maximets <i.maximets@ovn.org> > >> > >> Same for branch-2.13 patch. > > > > Thanks. > > > > Pushed the series to both 2.13 and 2.14 > > It looks like you moved ')' while applying the second patch: > > - DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ ]]*//g') > + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ ]]*//g' > > I don't think that it will work correctly. > > Force-push could solve this. :) > Thanks for the catch Ilya, resolved and pushed now. Ian > > > > Regard > > Ian > >> > >>> > >>> Thanks > >>> Ian > >>> > >>>> # Stripping out possible instruction set specific configuration that > DPDK > >>>> # forces in pkg-config since this could override user-specified options. > >>>> # It's enough to have -mssse3 to build with DPDK headers. > >>>> > >>>>> > >>>>> BR > >>>>> Ian > >>>>>> > >>>>>>> + # It's enough to have -mssse3 to build with DPDK headers. > >>>>>>> + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ > >>>> ]]*//g') > >>>>>>> OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE" > >>>>>>> OVS_ENABLE_OPTION([-mssse3]) > >>>>>>> > >>>>>>> > >>> > >
diff --git a/acinclude.m4 b/acinclude.m4 index 857067a88..14fe4218a 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -436,6 +436,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ if test "$DPDK_AUTO_DISCOVER" = "false"; then OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR" 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') OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE" OVS_ENABLE_OPTION([-mssse3])
DPDK flags may include -march. Forcing -march could be considered too heavy a requirement when users compile OVS from source and could override user provided options. Resolve this by stripping -march from provided DPDK flags. Signed-off-by: Ian Stokes <ian.stokes@intel.com> --- acinclude.m4 | 3 +++ 1 file changed, 3 insertions(+)