DPDK patches and discussions
 help / color / mirror / Atom feed
From: Olivier MATZ <olivier.matz@6wind.com>
To: Pavan Nikhilesh Bhagavatula <pbhagavatula@caviumnetworks.com>
Cc: ferruh.yigit@intel.com, thomas@monjalon.net, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 3/3] logs: remove log level config option
Date: Fri, 8 Dec 2017 16:37:32 +0100	[thread overview]
Message-ID: <20171208153731.qx5wvuvocq5j7n3e@platinum> (raw)
In-Reply-To: <20171207140713.d6s5mh6ggncs3b3p@Pavan-LT>


On Thu, Dec 07, 2017 at 07:37:14PM +0530, Pavan Nikhilesh Bhagavatula wrote:
> Hi Oliver,
> 
> Thanks for the review.
> 
> On Thu, Dec 07, 2017 at 02:21:21PM +0100, Olivier MATZ wrote:
> > On Wed, Nov 22, 2017 at 02:58:06PM +0530, Pavan Nikhilesh wrote:
> > > --- a/test/test/test.h
> > > +++ b/test/test/test.h
> > > @@ -204,7 +204,7 @@ struct unit_test_case {
> > >
> > >  #define TEST_CASES_END() { NULL, NULL, NULL, NULL, 0 }
> > >
> > > -#if RTE_LOG_LEVEL >= RTE_LOG_DEBUG
> > > +#if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG
> > >  #define TEST_HEXDUMP(file, title, buf, len) rte_hexdump(file, title, buf, len)
> > >  #else
> > >  #define TEST_HEXDUMP(file, title, buf, len) do {} while (0)
> >
> > For drivers, it looks correct to replace RTE_LOG_LEVEL by
> > RTE_LOG_DP_LEVEL, from what I see it's about dataplane logs.
> >
> > For l3fwd, I'm less sure, but it could make sense too.
> >
> > For test, I think we should replace TEST_HEXDUMP() by a function
> > that checks the runtime log level instead of relying on RTE_LOG_DP_LEVEL.
> > I can submit a patch for this.
> 
> Agreed.

Please see:
http://dpdk.org/ml/archives/dev/2017-December/083295.html

By the way, if you do another version on top of this patchset,
I think you can also do the following replacement for consistency:

  --- a/lib/librte_flow_classify/rte_flow_classify.c
  +++ b/lib/librte_flow_classify/rte_flow_classify.c
  @@ -685,7 +685,7 @@ static void
   librte_flow_classify_init_log(void)
   {
          librte_flow_classify_logtype =
  -               rte_log_register("librte.flow_classify");
  +               rte_log_register("lib.flow_classify");
          if (librte_flow_classify_logtype >= 0)
                  rte_log_set_level(librte_flow_classify_logtype, RTE_LOG_INFO);
   }
  diff --git a/lib/librte_member/rte_member.c b/lib/librte_member/rte_member.c
  index cc9ea84ae..84354cc38 100644
  --- a/lib/librte_member/rte_member.c
  +++ b/lib/librte_member/rte_member.c
  @@ -330,7 +330,7 @@ RTE_INIT(librte_member_init_log);
   static void
   librte_member_init_log(void)
   {
  -       librte_member_logtype = rte_log_register("librte.member");
  +       librte_member_logtype = rte_log_register("lib.member");
          if (librte_member_logtype >= 0)
                  rte_log_set_level(librte_member_logtype, RTE_LOG_DEBUG);
   }


Thanks,
Olivier

  reply	other threads:[~2017-12-08 15:37 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-22  9:28 [dpdk-dev] [PATCH 1/3] eal: update legacy modules dynamic logs regex Pavan Nikhilesh
2017-11-22  9:28 ` [dpdk-dev] [PATCH 2/3] eal: update default log levels Pavan Nikhilesh
2017-12-07 13:15   ` Olivier MATZ
2017-11-22  9:28 ` [dpdk-dev] [PATCH 3/3] logs: remove log level config option Pavan Nikhilesh
2017-12-07 13:21   ` Olivier MATZ
2017-12-07 14:07     ` Pavan Nikhilesh Bhagavatula
2017-12-08 15:37       ` Olivier MATZ [this message]
2017-11-22 22:57 ` [dpdk-dev] [PATCH 1/3] eal: update legacy modules dynamic logs regex Ferruh Yigit
2017-12-07 13:14 ` Olivier MATZ
2017-12-09 13:23 ` [dpdk-dev] [PATCH v2 " Pavan Nikhilesh
2017-12-09 13:23   ` [dpdk-dev] [PATCH v2 2/3] eal: update default log levels Pavan Nikhilesh
2017-12-09 13:23   ` [dpdk-dev] [PATCH v2 3/3] logs: remove log level config option Pavan Nikhilesh
2017-12-12 16:36     ` Olivier MATZ
2018-01-12 15:46       ` Thomas Monjalon
2018-01-12 15:52         ` Pavan Nikhilesh
2018-01-12 16:08           ` Thomas Monjalon
2018-01-17 13:22       ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171208153731.qx5wvuvocq5j7n3e@platinum \
    --to=olivier.matz@6wind.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=pbhagavatula@caviumnetworks.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).