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 9EC9E41CEC; Mon, 20 Feb 2023 17:32:46 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9244E43096; Mon, 20 Feb 2023 17:32:46 +0100 (CET) Received: from NAM12-BN8-obe.outbound.protection.outlook.com (mail-bn8nam12on2089.outbound.protection.outlook.com [40.107.237.89]) by mails.dpdk.org (Postfix) with ESMTP id D35DA40395 for ; Mon, 20 Feb 2023 17:32:44 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ezIbpge6IAf4rsn3S6lG0+a9CADsvgPchLQO2Q//HWrcwkGx/1ouJB33WIu3qfol1I13a9DK/YiJDZK8otkK6XgXgbkUo2cRnosvwm8ahJ2xK9eklWsH+vxEVqyv+KB/oWIGTZpWuK+DTVV4HBPdKBZcfKZirOWXe6auq7pCmlCsX4c6hSDpf/Sj1+UOJlp9rImLwZTjSjD2yeVLZd+WJ2UfWFdmvV4YnpKXEMEF+5QbLTrqCeMO7O5S4sA3LDAQjF3GFR/voVbPVpAXvbfEVlEGV+kAtRxP7FuTndmNy13GiDN8kHW8LsJj+ChyTGjNujheDGciqP2SoDb3LGpffw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=17Wtn4wt4PZVxTcdIjnMWCC/Sy3Wvpb8lovIAibhLiY=; b=dbCLBSGqVGAXMce16tuG22vzWVtNgCCImXbhyCRh7g+Gi7T7RfzZcLl5TvBIHmbRXzonC1E2EY4Af8boTkvhDVtyE/WO+cFAzg6cTfYFh7DwJdA94Xq1vdkzkx1gQ7B51n/1J/H4kgJ9KD6uoKvR/Os3p/QDdaaBAtxgzkCrZW4kYTa4u5vxzGigHgoWQsAaFbFg5Ohkvs9h9SlIbMpddLJazIfM7skHFxAXpNgwqfZ7l9iHKXqUCwYpZQczbvjV6YSOzdG/c2xx8SLufwaGj/ZeO98tRT+CEyoyUxmGSznxkVY+pBFltpLkkTZRs8qBNyp6PZ+E7RUD7M/Nr//4iQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=17Wtn4wt4PZVxTcdIjnMWCC/Sy3Wvpb8lovIAibhLiY=; b=44gywN4BctCt3rZ9Zownm2ZVFKpz8VmcDiK3G4fNfY8mS440PQiZInUiP7hrhMtDT5gOWDNziI/DXdkxBVBA5MApKXjtoDP44Sg2wWtRxrj726phRxKBhlPOsmu1YriEkbVWCVRhH5S/vEmNb9KLA/iQKXE78+4DiJW6jG7B2x4= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) by DM6PR12MB4330.namprd12.prod.outlook.com (2603:10b6:5:21d::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6111.20; Mon, 20 Feb 2023 16:32:43 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::3614:22ed:ed5:5b48]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::3614:22ed:ed5:5b48%8]) with mapi id 15.20.6111.020; Mon, 20 Feb 2023 16:32:42 +0000 Message-ID: <5076e27d-b356-2e67-0885-daf325d7faf8@amd.com> Date: Mon, 20 Feb 2023 16:32:37 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH 2/3] net/nfp: get rid of the usage of RTE log level type Content-Language: en-US To: Stephen Hemminger Cc: Chaoyong He , "dev@dpdk.org" , oss-drivers , Niklas Soderlund References: <20230217024539.16514-1-chaoyong.he@corigine.com> <20230217024539.16514-3-chaoyong.he@corigine.com> <357ee243-d0d1-37fb-f7f3-ba4d99a001ed@amd.com> <20230220081641.623a2a97@hermes.local> From: Ferruh Yigit In-Reply-To: <20230220081641.623a2a97@hermes.local> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-ClientProxiedBy: LO2P123CA0079.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:138::12) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|DM6PR12MB4330:EE_ X-MS-Office365-Filtering-Correlation-Id: 649f7d69-c375-43d1-a061-08db13601702 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: fDkuj3UaiulpC0pk45hTdM+B+7QaVuPoAlpfGaGOu0qPSTqrlsEV+nq8V03kMWThUQmhqyLABzcHjeyIcCKDenu2JNcHehInaSylVLyDH45Dbnu1kKGpUjM6Gc4iiRQlgNRvSoxj0OE5whGbXyxWuivJuhGCOEnHFU13nz20IzxlH2lVdA/jCbJn23GdXEsllc/rwgLRMGzWKGZu/rTio59n4DVQW51usFMTrJy7fBptqlO6KJQXnPA79gYjMpRvPpg/BziZAQLR7TwO6w7cULxgkI09OhLV+y67YIP1FHvOIhcJC2HrF3rc3VLiXrr8esX3U/4GlmlYEJ70jiTY4qJ32ZoKXDm6WZQARyQk8xL7SKelTN2ujaFyYhc6mH6K9WqkCPFlSF0x7zHq6v6qJnnDEQUwXAWpOhbCxhCaJ0n/50SOIMxuOeOJpRVM8PMh6IBBTezVto65QLpiubOwh3Whftj0THIbEBLH+vnGVAWQhULfg3R5UKPTvjyxCkVtSgj3N14f+Bds8pnPItK7k00BXyqrHH+pLOwa1W73AkY2Rj6b+UrznOvpJtPk8YYIQZ6VyoXfqIsRcLOS0T2E6LiajVzr6GW78ftKfUYO159V2JVbRzQRoABxxxyt/4hNkIRvSRPz02Xw+dFLYqbfOCfj2NBo4s0Qx77/UB97PWYvSd0HUl9uhRWMXELjb5CorDkHATnfyThHFDh/gM6p/F54BX1/GECuxsyYO1D0W97x2PNz6dfkFTzZ/jo9uv5rQkpgno3fnWlFREUO+8khLw== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH2PR12MB4294.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230025)(4636009)(366004)(396003)(376002)(346002)(39860400002)(136003)(451199018)(31686004)(2906002)(44832011)(8936002)(5660300002)(66574015)(83380400001)(36756003)(6666004)(53546011)(6506007)(186003)(6512007)(2616005)(26005)(38100700002)(6486002)(86362001)(31696002)(478600001)(41300700001)(8676002)(66476007)(4326008)(66556008)(6916009)(54906003)(316002)(66946007)(26583001)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?czlHbE84aUQwWk1YL0h4RTFlQnd2WDdoN1BoK09jaWpjWFZodjA1SGZ1MkhP?= =?utf-8?B?dG1BaUNDZWJYVGRQdmRGSmVyQmgyakpFQTU2ODQwZkg1U1FSN2N4STE2cXM4?= =?utf-8?B?Wk1FOEdYZVBzTDBhc2R2MVdXWmdmMk44TTZicnp6VUhRMG5QbHdIcStlRHZj?= =?utf-8?B?UVBDZHBoMDJybWliL3ZGU1ZrcmJjK0FpekZraWtzSTdUZG51TElTNUxMK2hU?= =?utf-8?B?cCsrd2tYaEhhcm14ellmSVB6ZkV0SmN5QnFkMGNKU0M4YW84TUxnR1UvNlM4?= =?utf-8?B?bGcwbThDY1VMR0lNRm1uYktkbFErV0dqSnc0NC9wQnJ2eU5YTVJWK2F0MENK?= =?utf-8?B?dnJ2ZWFjT2xjazN4eURabStmUlVwT1k5NFEwTWp5cEdwOG45bjVCRG5RWnBZ?= =?utf-8?B?K2o2S2FPa0oveVlDOHpXSlR2NmJSSXp0ZGd2Z3FTMkl4a2Y4MzhtR2owZ0o2?= =?utf-8?B?WjJzREwrSkhDOGM3dmtpSEZxaVpwT2RQZE9QQjJQNWxYUUpuR3hNeTJlMlQ3?= =?utf-8?B?MzBqcHRGRHIwanBtbnR2RS9yd21hS085NVIyM2s5NHFTdTVqMjlibDFHODNi?= =?utf-8?B?dFlNRDVDQ21PY2JEUTYycmdDUmg2ZXFFZGRxU0V0VXFGNWlUVW5Rd2NnZ0dt?= =?utf-8?B?My9sbno5Uk5MYnp2Sm5GeFNoVWM4eTdGZTExWnB0akZrNDMyUW1rTm1TbS8z?= =?utf-8?B?eHpXaXB4emlpVVNxNmJ6Ukd1THV3ZHc3Z1dZMjV0WGx0ZElVdGVqanN2MU9W?= =?utf-8?B?MzlZV3VzbXhPZHAvTGFGRXdWMGFmUzUzeXV0RzFmWklUenZaaXB5UTNhSkJp?= =?utf-8?B?MGZYaGFUVkxERUpFQTI5SXdVM1lOeUNMZnliYmxwdjNtajJvSExXWVJ6Szl6?= =?utf-8?B?Qi9HSkx1NUd2bDQ4NzZkSlRNZ3NpdkViclM0eGtWQzZlMEQ0OTJlMkN5Vk1R?= =?utf-8?B?bFk4NUR4elZDWHlDSUdLSFpYdGoxMWhOeHBlVy9LaGZqU3JuOUsxUEhVSisv?= =?utf-8?B?dE5CbHJSb0lvK2R0Q3M3aW0yQmk3dXZoMTNNRGxCU3MxMk01MFNNbERkRHRk?= =?utf-8?B?a1VXaWxxb0lSeWhtL1lqdk1JdGJ2bEJzNytzOGZaemNpMWhvMDN0L0NGRFgz?= =?utf-8?B?YzdsamZJSUFPcUpyaTNCOTdXcFZpdmgvYytwNVNPR2VHUDZUb25qT1Q4bWJu?= =?utf-8?B?NzBQNElhV00reHBVeVpOTUVqYVF6elRaZkkweEtxQklQeWFZdUVISERIZHNZ?= =?utf-8?B?UVMycngyd0Zkbmk1anZhMWgzaDN1M3EvckI2b0JWWk41RENBTlIyd2JvL0VF?= =?utf-8?B?QnZtZHZuSWowUjdJanNNZ05jSkJTejBlRDVNbVhHZVpvUURwcGVDUG1jbkIz?= =?utf-8?B?S2VzWGNpdWVPRmJjZHRHYVBSQTkzQTkwMlRqVDBWRzVId0J6aWtycFdKZjhT?= =?utf-8?B?cE1ISjB3TzdCWTRobzQyejZTWnpwYVk2S0dVNXpIVlMwZUhIWlI2Y1JTK2V0?= =?utf-8?B?RjBUcm9PWlhJTVJFWUFlV3czL3drR0dKYzZPSFdSdlllVGNnN1RDUUZ4SGV2?= =?utf-8?B?ajdadXJOZHFSOUVnZkJiNGh3NE1makpZTzFWOVlhWG5yL0YxNERCTHFqVkp5?= =?utf-8?B?MmlzaDlVblNVNy84dGRZMjRXUVltZE5QMU9vbVpja2lqcHhvdUduUHlDMy9k?= =?utf-8?B?TGNMVWVYR1JtU1M5eFlPY2ZjMllBZHliSGpMM09sSmF5NlkvL3BoVHhsMDlx?= =?utf-8?B?K2RUQ3BOOVE4clVUOTZBekV3ZFNWaEZQc0twd3FYc2lsV2FuNkRLTU5sVUNN?= =?utf-8?B?REx2SWdGUG9mOGY2RG1PT25tNEtNYTJ0Umd6OG9WUnArdERiOWlwVTBnQVJX?= =?utf-8?B?UmoyN3BqSDNFek1UaWk5L2xqU3NBWGQyc2g3b0RBN1BmMFJzRGo2QUNSRE5r?= =?utf-8?B?T0laTkU0YlNpcGwzZmJLMjRXZm1TTUxTbWwzRm96UlFIcUZXUWpYNzRRbG1r?= =?utf-8?B?ck55VzdJOG9GRWFRTndBV2R2L2xlV1lsVnVxTkpwd05FSk9FRnAwcFlkNnRk?= =?utf-8?B?T3NjTmRTQ1YxM1hiNkdDaWZpY2FVZWpjS3ltUHYxRVJ0enFFckRVVTBJWHFz?= =?utf-8?Q?7UgYaDz1w8xI2LPk/UckmyEhW?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 649f7d69-c375-43d1-a061-08db13601702 X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Feb 2023 16:32:42.8768 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: pFl7HHh8guC7Erx0iSbFPCz7lNiHjKfh968/aLBm0iMtU5fahPMO3M+zHtVK3JF6 X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4330 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 2/20/2023 4:16 PM, Stephen Hemminger wrote: > On Mon, 20 Feb 2023 10:09:51 +0000 > Ferruh Yigit wrote: > >> On 2/20/2023 1:36 AM, Chaoyong He wrote: >>>> On 2/17/2023 2:45 AM, Chaoyong He wrote: >>>>> Register the own RX/TX debug log level type, and get rid of the usage >>>>> of RTE_LOGTYPE_*. Then we can control the log by a independent switch. >>>>> >>>>> Signed-off-by: Chaoyong He >>>>> Reviewed-by: Niklas Söderlund >>>>> --- >>>>> drivers/net/nfp/nfp_logs.c | 10 ++++++++++ >>>>> drivers/net/nfp/nfp_logs.h | 8 ++++++-- >>>>> 2 files changed, 16 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/net/nfp/nfp_logs.c b/drivers/net/nfp/nfp_logs.c >>>>> index 48c42fe53f..cd58bcee43 100644 >>>>> --- a/drivers/net/nfp/nfp_logs.c >>>>> +++ b/drivers/net/nfp/nfp_logs.c >>>>> @@ -5,6 +5,16 @@ >>>>> >>>>> #include "nfp_logs.h" >>>>> >>>>> +#include >>>>> + >>>>> RTE_LOG_REGISTER_SUFFIX(nfp_logtype_init, init, NOTICE); >>>>> RTE_LOG_REGISTER_SUFFIX(nfp_logtype_driver, driver, NOTICE); >>>>> RTE_LOG_REGISTER_SUFFIX(nfp_logtype_cpp, cpp, NOTICE); >>>>> + >>>>> +#ifdef RTE_ETHDEV_DEBUG_RX >>>>> +RTE_LOG_REGISTER_SUFFIX(nfp_logtype_rx, rx, DEBUG) #endif >>>>> + >>>>> +#ifdef RTE_ETHDEV_DEBUG_TX >>>>> +RTE_LOG_REGISTER_SUFFIX(nfp_logtype_tx, tx, DEBUG) #endif >>>>> diff --git a/drivers/net/nfp/nfp_logs.h b/drivers/net/nfp/nfp_logs.h >>>>> index b7632ee72c..315a57811c 100644 >>>>> --- a/drivers/net/nfp/nfp_logs.h >>>>> +++ b/drivers/net/nfp/nfp_logs.h >>>>> @@ -15,15 +15,19 @@ extern int nfp_logtype_init; #define >>>>> PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>") >>>>> >>>>> #ifdef RTE_ETHDEV_DEBUG_RX >>>>> +extern int nfp_logtype_rx; >>>>> #define PMD_RX_LOG(level, fmt, args...) \ >>>>> - RTE_LOG(level, PMD, "%s() rx: " fmt "\n", __func__, ## args) >>>>> + rte_log(RTE_LOG_ ## level, nfp_logtype_rx, \ >>>>> + "%s(): " fmt "\n", __func__, ## args) >>>>> #else >>>>> #define PMD_RX_LOG(level, fmt, args...) do { } while (0) #endif >>>>> >>>>> #ifdef RTE_ETHDEV_DEBUG_TX >>>>> +extern int nfp_logtype_tx; >>>>> #define PMD_TX_LOG(level, fmt, args...) \ >>>>> - RTE_LOG(level, PMD, "%s() tx: " fmt "\n", __func__, ## args) >>>>> + rte_log(RTE_LOG_ ## level, nfp_logtype_tx, \ >>>>> + "%s(): " fmt "\n", __func__, ## args) >>>>> #else >>>>> #define PMD_TX_LOG(level, fmt, args...) do { } while (0) #endif >>>> >>>> Intention is to replace 'RTE_LOG_DP' with 'PMD_RX_LOG'/'PMD_TX_LOG', >>>> but these are not exactly same (although difference is minor). >>>> >>>> When 'RTE_ETHDEV_DEBUG_RX' is set, ethdev layer also adds some >>>> additional load, although I believe that will small comparing to logging in >>>> driver. >>>> If 'RTE_LOG_DP' used, the ethdev layer cost can be removed. >>>> >>>> With 'RTE_LOG_DP', log level more verbose than requested won't cause any >>>> performance impact. Like if ERR level requested, INFO, DEBUG etc logs will >>>> be compiled out and won't cause any performance impact. >>>> But with 'RTE_ETHDEV_DEBUG_RX', even log level only request ERR, all >>>> logging will add cost of at least an if branch (checking log level). >>>> >>>> >>>> For many cases I am not sure these differences matters, and already many >>>> drivers directly uses 'RTE_ETHDEV_DEBUG_RX' as done here. So you may >>>> prefer to keep as it is. >>>> >>>> But if there is a desire for this fine grain approach, it is possible to add a >>>> version of 'RTE_LOG_DP' macro that accepts dynamic log type (instead of >>>> static RTE_LOGTYPE_# type), what do you think? >>>> >>> >>> Thanks for the suggestion. >>> For now, we prefer to keep as it is. >>> If we does need the more refined design in the future, we would follow your advice here, thanks again. >> >> ack, I just wanted to double check. I will proceed as it is. > > As part of my patch series (work in progress) to get rid of RTE_LOGTYPE_PMD > needed to add a helper now for RTE_DP_LOG like this. > > diff --git a/lib/eal/include/rte_log.h b/lib/eal/include/rte_log.h > index 6d2b0856a565..f377bc6db79b 100644 > --- a/lib/eal/include/rte_log.h > +++ b/lib/eal/include/rte_log.h > @@ -336,6 +336,37 @@ int rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap) > rte_log(RTE_LOG_ ## l, \ > RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__) > > +/** > + * Generates a log message for data path. > + * > + * Similar to rte_log(), except that it is an inline function that > + * can be eliminated at compilation time if RTE_LOG_DP_LEVEL configuration > + * option is lower than the log level argument. > + * > + * @param level > + * Log level. A value between RTE_LOG_EMERG (1) and RTE_LOG_DEBUG (8). > + * @param logtype > + * The log type, for example, RTE_LOGTYPE_EAL. > + * @param format > + * The format string, as in printf(3), followed by the variable arguments > + * required by the format. > + * @return > + * - 0: Success. > + * - Negative on error. > + */ > +static inline __rte_format_printf(3, 4) > +void rte_log_dp(uint32_t level, uint32_t logtype, const char *format, ...) > + > +{ > + va_list ap; > + > + if (level <= RTE_LOG_DP_LEVEL) { > + va_start(ap, format); > + rte_vlog(level, logtype, format, ap); > + va_end(ap); > + } > +} > + > /** > * Generates a log message for data path. > * > @@ -357,10 +388,8 @@ int rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap) > * - Negative on error. > */ > #define RTE_LOG_DP(l, t, ...) \ > - (void)((RTE_LOG_ ## l <= RTE_LOG_DP_LEVEL) ? \ > - rte_log(RTE_LOG_ ## l, \ > - RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__) : \ > - 0) > + rte_log_dp(RTE_LOG_ ## l, \ > + RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__) > +1 to have 'rte_log_dp()' as above, this way data path logs can be added with dynamic log types.