From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9C481C433E6 for ; Tue, 23 Feb 2021 10:03:33 +0000 (UTC) Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by mail.kernel.org (Postfix) with ESMTP id 034B164E20 for ; Tue, 23 Feb 2021 10:03:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 034B164E20 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dev-bounces@dpdk.org Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2C9FA4067A; Tue, 23 Feb 2021 11:03:32 +0100 (CET) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id 0809E40041 for ; Tue, 23 Feb 2021 11:03:29 +0100 (CET) IronPort-SDR: dR2wY0sIZa9uvPc0cJ5CnwlvHbjBtDK5XRzBusugcw9qLbk/UOJxLSP2IhzMaOuKtFoWkucsp+ AANaM6kyuifA== X-IronPort-AV: E=McAfee;i="6000,8403,9903"; a="246172163" X-IronPort-AV: E=Sophos;i="5.81,199,1610438400"; d="scan'208";a="246172163" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Feb 2021 02:03:28 -0800 IronPort-SDR: BNPiOyjEX3yEVl9FGJWLcOqUFiDxaLs2y2E0LRJ9nL9kp2d6lHae37RpKEzA7yeGWVGl6BuTJN /Qv0vSRCImxg== X-IronPort-AV: E=Sophos;i="5.81,199,1610438400"; d="scan'208";a="441688036" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.16.220]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 23 Feb 2021 02:03:09 -0800 Date: Tue, 23 Feb 2021 10:03:02 +0000 From: Bruce Richardson To: Dmitry Kozlyuk Cc: Jie , dev@dpdk.org, roretzla@linux.microsoft.com Message-ID: <20210223100302.GA95@bricha3-MOBL.ger.corp.intel.com> References: <1614029102-30858-1-git-send-email-jizh@linux.microsoft.com> <20210223012415.142bf2fc@sovereign> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210223012415.142bf2fc@sovereign> Subject: Re: [dpdk-dev] [PATCH] rte_metrics: unconditionally export rte_metrics_tel_xxx functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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" On Tue, Feb 23, 2021 at 01:24:15AM +0300, Dmitry Kozlyuk wrote: > + Bruce > > On Mon, 22 Feb 2021 13:25:02 -0800, Jie wrote: > [...] > > diff --git a/config/meson.build b/config/meson.build > > index 3cf560b8a..892bd9677 100644 > > --- a/config/meson.build > > +++ b/config/meson.build > > @@ -292,6 +292,11 @@ if is_freebsd > > add_project_arguments('-D__BSD_VISIBLE', language: 'c') > > endif > > > > +jansson = dependency('jansson', required: false, method: 'pkg-config') > > +if jansson.found() > > + dpdk_conf.set('RTE_HAVE_JANSSON', 1) > > +endif > > DPDK seems to prefer "HAS" for such macros. > > Why not do this in lib/librte_telemetry/meson.build? > > [...] > > --- a/lib/librte_metrics/meson.build > > +++ b/lib/librte_metrics/meson.build > > @@ -4,11 +4,12 @@ > > sources = files('rte_metrics.c') > > headers = files('rte_metrics.h') > > > > -jansson = dependency('jansson', required: false, method: 'pkg-config') > > -if jansson.found() > > +if dpdk_conf.has('RTE_HAVE_JANSSON') > > ext_deps += jansson > > - sources += files('rte_metrics_telemetry.c') > > - headers += files('rte_metrics_telemetry.h') > > - deps += ['ethdev', 'telemetry'] > > - includes += include_directories('../librte_telemetry') > > endif > > + > > +sources += files('rte_metrics_telemetry.c') > > +headers += files('rte_metrics_telemetry.h') > > Can be merged with definitions above. > > [...] > > int32_t > > rte_metrics_tel_reg_all_ethdev(int *metrics_register_done, int *reg_index_list) > > { > > +#ifdef JANSSON > > Why not use RTE_HAS_JANSSON everywhere? (One more occurrence below.) > +1 for this suggestion. Also, since this is essentially stubbing out the functions in this file, can you reduce the amount of ifdefs by putting either at the start or the end the full set of stubs, leaving just the one ifdef block covering the whole file. /Bruce