[dpdk-dev] librte_log: add function to retrieve log_level

Message ID 1410683686-12389-1-git-send-email-mhall@mhcomputing.net (mailing list archive)
State Accepted, archived
Headers

Commit Message

Matthew Hall Sept. 14, 2014, 8:34 a.m. UTC
Signed-off-by: Matthew Hall <mhall@mhcomputing.net>
---
 lib/librte_eal/common/eal_common_log.c  | 7 +++++++
 lib/librte_eal/common/include/rte_log.h | 6 ++++++
 2 files changed, 13 insertions(+)
  

Comments

Matthew Hall Sept. 14, 2014, 8:36 a.m. UTC | #1
On Sun, Sep 14, 2014 at 01:34:46AM -0700, Matthew Hall wrote:
> Signed-off-by: Matthew Hall <mhall@mhcomputing.net>
> ---
>  lib/librte_eal/common/eal_common_log.c  | 7 +++++++
>  lib/librte_eal/common/include/rte_log.h | 6 ++++++
>  2 files changed, 13 insertions(+)

I forgot to mention in the comments, this patch is helpful when you want 
outside code to cooperate with and respect log levels set in DPDK. Then you 
can avoid using duplicate incompatible log code in the DPDK and non-DPDK parts 
of the app.

Matthew.
  
Bruce Richardson Sept. 15, 2014, 8:14 a.m. UTC | #2
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Matthew Hall
> Sent: Sunday, September 14, 2014 9:35 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] librte_log: add function to retrieve log_level
> 
> Signed-off-by: Matthew Hall <mhall@mhcomputing.net>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/librte_eal/common/eal_common_log.c  | 7 +++++++
>  lib/librte_eal/common/include/rte_log.h | 6 ++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/lib/librte_eal/common/eal_common_log.c
> b/lib/librte_eal/common/eal_common_log.c
> index e4df0b9..d979f28 100644
> --- a/lib/librte_eal/common/eal_common_log.c
> +++ b/lib/librte_eal/common/eal_common_log.c
> @@ -176,6 +176,13 @@ rte_set_log_level(uint32_t level)
>  	rte_logs.level = (uint32_t)level;
>  }
> 
> +/* Get global log level */
> +uint32_t
> +rte_get_log_level()
> +{
> +	return rte_logs.level;
> +}
> +
>  /* Set global log type */
>  void
>  rte_set_log_type(uint32_t type, int enable)
> diff --git a/lib/librte_eal/common/include/rte_log.h
> b/lib/librte_eal/common/include/rte_log.h
> index 565415a..7f1c2f9 100644
> --- a/lib/librte_eal/common/include/rte_log.h
> +++ b/lib/librte_eal/common/include/rte_log.h
> @@ -130,6 +130,12 @@ int rte_openlog_stream(FILE *f);
>  void rte_set_log_level(uint32_t level);
> 
>  /**
> + * Get the global log level.
> + *
> + */
> +uint32_t rte_get_log_level(void);
> +
> +/**
>   * Enable or disable the log type.
>   *
>   * @param type
> --
> 1.9.1
  
Matthew Hall Sept. 15, 2014, 8:16 a.m. UTC | #3
Thanks for the ack Bruce! Used this one to clean up a lot of grubby app-side code and I hate forking open source for too long if it can be avoided.

Matthew.
  
Bruce Richardson Sept. 15, 2014, 8:20 a.m. UTC | #4
> -----Original Message-----

> From: Matthew Hall [mailto:mhall@mhcomputing.net]

> Sent: Monday, September 15, 2014 9:17 AM

> To: Richardson, Bruce; dev@dpdk.org

> Subject: RE: [dpdk-dev] [PATCH] librte_log: add function to retrieve log_level

> 

> Thanks for the ack Bruce! Used this one to clean up a lot of grubby app-side

> code and I hate forking open source for too long if it can be avoided.


No problem. With such huge patches as this the code review takes many hours of strenuous effort! :-)

/Bruce
> 

> Matthew.

> --

> Sent from my mobile device.

> 

> On September 15, 2014 1:14:57 AM PDT, "Richardson, Bruce"

> <bruce.richardson@intel.com> wrote:

> >> -----Original Message-----

> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Matthew Hall

> >> Sent: Sunday, September 14, 2014 9:35 AM

> >> To: dev@dpdk.org

> >> Subject: [dpdk-dev] [PATCH] librte_log: add function to retrieve

> >log_level

> >>

> >> Signed-off-by: Matthew Hall <mhall@mhcomputing.net>

> >Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> >

> >> ---

> >>  lib/librte_eal/common/eal_common_log.c  | 7 +++++++

> >>  lib/librte_eal/common/include/rte_log.h | 6 ++++++

> >>  2 files changed, 13 insertions(+)

> >>

> >> diff --git a/lib/librte_eal/common/eal_common_log.c

> >> b/lib/librte_eal/common/eal_common_log.c

> >> index e4df0b9..d979f28 100644

> >> --- a/lib/librte_eal/common/eal_common_log.c

> >> +++ b/lib/librte_eal/common/eal_common_log.c

> >> @@ -176,6 +176,13 @@ rte_set_log_level(uint32_t level)

> >>  	rte_logs.level = (uint32_t)level;

> >>  }

> >>

> >> +/* Get global log level */

> >> +uint32_t

> >> +rte_get_log_level()

> >> +{

> >> +	return rte_logs.level;

> >> +}

> >> +

> >>  /* Set global log type */

> >>  void

> >>  rte_set_log_type(uint32_t type, int enable)

> >> diff --git a/lib/librte_eal/common/include/rte_log.h

> >> b/lib/librte_eal/common/include/rte_log.h

> >> index 565415a..7f1c2f9 100644

> >> --- a/lib/librte_eal/common/include/rte_log.h

> >> +++ b/lib/librte_eal/common/include/rte_log.h

> >> @@ -130,6 +130,12 @@ int rte_openlog_stream(FILE *f);

> >>  void rte_set_log_level(uint32_t level);

> >>

> >>  /**

> >> + * Get the global log level.

> >> + *

> >> + */

> >> +uint32_t rte_get_log_level(void);

> >> +

> >> +/**

> >>   * Enable or disable the log type.

> >>   *

> >>   * @param type

> >> --

> >> 1.9.1
  
Matthew Hall Sept. 15, 2014, 8:24 a.m. UTC | #5
The real effort was the client-side cleanup. I had to get rid of pages of logs for every packet flowing through and I hate yanking out logs... but some of them call weird functions like memdump and pktmbuf_dump. There's no good way to clean those up without a function to check the current loglevel. And reaching into private structs to get it seemed like an uncivilized thing to do.

Good news is, another couple weeks and some Coverity patches and I'll have shareable code to hand out.
  
Thomas Monjalon Sept. 18, 2014, 1:29 p.m. UTC | #6
> > Signed-off-by: Matthew Hall <mhall@mhcomputing.net>
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> > +/* Get global log level */
> > +uint32_t
> > +rte_get_log_level()

void is missing here

Applied with fix and commit log which was sent later.

Thanks
  
Matthew Hall Sept. 18, 2014, 8:25 p.m. UTC | #7
On Thu, Sep 18, 2014 at 03:29:11PM +0200, Thomas Monjalon wrote:
> void is missing here

Ah yes, sorry for missing it.

Thanks for correcting and applying, glad to have it available upstream.

Matthew.
  

Patch

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index e4df0b9..d979f28 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -176,6 +176,13 @@  rte_set_log_level(uint32_t level)
 	rte_logs.level = (uint32_t)level;
 }
 
+/* Get global log level */
+uint32_t
+rte_get_log_level()
+{
+	return rte_logs.level;
+}
+
 /* Set global log type */
 void
 rte_set_log_type(uint32_t type, int enable)
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 565415a..7f1c2f9 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -130,6 +130,12 @@  int rte_openlog_stream(FILE *f);
 void rte_set_log_level(uint32_t level);
 
 /**
+ * Get the global log level.
+ *
+ */
+uint32_t rte_get_log_level(void);
+
+/**
  * Enable or disable the log type.
  *
  * @param type