Hi Ferruh,
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yi...@intel.com>
> Sent: Saturday, November 21, 2020 1:50 AM
> To: Jiawei(Jonny) Wang <jiaw...@nvidia.com>; wenzhuo...@intel.com;
> beilei.x...@intel.com; bernard.iremon...@intel.com; Ori Kam
> <or...@nvidia.com>; Slava Ovsiienko <viachesl...@nvidia.com>; NBU-
> Contact-Thomas Monjalon <tho...@monjalon.net>; Raslan Darawsheh
> <rasl...@nvidia.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v3] app/testpmd: fix testpmd packets dump overlapping
>
> On 11/20/2020 5:35 PM, Jiawei Wang wrote:
> > When testpmd enabled the verbosity for the received packets, if two
> > packets were 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 handling the multiple queues simultaneously.
> >
> > The patch uses one string buffer that collects all the packet dump
> > output into this buffer and then printouts it at last, that guarantees
> > to printout separately the dump output per packet.
> >
> > Fixes: d862c45 ("app/testpmd: move dumping packets to a separate
> > function")
> >
> > Signed-off-by: Jiawei Wang <jiaw...@nvidia.com>
>
> <...>
>
> > @@ -74,13 +85,16 @@
> > uint32_t vx_vni;
> > const char *reason;
> > int dynf_index;
> > + int buf_size = MAX_STRING_LEN;
> > + char print_buf[buf_size];
> > + int cur_len = 0;
> >
> > + memset(print_buf, 0, sizeof(print_buf));
>
> Should 'print_buf' cleaned per each packet below, if not can we drop 'memset'
> completely?
>
Since the 'snprintf' always append the a terminating null character('\0') after
the written string,
and in the code the following character be appended start from previous null
character, so only one
'\0' will be appended in the last 'snprintf' calls,
snprintf(buf + cur_len, buf_size - cur_len ..
At the last 'printf("%s") will print the string buffer up to a terminating null
character ('\0').
so it's ok even not 'memset' calls to clean 'print_buf' per each packets.
> <...>
>
> > + if (cur_len >= buf_size) {
> > + printf("%s ...\n", print_buf);
> > + break;
>
> Why break here? Wouldn't just append some chars to indicate trancation and
> continue be OK?
>
>
Yes, doesn't need 'break' here.
