From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <reshma.pattan@intel.com>
Received: from mga03.intel.com (mga03.intel.com [134.134.136.65])
 by dpdk.org (Postfix) with ESMTP id D32FD1DB8
 for <dev@dpdk.org>; Fri, 29 Mar 2019 11:08:35 +0100 (CET)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from fmsmga001.fm.intel.com ([10.253.24.23])
 by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 29 Mar 2019 03:08:34 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.60,284,1549958400"; d="scan'208";a="159536826"
Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66])
 by fmsmga001.fm.intel.com with ESMTP; 29 Mar 2019 03:08:33 -0700
Received: from irsmsx110.ger.corp.intel.com ([169.254.15.102]) by
 IRSMSX152.ger.corp.intel.com ([169.254.6.139]) with mapi id 14.03.0415.000;
 Fri, 29 Mar 2019 10:08:32 +0000
From: "Pattan, Reshma" <reshma.pattan@intel.com>
To: "Varghese, Vipin" <vipin.varghese@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
CC: "Wiles, Keith" <keith.wiles@intel.com>, "Mcnamara, John"
 <john.mcnamara@intel.com>, "Byrne, Stephen1" <stephen1.byrne@intel.com>,
 "Tamboli, Amit S" <amit.tamboli@intel.com>, "Padubidri, Sanjay A"
 <sanjay.padubidri@intel.com>, "Patel, Amol" <amol.patel@intel.com>,
 "Kovacevic, Marko" <marko.kovacevic@intel.com>
Thread-Topic: [PATCH v3] app/pdump: enhance to support multi-core capture
Thread-Index: AQHU5Xd9BPo1sDxiaESKXmA9wsg486YiXWhQ
Date: Fri, 29 Mar 2019 10:08:32 +0000
Message-ID: <3AEA2BF9852C6F48A459DA490692831F2A432BA2@irsmsx110.ger.corp.intel.com>
References: <20190328145746.11786-1-vipin.varghese@intel.com>
 <20190328150406.12051-1-vipin.varghese@intel.com>
In-Reply-To: <20190328150406.12051-1-vipin.varghese@intel.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNTVmMjIyMDEtZmI3Yy00YWYyLWI2YzQtYTEwMDk3ZTM5NzliIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoibjVIdmRUU1FUT1BmZ1J4XC9mZkFrMk1CMngzaWZFbUFZZWE0eHYweFIxVUJWaWZmYytqczhSK1dpYThKWXJJekwifQ==
x-ctpclassification: CTP_NT
dlp-product: dlpe-windows
dlp-version: 11.0.400.15
dlp-reaction: no-action
x-originating-ip: [163.33.239.181]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v3] app/pdump: enhance to support multi-core
	capture
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 29 Mar 2019 10:08:36 -0000



> -----Original Message-----
> From: Varghese, Vipin
>=20
>  /* true if x is a power of 2 */
>  #define POWEROF2(x) ((((x)-1) & (x)) =3D=3D 0) @@ -144,7 +145,7 @@ stati=
c volatile
> uint8_t quit_signal;  static void  pdump_usage(const char *prgname)  {
> -	printf("usage: %s [EAL options] -- --pdump "
> +	printf("usage: %s [EAL options] -- [-l <list of cores>] --pdump "

Using -l option same as eal is confusing. Please use other name.
Also how about moving this  new option inside --pdump"" so it will be clear=
ly known that the particular core will be associated to that tuple.

Also, I have some major concern, check my below comments.

>  			"'(port=3D<port id> | device_id=3D<pci id or vdev name>),"
>  			"(queue=3D<queue_id>),"
>  			"(rx-dev=3D<iface or pcap file> |"
> @@ -415,6 +416,7 @@ print_pdump_stats(void)
>  	for (i =3D 0; i < num_tuples; i++) {
>  		printf("##### PDUMP DEBUG STATS #####\n");
>  		pt =3D &pdump_t[i];
> +		printf(" =3D=3D DPDK interface (%d) =3D=3D\n", i);

Here good to print the portid/deviceid and queue info, instead of printing =
pdump tuple index  i? User might not understand that.
Use ### instead of =3D=3D=3D as above.
=20
> +
>  static inline void
>  dump_packets(void)
>  {
>  	int i;
> -	struct pdump_tuples *pt;
> +	uint32_t lcore_id =3D 0;
> +
> +	lcore_id =3D rte_get_next_lcore(lcore_id, 1, 1);
> +
> +	if (rte_lcore_count() =3D=3D 1) {
> +		while (!quit_signal) {
> +			for (i =3D 0; i < num_tuples; i++) {
> +				struct pdump_tuples *pt =3D &pdump_t[i];
> +				pdump_packets(pt);
> +			}
> +		}
> +	} else {
> +		printf(" Tuples (%u) lcores (%u)\n",
> +			num_tuples, rte_lcore_count());
> +
> +		if ((uint32_t)num_tuples >=3D rte_lcore_count()) {
> +			printf("Insufficent Cores\n");
Typo %s/Insufficent/


> +	for (i =3D 0; i < argc; i++) {
> +		if (strstr(argv[i], "-l")) {
> +			snprintf(c_flag, RTE_DIM(c_flag), "-l %s", argv[i+1]);

You are taking this as application arguments then making it as eal argument=
  to run the application. =20
Why not enable the needed number of cores in core mask using eal options -l=
 and have new core param in pdump tuple to run that tuple on that core.

Ex:
If you check l3fwd as an example the cores should enabled using -c or -l an=
d then they have separate --config l3fwd option in=20
which they specify the core on which the packet processing should be run. P=
lease check that and similar would be good here too.

> +			strlcpy(argv[i], "", 2);
> +			strlcpy(argv[i + 1], "", 2);

Why is this? Anyway, rte_strlcpy should be used instead of strlcpy.

Thanks,
Reshma

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by dpdk.space (Postfix) with ESMTP id 7A85AA05D3
	for <public@inbox.dpdk.org>; Fri, 29 Mar 2019 11:08:38 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 6B1D82BD3;
	Fri, 29 Mar 2019 11:08:37 +0100 (CET)
Received: from mga03.intel.com (mga03.intel.com [134.134.136.65])
 by dpdk.org (Postfix) with ESMTP id D32FD1DB8
 for <dev@dpdk.org>; Fri, 29 Mar 2019 11:08:35 +0100 (CET)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from fmsmga001.fm.intel.com ([10.253.24.23])
 by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 29 Mar 2019 03:08:34 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.60,284,1549958400"; d="scan'208";a="159536826"
Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66])
 by fmsmga001.fm.intel.com with ESMTP; 29 Mar 2019 03:08:33 -0700
Received: from irsmsx110.ger.corp.intel.com ([169.254.15.102]) by
 IRSMSX152.ger.corp.intel.com ([169.254.6.139]) with mapi id 14.03.0415.000;
 Fri, 29 Mar 2019 10:08:32 +0000
From: "Pattan, Reshma" <reshma.pattan@intel.com>
To: "Varghese, Vipin" <vipin.varghese@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
CC: "Wiles, Keith" <keith.wiles@intel.com>, "Mcnamara, John"
 <john.mcnamara@intel.com>, "Byrne, Stephen1" <stephen1.byrne@intel.com>,
 "Tamboli, Amit S" <amit.tamboli@intel.com>, "Padubidri, Sanjay A"
 <sanjay.padubidri@intel.com>, "Patel, Amol" <amol.patel@intel.com>,
 "Kovacevic, Marko" <marko.kovacevic@intel.com>
Thread-Topic: [PATCH v3] app/pdump: enhance to support multi-core capture
Thread-Index: AQHU5Xd9BPo1sDxiaESKXmA9wsg486YiXWhQ
Date: Fri, 29 Mar 2019 10:08:32 +0000
Message-ID:
 <3AEA2BF9852C6F48A459DA490692831F2A432BA2@irsmsx110.ger.corp.intel.com>
References: <20190328145746.11786-1-vipin.varghese@intel.com>
 <20190328150406.12051-1-vipin.varghese@intel.com>
In-Reply-To: <20190328150406.12051-1-vipin.varghese@intel.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNTVmMjIyMDEtZmI3Yy00YWYyLWI2YzQtYTEwMDk3ZTM5NzliIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoibjVIdmRUU1FUT1BmZ1J4XC9mZkFrMk1CMngzaWZFbUFZZWE0eHYweFIxVUJWaWZmYytqczhSK1dpYThKWXJJekwifQ==
x-ctpclassification: CTP_NT
dlp-product: dlpe-windows
dlp-version: 11.0.400.15
dlp-reaction: no-action
x-originating-ip: [163.33.239.181]
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v3] app/pdump: enhance to support multi-core
	capture
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Message-ID: <20190329100832.Bqw4NJ5DSC8GK3s2npZV89vSkDXb8isPCnRgrVeim0M@z>



> -----Original Message-----
> From: Varghese, Vipin
>=20
>  /* true if x is a power of 2 */
>  #define POWEROF2(x) ((((x)-1) & (x)) =3D=3D 0) @@ -144,7 +145,7 @@ stati=
c volatile
> uint8_t quit_signal;  static void  pdump_usage(const char *prgname)  {
> -	printf("usage: %s [EAL options] -- --pdump "
> +	printf("usage: %s [EAL options] -- [-l <list of cores>] --pdump "

Using -l option same as eal is confusing. Please use other name.
Also how about moving this  new option inside --pdump"" so it will be clear=
ly known that the particular core will be associated to that tuple.

Also, I have some major concern, check my below comments.

>  			"'(port=3D<port id> | device_id=3D<pci id or vdev name>),"
>  			"(queue=3D<queue_id>),"
>  			"(rx-dev=3D<iface or pcap file> |"
> @@ -415,6 +416,7 @@ print_pdump_stats(void)
>  	for (i =3D 0; i < num_tuples; i++) {
>  		printf("##### PDUMP DEBUG STATS #####\n");
>  		pt =3D &pdump_t[i];
> +		printf(" =3D=3D DPDK interface (%d) =3D=3D\n", i);

Here good to print the portid/deviceid and queue info, instead of printing =
pdump tuple index  i? User might not understand that.
Use ### instead of =3D=3D=3D as above.
=20
> +
>  static inline void
>  dump_packets(void)
>  {
>  	int i;
> -	struct pdump_tuples *pt;
> +	uint32_t lcore_id =3D 0;
> +
> +	lcore_id =3D rte_get_next_lcore(lcore_id, 1, 1);
> +
> +	if (rte_lcore_count() =3D=3D 1) {
> +		while (!quit_signal) {
> +			for (i =3D 0; i < num_tuples; i++) {
> +				struct pdump_tuples *pt =3D &pdump_t[i];
> +				pdump_packets(pt);
> +			}
> +		}
> +	} else {
> +		printf(" Tuples (%u) lcores (%u)\n",
> +			num_tuples, rte_lcore_count());
> +
> +		if ((uint32_t)num_tuples >=3D rte_lcore_count()) {
> +			printf("Insufficent Cores\n");
Typo %s/Insufficent/


> +	for (i =3D 0; i < argc; i++) {
> +		if (strstr(argv[i], "-l")) {
> +			snprintf(c_flag, RTE_DIM(c_flag), "-l %s", argv[i+1]);

You are taking this as application arguments then making it as eal argument=
  to run the application. =20
Why not enable the needed number of cores in core mask using eal options -l=
 and have new core param in pdump tuple to run that tuple on that core.

Ex:
If you check l3fwd as an example the cores should enabled using -c or -l an=
d then they have separate --config l3fwd option in=20
which they specify the core on which the packet processing should be run. P=
lease check that and similar would be good here too.

> +			strlcpy(argv[i], "", 2);
> +			strlcpy(argv[i + 1], "", 2);

Why is this? Anyway, rte_strlcpy should be used instead of strlcpy.

Thanks,
Reshma