rte_log simplification using constructor scheme

Message ID BYAPR18MB24242A1618B1D210C736A9E1C8B10@BYAPR18MB2424.namprd18.prod.outlook.com (mailing list archive)
State Not Applicable, archived
Headers
Series rte_log simplification using constructor scheme |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK

Commit Message

Jerin Jacob Kollanukkaran May 27, 2020, 4:05 p.m. UTC
  Based on the discussion happed on http://patches.dpdk.org/patch/69681/,
I would like simply the rte_log to use constructor scheme and submit patch to
update existing consumer of rte_log to use new scheme for v20.08.
RTE_LOG_REGISTER will an optional symbol to use the rte_log registration mechanism.

Let me know if there any overlap on someone's else work on the same area or any objections?

Example cleanup for  octeontx2 driver.

 #endif
  

Comments

Thomas Monjalon June 8, 2020, 8:25 p.m. UTC | #1
27/05/2020 18:05, Jerin Kollanukkaran:
> +#define RTE_LOG_REGISTER(type, name, level)                            \
> +int type;                                                              \
> +RTE_INIT(__##type)                                                     \
> +{                                                                      \
> +       type = rte_log_register(RTE_STR(name));                         \
> +       if (type >= 0)                                                  \
> +               rte_log_set_level(type, RTE_LOG_##level);               \
> +}

We should consider using rte_log_register_type_and_pick_level()
which works for drivers loaded later in the init sequence.
I feel rte_log_register() could be deprecated.

The other question around log level is about the default level.
Do we want to allow having different default log levels per log type?
  
Jerin Jacob June 17, 2020, 6:40 a.m. UTC | #2
On Tue, Jun 9, 2020 at 1:55 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 27/05/2020 18:05, Jerin Kollanukkaran:
> > +#define RTE_LOG_REGISTER(type, name, level)                            \
> > +int type;                                                              \
> > +RTE_INIT(__##type)                                                     \
> > +{                                                                      \
> > +       type = rte_log_register(RTE_STR(name));                         \
> > +       if (type >= 0)                                                  \
> > +               rte_log_set_level(type, RTE_LOG_##level);               \
> > +}
>
> We should consider using rte_log_register_type_and_pick_level()
> which works for drivers loaded later in the init sequence.

I sent the first version[1] based on the current driver usage to avoid
any behavior change. (Major work is changing the drivers, we can
decide the RTE_LOG_REGISTER definition in next versions)
[1]
http://patches.dpdk.org/patch/71650/
I will send the v2 based on the consensus.
Please comment on http://patches.dpdk.org/patch/71650/.

> I feel rte_log_register() could be deprecated.

No strong opinion on this. @Olivier Matz , Thoughts?

> The other question around log level is about the default level.
> Do we want to allow having different default log levels per log type?

Currently, drivers are using various levels per log type. I can send
the v2 based on
the consensus.


>
>
  

Patch

diff --git a/drivers/common/octeontx2/otx2_common.c
b/drivers/common/octeontx2/otx2_common.c
index 1a257cf07..4d391a7e0 100644
--- a/drivers/common/octeontx2/otx2_common.c
+++ b/drivers/common/octeontx2/otx2_common.c
@@ -169,89 +169,13 @@  int otx2_npa_lf_obj_ref(void)
        return cnt ? 0 : -EINVAL;
 }
-/**
- * @internal
- */
-int otx2_logtype_base;
-/**
- * @internal
- */
-int otx2_logtype_mbox;
-/**
- * @internal
- */
-int otx2_logtype_npa;
-/**
- * @internal
- */
-int otx2_logtype_nix;
-/**
- * @internal
- */
-int otx2_logtype_npc;
-/**
- * @internal
- */
-int otx2_logtype_tm;
-/**
- * @internal
- */
-int otx2_logtype_sso;
-/**
- * @internal
- */
-int otx2_logtype_tim;
-/**
- * @internal
- */
-int otx2_logtype_dpi;
-/**
- * @internal
- */
-int otx2_logtype_ep;
-
-RTE_INIT(otx2_log_init);
-static void
-otx2_log_init(void)
-{
-       otx2_logtype_base = rte_log_register("pmd.octeontx2.base");
-       if (otx2_logtype_base >= 0)
-               rte_log_set_level(otx2_logtype_base, RTE_LOG_NOTICE);
-
-       otx2_logtype_mbox = rte_log_register("pmd.octeontx2.mbox");
-       if (otx2_logtype_mbox >= 0)
-               rte_log_set_level(otx2_logtype_mbox, RTE_LOG_NOTICE);
-
-       otx2_logtype_npa = rte_log_register("pmd.mempool.octeontx2");
-       if (otx2_logtype_npa >= 0)
-               rte_log_set_level(otx2_logtype_npa, RTE_LOG_NOTICE);
-
-       otx2_logtype_nix = rte_log_register("pmd.net.octeontx2");
-       if (otx2_logtype_nix >= 0)
-               rte_log_set_level(otx2_logtype_nix, RTE_LOG_NOTICE);
-
-       otx2_logtype_npc = rte_log_register("pmd.net.octeontx2.flow");
-       if (otx2_logtype_npc >= 0)
-               rte_log_set_level(otx2_logtype_npc, RTE_LOG_NOTICE);
-
-       otx2_logtype_tm = rte_log_register("pmd.net.octeontx2.tm");
-       if (otx2_logtype_tm >= 0)
-               rte_log_set_level(otx2_logtype_tm, RTE_LOG_NOTICE);
-
-       otx2_logtype_sso = rte_log_register("pmd.event.octeontx2");
-       if (otx2_logtype_sso >= 0)
-               rte_log_set_level(otx2_logtype_sso, RTE_LOG_NOTICE);
-
-       otx2_logtype_tim = rte_log_register("pmd.event.octeontx2.timer");
-       if (otx2_logtype_tim >= 0)
-               rte_log_set_level(otx2_logtype_tim, RTE_LOG_NOTICE);
-
-       otx2_logtype_dpi = rte_log_register("pmd.raw.octeontx2.dpi");
-       if (otx2_logtype_dpi >= 0)
-               rte_log_set_level(otx2_logtype_dpi, RTE_LOG_NOTICE);
-
-       otx2_logtype_ep = rte_log_register("pmd.raw.octeontx2.ep");
-       if (otx2_logtype_ep >= 0)
-               rte_log_set_level(otx2_logtype_ep, RTE_LOG_NOTICE);
-
-}
+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);

diff --git a/lib/librte_eal/include/rte_log.h b/lib/librte_eal/include/rte_log.h
index 1789ede56..22fc3802f 100644
--- a/lib/librte_eal/include/rte_log.h
+++ b/lib/librte_eal/include/rte_log.h
@@ -376,6 +376,15 @@  int rte_vlog(uint32_t level, uint32_t logtype,
const char *format, va_list ap)
                 RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__) :     \
         0)

+#define RTE_LOG_REGISTER(type, name, level)                            \
+int type;                                                              \
+RTE_INIT(__##type)                                                     \
+{                                                                      \
+       type = rte_log_register(RTE_STR(name));                         \
+       if (type >= 0)                                                  \
+               rte_log_set_level(type, RTE_LOG_##level);               \
+}
+
 #ifdef __cplusplus
 }