diff mbox series

[v8,1/7] app/procinfo: prepare for new debug functions

Message ID 20190107153829.34047-2-vipin.varghese@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series app/proc-info: enhance debug of proc-info tool |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Varghese, Vipin Jan. 7, 2019, 3:38 p.m. UTC 
  Update code base and meson build file to accomadate changes for the
new functionality.

Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
Acked-by: Reshma  Pattan <reshma.pattan@intel.com>
---
 app/proc-info/main.c      | 13 +++++++++++++
 app/proc-info/meson.build |  2 +-
 2 files changed, 14 insertions(+), 1 deletion(-)

Comments

Thomas Monjalon Jan. 9, 2019, 11:45 p.m. UTC | #1 
Hi Vipin,

The code split in this v8 looks really better,
except this patch which makes no sense alone.
I feel you can move these changes in next patches,
where appropriate, isn't it?

07/01/2019 16:38, Vipin Varghese:
> Update code base and meson build file to accomadate changes for the
> new functionality.
> 
> Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> Acked-by: Reshma  Pattan <reshma.pattan@intel.com>
> ---
>  app/proc-info/main.c      | 13 +++++++++++++
>  app/proc-info/meson.build |  2 +-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
> index c20effa4f..c7697389c 100644
> --- a/app/proc-info/main.c
> +++ b/app/proc-info/main.c
> @@ -29,6 +29,9 @@
>  #include <rte_branch_prediction.h>
>  #include <rte_string_fns.h>
>  #include <rte_metrics.h>
> +#include <rte_cycles.h>
> +#include <rte_security.h>
> +#include <rte_cryptodev.h>
>  
>  /* Maximum long option length for option parsing. */
>  #define MAX_LONG_OPT_SZ 64
> @@ -36,6 +39,10 @@
>  
>  #define MAX_STRING_LEN 256
>  
> +#define STATS_BDR_FMT "========================================"
> +#define STATS_BDR_STR(w, s) printf("%.*s%s%.*s\n", w, \
> +	STATS_BDR_FMT, s, w, STATS_BDR_FMT)
> +
>  /**< mask of enabled ports */
>  static uint32_t enabled_port_mask;
>  /**< Enable stats. */
> @@ -65,6 +72,9 @@ static char *xstats_name;
>  static uint32_t nb_xstats_ids;
>  static uint64_t xstats_ids[MAX_NB_XSTATS_IDS];
>  
> +/* show border */
> +static char bdr_str[MAX_STRING_LEN];
> +
>  /**< display usage */
>  static void
>  proc_info_usage(const char *prgname)
> @@ -668,5 +678,8 @@ main(int argc, char **argv)
>  	if (ret)
>  		printf("Error from rte_eal_cleanup(), %d\n", ret);
>  
> +	snprintf(bdr_str, MAX_STRING_LEN, " ");
> +	STATS_BDR_STR(50, bdr_str);
> +
>  	return 0;
>  }
> diff --git a/app/proc-info/meson.build b/app/proc-info/meson.build
> index a52b2ee4a..866b390d6 100644
> --- a/app/proc-info/meson.build
> +++ b/app/proc-info/meson.build
> @@ -3,4 +3,4 @@
>  
>  sources = files('main.c')
>  allow_experimental_apis = true
> -deps += ['ethdev', 'metrics']
> +deps += ['ethdev', 'metrics', 'security']
>
Varghese, Vipin Jan. 10, 2019, 3:08 a.m. UTC | #2 
snipped
> 
> Hi Vipin,
> 
> The code split in this v8 looks really better, except this patch which makes no
> sense alone.
Hi Thomas, I need this base patch as first one. These include the helper MACRO and include which pans out for next 7 patches.

> I feel you can move these changes in next patches, where appropriate, isn't it?
Patches v2 to v7 uses common features from v1. 

> 
> 07/01/2019 16:38, Vipin Varghese:
> > Update code base and meson build file to accomadate changes for the
> > new functionality.
> >
> > Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> > Acked-by: Reshma  Pattan <reshma.pattan@intel.com>
> > ---
> >  app/proc-info/main.c      | 13 +++++++++++++
> >  app/proc-info/meson.build |  2 +-
> >  2 files changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/proc-info/main.c b/app/proc-info/main.c index
> > c20effa4f..c7697389c 100644
> > --- a/app/proc-info/main.c
> > +++ b/app/proc-info/main.c
> > @@ -29,6 +29,9 @@
> >  #include <rte_branch_prediction.h>
> >  #include <rte_string_fns.h>
> >  #include <rte_metrics.h>
> > +#include <rte_cycles.h>
> > +#include <rte_security.h>
> > +#include <rte_cryptodev.h>
> >
> >  /* Maximum long option length for option parsing. */  #define
> > MAX_LONG_OPT_SZ 64 @@ -36,6 +39,10 @@
> >
> >  #define MAX_STRING_LEN 256
> >
> > +#define STATS_BDR_FMT
> "========================================"
> > +#define STATS_BDR_STR(w, s) printf("%.*s%s%.*s\n", w, \
> > +	STATS_BDR_FMT, s, w, STATS_BDR_FMT)
> > +
> >  /**< mask of enabled ports */
> >  static uint32_t enabled_port_mask;
> >  /**< Enable stats. */
> > @@ -65,6 +72,9 @@ static char *xstats_name;  static uint32_t
> > nb_xstats_ids;  static uint64_t xstats_ids[MAX_NB_XSTATS_IDS];
> >
> > +/* show border */
> > +static char bdr_str[MAX_STRING_LEN];
> > +
> >  /**< display usage */
> >  static void
> >  proc_info_usage(const char *prgname)
> > @@ -668,5 +678,8 @@ main(int argc, char **argv)
> >  	if (ret)
> >  		printf("Error from rte_eal_cleanup(), %d\n", ret);
> >
> > +	snprintf(bdr_str, MAX_STRING_LEN, " ");
> > +	STATS_BDR_STR(50, bdr_str);
> > +
> >  	return 0;
> >  }
> > diff --git a/app/proc-info/meson.build b/app/proc-info/meson.build
> > index a52b2ee4a..866b390d6 100644
> > --- a/app/proc-info/meson.build
> > +++ b/app/proc-info/meson.build
> > @@ -3,4 +3,4 @@
> >
> >  sources = files('main.c')
> >  allow_experimental_apis = true
> > -deps += ['ethdev', 'metrics']
> > +deps += ['ethdev', 'metrics', 'security']
> >
> 
> 
> 
>
Thomas Monjalon Jan. 10, 2019, 8:39 a.m. UTC | #3 
10/01/2019 04:08, Varghese, Vipin:
> snipped
> > 
> > Hi Vipin,
> > 
> > The code split in this v8 looks really better, except this patch which makes no
> > sense alone.
> Hi Thomas, I need this base patch as first one. These include the helper MACRO and include which pans out for next 7 patches.
> 
> > I feel you can move these changes in next patches, where appropriate, isn't it?
> Patches v2 to v7 uses common features from v1. 

At least, you should introduce the includes when needed.
If you need this patch for introducing some macros,
then rename it and explain the macros in the message.


> > 07/01/2019 16:38, Vipin Varghese:
> > > Update code base and meson build file to accomadate changes for the
> > > new functionality.
> > >
> > > Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> > > Acked-by: Reshma  Pattan <reshma.pattan@intel.com>
> > > ---
> > >  app/proc-info/main.c      | 13 +++++++++++++
> > >  app/proc-info/meson.build |  2 +-
> > >  2 files changed, 14 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/app/proc-info/main.c b/app/proc-info/main.c index
> > > c20effa4f..c7697389c 100644
> > > --- a/app/proc-info/main.c
> > > +++ b/app/proc-info/main.c
> > > @@ -29,6 +29,9 @@
> > >  #include <rte_branch_prediction.h>
> > >  #include <rte_string_fns.h>
> > >  #include <rte_metrics.h>
> > > +#include <rte_cycles.h>
> > > +#include <rte_security.h>
> > > +#include <rte_cryptodev.h>
> > >
> > >  /* Maximum long option length for option parsing. */  #define
> > > MAX_LONG_OPT_SZ 64 @@ -36,6 +39,10 @@
> > >
> > >  #define MAX_STRING_LEN 256
> > >
> > > +#define STATS_BDR_FMT
> > "========================================"
> > > +#define STATS_BDR_STR(w, s) printf("%.*s%s%.*s\n", w, \
> > > +	STATS_BDR_FMT, s, w, STATS_BDR_FMT)
> > > +
> > >  /**< mask of enabled ports */
> > >  static uint32_t enabled_port_mask;
> > >  /**< Enable stats. */
> > > @@ -65,6 +72,9 @@ static char *xstats_name;  static uint32_t
> > > nb_xstats_ids;  static uint64_t xstats_ids[MAX_NB_XSTATS_IDS];
> > >
> > > +/* show border */
> > > +static char bdr_str[MAX_STRING_LEN];
> > > +
> > >  /**< display usage */
> > >  static void
> > >  proc_info_usage(const char *prgname)
> > > @@ -668,5 +678,8 @@ main(int argc, char **argv)
> > >  	if (ret)
> > >  		printf("Error from rte_eal_cleanup(), %d\n", ret);
> > >
> > > +	snprintf(bdr_str, MAX_STRING_LEN, " ");
> > > +	STATS_BDR_STR(50, bdr_str);
> > > +
> > >  	return 0;
> > >  }
> > > diff --git a/app/proc-info/meson.build b/app/proc-info/meson.build
> > > index a52b2ee4a..866b390d6 100644
> > > --- a/app/proc-info/meson.build
> > > +++ b/app/proc-info/meson.build
> > > @@ -3,4 +3,4 @@
> > >
> > >  sources = files('main.c')
> > >  allow_experimental_apis = true
> > > -deps += ['ethdev', 'metrics']
> > > +deps += ['ethdev', 'metrics', 'security']
Varghese, Vipin Jan. 10, 2019, 8:53 a.m. UTC | #4 
Hi Thomas,

snipped
> 
> 10/01/2019 04:08, Varghese, Vipin:
> > snipped
> > >
> > > Hi Vipin,
> > >
> > > The code split in this v8 looks really better, except this patch
> > > which makes no sense alone.
> > Hi Thomas, I need this base patch as first one. These include the helper
> MACRO and include which pans out for next 7 patches.
> >
> > > I feel you can move these changes in next patches, where appropriate, isn't
> it?
> > Patches v2 to v7 uses common features from v1.
> 
> At least, you should introduce the includes when needed.
As mentioned earlier these are used in v2 to v7.

> If you need this patch for introducing some macros, then rename it and
> explain the macros in the message.
The MACRO introduced in this patch are 'MAX_STRING_LEN, STATS_BDR_FMT and STATS_BDR_STR'. As per code contribution guideline I may have missed out the stipulation requesting for explaining the same.

> 
> 
> > > 07/01/2019 16:38, Vipin Varghese:
> > > > Update code base and meson build file to accommodate changes for
> > > > the new functionality.
> > > >
> > > > Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> > > > Acked-by: Reshma  Pattan <reshma.pattan@intel.com>
> > > > ---
snipped
Thomas Monjalon Jan. 10, 2019, 9:09 a.m. UTC | #5 
10/01/2019 09:53, Varghese, Vipin:
> Hi Thomas,
> 
> snipped
> > 
> > 10/01/2019 04:08, Varghese, Vipin:
> > > snipped
> > > >
> > > > Hi Vipin,
> > > >
> > > > The code split in this v8 looks really better, except this patch
> > > > which makes no sense alone.
> > > Hi Thomas, I need this base patch as first one. These include the helper
> > MACRO and include which pans out for next 7 patches.
> > >
> > > > I feel you can move these changes in next patches, where appropriate, isn't
> > it?
> > > Patches v2 to v7 uses common features from v1.
> > 
> > At least, you should introduce the includes when needed.
> As mentioned earlier these are used in v2 to v7.

So?
They must be introduced first time they are used.
Vipin, please be cooperative.

> > If you need this patch for introducing some macros, then rename it and
> > explain the macros in the message.
> The MACRO introduced in this patch are 'MAX_STRING_LEN, STATS_BDR_FMT and STATS_BDR_STR'. As per code contribution guideline I may have missed out the stipulation requesting for explaining the same.
> 
> > 
> > 
> > > > 07/01/2019 16:38, Vipin Varghese:
> > > > > Update code base and meson build file to accommodate changes for
> > > > > the new functionality.
> > > > >
> > > > > Signed-off-by: Vipin Varghese <vipin.varghese@intel.com>
> > > > > Acked-by: Reshma  Pattan <reshma.pattan@intel.com>
> > > > > ---
> snipped
>
diff mbox series

Patch

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index c20effa4f..c7697389c 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -29,6 +29,9 @@ 
 #include <rte_branch_prediction.h>
 #include <rte_string_fns.h>
 #include <rte_metrics.h>
+#include <rte_cycles.h>
+#include <rte_security.h>
+#include <rte_cryptodev.h>
 
 /* Maximum long option length for option parsing. */
 #define MAX_LONG_OPT_SZ 64
@@ -36,6 +39,10 @@ 
 
 #define MAX_STRING_LEN 256
 
+#define STATS_BDR_FMT "========================================"
+#define STATS_BDR_STR(w, s) printf("%.*s%s%.*s\n", w, \
+	STATS_BDR_FMT, s, w, STATS_BDR_FMT)
+
 /**< mask of enabled ports */
 static uint32_t enabled_port_mask;
 /**< Enable stats. */
@@ -65,6 +72,9 @@  static char *xstats_name;
 static uint32_t nb_xstats_ids;
 static uint64_t xstats_ids[MAX_NB_XSTATS_IDS];
 
+/* show border */
+static char bdr_str[MAX_STRING_LEN];
+
 /**< display usage */
 static void
 proc_info_usage(const char *prgname)
@@ -668,5 +678,8 @@  main(int argc, char **argv)
 	if (ret)
 		printf("Error from rte_eal_cleanup(), %d\n", ret);
 
+	snprintf(bdr_str, MAX_STRING_LEN, " ");
+	STATS_BDR_STR(50, bdr_str);
+
 	return 0;
 }
diff --git a/app/proc-info/meson.build b/app/proc-info/meson.build
index a52b2ee4a..866b390d6 100644
--- a/app/proc-info/meson.build
+++ b/app/proc-info/meson.build
@@ -3,4 +3,4 @@ 
 
 sources = files('main.c')
 allow_experimental_apis = true
-deps += ['ethdev', 'metrics']
+deps += ['ethdev', 'metrics', 'security']