Re: [dpdk-dev] [PATCH v3 2/2] config: disable CONFIG_RTE_SCHED_VECTOR for arm

[Jan Viktorin <viktorin@rehivetech.com>]

On Mon, 30 Nov 2015 13:55:45 -0500
Jianbo Liu <jianbo.liu@linaro.org> wrote:

> On Mon, Nov 30, 2015 at 03:52:31PM +0530, Jerin Jacob wrote:
> > On Mon, Nov 30, 2015 at 12:03:21PM -0500, Jianbo Liu wrote:  
> > > On Mon, Nov 30, 2015 at 11:17:52AM +0530, Jerin Jacob wrote:  
> > > > On Sun, Nov 29, 2015 at 06:48:29PM -0500, Jianbo Liu wrote:  
> > > > > On Fri, Nov 27, 2015 at 07:04:28PM +0530, Jerin Jacob wrote:  
> > > > > > Commit 42ec27a0178a causes compiling error on arm, as RTE_SCHED_VECTOR
> > > > > > does support only SSE intrinsic, so disable it till we have neon support.
> > > > > > 
> > > > > > Fixes: 42ec27a0178a ("sched: enable SSE optimizations in config")
> > > > > > 
> > > > > > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > > > > > ---
> > > > > >  config/common_arm64                      | 1 +
> > > > > >  config/defconfig_arm-armv7a-linuxapp-gcc | 1 +
> > > > > >  2 files changed, 2 insertions(+)
> > > > > > 
> > > > > > diff --git a/config/common_arm64 b/config/common_arm64
> > > > > > index 5e5e303..d6a9cb9 100644
> > > > > > --- a/config/common_arm64
> > > > > > +++ b/config/common_arm64
> > > > > > @@ -46,3 +46,4 @@ CONFIG_RTE_LIBRTE_I40E_PMD=n
> > > > > >  CONFIG_RTE_LIBRTE_LPM=n
> > > > > >  CONFIG_RTE_LIBRTE_TABLE=n
> > > > > >  CONFIG_RTE_LIBRTE_PIPELINE=n
> > > > > > +CONFIG_RTE_SCHED_VECTOR=n
> > > > > > diff --git a/config/defconfig_arm-armv7a-linuxapp-gcc b/config/defconfig_arm-armv7a-linuxapp-gcc
> > > > > > index 82143af..9924ff9 100644
> > > > > > --- a/config/defconfig_arm-armv7a-linuxapp-gcc
> > > > > > +++ b/config/defconfig_arm-armv7a-linuxapp-gcc
> > > > > > @@ -57,6 +57,7 @@ CONFIG_RTE_LIBRTE_ACL=n
> > > > > >  CONFIG_RTE_LIBRTE_LPM=n
> > > > > >  CONFIG_RTE_LIBRTE_TABLE=n
> > > > > >  CONFIG_RTE_LIBRTE_PIPELINE=n
> > > > > > +CONFIG_RTE_SCHED_VECTOR=n
> > > > > >  
> > > > > >  # cannot use those on ARM
> > > > > >  CONFIG_RTE_KNI_KMOD=n
> > > > > > -- 
> > > > > > 2.1.0
> > > > > >   
> > > > > 
> > > > > Hi Jerin,  
> > > > 
> > > > Hi Jianbo, Thanks for the review.
> > > > Looking forward to seeing contributions to DPDK-ARM.
> > > > We definitely need more hands to make best DPDK-ARM port.
> > > >   
> > > > > In this way, we still have to modify two files each time a new feature
> > > > > is added but not verified on ARM architectures.
> > > > > Since disabling those drivers and libs are common for both armv7 and
> > > > > armv8, can you put them in one config file, for example: common_arm?   

Hello Jerin and Jianbo.

Do you think that changing a single line in two files (instead of a
single single) is really an issue? I don't think so. At least for now.

I believe (and have already expressed this idea) that this is not a
problem of architecture ports but it is a problem of the build system.
Love me or hate me, in my opinion the build system is broken :). The
build system should be able to solve this.

I've created privately an integration of kconfig into DPDK, however, it
is far from being usable and I did not have time to make at least an
RFC patch. If there is an attitude in the community to include such
thing in the future versions, I'd like to make some more effort in this
area.

For now, the separate armv7/armv8 configuration seems OK to me.

> > > > 
> > > > [snip]
> > > > 
> > > > 2) AFAIK, PCI NIC PMD's are not yet supported in ARMv7 platform yet
> > > > unlike ARMv8.
> > > > Till we have PCI NIC PMD support, armv7 config needs to be updated
> > > > for each and every new PMD inclusion.

Unfortunately yes. I don't like this state very much...

> > > > 
> > > > 3) neon capabilities are bit different in ARMv7 and ARMv8.
> > > > For instance, "vqtbl1q_u8" neon intrinsics is not defined in ARMv7 which used
> > > > in implementing ACL-NEON. i.e Need additional efforts to extend
> > > > the armv8 neon code to armv7(or vice versa).So it's better to
> > > > have fine control on the config file to enable selective features
> > > >   
> > > 
> > > The differences between ARMv7 and ARMv8 can't justify we only add new
> > > config for ARMv8. And this file is trying to disable drivers and libs
> > > which is not supported on ARM platforms for now.
> > >  
> > 
> > I thought difference and point 3 should justify the need for different config. No?

I vote yes.

> > [snip]
> > > 
> > 
> > I was thinking to have the common_arm64 file so that "FreeBSD, arm compiler,
> > clang, llvm" future version can include it directly.
> > But I agree with you. defconfig_arm64-armv8a-linuxapp-gcc can be treated as a
> > config for a common file for defconfig_arm64-*-linuxapp-gcc(anyway its same,
> > just the toolchain added in defconfig_arm64-*-linuxapp-gcc)
> > I will send out new version with defconfig_arm64-armv8a-linuxapp-gcc
> > as the base instead of common_arm64 file.

I think that unless we support more compilers/operating systems (and
this will take some time), there is no need to consider more general
configurations. I agree to stay with the armv[78]-linuxapp-gcc for now.

Kind Regards
Jan Viktorin