On Wed, Jun 24, 2020 at 11:40 PM Jerin Jacob <jerinjac...@gmail.com> wrote: > > 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.
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)
