[dpdk-dev] [PATCH v2] ethdev: enhance the API for getting burst mode information
Thomas Monjalon
thomas at monjalon.net
Wed Nov 6 09:19:32 CET 2019
06/11/2019 02:21, Wang, Haiyue:
> From: Thomas Monjalon <thomas at 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.
More information about the dev
mailing list