On 10/31/2019 3:07 PM, Wang, Haiyue wrote: >> -----Original Message----- >> From: Yigit, Ferruh <[email protected]> >> Sent: Thursday, October 31, 2019 22:58 >> To: Wang, Haiyue <[email protected]>; Jerin Jacob <[email protected]> >> Cc: Thomas Monjalon <[email protected]>; dpdk-dev <[email protected]>; Ye, >> Xiaolong >> <[email protected]>; Kinsella, Ray <[email protected]>; Iremonger, >> Bernard >> <[email protected]>; Sun, Chenmin <[email protected]>; Andrew >> Rybchenko >> <[email protected]>; Slava Ovsiienko <[email protected]>; >> Stephen Hemminger >> <[email protected]>; David Marchand <[email protected]>; >> Jerin Jacob >> <[email protected]> >> Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting burst >> mode information >> >> On 10/31/2019 11:16 AM, Wang, Haiyue wrote: >>>> -----Original Message----- >>>> From: Jerin Jacob <[email protected]> >>>> Sent: Thursday, October 31, 2019 18:46 >>>> To: Wang, Haiyue <[email protected]> >>>> Cc: Yigit, Ferruh <[email protected]>; Thomas Monjalon >>>> <[email protected]>; dpdk-dev >>>> <[email protected]>; Ye, Xiaolong <[email protected]>; Kinsella, Ray >>>> <[email protected]>; >>>> Iremonger, Bernard <[email protected]>; Sun, Chenmin >>>> <[email protected]>; Andrew >>>> Rybchenko <[email protected]>; Slava Ovsiienko >>>> <[email protected]>; Stephen >> Hemminger >>>> <[email protected]>; David Marchand <[email protected]>; >>>> Jerin Jacob >>>> <[email protected]> >>>> 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? >> > > Use 1024 ? > >>>> + >>>> 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. > > Since 'struct rte_eth_burst_mode' will be public, how about keep current > design for > rte_eth_rx/tx_burst_mode_get() ?
What do you mean keep current design, not having a string field in "struct rte_eth_burst_mode"? > > But change 'const char *rte_eth_burst_mode_option_name(uint64_t option)' to > 'int rte_eth_burst_mode_name(struct rte_eth_burst_mode *mode, char *name)' ? > When 'name' is NULL, return the buffer size will be used. If non-NULL, format > all the options. This will reduce the twice PMDs calls: > dev->dev_ops->rx/tx_burst_mode_get > And leave the filled *mode for application. > we need "dev_ops->rx/tx_burst_mode_get" to get information from PMDs, except from 'uint64_t options' the provided information already as string, if the ethdev APIs for these devops converts 'uint64_t options' to string and append to 'alternate_options', application will already have all the string, so won't need this 'rte_eth_burst_mode_option_name()' API anymore.
