[4/4] examples/dma: add minimal copy size parameter
Checks
Commit Message
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(-)
Comments
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"
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"
>
>
> .
>
@@ -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)