From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 22A12A2E1B for ; Wed, 4 Sep 2019 19:46:01 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2B7E21BEBF; Wed, 4 Sep 2019 19:46:00 +0200 (CEST) Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com [66.111.4.29]) by dpdk.org (Postfix) with ESMTP id 28B901BEB0 for ; Wed, 4 Sep 2019 19:45:59 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 8B5D622116; Wed, 4 Sep 2019 13:45:57 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 04 Sep 2019 13:45:57 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=27FRASDcxGEHmtZf6OxdJ0b0qokM2//QK+VHfAdx0uY=; b=I26+ejN0dtVn QN3c5EPpFUHonkhJzOEpuSkfu6wsMCtM29TJ5dMeemCqet1tNVOICdJJPC+81rMd SZKHzi1lSNaTrLM6aFCfBbmVKpg0VoftvmJGN7TTFcb+r3ClnCOC0ONwJ8pmFeue hrQxrzb9mDkU3vx/k+YWHbfmIxMUm38= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; bh=27FRASDcxGEHmtZf6OxdJ0b0qokM2//QK+VHfAdx0 uY=; b=KSP3jDhtig39ITF49fbhn+vSu+RNZSsjuQHw2nhd0APdoVu/qdzm9GlAR JkcF8r30ezk59nCBesxAPkqbq/RDZJc5W97v3eOWuGzC1YVP3J/i7ont6349UR87 WPfeIh1EIODarIYndDJs4RHSrdhME09bE40ZulGdpYvhLG+yHxsBbJtVycjUM+Yx xNTBlmBMvuQEOQFKrGdxlYa7UHN/pBdHze8KrvvwMU1oYWZ3LmPq3bh5TuPg7y4u fX7IXWM4DKzOShYHgW1slcnWl74gXaspv1ftqyLYzLXkc6qo7zIETN/fPwzrw4E+ rwBAijeJrl2KgTqimr7dM95t+TmkQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduvddrudejhedguddugecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc fkphepjeejrddufeegrddvtdefrddukeegnecurfgrrhgrmhepmhgrihhlfhhrohhmpeht hhhomhgrshesmhhonhhjrghlohhnrdhnvghtnecuvehluhhsthgvrhfuihiivgeptd X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 91C0E8005A; Wed, 4 Sep 2019 13:45:56 -0400 (EDT) From: Thomas Monjalon To: Ferruh Yigit , David Marchand Cc: dev@dpdk.org Date: Wed, 04 Sep 2019 19:45:55 +0200 Message-ID: <3044851.BoT7FmBCyV@xps> In-Reply-To: <6fd3bec6-15e4-33ed-4bd2-88d2ecaf6706@intel.com> References: <1566214919-32250-1-git-send-email-david.marchand@redhat.com> <6fd3bec6-15e4-33ed-4bd2-88d2ecaf6706@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH 02/11] log: define logtype register wrapper for drivers X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 03/09/2019 10:47, Ferruh Yigit: > On 9/3/2019 9:06 AM, David Marchand wrote: > > On Mon, Sep 2, 2019 at 4:29 PM Ferruh Yigit wrote: > >> On 8/19/2019 12:41 PM, David Marchand wrote: > >>> The function rte_log_register_type_and_pick_level() fills a gap for > >>> dynamically loaded code (especially drivers) who would not pick up > >>> the log level passed at startup. > >>> > >>> Let's promote it to stable and export it for use by drivers via > >>> a wrapper. > >>> > >>> Signed-off-by: David Marchand > >>> --- [...] > >>> /** > >>> - * @warning > >>> - * @b EXPERIMENTAL: this API may change without prior notice > >>> - * > >>> * Register a dynamic log type and try to pick its level from EAL options [...] > >>> -__rte_experimental > >>> int rte_log_register_type_and_pick_level(const char *name, uint32_t level_def); > >> > >> +1 to remove experimental from the API. I am not sure about this function API. Why we combined register and level setting in one function? > >>> +#define RTE_LOG_REGISTER(token, name, level, fallback) \ You really need to document this macro with doxygen. > >>> +{ \ > >>> + token = rte_log_register_type_and_pick_level(name, level); \ > >>> + if (token < 0) \ > >> > >> The failure can be because component can try to register existing log name, or > >> there is no enough memory, do you think does it worth to do log, or some > >> additional work if component is trying to register existing log name? [...] > >>> + token = fallback; \ > >> > >> Does the 'fallback' needs to be provided by user, it looks like everyone will > >> just copy/paste 'RTE_LOGTYPE_PMD' for drivers, and does it really needs to be > >> configurable since it is fallback. > > > > This series only touches drivers, but I expected other components > > would use this macro later. > > I can add a RTE_PMD_REGISTER_LOG macro that hides the RTE_LOGTYPE_PMD > > fallback value. I agree we don't need to configure the fallback log. If there is an error during log setup, we can log everything next (at debug level). Let's make fallback hardcoded. > >> Why not provide a hardcoded type for the failure case? And for that case perhaps > >> create a more generic logtype, something like "RTE_LOGTYPE_FALLBACK" so that it > >> can be as it is from all components? > > > > I prefer to map all drivers to a logtype that means something, like > > RTE_LOGTYPE_PMD. > > In that manner it make sense agreed, but previously drivers were using > 'RTE_LOGTYPE_PMD' instead of having their own log types, Stephen did some work > to replace the 'RTE_LOGTYPE_PMD' so that it can be deprecated, > > starting to use it again as fallback may lead drivers using it again as log type > in their drivers, may they wouldn't but this is what I concern. Something with > name 'RTE_LOGTYPE_FALLBACK' clear to not use as default logtype in drivers. > > > Having a "fallback" could be used for all components, but this would > > have to be a static logtype and adding one is not possible without > > breaking the abi (static entries are < 32 and all values are used). RTE_LOGTYPE_PMD can be renamed to RTE_LOGTYPE_FALLBACK. > There is a gap between 'RTE_LOGTYPE_GSO' & 'RTE_LOGTYPE_USER1' ... Yes, there is room here. But I prefer to rename and re-use RTE_LOGTYPE_PMD which is not used anymore. It is part of the EAL API but it is not supposed to be used externally. For out-of-tree PMDs, we are not supposed to provide such compat. So I would say don't care with deprecation here.