On Wed, Jun 24, 2020 at 9:13 PM Andrew Rybchenko <arybche...@solarflare.com> wrote: > > On 6/24/20 6:32 PM, Jerin Jacob wrote: > > On Wed, Jun 24, 2020 at 8:56 PM Andrew Rybchenko > > <arybche...@solarflare.com> wrote: > >> > >> On 6/24/20 4:11 PM, Jerin Jacob wrote: > >>> On Wed, Jun 17, 2020 at 3:51 PM Andrew Rybchenko > >>> <arybche...@solarflare.com> wrote: > >>>> > >>>> On 6/17/20 1:02 PM, David Marchand wrote: > >>>>> On Wed, Jun 17, 2020 at 8:30 AM <jer...@marvell.com> wrote: > >>>>>> > >>>>>> From: Jerin Jacob <jer...@marvell.com> > >>>>>> > >>>>>> 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 > >>>>>> sets > >>>>> > >>>>> Having 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. > > 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. [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)
