From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com [74.125.82.66]) by dpdk.org (Postfix) with ESMTP id 99D3F1B016 for ; Tue, 19 Dec 2017 11:01:34 +0100 (CET) Received: by mail-wm0-f66.google.com with SMTP id r78so2375774wme.5 for ; Tue, 19 Dec 2017 02:01:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=F+iRofxMYeR7VNdGJcYb8nkGii82Z4pInBMXf6X4b7c=; b=DChk5scIClbFxi7W8ry87tyve6zSLnDRgTJGhxBALtgT+PU6Z9iaYFW4Z6mSQI1lIC JhH/sDooFV9K76hiJOdaVtcmXfwyUEJ8ZWjzrPXFDMdGZdODtXauUpb93+BfFk/T2ZbW wIIpPvWCZrnjWzSnq+yxzkV5waCp+pj6uNkRV8rbH7i/Owau0z7ktm2vt3ukfd4HXdLx cx/R4s0o2VCHmg4D1aAa6hTY46IXO6UOxoqZFFVDJ0t9zNOp6Lrx2mbzaT3TwgfJsPnW 71+g6kp4JUrKLv15KTiuBSVG3jnH+NpPdaeMMbcLsjuJTC1eWd5U0TYZ5KN5gQHaSMHA ICBw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=F+iRofxMYeR7VNdGJcYb8nkGii82Z4pInBMXf6X4b7c=; b=a53GJOfM7ju5CzIPZphKrpFi14ztjDnL6SUM4K0qrVtFLPrVpJFQIErBwS7Y5V8jgf YCCy+q5JFwb8lJ1AJagVm52rqOAxtXDQadLaNkw4eUZVXNyn42EBoSdzaR2RrZMKxtv5 S9f5TcHJtg1QOzvL1RLjBP0a45/+CP+jRRysxCToN9KsNP4cW3SDXCwBnj54rgHstg8l nt3vUkcQTtjVYLpEYSV7/b3N31sv2TBUgRHpjCd56Z6RLTm6A+9VWJLKwa4pYZqrE8LL ecK5WK9reBNoTSdair/kKtfWp/jW+XaXUSDZURiwGAN4gNWDTrj6VCZuvgKeIy5A9JrU KHEg== X-Gm-Message-State: AKGB3mLICPWQ9FU7bzwEIAjN3AMu+YdrlQTjdXpoiw24MWH9UqAJN5Ms WRLiaEGhCMB4ht7qY4dLfoyZl+xc X-Google-Smtp-Source: ACJfBovbF2UZFblpIB2axmpVUQ1bVfEx72bAN9/hX3aLi1g9lL9zt+86XG8LrKVd4l4oLQwYiNJxZg== X-Received: by 10.28.137.5 with SMTP id l5mr3190586wmd.123.1513677694160; Tue, 19 Dec 2017 02:01:34 -0800 (PST) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id j3sm11423523edh.55.2017.12.19.02.01.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 19 Dec 2017 02:01:33 -0800 (PST) Date: Tue, 19 Dec 2017 11:01:17 +0100 From: Adrien Mazarguil To: Stephen Hemminger Cc: Thomas Monjalon , dev@dpdk.org, Ferruh Yigit Message-ID: <20171219100117.GA5377@6wind.com> References: <20171124172132.GW4062@6wind.com> <20171218162443.12971-2-adrien.mazarguil@6wind.com> <20171218102835.7602545f@xeon-e3> <7420927.98Cnld71HS@xps> <20171218131751.5c5d9467@xeon-e3> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171218131751.5c5d9467@xeon-e3> Subject: Re: [dpdk-dev] [PATCH v1 1/3] net/hyperv: introduce MS Hyper-V platform driver 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: , X-List-Received-Date: Tue, 19 Dec 2017 10:01:34 -0000 On Mon, Dec 18, 2017 at 01:17:51PM -0800, Stephen Hemminger wrote: > On Mon, 18 Dec 2017 20:54:16 +0100 > Thomas Monjalon wrote: > > > > > +#endif /* RTE_LIBRTE_HYPERV_DEBUG */ > > > > + > > > > +#define DEBUG(...) PMD_DRV_LOG(DEBUG, __VA_ARGS__) > > > > +#define INFO(...) PMD_DRV_LOG(INFO, __VA_ARGS__) > > > > +#define WARN(...) PMD_DRV_LOG(WARNING, __VA_ARGS__) > > > > +#define ERROR(...) PMD_DRV_LOG(ERR, __VA_ARGS__) > > > > + > > > > > > Please don't use DEBUG() etc macros. It makes it easier for tools that do > > > global updates or scans if all drivers use the same model of PMD_DRV_LOG > > > > The new standard is to use dynamic logtype. > > Agree, please use dynamic logging, and also don't redefine new macros like DEBUG/INFO/WARN/ERROR. > Instead use PMD_DRV_LOG or equivalent macros. Wait, the above definitions are only convenience wrappers to PMD_DRV_LOG(), itself a wrapper to RTE_LOG(), itself a wrapper to rte_log(), their presence is not triggered according to compilation options, did I miss something? Let me bring back some context from the original patch: #ifdef RTE_LIBRTE_HYPERV_DEBUG #define PMD_DRV_LOG(level, ...) \ RTE_LOG(level, PMD, \ RTE_FMT("%s:%u: %s(): " RTE_FMT_HEAD(__VA_ARGS__,) "\n", \ strrchr("/" __FILE__, '/') + 1, \ __LINE__, \ __func__, \ RTE_FMT_TAIL(__VA_ARGS__,))) #else /* RTE_LIBRTE_HYPERV_DEBUG */ #define PMD_DRV_LOG(level, ...) \ RTE_LOG(level, PMD, \ RTE_FMT(RTE_STR(HYPERV_DRIVER) ": " \ RTE_FMT_HEAD(__VA_ARGS__,) "\n", \ RTE_FMT_TAIL(__VA_ARGS__,))) #endif /* RTE_LIBRTE_HYPERV_DEBUG */ #define DEBUG(...) PMD_DRV_LOG(DEBUG, __VA_ARGS__) #define INFO(...) PMD_DRV_LOG(INFO, __VA_ARGS__) #define WARN(...) PMD_DRV_LOG(WARNING, __VA_ARGS__) #define ERROR(...) PMD_DRV_LOG(ERR, __VA_ARGS__) Enabling RTE_LIBRTE_HYPERV_DEBUG adds file and line information to log output, messages are otherwise unaffected by that compilation option. Adding this information required some sort of wrapper to avoid needless clutter. Nothing against outputting file/line information when compiled in debug mode right? > The base rule here is that all drivers should look the same as much > as reasonably possible. This makes reviewers of other subsystems more likely > to see problems. It also allows for later changes where some developer does a global > improvement across many PMD's. > > Drivers should not be snowflakes, each one is not unique. Point taken, do you confirm replacing i.e. WARN(...) with PMD_DRV_LOG(WARN, ...) and friends is all that's needed? -- Adrien Mazarguil 6WIND