On Tue, Oct 29, 2019 at 9:12 PM Wang, Haiyue <haiyue.w...@intel.com> wrote: > > > -----Original Message----- > > From: Jerin Jacob <jerinjac...@gmail.com> > > Sent: Tuesday, October 29, 2019 22:09 > > To: Wang, Haiyue <haiyue.w...@intel.com> > > Cc: Thomas Monjalon <tho...@monjalon.net>; Yigit, Ferruh > > <ferruh.yi...@intel.com>; dpdk-dev > > <dev@dpdk.org>; Ye, Xiaolong <xiaolong...@intel.com>; Kinsella, Ray > > <ray.kinse...@intel.com>; > > Iremonger, Bernard <bernard.iremon...@intel.com>; Sun, Chenmin > > <chenmin....@intel.com>; Andrew > > Rybchenko <arybche...@solarflare.com>; Slava Ovsiienko > > <viachesl...@mellanox.com>; Stephen Hemminger > > <step...@networkplumber.org>; David Marchand <david.march...@redhat.com>; > > Jerin Jacob > > <jer...@marvell.com> > > Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting > > burst mode information > > > > > > > > > > > > > > > > > > How about *_str_* style ? > > > > > > > > _name kind of implies it the string. may be _mode is good as it is > > > > short. > > > > > > > > > int > > > > > rte_eth_rx_burst_mode_str_get(uint16_t port_id, uint16_t queue_id, > > > > > char *buf, int buflen) > > > > > > > > > > About the function, keep the same is better ? Then we need no whole > > > replace, just update the parameters, and the parameters indicated that > > > it is in string format. > > > > In this case, we need additional PMD op to get the buflen as > > the application will not know the buffer size in advance. It needs to come > > from the driver and common code. > > > > > > See below. > > > > > > > > > We don't need buflen as it is not known to the application. The > > > > typical pattern, we followed, > > > > in dpdk is, when function called buf as NULL then the function returns > > > > the expected size so that > > > > the application can alloc and get the buffer from ethdev layer on the > > > > next iteration. > > > > > > > > > > > > > > A little complicated and too heavy for using ? where is the example code ? > > > > See rte_eth_xstats_get_names() API as example for dynamic buffer allocation > > and similar use case in DPDK. > > In fact, this is no so complex as dynamic string handling. :) > > /* Get count */ > cnt_xstats = rte_eth_xstats_get_names(port_id, NULL, 0); > if (cnt_xstats < 0) { > printf("Error: Cannot get count of xstats\n"); > return; > } > > xstats_names = malloc(sizeof(struct rte_eth_xstat_name) * cnt_xstats); > > struct rte_eth_xstat_name { > char name[RTE_ETH_XSTATS_NAME_SIZE]; /**< The statistic name. */ > };
not really, it will be,[1] count = rte_eth_rx_burst_mode_name(port_id, queue_id, NULL); buf = malloc(count); count = rte_eth_rx_burst_mode_name(port_id, queue_id, buf); > > > Frankly speaking, after re-thinking, I prefer to keep current design. > 1) Use the structure to expose the *raw* data. Make the APIs work as > a SDK, DON'T image to format the string for user. User can call the > API to dump to file, print to console etc. Because he has the raw > data. [1] Dont have such issue. > > struct rte_eth_burst_mode { > uint64_t options; > }; > > The 'options' bit definition reflects the rx/tx source code structure roughly: > > "tree drivers/net/ | grep rxtx" > │ ├── virtio_rxtx_simple_sse.c > └── vmxnet3_rxtx.c Files don't represent the actual mode PMD is running. So listing the file example is not correct. > > 2. add "char dev_specific[]" data as needed if a PMD wants to expose it, > enhance the APIs step by step, and do it as needed. This would change ABI for no reason and who allocates the memory for dev_specific?