On Tue, Apr 25, 2017 at 01:00:46PM +0000, Legacy, Allain wrote:
> > -----Original Message-----
> > From: Adrien Mazarguil [mailto:[email protected]]
> > Sent: Tuesday, April 25, 2017 8:50 AM
> <...>
> > > 2) RTE_STD_C11 needs to be included in the #ifdef __KERNEL__.
> >
> > Missed that one, however I suggest either:
> >
> > #ifndef __KERNEL__ around RTE_STD_C11
> >
> > or using __extension__ directly. Which do you prefer?
>
> I would prefer if it was done as it is done in rte_kni_common.h to provide
> consistency with other similar files. Like this:
>
> #ifdef __KERNEL__
> #include <linux/if.h>
> #define RTE_STD_C11
> #else
> #include <rte_common.h>
> #endif
>
> ...but if you disagree then I prefer the #ifndef __KERNEL__ option.
You're right in fact, I did not remember that was the method used for
KNI. Let's keep your suggestion.
> >
> > By the way, is the kernel module that depends on rte_avp_common.h
> > available somewhere to validate compilation against it?
>
> There is an older version of the module available on github, but it has not
> been updated since the AVP driver has been included in the DPDK. Since the
> AVP directory and files were significantly changed in order to meet the
> requirements of the DPDK it won't be much use to you. Until we can update
> it please make sure both Matt Peters and I are CC'd on the patch requests and
> we'll confirm compilation as quickly as possible.
>
>
> > > Would you mind changing the brackets (<>) to quotes ("") since this is a
> > local include file?
> > >
> > > #include "rte_avp_common.h"
> >
> > I will update it.
>
>
> Thank you.
Can I add your acked-by line directly assuming all the above is done as
described?
--
Adrien Mazarguil
6WIND