[v2] ethdev: enhance the API for getting burst mode information
diff mbox series

Message ID 20191104103920.64907-1-haiyue.wang@intel.com
State Superseded, archived
Headers show
Series
  • [v2] ethdev: enhance the API for getting burst mode information
Related show

Checks

Context Check Description
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-compilation success Compile Testing PASS
ci/iol-intel-Performance fail Performance Testing issues
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Haiyue Wang Nov. 4, 2019, 10:39 a.m. UTC
Change the type of burst mode information from bit field to free string
data, so that each PMD can describe the Rx/Tx busrt functions flexibly.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---

v2: - Drop the bit field for burst mode information handling.

v1: - http://patchwork.dpdk.org/patch/62289/
    - http://patchwork.dpdk.org/patch/62290/
    - http://patchwork.dpdk.org/patch/62291/

 app/test-pmd/config.c                    |  33 ++-----
 doc/guides/nics/features.rst             |   3 +-
 doc/guides/rel_notes/release_19_11.rst   |   2 -
 drivers/net/i40e/i40e_rxtx.c             | 121 ++++++++++++-----------
 drivers/net/ice/ice_rxtx.c               |  89 +++++++++--------
 lib/librte_ethdev/rte_ethdev.c           |  35 -------
 lib/librte_ethdev/rte_ethdev.h           |  43 ++------
 lib/librte_ethdev/rte_ethdev_version.map |   1 -
 8 files changed, 131 insertions(+), 196 deletions(-)

Comments

Kinsella, Ray Nov. 5, 2019, 3:51 p.m. UTC | #1
On 04/11/2019 10:39, Haiyue Wang wrote:
> Change the type of burst mode information from bit field to free string
> data, so that each PMD can describe the Rx/Tx busrt functions flexibly.
> 
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---
> 
> v2: - Drop the bit field for burst mode information handling.
> 
> v1: - http://patchwork.dpdk.org/patch/62289/
>     - http://patchwork.dpdk.org/patch/62290/
>     - http://patchwork.dpdk.org/patch/62291/
> 
>  app/test-pmd/config.c                    |  33 ++-----
>  doc/guides/nics/features.rst             |   3 +-
>  doc/guides/rel_notes/release_19_11.rst   |   2 -
>  drivers/net/i40e/i40e_rxtx.c             | 121 ++++++++++++-----------
>  drivers/net/ice/ice_rxtx.c               |  89 +++++++++--------
>  lib/librte_ethdev/rte_ethdev.c           |  35 -------
>  lib/librte_ethdev/rte_ethdev.h           |  43 ++------
>  lib/librte_ethdev/rte_ethdev_version.map |   1 -
>  8 files changed, 131 insertions(+), 196 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index efe2812a8..b6039749c 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -350,21 +350,6 @@ nic_stats_mapping_display(portid_t port_id)
>  	       nic_stats_mapping_border, nic_stats_mapping_border);
>  }
>  
> -static void
> -burst_mode_options_display(uint64_t options)
> -{
> -	int offset;
> -
> -	while (options != 0) {
> -		offset = rte_bsf64(options);
> -
> -		printf(" %s",
> -		       rte_eth_burst_mode_option_name(1ULL << offset));
> -
> -		options &= ~(1ULL << offset);
> -	}
> -}
> -
>  void
>  rx_queue_infos_display(portid_t port_id, uint16_t queue_id)
>  {
> @@ -397,10 +382,11 @@ rx_queue_infos_display(portid_t port_id, uint16_t queue_id)
>  		(qinfo.scattered_rx != 0) ? "on" : "off");
>  	printf("\nNumber of RXDs: %hu", qinfo.nb_desc);
>  
> -	if (rte_eth_rx_burst_mode_get(port_id, queue_id, &mode) == 0) {
> -		printf("\nBurst mode:");
> -		burst_mode_options_display(mode.options);
> -	}
> +	if (rte_eth_rx_burst_mode_get(port_id, queue_id, &mode) == 0)
> +		printf("\nBurst mode: %s%s",
> +		       mode.info,
> +		       mode.flags & RTE_ETH_BURST_FLAG_PER_QUEUE ?
> +				" (per queue)" : "");
>  
>  	printf("\n");
>  }
> @@ -433,10 +419,11 @@ tx_queue_infos_display(portid_t port_id, uint16_t queue_id)
>  		(qinfo.conf.tx_deferred_start != 0) ? "on" : "off");
>  	printf("\nNumber of TXDs: %hu", qinfo.nb_desc);
>  
> -	if (rte_eth_tx_burst_mode_get(port_id, queue_id, &mode) == 0) {
> -		printf("\nBurst mode:");
> -		burst_mode_options_display(mode.options);
> -	}
> +	if (rte_eth_tx_burst_mode_get(port_id, queue_id, &mode) == 0)
> +		printf("\nBurst mode: %s%s",
> +		       mode.info,
> +		       mode.flags & RTE_ETH_BURST_FLAG_PER_QUEUE ?
> +				" (per queue)" : "");
>  
>  	printf("\n");
>  }
> diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
> index d96696801..7a31cf7c8 100644
> --- a/doc/guides/nics/features.rst
> +++ b/doc/guides/nics/features.rst
> @@ -879,8 +879,7 @@ Burst mode info
>  Supports to get Rx/Tx packet burst mode information.
>  
>  * **[implements] eth_dev_ops**: ``rx_burst_mode_get``, ``tx_burst_mode_get``.
> -* **[related] API**: ``rte_eth_rx_burst_mode_get()``, ``rte_eth_tx_burst_mode_get()``,
> -  ``rte_eth_burst_mode_option_name()``.
> +* **[related] API**: ``rte_eth_rx_burst_mode_get()``, ``rte_eth_tx_burst_mode_get()``.
>  
>  .. _nic_features_other:
>  
> diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
> index ae8e7b2f0..8fd1e7e62 100644
> --- a/doc/guides/rel_notes/release_19_11.rst
> +++ b/doc/guides/rel_notes/release_19_11.rst
> @@ -114,8 +114,6 @@ New Features
>    ``rte_eth_tx_burst_mode_get`` that allow an application
>    to retrieve the mode information about RX/TX packet burst
>    such as Scalar or Vector, and Vector technology like AVX2.
> -  Another new function ``rte_eth_burst_mode_option_name`` is
> -  provided for burst mode options stringification.
>  
>  * **Updated the Intel ice driver.**
>  
> diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> index 6a66cec20..17dc8c78f 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -3017,49 +3017,45 @@ i40e_set_rx_function(struct rte_eth_dev *dev)
>  	}
>  }
>  
> -int
> -i40e_rx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
> -		       struct rte_eth_burst_mode *mode)
> -{
> -	eth_rx_burst_t pkt_burst = dev->rx_pkt_burst;
> -	uint64_t options;
> -
> -	if (pkt_burst == i40e_recv_scattered_pkts)
> -		options = RTE_ETH_BURST_SCALAR | RTE_ETH_BURST_SCATTERED;
> -	else if (pkt_burst == i40e_recv_pkts_bulk_alloc)
> -		options = RTE_ETH_BURST_SCALAR | RTE_ETH_BURST_BULK_ALLOC;
> -	else if (pkt_burst == i40e_recv_pkts)
> -		options = RTE_ETH_BURST_SCALAR;
> +static const struct {
> +	eth_rx_burst_t pkt_burst;
> +	const char *info;
> +} i40e_rx_burst_infos[] = {
> +	{ i40e_recv_scattered_pkts,          "Scalar Scattered" },
> +	{ i40e_recv_pkts_bulk_alloc,         "Scalar Bulk Alloc" },
> +	{ i40e_recv_pkts,                    "Scalar" },
>  #ifdef RTE_ARCH_X86
> -	else if (pkt_burst == i40e_recv_scattered_pkts_vec_avx2)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_AVX2 |
> -			  RTE_ETH_BURST_SCATTERED;
> -	else if (pkt_burst == i40e_recv_pkts_vec_avx2)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_AVX2;
> -	else if (pkt_burst == i40e_recv_scattered_pkts_vec)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE |
> -			  RTE_ETH_BURST_SCATTERED;
> -	else if (pkt_burst == i40e_recv_pkts_vec)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE;
> +	{ i40e_recv_scattered_pkts_vec_avx2, "Vector AVX2 Scattered" },
> +	{ i40e_recv_pkts_vec_avx2,           "Vector AVX2" },
> +	{ i40e_recv_scattered_pkts_vec,      "Vector SSE Scattered" },
> +	{ i40e_recv_pkts_vec,                "Vector SSE" },
>  #elif defined(RTE_ARCH_ARM64)
> -	else if (pkt_burst == i40e_recv_scattered_pkts_vec)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_NEON |
> -			  RTE_ETH_BURST_SCATTERED;
> -	else if (pkt_burst == i40e_recv_pkts_vec)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_NEON;
> +	{ i40e_recv_scattered_pkts_vec,      "Vector Neon Scattered" },
> +	{ i40e_recv_pkts_vec,                "Vector Neon" },
>  #elif defined(RTE_ARCH_PPC_64)
> -	else if (pkt_burst == i40e_recv_scattered_pkts_vec)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_ALTIVEC |
> -			  RTE_ETH_BURST_SCATTERED;
> -	else if (pkt_burst == i40e_recv_pkts_vec)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_ALTIVEC;
> +	{ i40e_recv_scattered_pkts_vec,      "Vector AltiVec Scattered" },
> +	{ i40e_recv_pkts_vec,                "Vector AltiVec" },
>  #endif
> -	else
> -		options = 0;
> +};
>  
> -	mode->options = options;
> +int
> +i40e_rx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
> +		       struct rte_eth_burst_mode *mode)
> +{
> +	eth_rx_burst_t pkt_burst = dev->rx_pkt_burst;
> +	int ret = -EINVAL;
> +	unsigned int i;
> +
> +	for (i = 0; i < RTE_DIM(i40e_rx_burst_infos); ++i) {
> +		if (pkt_burst == i40e_rx_burst_infos[i].pkt_burst) {
> +			snprintf(mode->info, sizeof(mode->info), "%s",
> +				 i40e_rx_burst_infos[i].info);
> +			ret = 0;
> +			break;
> +		}
> +	}
>  
> -	return options != 0 ? 0 : -EINVAL;
> +	return ret;
>  }
>  
>  void __attribute__((cold))
> @@ -3155,35 +3151,40 @@ i40e_set_tx_function(struct rte_eth_dev *dev)
>  	}
>  }
>  
> -int
> -i40e_tx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
> -		       struct rte_eth_burst_mode *mode)
> -{
> -	eth_tx_burst_t pkt_burst = dev->tx_pkt_burst;
> -	uint64_t options;
> -
> -	if (pkt_burst == i40e_xmit_pkts_simple)
> -		options = RTE_ETH_BURST_SCALAR | RTE_ETH_BURST_SIMPLE;
> -	else if (pkt_burst == i40e_xmit_pkts)
> -		options = RTE_ETH_BURST_SCALAR;
> +static const struct {
> +	eth_tx_burst_t pkt_burst;
> +	const char *info;
> +} i40e_tx_burst_infos[] = {
> +	{ i40e_xmit_pkts_simple,   "Scalar Simple" },
> +	{ i40e_xmit_pkts,          "Scalar" },
>  #ifdef RTE_ARCH_X86
> -	else if (pkt_burst == i40e_xmit_pkts_vec_avx2)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_AVX2;
> -	else if (pkt_burst == i40e_xmit_pkts_vec)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE;
> +	{ i40e_xmit_pkts_vec_avx2, "Vector AVX2" },
> +	{ i40e_xmit_pkts_vec,      "Vector SSE" },
>  #elif defined(RTE_ARCH_ARM64)
> -	else if (pkt_burst == i40e_xmit_pkts_vec)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_NEON;
> +	{ i40e_xmit_pkts_vec,      "Vector Neon" },
>  #elif defined(RTE_ARCH_PPC_64)
> -	else if (pkt_burst == i40e_xmit_pkts_vec)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_ALTIVEC;
> +	{ i40e_xmit_pkts_vec,      "Vector AltiVec" },
>  #endif
> -	else
> -		options = 0;
> +};
>  
> -	mode->options = options;
> +int
> +i40e_tx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
> +		       struct rte_eth_burst_mode *mode)
> +{
> +	eth_tx_burst_t pkt_burst = dev->tx_pkt_burst;
> +	int ret = -EINVAL;
> +	unsigned int i;
> +
> +	for (i = 0; i < RTE_DIM(i40e_tx_burst_infos); ++i) {
> +		if (pkt_burst == i40e_tx_burst_infos[i].pkt_burst) {
> +			snprintf(mode->info, sizeof(mode->info), "%s",
> +				 i40e_tx_burst_infos[i].info);
> +			ret = 0;
> +			break;
> +		}
> +	}
>  
> -	return options != 0 ? 0 : -EINVAL;
> +	return ret;
>  }
>  
>  void __attribute__((cold))
> diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
> index 8d4820d3c..bf14d337d 100644
> --- a/drivers/net/ice/ice_rxtx.c
> +++ b/drivers/net/ice/ice_rxtx.c
> @@ -2793,37 +2793,39 @@ ice_set_rx_function(struct rte_eth_dev *dev)
>  	}
>  }
>  
> +static const struct {
> +	eth_rx_burst_t pkt_burst;
> +	const char *info;
> +} ice_rx_burst_infos[] = {
> +	{ ice_recv_scattered_pkts,          "Scalar Scattered" },
> +	{ ice_recv_pkts_bulk_alloc,         "Scalar Bulk Alloc" },
> +	{ ice_recv_pkts,                    "Scalar" },
> +#ifdef RTE_ARCH_X86
> +	{ ice_recv_scattered_pkts_vec_avx2, "Vector AVX2 Scattered" },
> +	{ ice_recv_pkts_vec_avx2,           "Vector AVX2" },
> +	{ ice_recv_scattered_pkts_vec,      "Vector SSE Scattered" },
> +	{ ice_recv_pkts_vec,                "Vector SSE" },
> +#endif
> +};
> +
>  int
>  ice_rx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
>  		      struct rte_eth_burst_mode *mode)
>  {
>  	eth_rx_burst_t pkt_burst = dev->rx_pkt_burst;
> -	uint64_t options;
> -
> -	if (pkt_burst == ice_recv_scattered_pkts)
> -		options = RTE_ETH_BURST_SCALAR | RTE_ETH_BURST_SCATTERED;
> -	else if (pkt_burst == ice_recv_pkts_bulk_alloc)
> -		options = RTE_ETH_BURST_SCALAR | RTE_ETH_BURST_BULK_ALLOC;
> -	else if (pkt_burst == ice_recv_pkts)
> -		options = RTE_ETH_BURST_SCALAR;
> -#ifdef RTE_ARCH_X86
> -	else if (pkt_burst == ice_recv_scattered_pkts_vec_avx2)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_AVX2 |
> -			  RTE_ETH_BURST_SCATTERED;
> -	else if (pkt_burst == ice_recv_pkts_vec_avx2)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_AVX2;
> -	else if (pkt_burst == ice_recv_scattered_pkts_vec)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE |
> -			  RTE_ETH_BURST_SCATTERED;
> -	else if (pkt_burst == ice_recv_pkts_vec)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE;
> -#endif
> -	else
> -		options = 0;
> +	int ret = -EINVAL;
> +	unsigned int i;
>  
> -	mode->options = options;
> +	for (i = 0; i < RTE_DIM(ice_rx_burst_infos); ++i) {
> +		if (pkt_burst == ice_rx_burst_infos[i].pkt_burst) {
> +			snprintf(mode->info, sizeof(mode->info), "%s",
> +				 ice_rx_burst_infos[i].info);
> +			ret = 0;
> +			break;
> +		}
> +	}
>  
> -	return options != 0 ? 0 : -EINVAL;
> +	return ret;
>  }
>  
>  void __attribute__((cold))
> @@ -2949,29 +2951,36 @@ ice_set_tx_function(struct rte_eth_dev *dev)
>  	}
>  }
>  
> +static const struct {
> +	eth_tx_burst_t pkt_burst;
> +	const char *info;
> +} ice_tx_burst_infos[] = {
> +	{ ice_xmit_pkts_simple,   "Scalar Simple" },
> +	{ ice_xmit_pkts,          "Scalar" },
> +#ifdef RTE_ARCH_X86
> +	{ ice_xmit_pkts_vec_avx2, "Vector AVX2" },
> +	{ ice_xmit_pkts_vec,      "Vector SSE" },
> +#endif
> +};
> +
>  int
>  ice_tx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
>  		      struct rte_eth_burst_mode *mode)
>  {
>  	eth_tx_burst_t pkt_burst = dev->tx_pkt_burst;
> -	uint64_t options;
> -
> -	if (pkt_burst == ice_xmit_pkts_simple)
> -		options = RTE_ETH_BURST_SCALAR | RTE_ETH_BURST_SIMPLE;
> -	else if (pkt_burst == ice_xmit_pkts)
> -		options = RTE_ETH_BURST_SCALAR;
> -#ifdef RTE_ARCH_X86
> -	else if (pkt_burst == ice_xmit_pkts_vec_avx2)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_AVX2;
> -	else if (pkt_burst == ice_xmit_pkts_vec)
> -		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE;
> -#endif
> -	else
> -		options = 0;
> +	int ret = -EINVAL;
> +	unsigned int i;
>  
> -	mode->options = options;
> +	for (i = 0; i < RTE_DIM(ice_tx_burst_infos); ++i) {
> +		if (pkt_burst == ice_tx_burst_infos[i].pkt_burst) {
> +			snprintf(mode->info, sizeof(mode->info), "%s",
> +				 ice_tx_burst_infos[i].info);
> +			ret = 0;
> +			break;
> +		}
> +	}
>  
> -	return options != 0 ? 0 : -EINVAL;
> +	return ret;
>  }
>  
>  /* For each value it means, datasheet of hardware can tell more details
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 7743205d3..208362971 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -166,25 +166,6 @@ static const struct {
>  
>  #undef RTE_TX_OFFLOAD_BIT2STR
>  
> -static const struct {
> -	uint64_t option;
> -	const char *name;
> -} rte_burst_option_names[] = {
> -	{ RTE_ETH_BURST_SCALAR, "Scalar" },
> -	{ RTE_ETH_BURST_VECTOR, "Vector" },
> -
> -	{ RTE_ETH_BURST_ALTIVEC, "AltiVec" },
> -	{ RTE_ETH_BURST_NEON, "Neon" },
> -	{ RTE_ETH_BURST_SSE, "SSE" },
> -	{ RTE_ETH_BURST_AVX2, "AVX2" },
> -	{ RTE_ETH_BURST_AVX512, "AVX512" },
> -
> -	{ RTE_ETH_BURST_SCATTERED, "Scattered" },
> -	{ RTE_ETH_BURST_BULK_ALLOC, "Bulk Alloc" },
> -	{ RTE_ETH_BURST_SIMPLE, "Simple" },
> -	{ RTE_ETH_BURST_PER_QUEUE, "Per Queue" },
> -};
> -
>  /**
>   * The user application callback description.
>   *
> @@ -4284,22 +4265,6 @@ rte_eth_tx_burst_mode_get(uint16_t port_id, uint16_t queue_id,
>  		       dev->dev_ops->tx_burst_mode_get(dev, queue_id, mode));
>  }
>  
> -const char *
> -rte_eth_burst_mode_option_name(uint64_t option)
> -{
> -	const char *name = "";
> -	unsigned int i;
> -
> -	for (i = 0; i < RTE_DIM(rte_burst_option_names); ++i) {
> -		if (option == rte_burst_option_names[i].option) {
> -			name = rte_burst_option_names[i].name;
> -			break;
> -		}
> -	}
> -
> -	return name;
> -}
> -
>  int
>  rte_eth_dev_set_mc_addr_list(uint16_t port_id,
>  			     struct rte_ether_addr *mc_addr_set,
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index c36c1b631..88c83c7aa 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1248,32 +1248,23 @@ struct rte_eth_txq_info {
>  } __rte_cache_min_aligned;
>  
>  /**
> - * Burst mode types, values can be ORed to define the burst mode of a driver.
> + * Generic Burst mode flag definition, values can be ORed.
> + */
> +#define RTE_ETH_BURST_FLAG_PER_QUEUE     (1ULL << 0)
> +/**< If the queues have different burst mode description, this bit will be set
> + * by PMD, then the application can iterate to retrieve burst description for
> + * all other queues.
>   */
> -enum rte_eth_burst_mode_option {
> -	RTE_ETH_BURST_SCALAR = (1 << 0),
> -	RTE_ETH_BURST_VECTOR = (1 << 1),
> -
> -	/**< bits[15:2] are reserved for each vector type */
> -	RTE_ETH_BURST_ALTIVEC = (1 << 2),
> -	RTE_ETH_BURST_NEON = (1 << 3),
> -	RTE_ETH_BURST_SSE = (1 << 4),
> -	RTE_ETH_BURST_AVX2 = (1 << 5),
> -	RTE_ETH_BURST_AVX512 = (1 << 6),
> -
> -	RTE_ETH_BURST_SCATTERED = (1 << 16), /**< Support scattered packets */
> -	RTE_ETH_BURST_BULK_ALLOC = (1 << 17), /**< Support mbuf bulk alloc */
> -	RTE_ETH_BURST_SIMPLE = (1 << 18),
> -
> -	RTE_ETH_BURST_PER_QUEUE = (1 << 19), /**< Support per queue burst */
> -};
>  
>  /**
>   * 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 */
>  };
>  
>  /** Maximum name length for extended statistics counters */
> @@ -3706,20 +3697,6 @@ __rte_experimental
>  int rte_eth_tx_burst_mode_get(uint16_t port_id, uint16_t queue_id,
>  	struct rte_eth_burst_mode *mode);
>  
> -/**
> - * Retrieve name about burst mode option.
> - *
> - * @param option
> - *   The burst mode option of type *rte_eth_burst_mode_option*.
> - *
> - * @return
> - *   - "": Not found
> - *   - "xxx": name of the mode option.
> - */
> -__rte_experimental
> -const char *
> -rte_eth_burst_mode_option_name(uint64_t option);
> -
>  /**
>   * Retrieve device registers and register attributes (number of registers and
>   * register size)
> diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
> index e59d51648..5b00b9a40 100644
> --- a/lib/librte_ethdev/rte_ethdev_version.map
> +++ b/lib/librte_ethdev/rte_ethdev_version.map
> @@ -287,5 +287,4 @@ EXPERIMENTAL {
>  	# added in 19.11
>  	rte_eth_rx_burst_mode_get;
>  	rte_eth_tx_burst_mode_get;
> -	rte_eth_burst_mode_option_name;
>  };
> 

Acked-by: Ray Kinsella <mdr@ashroe.eu>
Thomas Monjalon Nov. 6, 2019, 12:33 a.m. UTC | #2
04/11/2019 11:39, Haiyue Wang:
> Change the type of burst mode information from bit field to free string
> data, so that each PMD can describe the Rx/Tx busrt functions flexibly.
> 
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---
> 
> v2: - Drop the bit field for burst mode information handling.

Please use --in-reply-to, so the versions of a patch can be in the same thread.

> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
>  /**
> - * Burst mode types, values can be ORed to define the burst mode of a driver.
> + * Generic Burst mode flag definition, values can be ORed.
> + */
> +#define RTE_ETH_BURST_FLAG_PER_QUEUE     (1ULL << 0)
> +/**< If the queues have different burst mode description, this bit will be set
> + * by PMD, then the application can iterate to retrieve burst description for
> + * all other queues.
>   */

I am not sure you can have a doxygen comment before and after the same item.

> -enum rte_eth_burst_mode_option {
> -	RTE_ETH_BURST_SCALAR = (1 << 0),
> -	RTE_ETH_BURST_VECTOR = (1 << 1),
> -
> -	/**< bits[15:2] are reserved for each vector type */
> -	RTE_ETH_BURST_ALTIVEC = (1 << 2),
> -	RTE_ETH_BURST_NEON = (1 << 3),
> -	RTE_ETH_BURST_SSE = (1 << 4),
> -	RTE_ETH_BURST_AVX2 = (1 << 5),
> -	RTE_ETH_BURST_AVX512 = (1 << 6),
> -
> -	RTE_ETH_BURST_SCATTERED = (1 << 16), /**< Support scattered packets */
> -	RTE_ETH_BURST_BULK_ALLOC = (1 << 17), /**< Support mbuf bulk alloc */
> -	RTE_ETH_BURST_SIMPLE = (1 << 18),
> -
> -	RTE_ETH_BURST_PER_QUEUE = (1 << 19), /**< Support per queue burst */
> -};

Thank you

>  /**
>   * 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)
Haiyue Wang Nov. 6, 2019, 1:21 a.m. UTC | #3
Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, November 6, 2019 08:34
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev@dpdk.org; jerinjacobk@gmail.com; Yigit, Ferruh <ferruh.yigit@intel.com>;
> arybchenko@solarflare.com; viacheslavo@mellanox.com; damarion@cisco.com; Ye, Xiaolong
> <xiaolong.ye@intel.com>; Sun, Chenmin <chenmin.sun@intel.com>; Kinsella, Ray <ray.kinsella@intel.com>;
> Liu, Yu Y <yu.y.liu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2] ethdev: enhance the API for getting burst mode information
> 
> 04/11/2019 11:39, Haiyue Wang:
> > Change the type of burst mode information from bit field to free string
> > data, so that each PMD can describe the Rx/Tx busrt functions flexibly.
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > ---
> >
> > v2: - Drop the bit field for burst mode information handling.
> 
> Please use --in-reply-to, so the versions of a patch can be in the same thread.
> 

Will take care next time.

> > --- a/lib/librte_ethdev/rte_ethdev.h
> > +++ b/lib/librte_ethdev/rte_ethdev.h
> >  /**
> > - * Burst mode types, values can be ORed to define the burst mode of a driver.
> > + * Generic Burst mode flag definition, values can be ORed.
> > + */
> > +#define RTE_ETH_BURST_FLAG_PER_QUEUE     (1ULL << 0)
> > +/**< If the queues have different burst mode description, this bit will be set
> > + * by PMD, then the application can iterate to retrieve burst description for
> > + * all other queues.
> >   */
> 
> I am not sure you can have a doxygen comment before and after the same item.
> 

The first is for all flags, but only one now, so looks like for the same item.
The second is just for RTE_ETH_BURST_FLAG_PER_QUEUE flag.

> > -enum rte_eth_burst_mode_option {
> > -	RTE_ETH_BURST_SCALAR = (1 << 0),
> > -	RTE_ETH_BURST_VECTOR = (1 << 1),
> > -
> > -	/**< bits[15:2] are reserved for each vector type */
> > -	RTE_ETH_BURST_ALTIVEC = (1 << 2),
> > -	RTE_ETH_BURST_NEON = (1 << 3),
> > -	RTE_ETH_BURST_SSE = (1 << 4),
> > -	RTE_ETH_BURST_AVX2 = (1 << 5),
> > -	RTE_ETH_BURST_AVX512 = (1 << 6),
> > -
> > -	RTE_ETH_BURST_SCATTERED = (1 << 16), /**< Support scattered packets */
> > -	RTE_ETH_BURST_BULK_ALLOC = (1 << 17), /**< Support mbuf bulk alloc */
> > -	RTE_ETH_BURST_SIMPLE = (1 << 18),
> > -
> > -	RTE_ETH_BURST_PER_QUEUE = (1 << 19), /**< Support per queue burst */
> > -};
> 
> Thank you
> 
> >  /**
> >   * 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).

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.


>
Haiyue Wang Nov. 6, 2019, 1:40 a.m. UTC | #4
> -----Original Message-----
> From: Wang, Haiyue
> Sent: Wednesday, November 6, 2019 09:22
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org; jerinjacobk@gmail.com; Yigit, Ferruh <ferruh.yigit@intel.com>;
> arybchenko@solarflare.com; viacheslavo@mellanox.com; damarion@cisco.com; Ye, Xiaolong
> <xiaolong.ye@intel.com>; Sun, Chenmin <chenmin.sun@intel.com>; Kinsella, Ray <ray.kinsella@intel.com>;
> Liu, Yu Y <yu.y.liu@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2] ethdev: enhance the API for getting burst mode information
> 
> Hi Thomas,
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Wednesday, November 6, 2019 08:34
> > To: Wang, Haiyue <haiyue.wang@intel.com>
> > Cc: dev@dpdk.org; jerinjacobk@gmail.com; Yigit, Ferruh <ferruh.yigit@intel.com>;
> > arybchenko@solarflare.com; viacheslavo@mellanox.com; damarion@cisco.com; Ye, Xiaolong
> > <xiaolong.ye@intel.com>; Sun, Chenmin <chenmin.sun@intel.com>; Kinsella, Ray
> <ray.kinsella@intel.com>;
> > Liu, Yu Y <yu.y.liu@intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v2] ethdev: enhance the API for getting burst mode information
> >
> > 04/11/2019 11:39, Haiyue Wang:
> > > Change the type of burst mode information from bit field to free string
> > > data, so that each PMD can describe the Rx/Tx busrt functions flexibly.
> > >
> > > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > > ---
> > >
> > > v2: - Drop the bit field for burst mode information handling.
> >
> 
> > > --- a/lib/librte_ethdev/rte_ethdev.h
> > > +++ b/lib/librte_ethdev/rte_ethdev.h
> > >  /**
> > > - * Burst mode types, values can be ORed to define the burst mode of a driver.
> > > + * Generic Burst mode flag definition, values can be ORed.
> > > + */
> > > +#define RTE_ETH_BURST_FLAG_PER_QUEUE     (1ULL << 0)
> > > +/**< If the queues have different burst mode description, this bit will be set
> > > + * by PMD, then the application can iterate to retrieve burst description for
> > > + * all other queues.
> > >   */
> >
> > I am not sure you can have a doxygen comment before and after the same item.
> >
> 
> The first is for all flags, but only one now, so looks like for the same item.
> The second is just for RTE_ETH_BURST_FLAG_PER_QUEUE flag.
> 

Yes, you are right. I misunderstand the doxygen's grammar. Its output is two comments
together.

RTE_ETH_BURST_FLAG_PER_QUEUE
#define RTE_ETH_BURST_FLAG_PER_QUEUE   (1ULL << 0)
Generic Burst mode flag definition, values can be ORed. If the queues have different burst mode description, this bit will be set by PMD, then the application can iterate to retrieve burst description for all other queues.

Definition at line 1253 of file rte_ethdev.h.

I submit a new patch, change the first line to C comment. Now the output is:

RTE_ETH_BURST_FLAG_PER_QUEUE
#define RTE_ETH_BURST_FLAG_PER_QUEUE   (1ULL << 0)
If the queues have different burst mode description, this bit will be set by PMD, then the application can iterate to retrieve burst description for all other queues.

Definition at line 1257 of file rte_ethdev.h.


> 
> >
Thomas Monjalon Nov. 6, 2019, 8:19 a.m. UTC | #5
06/11/2019 02:21, Wang, Haiyue:
> From: Thomas Monjalon <thomas@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.

Patch
diff mbox series

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index efe2812a8..b6039749c 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -350,21 +350,6 @@  nic_stats_mapping_display(portid_t port_id)
 	       nic_stats_mapping_border, nic_stats_mapping_border);
 }
 
-static void
-burst_mode_options_display(uint64_t options)
-{
-	int offset;
-
-	while (options != 0) {
-		offset = rte_bsf64(options);
-
-		printf(" %s",
-		       rte_eth_burst_mode_option_name(1ULL << offset));
-
-		options &= ~(1ULL << offset);
-	}
-}
-
 void
 rx_queue_infos_display(portid_t port_id, uint16_t queue_id)
 {
@@ -397,10 +382,11 @@  rx_queue_infos_display(portid_t port_id, uint16_t queue_id)
 		(qinfo.scattered_rx != 0) ? "on" : "off");
 	printf("\nNumber of RXDs: %hu", qinfo.nb_desc);
 
-	if (rte_eth_rx_burst_mode_get(port_id, queue_id, &mode) == 0) {
-		printf("\nBurst mode:");
-		burst_mode_options_display(mode.options);
-	}
+	if (rte_eth_rx_burst_mode_get(port_id, queue_id, &mode) == 0)
+		printf("\nBurst mode: %s%s",
+		       mode.info,
+		       mode.flags & RTE_ETH_BURST_FLAG_PER_QUEUE ?
+				" (per queue)" : "");
 
 	printf("\n");
 }
@@ -433,10 +419,11 @@  tx_queue_infos_display(portid_t port_id, uint16_t queue_id)
 		(qinfo.conf.tx_deferred_start != 0) ? "on" : "off");
 	printf("\nNumber of TXDs: %hu", qinfo.nb_desc);
 
-	if (rte_eth_tx_burst_mode_get(port_id, queue_id, &mode) == 0) {
-		printf("\nBurst mode:");
-		burst_mode_options_display(mode.options);
-	}
+	if (rte_eth_tx_burst_mode_get(port_id, queue_id, &mode) == 0)
+		printf("\nBurst mode: %s%s",
+		       mode.info,
+		       mode.flags & RTE_ETH_BURST_FLAG_PER_QUEUE ?
+				" (per queue)" : "");
 
 	printf("\n");
 }
diff --git a/doc/guides/nics/features.rst b/doc/guides/nics/features.rst
index d96696801..7a31cf7c8 100644
--- a/doc/guides/nics/features.rst
+++ b/doc/guides/nics/features.rst
@@ -879,8 +879,7 @@  Burst mode info
 Supports to get Rx/Tx packet burst mode information.
 
 * **[implements] eth_dev_ops**: ``rx_burst_mode_get``, ``tx_burst_mode_get``.
-* **[related] API**: ``rte_eth_rx_burst_mode_get()``, ``rte_eth_tx_burst_mode_get()``,
-  ``rte_eth_burst_mode_option_name()``.
+* **[related] API**: ``rte_eth_rx_burst_mode_get()``, ``rte_eth_tx_burst_mode_get()``.
 
 .. _nic_features_other:
 
diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
index ae8e7b2f0..8fd1e7e62 100644
--- a/doc/guides/rel_notes/release_19_11.rst
+++ b/doc/guides/rel_notes/release_19_11.rst
@@ -114,8 +114,6 @@  New Features
   ``rte_eth_tx_burst_mode_get`` that allow an application
   to retrieve the mode information about RX/TX packet burst
   such as Scalar or Vector, and Vector technology like AVX2.
-  Another new function ``rte_eth_burst_mode_option_name`` is
-  provided for burst mode options stringification.
 
 * **Updated the Intel ice driver.**
 
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 6a66cec20..17dc8c78f 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -3017,49 +3017,45 @@  i40e_set_rx_function(struct rte_eth_dev *dev)
 	}
 }
 
-int
-i40e_rx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
-		       struct rte_eth_burst_mode *mode)
-{
-	eth_rx_burst_t pkt_burst = dev->rx_pkt_burst;
-	uint64_t options;
-
-	if (pkt_burst == i40e_recv_scattered_pkts)
-		options = RTE_ETH_BURST_SCALAR | RTE_ETH_BURST_SCATTERED;
-	else if (pkt_burst == i40e_recv_pkts_bulk_alloc)
-		options = RTE_ETH_BURST_SCALAR | RTE_ETH_BURST_BULK_ALLOC;
-	else if (pkt_burst == i40e_recv_pkts)
-		options = RTE_ETH_BURST_SCALAR;
+static const struct {
+	eth_rx_burst_t pkt_burst;
+	const char *info;
+} i40e_rx_burst_infos[] = {
+	{ i40e_recv_scattered_pkts,          "Scalar Scattered" },
+	{ i40e_recv_pkts_bulk_alloc,         "Scalar Bulk Alloc" },
+	{ i40e_recv_pkts,                    "Scalar" },
 #ifdef RTE_ARCH_X86
-	else if (pkt_burst == i40e_recv_scattered_pkts_vec_avx2)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_AVX2 |
-			  RTE_ETH_BURST_SCATTERED;
-	else if (pkt_burst == i40e_recv_pkts_vec_avx2)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_AVX2;
-	else if (pkt_burst == i40e_recv_scattered_pkts_vec)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE |
-			  RTE_ETH_BURST_SCATTERED;
-	else if (pkt_burst == i40e_recv_pkts_vec)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE;
+	{ i40e_recv_scattered_pkts_vec_avx2, "Vector AVX2 Scattered" },
+	{ i40e_recv_pkts_vec_avx2,           "Vector AVX2" },
+	{ i40e_recv_scattered_pkts_vec,      "Vector SSE Scattered" },
+	{ i40e_recv_pkts_vec,                "Vector SSE" },
 #elif defined(RTE_ARCH_ARM64)
-	else if (pkt_burst == i40e_recv_scattered_pkts_vec)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_NEON |
-			  RTE_ETH_BURST_SCATTERED;
-	else if (pkt_burst == i40e_recv_pkts_vec)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_NEON;
+	{ i40e_recv_scattered_pkts_vec,      "Vector Neon Scattered" },
+	{ i40e_recv_pkts_vec,                "Vector Neon" },
 #elif defined(RTE_ARCH_PPC_64)
-	else if (pkt_burst == i40e_recv_scattered_pkts_vec)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_ALTIVEC |
-			  RTE_ETH_BURST_SCATTERED;
-	else if (pkt_burst == i40e_recv_pkts_vec)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_ALTIVEC;
+	{ i40e_recv_scattered_pkts_vec,      "Vector AltiVec Scattered" },
+	{ i40e_recv_pkts_vec,                "Vector AltiVec" },
 #endif
-	else
-		options = 0;
+};
 
-	mode->options = options;
+int
+i40e_rx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
+		       struct rte_eth_burst_mode *mode)
+{
+	eth_rx_burst_t pkt_burst = dev->rx_pkt_burst;
+	int ret = -EINVAL;
+	unsigned int i;
+
+	for (i = 0; i < RTE_DIM(i40e_rx_burst_infos); ++i) {
+		if (pkt_burst == i40e_rx_burst_infos[i].pkt_burst) {
+			snprintf(mode->info, sizeof(mode->info), "%s",
+				 i40e_rx_burst_infos[i].info);
+			ret = 0;
+			break;
+		}
+	}
 
-	return options != 0 ? 0 : -EINVAL;
+	return ret;
 }
 
 void __attribute__((cold))
@@ -3155,35 +3151,40 @@  i40e_set_tx_function(struct rte_eth_dev *dev)
 	}
 }
 
-int
-i40e_tx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
-		       struct rte_eth_burst_mode *mode)
-{
-	eth_tx_burst_t pkt_burst = dev->tx_pkt_burst;
-	uint64_t options;
-
-	if (pkt_burst == i40e_xmit_pkts_simple)
-		options = RTE_ETH_BURST_SCALAR | RTE_ETH_BURST_SIMPLE;
-	else if (pkt_burst == i40e_xmit_pkts)
-		options = RTE_ETH_BURST_SCALAR;
+static const struct {
+	eth_tx_burst_t pkt_burst;
+	const char *info;
+} i40e_tx_burst_infos[] = {
+	{ i40e_xmit_pkts_simple,   "Scalar Simple" },
+	{ i40e_xmit_pkts,          "Scalar" },
 #ifdef RTE_ARCH_X86
-	else if (pkt_burst == i40e_xmit_pkts_vec_avx2)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_AVX2;
-	else if (pkt_burst == i40e_xmit_pkts_vec)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE;
+	{ i40e_xmit_pkts_vec_avx2, "Vector AVX2" },
+	{ i40e_xmit_pkts_vec,      "Vector SSE" },
 #elif defined(RTE_ARCH_ARM64)
-	else if (pkt_burst == i40e_xmit_pkts_vec)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_NEON;
+	{ i40e_xmit_pkts_vec,      "Vector Neon" },
 #elif defined(RTE_ARCH_PPC_64)
-	else if (pkt_burst == i40e_xmit_pkts_vec)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_ALTIVEC;
+	{ i40e_xmit_pkts_vec,      "Vector AltiVec" },
 #endif
-	else
-		options = 0;
+};
 
-	mode->options = options;
+int
+i40e_tx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
+		       struct rte_eth_burst_mode *mode)
+{
+	eth_tx_burst_t pkt_burst = dev->tx_pkt_burst;
+	int ret = -EINVAL;
+	unsigned int i;
+
+	for (i = 0; i < RTE_DIM(i40e_tx_burst_infos); ++i) {
+		if (pkt_burst == i40e_tx_burst_infos[i].pkt_burst) {
+			snprintf(mode->info, sizeof(mode->info), "%s",
+				 i40e_tx_burst_infos[i].info);
+			ret = 0;
+			break;
+		}
+	}
 
-	return options != 0 ? 0 : -EINVAL;
+	return ret;
 }
 
 void __attribute__((cold))
diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 8d4820d3c..bf14d337d 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -2793,37 +2793,39 @@  ice_set_rx_function(struct rte_eth_dev *dev)
 	}
 }
 
+static const struct {
+	eth_rx_burst_t pkt_burst;
+	const char *info;
+} ice_rx_burst_infos[] = {
+	{ ice_recv_scattered_pkts,          "Scalar Scattered" },
+	{ ice_recv_pkts_bulk_alloc,         "Scalar Bulk Alloc" },
+	{ ice_recv_pkts,                    "Scalar" },
+#ifdef RTE_ARCH_X86
+	{ ice_recv_scattered_pkts_vec_avx2, "Vector AVX2 Scattered" },
+	{ ice_recv_pkts_vec_avx2,           "Vector AVX2" },
+	{ ice_recv_scattered_pkts_vec,      "Vector SSE Scattered" },
+	{ ice_recv_pkts_vec,                "Vector SSE" },
+#endif
+};
+
 int
 ice_rx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
 		      struct rte_eth_burst_mode *mode)
 {
 	eth_rx_burst_t pkt_burst = dev->rx_pkt_burst;
-	uint64_t options;
-
-	if (pkt_burst == ice_recv_scattered_pkts)
-		options = RTE_ETH_BURST_SCALAR | RTE_ETH_BURST_SCATTERED;
-	else if (pkt_burst == ice_recv_pkts_bulk_alloc)
-		options = RTE_ETH_BURST_SCALAR | RTE_ETH_BURST_BULK_ALLOC;
-	else if (pkt_burst == ice_recv_pkts)
-		options = RTE_ETH_BURST_SCALAR;
-#ifdef RTE_ARCH_X86
-	else if (pkt_burst == ice_recv_scattered_pkts_vec_avx2)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_AVX2 |
-			  RTE_ETH_BURST_SCATTERED;
-	else if (pkt_burst == ice_recv_pkts_vec_avx2)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_AVX2;
-	else if (pkt_burst == ice_recv_scattered_pkts_vec)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE |
-			  RTE_ETH_BURST_SCATTERED;
-	else if (pkt_burst == ice_recv_pkts_vec)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE;
-#endif
-	else
-		options = 0;
+	int ret = -EINVAL;
+	unsigned int i;
 
-	mode->options = options;
+	for (i = 0; i < RTE_DIM(ice_rx_burst_infos); ++i) {
+		if (pkt_burst == ice_rx_burst_infos[i].pkt_burst) {
+			snprintf(mode->info, sizeof(mode->info), "%s",
+				 ice_rx_burst_infos[i].info);
+			ret = 0;
+			break;
+		}
+	}
 
-	return options != 0 ? 0 : -EINVAL;
+	return ret;
 }
 
 void __attribute__((cold))
@@ -2949,29 +2951,36 @@  ice_set_tx_function(struct rte_eth_dev *dev)
 	}
 }
 
+static const struct {
+	eth_tx_burst_t pkt_burst;
+	const char *info;
+} ice_tx_burst_infos[] = {
+	{ ice_xmit_pkts_simple,   "Scalar Simple" },
+	{ ice_xmit_pkts,          "Scalar" },
+#ifdef RTE_ARCH_X86
+	{ ice_xmit_pkts_vec_avx2, "Vector AVX2" },
+	{ ice_xmit_pkts_vec,      "Vector SSE" },
+#endif
+};
+
 int
 ice_tx_burst_mode_get(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id,
 		      struct rte_eth_burst_mode *mode)
 {
 	eth_tx_burst_t pkt_burst = dev->tx_pkt_burst;
-	uint64_t options;
-
-	if (pkt_burst == ice_xmit_pkts_simple)
-		options = RTE_ETH_BURST_SCALAR | RTE_ETH_BURST_SIMPLE;
-	else if (pkt_burst == ice_xmit_pkts)
-		options = RTE_ETH_BURST_SCALAR;
-#ifdef RTE_ARCH_X86
-	else if (pkt_burst == ice_xmit_pkts_vec_avx2)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_AVX2;
-	else if (pkt_burst == ice_xmit_pkts_vec)
-		options = RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE;
-#endif
-	else
-		options = 0;
+	int ret = -EINVAL;
+	unsigned int i;
 
-	mode->options = options;
+	for (i = 0; i < RTE_DIM(ice_tx_burst_infos); ++i) {
+		if (pkt_burst == ice_tx_burst_infos[i].pkt_burst) {
+			snprintf(mode->info, sizeof(mode->info), "%s",
+				 ice_tx_burst_infos[i].info);
+			ret = 0;
+			break;
+		}
+	}
 
-	return options != 0 ? 0 : -EINVAL;
+	return ret;
 }
 
 /* For each value it means, datasheet of hardware can tell more details
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 7743205d3..208362971 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -166,25 +166,6 @@  static const struct {
 
 #undef RTE_TX_OFFLOAD_BIT2STR
 
-static const struct {
-	uint64_t option;
-	const char *name;
-} rte_burst_option_names[] = {
-	{ RTE_ETH_BURST_SCALAR, "Scalar" },
-	{ RTE_ETH_BURST_VECTOR, "Vector" },
-
-	{ RTE_ETH_BURST_ALTIVEC, "AltiVec" },
-	{ RTE_ETH_BURST_NEON, "Neon" },
-	{ RTE_ETH_BURST_SSE, "SSE" },
-	{ RTE_ETH_BURST_AVX2, "AVX2" },
-	{ RTE_ETH_BURST_AVX512, "AVX512" },
-
-	{ RTE_ETH_BURST_SCATTERED, "Scattered" },
-	{ RTE_ETH_BURST_BULK_ALLOC, "Bulk Alloc" },
-	{ RTE_ETH_BURST_SIMPLE, "Simple" },
-	{ RTE_ETH_BURST_PER_QUEUE, "Per Queue" },
-};
-
 /**
  * The user application callback description.
  *
@@ -4284,22 +4265,6 @@  rte_eth_tx_burst_mode_get(uint16_t port_id, uint16_t queue_id,
 		       dev->dev_ops->tx_burst_mode_get(dev, queue_id, mode));
 }
 
-const char *
-rte_eth_burst_mode_option_name(uint64_t option)
-{
-	const char *name = "";
-	unsigned int i;
-
-	for (i = 0; i < RTE_DIM(rte_burst_option_names); ++i) {
-		if (option == rte_burst_option_names[i].option) {
-			name = rte_burst_option_names[i].name;
-			break;
-		}
-	}
-
-	return name;
-}
-
 int
 rte_eth_dev_set_mc_addr_list(uint16_t port_id,
 			     struct rte_ether_addr *mc_addr_set,
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index c36c1b631..88c83c7aa 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1248,32 +1248,23 @@  struct rte_eth_txq_info {
 } __rte_cache_min_aligned;
 
 /**
- * Burst mode types, values can be ORed to define the burst mode of a driver.
+ * Generic Burst mode flag definition, values can be ORed.
+ */
+#define RTE_ETH_BURST_FLAG_PER_QUEUE     (1ULL << 0)
+/**< If the queues have different burst mode description, this bit will be set
+ * by PMD, then the application can iterate to retrieve burst description for
+ * all other queues.
  */
-enum rte_eth_burst_mode_option {
-	RTE_ETH_BURST_SCALAR = (1 << 0),
-	RTE_ETH_BURST_VECTOR = (1 << 1),
-
-	/**< bits[15:2] are reserved for each vector type */
-	RTE_ETH_BURST_ALTIVEC = (1 << 2),
-	RTE_ETH_BURST_NEON = (1 << 3),
-	RTE_ETH_BURST_SSE = (1 << 4),
-	RTE_ETH_BURST_AVX2 = (1 << 5),
-	RTE_ETH_BURST_AVX512 = (1 << 6),
-
-	RTE_ETH_BURST_SCATTERED = (1 << 16), /**< Support scattered packets */
-	RTE_ETH_BURST_BULK_ALLOC = (1 << 17), /**< Support mbuf bulk alloc */
-	RTE_ETH_BURST_SIMPLE = (1 << 18),
-
-	RTE_ETH_BURST_PER_QUEUE = (1 << 19), /**< Support per queue burst */
-};
 
 /**
  * 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 */
 };
 
 /** Maximum name length for extended statistics counters */
@@ -3706,20 +3697,6 @@  __rte_experimental
 int rte_eth_tx_burst_mode_get(uint16_t port_id, uint16_t queue_id,
 	struct rte_eth_burst_mode *mode);
 
-/**
- * Retrieve name about burst mode option.
- *
- * @param option
- *   The burst mode option of type *rte_eth_burst_mode_option*.
- *
- * @return
- *   - "": Not found
- *   - "xxx": name of the mode option.
- */
-__rte_experimental
-const char *
-rte_eth_burst_mode_option_name(uint64_t option);
-
 /**
  * Retrieve device registers and register attributes (number of registers and
  * register size)
diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index e59d51648..5b00b9a40 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -287,5 +287,4 @@  EXPERIMENTAL {
 	# added in 19.11
 	rte_eth_rx_burst_mode_get;
 	rte_eth_tx_burst_mode_get;
-	rte_eth_burst_mode_option_name;
 };