[dpdk-dev] [PATCH v2] ethdev: enhance the API for getting burst mode information

[dpdk-dev] [PATCH v2] ethdev: enhance the API for getting burst mode information

[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.

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;
 };
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2] ethdev: enhance the API for getting burst mode information

[Ray Kinsella]

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>

Re: [dpdk-dev] [PATCH v2] ethdev: enhance the API for getting burst mode information

[Thomas Monjalon]

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)

Re: [dpdk-dev] [PATCH v2] ethdev: enhance the API for getting burst mode information

[Wang, Haiyue]

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.


> 

[dpdk-dev] [PATCH v3] ethdev: enhance the API for getting burst mode information

[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>
Acked-by: Ray Kinsella <mdr@ashroe.eu>
---
v3: - Fix one doxygen issue.

v2: - http://patchwork.dpdk.org/patch/62368/
    - 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 a009bc068..9da094ec7 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..348b5afb3 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1247,33 +1247,24 @@ struct rte_eth_txq_info {
 	uint16_t nb_desc;           /**< configured number of TXDs. */
 } __rte_cache_min_aligned;
 
+/* Generic Burst mode flag definition, values can be ORed. */
+
 /**
- * Burst mode types, values can be ORed to define the burst mode of a driver.
+ * 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 */
-};
+#define RTE_ETH_BURST_FLAG_PER_QUEUE     (1ULL << 0)
 
 /**
  * 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;
 };
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2] ethdev: enhance the API for getting burst mode information

[Wang, Haiyue]

> -----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.


> 
> >

Re: [dpdk-dev] [PATCH v2] ethdev: enhance the API for getting burst mode information

[Thomas Monjalon]

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.

Re: [dpdk-dev] [PATCH v3] ethdev: enhance the API for getting burst mode information

[Thomas Monjalon]

06/11/2019 02:30, 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>
> Acked-by: Ray Kinsella <mdr@ashroe.eu>

Thank you for accepting my comments which came late.

Acked-by: Thomas Monjalon <thomas@monjalon.net>

Re: [dpdk-dev] [PATCH v3] ethdev: enhance the API for getting burst mode information

[Slava Ovsiienko]

> -----Original Message-----
> From: Haiyue Wang <haiyue.wang@intel.com>
> Sent: Wednesday, November 6, 2019 3:30
> To: dev@dpdk.org; Thomas Monjalon <thomas@monjalon.net>;
> jerinjacobk@gmail.com; ferruh.yigit@intel.com;
> arybchenko@solarflare.com; Slava Ovsiienko <viacheslavo@mellanox.com>;
> damarion@cisco.com; xiaolong.ye@intel.com; chenmin.sun@intel.com;
> ray.kinsella@intel.com; yu.y.liu@intel.com
> Cc: Haiyue Wang <haiyue.wang@intel.com>
> Subject: [PATCH v3] ethdev: enhance the API for getting burst mode
> information
> 
> 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>
> Acked-by: Ray Kinsella <mdr@ashroe.eu>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

Re: [dpdk-dev] [PATCH v3] ethdev: enhance the API for getting burst mode information

[Wang, Haiyue]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, November 6, 2019 17:20
> 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 v3] ethdev: enhance the API for getting burst mode information
> 
> 06/11/2019 02:30, 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>
> > Acked-by: Ray Kinsella <mdr@ashroe.eu>
> 
> Thank you for accepting my comments which came late.
> 
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> 

Thanks for sharing the good programming practice and your patience to
listen and reply.

BR,
Haiyue

Re: [dpdk-dev] [PATCH v3] ethdev: enhance the API for getting burst mode information

[Ferruh Yigit]

On 11/6/2019 10:49 AM, Wang, Haiyue wrote:
>> -----Original Message-----
>> From: Thomas Monjalon <thomas@monjalon.net>
>> Sent: Wednesday, November 6, 2019 17:20
>> 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 v3] ethdev: enhance the API for getting burst mode information
>>
>> 06/11/2019 02:30, 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>
>>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>>
>> Thank you for accepting my comments which came late.
>>
>> Acked-by: Thomas Monjalon <thomas@monjalon.net>
>>

Applied to dpdk-next-net/master, thanks.