From: Ferruh Yigit <ferruh.yigit@intel.com> To: "Jiawei(Jonny) Wang" <jiaweiw@nvidia.com>, "wenzhuo.lu@intel.com" <wenzhuo.lu@intel.com>, "beilei.xing@intel.com" <beilei.xing@intel.com>, "bernard.iremonger@intel.com" <bernard.iremonger@intel.com>, Ori Kam <orika@nvidia.com>, NBU-Contact-Thomas Monjalon <thomas@monjalon.net>, Raslan Darawsheh <rasland@nvidia.com> Cc: "dev@dpdk.org" <dev@dpdk.org> Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd packets dump overlapping Date: Thu, 19 Nov 2020 10:30:04 +0000 Message-ID: <7591f28a-da35-195f-93aa-775a40cb3fc7@intel.com> (raw) In-Reply-To: <BL0PR12MB2419E5B67172DF21B4297FEFC6E10@BL0PR12MB2419.namprd12.prod.outlook.com> On 11/18/2020 6:09 PM, Jiawei(Jonny) Wang wrote: > Hi Ferruh, > >> -----Original Message----- >> From: Ferruh Yigit <ferruh.yigit@intel.com> >> Sent: Tuesday, November 17, 2020 8:07 PM >> To: Jiawei(Jonny) Wang <jiaweiw@nvidia.com>; wenzhuo.lu@intel.com; >> beilei.xing@intel.com; bernard.iremonger@intel.com; Ori Kam >> <orika@nvidia.com>; NBU-Contact-Thomas Monjalon >> <thomas@monjalon.net>; Raslan Darawsheh <rasland@nvidia.com> >> 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 <jiaweiw@nvidia.com> >>> --- >>> 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 <rte_vxlan.h> >>> #include <rte_ethdev.h> >>> #include <rte_flow.h> >>> +#include <rte_log.h> >>> >>> #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.
next prev parent reply other threads:[~2020-11-19 10:30 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-12 8:36 [dpdk-dev] [PATCH] " Jiawei Wang 2020-11-12 16:44 ` Ferruh Yigit 2020-11-13 16:30 ` Jiawei(Jonny) Wang 2020-11-14 12:01 ` [dpdk-dev] [PATCH v2] " Jiawei Wang 2020-11-17 12:06 ` Ferruh Yigit 2020-11-18 18:09 ` Jiawei(Jonny) Wang 2020-11-19 10:30 ` Ferruh Yigit [this message] 2020-11-20 17:35 ` [dpdk-dev] [PATCH v3] " Jiawei Wang 2020-11-20 17:50 ` Ferruh Yigit 2020-11-23 6:13 ` Jiawei(Jonny) Wang 2020-11-23 10:22 ` Ferruh Yigit 2020-11-23 11:03 ` Jiawei(Jonny) Wang 2020-11-23 8:29 ` [dpdk-dev] [PATCH v4] " Jiawei Wang 2020-11-23 11:00 ` [dpdk-dev] [PATCH v5] " Jiawei Wang 2020-11-23 14:20 ` Ferruh Yigit 2021-01-06 14:13 ` [dpdk-dev] [PATCH v6] " Jiawei Wang 2021-01-06 15:54 ` Stephen Hemminger 2021-01-08 8:31 ` Jiawei(Jonny) Wang 2021-01-06 15:55 ` Stephen Hemminger 2021-01-08 8:31 ` Jiawei(Jonny) Wang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=7591f28a-da35-195f-93aa-775a40cb3fc7@intel.com \ --to=ferruh.yigit@intel.com \ --cc=beilei.xing@intel.com \ --cc=bernard.iremonger@intel.com \ --cc=dev@dpdk.org \ --cc=jiaweiw@nvidia.com \ --cc=orika@nvidia.com \ --cc=rasland@nvidia.com \ --cc=thomas@monjalon.net \ --cc=wenzhuo.lu@intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
DPDK patches and discussions This inbox may be cloned and mirrored by anyone: git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \ dev@dpdk.org public-inbox-index dev Example config snippet for mirrors. Newsgroup available over NNTP: nntp://inbox.dpdk.org/inbox.dpdk.dev AGPL code for this site: git clone https://public-inbox.org/public-inbox.git