From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id A94AEA04FD; Mon, 11 Apr 2022 11:27:13 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 45E8340685; Mon, 11 Apr 2022 11:27:13 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mails.dpdk.org (Postfix) with ESMTP id C87BF4067B for ; Mon, 11 Apr 2022 11:27:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1649669232; x=1681205232; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=XzXSUwTm364powO6a7ps1bnTBQFxr7A/MBzExr/EGIg=; b=YXCXD2Ry1NIIroNYuHi5B3AU3v4es6Jin8uMEVTrIRjFmxRbcz8Y50Zj J7p4eluWoac3ldNousrTFS/4zT1imdgVk/FrxAIHHI5ng5tULy37Lbmv1 Jd6ytgivnwX9OySpqI3dYlaPdQQyr6Zsl+Wot9Zs2656YbiYgYy5ZjZWC CvY4SzG4XqlHzoc2uVdWD7IlcO8xttVtgG6szP0aOukW4LS7IqG0FKcOb ZzNaif6tmvXLqt7jusOKFg6XPDcq6OBarIG+gNaeOs4IdO3/w660O8u3J zftoTUNA9RmaD0CjW3OPkFIRggo1acGD1EmrcA3l/+uBaB/ycXEFdFPfm Q==; X-IronPort-AV: E=McAfee;i="6400,9594,10313"; a="260921989" X-IronPort-AV: E=Sophos;i="5.90,251,1643702400"; d="scan'208";a="260921989" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Apr 2022 02:27:10 -0700 X-IronPort-AV: E=Sophos;i="5.90,251,1643702400"; d="scan'208";a="507040125" Received: from bricha3-mobl.ger.corp.intel.com ([10.213.197.243]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 11 Apr 2022 02:27:09 -0700 Date: Mon, 11 Apr 2022 10:27:06 +0100 From: Bruce Richardson To: Chengwen Feng Cc: thomas@monjalon.net, kevin.laatz@intel.com, dev@dpdk.org Subject: Re: [PATCH 4/4] examples/dma: add minimal copy size parameter Message-ID: References: <20220411025634.33032-1-fengchengwen@huawei.com> <20220411025634.33032-5-fengchengwen@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220411025634.33032-5-fengchengwen@huawei.com> X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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 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"