[PATCH 0/4] bugfix and enhance features for DMA example

[PATCH 0/4] bugfix and enhance features for DMA example

[Chengwen Feng]

This patch set contains four bugfix and enhanced feature patches.

Chengwen Feng (3):
  examples/dma: fix Tx drop statistic is not collected
  examples/dma: support enqueue drop statistic
  examples/dma: add minimal copy size parameter

Huisong Li (1):
  examples/dma: fix MTU configuration

 examples/dma/dmafwd.c | 111 +++++++++++++++++++++++++++++-------------
 1 file changed, 77 insertions(+), 34 deletions(-)

-- 
2.33.0

[PATCH 1/4] examples/dma: fix MTU configuration

[Chengwen Feng]

From: Huisong Li <lihuisong@huawei.com>

The MTU in dma App can be configured by 'max_frame_size' parameters which
have a default value(1518). It's not reasonable to use it directly as MTU.
This patch fix it.

Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 examples/dma/dmafwd.c | 43 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
index 608487e35c..a03ca05129 100644
--- a/examples/dma/dmafwd.c
+++ b/examples/dma/dmafwd.c
@@ -117,7 +117,7 @@ static uint16_t nb_txd = TX_DEFAULT_RINGSIZE;
 static volatile bool force_quit;
 
 static uint32_t dma_batch_sz = MAX_PKT_BURST;
-static uint32_t max_frame_size = RTE_ETHER_MAX_LEN;
+static uint32_t max_frame_size;
 
 /* ethernet addresses of ports */
 static struct rte_ether_addr dma_ports_eth_addr[RTE_MAX_ETHPORTS];
@@ -851,6 +851,38 @@ assign_rings(void)
 }
 /* >8 End of assigning ring structures for packet exchanging. */
 
+static uint32_t
+eth_dev_get_overhead_len(uint32_t max_rx_pktlen, uint16_t max_mtu)
+{
+	uint32_t overhead_len;
+
+	if (max_mtu != UINT16_MAX && max_rx_pktlen > max_mtu)
+		overhead_len = max_rx_pktlen - max_mtu;
+	else
+		overhead_len = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
+
+	return overhead_len;
+}
+
+static int
+config_port_max_pkt_len(struct rte_eth_conf *conf,
+		struct rte_eth_dev_info *dev_info)
+{
+	uint32_t overhead_len;
+
+	if (max_frame_size == 0)
+		return 0;
+
+	if (max_frame_size < RTE_ETHER_MIN_LEN)
+		return -1;
+
+	overhead_len = eth_dev_get_overhead_len(dev_info->max_rx_pktlen,
+			dev_info->max_mtu);
+	conf->rxmode.mtu = max_frame_size - overhead_len;
+
+	return 0;
+}
+
 /*
  * Initializes a given port using global settings and with the RX buffers
  * coming from the mbuf_pool passed as a parameter.
@@ -878,9 +910,6 @@ port_init(uint16_t portid, struct rte_mempool *mbuf_pool, uint16_t nb_queues)
 	struct rte_eth_dev_info dev_info;
 	int ret, i;
 
-	if (max_frame_size > local_port_conf.rxmode.mtu)
-		local_port_conf.rxmode.mtu = max_frame_size;
-
 	/* Skip ports that are not enabled */
 	if ((dma_enabled_port_mask & (1 << portid)) == 0) {
 		printf("Skipping disabled port %u\n", portid);
@@ -895,6 +924,12 @@ port_init(uint16_t portid, struct rte_mempool *mbuf_pool, uint16_t nb_queues)
 		rte_exit(EXIT_FAILURE, "Cannot get device info: %s, port=%u\n",
 			rte_strerror(-ret), portid);
 
+	ret = config_port_max_pkt_len(&local_port_conf, &dev_info);
+	if (ret != 0)
+		rte_exit(EXIT_FAILURE,
+			"Invalid max frame size: %u (port %u)\n",
+			max_frame_size, portid);
+
 	local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
 		dev_info.flow_type_rss_offloads;
 	ret = rte_eth_dev_configure(portid, nb_queues, 1, &local_port_conf);
-- 
2.33.0

[PATCH 2/4] examples/dma: fix Tx drop statistic is not collected

[Chengwen Feng]

The Tx drop statistic was designed to collected by
rte_eth_dev_tx_buffer mechanism, but the application uses
rte_eth_tx_burst to send packets and this lead the Tx drop statistic
was not collected.

This patch removes rte_eth_dev_tx_buffer mechanism to fix the problem.

Fixes: 632bcd9b5d4f ("examples/ioat: print statistics")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 examples/dma/dmafwd.c | 27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
index a03ca05129..dd576bcf77 100644
--- a/examples/dma/dmafwd.c
+++ b/examples/dma/dmafwd.c
@@ -122,7 +122,6 @@ static uint32_t max_frame_size;
 /* ethernet addresses of ports */
 static struct rte_ether_addr dma_ports_eth_addr[RTE_MAX_ETHPORTS];
 
-static struct rte_eth_dev_tx_buffer *tx_buffer[RTE_MAX_ETHPORTS];
 struct rte_mempool *dma_pktmbuf_pool;
 
 /* Print out statistics for one port. */
@@ -484,10 +483,13 @@ dma_tx_port(struct rxtx_port_config *tx_config)
 
 		port_statistics.tx[tx_config->rxtx_port] += nb_tx;
 
-		/* Free any unsent packets. */
-		if (unlikely(nb_tx < nb_dq))
+		if (unlikely(nb_tx < nb_dq)) {
+			port_statistics.tx_dropped[tx_config->rxtx_port] +=
+				(nb_dq - nb_tx);
+			/* Free any unsent packets. */
 			rte_mempool_put_bulk(dma_pktmbuf_pool,
 			(void *)&mbufs[nb_tx], nb_dq - nb_tx);
+		}
 	}
 }
 /* >8 End of transmitting packets from dmadev. */
@@ -970,25 +972,6 @@ port_init(uint16_t portid, struct rte_mempool *mbuf_pool, uint16_t nb_queues)
 			"rte_eth_tx_queue_setup:err=%d,port=%u\n",
 			ret, portid);
 
-	/* Initialize TX buffers */
-	tx_buffer[portid] = rte_zmalloc_socket("tx_buffer",
-			RTE_ETH_TX_BUFFER_SIZE(MAX_PKT_BURST), 0,
-			rte_eth_dev_socket_id(portid));
-	if (tx_buffer[portid] == NULL)
-		rte_exit(EXIT_FAILURE,
-			"Cannot allocate buffer for tx on port %u\n",
-			portid);
-
-	rte_eth_tx_buffer_init(tx_buffer[portid], MAX_PKT_BURST);
-
-	ret = rte_eth_tx_buffer_set_err_callback(tx_buffer[portid],
-		rte_eth_tx_buffer_count_callback,
-		&port_statistics.tx_dropped[portid]);
-	if (ret < 0)
-		rte_exit(EXIT_FAILURE,
-			"Cannot set error callback for tx buffer on port %u\n",
-			portid);
-
 	/* Start device. 8< */
 	ret = rte_eth_dev_start(portid);
 	if (ret < 0)
-- 
2.33.0

[PATCH 3/4] examples/dma: support enqueue drop statistic

[Chengwen Feng]

The copy drop statistic counted in two scenarios: DMA copy failures and
enqueue failures. so it is difficult to locate the problem.

This patch adds enqueue drop statistic to fix the it.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 examples/dma/dmafwd.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
index dd576bcf77..6b1b777cb8 100644
--- a/examples/dma/dmafwd.c
+++ b/examples/dma/dmafwd.c
@@ -58,6 +58,7 @@ struct dma_port_statistics {
 	uint64_t tx[RTE_MAX_ETHPORTS];
 	uint64_t tx_dropped[RTE_MAX_ETHPORTS];
 	uint64_t copy_dropped[RTE_MAX_ETHPORTS];
+	uint64_t enqueue_dropped[RTE_MAX_ETHPORTS];
 };
 struct dma_port_statistics port_statistics;
 struct total_statistics {
@@ -132,12 +133,14 @@ print_port_stats(uint16_t port_id)
 		"\nPackets sent: %34"PRIu64
 		"\nPackets received: %30"PRIu64
 		"\nPackets dropped on tx: %25"PRIu64
-		"\nPackets dropped on copy: %23"PRIu64,
+		"\nPackets dropped on copy: %23"PRIu64
+		"\nPackets dropped on enqueue: %20"PRIu64,
 		port_id,
 		port_statistics.tx[port_id],
 		port_statistics.rx[port_id],
 		port_statistics.tx_dropped[port_id],
-		port_statistics.copy_dropped[port_id]);
+		port_statistics.copy_dropped[port_id],
+		port_statistics.enqueue_dropped[port_id]);
 }
 
 /* Print out statistics for one dmadev device. */
@@ -227,8 +230,9 @@ print_stats(char *prgname)
 			print_port_stats(port_id);
 
 			delta_ts.total_packets_dropped +=
-				port_statistics.tx_dropped[port_id]
-				+ port_statistics.copy_dropped[port_id];
+				port_statistics.tx_dropped[port_id] +
+				port_statistics.copy_dropped[port_id] +
+				port_statistics.enqueue_dropped[port_id];
 			delta_ts.total_packets_tx +=
 				port_statistics.tx[port_id];
 			delta_ts.total_packets_rx +=
@@ -450,7 +454,7 @@ dma_rx_port(struct rxtx_port_config *rx_config)
 			(void *)&pkts_burst_copy[nb_enq],
 			nb_rx - nb_enq);
 
-		port_statistics.copy_dropped[rx_config->rxtx_port] +=
+		port_statistics.enqueue_dropped[rx_config->rxtx_port] +=
 			(nb_rx - nb_enq);
 	}
 }
-- 
2.33.0

[PATCH 4/4] examples/dma: add minimal copy size parameter

[Chengwen Feng]

This patch adds minimal copy size parameter(-m/--min-copy-size), so
when do copy by CPU or DMA, the real copy size will be the maximum of
mbuf's data_len and this parameter.

This parameter was designed to compare the performance between CPU copy
and DMA copy. User could send small packets with a high rate to drive
the performance test.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 examples/dma/dmafwd.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
index 6b1b777cb8..83094ba378 100644
--- a/examples/dma/dmafwd.c
+++ b/examples/dma/dmafwd.c
@@ -25,6 +25,7 @@
 #define CMD_LINE_OPT_RING_SIZE "ring-size"
 #define CMD_LINE_OPT_BATCH_SIZE "dma-batch-size"
 #define CMD_LINE_OPT_FRAME_SIZE "max-frame-size"
+#define CMD_LINE_OPT_COPY_SIZE	"min-copy-size"
 #define CMD_LINE_OPT_STATS_INTERVAL "stats-interval"
 
 /* configurable number of RX/TX ring descriptors */
@@ -119,6 +120,7 @@ static volatile bool force_quit;
 
 static uint32_t dma_batch_sz = MAX_PKT_BURST;
 static uint32_t max_frame_size;
+static uint32_t min_copy_size;
 
 /* ethernet addresses of ports */
 static struct rte_ether_addr dma_ports_eth_addr[RTE_MAX_ETHPORTS];
@@ -208,7 +210,12 @@ print_stats(char *prgname)
 		"Rx Queues = %d, ", nb_queues);
 	status_strlen += snprintf(status_string + status_strlen,
 		sizeof(status_string) - status_strlen,
-		"Ring Size = %d", ring_size);
+		"Ring Size = %d\n", ring_size);
+	status_strlen += snprintf(status_string + status_strlen,
+		sizeof(status_string) - status_strlen,
+		"Min Copy Size = %u Packet Data Room Size = %u",
+		min_copy_size, rte_pktmbuf_data_room_size(dma_pktmbuf_pool) -
+		RTE_PKTMBUF_HEADROOM);
 
 	memset(&ts, 0, sizeof(struct total_statistics));
 
@@ -307,7 +314,8 @@ static inline void
 pktmbuf_sw_copy(struct rte_mbuf *src, struct rte_mbuf *dst)
 {
 	rte_memcpy(rte_pktmbuf_mtod(dst, char *),
-		rte_pktmbuf_mtod(src, char *), src->data_len);
+		rte_pktmbuf_mtod(src, char *),
+		RTE_MAX(src->data_len, min_copy_size));
 }
 /* >8 End of perform packet copy there is a user-defined function. */
 
@@ -324,7 +332,8 @@ dma_enqueue_packets(struct rte_mbuf *pkts[], struct rte_mbuf *pkts_copy[],
 		ret = rte_dma_copy(dev_id, 0,
 			rte_pktmbuf_iova(pkts[i]),
 			rte_pktmbuf_iova(pkts_copy[i]),
-			rte_pktmbuf_data_len(pkts[i]), 0);
+			RTE_MAX(rte_pktmbuf_data_len(pkts[i]), min_copy_size),
+			0);
 
 		if (ret < 0)
 			break;
@@ -576,6 +585,7 @@ dma_usage(const char *prgname)
 	printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
 		"  -b --dma-batch-size: number of requests per DMA batch\n"
 		"  -f --max-frame-size: max frame size\n"
+		"  -m --min-copy-size: minimum copy length\n"
 		"  -p --portmask: hexadecimal bitmask of ports to configure\n"
 		"  -q NQ: number of RX queues per port (default is 1)\n"
 		"  --[no-]mac-updating: Enable or disable MAC addresses updating (enabled by default)\n"
@@ -621,6 +631,7 @@ dma_parse_args(int argc, char **argv, unsigned int nb_ports)
 		"b:"  /* dma batch size */
 		"c:"  /* copy type (sw|hw) */
 		"f:"  /* max frame size */
+		"m:"  /* min copy size */
 		"p:"  /* portmask */
 		"q:"  /* number of RX queues per port */
 		"s:"  /* ring size */
@@ -636,6 +647,7 @@ dma_parse_args(int argc, char **argv, unsigned int nb_ports)
 		{CMD_LINE_OPT_RING_SIZE, required_argument, NULL, 's'},
 		{CMD_LINE_OPT_BATCH_SIZE, required_argument, NULL, 'b'},
 		{CMD_LINE_OPT_FRAME_SIZE, required_argument, NULL, 'f'},
+		{CMD_LINE_OPT_COPY_SIZE, required_argument, NULL, 'm'},
 		{CMD_LINE_OPT_STATS_INTERVAL, required_argument, NULL, 'i'},
 		{NULL, 0, 0, 0}
 	};
@@ -670,6 +682,10 @@ dma_parse_args(int argc, char **argv, unsigned int nb_ports)
 			}
 			break;
 
+		case 'm':
+			min_copy_size = atoi(optarg);
+			break;
+
 		/* portmask */
 		case 'p':
 			dma_enabled_port_mask = dma_parse_portmask(optarg);
@@ -1068,6 +1084,11 @@ main(int argc, char **argv)
 		rte_exit(EXIT_FAILURE, "Cannot init mbuf pool\n");
 	/* >8 End of allocates mempool to hold the mbufs. */
 
+	if (min_copy_size >
+		(uint32_t)(rte_pktmbuf_data_room_size(dma_pktmbuf_pool) -
+			   RTE_PKTMBUF_HEADROOM))
+		rte_exit(EXIT_FAILURE, "Min copy size > packet mbuf size\n");
+
 	/* Initialize each port. 8< */
 	cfg.nb_ports = 0;
 	RTE_ETH_FOREACH_DEV(portid)
-- 
2.33.0

Re: [PATCH 4/4] examples/dma: add minimal copy size parameter

[Bruce Richardson]

On Mon, Apr 11, 2022 at 10:56:34AM +0800, Chengwen Feng wrote:
> This patch adds minimal copy size parameter(-m/--min-copy-size), so
> when do copy by CPU or DMA, the real copy size will be the maximum of
> mbuf's data_len and this parameter.
> 
> This parameter was designed to compare the performance between CPU copy
> and DMA copy. User could send small packets with a high rate to drive
> the performance test.
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>

Hi,

few comments inline below.

/Bruce

> ---
>  examples/dma/dmafwd.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
> index 6b1b777cb8..83094ba378 100644
> --- a/examples/dma/dmafwd.c
> +++ b/examples/dma/dmafwd.c
> @@ -25,6 +25,7 @@
>  #define CMD_LINE_OPT_RING_SIZE "ring-size"
>  #define CMD_LINE_OPT_BATCH_SIZE "dma-batch-size"
>  #define CMD_LINE_OPT_FRAME_SIZE "max-frame-size"
> +#define CMD_LINE_OPT_COPY_SIZE	"min-copy-size"

While I'm not sure this strictly belongs in an example app to show use of
dmadev, I can see the value of it. However, I suggest we need to make it
clearer that it's not directly relevant to the normal use of the app. I
suggest making the parameter "force-min-copy-size" to make it clearer that
it's an explicit override.

>  #define CMD_LINE_OPT_STATS_INTERVAL "stats-interval"
>  
>  /* configurable number of RX/TX ring descriptors */
> @@ -119,6 +120,7 @@ static volatile bool force_quit;
>  
>  static uint32_t dma_batch_sz = MAX_PKT_BURST;
>  static uint32_t max_frame_size;
> +static uint32_t min_copy_size;
>  
>  /* ethernet addresses of ports */
>  static struct rte_ether_addr dma_ports_eth_addr[RTE_MAX_ETHPORTS];
> @@ -208,7 +210,12 @@ print_stats(char *prgname)
>  		"Rx Queues = %d, ", nb_queues);
>  	status_strlen += snprintf(status_string + status_strlen,
>  		sizeof(status_string) - status_strlen,
> -		"Ring Size = %d", ring_size);
> +		"Ring Size = %d\n", ring_size);
> +	status_strlen += snprintf(status_string + status_strlen,
> +		sizeof(status_string) - status_strlen,
> +		"Min Copy Size = %u Packet Data Room Size = %u",
> +		min_copy_size, rte_pktmbuf_data_room_size(dma_pktmbuf_pool) -
> +		RTE_PKTMBUF_HEADROOM);
>  
>  	memset(&ts, 0, sizeof(struct total_statistics));
>  
> @@ -307,7 +314,8 @@ static inline void
>  pktmbuf_sw_copy(struct rte_mbuf *src, struct rte_mbuf *dst)
>  {
>  	rte_memcpy(rte_pktmbuf_mtod(dst, char *),
> -		rte_pktmbuf_mtod(src, char *), src->data_len);
> +		rte_pktmbuf_mtod(src, char *),
> +		RTE_MAX(src->data_len, min_copy_size));
>  }
>  /* >8 End of perform packet copy there is a user-defined function. */
>  
> @@ -324,7 +332,8 @@ dma_enqueue_packets(struct rte_mbuf *pkts[], struct rte_mbuf *pkts_copy[],
>  		ret = rte_dma_copy(dev_id, 0,
>  			rte_pktmbuf_iova(pkts[i]),
>  			rte_pktmbuf_iova(pkts_copy[i]),
> -			rte_pktmbuf_data_len(pkts[i]), 0);
> +			RTE_MAX(rte_pktmbuf_data_len(pkts[i]), min_copy_size),
> +			0);
>  
>  		if (ret < 0)
>  			break;
> @@ -576,6 +585,7 @@ dma_usage(const char *prgname)
>  	printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
>  		"  -b --dma-batch-size: number of requests per DMA batch\n"
>  		"  -f --max-frame-size: max frame size\n"
> +		"  -m --min-copy-size: minimum copy length\n"

The help text needs to be expanded, again to make clear that this is for
perf comparison and the like. Something like "Force a minimum copy length,
	even for smaller packets"

[PATCH v2 0/4] bugfix and enhance features for DMA example

[Chengwen Feng]

This patch set contains four bugfix and enhanced feature patches.

Chengwen Feng (3):
  examples/dma: fix Tx drop statistic is not collected
  examples/dma: support enqueue drop statistic
  examples/dma: add force minimal copy size parameter

Huisong Li (1):
  examples/dma: fix MTU configuration

---
v2:
* add "force" prefix for the minimal copy size parameter to make it 
  clearer to user.

 examples/dma/dmafwd.c | 114 +++++++++++++++++++++++++++++-------------
 1 file changed, 80 insertions(+), 34 deletions(-)

-- 
2.33.0

[PATCH v2 1/4] examples/dma: fix MTU configuration

[Chengwen Feng]

From: Huisong Li <lihuisong@huawei.com>

The MTU in dma App can be configured by 'max_frame_size' parameters which
have a default value(1518). It's not reasonable to use it directly as MTU.
This patch fix it.

Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 examples/dma/dmafwd.c | 43 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
index 608487e35c..a03ca05129 100644
--- a/examples/dma/dmafwd.c
+++ b/examples/dma/dmafwd.c
@@ -117,7 +117,7 @@ static uint16_t nb_txd = TX_DEFAULT_RINGSIZE;
 static volatile bool force_quit;
 
 static uint32_t dma_batch_sz = MAX_PKT_BURST;
-static uint32_t max_frame_size = RTE_ETHER_MAX_LEN;
+static uint32_t max_frame_size;
 
 /* ethernet addresses of ports */
 static struct rte_ether_addr dma_ports_eth_addr[RTE_MAX_ETHPORTS];
@@ -851,6 +851,38 @@ assign_rings(void)
 }
 /* >8 End of assigning ring structures for packet exchanging. */
 
+static uint32_t
+eth_dev_get_overhead_len(uint32_t max_rx_pktlen, uint16_t max_mtu)
+{
+	uint32_t overhead_len;
+
+	if (max_mtu != UINT16_MAX && max_rx_pktlen > max_mtu)
+		overhead_len = max_rx_pktlen - max_mtu;
+	else
+		overhead_len = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
+
+	return overhead_len;
+}
+
+static int
+config_port_max_pkt_len(struct rte_eth_conf *conf,
+		struct rte_eth_dev_info *dev_info)
+{
+	uint32_t overhead_len;
+
+	if (max_frame_size == 0)
+		return 0;
+
+	if (max_frame_size < RTE_ETHER_MIN_LEN)
+		return -1;
+
+	overhead_len = eth_dev_get_overhead_len(dev_info->max_rx_pktlen,
+			dev_info->max_mtu);
+	conf->rxmode.mtu = max_frame_size - overhead_len;
+
+	return 0;
+}
+
 /*
  * Initializes a given port using global settings and with the RX buffers
  * coming from the mbuf_pool passed as a parameter.
@@ -878,9 +910,6 @@ port_init(uint16_t portid, struct rte_mempool *mbuf_pool, uint16_t nb_queues)
 	struct rte_eth_dev_info dev_info;
 	int ret, i;
 
-	if (max_frame_size > local_port_conf.rxmode.mtu)
-		local_port_conf.rxmode.mtu = max_frame_size;
-
 	/* Skip ports that are not enabled */
 	if ((dma_enabled_port_mask & (1 << portid)) == 0) {
 		printf("Skipping disabled port %u\n", portid);
@@ -895,6 +924,12 @@ port_init(uint16_t portid, struct rte_mempool *mbuf_pool, uint16_t nb_queues)
 		rte_exit(EXIT_FAILURE, "Cannot get device info: %s, port=%u\n",
 			rte_strerror(-ret), portid);
 
+	ret = config_port_max_pkt_len(&local_port_conf, &dev_info);
+	if (ret != 0)
+		rte_exit(EXIT_FAILURE,
+			"Invalid max frame size: %u (port %u)\n",
+			max_frame_size, portid);
+
 	local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
 		dev_info.flow_type_rss_offloads;
 	ret = rte_eth_dev_configure(portid, nb_queues, 1, &local_port_conf);
-- 
2.33.0

[PATCH v2 2/4] examples/dma: fix Tx drop statistic is not collected

[Chengwen Feng]

The Tx drop statistic was designed to collected by
rte_eth_dev_tx_buffer mechanism, but the application uses
rte_eth_tx_burst to send packets and this lead the Tx drop statistic
was not collected.

This patch removes rte_eth_dev_tx_buffer mechanism to fix the problem.

Fixes: 632bcd9b5d4f ("examples/ioat: print statistics")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 examples/dma/dmafwd.c | 27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
index a03ca05129..dd576bcf77 100644
--- a/examples/dma/dmafwd.c
+++ b/examples/dma/dmafwd.c
@@ -122,7 +122,6 @@ static uint32_t max_frame_size;
 /* ethernet addresses of ports */
 static struct rte_ether_addr dma_ports_eth_addr[RTE_MAX_ETHPORTS];
 
-static struct rte_eth_dev_tx_buffer *tx_buffer[RTE_MAX_ETHPORTS];
 struct rte_mempool *dma_pktmbuf_pool;
 
 /* Print out statistics for one port. */
@@ -484,10 +483,13 @@ dma_tx_port(struct rxtx_port_config *tx_config)
 
 		port_statistics.tx[tx_config->rxtx_port] += nb_tx;
 
-		/* Free any unsent packets. */
-		if (unlikely(nb_tx < nb_dq))
+		if (unlikely(nb_tx < nb_dq)) {
+			port_statistics.tx_dropped[tx_config->rxtx_port] +=
+				(nb_dq - nb_tx);
+			/* Free any unsent packets. */
 			rte_mempool_put_bulk(dma_pktmbuf_pool,
 			(void *)&mbufs[nb_tx], nb_dq - nb_tx);
+		}
 	}
 }
 /* >8 End of transmitting packets from dmadev. */
@@ -970,25 +972,6 @@ port_init(uint16_t portid, struct rte_mempool *mbuf_pool, uint16_t nb_queues)
 			"rte_eth_tx_queue_setup:err=%d,port=%u\n",
 			ret, portid);
 
-	/* Initialize TX buffers */
-	tx_buffer[portid] = rte_zmalloc_socket("tx_buffer",
-			RTE_ETH_TX_BUFFER_SIZE(MAX_PKT_BURST), 0,
-			rte_eth_dev_socket_id(portid));
-	if (tx_buffer[portid] == NULL)
-		rte_exit(EXIT_FAILURE,
-			"Cannot allocate buffer for tx on port %u\n",
-			portid);
-
-	rte_eth_tx_buffer_init(tx_buffer[portid], MAX_PKT_BURST);
-
-	ret = rte_eth_tx_buffer_set_err_callback(tx_buffer[portid],
-		rte_eth_tx_buffer_count_callback,
-		&port_statistics.tx_dropped[portid]);
-	if (ret < 0)
-		rte_exit(EXIT_FAILURE,
-			"Cannot set error callback for tx buffer on port %u\n",
-			portid);
-
 	/* Start device. 8< */
 	ret = rte_eth_dev_start(portid);
 	if (ret < 0)
-- 
2.33.0

[PATCH v2 3/4] examples/dma: support enqueue drop statistic

[Chengwen Feng]

The copy drop statistic counted in two scenarios: DMA copy failures and
enqueue failures. so it is difficult to locate the problem.

This patch adds enqueue drop statistic to fix the it.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 examples/dma/dmafwd.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
index dd576bcf77..6b1b777cb8 100644
--- a/examples/dma/dmafwd.c
+++ b/examples/dma/dmafwd.c
@@ -58,6 +58,7 @@ struct dma_port_statistics {
 	uint64_t tx[RTE_MAX_ETHPORTS];
 	uint64_t tx_dropped[RTE_MAX_ETHPORTS];
 	uint64_t copy_dropped[RTE_MAX_ETHPORTS];
+	uint64_t enqueue_dropped[RTE_MAX_ETHPORTS];
 };
 struct dma_port_statistics port_statistics;
 struct total_statistics {
@@ -132,12 +133,14 @@ print_port_stats(uint16_t port_id)
 		"\nPackets sent: %34"PRIu64
 		"\nPackets received: %30"PRIu64
 		"\nPackets dropped on tx: %25"PRIu64
-		"\nPackets dropped on copy: %23"PRIu64,
+		"\nPackets dropped on copy: %23"PRIu64
+		"\nPackets dropped on enqueue: %20"PRIu64,
 		port_id,
 		port_statistics.tx[port_id],
 		port_statistics.rx[port_id],
 		port_statistics.tx_dropped[port_id],
-		port_statistics.copy_dropped[port_id]);
+		port_statistics.copy_dropped[port_id],
+		port_statistics.enqueue_dropped[port_id]);
 }
 
 /* Print out statistics for one dmadev device. */
@@ -227,8 +230,9 @@ print_stats(char *prgname)
 			print_port_stats(port_id);
 
 			delta_ts.total_packets_dropped +=
-				port_statistics.tx_dropped[port_id]
-				+ port_statistics.copy_dropped[port_id];
+				port_statistics.tx_dropped[port_id] +
+				port_statistics.copy_dropped[port_id] +
+				port_statistics.enqueue_dropped[port_id];
 			delta_ts.total_packets_tx +=
 				port_statistics.tx[port_id];
 			delta_ts.total_packets_rx +=
@@ -450,7 +454,7 @@ dma_rx_port(struct rxtx_port_config *rx_config)
 			(void *)&pkts_burst_copy[nb_enq],
 			nb_rx - nb_enq);
 
-		port_statistics.copy_dropped[rx_config->rxtx_port] +=
+		port_statistics.enqueue_dropped[rx_config->rxtx_port] +=
 			(nb_rx - nb_enq);
 	}
 }
-- 
2.33.0

[PATCH v2 4/4] examples/dma: add force minimal copy size parameter

[Chengwen Feng]

This patch adds force minimal copy size parameter
(-m/--force-min-copy-size), so when do copy by CPU or DMA, the real copy
size will be the maximum of mbuf's data_len and this parameter.

This parameter was designed to compare the performance between CPU copy
and DMA copy. User could send small packets with a high rate to drive
the performance test.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 examples/dma/dmafwd.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
index 6b1b777cb8..3bd86b98c1 100644
--- a/examples/dma/dmafwd.c
+++ b/examples/dma/dmafwd.c
@@ -25,6 +25,7 @@
 #define CMD_LINE_OPT_RING_SIZE "ring-size"
 #define CMD_LINE_OPT_BATCH_SIZE "dma-batch-size"
 #define CMD_LINE_OPT_FRAME_SIZE "max-frame-size"
+#define CMD_LINE_OPT_FORCE_COPY_SIZE "force-min-copy-size"
 #define CMD_LINE_OPT_STATS_INTERVAL "stats-interval"
 
 /* configurable number of RX/TX ring descriptors */
@@ -119,6 +120,7 @@ static volatile bool force_quit;
 
 static uint32_t dma_batch_sz = MAX_PKT_BURST;
 static uint32_t max_frame_size;
+static uint32_t force_min_copy_size;
 
 /* ethernet addresses of ports */
 static struct rte_ether_addr dma_ports_eth_addr[RTE_MAX_ETHPORTS];
@@ -208,7 +210,13 @@ print_stats(char *prgname)
 		"Rx Queues = %d, ", nb_queues);
 	status_strlen += snprintf(status_string + status_strlen,
 		sizeof(status_string) - status_strlen,
-		"Ring Size = %d", ring_size);
+		"Ring Size = %d\n", ring_size);
+	status_strlen += snprintf(status_string + status_strlen,
+		sizeof(status_string) - status_strlen,
+		"Force Min Copy Size = %u Packet Data Room Size = %u",
+		force_min_copy_size,
+		rte_pktmbuf_data_room_size(dma_pktmbuf_pool) -
+		RTE_PKTMBUF_HEADROOM);
 
 	memset(&ts, 0, sizeof(struct total_statistics));
 
@@ -307,7 +315,8 @@ static inline void
 pktmbuf_sw_copy(struct rte_mbuf *src, struct rte_mbuf *dst)
 {
 	rte_memcpy(rte_pktmbuf_mtod(dst, char *),
-		rte_pktmbuf_mtod(src, char *), src->data_len);
+		rte_pktmbuf_mtod(src, char *),
+		RTE_MAX(src->data_len, force_min_copy_size));
 }
 /* >8 End of perform packet copy there is a user-defined function. */
 
@@ -324,7 +333,9 @@ dma_enqueue_packets(struct rte_mbuf *pkts[], struct rte_mbuf *pkts_copy[],
 		ret = rte_dma_copy(dev_id, 0,
 			rte_pktmbuf_iova(pkts[i]),
 			rte_pktmbuf_iova(pkts_copy[i]),
-			rte_pktmbuf_data_len(pkts[i]), 0);
+			RTE_MAX(rte_pktmbuf_data_len(pkts[i]),
+				force_min_copy_size),
+			0);
 
 		if (ret < 0)
 			break;
@@ -576,6 +587,7 @@ dma_usage(const char *prgname)
 	printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
 		"  -b --dma-batch-size: number of requests per DMA batch\n"
 		"  -f --max-frame-size: max frame size\n"
+		"  -m --force-min-copy-size: force a minimum copy length, even for smaller packets\n"
 		"  -p --portmask: hexadecimal bitmask of ports to configure\n"
 		"  -q NQ: number of RX queues per port (default is 1)\n"
 		"  --[no-]mac-updating: Enable or disable MAC addresses updating (enabled by default)\n"
@@ -621,6 +633,7 @@ dma_parse_args(int argc, char **argv, unsigned int nb_ports)
 		"b:"  /* dma batch size */
 		"c:"  /* copy type (sw|hw) */
 		"f:"  /* max frame size */
+		"m:"  /* force min copy size */
 		"p:"  /* portmask */
 		"q:"  /* number of RX queues per port */
 		"s:"  /* ring size */
@@ -636,6 +649,7 @@ dma_parse_args(int argc, char **argv, unsigned int nb_ports)
 		{CMD_LINE_OPT_RING_SIZE, required_argument, NULL, 's'},
 		{CMD_LINE_OPT_BATCH_SIZE, required_argument, NULL, 'b'},
 		{CMD_LINE_OPT_FRAME_SIZE, required_argument, NULL, 'f'},
+		{CMD_LINE_OPT_FORCE_COPY_SIZE, required_argument, NULL, 'm'},
 		{CMD_LINE_OPT_STATS_INTERVAL, required_argument, NULL, 'i'},
 		{NULL, 0, 0, 0}
 	};
@@ -670,6 +684,10 @@ dma_parse_args(int argc, char **argv, unsigned int nb_ports)
 			}
 			break;
 
+		case 'm':
+			force_min_copy_size = atoi(optarg);
+			break;
+
 		/* portmask */
 		case 'p':
 			dma_enabled_port_mask = dma_parse_portmask(optarg);
@@ -1068,6 +1086,12 @@ main(int argc, char **argv)
 		rte_exit(EXIT_FAILURE, "Cannot init mbuf pool\n");
 	/* >8 End of allocates mempool to hold the mbufs. */
 
+	if (force_min_copy_size >
+		(uint32_t)(rte_pktmbuf_data_room_size(dma_pktmbuf_pool) -
+			   RTE_PKTMBUF_HEADROOM))
+		rte_exit(EXIT_FAILURE,
+			 "Force min copy size > packet mbuf size\n");
+
 	/* Initialize each port. 8< */
 	cfg.nb_ports = 0;
 	RTE_ETH_FOREACH_DEV(portid)
-- 
2.33.0

Re: [PATCH 4/4] examples/dma: add minimal copy size parameter

[fengchengwen]

Hi Bruce, already fix in v2, Thanks.

On 2022/4/11 17:27, Bruce Richardson wrote:
> On Mon, Apr 11, 2022 at 10:56:34AM +0800, Chengwen Feng wrote:
>> This patch adds minimal copy size parameter(-m/--min-copy-size), so
>> when do copy by CPU or DMA, the real copy size will be the maximum of
>> mbuf's data_len and this parameter.
>>
>> This parameter was designed to compare the performance between CPU copy
>> and DMA copy. User could send small packets with a high rate to drive
>> the performance test.
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> 
> Hi,
> 
> few comments inline below.
> 
> /Bruce
> 
>> ---
>>  examples/dma/dmafwd.c | 27 ++++++++++++++++++++++++---
>>  1 file changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
>> index 6b1b777cb8..83094ba378 100644
>> --- a/examples/dma/dmafwd.c
>> +++ b/examples/dma/dmafwd.c
>> @@ -25,6 +25,7 @@
>>  #define CMD_LINE_OPT_RING_SIZE "ring-size"
>>  #define CMD_LINE_OPT_BATCH_SIZE "dma-batch-size"
>>  #define CMD_LINE_OPT_FRAME_SIZE "max-frame-size"
>> +#define CMD_LINE_OPT_COPY_SIZE	"min-copy-size"
> 
> While I'm not sure this strictly belongs in an example app to show use of
> dmadev, I can see the value of it. However, I suggest we need to make it
> clearer that it's not directly relevant to the normal use of the app. I
> suggest making the parameter "force-min-copy-size" to make it clearer that
> it's an explicit override.
> 
>>  #define CMD_LINE_OPT_STATS_INTERVAL "stats-interval"
>>  
>>  /* configurable number of RX/TX ring descriptors */
>> @@ -119,6 +120,7 @@ static volatile bool force_quit;
>>  
>>  static uint32_t dma_batch_sz = MAX_PKT_BURST;
>>  static uint32_t max_frame_size;

...

>> @@ -576,6 +585,7 @@ dma_usage(const char *prgname)
>>  	printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
>>  		"  -b --dma-batch-size: number of requests per DMA batch\n"
>>  		"  -f --max-frame-size: max frame size\n"
>> +		"  -m --min-copy-size: minimum copy length\n"
> 
> The help text needs to be expanded, again to make clear that this is for
> perf comparison and the like. Something like "Force a minimum copy length,
> 	even for smaller packets"
> 
> 
> .
> 

Re: [PATCH v2 2/4] examples/dma: fix Tx drop statistic is not collected

[Bruce Richardson]

On Mon, Apr 11, 2022 at 08:14:57PM +0800, Chengwen Feng wrote:
> The Tx drop statistic was designed to collected by
> rte_eth_dev_tx_buffer mechanism, but the application uses
> rte_eth_tx_burst to send packets and this lead the Tx drop statistic
> was not collected.
> 
> This patch removes rte_eth_dev_tx_buffer mechanism to fix the problem.
> 
> Fixes: 632bcd9b5d4f ("examples/ioat: print statistics")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [PATCH v2 3/4] examples/dma: support enqueue drop statistic

[Bruce Richardson]

On Mon, Apr 11, 2022 at 08:14:58PM +0800, Chengwen Feng wrote:
> The copy drop statistic counted in two scenarios: DMA copy failures and
> enqueue failures. so it is difficult to locate the problem.
> 

Is the app actually tracking copy failures? From a quick glance at the code
it looks to me like the only "copy_failures" are the enqueue failures, in
which case the stat should just be renamed.

> This patch adds enqueue drop statistic to fix the it.
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>  examples/dma/dmafwd.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 

Re: [PATCH v2 4/4] examples/dma: add force minimal copy size parameter

[Bruce Richardson]

On Mon, Apr 11, 2022 at 08:14:59PM +0800, Chengwen Feng wrote:
> This patch adds force minimal copy size parameter
> (-m/--force-min-copy-size), so when do copy by CPU or DMA, the real copy
> size will be the maximum of mbuf's data_len and this parameter.
> 
> This parameter was designed to compare the performance between CPU copy
> and DMA copy. User could send small packets with a high rate to drive
> the performance test.
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [PATCH v2 3/4] examples/dma: support enqueue drop statistic

[fengchengwen]

DMA is a memcopy engine, and from that perspective, I think it's appropriate
to use copy_failures when dma_enqueue fails.

The newly added enqueue_failures is mainly used for rte_ring_enqueue_burst
failures.

Since the app doesn't have a command line, I think adding this field can
immediately identify where it failed.

On 2022/4/13 23:01, Bruce Richardson wrote:
> On Mon, Apr 11, 2022 at 08:14:58PM +0800, Chengwen Feng wrote:
>> The copy drop statistic counted in two scenarios: DMA copy failures and
>> enqueue failures. so it is difficult to locate the problem.
>>
> 
> Is the app actually tracking copy failures? From a quick glance at the code
> it looks to me like the only "copy_failures" are the enqueue failures, in
> which case the stat should just be renamed.
> 
>> This patch adds enqueue drop statistic to fix the it.
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> ---
>>  examples/dma/dmafwd.c | 14 +++++++++-----
>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>
> 
> .
> 

Re: [PATCH v2 3/4] examples/dma: support enqueue drop statistic

[Bruce Richardson]

On Sat, Apr 16, 2022 at 02:19:24PM +0800, fengchengwen wrote:
> DMA is a memcopy engine, and from that perspective, I think it's appropriate
> to use copy_failures when dma_enqueue fails.
> 
> The newly added enqueue_failures is mainly used for rte_ring_enqueue_burst
> failures.
> 
> Since the app doesn't have a command line, I think adding this field can
> immediately identify where it failed.
> 

So one stat is for the HW path and the other is for the SW one? If that is
the case, only one stat should probably be printed out by the app depending
on the mode is it in.

> On 2022/4/13 23:01, Bruce Richardson wrote:
> > On Mon, Apr 11, 2022 at 08:14:58PM +0800, Chengwen Feng wrote:
> >> The copy drop statistic counted in two scenarios: DMA copy failures and
> >> enqueue failures. so it is difficult to locate the problem.
> >>
> > 
> > Is the app actually tracking copy failures? From a quick glance at the code
> > it looks to me like the only "copy_failures" are the enqueue failures, in
> > which case the stat should just be renamed.
> > 
> >> This patch adds enqueue drop statistic to fix the it.
> >>
> >> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> >> ---
> >>  examples/dma/dmafwd.c | 14 +++++++++-----
> >>  1 file changed, 9 insertions(+), 5 deletions(-)
> >>
> > 
> > .
> > 
> 

Re: [PATCH v2 3/4] examples/dma: support enqueue drop statistic

[fengchengwen]

On 2022/4/19 16:45, Bruce Richardson wrote:
> On Sat, Apr 16, 2022 at 02:19:24PM +0800, fengchengwen wrote:
>> DMA is a memcopy engine, and from that perspective, I think it's appropriate
>> to use copy_failures when dma_enqueue fails.
>>
>> The newly added enqueue_failures is mainly used for rte_ring_enqueue_burst
>> failures.
>>
>> Since the app doesn't have a command line, I think adding this field can
>> immediately identify where it failed.
>>
> 
> So one stat is for the HW path and the other is for the SW one? If that is
> the case, only one stat should probably be printed out by the app depending
> on the mode is it in.

For HW path, both copy_failures and enqueue_failures
For SW path, only the enqueue_failure, PS: the value of copy_failures is fixed to be 0

> 
>> On 2022/4/13 23:01, Bruce Richardson wrote:
>>> On Mon, Apr 11, 2022 at 08:14:58PM +0800, Chengwen Feng wrote:
>>>> The copy drop statistic counted in two scenarios: DMA copy failures and
>>>> enqueue failures. so it is difficult to locate the problem.
>>>>
>>>
>>> Is the app actually tracking copy failures? From a quick glance at the code
>>> it looks to me like the only "copy_failures" are the enqueue failures, in
>>> which case the stat should just be renamed.
>>>
>>>> This patch adds enqueue drop statistic to fix the it.
>>>>
>>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>>>> ---
>>>>  examples/dma/dmafwd.c | 14 +++++++++-----
>>>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>>>
>>>
>>> .
>>>
>>
> 
> .
> 

Re: [PATCH v2 3/4] examples/dma: support enqueue drop statistic

[fengchengwen]

After thinking about it, there are other ways (e.g. dmadump/telemetry) to know
which failed, so I think we could drop this patch.

I will send v3 for other patches.

Thanks.

On 2022/4/19 20:09, fengchengwen wrote:
> On 2022/4/19 16:45, Bruce Richardson wrote:
>> On Sat, Apr 16, 2022 at 02:19:24PM +0800, fengchengwen wrote:
>>> DMA is a memcopy engine, and from that perspective, I think it's appropriate
>>> to use copy_failures when dma_enqueue fails.
>>>
>>> The newly added enqueue_failures is mainly used for rte_ring_enqueue_burst
>>> failures.
>>>
>>> Since the app doesn't have a command line, I think adding this field can
>>> immediately identify where it failed.
>>>
>>
>> So one stat is for the HW path and the other is for the SW one? If that is
>> the case, only one stat should probably be printed out by the app depending
>> on the mode is it in.
> 
> For HW path, both copy_failures and enqueue_failures
> For SW path, only the enqueue_failure, PS: the value of copy_failures is fixed to be 0
> 
>>
>>> On 2022/4/13 23:01, Bruce Richardson wrote:
>>>> On Mon, Apr 11, 2022 at 08:14:58PM +0800, Chengwen Feng wrote:
>>>>> The copy drop statistic counted in two scenarios: DMA copy failures and
>>>>> enqueue failures. so it is difficult to locate the problem.
>>>>>
>>>>
>>>> Is the app actually tracking copy failures? From a quick glance at the code
>>>> it looks to me like the only "copy_failures" are the enqueue failures, in
>>>> which case the stat should just be renamed.
>>>>
>>>>> This patch adds enqueue drop statistic to fix the it.
>>>>>
>>>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>>>>> ---
>>>>>  examples/dma/dmafwd.c | 14 +++++++++-----
>>>>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>>>>
>>>>
>>>> .
>>>>
>>>
>>
>> .
>>
> 
> 
> .
> 

[PATCH v3 0/3] bugfix and enhance features for DMA example

[Chengwen Feng]

This patch set contains three bugfix and enhanced feature patches.

Chengwen Feng (2):
  examples/dma: fix Tx drop statistic is not collected
  examples/dma: add force minimal copy size parameter

Huisong Li (1):
  examples/dma: fix MTU configuration

---
v3:
* remove 'enqueue drop statistic' patch.
v2:
* add "force" prefix for the minimal copy size parameter to make it
  clearer to user.

 examples/dma/dmafwd.c | 100 ++++++++++++++++++++++++++++++------------
 1 file changed, 71 insertions(+), 29 deletions(-)

-- 
2.33.0

[PATCH v3 1/3] examples/dma: fix MTU configuration

[Chengwen Feng]

From: Huisong Li <lihuisong@huawei.com>

The MTU in dma App can be configured by 'max_frame_size' parameters which
have a default value(1518). It's not reasonable to use it directly as MTU.
This patch fix it.

Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 examples/dma/dmafwd.c | 43 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
index 608487e35c..a03ca05129 100644
--- a/examples/dma/dmafwd.c
+++ b/examples/dma/dmafwd.c
@@ -117,7 +117,7 @@ static uint16_t nb_txd = TX_DEFAULT_RINGSIZE;
 static volatile bool force_quit;
 
 static uint32_t dma_batch_sz = MAX_PKT_BURST;
-static uint32_t max_frame_size = RTE_ETHER_MAX_LEN;
+static uint32_t max_frame_size;
 
 /* ethernet addresses of ports */
 static struct rte_ether_addr dma_ports_eth_addr[RTE_MAX_ETHPORTS];
@@ -851,6 +851,38 @@ assign_rings(void)
 }
 /* >8 End of assigning ring structures for packet exchanging. */
 
+static uint32_t
+eth_dev_get_overhead_len(uint32_t max_rx_pktlen, uint16_t max_mtu)
+{
+	uint32_t overhead_len;
+
+	if (max_mtu != UINT16_MAX && max_rx_pktlen > max_mtu)
+		overhead_len = max_rx_pktlen - max_mtu;
+	else
+		overhead_len = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
+
+	return overhead_len;
+}
+
+static int
+config_port_max_pkt_len(struct rte_eth_conf *conf,
+		struct rte_eth_dev_info *dev_info)
+{
+	uint32_t overhead_len;
+
+	if (max_frame_size == 0)
+		return 0;
+
+	if (max_frame_size < RTE_ETHER_MIN_LEN)
+		return -1;
+
+	overhead_len = eth_dev_get_overhead_len(dev_info->max_rx_pktlen,
+			dev_info->max_mtu);
+	conf->rxmode.mtu = max_frame_size - overhead_len;
+
+	return 0;
+}
+
 /*
  * Initializes a given port using global settings and with the RX buffers
  * coming from the mbuf_pool passed as a parameter.
@@ -878,9 +910,6 @@ port_init(uint16_t portid, struct rte_mempool *mbuf_pool, uint16_t nb_queues)
 	struct rte_eth_dev_info dev_info;
 	int ret, i;
 
-	if (max_frame_size > local_port_conf.rxmode.mtu)
-		local_port_conf.rxmode.mtu = max_frame_size;
-
 	/* Skip ports that are not enabled */
 	if ((dma_enabled_port_mask & (1 << portid)) == 0) {
 		printf("Skipping disabled port %u\n", portid);
@@ -895,6 +924,12 @@ port_init(uint16_t portid, struct rte_mempool *mbuf_pool, uint16_t nb_queues)
 		rte_exit(EXIT_FAILURE, "Cannot get device info: %s, port=%u\n",
 			rte_strerror(-ret), portid);
 
+	ret = config_port_max_pkt_len(&local_port_conf, &dev_info);
+	if (ret != 0)
+		rte_exit(EXIT_FAILURE,
+			"Invalid max frame size: %u (port %u)\n",
+			max_frame_size, portid);
+
 	local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
 		dev_info.flow_type_rss_offloads;
 	ret = rte_eth_dev_configure(portid, nb_queues, 1, &local_port_conf);
-- 
2.33.0

[PATCH v3 2/3] examples/dma: fix Tx drop statistic is not collected

[Chengwen Feng]

The Tx drop statistic was designed to collected by
rte_eth_dev_tx_buffer mechanism, but the application uses
rte_eth_tx_burst to send packets and this lead the Tx drop statistic
was not collected.

This patch removes rte_eth_dev_tx_buffer mechanism to fix the problem.

Fixes: 632bcd9b5d4f ("examples/ioat: print statistics")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/dma/dmafwd.c | 27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
index a03ca05129..dd576bcf77 100644
--- a/examples/dma/dmafwd.c
+++ b/examples/dma/dmafwd.c
@@ -122,7 +122,6 @@ static uint32_t max_frame_size;
 /* ethernet addresses of ports */
 static struct rte_ether_addr dma_ports_eth_addr[RTE_MAX_ETHPORTS];
 
-static struct rte_eth_dev_tx_buffer *tx_buffer[RTE_MAX_ETHPORTS];
 struct rte_mempool *dma_pktmbuf_pool;
 
 /* Print out statistics for one port. */
@@ -484,10 +483,13 @@ dma_tx_port(struct rxtx_port_config *tx_config)
 
 		port_statistics.tx[tx_config->rxtx_port] += nb_tx;
 
-		/* Free any unsent packets. */
-		if (unlikely(nb_tx < nb_dq))
+		if (unlikely(nb_tx < nb_dq)) {
+			port_statistics.tx_dropped[tx_config->rxtx_port] +=
+				(nb_dq - nb_tx);
+			/* Free any unsent packets. */
 			rte_mempool_put_bulk(dma_pktmbuf_pool,
 			(void *)&mbufs[nb_tx], nb_dq - nb_tx);
+		}
 	}
 }
 /* >8 End of transmitting packets from dmadev. */
@@ -970,25 +972,6 @@ port_init(uint16_t portid, struct rte_mempool *mbuf_pool, uint16_t nb_queues)
 			"rte_eth_tx_queue_setup:err=%d,port=%u\n",
 			ret, portid);
 
-	/* Initialize TX buffers */
-	tx_buffer[portid] = rte_zmalloc_socket("tx_buffer",
-			RTE_ETH_TX_BUFFER_SIZE(MAX_PKT_BURST), 0,
-			rte_eth_dev_socket_id(portid));
-	if (tx_buffer[portid] == NULL)
-		rte_exit(EXIT_FAILURE,
-			"Cannot allocate buffer for tx on port %u\n",
-			portid);
-
-	rte_eth_tx_buffer_init(tx_buffer[portid], MAX_PKT_BURST);
-
-	ret = rte_eth_tx_buffer_set_err_callback(tx_buffer[portid],
-		rte_eth_tx_buffer_count_callback,
-		&port_statistics.tx_dropped[portid]);
-	if (ret < 0)
-		rte_exit(EXIT_FAILURE,
-			"Cannot set error callback for tx buffer on port %u\n",
-			portid);
-
 	/* Start device. 8< */
 	ret = rte_eth_dev_start(portid);
 	if (ret < 0)
-- 
2.33.0

[PATCH v3 3/3] examples/dma: add force minimal copy size parameter

[Chengwen Feng]

This patch adds force minimal copy size parameter
(-m/--force-min-copy-size), so when do copy by CPU or DMA, the real copy
size will be the maximum of mbuf's data_len and this parameter.

This parameter was designed to compare the performance between CPU copy
and DMA copy. User could send small packets with a high rate to drive
the performance test.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/dma/dmafwd.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
index dd576bcf77..67b5a9b22b 100644
--- a/examples/dma/dmafwd.c
+++ b/examples/dma/dmafwd.c
@@ -25,6 +25,7 @@
 #define CMD_LINE_OPT_RING_SIZE "ring-size"
 #define CMD_LINE_OPT_BATCH_SIZE "dma-batch-size"
 #define CMD_LINE_OPT_FRAME_SIZE "max-frame-size"
+#define CMD_LINE_OPT_FORCE_COPY_SIZE "force-min-copy-size"
 #define CMD_LINE_OPT_STATS_INTERVAL "stats-interval"
 
 /* configurable number of RX/TX ring descriptors */
@@ -118,6 +119,7 @@ static volatile bool force_quit;
 
 static uint32_t dma_batch_sz = MAX_PKT_BURST;
 static uint32_t max_frame_size;
+static uint32_t force_min_copy_size;
 
 /* ethernet addresses of ports */
 static struct rte_ether_addr dma_ports_eth_addr[RTE_MAX_ETHPORTS];
@@ -205,7 +207,13 @@ print_stats(char *prgname)
 		"Rx Queues = %d, ", nb_queues);
 	status_strlen += snprintf(status_string + status_strlen,
 		sizeof(status_string) - status_strlen,
-		"Ring Size = %d", ring_size);
+		"Ring Size = %d\n", ring_size);
+	status_strlen += snprintf(status_string + status_strlen,
+		sizeof(status_string) - status_strlen,
+		"Force Min Copy Size = %u Packet Data Room Size = %u",
+		force_min_copy_size,
+		rte_pktmbuf_data_room_size(dma_pktmbuf_pool) -
+		RTE_PKTMBUF_HEADROOM);
 
 	memset(&ts, 0, sizeof(struct total_statistics));
 
@@ -303,7 +311,8 @@ static inline void
 pktmbuf_sw_copy(struct rte_mbuf *src, struct rte_mbuf *dst)
 {
 	rte_memcpy(rte_pktmbuf_mtod(dst, char *),
-		rte_pktmbuf_mtod(src, char *), src->data_len);
+		rte_pktmbuf_mtod(src, char *),
+		RTE_MAX(src->data_len, force_min_copy_size));
 }
 /* >8 End of perform packet copy there is a user-defined function. */
 
@@ -320,7 +329,9 @@ dma_enqueue_packets(struct rte_mbuf *pkts[], struct rte_mbuf *pkts_copy[],
 		ret = rte_dma_copy(dev_id, 0,
 			rte_pktmbuf_iova(pkts[i]),
 			rte_pktmbuf_iova(pkts_copy[i]),
-			rte_pktmbuf_data_len(pkts[i]), 0);
+			RTE_MAX(rte_pktmbuf_data_len(pkts[i]),
+				force_min_copy_size),
+			0);
 
 		if (ret < 0)
 			break;
@@ -572,6 +583,7 @@ dma_usage(const char *prgname)
 	printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
 		"  -b --dma-batch-size: number of requests per DMA batch\n"
 		"  -f --max-frame-size: max frame size\n"
+		"  -m --force-min-copy-size: force a minimum copy length, even for smaller packets\n"
 		"  -p --portmask: hexadecimal bitmask of ports to configure\n"
 		"  -q NQ: number of RX queues per port (default is 1)\n"
 		"  --[no-]mac-updating: Enable or disable MAC addresses updating (enabled by default)\n"
@@ -617,6 +629,7 @@ dma_parse_args(int argc, char **argv, unsigned int nb_ports)
 		"b:"  /* dma batch size */
 		"c:"  /* copy type (sw|hw) */
 		"f:"  /* max frame size */
+		"m:"  /* force min copy size */
 		"p:"  /* portmask */
 		"q:"  /* number of RX queues per port */
 		"s:"  /* ring size */
@@ -632,6 +645,7 @@ dma_parse_args(int argc, char **argv, unsigned int nb_ports)
 		{CMD_LINE_OPT_RING_SIZE, required_argument, NULL, 's'},
 		{CMD_LINE_OPT_BATCH_SIZE, required_argument, NULL, 'b'},
 		{CMD_LINE_OPT_FRAME_SIZE, required_argument, NULL, 'f'},
+		{CMD_LINE_OPT_FORCE_COPY_SIZE, required_argument, NULL, 'm'},
 		{CMD_LINE_OPT_STATS_INTERVAL, required_argument, NULL, 'i'},
 		{NULL, 0, 0, 0}
 	};
@@ -666,6 +680,10 @@ dma_parse_args(int argc, char **argv, unsigned int nb_ports)
 			}
 			break;
 
+		case 'm':
+			force_min_copy_size = atoi(optarg);
+			break;
+
 		/* portmask */
 		case 'p':
 			dma_enabled_port_mask = dma_parse_portmask(optarg);
@@ -1064,6 +1082,12 @@ main(int argc, char **argv)
 		rte_exit(EXIT_FAILURE, "Cannot init mbuf pool\n");
 	/* >8 End of allocates mempool to hold the mbufs. */
 
+	if (force_min_copy_size >
+		(uint32_t)(rte_pktmbuf_data_room_size(dma_pktmbuf_pool) -
+			   RTE_PKTMBUF_HEADROOM))
+		rte_exit(EXIT_FAILURE,
+			 "Force min copy size > packet mbuf size\n");
+
 	/* Initialize each port. 8< */
 	cfg.nb_ports = 0;
 	RTE_ETH_FOREACH_DEV(portid)
-- 
2.33.0

Re: [PATCH v3 2/3] examples/dma: fix Tx drop statistic is not collected

[Kevin Laatz]

On 24/04/2022 07:07, Chengwen Feng wrote:
> The Tx drop statistic was designed to collected by
> rte_eth_dev_tx_buffer mechanism, but the application uses
> rte_eth_tx_burst to send packets and this lead the Tx drop statistic
> was not collected.
>
> This patch removes rte_eth_dev_tx_buffer mechanism to fix the problem.
>
> Fixes: 632bcd9b5d4f ("examples/ioat: print statistics")
> Cc: stable@dpdk.org
>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   examples/dma/dmafwd.c | 27 +++++----------------------
>   1 file changed, 5 insertions(+), 22 deletions(-)
>
Acked-by: Kevin Laatz <kevin.laatz@intel.com>

Re: [PATCH v3 3/3] examples/dma: add force minimal copy size parameter

[Kevin Laatz]

On 24/04/2022 07:07, Chengwen Feng wrote:
> This patch adds force minimal copy size parameter
> (-m/--force-min-copy-size), so when do copy by CPU or DMA, the real copy
> size will be the maximum of mbuf's data_len and this parameter.
>
> This parameter was designed to compare the performance between CPU copy
> and DMA copy. User could send small packets with a high rate to drive
> the performance test.
>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   examples/dma/dmafwd.c | 30 +++++++++++++++++++++++++++---
>   1 file changed, 27 insertions(+), 3 deletions(-)

Acked-by: Kevin Laatz <kevin.laatz@intel.com>

Re: [PATCH v3 1/3] examples/dma: fix MTU configuration

[fengchengwen]

Hi Ferruh

  Could you please review this patch ?

  I notice many examples (e.g. l3fwd/l3fwd-acl/l3fwd-power/l3fwd-thread/l3fwd-graph)
use the same approach (which define eth_dev_get_overhead_len/config_port_max_pkt_len).

  Thanks.

On 2022/4/24 14:07, Chengwen Feng wrote:
> From: Huisong Li <lihuisong@huawei.com>
> 
> The MTU in dma App can be configured by 'max_frame_size' parameters which
> have a default value(1518). It's not reasonable to use it directly as MTU.
> This patch fix it.
> 
> Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>  examples/dma/dmafwd.c | 43 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 39 insertions(+), 4 deletions(-)
> 
> diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
> index 608487e35c..a03ca05129 100644
> --- a/examples/dma/dmafwd.c
> +++ b/examples/dma/dmafwd.c
> @@ -117,7 +117,7 @@ static uint16_t nb_txd = TX_DEFAULT_RINGSIZE;
>  static volatile bool force_quit;
>  
>  static uint32_t dma_batch_sz = MAX_PKT_BURST;
> -static uint32_t max_frame_size = RTE_ETHER_MAX_LEN;
> +static uint32_t max_frame_size;
>  
>  /* ethernet addresses of ports */
>  static struct rte_ether_addr dma_ports_eth_addr[RTE_MAX_ETHPORTS];
> @@ -851,6 +851,38 @@ assign_rings(void)
>  }
>  /* >8 End of assigning ring structures for packet exchanging. */
>  
> +static uint32_t
> +eth_dev_get_overhead_len(uint32_t max_rx_pktlen, uint16_t max_mtu)
> +{
> +	uint32_t overhead_len;
> +
> +	if (max_mtu != UINT16_MAX && max_rx_pktlen > max_mtu)
> +		overhead_len = max_rx_pktlen - max_mtu;
> +	else
> +		overhead_len = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
> +
> +	return overhead_len;
> +}
> +
> +static int
> +config_port_max_pkt_len(struct rte_eth_conf *conf,
> +		struct rte_eth_dev_info *dev_info)
> +{
> +	uint32_t overhead_len;
> +
> +	if (max_frame_size == 0)
> +		return 0;
> +
> +	if (max_frame_size < RTE_ETHER_MIN_LEN)
> +		return -1;
> +
> +	overhead_len = eth_dev_get_overhead_len(dev_info->max_rx_pktlen,
> +			dev_info->max_mtu);
> +	conf->rxmode.mtu = max_frame_size - overhead_len;
> +
> +	return 0;
> +}
> +
>  /*
>   * Initializes a given port using global settings and with the RX buffers
>   * coming from the mbuf_pool passed as a parameter.
> @@ -878,9 +910,6 @@ port_init(uint16_t portid, struct rte_mempool *mbuf_pool, uint16_t nb_queues)
>  	struct rte_eth_dev_info dev_info;
>  	int ret, i;
>  
> -	if (max_frame_size > local_port_conf.rxmode.mtu)
> -		local_port_conf.rxmode.mtu = max_frame_size;
> -
>  	/* Skip ports that are not enabled */
>  	if ((dma_enabled_port_mask & (1 << portid)) == 0) {
>  		printf("Skipping disabled port %u\n", portid);
> @@ -895,6 +924,12 @@ port_init(uint16_t portid, struct rte_mempool *mbuf_pool, uint16_t nb_queues)
>  		rte_exit(EXIT_FAILURE, "Cannot get device info: %s, port=%u\n",
>  			rte_strerror(-ret), portid);
>  
> +	ret = config_port_max_pkt_len(&local_port_conf, &dev_info);
> +	if (ret != 0)
> +		rte_exit(EXIT_FAILURE,
> +			"Invalid max frame size: %u (port %u)\n",
> +			max_frame_size, portid);
> +
>  	local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
>  		dev_info.flow_type_rss_offloads;
>  	ret = rte_eth_dev_configure(portid, nb_queues, 1, &local_port_conf);
> 

Re: [PATCH v3 0/3] bugfix and enhance features for DMA example

[Thomas Monjalon]

24/04/2022 08:07, Chengwen Feng:
> This patch set contains three bugfix and enhanced feature patches.
> 
> Chengwen Feng (2):
>   examples/dma: fix Tx drop statistic is not collected
>   examples/dma: add force minimal copy size parameter
> 
> Huisong Li (1):
>   examples/dma: fix MTU configuration

Applied, thanks.