[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

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(-)
  

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.
> 
>