On 4/13/2017 12:03 PM, Lu, Wenzhuo wrote: > Hi Ferruh, > > -----Original Message----- > From: Yigit, Ferruh > Sent: Thursday, April 13, 2017 5:14 PM > To: Richardson, Bruce <bruce.richard...@intel.com>; Lu, Wenzhuo > <wenzhuo...@intel.com> > Cc: Zhang, Helin <helin.zh...@intel.com>; Olivier Matz > <olivier.m...@6wind.com>; dev@dpdk.org; Wu, Jingjing <jingjing...@intel.com> > Subject: Re: [dpdk-dev] [PATCH] net/i40e: disable init and driver logs by > default > > On 4/12/2017 11:02 AM, Bruce Richardson wrote: >> On Wed, Apr 12, 2017 at 04:12:18AM +0100, Lu, Wenzhuo wrote: >>> Hi, >>> >>> >>>> -----Original Message----- >>>> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Zhang, Helin >>>> Sent: Friday, April 7, 2017 10:03 AM >>>> To: Richardson, Bruce >>>> Cc: Olivier Matz; dev@dpdk.org; Wu, Jingjing; Yigit, Ferruh >>>> Subject: Re: [dpdk-dev] [PATCH] net/i40e: disable init and driver >>>> logs by default >>>> >>>> >>>> >>>>> -----Original Message----- >>>>> From: Richardson, Bruce >>>>> Sent: Thursday, April 6, 2017 10:37 PM >>>>> To: Zhang, Helin >>>>> Cc: Olivier Matz; dev@dpdk.org; Wu, Jingjing; Yigit, Ferruh >>>>> Subject: Re: [PATCH] net/i40e: disable init and driver logs by >>>>> default >>>>> >>>>> On Thu, Apr 06, 2017 at 03:31:23PM +0100, Zhang, Helin wrote: >>>>>> >>>>>> >>>>>> -----Original Message----- >>>>>> From: Olivier Matz [mailto:olivier.m...@6wind.com] >>>>>> Sent: Thursday, April 6, 2017 10:17 PM >>>>>> To: dev@dpdk.org >>>>>> Cc: Zhang, Helin <helin.zh...@intel.com>; Wu, Jingjing >>>>>> <jingjing...@intel.com>; Richardson, Bruce >>>>>> <bruce.richard...@intel.com>; Yigit, Ferruh >>>>>> <ferruh.yi...@intel.com> >>>>>> Subject: [PATCH] net/i40e: disable init and driver logs by default >>>>>> >>>>>> Since "net/i40e: use dynamic log type for control logs", the i40e >>>>>> driver is >>>>> more verbose by default, which could result in testpmd being >>>>> flooded by log messages in some conditions: >>>>>> >>>>>> Checking link statuses... >>>>>> i40e_dev_handle_aq_msg(): Request 2561 is not supported yet >>>>>> i40e_dev_handle_aq_msg(): Request 2561 is not supported yet >>>>>> i40e_dev_handle_aq_msg(): Request 2561 is not supported yet >>>>>> i40e_dev_handle_aq_msg(): Request 2561 is not supported yet >>>>>> Port 0 Link Up - speed 40000 Mbps - full-duplex >>>>>> Port 1 Link Up - speed 40000 Mbps - full-duplex >>>>>> Done >>>>>> testpmd> i40e_dev_handle_aq_msg(): Request 4097 is not supported >>>> yet >>>>>> i40e_dev_handle_aq_msg(): Request 4097 is not supported yet >>>>>> i40e_dev_handle_aq_msg(): Request 4097 is not supported yet >>>>>> >>>>>> Fix this by disabling the dynamic logs by default. It is still >>>>>> possible to enable >>>>> them at runtime. >>>>>> >>>>>> Fixes: c143e5a3d9e1 ("net/i40e: use dynamic log type for control >>>>>> logs") >>>>>> >>>>>> Signed-off-by: Olivier Matz <olivier.m...@6wind.com> >>>>>> Acked-by: Helin Zhang <helin.zh...@intel.com> > > <...> > >>>>> >>>>> Hi Helin, >>>>> >>>>> Is this the correct fix? IMHO, if this is a problem, then we should >>>>> surely not be hiding and ignoring the error. If it's not a problem, >>>>> then the log level should be reduced to a lower level, e.g. INFO or >>>>> NOTICE. >>>>> >>>>> Alternatively, this code could be modified to only print an error >>>>> once for each unsupported request type. >>>>> >>>>> Ideally both solutions should be used, I think. I'm not sure I like >>>>> setting the default log level to just show EMERG messages. >>>>> >>>>> Regards, >>>>> /Bruce >>>> Bruce, this is not a fix for the issue, which is under investigation >>>> by developers now. >>>> >>>> For the by default log level, any guideline for that? >>>> I was OK with that changes, but not sure if there is any better choices. >>> >>> To my opinion, it's not appropriate to say something not supported is >>> an error. How about fix this problem like, >>> >>> diff --git a/drivers/net/i40e/i40e_ethdev.c >>> b/drivers/net/i40e/i40e_ethdev.c index 6927fde..91df587 100644 >>> --- a/drivers/net/i40e/i40e_ethdev.c >>> +++ b/drivers/net/i40e/i40e_ethdev.c >>> @@ -5794,7 +5794,7 @@ struct i40e_vsi * >>> } >>> break; >>> default: >>> - PMD_DRV_LOG(ERR, "Request %u is not supported yet", >>> + PMD_DRV_LOG(DEBUG, "Request %u is not >>> + supported yet", >>> opcode); >>> break; >>> } >> >> Ok to me. > >> +1. > >> Wenzhuo would you mind sending this as patch? > Sure. Will do.
Sent [1], this patch marked as superseded. Thanks, ferruh [1] http://dpdk.org/dev/patchwork/patch/23635/
