Re: [dpdk-dev] [PATCH v10 02/18] lib: create private header file

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2017-03-16 10:43, Hunt, David:
> On 15/3/2017 5:18 PM, Thomas Monjalon wrote:
> > 2017-03-15 06:19, David Hunt:
> >> +/**
> >> + * Number of packets to deal with in bursts. Needs to be 8 so as to
> >> + * fit in one cache line.
> >> + */
> >> +#define RTE_DIST_BURST_SIZE (sizeof(rte_xmm_t) / sizeof(uint16_t))
> > error: 'rte_xmm_t' undeclared here (arm compilation)
> >
> > Can it be fixed by including rte_vect.h?
> >
> > Ideally I would prefer we stop using XMM types in a generic code.
> > XMM are x86-only registers. It has been translated for other arches
> > but we should use a more generic name.
> >
> > What was the intention here? SSE-optimized code or 128-bit size?
> > Please check lib/librte_eal/common/include/generic/rte_vect.h
> > for a generic type.
> 
> Thomas,
> 
> Including rte_vect.h does indeed resolve the issue.
> 
> I had originally had "#define RTE_DIST_BURST_SIZE 8" but thought that latest
> definition would give further clarity as to why it's set to 8.
> 
> There are 2 reasons.
> 1. The vector instruction I use for the matching works on 8 uint16s at a 
> time
> 2. The (x86) cache lines communicating with the worker cores fit 8 mbuf 
> pointers at a time.
> 
> So there are 2 options to resolve:
> 1. #include <rte_vect.h> at the top of rte_distributor_private.h
> 2. revert back to "#define RTE_DIST_BURST_SIZE 8"
> 
> Personally, I'd probably lean towards option 2 (with additional comment) 
> , as it removes
> the mention of xmm from the generic header file, as well as being valid 
> for both
> reasons, whereas the xmm #define really only helps to explain one reason.
> 
> Do you have any preference? Let me know and I can push up a v11.
> 
> Regards,
> Dave.
> 
> 
> P.S. Suggested change:
> 
> /*
>   * Transfer up to 8 mbufs at a time to/from workers, and
>   * flow matching algorithm optimised for 8 flow IDs at a time
>   */
> #define RTE_DIST_BURST_SIZE 8

OK for the option 2