Re: [dpdk-dev] [PATCH v3] app/pdump: enhance to support multi-core capture

["Pattan, Reshma" <reshma.pattan@intel.com>]

> -----Original Message-----
> From: Varghese, Vipin
> Subject: RE: [PATCH v3] app/pdump: enhance to support multi-core capture
> >
> > >  			"'(port=<port id> | device_id=<pci id or vdev name>),"
> > >  			"(queue=<queue_id>),"
> > >  			"(rx-dev=<iface or pcap file> |"
> > > @@ -415,6 +416,7 @@ print_pdump_stats(void)
> > >  	for (i = 0; i < num_tuples; i++) {
> > >  		printf("##### PDUMP DEBUG STATS #####\n");
> > >  		pt = &pdump_t[i];
> > > +		printf(" == DPDK interface (%d) ==\n", i);
> >
> > Here good to print the portid/deviceid and queue info, instead of
> > printing pdump tuple index  i? User might not understand that.
> I am not sure, why you mention that I am displaying tuple index with I here?
> 

What is i here?
i is the index used in for loop to iterate through the pdump_tuples array.
So printing i doesn't make sense, instead port and queue info are good 
options to print if you want to have this log.

> > Use ### instead of === as above.
> I can do this, but is there specific reasoning for "####" as it is used to represent
> main header?

to follow consistency with  existing display of ###.

> > Why not enable the needed number of cores in core mask using eal
> > options -l
> I think what you are saying is "allow user to pass -l option or -c option before `--

Yes, and remove existing static -c1 method.
So cores that should be enabled will come from eal options, then in --pdump  you 
have new core param which will be used to launch packet dump function for that pdump tuple.
While parsing application pdump core param, you should check this core is enabled in coremask of eal. That is the way how other applications do.

> `". Then before invoking rte_eal_init replace it. Is this your requirement?
> 

After new suggestion of removing -c1 static approach the above question will not be applicable.

> > Ex:
> > If you check l3fwd as an example the cores should enabled using -c or
> > -l and then they have separate --config l3fwd option in which they
> > specify the core on which the packet processing should be run. Please
> > check that and similar would be good here too.
> I have already explained, pdump application makes static assignment of '-cx1'. If
> you try passing '-c' or '-l' the error check in rte_eal_init will prevent such
> assignment.

Now you will remove existing -c1 static assignment and use eal passed -l and -c , so it should be fine now.