From patchwork Wed Nov 4 18:39:57 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Hemminger X-Patchwork-Id: 8682 Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 389DF8E9A; Wed, 4 Nov 2015 19:39:50 +0100 (CET) Received: from mail-pa0-f41.google.com (mail-pa0-f41.google.com [209.85.220.41]) by dpdk.org (Postfix) with ESMTP id AF8B28E93 for ; Wed, 4 Nov 2015 19:39:48 +0100 (CET) Received: by pacdm15 with SMTP id dm15so35987992pac.3 for ; Wed, 04 Nov 2015 10:39:48 -0800 (PST) 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-type:content-transfer-encoding; bh=c3VIwaADjHksVGtieM01t8nlIYfwL6XMMcOgKbchmyQ=; b=T3Kp+IkjqlDbitYcLq+kl/ff+Sk9ahh9vL2ZZSPonbZ2OAnZGsDcEWoFfr4PUsJiNL dhD6ZO6EkJRsA5Lk7K+xp/ZcJiC3XlI7Y608agDXuX7nv0dH3vrGDZchr2kyoUMcXBms W3P40BVnBvwT6hZODjdi1LTRP/r36EkzhTYhREY5gERQwDLWXyA1LCpnTvTEsAd+RHFU d0xvJYbybIpTkVJ983CEl9bDKq8WgE/DVrtdrOKCwIJGCGqHOUsaOYxOAdhUWcrh1JZy 3Se63YDgcQDVaNAHOYACgrFiBnCdarSQhw7UAfpCsZPtJNU9rAGtdqU2D4f87zfpPQ// 7EXQ== 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-type:content-transfer-encoding; bh=c3VIwaADjHksVGtieM01t8nlIYfwL6XMMcOgKbchmyQ=; b=gyGS7PbtCiBCJllJHJh5Vu07io+cUtuTEnqRxW5cK32ei81m8uXVmXciZh183NL3gf BM0PArLvnRvCQxxkXuK/DyuPJp0+expavl1PQgxOJB0/rv4/uS2cju/3GawTlNKMGdWu EReYqhXYdjkS0w2ayJSKWYpiSk24s3i6kkQBNEUmGmApsMx4KpCnHBdyEIhWgbMBnMnT 1AHf5wI0DMbMXFmLLM2SgLmYZMCGUcXuDcpPfV5tmQePze8vtdKYL7V6P4jXyNcq9nQ4 bIxKZQdFtyE29zMYdh4Ic49hNM6rMY4IVuUN+sooViLZtCoZ0AvvV6kdQ7HDXL4ZV0r3 rBxA== X-Gm-Message-State: ALoCoQmj4qSGmvV3JWCD8YqLBpG+3T4mOPN3ryDJ5LTAt+WIbsJRJDe2on46pRg2BqvdbfdLH5T3 X-Received: by 10.68.162.35 with SMTP id xx3mr3691292pbb.79.1446662388034; Wed, 04 Nov 2015 10:39:48 -0800 (PST) Received: from xeon-e3 (static-50-53-82-155.bvtn.or.frontiernet.net. [50.53.82.155]) by smtp.gmail.com with ESMTPSA id ha1sm3359785pbc.54.2015.11.04.10.39.45 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 04 Nov 2015 10:39:47 -0800 (PST) Date: Wed, 4 Nov 2015 10:39:57 -0800 From: Stephen Hemminger To: Adrien Mazarguil Message-ID: <20151104103957.4cabd090@xeon-e3> In-Reply-To: <20151104102418.GN3518@6wind.com> References: <1441811374-28984-1-git-send-email-bruce.richardson@intel.com> <1446552059-5446-1-git-send-email-bruce.richardson@intel.com> <1446552059-5446-3-git-send-email-bruce.richardson@intel.com> <4698587.GS9blBozDC@xps13> <20151104102418.GN3518@6wind.com> MIME-Version: 1.0 Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v3 2/4] ethdev: move error checking macros to header 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, 4 Nov 2015 11:24:18 +0100 Adrien Mazarguil wrote: > On Wed, Nov 04, 2015 at 02:19:36AM +0100, Thomas Monjalon wrote: > > 2015-11-03 12:00, Bruce Richardson: > > > Move the function ptr and port id checking macros to the header file, so > > > that they can be used in the static inline functions there. In doxygen > > > comments, mark them as for internal use only. > > [...] > > > +/** > > > + * @internal > > > + * Macro to print a message if in debugging mode > > > + */ > > > +#ifdef RTE_LIBRTE_ETHDEV_DEBUG > > > +#define RTE_PMD_DEBUG_TRACE(fmt, args...) \ > > > + RTE_LOG(ERR, PMD, "%s: " fmt, __func__, ## args) > > > +#else > > > +#define RTE_PMD_DEBUG_TRACE(fmt, args...) > > > +#endif > > > > It does not compile because Mellanox drivers are pedantic: > > > > In file included from /home/thomas/projects/dpdk/dpdk/drivers/net/mlx4/mlx4.c:78:0: > > /home/thomas/projects/dpdk/dpdk/x86_64-native-linuxapp-gcc-shared-next/include/rte_ethdev.h: At top level: > > /home/thomas/projects/dpdk/dpdk/x86_64-native-linuxapp-gcc-shared-next/include/rte_ethdev.h:933:38: error: ISO C does not permit named variadic macros [-Werror=variadic-macros] > > #define RTE_PMD_DEBUG_TRACE(fmt, args...) \ > > I suggest something like the following definitions as a pedantic-proof and > standard compliant method (one drawback being that it cannot be done with a > single macro), see PMD_DRV_LOG() in drivers/net/mlx5/mlx5_utils.h which also > automatically appends a line feed: > > #ifdef RTE_LIBRTE_ETHDEV_DEBUG > > #define STRIP(a, b) a > #define OPAREN ( > #define CPAREN ) > #define COMMA , > > #define RTE_PMD_DEBUG_TRACE(...) \ > RTE_PMD_DEBUG_TRACE_(__VA_ARGS__ STRIP OPAREN, CPAREN) > > #define RTE_PMD_DEBUG_TRACE_(fmt, ...) \ > RTE_PMD_DEBUG_TRACE__(fmt COMMA __func__, __VA_ARGS__) > > #define RTE_PMD_DEBUG_TRACE__(...) \ > RTE_LOG(ERR, PMD, "%s: " __VA_ARGS__) > > #else /* RTE_LIBRTE_ETHDEV_DEBUG */ > > #define RTE_PMD_DEBUG_TRACE(...) > > #endif /* RTE_LIBRTE_ETHDEV_DEBUG */ > > STRIP() and other helper macros are used to manage the dangling comma issue > when __VA_ARGS__ is empty as in the first call below: > > RTE_PMD_DEBUG_TRACE("foo\n"); > RTE_PMD_DEBUG_TRACE("foo %u\n", 42); That solution is really ugly. Why not do something that keeps the expected checks. diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h index ede0dca..f3a3d34 100644 --- a/lib/librte_eal/common/include/rte_log.h +++ b/lib/librte_eal/common/include/rte_log.h @@ -99,6 +99,8 @@ extern struct rte_logs rte_logs; #define RTE_LOG_INFO 7U /**< Informational. */ #define RTE_LOG_DEBUG 8U /**< Debug-level messages. */ +#define RTE_LOG_DISABLED 99U /**< Never printed */ + /** The default log stream. */ extern FILE *eal_default_log_stream; diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h index eee1194..e431f2e 100644 --- a/lib/librte_ether/rte_ethdev.h +++ b/lib/librte_ether/rte_ethdev.h @@ -931,6 +931,61 @@ struct rte_eth_dev_callback; /** @internal Structure to keep track of registered callbacks */ TAILQ_HEAD(rte_eth_dev_cb_list, rte_eth_dev_callback); +/** + * @internal + * Macro to print a message if in debugging mode + */ +#ifdef RTE_LIBRTE_ETHDEV_DEBUG +#define RTE_PMD_DEBUG_TRACE(fmt, args...) \ + RTE_LOG(ERR, PMD, "%s: " fmt, __func__, ## args) +#else +#define RTE_PMD_DEBUG_TRACE(fmt, args...) \ + RTE_LOG(DISABLED, PMD, "%s: " fmt, __func__, ## args) +#endif