Message ID | 20181213050842.64587-1-vipin.varghese@intel.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3E8921B47A; Thu, 13 Dec 2018 06:08:59 +0100 (CET) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id AD3452BF5 for <dev@dpdk.org>; Thu, 13 Dec 2018 06:08:57 +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 fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Dec 2018 21:08:56 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,347,1539673200"; d="scan'208";a="129528820" Received: from unknown (HELO saesrv02-S2600CWR.intel.com) ([10.224.122.203]) by fmsmga001.fm.intel.com with ESMTP; 12 Dec 2018 21:08:54 -0800 From: Vipin Varghese <vipin.varghese@intel.com> To: konstantin.ananyev@intel.com, stephen@networkplumber.org, reshma.pattan@intel.com, dev@dpdk.org, john.mcnamara@intel.com Cc: thomas@monjalon.net, stephen1.byrne@intel.com, amol.patel@intel.com, Vipin Varghese <vipin.varghese@intel.com> Date: Thu, 13 Dec 2018 10:38:33 +0530 Message-Id: <20181213050842.64587-1-vipin.varghese@intel.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20181203055000.39012-2-vipin.varghese@intel.com> References: <20181203055000.39012-2-vipin.varghese@intel.com> Subject: [dpdk-dev] [PATCH v7 0/9] app/proc-info: improve debug of proc-info tool 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> |
Series |
app/proc-info: improve debug of proc-info tool
|
|
Message
Varghese, Vipin
Dec. 13, 2018, 5:08 a.m. UTC
This patch adds new debug functions to existing proc-info tool. Motivation ========== DPDK proc-info tool is been widely used as secondary process to collect stats for any primary process. But these are limited to DPDK NIC ports and basic memory usage. Motivation ========== This patch tries to address the short coming by adding debug for port, traffic manager, crypto, ring and mempool. With these additional information in tool will be able to deliver helpful data to debug issues and performance variance. Status ====== With the following patch set debug data has been collected from customers using DPDK instances. Analysing the information helped to suggest the next debug steps and solutions for fixing the issues. Next Steps ========== - add event dev debug information. - enhance iter_mempool for INLINE crypto entries. - add debug for libraries like hash and acl. - explore debug possibility for ring and list. - add links and reference to 'how to guide' for 'debug and troubleshoot'. Patch set Information ===================== Patch includes set of 9 patches: * 0 : to introduce the debug function enhancement for proc-info * 1 : MACRO and help usage for new functions * 2 : string compare for new functions * 3 : invocation for the new prototypes * 4 : show port * 5 : show tm * 6 : show crypto * 7 : ring element debug * 8 : mempool element debug * 9 : iterate mempoool elements Vipin Varghese (9): app/procinfo: add usage for new debug app/procinfo: add compare for new options app/procinfo: add prototype for debug instances app/procinfo: add support for show port app/procinfo: add support for show tm app/procinfo: add support for show crypto app/procinfo: add support for debug ring app/procinfo: add support for show iter mempool doc/procinfo: add information for debug options app/proc-info/main.c | 681 ++++++++++++++++++++++++++++++++- app/proc-info/meson.build | 2 +- doc/guides/tools/proc_info.rst | 35 +- 3 files changed, 712 insertions(+), 6 deletions(-)
Comments
Hi > -----Original Message----- > From: Varghese, Vipin > Sent: Thursday, December 13, 2018 5:09 AM > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; > stephen@networkplumber.org; Pattan, Reshma <reshma.pattan@intel.com>; > dev@dpdk.org; Mcnamara, John <john.mcnamara@intel.com> > Cc: thomas@monjalon.net; Byrne, Stephen1 <stephen1.byrne@intel.com>; > Patel, Amol <amol.patel@intel.com>; Varghese, Vipin > <vipin.varghese@intel.com> > Subject: [PATCH v7 0/9] app/proc-info: improve debug of proc-info tool > > This patch adds new debug functions to existing proc-info tool. > > Motivation > ========== > > DPDK proc-info tool is been widely used as secondary process to collect stats for > any primary process. But these are limited to DPDK NIC ports and basic memory > usage. > > Motivation > ========== > > This patch tries to address the short coming by adding debug for port, traffic > manager, crypto, ring and mempool. With these additional information in tool > will be able to deliver helpful data to debug issues and performance variance. > > Status > ====== > > With the following patch set debug data has been collected from customers > using DPDK instances. Analysing the information helped to suggest the next > debug steps and solutions for fixing the issues. > > Next Steps > ========== > > - add event dev debug information. > - enhance iter_mempool for INLINE crypto entries. > - add debug for libraries like hash and acl. > - explore debug possibility for ring and list. > - add links and reference to 'how to guide' for 'debug and troubleshoot'. > > Patch set Information > ===================== > > Patch includes set of 9 patches: > * 0 : to introduce the debug function enhancement for proc-info > * 1 : MACRO and help usage for new functions > * 2 : string compare for new functions > * 3 : invocation for the new prototypes > * 4 : show port > * 5 : show tm > * 6 : show crypto > * 7 : ring element debug > * 8 : mempool element debug > * 9 : iterate mempoool elements > Small nits 9th patch in this set is doc. So above info need to be corrected. if you are addressing my earlier comment of separating out mempool element iteration changes in to separate new patch 9/10 .Please keep my ack in next version Reviewed-by: Reshma Pattan <reshma.pattan@intel.com> Acked-by: Reshma Pattan <reshma.pattan@intel.com>
HI Reshma > -----Original Message----- > From: Pattan, Reshma > Sent: Thursday, December 13, 2018 8:13 PM > To: Varghese, Vipin <vipin.varghese@intel.com>; Ananyev, Konstantin > <konstantin.ananyev@intel.com>; stephen@networkplumber.org; > dev@dpdk.org; Mcnamara, John <john.mcnamara@intel.com> > Cc: thomas@monjalon.net; Byrne, Stephen1 <stephen1.byrne@intel.com>; > Patel, Amol <amol.patel@intel.com> > Subject: RE: [PATCH v7 0/9] app/proc-info: improve debug of proc-info tool > > Hi > > > -----Original Message----- > > From: Varghese, Vipin > > Sent: Thursday, December 13, 2018 5:09 AM > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; > > stephen@networkplumber.org; Pattan, Reshma > <reshma.pattan@intel.com>; > > dev@dpdk.org; Mcnamara, John <john.mcnamara@intel.com> > > Cc: thomas@monjalon.net; Byrne, Stephen1 <stephen1.byrne@intel.com>; > > Patel, Amol <amol.patel@intel.com>; Varghese, Vipin > > <vipin.varghese@intel.com> > > Subject: [PATCH v7 0/9] app/proc-info: improve debug of proc-info tool > > > > This patch adds new debug functions to existing proc-info tool. > > > > Motivation > > ========== > > > > DPDK proc-info tool is been widely used as secondary process to > > collect stats for any primary process. But these are limited to DPDK > > NIC ports and basic memory usage. > > > > Motivation > > ========== > > > > This patch tries to address the short coming by adding debug for port, > > traffic manager, crypto, ring and mempool. With these additional > > information in tool will be able to deliver helpful data to debug issues and > performance variance. > > > > Status > > ====== > > > > With the following patch set debug data has been collected from > > customers using DPDK instances. Analysing the information helped to > > suggest the next debug steps and solutions for fixing the issues. > > > > Next Steps > > ========== > > > > - add event dev debug information. > > - enhance iter_mempool for INLINE crypto entries. > > - add debug for libraries like hash and acl. > > - explore debug possibility for ring and list. > > - add links and reference to 'how to guide' for 'debug and troubleshoot'. > > > > Patch set Information > > ===================== > > > > Patch includes set of 9 patches: > > * 0 : to introduce the debug function enhancement for proc-info > > * 1 : MACRO and help usage for new functions > > * 2 : string compare for new functions > > * 3 : invocation for the new prototypes > > * 4 : show port > > * 5 : show tm > > * 6 : show crypto > > * 7 : ring element debug > > * 8 : mempool element debug > > * 9 : iterate mempoool elements > > > > Small nits > 9th patch in this set is doc. So above info need to be corrected. > if you are addressing my earlier comment of separating out mempool element > iteration changes in to separate new patch 9/10 .Please keep my ack in next > version Thanks for pointing this out, Like updated in email and chat I am not planning to split it. Hence no version 8. > > Reviewed-by: Reshma Pattan <reshma.pattan@intel.com> > Acked-by: Reshma Pattan <reshma.pattan@intel.com> > > > > >
13/12/2018 17:21, Varghese, Vipin: > HI Reshma > > From: Pattan, Reshma > > From: Varghese, Vipin > > > > > > This patch adds new debug functions to existing proc-info tool. > > > > > > Motivation > > > ========== > > > > > > DPDK proc-info tool is been widely used as secondary process to > > > collect stats for any primary process. But these are limited to DPDK > > > NIC ports and basic memory usage. > > > > > > Motivation > > > ========== > > > > > > This patch tries to address the short coming by adding debug for port, > > > traffic manager, crypto, ring and mempool. With these additional > > > information in tool will be able to deliver helpful data to debug issues and > > performance variance. > > > > > > Status > > > ====== > > > > > > With the following patch set debug data has been collected from > > > customers using DPDK instances. Analysing the information helped to > > > suggest the next debug steps and solutions for fixing the issues. > > > > > > Next Steps > > > ========== > > > > > > - add event dev debug information. > > > - enhance iter_mempool for INLINE crypto entries. > > > - add debug for libraries like hash and acl. > > > - explore debug possibility for ring and list. > > > - add links and reference to 'how to guide' for 'debug and troubleshoot'. > > > > > > Patch set Information > > > ===================== > > > > > > Patch includes set of 9 patches: > > > * 0 : to introduce the debug function enhancement for proc-info > > > * 1 : MACRO and help usage for new functions > > > * 2 : string compare for new functions > > > * 3 : invocation for the new prototypes > > > * 4 : show port > > > * 5 : show tm > > > * 6 : show crypto > > > * 7 : ring element debug > > > * 8 : mempool element debug > > > * 9 : iterate mempoool elements > > > > > > > Small nits > > 9th patch in this set is doc. So above info need to be corrected. > > if you are addressing my earlier comment of separating out mempool element > > iteration changes in to separate new patch 9/10 .Please keep my ack in next > > version > > Thanks for pointing this out, Like updated in email and chat I am not planning to split it. Hence no version 8. So, no ack and no merge? Looking at the first patches + doc patch, the split is not meaningful. You should merge doc and option parsing in the related patches. For instance, parsing and doc of "tm" option should be in the "tm" patch.
HI Thomas, Snipped > > > Small nits > > > 9th patch in this set is doc. So above info need to be corrected. > > > if you are addressing my earlier comment of separating out mempool > > > element iteration changes in to separate new patch 9/10 .Please keep > > > my ack in next version > > > > Thanks for pointing this out, Like updated in email and chat I am not > planning to split it. Hence no version 8. > > So, no ack and no merge? > > Looking at the first patches + doc patch, the split is not meaningful. > You should merge doc and option parsing in the related patches. > For instance, parsing and doc of "tm" option should be in the "tm" patch. > > I did not follow you request. Are you stating, for each functionality I should be updating document rather than 1 document update after adding the new functions? If former is true I am not able to find such reasoning stated in guideline or documentation or from the maintainer. Thanks Vipin Varghese
26/12/2018 06:21, Varghese, Vipin: > HI Thomas, > > Snipped > > > > > Small nits > > > > 9th patch in this set is doc. So above info need to be corrected. > > > > if you are addressing my earlier comment of separating out mempool > > > > element iteration changes in to separate new patch 9/10 .Please keep > > > > my ack in next version > > > > > > Thanks for pointing this out, Like updated in email and chat I am not > > planning to split it. Hence no version 8. > > > > So, no ack and no merge? > > > > Looking at the first patches + doc patch, the split is not meaningful. > > You should merge doc and option parsing in the related patches. > > For instance, parsing and doc of "tm" option should be in the "tm" patch. > > I did not follow you request. Are you stating, for each functionality I should be updating document rather than 1 document update after adding the new functions? If former is true I am not able to find such reasoning stated in guideline or documentation or from the maintainer. Yes, you should update the doc while adding a new feature. But most importantly, there is no reason to do a patch adding some empty functions and filling them later. And please consider the option parsing is part of the feature.
snipped > > > > > Small nits > > > > > 9th patch in this set is doc. So above info need to be corrected. > > > > > if you are addressing my earlier comment of separating out > > > > > mempool element iteration changes in to separate new patch 9/10 > > > > > .Please keep my ack in next version > > > > > > > > Thanks for pointing this out, Like updated in email and chat I am > > > > not > > > planning to split it. Hence no version 8. > > > > > > So, no ack and no merge? > > > > > > Looking at the first patches + doc patch, the split is not meaningful. > > > You should merge doc and option parsing in the related patches. > > > For instance, parsing and doc of "tm" option should be in the "tm" patch. > > > > I did not follow you request. Are you stating, for each functionality I should > be updating document rather than 1 document update after adding the new > functions? If former is true I am not able to find such reasoning stated in > guideline or documentation or from the maintainer. > > Yes, you should update the doc while adding a new feature. Ok, I will comply to your requirement even though it is not in 'guideline, documentation or from maintainer'. Humbly requesting to update documentation and guideline suggesting the same. This will also help others to submit patches according the new guideline. Once reflected it will be justified for sending a v8. > But most importantly, there is no reason to do a patch adding some empty > functions and filling them later. Following are the reasons for using stub function from v1 onwards till v7 1. Without the dummy function there are compiler warnings for unused variables. 2. It is logical to have stub functions for the new parse option being added in one go. These are based on the suggestion from the maintainer. > And please consider the option parsing is part of the feature. As mentioned above please find the reasoning stated for patches from v1 to v7.
27/12/2018 03:46, Varghese, Vipin: > snipped > > > > > > Small nits > > > > > > 9th patch in this set is doc. So above info need to be corrected. > > > > > > if you are addressing my earlier comment of separating out > > > > > > mempool element iteration changes in to separate new patch 9/10 > > > > > > .Please keep my ack in next version > > > > > > > > > > Thanks for pointing this out, Like updated in email and chat I am > > > > > not > > > > planning to split it. Hence no version 8. > > > > > > > > So, no ack and no merge? > > > > > > > > Looking at the first patches + doc patch, the split is not meaningful. > > > > You should merge doc and option parsing in the related patches. > > > > For instance, parsing and doc of "tm" option should be in the "tm" patch. > > > > > > I did not follow you request. Are you stating, for each functionality I should > > be updating document rather than 1 document update after adding the new > > functions? If former is true I am not able to find such reasoning stated in > > guideline or documentation or from the maintainer. > > > > Yes, you should update the doc while adding a new feature. > Ok, I will comply to your requirement even though it is not in 'guideline, documentation or from maintainer'. Humbly requesting to update documentation and guideline suggesting the same. This will also help others to submit patches according the new guideline. Once reflected it will be justified for sending a v8. Vipin, please read the doc carefully: http://git.dpdk.org/dpdk/commit/?id=9e0e4a00df775 > > But most importantly, there is no reason to do a patch adding some empty > > functions and filling them later. > Following are the reasons for using stub function from v1 onwards till v7 > 1. Without the dummy function there are compiler warnings for unused variables. > 2. It is logical to have stub functions for the new parse option being added in one go. > > These are based on the suggestion from the maintainer. > > > And please consider the option parsing is part of the feature. > As mentioned above please find the reasoning stated for patches from v1 to v7. You keep thinking that parsing should be introduced separately. I keep saying it is part of the feature.
> -----Original Message----- > From: Thomas Monjalon <thomas@monjalon.net> > Sent: Thursday, December 27, 2018 3:03 PM > To: Varghese, Vipin <vipin.varghese@intel.com> > Cc: Pattan, Reshma <reshma.pattan@intel.com>; dev@dpdk.org; Ananyev, > Konstantin <konstantin.ananyev@intel.com>; stephen@networkplumber.org; > Mcnamara, John <john.mcnamara@intel.com>; Byrne, Stephen1 > <stephen1.byrne@intel.com>; Patel, Amol <amol.patel@intel.com> > Subject: Re: [dpdk-dev] [PATCH v7 0/9] app/proc-info: improve debug of proc- > info tool > > 27/12/2018 03:46, Varghese, Vipin: > > snipped > > > > > > > Small nits > > > > > > > 9th patch in this set is doc. So above info need to be corrected. > > > > > > > if you are addressing my earlier comment of separating out > > > > > > > mempool element iteration changes in to separate new patch > > > > > > > 9/10 .Please keep my ack in next version > > > > > > > > > > > > Thanks for pointing this out, Like updated in email and chat I > > > > > > am not > > > > > planning to split it. Hence no version 8. > > > > > > > > > > So, no ack and no merge? > > > > > > > > > > Looking at the first patches + doc patch, the split is not meaningful. > > > > > You should merge doc and option parsing in the related patches. > > > > > For instance, parsing and doc of "tm" option should be in the "tm" > patch. > > > > > > > > I did not follow you request. Are you stating, for each > > > > functionality I should > > > be updating document rather than 1 document update after adding the > > > new functions? If former is true I am not able to find such > > > reasoning stated in guideline or documentation or from the maintainer. > > > > > > Yes, you should update the doc while adding a new feature. > > Ok, I will comply to your requirement even though it is not in 'guideline, > documentation or from maintainer'. Humbly requesting to update > documentation and guideline suggesting the same. This will also help others > to submit patches according the new guideline. Once reflected it will be > justified for sending a v8. > > Vipin, please read the doc carefully: > http://git.dpdk.org/dpdk/commit/?id=9e0e4a00df775 Thank you Thomas for this update, I will make the changes for v8 and wait for your ACK. > > > > But most importantly, there is no reason to do a patch adding some > > > empty functions and filling them later. > > Following are the reasons for using stub function from v1 onwards till > > v7 1. Without the dummy function there are compiler warnings for unused > variables. > > 2. It is logical to have stub functions for the new parse option being added in > one go. > > > > These are based on the suggestion from the maintainer. > > > > > And please consider the option parsing is part of the feature. > > As mentioned above please find the reasoning stated for patches from v1 to > v7. > > You keep thinking that parsing should be introduced separately. > I keep saying it is part of the feature. > >