From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH 5/5] sfc: don't use RTE_LOGTYPE_PMD Date: Wed, 27 Feb 2019 12:19:32 +0000 Message-ID: <9f01db85-b80c-fa2d-25cd-d3cb6d139812@intel.com> References: <20190226213424.10567-1-stephen@networkplumber.org> <20190226213424.10567-6-stephen@networkplumber.org> <585db3f2-d9e6-1efd-b05f-7050758423a6@intel.com> <7766ed09-9807-2c81-ec21-9b225bfeea7a@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Igor Romanov , Olivier MATZ To: Andrew Rybchenko , Stephen Hemminger , dev@dpdk.org Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 514424C96 for ; Wed, 27 Feb 2019 13:19:36 +0100 (CET) In-Reply-To: <7766ed09-9807-2c81-ec21-9b225bfeea7a@intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 2/27/2019 11:50 AM, Ferruh Yigit wrote: > On 2/27/2019 11:24 AM, Andrew Rybchenko wrote: >> On 2/27/19 2:21 PM, Ferruh Yigit wrote: >>> On 2/26/2019 9:34 PM, Stephen Hemminger wrote: >>>> The sfc driver was still using RTE_LOGTYPE_PMD which was superseded >>>> by local logging. >>>> >>>> Signed-off-by: Stephen Hemminger >>>> --- >>>> drivers/net/sfc/sfc.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c >>>> index 898603884fa0..2cd7126015fd 100644 >>>> --- a/drivers/net/sfc/sfc.c >>>> +++ b/drivers/net/sfc/sfc.c >>>> @@ -1115,12 +1115,12 @@ sfc_register_logtype(const struct rte_pci_addr *pci_addr, >>>> ++lt_prefix_str_size; /* Reserve space for prefix separator */ >>>> lt_str_size_max = lt_prefix_str_size + PCI_PRI_STR_SIZE + 1; >>>> } else { >>>> - return RTE_LOGTYPE_PMD; >>>> + return sfc_logtype_driver; >>>> } >>>> >>>> lt_str = rte_zmalloc("logtype_str", lt_str_size_max, 0); >>>> if (lt_str == NULL) >>>> - return RTE_LOGTYPE_PMD; >>>> + return sfc_logtype_driver; >>>> >>>> strncpy(lt_str, lt_prefix_str, lt_prefix_str_size); >>>> lt_str[lt_prefix_str_size - 1] = '.'; >>>> >>> Overall I think it is good idea to remove RTE_LOGTYPE_PMD, but sfc has a few >>> more usage of it around same manner, as a fallback value if allocating dynamic >>> one fails. >>> >>> >>> Andrew, >>> >>> Can be possible to update this sfc patch to completely eliminate RTE_LOGTYPE_PMD >>> usage? What do you think? >>> >>> Thanks, >>> ferruh >> >> I'm OK to use  sfc_logtype_driverif dynamic log type register fails, but >> what should I do if sfc_logtype_driverregister fails? > > Right. Same concern is valid for all dynamic log registration but we are > ignoring it. > > What do you think instead of each pmd/lib cover the error case, > 'rte_log_register_type_and_pick_level()' & 'rte_log_register()' always return > valid value. Like if they fail internally, return 'RTE_LOGTYPE_GENERIC' kind of > value. Changing the log functions is one above step, are you OK to replace all occurrence of 'RTE_LOGTYPE_PMD' with 'sfc_logtype_driver' in 'sfc_register_logtype()', there is one more occurrence than this patch updates. I see your point to keep the one in 'sfc_driver_register_logtype()', perhaps we can address it later separately with above suggestion or something else.