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 851EEA0588; Wed, 1 Apr 2020 18:25:40 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id CEFA01BF4F; Wed, 1 Apr 2020 18:25:39 +0200 (CEST) Received: from mail-il1-f195.google.com (mail-il1-f195.google.com [209.85.166.195]) by dpdk.org (Postfix) with ESMTP id A28951BF3C for ; Wed, 1 Apr 2020 18:25:38 +0200 (CEST) Received: by mail-il1-f195.google.com with SMTP id x16so493125ilp.12 for ; Wed, 01 Apr 2020 09:25:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=3WQqzqRTdyVK0Ooib3B013qPsNGE0iZGL4ZbbuBCY7k=; b=HbpX2sBmne3kMWUtAYGhPnz1atPm8yjtbGlh4B67bHMyqlQp+stoHxu5Wuspgl3TWU /YXWRawT8JUaoMg42ZfHG26sL6V2qwFnnXLJ/jh29H2nZhq3MXYKB9gIslbVSEpl0AL9 Sfy6Oc+bYccqMRiczN9DcxylpFqoCph+oD3S+wEyEjsBXxPGspfPx2yzMrBgpzr/9ca7 LMgAEF94eQE5LIFXCYFMlIS9apYtHr6YgdjtgFS2GcpN/OR3XLeKxs3VLiMCl4Um21t1 uUIL2HvNF6vn31kYRFOOnDoL0lhyaKTI9nftzWzJGRkCyP44f6ptzstXRHjJGI5rIUt5 utSw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=3WQqzqRTdyVK0Ooib3B013qPsNGE0iZGL4ZbbuBCY7k=; b=SfiM9Nlq1ZRcR0hRPDPMacyFH+R+tzB/DKfc+1HveJ/yy1yrqyM4KDthKG4xDGaykt rY12m4LwKCXdN7qbiGXTGGg8yY+uSurohU370r0wM/G3+ciL+BkdGSpocGwtc6pAgCtc cNIsZ2Ayw3lyoZ8u5LIVFPklcNHZLkMdVuhl/YuSnv/gibeJtu4Qz6iOGpfHcSgyebFC +trV/hNg5RGMX29eWqCKYtiOcIneA0mfDYrIHmDdKu75xsPxcioMsoR2JxpQUm2Y347w I8C4DqL6sXvdHwY1ANvzuqsavv+SOBwyhf0ZAWcepjSd4T3AruNhg0M4FPyg379I4jW2 sAFg== X-Gm-Message-State: ANhLgQ23OaL59iV7uf7TVqDcQnKaaSLRzQThCK4Vfhl+8BslNfl0hktR VZiQDvbkg6Ds83HwXDojxpcxAJga/UF796SqMn8= X-Google-Smtp-Source: ADFU+vuld6x3gY7TLWO/L/dKWIIlxYBmDEAXYVGmGaA7I7oLbgm2IYPSs/PCxtcHu+jLPu7tWKWHTQaLCHlaRQx1sis= X-Received: by 2002:a92:48cb:: with SMTP id j72mr22570595ilg.162.1585758337704; Wed, 01 Apr 2020 09:25:37 -0700 (PDT) MIME-Version: 1.0 References: <20200325211603.240288-1-jerinj@marvell.com> <20200329144342.1543749-1-jerinj@marvell.com> In-Reply-To: From: Jerin Jacob Date: Wed, 1 Apr 2020 21:55:20 +0530 Message-ID: To: David Marchand Cc: Jerin Jacob Kollanukkaran , Thomas Monjalon , dev , Bruce Richardson , =?UTF-8?Q?Mattias_R=C3=B6nnblom?= , Sunil Kumar Kori , "Yigit, Ferruh" , Andrew Rybchenko , Declan Doherty , Olivier Matz , Neil Horman , Ray Kinsella Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v3 00/33] DPDK Trace support 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" On Wed, Apr 1, 2020 at 7:42 PM David Marchand wrote: > > On Wed, Apr 1, 2020 at 12:05 PM Jerin Jacob wrote: > > On Wed, Apr 1, 2020 at 1:49 PM David Marchand wrote: > > > - Regardless of the trace framework, the ALLOW_EXPERIMENTAL_API flag > > > gates access to APIs so that external users are aware of their status. > > > I have been considering setting this flag unconditionally for internal > > > users in the top Makefile/meson for app/ lib/ and drivers/. > > > I could look at this and prepare a patch about this, but this is not > > > enough here. > > > > It makes sense to me. Let me know when you are planning to send that patch, > > I will rebase this series on top of that. > > Feel free to take over, thanks. OK. > > > > > > If you don't have time then I can send the patch too. > > I assume the patch content will be: > > 1) Removing experimental API from app, lib, drivers, examples with > > make and meson > > 2) Have it enabled at the global level. > > examples are a special case as they can be compiled out of the dpdk sources. > This is why I excluded them of the list in my mail before. There are a lot of examples already that have ALLOW_EXPERIMENTAL_API. What is the issue in enabling ALLOW_EXPERIMENTAL_API in the in tree example application? > > > > Since the trace changes in the "inline" functions of the specific > > library already > > disabled under _compile time_ RTE_ENABLE_TRACE_DP flag and > > it is using RTE_TRACE_POINT_DP() to define the trace unlike slow path > > trace like RTE_TRACE_POINT(). > > Ah indeed. > Note: RTE_ENABLE_TRACE_DP is not plugged with meson. If you see the " eal: define the public API for trace support" patch, I have updated rte_config.h for meson likewise for other configuration such log for DP. Would you like to have a meson option? and update in meson_options.txt. I don't have a strong opinion on either way? Bruce ,any thoughts? diff --git a/config/rte_config.h b/config/rte_config.h index d30786bc0..6b250288c 100644 --- a/config/rte_config.h +++ b/config/rte_config.h @@ -42,6 +42,7 @@ #define RTE_MAX_MEMZONE 2560 #define RTE_MAX_TAILQ 32 #define RTE_LOG_DP_LEVEL RTE_LOG_INFO +#define RTE_ENABLE_TRACE_DP 0 #define RTE_BACKTRACE 1 #define RTE_MAX_VFIO_CONTAINERS 64 > > > > So I can improve RTE_TRACE_POINT_DP() to make absolute NOP if > > ALLOW_EXPERIMENTAL_API not defined. > > > > On the upside, > > The tracing code will be enabled by default(runtime it is disabled by > > default anyway). > > If some need to fastpath API tracing then ALLOW_EXPERIMENTAL_API need > > to enable. > > So this won't break applications. > > So either keep the RTE_ENABLE_TRACE_DP flag or use > ALLOW_EXPERIMENTAL_API... no opinion. > Thomas? > Anyway we need a compile-time option? > The option is just for compatibility? > Then ALLOW_EXPERIMENTAL_API looks to be the right option. Just like log there is a compile-time option( CONFIG_RTE_LOG_DP_LEVEL for log) to disable trace generation in fastpath using RTE_ENABLE_TRACE_DP. This is to avoid "any" performance impact in fastpath code due to tracer. ALLOW_EXPERIMENTAL_API and RTE_ENABLE_TRACE_DP has different scope. I will add ALLOW_EXPERIMENTAL_API and RTE_ENABLE_TRACE_DP. ALLOW_EXPERIMENTAL_API can be removed later when it is stable. NOTE: I have used if (0) c style approach in the code to disable the code with RTE_ENABLE_TRACE_DP. This is to make sure the code always has compilation check respective RTE_ENABLE_TRACE_DP status. So ALLOW_EXPERIMENTAL_API need to avoid code compilation issues with external applications.