From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f43.google.com (mail-pa0-f43.google.com [209.85.220.43]) by dpdk.org (Postfix) with ESMTP id 4EC688025 for ; Thu, 31 Mar 2016 23:51:35 +0200 (CEST) Received: by mail-pa0-f43.google.com with SMTP id zm5so74935036pac.0 for ; Thu, 31 Mar 2016 14:51:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=l1vEuH5JQKHyTGt7rm0SXwqChLEX2bqefgWVDQKL9+4=; b=Imz+ACFSyrfFYxtkuMavLbDAIblj0tD0Awcwp0INEs7PksgGrFzEPHN+ebvPdsUHjt BQ4SZNPxGle6vBdinStHgqbMWVcUqqADx2QTPHPh1hN34ruX8MQMDErJTe/rDzkyf00K Az8r2IXUwqD1sF4pdVexP/usyG7MMf8jq/c/HsdEdhWvUj2/PR6Wx/yeyNG58q97nIeD CM45syKdkX1b+4cTtlYwCA5s73Vk7H95zy01EtntD8l7J96LAz1bGUwzskIdOzgkw6Xu c5FjaYJ40vcGipuxQaC+rTwhi7tFyph4h37v0ipUirydsmSqtyFksxYqx/lAAOZT3aQa sSBQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=l1vEuH5JQKHyTGt7rm0SXwqChLEX2bqefgWVDQKL9+4=; b=RTx2zFWbokHTNakD2k08DEH55YIa+vM3bKxbH61Mz+cGRvxB1TbSSMX+88wIFxL2/q iPW/6GYaB2UNNLAFKx8lWAvXchwXJI3BCAAjkqfFZUZz73gEEzi9rmguE+DQT7C3sk4+ TPeDLXsTYGsWa3yIQUtg9vzv8Au2B3BejvzX/IK+uXJTPs6U4v+NN/rhgdN0hgatUW7m 89GXyXecA/2bK8Uqm3C5DUSewU68MzLxf3KEQ89lI6kZThhX6TxjVgCSqXDEv+loiKds JVcIZUzSp3RDlnbcmID4qCXmboeX/QJ6a5J+pbKAMSXRnvH3r68P86E8vVFEX6qXj0aB 2ZLg== X-Gm-Message-State: AD7BkJKR+VvQF1tOx5qMCwRcwSuruU91SQHkUP6DArazPccrNEYyvJh+Uza8U4UcyJQ4WQ== X-Received: by 10.66.139.137 with SMTP id qy9mr25447021pab.57.1459461094696; Thu, 31 Mar 2016 14:51:34 -0700 (PDT) Received: from xeon-e3 (static-50-53-73-178.bvtn.or.frontiernet.net. [50.53.73.178]) by smtp.gmail.com with ESMTPSA id s197sm15586336pfs.62.2016.03.31.14.51.34 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 31 Mar 2016 14:51:34 -0700 (PDT) Date: Thu, 31 Mar 2016 14:51:49 -0700 From: Stephen Hemminger To: Harish Patil Cc: "bruce.richardson@intel.com" , "thomas.monjalon@6wind.com" , "dev@dpdk.org" , Rasesh Mody Message-ID: <20160331145149.58e750c8@xeon-e3> In-Reply-To: References: <1459315705-25001-1-git-send-email-rasesh.mody@qlogic.com> <1459315705-25001-6-git-send-email-rasesh.mody@qlogic.com> <20160330093926.6bff277c@xeon-e3> <20160330153743.63605112@xeon-e3> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v4 05/10] qede: Add core driver X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 31 Mar 2016 21:51:35 -0000 On Thu, 31 Mar 2016 19:36:55 +0000 Harish Patil wrote: > > >=20 > >On Wed, 30 Mar 2016 22:16:51 +0000 > >Harish Patil wrote: > > > >> > > >> >On Tue, 29 Mar 2016 22:28:20 -0700 > >> >Rasesh Mody wrote: > >> > > >> >> + > >> >> +static void qede_print_adapter_info(struct qede_dev *qdev) > >> >> +{ > >> >> + struct ecore_dev *edev =3D &qdev->edev; > >> >> + struct qed_dev_info *info =3D &qdev->dev_info.common; > >> >> + char ver_str[QED_DRV_VER_STR_SIZE] =3D { 0 }; > >> >> + > >> >> + RTE_LOG(INFO, PMD, > >> >> + " Chip details : %s%d\n", > >> >> + ECORE_IS_BB(edev) ? "BB" : "AH", > >> >> + CHIP_REV_IS_A0(edev) ? 0 : 1); > >> >> + > >> >> + sprintf(ver_str, "%s %s_%d.%d.%d.%d", QEDE_PMD_VER_PREFIX, > >> >> + edev->ver_str, QEDE_PMD_VERSION_MAJOR, QEDE_PMD_VERSION_MINOR, > >> >> + QEDE_PMD_VERSION_REVISION, QEDE_PMD_VERSION_PATCH); > >> >> + strcpy(qdev->drv_ver, ver_str); > >> >> + RTE_LOG(INFO, PMD, " Driver version : %s\n", ver_str); > >> >> + > >> >> + ver_str[0] =3D '\0'; > >> >> + sprintf(ver_str, "%d.%d.%d.%d", info->fw_major, info->fw_minor, > >> >> + info->fw_rev, info->fw_eng); > >> >> + RTE_LOG(INFO, PMD, " Firmware version : %s\n", ver_str); > >> >> + > >> >> + ver_str[0] =3D '\0'; > >> >> + sprintf(ver_str, "%d.%d.%d.%d", > >> >> + (info->mfw_rev >> 24) & 0xff, > >> >> + (info->mfw_rev >> 16) & 0xff, > >> >> + (info->mfw_rev >> 8) & 0xff, (info->mfw_rev) & 0xff); > >> >> + RTE_LOG(INFO, PMD, " Management firmware version : %s\n", ver_str= ); > >> >> + > >> >> + RTE_LOG(INFO, PMD, " Firmware file : %s\n", QEDE_FW_FILE_NAME); > >> > > >> >This means the driver is far too chatty in the logs. > >> >Can't this be made DEBUG level? > >> > > >> Not clear what is the issue here? > >> RTE_LOG is used here to display basic adapter info like firmware/driver > >> versions etc without the need to enable any debug flags. > >> The driver debug logging is under the control of appropriate debug > >>flags. > >>=20 > > > >The DPDK log messages end up being examined by tech support and customer= s. > >Too much code puts extra stuff in so it is hard to find the real problem. > >This is obviously debug info, so either: > > 1) make it conditionally compiled > > 2) change log level to DEBUG > > 3) remove it. > > >=20 > This is not really a debug msg per se. We want it to be printed on the > stdout unconditionally (displayed only once) so that we can readily know > what firmware/driver/hw versions we are dealing with. We don=E2=80=99t wa= nt to > depend on the apps to print those and many of the apps may not do so > including testpmd. Even the linux drivers prints basic stuff under dmesg. > We have found this to be useful for triage and would like to retain it if > there is no objection. And Linux drivers are wrong to do this. This a case where developer thinks = printing stuff is important, but for operations it isn't. Better to provide this info thro= ugh some API, (maybe DPDK needs extensions to get_info). Also, you don't have to format twice, once into a buffer (with sprintf) the= n again by calling RTE_LOG. Not only that sprintf() is guaranteed to null terminate t= he string, so your paranoid ver_str[0] =3D 0; is not needed either. Sorry for being so picky, but you are getting the feedback that comes from experience dealing with large volumes of log info.