Re: [dpdk-dev] [PATCH v3 00/10] qede: Add qede PMD

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Mar 29, 2016 at 08:52:38PM +0000, Rasesh Mody wrote:
> Hi Bruce,
> 
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Tuesday, March 22, 2016 4:30 AM
> > 
> > On Tue, Mar 22, 2016 at 11:21:25AM +0000, Richardson, Bruce wrote:
> > > I've had a quick scan over this patchset, and as you've probably seen I've
> > made some public comments on it. General comments on the whole
> > patchset are:
> > > * Please run checkpatch on the patchset and clear up as many issues as
> > > you can. There are a number of typos called out which especially must
> > > be fixed. Both myself and Thomas always run checkpatch against patches
> > > before applying them. [I suggest using Thomas's checkpatches.sh script
> > > to do the checks as it disables many unnecessary warnings from
> > > checkpatch]
> > > * Please put in commit descriptions for all patches bar those doing trivial
> > things. The first three patches probably don't need a commit message, but
> > the rest do.
> > >
> > > /Bruce
> > >
> > > > -----Original Message-----
> > > > From: Rasesh Mody [mailto:rasesh.mody@qlogic.com]
> > > > Sent: Saturday, March 19, 2016 12:53 AM
> > > > To: thomas.monjalon@6wind.com; Richardson, Bruce
> > > > <bruce.richardson@intel.com>
> > > > Cc: dev@dpdk.org; ameen.rahman@qlogic.com; harish.patil@qlogic.com;
> > > > sony.chacko@qlogic.com; Rasesh Mody <rasesh.mody@qlogic.com>
> > > > Subject: [PATCH v3 00/10] qede: Add qede PMD
> > > >
> > > > Submitting v3 patch series for QEDE PMD. There is no code change
> > > > from v2 series except PMD version change. Earlier we had generated
> > > > and tested the
> > > > v2 series against dpdk tree then latest.
> > > >
> > > > The v3 series includes:
> > > >  - Patches generated and tested against latest dpdk-next-net
> > > >  - Reworked MAINTAINERS patch to make it apply cleanly
> > > >  - Incorporated Overview.rst update in the documentation patch
> > > >
> > > > Please Apply.
> > > >
> > > > Thanks!
> > > > Rasesh
> > > >
> > > > Rasesh Mody (10):
> > > >   qede: Add maintainers
> > > >   qede: Add documentation
> > > >   qede: Add license file
> > > >   qede: Add base driver
> > > >   qede: Add core driver
> > > >   qede: Add L2 support
> > > >   qede: Add SRIOV support
> > > >   qede: Add attention support
> > > >   qede: Add DCBX support
> > > >   qede: Enable PMD build
> > > >
> > Clang gives a compile error after applying this patchset. Please investigate.
> > 
> > == Build drivers/net/qede
> >   CC base/ecore_dev.o
> > fatal error: unknown warning option '-Wno-shift-negative-value'; did you
> > mean
> >       '-Wno-shift-sign-overflow'? [-Wunknown-warning-option]
> > /home/bruce/next-net/dpdk-next-net/mk/internal/rte.compile-
> > pre.mk:126: recipe for target 'base/ecore_dev.o' failed
> > 
> > This is seen with clang 3.6 on Fedora 23.
> > 	"clang version 3.6.0 (tags/RELEASE_360/final)"
> 
> We had compiled all our previously submitted driver patches against clang v3.8 on  RH7.1 and didn't see a similar error. The '-Wno-shift-negative-value' option got introduced after 3.6.0 release.  Pls. let us know if we need to support a minimum version of clang. 
>         "clang version 3.8.0 (trunk 249596) (llvm/trunk 249595)"
>  

Thanks for the update. I'm not sure that we have a minimum version of clang that
we have to support - we tend to test more with gcc than clang. If the latest
clang version most common distro's compiles this fine, I'd say it's ok. [The latest
on Fedora 23 is actually 3.7, so we are ok with that].
Anyone in the community have a minimum clang version that they need DPDK to
support?

> > Similarly, 32-bit (i686) build fails:
> > 
> > == Build drivers/net/qede
> >   CC base/ecore_dev.o
> > /home/bruce/next-net/dpdk-next-
> > net/drivers/net/qede/base/ecore_dev.c: In function
> > ‘ecore_chain_alloc_sanity_check’:
> > /home/bruce/next-net/dpdk-next-
> > net/drivers/net/qede/base/ecore_dev.c:2571:32: error: format ‘%lx’
> > expects argument of type ‘long unsigned int’, but argument 6 has type ‘u64
> > {aka long long unsigned int}’ [-Werror=format=] compilation terminated due
> > to -Wfatal-errors.
> > cc1: all warnings being treated as errors
> > 
> > This is seen with gcc 5.3.1 on Fedora 23.
> 
> For 32-bit, we are preparing our driver to compile against gcc version 4.3.4.
> 
> Similary, we had compile tested all our previously submitted driver patches on FreeBSD using:
>         FreeBSD clang version 3.6.1 and
>         gcc version 4.8.5

Great, thanks.

/Bruce

> 
> Thanks!
> Rasesh
> 
> > Regards,
> > /Bruce