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

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

On 10/31/2019 11:16 AM, Wang, Haiyue wrote:
>> -----Original Message-----
>> From: Jerin Jacob <jerinjacobk@gmail.com>
>> Sent: Thursday, October 31, 2019 18:46
>> To: Wang, Haiyue <haiyue.wang@intel.com>
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net>; dpdk-dev
>> <dev@dpdk.org>; Ye, Xiaolong <xiaolong.ye@intel.com>; Kinsella, Ray <ray.kinsella@intel.com>;
>> Iremonger, Bernard <bernard.iremonger@intel.com>; Sun, Chenmin <chenmin.sun@intel.com>; Andrew
>> Rybchenko <arybchenko@solarflare.com>; Slava Ovsiienko <viacheslavo@mellanox.com>; Stephen Hemminger
>> <stephen@networkplumber.org>; David Marchand <david.marchand@redhat.com>; Jerin Jacob
>> <jerinj@marvell.com>
>> Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting burst mode information
>>
>>>> 'rte_eth_burst_mode_option_name()' can get "struct rte_eth_burst_mode" as
>>>> parameter and convert the 'options' to string and combine into single string as
>>>> a helper function to the applications.
>>>>
>>>
>>> Change:
>>> const char *
>>> rte_eth_burst_mode_option_name(uint64_t option)
>>>
>>> to:
>>> int
>>> rte_eth_burst_mode_option_name(struct rte_eth_burst_mode *mode, char *str) ?
>>
>>
>> Since we are not ready to _remove_ flags in public API and rc2 time is
>> ticking, probably the following the change
>> would be enough. IMO, This API can be used only for logging purpose, I
>> don't want to spend too
>> many cycles on this discussion. I am leaving the decision to ethdev
>> maintainers to accommodate
>> the specifics of adding a string-based alternate options scheme.
>>
> 
> Thanks, Jerin.
> 
>>
>> [master][dpdk.org] $ git diff
>> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
>> index c36c1b631..2f9d2c0a7 100644
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> @@ -1272,8 +1272,11 @@ enum rte_eth_burst_mode_option {
>>   * Ethernet device RX/TX queue packet burst mode information structure.
>>   * Used to retrieve information about packet burst mode setting.
>>   */
>> +#define RTE_ETH_BURST_MODE_ALT_OPT_SIZE 128

Should we make it bigger to prevent ABI break just because of this later? Like
512 or bigger?

>> +
>>  struct rte_eth_burst_mode {
>>         uint64_t options;
>> +       char alternate_options[RTE_ETH_BURST_MODE_ALT_OPT_SIZE];
>>  };
> 
> +1
> 

+1

It is not as flexible as getting the size from the PMD but much simpler, both
for the user and PMD.

And what about dropping the 'rte_eth_burst_mode_option_name()', if
'rte_eth_tx_burst_mode_get()' converts the 'options' to text
('alternate_options') as Jerin suggested, we can drop it.

So only difference from what Jerin suggested will be keeping 'uint64_t options'
public for known/limited standardized options, which is currently only for
vectorization information.