Re: [dpdk-dev] [PATCH v3 1/4] ethdev: add the API for getting burst mode information

[Ferruh Yigit <ferruh.yigit@intel.com>]

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.wang@intel.com>; dev@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>
>> Cc: Kinsella, Ray <ray.kinsella@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>; Sun,
>> Chenmin <chenmin.sun@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.wang@intel.com>
>>> Acked-by: Bernard Iremonger <bernard.iremonger@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.ye@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.