On Tue, Apr 25, 2017 at 01:00:46PM +0000, Legacy, Allain wrote:
> > -----Original Message-----
> > From: Adrien Mazarguil [mailto:adrien.mazarg...@6wind.com]
> > 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

Reply via email to