From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0C387A04DD; Thu, 19 Nov 2020 11:30:16 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 444EDF90; Thu, 19 Nov 2020 11:30:14 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 2FCAA3B5 for ; Thu, 19 Nov 2020 11:30:12 +0100 (CET) IronPort-SDR: d541mk9IiSBglDl4MzquKf1+9US3swsUdSrHkcJQEGOzMHDIhFPGVl5C9woqMpecMA0M0VjvEn OG1hRe2Dafng== X-IronPort-AV: E=McAfee;i="6000,8403,9809"; a="150535579" X-IronPort-AV: E=Sophos;i="5.77,490,1596524400"; d="scan'208";a="150535579" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Nov 2020 02:30:10 -0800 IronPort-SDR: 46o63m46t/x3g4aTO0aMcRz35J8nOhhFjjAa2GLBXnSPytZiS39tFXU1E9dHUrGT0YJVBh4ZGL t7HbvRGtdW3w== X-IronPort-AV: E=Sophos;i="5.77,490,1596524400"; d="scan'208";a="359918777" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.211.127]) ([10.213.211.127]) by fmsmga004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Nov 2020 02:30:08 -0800 To: "Jiawei(Jonny) Wang" , "wenzhuo.lu@intel.com" , "beilei.xing@intel.com" , "bernard.iremonger@intel.com" , Ori Kam , NBU-Contact-Thomas Monjalon , Raslan Darawsheh Cc: "dev@dpdk.org" References: <1605170202-293829-1-git-send-email-jiaweiw@nvidia.com> <1605355308-427475-1-git-send-email-jiaweiw@nvidia.com> From: Ferruh Yigit Message-ID: <7591f28a-da35-195f-93aa-775a40cb3fc7@intel.com> Date: Thu, 19 Nov 2020 10:30:04 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd packets dump overlapping X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 11/18/2020 6:09 PM, Jiawei(Jonny) Wang wrote: > Hi Ferruh, > >> -----Original Message----- >> From: Ferruh Yigit >> Sent: Tuesday, November 17, 2020 8:07 PM >> To: Jiawei(Jonny) Wang ; wenzhuo.lu@intel.com; >> beilei.xing@intel.com; bernard.iremonger@intel.com; Ori Kam >> ; NBU-Contact-Thomas Monjalon >> ; Raslan Darawsheh >> Cc: dev@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd packets dump >> overlapping >> >> On 11/14/2020 12:01 PM, Jiawei Wang wrote: >>> When testpmd enabled the verbosity for the received packets, if two >>> packets was received at the same time, for example, sampling packet >>> and normal packet, the dump output of these packets may be overlapping >>> due to multiple core handled the multiple queues simultaneously. >>> >>> The patch uses one string buffer that collects all the packet dump >>> output into this buffer and then printout it at last, that guarantee >>> to printout separately the dump output per packet. >>> >>> Fixes: d862c45 ("app/testpmd: move dumping packets to a separate >>> function") >>> >>> Signed-off-by: Jiawei Wang >>> --- >>> v2: >>> * Print dump output of per packet instead of per burst. >>> * Add the checking for return value of 'snprintf' and exit if required size >> exceed the print buffer size. >>> * Update the log messages. >>> --- >>> app/test-pmd/util.c | 378 >> ++++++++++++++++++++++++++++++++++++++++------------ >>> 1 file changed, 295 insertions(+), 83 deletions(-) >>> >>> diff --git a/app/test-pmd/util.c b/app/test-pmd/util.c index >>> 649bf8f..ffae590 100644 >>> --- a/app/test-pmd/util.c >>> +++ b/app/test-pmd/util.c >>> @@ -12,15 +12,20 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> #include "testpmd.h" >>> >>> -static inline void >>> -print_ether_addr(const char *what, const struct rte_ether_addr >>> *eth_addr) >>> +#define MAX_STRING_LEN 8192 >> >> Isn't '8192' too large for a single packet, what do you think for '2048'? >> > 2K is ok for the normal case, here consider the case that registered mbuf dynamic flags names, > Then total maximum length can reach to 64* RTE_MBUF_DYN_NAMESIZE = 4096 > // char dynf_names[64][RTE_MBUF_DYN_NAMESIZE]; > So 2048 is not enough if all dynf_names be configured as maximum length of dyn_names. > > How about the 5K? I think should be enough for now. OK, and no need to be tight, if there are cases requesting longer string perhaps can keep the size as it is. >> <...> >> >>> @@ -93,95 +103,269 @@ >>> is_encapsulation = RTE_ETH_IS_TUNNEL_PKT(packet_type); >>> ret = rte_flow_get_restore_info(port_id, mb, &info, &error); >>> if (!ret) { >>> - printf("restore info:"); >>> + cur_len += snprintf(print_buf + cur_len, >>> + buf_size - cur_len, >>> + "restore info:"); >> >> What do you think having a macro like below to simplfy the code: >> >> #define FOO(buf, buf_size, cur_len, ...) \ >> do { \ >> if (cur_len >= buf_size) break; \ >> cur_len += snprintf(buf + cur_len, buf_size - cur_len, __VA_ARGS__); \ >> } while (0) >> >> It can be used as: >> FOO(print_buf, buf_size, cur_len, "restore info:"); >> > Agree, will move these common code into a MARCO, > I will use the "MKDUMPSTR" as MARCO name, means that making a dump string buffer. > Ack >>> + if (cur_len >= buf_size) >>> + goto error; >> >> This 'goto' goes out of the loop, not sure about breking the loop and not >> processing all packets if buffer is out of space for one of the packets. >> Anyway if you switch to macro above, the 'goto' is removed completely. >> > Use 'break' only break the do{} while and continue to following processing, but won't filled anymore into printbuf since it will break firstly in the MARCO checking. > Or could use 'goto lable' instead 'break' in the marco? I think cleaner to not have the 'goto', yes processing will continue but no space left case expected to happen very rare.