> -----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() ?
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.