[dpdk-dev,v2,12/17] e1000: use the right debug macro

Message ID 1409567080-27083-13-git-send-email-david.marchand@6wind.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

David Marchand Sept. 1, 2014, 10:24 a.m. UTC
  - We should not use DEBUGOUT* / DEBUGFUNC macros in non-shared code.
These macros come as compat wrappers for shared code.
- We should avoid calling RTE_LOG directly as pmd provides a wrapper for logs.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_pmd_e1000/em_rxtx.c    |   32 ++++++++++++++++++--------------
 lib/librte_pmd_e1000/igb_ethdev.c |    9 +++++----
 lib/librte_pmd_e1000/igb_pf.c     |    5 +++--
 lib/librte_pmd_e1000/igb_rxtx.c   |   16 +++++++---------
 4 files changed, 33 insertions(+), 29 deletions(-)
  

Comments

Jay Rolette Sept. 2, 2014, 6:29 p.m. UTC | #1
On Mon, Sep 1, 2014 at 5:24 AM, David Marchand <david.marchand@6wind.com>
wrote:

> - We should not use DEBUGOUT* / DEBUGFUNC macros in non-shared code.
> These macros come as compat wrappers for shared code.
> - We should avoid calling RTE_LOG directly as pmd provides a wrapper for
> logs.
>
> Signed-off-by: David Marchand <david.marchand@6wind.com>
> ---
>  lib/librte_pmd_e1000/em_rxtx.c    |   32 ++++++++++++++++++--------------
>  lib/librte_pmd_e1000/igb_ethdev.c |    9 +++++----
>  lib/librte_pmd_e1000/igb_pf.c     |    5 +++--
>  lib/librte_pmd_e1000/igb_rxtx.c   |   16 +++++++---------
>  4 files changed, 33 insertions(+), 29 deletions(-)
>
> diff --git a/lib/librte_pmd_e1000/em_rxtx.c
> b/lib/librte_pmd_e1000/em_rxtx.c
> index f254858..01efa50 100644
> --- a/lib/librte_pmd_e1000/em_rxtx.c
> +++ b/lib/librte_pmd_e1000/em_rxtx.c
> @@ -1211,18 +1211,21 @@ eth_em_tx_queue_setup(struct rte_eth_dev *dev,
>                                         DEFAULT_TX_RS_THRESH);
>
>         if (tx_free_thresh >= (nb_desc - 3)) {
> -               RTE_LOG(ERR, PMD, "tx_free_thresh must be less than the "
> -                       "number of TX descriptors minus 3.
> (tx_free_thresh=%u "
> -                       "port=%d queue=%d)\n", (unsigned
> int)tx_free_thresh,
> -                               (int)dev->data->port_id, (int)queue_idx);
> +               PMD_INIT_LOG(ERR, "tx_free_thresh must be less than the "
> +                            "number of TX descriptors minus 3. "
> +                            "(tx_free_thresh=%u port=%d queue=%d)\n",
> +                            (unsigned int)tx_free_thresh,
> +                            (int)dev->data->port_id, (int)queue_idx);
>                 return -(EINVAL);
>         }
>         if (tx_rs_thresh > tx_free_thresh) {
> -               RTE_LOG(ERR, PMD, "tx_rs_thresh must be less than or equal
> to "
> -                       "tx_free_thresh. (tx_free_thresh=%u
> tx_rs_thresh=%u "
> -                       "port=%d queue=%d)\n", (unsigned
> int)tx_free_thresh,
> -                       (unsigned int)tx_rs_thresh,
> (int)dev->data->port_id,
> -                                                       (int)queue_idx);
> +               PMD_INIT_LOG(ERR, "tx_rs_thresh must be less than or equal
> to "
> +                            "tx_free_thresh. (tx_free_thresh=%u "
> +                            "tx_rs_thresh=%u port=%d queue=%d)\n",
> +                            (unsigned int)tx_free_thresh,
> +                            (unsigned int)tx_rs_thresh,
> +                            (int)dev->data->port_id,
> +                            (int)queue_idx);
>                 return -(EINVAL);
>         }
>
> @@ -1233,10 +1236,10 @@ eth_em_tx_queue_setup(struct rte_eth_dev *dev,
>          * accumulates WTHRESH descriptors.
>          */
>         if (tx_conf->tx_thresh.wthresh != 0 && tx_rs_thresh != 1) {
> -               RTE_LOG(ERR, PMD, "TX WTHRESH must be set to 0 if "
> -                       "tx_rs_thresh is greater than 1. (tx_rs_thresh=%u "
> -                       "port=%d queue=%d)\n", (unsigned int)tx_rs_thresh,
> -                               (int)dev->data->port_id, (int)queue_idx);
> +               PMD_INIT_LOG(ERR, "TX WTHRESH must be set to 0 if "
> +                            "tx_rs_thresh is greater than 1.
> (tx_rs_thresh=%u "
> +                            "port=%d queue=%d)\n", (unsigned
> int)tx_rs_thresh,
> +                            (int)dev->data->port_id, (int)queue_idx);
>                 return -(EINVAL);
>         }
>
> @@ -1366,7 +1369,8 @@ eth_em_rx_queue_setup(struct rte_eth_dev *dev,
>          * EM devices don't support drop_en functionality
>          */
>         if (rx_conf->rx_drop_en) {
> -               RTE_LOG(ERR, PMD, "drop_en functionality not supported by
> device\n");
> +               PMD_INIT_LOG(ERR, "drop_en functionality not supported by "
> +                            "device\n");
>                 return (-EINVAL);
>         }
>
> diff --git a/lib/librte_pmd_e1000/igb_ethdev.c
> b/lib/librte_pmd_e1000/igb_ethdev.c
> index 3187d92..b45eb24 100644
> --- a/lib/librte_pmd_e1000/igb_ethdev.c
> +++ b/lib/librte_pmd_e1000/igb_ethdev.c
> @@ -400,7 +400,7 @@ igb_reset_swfw_lock(struct e1000_hw *hw)
>          * So force the release of the faulty lock.
>          */
>         if (e1000_get_hw_semaphore_generic(hw) < 0) {
> -               DEBUGOUT("SMBI lock released");
> +               PMD_DRV_LOG(DEBUG, "SMBI lock released");
>         }
>         e1000_put_hw_semaphore_generic(hw);
>
> @@ -416,7 +416,8 @@ igb_reset_swfw_lock(struct e1000_hw *hw)
>                 if (hw->bus.func > E1000_FUNC_1)
>                         mask <<= 2;
>                 if (hw->mac.ops.acquire_swfw_sync(hw, mask) < 0) {
> -                       DEBUGOUT1("SWFW phy%d lock released",
> hw->bus.func);
> +                       PMD_DRV_LOG(DEBUG, "SWFW phy%d lock released",
> +                                   hw->bus.func);
>                 }
>                 hw->mac.ops.release_swfw_sync(hw, mask);
>
> @@ -428,7 +429,7 @@ igb_reset_swfw_lock(struct e1000_hw *hw)
>                  */
>                 mask = E1000_SWFW_EEP_SM;
>                 if (hw->mac.ops.acquire_swfw_sync(hw, mask) < 0) {
> -                       DEBUGOUT("SWFW common locks released");
> +                       PMD_DRV_LOG(DEBUG, "SWFW common locks released");
>                 }
>                 hw->mac.ops.release_swfw_sync(hw, mask);
>         }
> @@ -707,7 +708,7 @@ igb_vmdq_vlan_hw_filter_enable(struct rte_eth_dev *dev)
>  static int
>  rte_igbvf_pmd_init(const char *name __rte_unused, const char *params
> __rte_unused)
>  {
> -       DEBUGFUNC("rte_igbvf_pmd_init");
> +       PMD_INIT_FUNC_TRACE();
>
>         rte_eth_driver_register(&rte_igbvf_pmd);
>         return (0);
> diff --git a/lib/librte_pmd_e1000/igb_pf.c b/lib/librte_pmd_e1000/igb_pf.c
> index 3d405f0..76033ad 100644
> --- a/lib/librte_pmd_e1000/igb_pf.c
> +++ b/lib/librte_pmd_e1000/igb_pf.c
> @@ -404,7 +404,7 @@ igb_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t
> vf)
>
>         retval = e1000_read_mbx(hw, msgbuf, mbx_size, vf);
>         if (retval) {
> -               RTE_LOG(ERR, PMD, "Error mbx recv msg from VF %d\n", vf);
> +               PMD_INIT_LOG(ERR, "Error mbx recv msg from VF %d\n", vf);
>                 return retval;
>         }
>
> @@ -432,7 +432,8 @@ igb_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t
> vf)
>                 retval = igb_vf_set_vlan(dev, vf, msgbuf);
>                 break;
>         default:
> -               RTE_LOG(DEBUG, PMD, "Unhandled Msg %8.8x\n", (unsigned)
> msgbuf[0]);
> +               PMD_INIT_LOG(DEBUG, "Unhandled Msg %8.8x\n",
> +                            (unsigned) msgbuf[0]);
>                 retval = E1000_ERR_MBX;
>                 break;
>         }
> diff --git a/lib/librte_pmd_e1000/igb_rxtx.c
> b/lib/librte_pmd_e1000/igb_rxtx.c
> index 977c4a2..3aa9609 100644
> --- a/lib/librte_pmd_e1000/igb_rxtx.c
> +++ b/lib/librte_pmd_e1000/igb_rxtx.c
> @@ -1210,17 +1210,15 @@ eth_igb_tx_queue_setup(struct rte_eth_dev *dev,
>          * driver.
>          */
>         if (tx_conf->tx_free_thresh != 0)
> -               RTE_LOG(WARNING, PMD,
> -                       "The tx_free_thresh parameter is not "
> -                       "used for the 1G driver.\n");
> +               PMD_INIT_LOG(WARNING, "The tx_free_thresh parameter is not
> "
> +                            "used for the 1G driver.\n");
>         if (tx_conf->tx_rs_thresh != 0)
> -               RTE_LOG(WARNING, PMD,
> -                       "The tx_rs_thresh parameter is not "
> -                       "used for the 1G driver.\n");
> +               PMD_INIT_LOG(WARNING, "The tx_rs_thresh parameter is not "
> +                            "used for the 1G driver.\n");
>         if (tx_conf->tx_thresh.wthresh == 0)
> -               RTE_LOG(WARNING, PMD,
> -                       "To improve 1G driver performance, consider
> setting "
> -                       "the TX WTHRESH value to 4, 8, or 16.\n");
> +               PMD_INIT_LOG(WARNING, "To improve 1G driver performance, "
> +                            "consider setting the TX WTHRESH value to 4,
> 8, "
> +                            "or 16.\n");
>
>         /* Free memory prior to re-allocation if needed */
>         if (dev->data->tx_queues[queue_idx] != NULL) {
> --
> 1.7.10.4
>
> Reviewed-by: Jay Rolette <rolette@infiniteio.com>
  

Patch

diff --git a/lib/librte_pmd_e1000/em_rxtx.c b/lib/librte_pmd_e1000/em_rxtx.c
index f254858..01efa50 100644
--- a/lib/librte_pmd_e1000/em_rxtx.c
+++ b/lib/librte_pmd_e1000/em_rxtx.c
@@ -1211,18 +1211,21 @@  eth_em_tx_queue_setup(struct rte_eth_dev *dev,
 					DEFAULT_TX_RS_THRESH);
 
 	if (tx_free_thresh >= (nb_desc - 3)) {
-		RTE_LOG(ERR, PMD, "tx_free_thresh must be less than the "
-			"number of TX descriptors minus 3. (tx_free_thresh=%u "
-			"port=%d queue=%d)\n", (unsigned int)tx_free_thresh,
-				(int)dev->data->port_id, (int)queue_idx);
+		PMD_INIT_LOG(ERR, "tx_free_thresh must be less than the "
+			     "number of TX descriptors minus 3. "
+			     "(tx_free_thresh=%u port=%d queue=%d)\n",
+			     (unsigned int)tx_free_thresh,
+			     (int)dev->data->port_id, (int)queue_idx);
 		return -(EINVAL);
 	}
 	if (tx_rs_thresh > tx_free_thresh) {
-		RTE_LOG(ERR, PMD, "tx_rs_thresh must be less than or equal to "
-			"tx_free_thresh. (tx_free_thresh=%u tx_rs_thresh=%u "
-			"port=%d queue=%d)\n", (unsigned int)tx_free_thresh,
-			(unsigned int)tx_rs_thresh, (int)dev->data->port_id,
-							(int)queue_idx);
+		PMD_INIT_LOG(ERR, "tx_rs_thresh must be less than or equal to "
+			     "tx_free_thresh. (tx_free_thresh=%u "
+			     "tx_rs_thresh=%u port=%d queue=%d)\n",
+			     (unsigned int)tx_free_thresh,
+			     (unsigned int)tx_rs_thresh,
+			     (int)dev->data->port_id,
+			     (int)queue_idx);
 		return -(EINVAL);
 	}
 
@@ -1233,10 +1236,10 @@  eth_em_tx_queue_setup(struct rte_eth_dev *dev,
 	 * accumulates WTHRESH descriptors.
 	 */
 	if (tx_conf->tx_thresh.wthresh != 0 && tx_rs_thresh != 1) {
-		RTE_LOG(ERR, PMD, "TX WTHRESH must be set to 0 if "
-			"tx_rs_thresh is greater than 1. (tx_rs_thresh=%u "
-			"port=%d queue=%d)\n", (unsigned int)tx_rs_thresh,
-				(int)dev->data->port_id, (int)queue_idx);
+		PMD_INIT_LOG(ERR, "TX WTHRESH must be set to 0 if "
+			     "tx_rs_thresh is greater than 1. (tx_rs_thresh=%u "
+			     "port=%d queue=%d)\n", (unsigned int)tx_rs_thresh,
+			     (int)dev->data->port_id, (int)queue_idx);
 		return -(EINVAL);
 	}
 
@@ -1366,7 +1369,8 @@  eth_em_rx_queue_setup(struct rte_eth_dev *dev,
 	 * EM devices don't support drop_en functionality
 	 */
 	if (rx_conf->rx_drop_en) {
-		RTE_LOG(ERR, PMD, "drop_en functionality not supported by device\n");
+		PMD_INIT_LOG(ERR, "drop_en functionality not supported by "
+			     "device\n");
 		return (-EINVAL);
 	}
 
diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c
index 3187d92..b45eb24 100644
--- a/lib/librte_pmd_e1000/igb_ethdev.c
+++ b/lib/librte_pmd_e1000/igb_ethdev.c
@@ -400,7 +400,7 @@  igb_reset_swfw_lock(struct e1000_hw *hw)
 	 * So force the release of the faulty lock.
 	 */
 	if (e1000_get_hw_semaphore_generic(hw) < 0) {
-		DEBUGOUT("SMBI lock released");
+		PMD_DRV_LOG(DEBUG, "SMBI lock released");
 	}
 	e1000_put_hw_semaphore_generic(hw);
 
@@ -416,7 +416,8 @@  igb_reset_swfw_lock(struct e1000_hw *hw)
 		if (hw->bus.func > E1000_FUNC_1)
 			mask <<= 2;
 		if (hw->mac.ops.acquire_swfw_sync(hw, mask) < 0) {
-			DEBUGOUT1("SWFW phy%d lock released", hw->bus.func);
+			PMD_DRV_LOG(DEBUG, "SWFW phy%d lock released",
+				    hw->bus.func);
 		}
 		hw->mac.ops.release_swfw_sync(hw, mask);
 
@@ -428,7 +429,7 @@  igb_reset_swfw_lock(struct e1000_hw *hw)
 		 */
 		mask = E1000_SWFW_EEP_SM;
 		if (hw->mac.ops.acquire_swfw_sync(hw, mask) < 0) {
-			DEBUGOUT("SWFW common locks released");
+			PMD_DRV_LOG(DEBUG, "SWFW common locks released");
 		}
 		hw->mac.ops.release_swfw_sync(hw, mask);
 	}
@@ -707,7 +708,7 @@  igb_vmdq_vlan_hw_filter_enable(struct rte_eth_dev *dev)
 static int
 rte_igbvf_pmd_init(const char *name __rte_unused, const char *params __rte_unused)
 {
-	DEBUGFUNC("rte_igbvf_pmd_init");
+	PMD_INIT_FUNC_TRACE();
 
 	rte_eth_driver_register(&rte_igbvf_pmd);
 	return (0);
diff --git a/lib/librte_pmd_e1000/igb_pf.c b/lib/librte_pmd_e1000/igb_pf.c
index 3d405f0..76033ad 100644
--- a/lib/librte_pmd_e1000/igb_pf.c
+++ b/lib/librte_pmd_e1000/igb_pf.c
@@ -404,7 +404,7 @@  igb_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t vf)
 
 	retval = e1000_read_mbx(hw, msgbuf, mbx_size, vf);
 	if (retval) {
-		RTE_LOG(ERR, PMD, "Error mbx recv msg from VF %d\n", vf);
+		PMD_INIT_LOG(ERR, "Error mbx recv msg from VF %d\n", vf);
 		return retval;
 	}
 
@@ -432,7 +432,8 @@  igb_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t vf)
 		retval = igb_vf_set_vlan(dev, vf, msgbuf);
 		break;
 	default:
-		RTE_LOG(DEBUG, PMD, "Unhandled Msg %8.8x\n", (unsigned) msgbuf[0]);
+		PMD_INIT_LOG(DEBUG, "Unhandled Msg %8.8x\n",
+			     (unsigned) msgbuf[0]);
 		retval = E1000_ERR_MBX;
 		break;
 	}
diff --git a/lib/librte_pmd_e1000/igb_rxtx.c b/lib/librte_pmd_e1000/igb_rxtx.c
index 977c4a2..3aa9609 100644
--- a/lib/librte_pmd_e1000/igb_rxtx.c
+++ b/lib/librte_pmd_e1000/igb_rxtx.c
@@ -1210,17 +1210,15 @@  eth_igb_tx_queue_setup(struct rte_eth_dev *dev,
 	 * driver.
 	 */
 	if (tx_conf->tx_free_thresh != 0)
-		RTE_LOG(WARNING, PMD,
-			"The tx_free_thresh parameter is not "
-			"used for the 1G driver.\n");
+		PMD_INIT_LOG(WARNING, "The tx_free_thresh parameter is not "
+			     "used for the 1G driver.\n");
 	if (tx_conf->tx_rs_thresh != 0)
-		RTE_LOG(WARNING, PMD,
-			"The tx_rs_thresh parameter is not "
-			"used for the 1G driver.\n");
+		PMD_INIT_LOG(WARNING, "The tx_rs_thresh parameter is not "
+			     "used for the 1G driver.\n");
 	if (tx_conf->tx_thresh.wthresh == 0)
-		RTE_LOG(WARNING, PMD,
-			"To improve 1G driver performance, consider setting "
-			"the TX WTHRESH value to 4, 8, or 16.\n");
+		PMD_INIT_LOG(WARNING, "To improve 1G driver performance, "
+			     "consider setting the TX WTHRESH value to 4, 8, "
+			     "or 16.\n");
 
 	/* Free memory prior to re-allocation if needed */
 	if (dev->data->tx_queues[queue_idx] != NULL) {