> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Tuesday, October 15, 2019 00:39
> To: Wang, Haiyue <haiyue.w...@intel.com>; dev@dpdk.org; Ye, Xiaolong
> <xiaolong...@intel.com>
> Cc: Kinsella, Ray <ray.kinse...@intel.com>; Iremonger, Bernard
> <bernard.iremon...@intel.com>; Sun,
> Chenmin <chenmin....@intel.com>
> Subject: Re: [PATCH v3 1/4] ethdev: add the API for getting burst mode
> information
>
> On 10/14/2019 4:35 PM, Haiyue Wang wrote:
> > Some PMDs have more than one RX/TX burst paths, add the ethdev API
> > that allows an application to retrieve the mode information about
> > Rx/Tx packet burst such as Scalar or Vector, and Vector technology
> > like AVX2.
> >
> > Signed-off-by: Haiyue Wang <haiyue.w...@intel.com>
> > Acked-by: Bernard Iremonger <bernard.iremon...@intel.com>
>
> As far as I can see Bernard has ack only on testpmd patch, 4/4, not for reset
> of
> the patchset, can you please confirm this offline?
>
> > Reviewed-by: Xiaolong Ye <xiaolong...@intel.com>
>
> <...>
>
> > +const char *
> > +rte_eth_burst_mode_option_name(uint64_t option)
> > +{
> > + switch (option) {
> > + case RTE_ETH_BURST_SCALAR: return "Scalar";
> > + case RTE_ETH_BURST_VECTOR: return "Vector";
> > +
> > + case RTE_ETH_BURST_ALTIVEC: return "AltiVec";
> > + case RTE_ETH_BURST_NEON: return "Neon";
> > + case RTE_ETH_BURST_SSE: return "SSE";
> > + case RTE_ETH_BURST_AVX2: return "AVX2";
> > + case RTE_ETH_BURST_AVX512: return "AVX512";
> > +
> > + case RTE_ETH_BURST_SCATTERED: return "Scattered";
> > + case RTE_ETH_BURST_BULK_ALLOC: return "Bulk Alloc";
> > + case RTE_ETH_BURST_SIMPLE: return "Simple";
> > +
> > + case RTE_ETH_BURST_PER_QUEUE: return "Per Queue";
> > + }
> > +
> > + return "";
> > +}
>
> Hi Haiyue,
>
> The string representation of a vector mode is a data, and I think better to
> keep
> it separately as an array instead of keeping this information in the function
> and make the function use that data.
> So that when new type are added it won't require to update the function
> itself.
>
Hi Ferruh,
Even the vector mode is a data, it still is a bit field option, if we treated
them
differently, that will make 'rte_eth_burst_mode_option_name' ugly like:
switch (option) {
case RTE_ETH_BURST_SCALAR: return "Scalar";
case RTE_ETH_BURST_VECTOR: return "Vector";
case RTE_ETH_BURST_SCATTERED: return "Scattered";
case RTE_ETH_BURST_BULK_ALLOC: return "Bulk Alloc";
case RTE_ETH_BURST_SIMPLE: return "Simple";
case RTE_ETH_BURST_PER_QUEUE: return "Per Queue";
}
static const struct {
uint64_t vector;
const char *name;
} rte_burst_vector_names[] = {
{ RTE_ETH_BURST_ALTIVEC, "AltiVec" },
{ RTE_ETH_BURST_NEON, "Neon" },
{ RTE_ETH_BURST_SSE, "SSE" },
{ RTE_ETH_BURST_AVX2, "AVX2" },
{ RTE_ETH_BURST_AVX512, "AVX512" },
};
for (i = 0; i < RTE_DIM(rte_burst_vector_names); ++i) {
if (option == rte_burst_ vector _names[i].option)
return rte_burst_option_names[i].name;
}
Why just put them together ?
static const struct {
uint64_t option;
const char *name;
} rte_burst_option_names[] = {
{ RTE_ETH_BURST_SCALAR, "Scalar" },
{ RTE_ETH_BURST_VECTOR, "Vector" },
{ RTE_ETH_BURST_ALTIVEC, "AltiVec" },
{ RTE_ETH_BURST_NEON, "Neon" },
{ RTE_ETH_BURST_SSE, "SSE" },
{ RTE_ETH_BURST_AVX2, "AVX2" },
{ RTE_ETH_BURST_AVX512, "AVX512" },
{ RTE_ETH_BURST_SCATTERED, "Scattered" },
{ RTE_ETH_BURST_BULK_ALLOC, "Bulk Alloc" },
{ RTE_ETH_BURST_SIMPLE, "Simple" },
{ RTE_ETH_BURST_PER_QUEUE, "Per Queue" },
};
const char *
rte_eth_burst_mode_option_name(uint64_t option)
{
const char *name = "";
unsigned int i;
for (i = 0; i < RTE_DIM(rte_burst_option_names); ++i) {
if (option == rte_burst_option_names[i].option) {
name = rte_burst_option_names[i].name;
break;
}
}
return name;
}
> 'rte_rx_offload_names' and 'rte_eth_dev_rx_offload_name()' is the good sample
> of
> what I mentioned above.
>
> Thanks,
> ferruh
