06/11/2019 02:21, Wang, Haiyue:
> From: Thomas Monjalon <tho...@monjalon.net>
> > 04/11/2019 11:39, Haiyue Wang:
> > > /**
> > > * Ethernet device RX/TX queue packet burst mode information structure.
> > > * Used to retrieve information about packet burst mode setting.
> > > */
> > > struct rte_eth_burst_mode {
> > > - uint64_t options;
> > > + uint64_t flags; /**< The ORed values of RTE_ETH_BURST_FLAG_xxx */
> > > +
> > > +#define RTE_ETH_BURST_MODE_INFO_SIZE 1024 /**< Maximum size for
> > > information */
> > > + char info[RTE_ETH_BURST_MODE_INFO_SIZE]; /**< burst mode information */
> > > };
> >
> > I think the API can be simpler by passing the flags as function parameter.
> >
> > In my understanding the burst mode name is fixed per Rx/Tx function,
> > so it can be a constant string referenced with a simple char*.
> >
> > This is the current API:
> >
> > int rte_eth_rx_burst_mode_get(uint16_t port_id, uint16_t queue_id,
> > struct rte_eth_burst_mode *mode);
> >
> > I wonder what do you think of such API? (just a proposal for comments):
> >
> > char *rte_eth_rx_burst_mode_get(uint16_t port_id, uint16_t queue_id,
> > uint64_t flags);
> >
> > Or is there some cases where you want to build the string with snprintf?
> > (I cannot think about a case, given it should mapped to a C-function)
> >
>
> 1. 'a constant string' is hard for PMD expanding if it wants to make the
> string
> dynamic according to the setting, like:
> http://patchwork.dpdk.org/patch/62352/
> (although based on bit options design).
Yes, constant string is less flexible in the PMD implementation.
> 2. And for dynamic string, if it is *return type*, then the PMD needs to
> handle the memory allocation, and the application frees it. And 'uint64_t
> flags'
> is output parameter, so it should be like 'uint64_t *flags', but this
> needs the
> application to declare it or not, and needs PMDs to check whether it is
> passed
> or not, then set it.
>
> So for making things easy, the 'struct rte_eth_burst_mode' may be nice,
> then the
> application just declares one line : 'struct rte_eth_burst_mode mode',
> then all
> things are filled by PMD in one place.
I agree it is a lot simpler to use a struct.
