From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 03F4345A8B; Mon, 7 Oct 2024 07:45:42 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C9872402A5; Mon, 7 Oct 2024 07:45:41 +0200 (CEST) Received: from mail-qk1-f169.google.com (mail-qk1-f169.google.com [209.85.222.169]) by mails.dpdk.org (Postfix) with ESMTP id 3B6FA40151 for ; Mon, 7 Oct 2024 07:45:40 +0200 (CEST) Received: by mail-qk1-f169.google.com with SMTP id af79cd13be357-7a9ad8a7c63so415364685a.3 for ; Sun, 06 Oct 2024 22:45:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1728279939; x=1728884739; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=TitxpNiD9Y0ajgYZlcYtjOAK7upQuVZY3QMKHSDcbZo=; b=Am4eMIOHOCyCVdHQOglekxB/AewbDXTyxQmsOhrnReR+0obvrA4+K61V0anCiofyow V183lH+3ysd4l2ZgJW0B5A0KhQJiBrVS+594gPXtLsxWcwqLwqlFX8Jd9376ikQ6v3Lb 6C22lCaxc8SXdT5Ktlr00IgNV+1jen1dq1g45uMRP2AceuqBWcXhQkyGRIcSdTiao8ey 5PBVnAhGq1XfqsakWB86b/WExbID+WOCRJF0TdOwgv5B6D57Vg7h4adcvMWiRx/rTh8N DzbtQH7EAeyX+N3J8t6+nP5n5aHi47LYmILfgCwqmoXq5XMh1cJTqNUGaqN7j3ugMgpc gsAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728279939; x=1728884739; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=TitxpNiD9Y0ajgYZlcYtjOAK7upQuVZY3QMKHSDcbZo=; b=ofKisSsyW5COHFNha8IDyhrKC+dz9oT3bz54R6pN7jpO7I5G/nZw3JWQDTf42nTa3c 1xXYwvNx6JWsImefNXFaudtP2pMjBcmiylQq5S4z4kKEaRPgAvyByNWUdj3AdL2T8sb2 CP37qFkSurxwJsrs4274g/Hv/SISmZp5HDdBK3bmObYEb9eFYH0xJQ1O3nRonGEHyCPW 1Whmz7wcA/iLVa0gy0BdjPTiIbLbqOEkMGiQiE27NjBcc+o1eJ+VU2CrPb2qDZg5h+g/ UbqrAW1A1bKbFCfUEdqG5W0OVuZ7tfYaynEvWfWNNXLDKeofCHeiV9/BR6cuXV4Xw4Vf TPnw== X-Forwarded-Encrypted: i=1; AJvYcCXn4d3wiFi3CStTXGCd5kH4qy92nDTAV1CQLK2djkVkmEBmWhcXc2rdwLAqcETs6Ps3lfk=@dpdk.org X-Gm-Message-State: AOJu0YyZaDNlfeNcZlFilciUCfF5vjLaxj7UCQ7P6i9StoKLnkRDpWkK ozZIeaLzOZK5sMkYZmTTlUH/PRZiMXMWintKSmQOOf2kO7LPNCI4QaSckHEa1fejHbeibo3QgRQ 2VwB25fA5MhLuwLxaXQRt+I4pWl4= X-Google-Smtp-Source: AGHT+IEC76gJs0aqeDRTYhdg+B0IaZufgo39lCSTJrut8DNPriMPCjI9+n4P5KldlYskQmmJmuH5/Hu+UR0ozfop2zg= X-Received: by 2002:a05:620a:2a11:b0:7a9:a661:aa63 with SMTP id af79cd13be357-7ae6f4868f7mr1726990685a.52.1728279939315; Sun, 06 Oct 2024 22:45:39 -0700 (PDT) MIME-Version: 1.0 References: <20240918085551.231015-1-mb@smartsharesystems.com> <20241006135858.2768371-1-mb@smartsharesystems.com> In-Reply-To: <20241006135858.2768371-1-mb@smartsharesystems.com> From: Jerin Jacob Date: Mon, 7 Oct 2024 11:15:13 +0530 Message-ID: Subject: Re: [PATCH v6] eal: add build-time option to omit trace To: =?UTF-8?Q?Morten_Br=C3=B8rup?= Cc: Jerin Jacob , Sunil Kumar Kori , dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 On Sun, Oct 6, 2024 at 7:44=E2=80=AFPM Morten Br=C3=B8rup wrote: > > Some applications want to omit the trace feature. > Either to reduce the memory footprint, to reduce the exposed attack > surface, or for other reasons. > > This patch adds an option in rte_config.h to include or omit trace in the > build. Trace is included by default. > > Omitting trace works by omitting all trace points. > For API and ABI compatibility, the trace feature itself remains. > > Signed-off-by: Morten Br=C3=B8rup > --- > v6: > Removed test_trace_perf.c changes; they don't compile for Windows > target, and are superfluous. > v5: > Added public function rte_trace_feature_is_enabled(), to test if trace > is build time enabled in both the DPDK and the application. Use in test > application instead of private function. (Jerin Jacob) > v4: > * Added check for generic trace enabled when registering trace points, in > RTE_INIT. (Jerin Jacob) > * Test application uses function instead of macro to check if generic > trace is enabled. (Jerin Jacob) > * Performance test application uses function to check if generic trace is > enabled. > v3: > * Simpler version with much fewer ifdefs. (Jerin Jacob) > v2: > * Added/modified macros required for building applications with trace > omitted. > --- > app/test/test_trace.c | 4 +++ > config/rte_config.h | 1 + > lib/eal/common/eal_common_trace.c | 10 +++++++ > lib/eal/include/rte_trace.h | 33 ++++++++++++++++++++++ > lib/eal/include/rte_trace_point.h | 21 ++++++++++++++ > lib/eal/include/rte_trace_point_register.h | 2 ++ > lib/eal/version.map | 3 ++ > 7 files changed, 74 insertions(+) > > diff --git a/app/test/test_trace.c b/app/test/test_trace.c > index 00809f433b..8ea1443044 100644 > --- a/app/test/test_trace.c > +++ b/app/test/test_trace.c > @@ -245,6 +245,10 @@ static struct unit_test_suite trace_tests =3D { > static int > test_trace(void) > { > + if (!rte_trace_feature_is_enabled()) { > + printf("Trace omitted at build-time, skipping test\n"); > + return TEST_SKIPPED; > + } > return unit_test_suite_runner(&trace_tests); > } > > diff --git a/config/rte_config.h b/config/rte_config.h > index dd7bb0d35b..fd6f8a2f1a 100644 > --- a/config/rte_config.h > +++ b/config/rte_config.h > @@ -49,6 +49,7 @@ > #define RTE_MAX_TAILQ 32 > #define RTE_LOG_DP_LEVEL RTE_LOG_INFO > #define RTE_MAX_VFIO_CONTAINERS 64 > +#define RTE_TRACE 1 > > /* bsd module defines */ > #define RTE_CONTIGMEM_MAX_NUM_BUFS 64 > diff --git a/lib/eal/common/eal_common_trace.c b/lib/eal/common/eal_commo= n_trace.c > index 918f49bf4f..06130c756d 100644 > --- a/lib/eal/common/eal_common_trace.c > +++ b/lib/eal/common/eal_common_trace.c > @@ -100,6 +100,16 @@ rte_trace_is_enabled(void) > return rte_atomic_load_explicit(&trace.status, rte_memory_order_a= cquire) !=3D 0; > } > > +bool > +__rte_trace_feature_is_enabled(void) > +{ > +#ifdef RTE_TRACE > + return true; > +#else > + return false; > +#endif > +} > +static __rte_always_inline > +bool rte_trace_feature_is_enabled(void) > +{ > +#ifdef RTE_TRACE > + return __rte_trace_feature_is_enabled(); > +#else > + return false; > +#endif > +} > +__rte_experimental > +static __rte_always_inline bool > +__rte_trace_point_generic_is_enabled(void) > +{ > +#ifdef RTE_TRACE > + return true; > +#else > + return false; > +#endif __rte_trace_feature_is_enabled(), rte_trace_feature_is_enabled(), __rte_trace_point_generic_is_enabled() are duplicates. There is no harm in using a public API inside the implementation. Please keep only rte_trace_feature_is_enabled() and use it across implementation and app/test. > +} > + > /** > * @internal > * > rte_vfio_get_device_info; # WINDOWS_NO_EXPORT > + > + # added in 24.11 > + __rte_trace_feature_is_enabled; rte_trace_feature_is_enabled; With the above changes, Acked-by: Jerin Jacob > }; > > INTERNAL { > -- > 2.43.0 >