[dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting burst mode information

[Wang, Haiyue]

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk at gmail.com>
> Sent: Tuesday, October 29, 2019 22:09
> To: Wang, Haiyue <haiyue.wang at intel.com>
> Cc: Thomas Monjalon <thomas at monjalon.net>; Yigit, Ferruh <ferruh.yigit at intel.com>; dpdk-dev
> <dev at dpdk.org>; Ye, Xiaolong <xiaolong.ye at intel.com>; Kinsella, Ray <ray.kinsella at intel.com>;
> Iremonger, Bernard <bernard.iremonger at intel.com>; Sun, Chenmin <chenmin.sun at intel.com>; Andrew
> Rybchenko <arybchenko at solarflare.com>; Slava Ovsiienko <viacheslavo at mellanox.com>; Stephen Hemminger
> <stephen at networkplumber.org>; David Marchand <david.marchand at redhat.com>; Jerin Jacob
> <jerinj at 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. */
};


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.

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"

│   ├── i40e_rxtx.c
│   ├── i40e_rxtx.h
│   ├── i40e_rxtx_vec_altivec.c
│   ├── i40e_rxtx_vec_avx2.c
│   ├── i40e_rxtx_vec_common.h
│   ├── i40e_rxtx_vec_neon.c
│   ├── i40e_rxtx_vec_sse.c
│   ├── iavf_rxtx.c
│   ├── iavf_rxtx.h
│   ├── iavf_rxtx_vec_avx2.c
│   ├── iavf_rxtx_vec_common.h
│   ├── iavf_rxtx_vec_sse.c
│   ├── ice_rxtx.c
│   ├── ice_rxtx.h
│   ├── ice_rxtx_vec_avx2.c
│   ├── ice_rxtx_vec_common.h
│   ├── ice_rxtx_vec_sse.c
│   ├── ixgbe_rxtx.c
│   ├── ixgbe_rxtx.h
│   ├── ixgbe_rxtx_vec_common.h
│   ├── ixgbe_rxtx_vec_neon.c
│   ├── ixgbe_rxtx_vec_sse.c
│   ├── lio_rxtx.c
│   ├── lio_rxtx.h
│   ├── mlx4_rxtx.c
│   ├── mlx4_rxtx.h
│   ├── mlx5_rxtx.c
│   ├── mlx5_rxtx.h
│   ├── mlx5_rxtx_vec_altivec.h
│   ├── mlx5_rxtx_vec.c
│   ├── mlx5_rxtx_vec.h
│   ├── mlx5_rxtx_vec_neon.h
│   ├── mlx5_rxtx_vec_sse.h
│   ├── mvneta_rxtx.c
│   ├── mvneta_rxtx.h
│   ├── hn_rxtx.c
│   ├── octeontx_rxtx.c
│   ├── octeontx_rxtx.h
│   ├── qede_rxtx.c
│   ├── qede_rxtx.h
│   ├── nicvf_rxtx.c
│   ├── nicvf_rxtx.h
│   ├── virtio_rxtx.c
│   ├── virtio_rxtx.h
│   ├── virtio_rxtx_simple_altivec.c
│   ├── virtio_rxtx_simple.c
│   ├── virtio_rxtx_simple.h
│   ├── virtio_rxtx_simple_neon.c
│   ├── virtio_rxtx_simple_sse.c
    └── vmxnet3_rxtx.c

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.