Re: [dpdk-dev] [PATCH 01/13] eal/log: introduce log register macro
From: Jerin Jacob <hidden>
Date: 2020-06-26 11:16:19
On Wed, Jun 24, 2020 at 11:40 PM Jerin Jacob [off-list ref] wrote:
On Wed, Jun 24, 2020 at 9:13 PM Andrew Rybchenko [off-list ref] wrote:quoted
On 6/24/20 6:32 PM, Jerin Jacob wrote:quoted
On Wed, Jun 24, 2020 at 8:56 PM Andrew Rybchenko [off-list ref] wrote:quoted
On 6/24/20 4:11 PM, Jerin Jacob wrote:quoted
On Wed, Jun 17, 2020 at 3:51 PM Andrew Rybchenko [off-list ref] wrote:quoted
On 6/17/20 1:02 PM, David Marchand wrote:quoted
On Wed, Jun 17, 2020 at 8:30 AM [off-list ref] wrote:quoted
From: Jerin Jacob <redacted> Introducing the RTE_LOG_REGISTER macro to avoid the code duplication in the log registration process. It is a wrapper macro for declaring the logtype, register the log and setsHaving the logtype variable declared as part of the macro will force us to have global symbols (for the cases where it is needed). I'd rather leave the declaration to the caller, and only handle the registering part in this macro.I agree with David that it is important to avoid global symbols when it is not needed.David, Andrew, Since it is executed in "constructor" context, it will be always from the global variable. Right? i.e DPDK is not yet initialized in when "constructor" being called. In addition to that, It will be adding more lines of code in the consumer of this MACRO. Thoughts?The problem is rather 'extern' vs 'static'. Before the patch many variables are static, but become externally visible after the patch.OK. How about RTE_LOG_REGISTER_EXTERN or RTE_LOG_REGISTER_STATIC then? It will allow less code in the consumer of this macro. May be default we an make it as static so RTE_LOG_REGISTER and RTE_LOG_REGISTER_EXTERN for the different needs. Thoughts?Yes, I think it is a possible solution to use static in RTE_LOG_REGISTER and use RTE_LOG_REGISTER_EXTERN for non-static version. If we go this way, I'd prefer the option.OK.quoted
Alternative is to keep variable declaration outside, as David suggested, and I tend to agree that it is a bit better. Macro name says 'register'. It is not 'declare and register'. Also it avoids static-vs-extern problem completely. The solution allows to keep the variable declaration untouched and put constructor (macro) at the end of fine where constructors typically reside.My only concern with that approach is that, We can not save a lot of code duplication with that scheme. ie. it is [1] vs [2]. We can change the MACRO name accordingly if that is a concern. Any suggestions? Let me know your preference on [1] vs [2], I will stick with for the next version.
If there are no other comments, I change RTE_LOG_REGISTER to static version and RTE_LOG_REGISTER_EXTERN for a non-static version and send the next version.
[1] RTE_LOG_REGISTER(otx2_logtype_base, pmd.octeontx2.base, NOTICE); RTE_LOG_REGISTER(otx2_logtype_mbox, pmd.octeontx2.mbox, NOTICE); RTE_LOG_REGISTER(otx2_logtype_npa, pmd.mempool.octeontx2, NOTICE); RTE_LOG_REGISTER(otx2_logtype_nix, pmd.net.octeontx2, NOTICE); RTE_LOG_REGISTER(otx2_logtype_npc, pmd.net.octeontx2.flow, NOTICE); RTE_LOG_REGISTER(otx2_logtype_tm, pmd.net.octeontx2.tm, NOTICE); RTE_LOG_REGISTER(otx2_logtype_sso, pmd.event.octeontx2, NOTICE); RTE_LOG_REGISTER(otx2_logtype_tim, pmd.event.octeontx2.timer, NOTICE); RTE_LOG_REGISTER(otx2_logtype_dpi, pmd.raw.octeontx2.dpi, NOTICE); RTE_LOG_REGISTER(otx2_logtype_ep, pmd.raw.octeontx2.ep, NOTICE) [2] int otx2_logtype_base; int otx2_logtype_mbox; int otx2_logtype_npa; int otx2_logtype_nix; int otx2_logtype_npc; int otx2_logtype_tm; int otx2_logtype_sso; int otx2_logtype_tim; int otx2_logtype_dpi; int otx2_logtype_ep; RTE_LOG_REGISTER(otx2_logtype_base, pmd.octeontx2.base, NOTICE); RTE_LOG_REGISTER(otx2_logtype_mbox, pmd.octeontx2.mbox, NOTICE); RTE_LOG_REGISTER(otx2_logtype_npa, pmd.mempool.octeontx2, NOTICE); RTE_LOG_REGISTER(otx2_logtype_nix, pmd.net.octeontx2, NOTICE); RTE_LOG_REGISTER(otx2_logtype_npc, pmd.net.octeontx2.flow, NOTICE); RTE_LOG_REGISTER(otx2_logtype_tm, pmd.net.octeontx2.tm, NOTICE); RTE_LOG_REGISTER(otx2_logtype_sso, pmd.event.octeontx2, NOTICE); RTE_LOG_REGISTER(otx2_logtype_tim, pmd.event.octeontx2.timer, NOTICE); RTE_LOG_REGISTER(otx2_logtype_dpi, pmd.raw.octeontx2.dpi, NOTICE); RTE_LOG_REGISTER(otx2_logtype_ep, pmd.raw.octeontx2.ep, NOTICE)