[PATCH 0/3] Reduce logging level for dpaax and fslmc

[PATCH 0/3] Reduce logging level for dpaax and fslmc

[Shreyansh Jain]

Based on discussion in [1], some logs for dpaax have been reduced
to DEBUG state only. All logs which are not part of exposed API
(like dpaax_iova_table_populate), only dump DEBUG now. Other exposed
API also don't dump ERR as this library needs to be transparent. Only
WARN is used.

Patch 1:
 .) change ERR -> DEBUG
 .) Default log level is set to ERR
Patch 2:
 .) Not checking the returns from dpaax library - even if that is failed,
    the PA->VA translation would still work, albeit with old method.
Patch 3:
 .) While working on Patch 1, some logs from net/dpaa2 were also
    observed which shouldn't necessarily be ERR. Changed them to
    DEBUG.

[1] http://mails.dpdk.org/archives/dev/2018-October/116096.html

Shreyansh Jain (3):
  common/dpaax: reduce logging level
  bus/fslmc: ignore dpaax pa-va table errors
  net/dpaa2: convert logs from errors to debug

 drivers/bus/fslmc/fslmc_bus.c           |  8 +++---
 drivers/common/dpaax/dpaax_iova_table.c | 35 +++++++++++++------------
 drivers/net/dpaa2/dpaa2_ethdev.c        |  4 +--
 3 files changed, 24 insertions(+), 23 deletions(-)

-- 
2.17.1

[PATCH 1/3] common/dpaax: reduce logging level

[Shreyansh Jain]

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 drivers/common/dpaax/dpaax_iova_table.c | 35 +++++++++++++------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/common/dpaax/dpaax_iova_table.c b/drivers/common/dpaax/dpaax_iova_table.c
index 075053b1a..213f75dff 100644
--- a/drivers/common/dpaax/dpaax_iova_table.c
+++ b/drivers/common/dpaax/dpaax_iova_table.c
@@ -69,8 +69,8 @@ read_memory_node(unsigned int *count)
 
 	ret = glob(MEM_NODE_PATH_GLOB, 0, NULL, &result);
 	if (ret != 0) {
-		DPAAX_ERR("Unable to glob device-tree memory node: (%s)(%d)",
-			   MEM_NODE_PATH_GLOB, ret);
+		DPAAX_DEBUG("Unable to glob device-tree memory node: (%s)(%d)",
+			    MEM_NODE_PATH_GLOB, ret);
 		goto out;
 	}
 
@@ -78,8 +78,8 @@ read_memory_node(unsigned int *count)
 		/* Either more than one memory@<addr> node found, or none.
 		 * In either case, cannot work ahead.
 		 */
-		DPAAX_ERR("Found (%zu) entries in device-tree. Not supported!",
-			  result.gl_pathc);
+		DPAAX_DEBUG("Found (%zu) entries in device-tree. Not supported!",
+			    result.gl_pathc);
 		goto out;
 	}
 
@@ -87,28 +87,29 @@ read_memory_node(unsigned int *count)
 		    result.gl_pathv[0]);
 	fd = open(result.gl_pathv[0], O_RDONLY);
 	if (fd < 0) {
-		DPAAX_ERR("Unable to open the device-tree node: (%s)(fd=%d)",
-			  MEM_NODE_PATH_GLOB, fd);
+		DPAAX_DEBUG("Unable to open the device-tree node: (%s)(fd=%d)",
+			    MEM_NODE_PATH_GLOB, fd);
 		goto cleanup;
 	}
 
 	/* Stat to get the file size */
 	ret = fstat(fd, &statbuf);
 	if (ret != 0) {
-		DPAAX_ERR("Unable to get device-tree memory node size.");
+		DPAAX_DEBUG("Unable to get device-tree memory node size.");
 		goto cleanup;
 	}
 
 	DPAAX_DEBUG("Size of device-tree mem node: %lu", statbuf.st_size);
 	if (statbuf.st_size > MEM_NODE_FILE_LEN) {
-		DPAAX_WARN("More memory nodes available than assumed.");
-		DPAAX_WARN("System may not work properly!");
+		DPAAX_DEBUG("More memory nodes available than assumed.");
+		DPAAX_DEBUG("System may not work properly!");
 	}
 
 	ret = read(fd, file_data, statbuf.st_size > MEM_NODE_FILE_LEN ?
 				  MEM_NODE_FILE_LEN : statbuf.st_size);
 	if (ret <= 0) {
-		DPAAX_ERR("Unable to read device-tree memory node: (%d)", ret);
+		DPAAX_DEBUG("Unable to read device-tree memory node: (%d)",
+			    ret);
 		goto cleanup;
 	}
 
@@ -117,15 +118,15 @@ read_memory_node(unsigned int *count)
 	 */
 	*count = (statbuf.st_size / 16);
 	if ((*count) <= 0 || (statbuf.st_size % 16 != 0)) {
-		DPAAX_ERR("Invalid memory node values or count. (size=%lu)",
-			  statbuf.st_size);
+		DPAAX_DEBUG("Invalid memory node values or count. (size=%lu)",
+			    statbuf.st_size);
 		goto cleanup;
 	}
 
 	/* each entry is of 16 bytes, and size/16 is total count of entries */
 	nodes = malloc(sizeof(struct reg_node) * (*count));
 	if (!nodes) {
-		DPAAX_ERR("Failure in allocating working memory.");
+		DPAAX_DEBUG("Failure in allocating working memory.");
 		goto cleanup;
 	}
 	memset(nodes, 0, sizeof(struct reg_node) * (*count));
@@ -420,9 +421,9 @@ dpaax_memevent_cb(enum rte_mem_event type, const void *addr, size_t len,
 			ret = dpaax_iova_table_update(phys_addr, 0, map_len);
 
 		if (ret != 0) {
-			DPAAX_ERR("PA-Table entry update failed. "
-				  "Map=%d, addr=%p, len=%zu, err:(%d)",
-				  type, va, map_len, ret);
+			DPAAX_DEBUG("PA-Table entry update failed. "
+				    "Map=%d, addr=%p, len=%zu, err:(%d)",
+				    type, va, map_len, ret);
 			return;
 		}
 
@@ -460,5 +461,5 @@ RTE_INIT(dpaax_log)
 {
 	dpaax_logger = rte_log_register("pmd.common.dpaax");
 	if (dpaax_logger >= 0)
-		rte_log_set_level(dpaax_logger, RTE_LOG_NOTICE);
+		rte_log_set_level(dpaax_logger, RTE_LOG_ERR);
 }
-- 
2.17.1

[PATCH 2/3] bus/fslmc: ignore dpaax pa-va table errors

[Shreyansh Jain]

Presence of PA-VA Table is transparent to the drivers. Ignoring the
return values from table update call.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 drivers/bus/fslmc/fslmc_bus.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 5ba5ce96b..db3026f4e 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -386,11 +386,11 @@ rte_fslmc_probe(void)
 	 * This has to be done before probe as some device initialization
 	 * (during) probe allocate memory (dpaa2_sec) which needs to be pinned
 	 * to this table.
+	 *
+	 * Error is ignored as relevant logs are handled within dpaax and
+	 * handling for unavailable dpaax table too is transparent to caller.
 	 */
-	ret = dpaax_iova_table_populate();
-	if (ret) {
-		DPAA2_BUS_WARN("PA->VA Translation table not available;");
-	}
+	dpaax_iova_table_populate();
 
 	TAILQ_FOREACH(dev, &rte_fslmc_bus.device_list, next) {
 		TAILQ_FOREACH(drv, &rte_fslmc_bus.driver_list, next) {
-- 
2.17.1

[PATCH 3/3] net/dpaa2: convert logs from errors to debug

[Shreyansh Jain]

In case the link is down during initial link state check, messages for
link state check flood the console. Reducing the log level for these.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 drivers/net/dpaa2/dpaa2_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index 3987d13df..d8b7fa186 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -1412,7 +1412,7 @@ dpaa2_dev_link_update(struct rte_eth_dev *dev,
 
 	ret = dpni_get_link_state(dpni, CMD_PRI_LOW, priv->token, &state);
 	if (ret < 0) {
-		DPAA2_PMD_ERR("error: dpni_get_link_state %d", ret);
+		DPAA2_PMD_DEBUG("error: dpni_get_link_state %d", ret);
 		return -1;
 	}
 
@@ -1474,7 +1474,7 @@ dpaa2_dev_set_link_up(struct rte_eth_dev *dev)
 	}
 	ret = dpni_get_link_state(dpni, CMD_PRI_LOW, priv->token, &state);
 	if (ret < 0) {
-		DPAA2_PMD_ERR("Unable to get link state (%d)", ret);
+		DPAA2_PMD_DEBUG("Unable to get link state (%d)", ret);
 		return -1;
 	}
 
-- 
2.17.1

Re: [PATCH 1/3] common/dpaax: reduce logging level

[Thomas Monjalon]

17/10/2018 11:12, Shreyansh Jain:
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>

Please add a description explaining why you do this change.

[PATCH v2 0/3] Reduce logging level for dpaax and fslmc

[Shreyansh Jain]

Based on discussion in [1], some logs for dpaax have been reduced
to DEBUG state only. All logs which are not part of exposed API
(like dpaax_iova_table_populate), only dump DEBUG now. Other exposed
API also don't dump ERR as this library needs to be transparent. Only
WARN is used.

Patch 1:
 .) change ERR -> DEBUG
 .) Default log level is set to ERR
Patch 2:
 .) Not checking the returns from dpaax library - even if that is failed,
    the PA->VA translation would still work, albeit with old method.
Patch 3:
 .) While working on Patch 1, some logs from net/dpaa2 were also
    observed which shouldn't necessarily be ERR. Changed them to
    DEBUG.

[1] http://mails.dpdk.org/archives/dev/2018-October/116096.html

Version History:
v2:
 - Added patch description in patch 1

Shreyansh Jain (3):
  common/dpaax: reduce logging level
  bus/fslmc: ignore dpaax pa-va table errors
  net/dpaa2: convert logs from errors to debug

 drivers/bus/fslmc/fslmc_bus.c           |  8 +++---
 drivers/common/dpaax/dpaax_iova_table.c | 35 +++++++++++++------------
 drivers/net/dpaa2/dpaa2_ethdev.c        |  4 +--
 3 files changed, 24 insertions(+), 23 deletions(-)

-- 
2.17.1

[PATCH v2 1/3] common/dpaax: reduce logging level

[Shreyansh Jain]

DPAAX is a library used by various NXP drivers. In case of non-NXP
environment, this start spewing message about unavailability of
necessary environment.

This patch reduces the log level for certain messages as well as
reduces overall log-level. As a library, these message are not
necessarily relevant at higher log level, either.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 drivers/common/dpaax/dpaax_iova_table.c | 35 +++++++++++++------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/common/dpaax/dpaax_iova_table.c b/drivers/common/dpaax/dpaax_iova_table.c
index 075053b1a..213f75dff 100644
--- a/drivers/common/dpaax/dpaax_iova_table.c
+++ b/drivers/common/dpaax/dpaax_iova_table.c
@@ -69,8 +69,8 @@ read_memory_node(unsigned int *count)
 
 	ret = glob(MEM_NODE_PATH_GLOB, 0, NULL, &result);
 	if (ret != 0) {
-		DPAAX_ERR("Unable to glob device-tree memory node: (%s)(%d)",
-			   MEM_NODE_PATH_GLOB, ret);
+		DPAAX_DEBUG("Unable to glob device-tree memory node: (%s)(%d)",
+			    MEM_NODE_PATH_GLOB, ret);
 		goto out;
 	}
 
@@ -78,8 +78,8 @@ read_memory_node(unsigned int *count)
 		/* Either more than one memory@<addr> node found, or none.
 		 * In either case, cannot work ahead.
 		 */
-		DPAAX_ERR("Found (%zu) entries in device-tree. Not supported!",
-			  result.gl_pathc);
+		DPAAX_DEBUG("Found (%zu) entries in device-tree. Not supported!",
+			    result.gl_pathc);
 		goto out;
 	}
 
@@ -87,28 +87,29 @@ read_memory_node(unsigned int *count)
 		    result.gl_pathv[0]);
 	fd = open(result.gl_pathv[0], O_RDONLY);
 	if (fd < 0) {
-		DPAAX_ERR("Unable to open the device-tree node: (%s)(fd=%d)",
-			  MEM_NODE_PATH_GLOB, fd);
+		DPAAX_DEBUG("Unable to open the device-tree node: (%s)(fd=%d)",
+			    MEM_NODE_PATH_GLOB, fd);
 		goto cleanup;
 	}
 
 	/* Stat to get the file size */
 	ret = fstat(fd, &statbuf);
 	if (ret != 0) {
-		DPAAX_ERR("Unable to get device-tree memory node size.");
+		DPAAX_DEBUG("Unable to get device-tree memory node size.");
 		goto cleanup;
 	}
 
 	DPAAX_DEBUG("Size of device-tree mem node: %lu", statbuf.st_size);
 	if (statbuf.st_size > MEM_NODE_FILE_LEN) {
-		DPAAX_WARN("More memory nodes available than assumed.");
-		DPAAX_WARN("System may not work properly!");
+		DPAAX_DEBUG("More memory nodes available than assumed.");
+		DPAAX_DEBUG("System may not work properly!");
 	}
 
 	ret = read(fd, file_data, statbuf.st_size > MEM_NODE_FILE_LEN ?
 				  MEM_NODE_FILE_LEN : statbuf.st_size);
 	if (ret <= 0) {
-		DPAAX_ERR("Unable to read device-tree memory node: (%d)", ret);
+		DPAAX_DEBUG("Unable to read device-tree memory node: (%d)",
+			    ret);
 		goto cleanup;
 	}
 
@@ -117,15 +118,15 @@ read_memory_node(unsigned int *count)
 	 */
 	*count = (statbuf.st_size / 16);
 	if ((*count) <= 0 || (statbuf.st_size % 16 != 0)) {
-		DPAAX_ERR("Invalid memory node values or count. (size=%lu)",
-			  statbuf.st_size);
+		DPAAX_DEBUG("Invalid memory node values or count. (size=%lu)",
+			    statbuf.st_size);
 		goto cleanup;
 	}
 
 	/* each entry is of 16 bytes, and size/16 is total count of entries */
 	nodes = malloc(sizeof(struct reg_node) * (*count));
 	if (!nodes) {
-		DPAAX_ERR("Failure in allocating working memory.");
+		DPAAX_DEBUG("Failure in allocating working memory.");
 		goto cleanup;
 	}
 	memset(nodes, 0, sizeof(struct reg_node) * (*count));
@@ -420,9 +421,9 @@ dpaax_memevent_cb(enum rte_mem_event type, const void *addr, size_t len,
 			ret = dpaax_iova_table_update(phys_addr, 0, map_len);
 
 		if (ret != 0) {
-			DPAAX_ERR("PA-Table entry update failed. "
-				  "Map=%d, addr=%p, len=%zu, err:(%d)",
-				  type, va, map_len, ret);
+			DPAAX_DEBUG("PA-Table entry update failed. "
+				    "Map=%d, addr=%p, len=%zu, err:(%d)",
+				    type, va, map_len, ret);
 			return;
 		}
 
@@ -460,5 +461,5 @@ RTE_INIT(dpaax_log)
 {
 	dpaax_logger = rte_log_register("pmd.common.dpaax");
 	if (dpaax_logger >= 0)
-		rte_log_set_level(dpaax_logger, RTE_LOG_NOTICE);
+		rte_log_set_level(dpaax_logger, RTE_LOG_ERR);
 }
-- 
2.17.1

[PATCH v2 2/3] bus/fslmc: ignore dpaax pa-va table errors

[Shreyansh Jain]

Presence of PA-VA Table is transparent to the drivers. Ignoring the
return values from table update call.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 drivers/bus/fslmc/fslmc_bus.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index 5ba5ce96b..db3026f4e 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -386,11 +386,11 @@ rte_fslmc_probe(void)
 	 * This has to be done before probe as some device initialization
 	 * (during) probe allocate memory (dpaa2_sec) which needs to be pinned
 	 * to this table.
+	 *
+	 * Error is ignored as relevant logs are handled within dpaax and
+	 * handling for unavailable dpaax table too is transparent to caller.
 	 */
-	ret = dpaax_iova_table_populate();
-	if (ret) {
-		DPAA2_BUS_WARN("PA->VA Translation table not available;");
-	}
+	dpaax_iova_table_populate();
 
 	TAILQ_FOREACH(dev, &rte_fslmc_bus.device_list, next) {
 		TAILQ_FOREACH(drv, &rte_fslmc_bus.driver_list, next) {
-- 
2.17.1

[PATCH v2 3/3] net/dpaa2: convert logs from errors to debug

[Shreyansh Jain]

In case the link is down during initial link state check, messages for
link state check flood the console. Reducing the log level for these.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 drivers/net/dpaa2/dpaa2_ethdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index 3987d13df..d8b7fa186 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -1412,7 +1412,7 @@ dpaa2_dev_link_update(struct rte_eth_dev *dev,
 
 	ret = dpni_get_link_state(dpni, CMD_PRI_LOW, priv->token, &state);
 	if (ret < 0) {
-		DPAA2_PMD_ERR("error: dpni_get_link_state %d", ret);
+		DPAA2_PMD_DEBUG("error: dpni_get_link_state %d", ret);
 		return -1;
 	}
 
@@ -1474,7 +1474,7 @@ dpaa2_dev_set_link_up(struct rte_eth_dev *dev)
 	}
 	ret = dpni_get_link_state(dpni, CMD_PRI_LOW, priv->token, &state);
 	if (ret < 0) {
-		DPAA2_PMD_ERR("Unable to get link state (%d)", ret);
+		DPAA2_PMD_DEBUG("Unable to get link state (%d)", ret);
 		return -1;
 	}
 
-- 
2.17.1

Re: [PATCH v2 1/3] common/dpaax: reduce logging level

[Jerin Jacob]

-----Original Message-----
> Date: Wed, 17 Oct 2018 10:10:34 +0000
> From: Shreyansh Jain <shreyansh.jain@nxp.com>
> To: "thomas@monjalon.net" <thomas@monjalon.net>
> CC: "dev@dpdk.org" <dev@dpdk.org>, Shreyansh Jain <shreyansh.jain@nxp.com>
> Subject: [dpdk-dev] [PATCH v2 1/3] common/dpaax: reduce logging level
> x-mailer: git-send-email 2.17.1
> 
> 
> DPAAX is a library used by various NXP drivers. In case of non-NXP
> environment, this start spewing message about unavailability of
> necessary environment.
> 
> This patch reduces the log level for certain messages as well as
> reduces overall log-level. As a library, these message are not
> necessarily relevant at higher log level, either.
> 
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>

Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Test-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

Re: [PATCH v2 2/3] bus/fslmc: ignore dpaax pa-va table errors

[Jerin Jacob]

-----Original Message-----
> Date: Wed, 17 Oct 2018 10:10:36 +0000
> From: Shreyansh Jain <shreyansh.jain@nxp.com>
> To: "thomas@monjalon.net" <thomas@monjalon.net>
> CC: "dev@dpdk.org" <dev@dpdk.org>, Shreyansh Jain <shreyansh.jain@nxp.com>
> Subject: [dpdk-dev] [PATCH v2 2/3] bus/fslmc: ignore dpaax pa-va table
>  errors
> x-mailer: git-send-email 2.17.1
> 
> 
> Presence of PA-VA Table is transparent to the drivers. Ignoring the
> return values from table update call.
> 
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>

Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

> ---
>  drivers/bus/fslmc/fslmc_bus.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
> index 5ba5ce96b..db3026f4e 100644
> --- a/drivers/bus/fslmc/fslmc_bus.c
> +++ b/drivers/bus/fslmc/fslmc_bus.c
> @@ -386,11 +386,11 @@ rte_fslmc_probe(void)
>          * This has to be done before probe as some device initialization
>          * (during) probe allocate memory (dpaa2_sec) which needs to be pinned
>          * to this table.
> +        *
> +        * Error is ignored as relevant logs are handled within dpaax and
> +        * handling for unavailable dpaax table too is transparent to caller.
>          */
> -       ret = dpaax_iova_table_populate();
> -       if (ret) {
> -               DPAA2_BUS_WARN("PA->VA Translation table not available;");
> -       }
> +       dpaax_iova_table_populate();
> 
>         TAILQ_FOREACH(dev, &rte_fslmc_bus.device_list, next) {
>                 TAILQ_FOREACH(drv, &rte_fslmc_bus.driver_list, next) {
> --
> 2.17.1
> 

Re: [PATCH v2 0/3] Reduce logging level for dpaax and fslmc

[Thomas Monjalon]

17/10/2018 12:10, Shreyansh Jain:
> Shreyansh Jain (3):
>   common/dpaax: reduce logging level
>   bus/fslmc: ignore dpaax pa-va table errors
>   net/dpaa2: convert logs from errors to debug

Applied, thanks