On 10/15/2019 2:39 AM, Wang, Haiyue wrote:
>> -----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.
>>
<...>
> 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;
> }
+1, this my intention, thanks.
