* [dpdk-dev] [PATCH] net/e1000 igb: fix compile issue with log register
@ 2018-05-21 9:31 Harry van Haaren
2018-05-21 10:10 ` Varghese, Vipin
0 siblings, 1 reply; 3+ messages in thread
From: Harry van Haaren @ 2018-05-21 9:31 UTC (permalink / raw)
To: dev; +Cc: Harry van Haaren, vipin.varghese, wenzhuo.lu, helin.zhang
This commit fixes a compilation error if EM_PMD is
not defined, bug IGB_PMD is. The root cause of the
issue was that log init variables are declared as
extern in a header file, while the definition of the
variables was in e1000_ethdev.c. Hence, the definitions
were not available if the e1000 PMD is disabled.
To fix this, a new file is added e1000_logs.c, which
matches the e1000_logs.h header. The log variables are
always compiled in, but the PMD logs are only registered
if a PMD is enabled in the configuration. Extra checks
are added in order to avoid duplicate registering.
Fixes: ed5bbb767c3e ("net/e1000: implement dynamic logging")
Reported-by: Vipin Varghese <vipin.varghese@intel.com>
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
Cc: vipin.varghese@intel.com
Cc: wenzhuo.lu@intel.com
Cc: helin.zhang@intel.com
---
drivers/net/e1000/Makefile | 1 +
drivers/net/e1000/e1000_logs.c | 26 ++++++++++++++++++++++++++
drivers/net/e1000/e1000_logs.h | 6 ++++++
drivers/net/e1000/em_ethdev.c | 16 ++++------------
drivers/net/e1000/igb_ethdev.c | 8 ++++++++
drivers/net/e1000/meson.build | 1 +
6 files changed, 46 insertions(+), 12 deletions(-)
create mode 100644 drivers/net/e1000/e1000_logs.c
diff --git a/drivers/net/e1000/Makefile b/drivers/net/e1000/Makefile
index 87cfb26..9c87e88 100644
--- a/drivers/net/e1000/Makefile
+++ b/drivers/net/e1000/Makefile
@@ -62,6 +62,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_82575.c
SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_i210.c
SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_api.c
SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_ich8lan.c
+SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_logs.c
SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_mac.c
SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_manage.c
SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_mbx.c
diff --git a/drivers/net/e1000/e1000_logs.c b/drivers/net/e1000/e1000_logs.c
new file mode 100644
index 0000000..2217393
--- /dev/null
+++ b/drivers/net/e1000/e1000_logs.c
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2018 Intel Corporation
+ */
+
+#include "e1000_logs.h"
+
+/* declared as extern in e1000_logs.h */
+int e1000_logtype_init;
+int e1000_logtype_driver;
+
+/* avoids double registering of logs if EM and IGB drivers are in use */
+static int e1000_log_initialized;
+
+void
+e1000_igb_init_log(void)
+{
+ if (!e1000_log_initialized) {
+ e1000_logtype_init = rte_log_register("pmd.net.e1000.init");
+ if (e1000_logtype_init >= 0)
+ rte_log_set_level(e1000_logtype_init, RTE_LOG_NOTICE);
+ e1000_logtype_driver = rte_log_register("pmd.net.e1000.driver");
+ if (e1000_logtype_driver >= 0)
+ rte_log_set_level(e1000_logtype_driver, RTE_LOG_NOTICE);
+ e1000_log_initialized = 1;
+ }
+}
diff --git a/drivers/net/e1000/e1000_logs.h b/drivers/net/e1000/e1000_logs.h
index 50348e9..69d3d31 100644
--- a/drivers/net/e1000/e1000_logs.h
+++ b/drivers/net/e1000/e1000_logs.h
@@ -5,6 +5,8 @@
#ifndef _E1000_LOGS_H_
#define _E1000_LOGS_H_
+#include <rte_log.h>
+
extern int e1000_logtype_init;
#define PMD_INIT_LOG(level, fmt, args...) \
rte_log(RTE_LOG_ ## level, e1000_logtype_init, \
@@ -41,4 +43,8 @@ extern int e1000_logtype_driver;
#define PMD_DRV_LOG(level, fmt, args...) \
PMD_DRV_LOG_RAW(level, fmt "\n", ## args)
+
+/* log init function shared by e1000 and igb drivers */
+void e1000_igb_init_log(void);
+
#endif /* _E1000_LOGS_H_ */
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 4e890ad..7039dc1 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -11,7 +11,6 @@
#include <rte_common.h>
#include <rte_interrupts.h>
#include <rte_byteorder.h>
-#include <rte_log.h>
#include <rte_debug.h>
#include <rte_pci.h>
#include <rte_bus_pci.h>
@@ -106,9 +105,6 @@ static int eth_em_set_mc_addr_list(struct rte_eth_dev *dev,
static enum e1000_fc_mode em_fc_setting = e1000_fc_full;
-int e1000_logtype_init;
-int e1000_logtype_driver;
-
/*
* The set of PCI devices this driver supports
*/
@@ -1826,14 +1822,10 @@ RTE_PMD_REGISTER_PCI(net_e1000_em, rte_em_pmd);
RTE_PMD_REGISTER_PCI_TABLE(net_e1000_em, pci_id_em_map);
RTE_PMD_REGISTER_KMOD_DEP(net_e1000_em, "* igb_uio | uio_pci_generic | vfio-pci");
-RTE_INIT(e1000_init_log);
+/* see e1000_logs.c */
+RTE_INIT(igb_init_log);
static void
-e1000_init_log(void)
+igb_init_log(void)
{
- e1000_logtype_init = rte_log_register("pmd.net.e1000.init");
- if (e1000_logtype_init >= 0)
- rte_log_set_level(e1000_logtype_init, RTE_LOG_NOTICE);
- e1000_logtype_driver = rte_log_register("pmd.net.e1000.driver");
- if (e1000_logtype_driver >= 0)
- rte_log_set_level(e1000_logtype_driver, RTE_LOG_NOTICE);
+ e1000_igb_init_log();
}
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 04d34bd..edc7be3 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -5681,3 +5681,11 @@ RTE_PMD_REGISTER_KMOD_DEP(net_e1000_igb, "* igb_uio | uio_pci_generic | vfio-pci
RTE_PMD_REGISTER_PCI(net_e1000_igb_vf, rte_igbvf_pmd);
RTE_PMD_REGISTER_PCI_TABLE(net_e1000_igb_vf, pci_id_igbvf_map);
RTE_PMD_REGISTER_KMOD_DEP(net_e1000_igb_vf, "* igb_uio | vfio-pci");
+
+/* see e1000_logs.c */
+RTE_INIT(e1000_init_log);
+static void
+e1000_init_log(void)
+{
+ e1000_igb_init_log();
+}
diff --git a/drivers/net/e1000/meson.build b/drivers/net/e1000/meson.build
index 3a1bf5a..cf45699 100644
--- a/drivers/net/e1000/meson.build
+++ b/drivers/net/e1000/meson.build
@@ -5,6 +5,7 @@ subdir('base')
objs = [base_objs]
sources = files(
+ 'e1000_logs.c',
'em_ethdev.c',
'em_rxtx.c',
'igb_ethdev.c',
--
2.7.4
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH] net/e1000 igb: fix compile issue with log register
2018-05-21 9:31 [dpdk-dev] [PATCH] net/e1000 igb: fix compile issue with log register Harry van Haaren
@ 2018-05-21 10:10 ` Varghese, Vipin
2018-05-22 2:03 ` Zhang, Helin
0 siblings, 1 reply; 3+ messages in thread
From: Varghese, Vipin @ 2018-05-21 10:10 UTC (permalink / raw)
To: Van Haaren, Harry, dev; +Cc: Lu, Wenzhuo, Zhang, Helin
Acked-by: Vipin Varghese <vipin.varghese@intel.com>
> -----Original Message-----
> From: Van Haaren, Harry
> Sent: Monday, May 21, 2018 3:02 PM
> To: dev@dpdk.org
> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Varghese, Vipin
> <vipin.varghese@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhang,
> Helin <helin.zhang@intel.com>
> Subject: [PATCH] net/e1000 igb: fix compile issue with log register
>
> This commit fixes a compilation error if EM_PMD is not defined, bug IGB_PMD
> is. The root cause of the issue was that log init variables are declared as extern
> in a header file, while the definition of the variables was in e1000_ethdev.c.
> Hence, the definitions were not available if the e1000 PMD is disabled.
>
> To fix this, a new file is added e1000_logs.c, which matches the e1000_logs.h
> header. The log variables are always compiled in, but the PMD logs are only
> registered if a PMD is enabled in the configuration. Extra checks are added in
> order to avoid duplicate registering.
>
> Fixes: ed5bbb767c3e ("net/e1000: implement dynamic logging")
>
> Reported-by: Vipin Varghese <vipin.varghese@intel.com>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
>
> ---
>
> Cc: vipin.varghese@intel.com
> Cc: wenzhuo.lu@intel.com
> Cc: helin.zhang@intel.com
> ---
> drivers/net/e1000/Makefile | 1 +
> drivers/net/e1000/e1000_logs.c | 26 ++++++++++++++++++++++++++
> drivers/net/e1000/e1000_logs.h | 6 ++++++ drivers/net/e1000/em_ethdev.c |
> 16 ++++------------ drivers/net/e1000/igb_ethdev.c | 8 ++++++++
> drivers/net/e1000/meson.build | 1 +
> 6 files changed, 46 insertions(+), 12 deletions(-) create mode 100644
> drivers/net/e1000/e1000_logs.c
>
> diff --git a/drivers/net/e1000/Makefile b/drivers/net/e1000/Makefile index
> 87cfb26..9c87e88 100644
> --- a/drivers/net/e1000/Makefile
> +++ b/drivers/net/e1000/Makefile
> @@ -62,6 +62,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) +=
> e1000_82575.c
> SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_i210.c
> SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_api.c
> SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_ich8lan.c
> +SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_logs.c
> SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_mac.c
> SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_manage.c
> SRCS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += e1000_mbx.c diff --git
> a/drivers/net/e1000/e1000_logs.c b/drivers/net/e1000/e1000_logs.c new file
> mode 100644 index 0000000..2217393
> --- /dev/null
> +++ b/drivers/net/e1000/e1000_logs.c
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Intel Corporation
> + */
> +
> +#include "e1000_logs.h"
> +
> +/* declared as extern in e1000_logs.h */ int e1000_logtype_init; int
> +e1000_logtype_driver;
> +
> +/* avoids double registering of logs if EM and IGB drivers are in use
> +*/ static int e1000_log_initialized;
> +
> +void
> +e1000_igb_init_log(void)
> +{
> + if (!e1000_log_initialized) {
> + e1000_logtype_init = rte_log_register("pmd.net.e1000.init");
> + if (e1000_logtype_init >= 0)
> + rte_log_set_level(e1000_logtype_init,
> RTE_LOG_NOTICE);
> + e1000_logtype_driver =
> rte_log_register("pmd.net.e1000.driver");
> + if (e1000_logtype_driver >= 0)
> + rte_log_set_level(e1000_logtype_driver,
> RTE_LOG_NOTICE);
> + e1000_log_initialized = 1;
> + }
> +}
> diff --git a/drivers/net/e1000/e1000_logs.h b/drivers/net/e1000/e1000_logs.h
> index 50348e9..69d3d31 100644
> --- a/drivers/net/e1000/e1000_logs.h
> +++ b/drivers/net/e1000/e1000_logs.h
> @@ -5,6 +5,8 @@
> #ifndef _E1000_LOGS_H_
> #define _E1000_LOGS_H_
>
> +#include <rte_log.h>
> +
> extern int e1000_logtype_init;
> #define PMD_INIT_LOG(level, fmt, args...) \
> rte_log(RTE_LOG_ ## level, e1000_logtype_init, \ @@ -41,4 +43,8 @@
> extern int e1000_logtype_driver; #define PMD_DRV_LOG(level, fmt, args...) \
> PMD_DRV_LOG_RAW(level, fmt "\n", ## args)
>
> +
> +/* log init function shared by e1000 and igb drivers */ void
> +e1000_igb_init_log(void);
> +
> #endif /* _E1000_LOGS_H_ */
> diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
> index 4e890ad..7039dc1 100644
> --- a/drivers/net/e1000/em_ethdev.c
> +++ b/drivers/net/e1000/em_ethdev.c
> @@ -11,7 +11,6 @@
> #include <rte_common.h>
> #include <rte_interrupts.h>
> #include <rte_byteorder.h>
> -#include <rte_log.h>
> #include <rte_debug.h>
> #include <rte_pci.h>
> #include <rte_bus_pci.h>
> @@ -106,9 +105,6 @@ static int eth_em_set_mc_addr_list(struct rte_eth_dev
> *dev,
>
> static enum e1000_fc_mode em_fc_setting = e1000_fc_full;
>
> -int e1000_logtype_init;
> -int e1000_logtype_driver;
> -
> /*
> * The set of PCI devices this driver supports
> */
> @@ -1826,14 +1822,10 @@ RTE_PMD_REGISTER_PCI(net_e1000_em,
> rte_em_pmd); RTE_PMD_REGISTER_PCI_TABLE(net_e1000_em,
> pci_id_em_map); RTE_PMD_REGISTER_KMOD_DEP(net_e1000_em, "* igb_uio
> | uio_pci_generic | vfio-pci");
>
> -RTE_INIT(e1000_init_log);
> +/* see e1000_logs.c */
> +RTE_INIT(igb_init_log);
> static void
> -e1000_init_log(void)
> +igb_init_log(void)
> {
> - e1000_logtype_init = rte_log_register("pmd.net.e1000.init");
> - if (e1000_logtype_init >= 0)
> - rte_log_set_level(e1000_logtype_init, RTE_LOG_NOTICE);
> - e1000_logtype_driver = rte_log_register("pmd.net.e1000.driver");
> - if (e1000_logtype_driver >= 0)
> - rte_log_set_level(e1000_logtype_driver, RTE_LOG_NOTICE);
> + e1000_igb_init_log();
> }
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
> index 04d34bd..edc7be3 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -5681,3 +5681,11 @@ RTE_PMD_REGISTER_KMOD_DEP(net_e1000_igb,
> "* igb_uio | uio_pci_generic | vfio-pci
> RTE_PMD_REGISTER_PCI(net_e1000_igb_vf, rte_igbvf_pmd);
> RTE_PMD_REGISTER_PCI_TABLE(net_e1000_igb_vf, pci_id_igbvf_map);
> RTE_PMD_REGISTER_KMOD_DEP(net_e1000_igb_vf, "* igb_uio | vfio-pci");
> +
> +/* see e1000_logs.c */
> +RTE_INIT(e1000_init_log);
> +static void
> +e1000_init_log(void)
> +{
> + e1000_igb_init_log();
> +}
> diff --git a/drivers/net/e1000/meson.build b/drivers/net/e1000/meson.build
> index 3a1bf5a..cf45699 100644
> --- a/drivers/net/e1000/meson.build
> +++ b/drivers/net/e1000/meson.build
> @@ -5,6 +5,7 @@ subdir('base')
> objs = [base_objs]
>
> sources = files(
> + 'e1000_logs.c',
> 'em_ethdev.c',
> 'em_rxtx.c',
> 'igb_ethdev.c',
> --
> 2.7.4
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH] net/e1000 igb: fix compile issue with log register
2018-05-21 10:10 ` Varghese, Vipin
@ 2018-05-22 2:03 ` Zhang, Helin
0 siblings, 0 replies; 3+ messages in thread
From: Zhang, Helin @ 2018-05-22 2:03 UTC (permalink / raw)
To: Varghese, Vipin, Van Haaren, Harry, dev; +Cc: Lu, Wenzhuo, Zhang, Qi Z
> -----Original Message-----
> From: Varghese, Vipin
> Sent: Monday, May 21, 2018 6:10 PM
> To: Van Haaren, Harry; dev@dpdk.org
> Cc: Lu, Wenzhuo; Zhang, Helin
> Subject: RE: [PATCH] net/e1000 igb: fix compile issue with log register
>
>
> > -----Original Message-----
> > From: Van Haaren, Harry
> > Sent: Monday, May 21, 2018 3:02 PM
> > To: dev@dpdk.org
> > Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Varghese, Vipin
> > <vipin.varghese@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhang,
> > Helin <helin.zhang@intel.com>
> > Subject: [PATCH] net/e1000 igb: fix compile issue with log register
> >
> > This commit fixes a compilation error if EM_PMD is not defined, bug
> > IGB_PMD is. The root cause of the issue was that log init variables
> > are declared as extern in a header file, while the definition of the variables
> was in e1000_ethdev.c.
> > Hence, the definitions were not available if the e1000 PMD is disabled.
> >
> > To fix this, a new file is added e1000_logs.c, which matches the
> > e1000_logs.h header. The log variables are always compiled in, but the
> > PMD logs are only registered if a PMD is enabled in the configuration.
> > Extra checks are added in order to avoid duplicate registering.
> >
> > Fixes: ed5bbb767c3e ("net/e1000: implement dynamic logging")
> >
> > Reported-by: Vipin Varghese <vipin.varghese@intel.com>
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> Acked-by: Vipin Varghese <vipin.varghese@intel.com>
Applied to dpdk-next-net-intel, thanks!
/Helin
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-05-22 2:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-21 9:31 [dpdk-dev] [PATCH] net/e1000 igb: fix compile issue with log register Harry van Haaren
2018-05-21 10:10 ` Varghese, Vipin
2018-05-22 2:03 ` Zhang, Helin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).