DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 00/11] cleanup logs in main PMDs
@ 2014-08-26 14:09 David Marchand
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 01/11] ixgbe: clean log messages David Marchand
                   ` (10 more replies)
  0 siblings, 11 replies; 17+ messages in thread
From: David Marchand @ 2014-08-26 14:09 UTC (permalink / raw)
  To: dev

Here is a patchset that reworks the log macro in e1000, ixgbe and i40e PMDs.
The idea behind this is to make it easier to debug some init failures and to be
sure of the datapath selected in these PMDs (rx / tx handlers selection).

The PMDs changes involve adding more debug messages in the default build.
A new eal option has been added to set the default log level, so that you can
render the eal a little less noisy.

I did not change the default log level for now, as some eal log messages are
marked as DEBUG while being interesting (from my point of view).
I suppose we can change the default log level later once the eal has been
cleaned up.


-- 
David Marchand

David Marchand (11):
  ixgbe: clean log messages
  ixgbe: always log init messages
  ixgbe: add a message when forcing scatter mode
  ixgbe: add log messages when rx bulk mode is not usable
  i40e: clean log messages
  i40e: always log init messages
  i40e: add log messages when rx bulk mode is not usable
  e1000: clean log messages
  e1000: always log init messages
  e1000: add a message when forcing scatter mode
  eal: set log level from command line

 lib/librte_eal/bsdapp/eal/eal.c                    |   43 +++++++++
 .../bsdapp/eal/include/eal_internal_cfg.h          |    1 +
 lib/librte_eal/linuxapp/eal/eal.c                  |   43 +++++++++
 .../linuxapp/eal/include/eal_internal_cfg.h        |    1 +
 lib/librte_pmd_e1000/e1000_logs.h                  |   17 ++--
 lib/librte_pmd_e1000/em_ethdev.c                   |   47 +++++-----
 lib/librte_pmd_e1000/em_rxtx.c                     |   39 ++++----
 lib/librte_pmd_e1000/igb_ethdev.c                  |   75 +++++++--------
 lib/librte_pmd_e1000/igb_rxtx.c                    |   23 ++++-
 lib/librte_pmd_i40e/i40e/i40e_osdep.h              |    2 +-
 lib/librte_pmd_i40e/i40e_ethdev.c                  |   43 ++++-----
 lib/librte_pmd_i40e/i40e_ethdev_vf.c               |    4 +-
 lib/librte_pmd_i40e/i40e_logs.h                    |   17 ++--
 lib/librte_pmd_i40e/i40e_pf.c                      |    4 +-
 lib/librte_pmd_i40e/i40e_rxtx.c                    |   35 +++++--
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c                |   99 ++++++++++----------
 lib/librte_pmd_ixgbe/ixgbe_fdir.c                  |   12 +--
 lib/librte_pmd_ixgbe/ixgbe_logs.h                  |   17 ++--
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c                  |   51 +++++++---
 19 files changed, 360 insertions(+), 213 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 01/11] ixgbe: clean log messages
  2014-08-26 14:09 [dpdk-dev] [PATCH 00/11] cleanup logs in main PMDs David Marchand
@ 2014-08-26 14:09 ` David Marchand
  2014-08-26 14:23   ` Jay Rolette
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 02/11] ixgbe: always log init messages David Marchand
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 17+ messages in thread
From: David Marchand @ 2014-08-26 14:09 UTC (permalink / raw)
  To: dev

Clean log messages:
- remove superfluous \n in log macros and add some \n where needed,
- remove leading \n in some messages,
- split multi lines messages,
- replace some PMD_INIT_LOG(DEBUG, "some_func\n") with PMD_INIT_FUNC_TRACE().

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c |   99 +++++++++++++++++------------------
 lib/librte_pmd_ixgbe/ixgbe_fdir.c   |   12 ++---
 lib/librte_pmd_ixgbe/ixgbe_logs.h   |   12 ++---
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c   |   14 ++---
 4 files changed, 68 insertions(+), 69 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 59122a1..ac00323 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -737,7 +737,7 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 #endif /* RTE_NIC_BYPASS */
 
 	if (diag != IXGBE_SUCCESS) {
-		PMD_INIT_LOG(ERR, "Shared code init failed: %d", diag);
+		PMD_INIT_LOG(ERR, "Shared code init failed: %d\n", diag);
 		return -EIO;
 	}
 
@@ -763,7 +763,7 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 	/* Make sure we have a good EEPROM before we read from it */
 	diag = ixgbe_validate_eeprom_checksum(hw, &csum);
 	if (diag != IXGBE_SUCCESS) {
-		PMD_INIT_LOG(ERR, "The EEPROM checksum is not valid: %d", diag);
+		PMD_INIT_LOG(ERR, "The EEPROM checksum is not valid: %d\n", diag);
 		return -EIO;
 	}
 
@@ -791,13 +791,14 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 	if (diag == IXGBE_ERR_EEPROM_VERSION) {
 		PMD_INIT_LOG(ERR, "This device is a pre-production adapter/"
 		    "LOM.  Please be aware there may be issues associated "
-		    "with your hardware.\n If you are experiencing problems "
+		    "with your hardware.\n");
+		PMD_INIT_LOG(ERR, "If you are experiencing problems "
 		    "please contact your Intel or hardware representative "
 		    "who provided you with this hardware.\n");
 	} else if (diag == IXGBE_ERR_SFP_NOT_SUPPORTED)
 		PMD_INIT_LOG(ERR, "Unsupported SFP+ Module\n");
 	if (diag) {
-		PMD_INIT_LOG(ERR, "Hardware Initialization Failure: %d", diag);
+		PMD_INIT_LOG(ERR, "Hardware Initialization Failure: %d\n", diag);
 		return -EIO;
 	}
 
@@ -813,7 +814,7 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 	if (eth_dev->data->mac_addrs == NULL) {
 		PMD_INIT_LOG(ERR,
 			"Failed to allocate %u bytes needed to store "
-			"MAC addresses",
+			"MAC addresses\n",
 			ETHER_ADDR_LEN * hw->mac.num_rar_entries);
 		return -ENOMEM;
 	}
@@ -826,7 +827,8 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 			IXGBE_VMDQ_NUM_UC_MAC, 0);
 	if (eth_dev->data->hash_mac_addrs == NULL) {
 		PMD_INIT_LOG(ERR,
-			"Failed to allocate %d bytes needed to store MAC addresses",
+			"Failed to allocate %d bytes needed to store "
+			"MAC addresses\n",
 			ETHER_ADDR_LEN * IXGBE_VMDQ_NUM_UC_MAC);
 		return -ENOMEM;
 	}
@@ -850,14 +852,14 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 
 	if (ixgbe_is_sfp(hw) && hw->phy.sfp_type != ixgbe_sfp_type_not_present)
 		PMD_INIT_LOG(DEBUG,
-			     "MAC: %d, PHY: %d, SFP+: %d<n",
+			     "MAC: %d, PHY: %d, SFP+: %d\n",
 			     (int) hw->mac.type, (int) hw->phy.type,
 			     (int) hw->phy.sfp_type);
 	else
 		PMD_INIT_LOG(DEBUG, "MAC: %d, PHY: %d\n",
 			     (int) hw->mac.type, (int) hw->phy.type);
 
-	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x",
+	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x\n",
 			eth_dev->data->port_id, pci_dev->id.vendor_id,
 			pci_dev->id.device_id);
 
@@ -933,7 +935,7 @@ eth_ixgbevf_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 		IXGBE_DEV_PRIVATE_TO_HWSTRIP_BITMAP(eth_dev->data->dev_private);
 	struct ether_addr *perm_addr = (struct ether_addr *) hw->mac.perm_addr;
 
-	PMD_INIT_LOG(DEBUG, "eth_ixgbevf_dev_init");
+	PMD_INIT_FUNC_TRACE();
 
 	eth_dev->dev_ops = &ixgbevf_eth_dev_ops;
 	eth_dev->rx_pkt_burst = &ixgbe_recv_pkts;
@@ -963,7 +965,8 @@ eth_ixgbevf_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 	/* Initialize the shared code (base driver) */
 	diag = ixgbe_init_shared_code(hw);
 	if (diag != IXGBE_SUCCESS) {
-		PMD_INIT_LOG(ERR, "Shared code init failed for ixgbevf: %d", diag);
+		PMD_INIT_LOG(ERR, "Shared code init failed for ixgbevf: %d\n",
+			     diag);
 		return -EIO;
 	}
 
@@ -982,7 +985,7 @@ eth_ixgbevf_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 	 * In this case, assign a random MAC address.
 	 */
 	if ((diag != IXGBE_SUCCESS) && (diag != IXGBE_ERR_INVALID_MAC_ADDR)) {
-		PMD_INIT_LOG(ERR, "VF Initialization Failure: %d", diag);
+		PMD_INIT_LOG(ERR, "VF Initialization Failure: %d\n", diag);
 		return (diag);
 	}
 
@@ -998,7 +1001,7 @@ eth_ixgbevf_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 	if (eth_dev->data->mac_addrs == NULL) {
 		PMD_INIT_LOG(ERR,
 			"Failed to allocate %u bytes needed to store "
-			"MAC addresses",
+			"MAC addresses\n",
 			ETHER_ADDR_LEN * hw->mac.num_rar_entries);
 		return -ENOMEM;
 	}
@@ -1034,11 +1037,12 @@ eth_ixgbevf_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 			break;
 
 		default:
-			PMD_INIT_LOG(ERR, "VF Initialization Failure: %d", diag);
+			PMD_INIT_LOG(ERR, "VF Initialization Failure: %d\n",
+				     diag);
 			return (-EIO);
 	}
 
-	PMD_INIT_LOG(DEBUG, "\nport %d vendorID=0x%x deviceID=0x%x mac.type=%s\n",
+	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s\n",
 			 eth_dev->data->port_id, pci_dev->id.vendor_id, pci_dev->id.device_id,
 			 "ixgbe_mac_82599_vf");
 
@@ -1207,7 +1211,7 @@ ixgbe_vlan_hw_strip_disable(struct rte_eth_dev *dev, uint16_t queue)
 
 	if (hw->mac.type == ixgbe_mac_82598EB) {
 		/* No queue level support */
-		PMD_INIT_LOG(INFO, "82598EB not support queue level hw strip");
+		PMD_INIT_LOG(INFO, "82598EB not support queue level hw strip\n");
 		return;
 	}
 	else {
@@ -1231,7 +1235,7 @@ ixgbe_vlan_hw_strip_enable(struct rte_eth_dev *dev, uint16_t queue)
 
 	if (hw->mac.type == ixgbe_mac_82598EB) {
 		/* No queue level supported */
-		PMD_INIT_LOG(INFO, "82598EB not support queue level hw strip");
+		PMD_INIT_LOG(INFO, "82598EB not support queue level hw strip\n");
 		return;
 	}
 	else {
@@ -1543,7 +1547,7 @@ skip_link_setup:
 	return (0);
 
 error:
-	PMD_INIT_LOG(ERR, "failure in ixgbe_dev_start(): %d", err);
+	PMD_INIT_LOG(ERR, "failure in ixgbe_dev_start(): %d\n", err);
 	ixgbe_dev_clear_queues(dev);
 	return -EIO;
 }
@@ -1599,10 +1603,8 @@ ixgbe_dev_set_link_up(struct rte_eth_dev *dev)
 #ifdef RTE_NIC_BYPASS
 		if (hw->device_id == IXGBE_DEV_ID_82599_BYPASS) {
 			/* Not suported in bypass mode */
-			PMD_INIT_LOG(ERR,
-				"\nSet link up is not supported "
-				"by device id 0x%x\n",
-				hw->device_id);
+			PMD_INIT_LOG(ERR, "Set link up is not supported "
+				     "by device id 0x%x\n", hw->device_id);
 			return -ENOTSUP;
 		}
 #endif
@@ -1611,8 +1613,8 @@ ixgbe_dev_set_link_up(struct rte_eth_dev *dev)
 		return 0;
 	}
 
-	PMD_INIT_LOG(ERR, "\nSet link up is not supported by device id 0x%x\n",
-		hw->device_id);
+	PMD_INIT_LOG(ERR, "Set link up is not supported by device id 0x%x\n",
+		     hw->device_id);
 	return -ENOTSUP;
 }
 
@@ -1628,10 +1630,8 @@ ixgbe_dev_set_link_down(struct rte_eth_dev *dev)
 #ifdef RTE_NIC_BYPASS
 		if (hw->device_id == IXGBE_DEV_ID_82599_BYPASS) {
 			/* Not suported in bypass mode */
-			PMD_INIT_LOG(ERR,
-				"\nSet link down is not supported "
-				"by device id 0x%x\n",
-				 hw->device_id);
+			PMD_INIT_LOG(ERR, "Set link down is not supported "
+				     "by device id 0x%x\n", hw->device_id);
 			return -ENOTSUP;
 		}
 #endif
@@ -1640,9 +1640,8 @@ ixgbe_dev_set_link_down(struct rte_eth_dev *dev)
 		return 0;
 	}
 
-	PMD_INIT_LOG(ERR,
-		"\nSet link down is not supported by device id 0x%x\n",
-		 hw->device_id);
+	PMD_INIT_LOG(ERR, "Set link down is not supported by device id 0x%x\n",
+		     hw->device_id);
 	return -ENOTSUP;
 }
 
@@ -2113,7 +2112,7 @@ ixgbe_dev_interrupt_get_status(struct rte_eth_dev *dev)
 
 	/* read-on-clear nic registers here */
 	eicr = IXGBE_READ_REG(hw, IXGBE_EICR);
-	PMD_DRV_LOG(INFO, "eicr %x", eicr);
+	PMD_DRV_LOG(INFO, "eicr %x\n", eicr);
 
 	intr->flags = 0;
 	if (eicr & IXGBE_EICR_LSC) {
@@ -2145,16 +2144,16 @@ ixgbe_dev_link_status_print(struct rte_eth_dev *dev)
 	memset(&link, 0, sizeof(link));
 	rte_ixgbe_dev_atomic_read_link_status(dev, &link);
 	if (link.link_status) {
-		PMD_INIT_LOG(INFO, "Port %d: Link Up - speed %u Mbps - %s",
+		PMD_INIT_LOG(INFO, "Port %d: Link Up - speed %u Mbps - %s\n",
 					(int)(dev->data->port_id),
 					(unsigned)link.link_speed,
 			link.link_duplex == ETH_LINK_FULL_DUPLEX ?
 					"full-duplex" : "half-duplex");
 	} else {
-		PMD_INIT_LOG(INFO, " Port %d: Link Down",
+		PMD_INIT_LOG(INFO, " Port %d: Link Down\n",
 				(int)(dev->data->port_id));
 	}
-	PMD_INIT_LOG(INFO, "PCI Address: %04d:%02d:%02d:%d",
+	PMD_INIT_LOG(INFO, "PCI Address: %04d:%02d:%02d:%d\n",
 				dev->pci_dev->addr.domain,
 				dev->pci_dev->addr.bus,
 				dev->pci_dev->addr.devid,
@@ -2211,9 +2210,9 @@ ixgbe_dev_interrupt_action(struct rte_eth_dev *dev)
 	if (intr_enable_delay) {
 		if (rte_eal_alarm_set(timeout * 1000,
 				      ixgbe_dev_interrupt_delayed_handler, (void*)dev) < 0)
-			PMD_DRV_LOG(ERR, "Error setting alarm");
+			PMD_DRV_LOG(ERR, "Error setting alarm\n");
 	} else {
-		PMD_DRV_LOG(DEBUG, "enable intr immediately");
+		PMD_DRV_LOG(DEBUG, "enable intr immediately\n");
 		ixgbe_enable_intr(dev);
 		rte_intr_enable(&(dev->pci_dev->intr_handle));
 	}
@@ -2371,7 +2370,7 @@ ixgbe_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 	if (fc_conf->autoneg != !hw->fc.disable_fc_autoneg)
 		return -ENOTSUP;
 	rx_buf_size = IXGBE_READ_REG(hw, IXGBE_RXPBSIZE(0));
-	PMD_INIT_LOG(DEBUG, "Rx packet buffer size = 0x%x \n", rx_buf_size);
+	PMD_INIT_LOG(DEBUG, "Rx packet buffer size = 0x%x\n", rx_buf_size);
 
 	/*
 	 * At least reserve one Ethernet frame for watermark
@@ -2413,7 +2412,7 @@ ixgbe_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 		return 0;
 	}
 
-	PMD_INIT_LOG(ERR, "ixgbe_fc_enable = 0x%x \n", err);
+	PMD_INIT_LOG(ERR, "ixgbe_fc_enable = 0x%x\n", err);
 	return -EIO;
 }
 
@@ -2593,7 +2592,7 @@ ixgbe_priority_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_pfc_conf *p
 	ixgbe_dcb_unpack_map_cee(dcb_config, IXGBE_DCB_RX_CONFIG, map);
 	tc_num = map[pfc_conf->priority];
 	rx_buf_size = IXGBE_READ_REG(hw, IXGBE_RXPBSIZE(tc_num));
-	PMD_INIT_LOG(DEBUG, "Rx packet buffer size = 0x%x \n", rx_buf_size);
+	PMD_INIT_LOG(DEBUG, "Rx packet buffer size = 0x%x\n", rx_buf_size);
 	/*
 	 * At least reserve one Ethernet frame for watermark
 	 * high_water/low_water in kilo bytes for ixgbe
@@ -2618,7 +2617,7 @@ ixgbe_priority_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_pfc_conf *p
 	if ((err == IXGBE_SUCCESS) || (err == IXGBE_ERR_FC_NOT_NEGOTIATED))
 		return 0;
 
-	PMD_INIT_LOG(ERR, "ixgbe_dcb_pfc_enable = 0x%x \n", err);
+	PMD_INIT_LOG(ERR, "ixgbe_dcb_pfc_enable = 0x%x\n", err);
 	return -EIO;
 }
 
@@ -2765,7 +2764,7 @@ ixgbe_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 static void
 ixgbevf_intr_disable(struct ixgbe_hw *hw)
 {
-	PMD_INIT_LOG(DEBUG, "ixgbevf_intr_disable");
+	PMD_INIT_FUNC_TRACE();
 
 	/* Clear interrupt mask to stop from interrupts being generated */
 	IXGBE_WRITE_REG(hw, IXGBE_VTEIMC, IXGBE_VF_IRQ_CLEAR_MASK);
@@ -2778,8 +2777,8 @@ ixgbevf_dev_configure(struct rte_eth_dev *dev)
 {
 	struct rte_eth_conf* conf = &dev->data->dev_conf;
 
-	PMD_INIT_LOG(DEBUG, "\nConfigured Virtual Function port id: %d\n",
-		dev->data->port_id);
+	PMD_INIT_LOG(DEBUG, "Configured Virtual Function port id: %d\n",
+		     dev->data->port_id);
 
 	/*
 	 * VF has no ability to enable/disable HW CRC
@@ -2807,7 +2806,7 @@ ixgbevf_dev_start(struct rte_eth_dev *dev)
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	int err, mask = 0;
 
-	PMD_INIT_LOG(DEBUG, "ixgbevf_dev_start");
+	PMD_INIT_FUNC_TRACE();
 
 	hw->mac.ops.reset_hw(hw);
 
@@ -2842,7 +2841,7 @@ ixgbevf_dev_stop(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-	PMD_INIT_LOG(DEBUG, "ixgbevf_dev_stop");
+	PMD_INIT_FUNC_TRACE();
 
 	hw->adapter_stopped = TRUE;
 	ixgbe_stop_adapter(hw);
@@ -2861,7 +2860,7 @@ ixgbevf_dev_close(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-	PMD_INIT_LOG(DEBUG, "ixgbevf_dev_close");
+	PMD_INIT_FUNC_TRACE();
 
 	ixgbe_reset_hw(hw);
 
@@ -2908,7 +2907,7 @@ ixgbevf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
 	/* vind is not used in VF driver, set to 0, check ixgbe_set_vfta_vf */
 	ret = ixgbe_set_vfta(hw, vlan_id, 0, !!on);
 	if(ret){
-		PMD_INIT_LOG(ERR, "Unable to set VF vlan");
+		PMD_INIT_LOG(ERR, "Unable to set VF vlan\n");
 		return ret;
 	}
 	vid_idx = (uint32_t) ((vlan_id >> 5) & 0x7F);
@@ -3477,7 +3476,7 @@ ixgbevf_add_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 	diag = ixgbevf_set_uc_addr_vf(hw, 2, mac_addr->addr_bytes);
 	if (diag == 0)
 		return;
-	PMD_DRV_LOG(ERR, "Unable to add MAC address - diag=%d", diag);
+	PMD_DRV_LOG(ERR, "Unable to add MAC address - diag=%d\n", diag);
 }
 
 static void
@@ -3517,7 +3516,7 @@ ixgbevf_remove_mac_addr(struct rte_eth_dev *dev, uint32_t index)
 			PMD_DRV_LOG(ERR,
 				    "Adding again MAC address "
 				    "%02x:%02x:%02x:%02x:%02x:%02x failed "
-				    "diag=%d",
+				    "diag=%d\n",
 				    mac_addr->addr_bytes[0],
 				    mac_addr->addr_bytes[1],
 				    mac_addr->addr_bytes[2],
@@ -3806,7 +3805,7 @@ ixgbe_add_5tuple_filter(struct rte_eth_dev *dev, uint16_t index,
 		return -EINVAL;  /* filter index is out of range. */
 
 	if (filter->tcp_flags) {
-		PMD_INIT_LOG(INFO, "82599EB not tcp flags in 5tuple");
+		PMD_INIT_LOG(INFO, "82599EB not tcp flags in 5tuple\n");
 		return -EINVAL;
 	}
 
diff --git a/lib/librte_pmd_ixgbe/ixgbe_fdir.c b/lib/librte_pmd_ixgbe/ixgbe_fdir.c
index 6c0a530..c78a450 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_fdir.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_fdir.c
@@ -139,7 +139,7 @@ configure_fdir_flags(struct rte_fdir_conf *conf, uint32_t *fdirctrl)
 		break;
 	default:
 		/* bad value */
-		PMD_INIT_LOG(ERR, "Invalid fdir_conf->pballoc value");
+		PMD_INIT_LOG(ERR, "Invalid fdir_conf->pballoc value\n");
 		return -EINVAL;
 	};
 
@@ -158,7 +158,7 @@ configure_fdir_flags(struct rte_fdir_conf *conf, uint32_t *fdirctrl)
 		break;
 	default:
 		/* bad value */
-		PMD_INIT_LOG(ERR, "Invalid fdir_conf->status value");
+		PMD_INIT_LOG(ERR, "Invalid fdir_conf->status value\n");
 		return -EINVAL;
 	};
 
@@ -395,7 +395,7 @@ fdir_filter_to_atr_input(struct rte_fdir_filter *fdir_filter,
 	if ((fdir_filter->l4type == RTE_FDIR_L4TYPE_SCTP ||
 			fdir_filter->l4type == RTE_FDIR_L4TYPE_NONE) &&
 			(fdir_filter->port_src || fdir_filter->port_dst)) {
-		PMD_INIT_LOG(ERR, "Invalid fdir_filter");
+		PMD_INIT_LOG(ERR, "Invalid fdir_filter\n");
 		return -EINVAL;
 	}
 
@@ -420,7 +420,7 @@ fdir_filter_to_atr_input(struct rte_fdir_filter *fdir_filter,
 		input->formatted.flow_type = IXGBE_ATR_FLOW_TYPE_IPV4;
 		break;
 	default:
-		PMD_INIT_LOG(ERR, " Error on l4type input");
+		PMD_INIT_LOG(ERR, " Error on l4type input\n");
 		return -EINVAL;
 	}
 
@@ -524,7 +524,7 @@ fdir_erase_filter_82599(struct ixgbe_hw *hw,
 	}
 
 	if (!retry_count) {
-		PMD_INIT_LOG(ERR, "Timeout querying for flow director filter");
+		PMD_INIT_LOG(ERR, "Timeout querying for flow director filter\n");
 		err = -EIO;
 	}
 
@@ -678,7 +678,7 @@ ixgbe_fdir_set_masks(struct rte_eth_dev *dev, struct rte_fdir_masks *fdir_masks)
 
 	err = ixgbe_reinit_fdir_tables_82599(hw);
 	if (err) {
-		PMD_INIT_LOG(ERR, "reinit of fdir tables failed");
+		PMD_INIT_LOG(ERR, "reinit of fdir tables failed\n");
 		return -EIO;
 	}
 
diff --git a/lib/librte_pmd_ixgbe/ixgbe_logs.h b/lib/librte_pmd_ixgbe/ixgbe_logs.h
index 9f0a684..22d8cfb 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_logs.h
+++ b/lib/librte_pmd_ixgbe/ixgbe_logs.h
@@ -36,8 +36,8 @@
 
 #ifdef RTE_LIBRTE_IXGBE_DEBUG_INIT
 #define PMD_INIT_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
-#define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
+	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
+#define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>\n")
 #else
 #define PMD_INIT_LOG(level, fmt, args...) do { } while(0)
 #define PMD_INIT_FUNC_TRACE() do { } while(0)
@@ -45,28 +45,28 @@
 
 #ifdef RTE_LIBRTE_IXGBE_DEBUG_RX
 #define PMD_RX_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
 #else
 #define PMD_RX_LOG(level, fmt, args...) do { } while(0)
 #endif
 
 #ifdef RTE_LIBRTE_IXGBE_DEBUG_TX
 #define PMD_TX_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
 #else
 #define PMD_TX_LOG(level, fmt, args...) do { } while(0)
 #endif
 
 #ifdef RTE_LIBRTE_IXGBE_DEBUG_TX_FREE
 #define PMD_TX_FREE_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
 #else
 #define PMD_TX_FREE_LOG(level, fmt, args...) do { } while(0)
 #endif
 
 #ifdef RTE_LIBRTE_IXGBE_DEBUG_DRIVER
 #define PMD_DRV_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
 #else
 #define PMD_DRV_LOG(level, fmt, args...) do { } while(0)
 #endif
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index dfc2076..cbec821 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -492,7 +492,7 @@ ixgbe_xmit_cleanup(struct igb_tx_queue *txq)
 	{
 		PMD_TX_FREE_LOG(DEBUG,
 				"TX descriptor %4u is not done"
-				"(port=%d queue=%d)",
+				"(port=%d queue=%d)\n",
 				desc_to_clean_to,
 				txq->port_id, txq->queue_id);
 		/* Failed to clean any descriptors, better luck next time */
@@ -509,7 +509,7 @@ ixgbe_xmit_cleanup(struct igb_tx_queue *txq)
 
 	PMD_TX_FREE_LOG(DEBUG,
 			"Cleaning %4u TX descriptors: %4u to %4u "
-			"(port=%d queue=%d)",
+			"(port=%d queue=%d)\n",
 			nb_tx_to_clean, last_desc_cleaned, desc_to_clean_to,
 			txq->port_id, txq->queue_id);
 
@@ -630,7 +630,7 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			PMD_TX_FREE_LOG(DEBUG,
 					"Not enough free TX descriptors "
 					"nb_used=%4u nb_free=%4u "
-					"(port=%d queue=%d)",
+					"(port=%d queue=%d)\n",
 					nb_used, txq->nb_tx_free,
 					txq->port_id, txq->queue_id);
 
@@ -650,7 +650,7 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 					"performance."
 					"nb_used=%4u nb_free=%4u "
 					"tx_rs_thresh=%4u. "
-					"(port=%d queue=%d)",
+					"(port=%d queue=%d)\n",
 					nb_used, txq->nb_tx_free,
 					txq->tx_rs_thresh,
 					txq->port_id, txq->queue_id);
@@ -782,7 +782,7 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		if (txq->nb_tx_used >= txq->tx_rs_thresh) {
 			PMD_TX_FREE_LOG(DEBUG,
 					"Setting RS bit on TXD id="
-					"%4u (port=%d queue=%d)",
+					"%4u (port=%d queue=%d)\n",
 					tx_last, txq->port_id, txq->queue_id);
 
 			cmd_type_len |= IXGBE_TXD_CMD_RS;
@@ -798,7 +798,7 @@ end_of_tx:
 	/*
 	 * Set the Transmit Descriptor Tail (TDT)
 	 */
-	PMD_TX_LOG(DEBUG, "port_id=%u queue_id=%u tx_tail=%u nb_tx=%u",
+	PMD_TX_LOG(DEBUG, "port_id=%u queue_id=%u tx_tail=%u nb_tx=%u\n",
 		   (unsigned) txq->port_id, (unsigned) txq->queue_id,
 		   (unsigned) tx_id, (unsigned) nb_tx);
 	IXGBE_PCI_REG_WRITE(txq->tdt_reg_addr, tx_id);
@@ -1383,7 +1383,7 @@ ixgbe_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		 * to happen by sending specific "back-pressure" flow control
 		 * frames to its peer(s).
 		 */
-		PMD_RX_LOG(DEBUG, "\nport_id=%u queue_id=%u rx_id=%u "
+		PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_id=%u "
 			   "staterr=0x%x data_len=%u\n",
 			   (unsigned) rxq->port_id, (unsigned) rxq->queue_id,
 			   (unsigned) rx_id, (unsigned) staterr,
-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 02/11] ixgbe: always log init messages
  2014-08-26 14:09 [dpdk-dev] [PATCH 00/11] cleanup logs in main PMDs David Marchand
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 01/11] ixgbe: clean log messages David Marchand
@ 2014-08-26 14:09 ` David Marchand
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 03/11] ixgbe: add a message when forcing scatter mode David Marchand
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: David Marchand @ 2014-08-26 14:09 UTC (permalink / raw)
  To: dev

'init' messages should always be logged and filtered at runtime by rte_log.
All the more so as these messages are not in the datapath.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_pmd_ixgbe/ixgbe_logs.h |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_logs.h b/lib/librte_pmd_ixgbe/ixgbe_logs.h
index 22d8cfb..88fa7d5 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_logs.h
+++ b/lib/librte_pmd_ixgbe/ixgbe_logs.h
@@ -34,12 +34,13 @@
 #ifndef _IXGBE_LOGS_H_
 #define _IXGBE_LOGS_H_
 
-#ifdef RTE_LIBRTE_IXGBE_DEBUG_INIT
 #define PMD_INIT_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
+	rte_log(RTE_LOG_ ## level, RTE_LOGTYPE_PMD, \
+		"PMD: %s(): " fmt, __func__, ##args)
+
+#ifdef RTE_LIBRTE_IXGBE_DEBUG_INIT
 #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>\n")
 #else
-#define PMD_INIT_LOG(level, fmt, args...) do { } while(0)
 #define PMD_INIT_FUNC_TRACE() do { } while(0)
 #endif
 
-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 03/11] ixgbe: add a message when forcing scatter mode
  2014-08-26 14:09 [dpdk-dev] [PATCH 00/11] cleanup logs in main PMDs David Marchand
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 01/11] ixgbe: clean log messages David Marchand
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 02/11] ixgbe: always log init messages David Marchand
@ 2014-08-26 14:09 ` David Marchand
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 04/11] ixgbe: add log messages when rx bulk mode is not usable David Marchand
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: David Marchand @ 2014-08-26 14:09 UTC (permalink / raw)
  To: dev

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index cbec821..e2804bd 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -3482,12 +3482,16 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev)
 		/* It adds dual VLAN length for supporting dual VLAN */
 		if ((dev->data->dev_conf.rxmode.max_rx_pkt_len +
 				2 * IXGBE_VLAN_TAG_SIZE) > buf_size){
+			if (!dev->data->scattered_rx)
+				PMD_INIT_LOG(DEBUG, "forcing scatter mode\n");
 			dev->data->scattered_rx = 1;
 			dev->rx_pkt_burst = ixgbe_recv_scattered_pkts;
 		}
 	}
 
 	if (dev->data->dev_conf.rxmode.enable_scatter) {
+		if (!dev->data->scattered_rx)
+			PMD_INIT_LOG(DEBUG, "forcing scatter mode\n");
 		dev->rx_pkt_burst = ixgbe_recv_scattered_pkts;
 		dev->data->scattered_rx = 1;
 	}
@@ -3975,12 +3979,16 @@ ixgbevf_dev_rx_init(struct rte_eth_dev *dev)
 		/* It adds dual VLAN length for supporting dual VLAN */
 		if ((dev->data->dev_conf.rxmode.max_rx_pkt_len +
 				2 * IXGBE_VLAN_TAG_SIZE) > buf_size) {
+			if (!dev->data->scattered_rx)
+				PMD_INIT_LOG(DEBUG, "forcing scatter mode\n");
 			dev->data->scattered_rx = 1;
 			dev->rx_pkt_burst = ixgbe_recv_scattered_pkts;
 		}
 	}
 
 	if (dev->data->dev_conf.rxmode.enable_scatter) {
+		if (!dev->data->scattered_rx)
+			PMD_INIT_LOG(DEBUG, "forcing scatter mode\n");
 		dev->rx_pkt_burst = ixgbe_recv_scattered_pkts;
 		dev->data->scattered_rx = 1;
 	}
-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 04/11] ixgbe: add log messages when rx bulk mode is not usable
  2014-08-26 14:09 [dpdk-dev] [PATCH 00/11] cleanup logs in main PMDs David Marchand
                   ` (2 preceding siblings ...)
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 03/11] ixgbe: add a message when forcing scatter mode David Marchand
@ 2014-08-26 14:09 ` David Marchand
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 05/11] i40e: clean log messages David Marchand
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: David Marchand @ 2014-08-26 14:09 UTC (permalink / raw)
  To: dev

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c |   29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index e2804bd..7d6c7ea 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -1969,15 +1969,34 @@ check_rx_burst_bulk_alloc_preconditions(__rte_unused struct igb_rx_queue *rxq)
 	 * outside of this function.
 	 */
 #ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC
-	if (! (rxq->rx_free_thresh >= RTE_PMD_IXGBE_RX_MAX_BURST))
+	if (! (rxq->rx_free_thresh >= RTE_PMD_IXGBE_RX_MAX_BURST)) {
+		PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions: "
+			     "rxq->rx_free_thresh=%d, "
+			     "RTE_PMD_IXGBE_RX_MAX_BURST=%d\n",
+			     rxq->rx_free_thresh, RTE_PMD_IXGBE_RX_MAX_BURST);
 		ret = -EINVAL;
-	else if (! (rxq->rx_free_thresh < rxq->nb_rx_desc))
+	} else if (! (rxq->rx_free_thresh < rxq->nb_rx_desc)) {
+		PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions: "
+			     "rxq->rx_free_thresh=%d, "
+			     "rxq->nb_rx_desc=%d\n",
+			     rxq->rx_free_thresh, rxq->nb_rx_desc);
 		ret = -EINVAL;
-	else if (! ((rxq->nb_rx_desc % rxq->rx_free_thresh) == 0))
+	} else if (! ((rxq->nb_rx_desc % rxq->rx_free_thresh) == 0)) {
+		PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions: "
+			     "rxq->nb_rx_desc=%d, "
+			     "rxq->rx_free_thresh=%d\n",
+			     rxq->nb_rx_desc, rxq->rx_free_thresh);
 		ret = -EINVAL;
-	else if (! (rxq->nb_rx_desc <
-	       (IXGBE_MAX_RING_DESC - RTE_PMD_IXGBE_RX_MAX_BURST)))
+	} else if (! (rxq->nb_rx_desc <
+	       (IXGBE_MAX_RING_DESC - RTE_PMD_IXGBE_RX_MAX_BURST))) {
+		PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions: "
+			     "rxq->nb_rx_desc=%d, "
+			     "IXGBE_MAX_RING_DESC=%d, "
+			     "RTE_PMD_IXGBE_RX_MAX_BURST=%d\n",
+			     rxq->nb_rx_desc, IXGBE_MAX_RING_DESC,
+			     RTE_PMD_IXGBE_RX_MAX_BURST);
 		ret = -EINVAL;
+	}
 #else
 	ret = -EINVAL;
 #endif
-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 05/11] i40e: clean log messages
  2014-08-26 14:09 [dpdk-dev] [PATCH 00/11] cleanup logs in main PMDs David Marchand
                   ` (3 preceding siblings ...)
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 04/11] ixgbe: add log messages when rx bulk mode is not usable David Marchand
@ 2014-08-26 14:09 ` David Marchand
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 06/11] i40e: always log init messages David Marchand
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: David Marchand @ 2014-08-26 14:09 UTC (permalink / raw)
  To: dev

Clean log messages:
- remove superfluous \n in log macros and add some \n where needed,
- remove leading \n in some messages,
- split multi lines messages,
- replace some PMD_INIT_LOG(DEBUG, "some_func\n") with PMD_INIT_FUNC_TRACE().

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_pmd_i40e/i40e/i40e_osdep.h |    2 +-
 lib/librte_pmd_i40e/i40e_ethdev.c     |   43 +++++++++++++++++----------------
 lib/librte_pmd_i40e/i40e_ethdev_vf.c  |    4 +--
 lib/librte_pmd_i40e/i40e_logs.h       |   12 ++++-----
 lib/librte_pmd_i40e/i40e_pf.c         |    4 +--
 lib/librte_pmd_i40e/i40e_rxtx.c       |    6 ++---
 6 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/lib/librte_pmd_i40e/i40e/i40e_osdep.h b/lib/librte_pmd_i40e/i40e/i40e_osdep.h
index 0ed4b65..5f1c98f 100644
--- a/lib/librte_pmd_i40e/i40e/i40e_osdep.h
+++ b/lib/librte_pmd_i40e/i40e/i40e_osdep.h
@@ -112,7 +112,7 @@ typedef enum i40e_status_code i40e_status;
 #define i40e_debug(h, m, s, ...)                                \
 do {                                                            \
 	if (((m) & (h)->debug_mask))                            \
-		PMD_DRV_LOG(DEBUG, "i40e %02x.%x " s,           \
+		PMD_DRV_LOG(DEBUG, "i40e %02x.%x\n" s,          \
 			(h)->bus.device, (h)->bus.func,         \
 					##__VA_ARGS__);         \
 } while (0)
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index 4e65ca4..d6104ed 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -388,14 +388,15 @@ eth_i40e_dev_init(__rte_unused struct eth_driver *eth_drv,
 	/* Reset here to make sure all is clean for each PF */
 	ret = i40e_pf_reset(hw);
 	if (ret) {
-		PMD_INIT_LOG(ERR, "Failed to reset pf: %d", ret);
+		PMD_INIT_LOG(ERR, "Failed to reset pf: %d\n", ret);
 		return ret;
 	}
 
 	/* Initialize the shared code (base driver) */
 	ret = i40e_init_shared_code(hw);
 	if (ret) {
-		PMD_INIT_LOG(ERR, "Failed to init shared code (base driver): %d", ret);
+		PMD_INIT_LOG(ERR, "Failed to init shared code (base driver):"
+			     "%d\n", ret);
 		return ret;
 	}
 
@@ -403,7 +404,7 @@ eth_i40e_dev_init(__rte_unused struct eth_driver *eth_drv,
 	i40e_init_adminq_parameter(hw);
 	ret = i40e_init_adminq(hw);
 	if (ret != I40E_SUCCESS) {
-		PMD_INIT_LOG(ERR, "Failed to init adminq: %d", ret);
+		PMD_INIT_LOG(ERR, "Failed to init adminq: %d\n", ret);
 		return -EIO;
 	}
 	PMD_INIT_LOG(INFO, "FW %d.%d API %d.%d NVM "
@@ -425,14 +426,14 @@ eth_i40e_dev_init(__rte_unused struct eth_driver *eth_drv,
 	/* Get hw capabilities */
 	ret = i40e_get_cap(hw);
 	if (ret != I40E_SUCCESS) {
-		PMD_INIT_LOG(ERR, "Failed to get capabilities: %d", ret);
+		PMD_INIT_LOG(ERR, "Failed to get capabilities: %d\n", ret);
 		goto err_get_capabilities;
 	}
 
 	/* Initialize parameters for PF */
 	ret = i40e_pf_parameter_init(dev);
 	if (ret != 0) {
-		PMD_INIT_LOG(ERR, "Failed to do parameter init: %d", ret);
+		PMD_INIT_LOG(ERR, "Failed to do parameter init: %d\n", ret);
 		goto err_parameter_init;
 	}
 
@@ -453,21 +454,21 @@ eth_i40e_dev_init(__rte_unused struct eth_driver *eth_drv,
 	ret = i40e_init_lan_hmc(hw, hw->func_caps.num_tx_qp,
 				hw->func_caps.num_rx_qp, 0, 0);
 	if (ret != I40E_SUCCESS) {
-		PMD_INIT_LOG(ERR, "Failed to init lan hmc: %d", ret);
+		PMD_INIT_LOG(ERR, "Failed to init lan hmc: %d\n", ret);
 		goto err_init_lan_hmc;
 	}
 
 	/* Configure lan hmc */
 	ret = i40e_configure_lan_hmc(hw, I40E_HMC_MODEL_DIRECT_ONLY);
 	if (ret != I40E_SUCCESS) {
-		PMD_INIT_LOG(ERR, "Failed to configure lan hmc: %d", ret);
+		PMD_INIT_LOG(ERR, "Failed to configure lan hmc: %d\n", ret);
 		goto err_configure_lan_hmc;
 	}
 
 	/* Get and check the mac address */
 	i40e_get_mac_addr(hw, hw->mac.addr);
 	if (i40e_validate_mac_addr(hw->mac.addr) != I40E_SUCCESS) {
-		PMD_INIT_LOG(ERR, "mac address is not valid");
+		PMD_INIT_LOG(ERR, "mac address is not valid\n");
 		ret = -EIO;
 		goto err_get_mac_addr;
 	}
@@ -482,7 +483,7 @@ eth_i40e_dev_init(__rte_unused struct eth_driver *eth_drv,
 	/* PF setup, which includes VSI setup */
 	ret = i40e_pf_setup(pf);
 	if (ret) {
-		PMD_INIT_LOG(ERR, "Failed to setup pf switch: %d", ret);
+		PMD_INIT_LOG(ERR, "Failed to setup pf switch: %d\n", ret);
 		goto err_setup_pf_switch;
 	}
 
@@ -499,8 +500,8 @@ eth_i40e_dev_init(__rte_unused struct eth_driver *eth_drv,
 	/* Should be after VSI initialized */
 	dev->data->mac_addrs = rte_zmalloc("i40e", len, 0);
 	if (!dev->data->mac_addrs) {
-		PMD_INIT_LOG(ERR, "Failed to allocated memory "
-					"for storing mac address");
+		PMD_INIT_LOG(ERR, "Failed to allocated memory for storing "
+			     "mac address\n");
 		goto err_get_mac_addr;
 	}
 	ether_addr_copy((struct ether_addr *)hw->mac.perm_addr,
@@ -723,9 +724,9 @@ i40e_phy_conf_link(struct i40e_hw *hw, uint8_t abilities, uint8_t force_speed)
 	phy_conf.eeer = phy_ab.eeer_val;
 	phy_conf.low_power_ctrl = phy_ab.d3_lpan;
 
-	PMD_DRV_LOG(DEBUG, "\n\tCurrent: abilities %x, link_speed %x\n"
-		    "\tConfig:  abilities %x, link_speed %x",
-		    phy_ab.abilities, phy_ab.link_speed,
+	PMD_DRV_LOG(DEBUG, "\tCurrent: abilities %x, link_speed %x\n"
+		    phy_ab.abilities, phy_ab.link_speed);
+	PMD_DRV_LOG(DEBUG, "\tConfig:  abilities %x, link_speed %x\n",
 		    phy_conf.abilities, phy_conf.link_speed);
 
 	status = i40e_aq_set_phy_config(hw, &phy_conf, NULL);
@@ -2521,7 +2522,7 @@ i40e_vsi_dump_bw_config(struct i40e_vsi *vsi)
 					ets_sla_config.share_credits[i]);
 		PMD_DRV_LOG(INFO, "\tVSI TC%u:credits %u\n", i,
 			rte_le_to_cpu_16(ets_sla_config.credits[i]));
-		PMD_DRV_LOG(INFO, "\tVSI TC%u: max credits: %u", i,
+		PMD_DRV_LOG(INFO, "\tVSI TC%u: max credits: %u\n", i,
 			rte_le_to_cpu_16(ets_sla_config.credits[i / 4]) >>
 								(i * 4));
 	}
@@ -2589,7 +2590,7 @@ i40e_vsi_setup(struct i40e_pf *pf,
 	}
 	ret = i40e_res_pool_alloc(&pf->qp_pool, vsi->nb_qps);
 	if (ret < 0) {
-		PMD_DRV_LOG(ERR, "VSI %d allocate queue failed %d",
+		PMD_DRV_LOG(ERR, "VSI %d allocate queue failed %d\n",
 				vsi->seid, ret);
 		goto fail_mem;
 	}
@@ -2599,7 +2600,7 @@ i40e_vsi_setup(struct i40e_pf *pf,
 	if (type != I40E_VSI_SRIOV) {
 		ret = i40e_res_pool_alloc(&pf->msix_pool, 1);
 		if (ret < 0) {
-			PMD_DRV_LOG(ERR, "VSI %d get heap failed %d", vsi->seid, ret);
+			PMD_DRV_LOG(ERR, "VSI %d get heap failed %d\n", vsi->seid, ret);
 			goto fail_queue_alloc;
 		}
 		vsi->msix_intr = ret;
@@ -2909,14 +2910,14 @@ i40e_pf_setup(struct i40e_pf *pf)
 
 	ret = i40e_pf_get_switch_config(pf);
 	if (ret != I40E_SUCCESS) {
-		PMD_DRV_LOG(ERR, "Could not get switch config, err %d", ret);
+		PMD_DRV_LOG(ERR, "Could not get switch config, err %d\n", ret);
 		return ret;
 	}
 
 	/* VSI setup */
 	vsi = i40e_vsi_setup(pf, I40E_VSI_MAIN, NULL, 0);
 	if (!vsi) {
-		PMD_DRV_LOG(ERR, "Setup of main vsi failed");
+		PMD_DRV_LOG(ERR, "Setup of main vsi failed\n");
 		return I40E_ERR_NOT_READY;
 	}
 	pf->main_vsi = vsi;
@@ -2931,8 +2932,8 @@ i40e_pf_setup(struct i40e_pf *pf)
 	settings.enable_macvlan = TRUE;
 	ret = i40e_set_filter_control(hw, &settings);
 	if (ret)
-		PMD_INIT_LOG(WARNING, "setup_pf_filter_control failed: %d",
-								ret);
+		PMD_INIT_LOG(WARNING, "setup_pf_filter_control failed: %d\n",
+			     ret);
 
 	/* Update flow control according to the auto negotiation */
 	i40e_update_flow_control(hw);
diff --git a/lib/librte_pmd_i40e/i40e_ethdev_vf.c b/lib/librte_pmd_i40e/i40e_ethdev_vf.c
index d8552ad..7672aa8 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev_vf.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev_vf.c
@@ -1101,7 +1101,7 @@ i40evf_dev_init(__rte_unused struct eth_driver *eth_drv,
 					ETHER_ADDR_LEN, 0);
 	if (eth_dev->data->mac_addrs == NULL) {
 		PMD_INIT_LOG(ERR, "Failed to allocate %d bytes needed to "
-				"store MAC addresses", ETHER_ADDR_LEN);
+				"store MAC addresses\n", ETHER_ADDR_LEN);
 		return -ENOMEM;
 	}
 	ether_addr_copy((struct ether_addr *)hw->mac.addr,
@@ -1384,7 +1384,7 @@ i40evf_dev_start(struct rte_eth_dev *dev)
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ether_addr mac_addr;
 
-	PMD_DRV_LOG(DEBUG, "i40evf_dev_start");
+	PMD_INIT_FUNC_TRACE();
 
 	vf->max_pkt_len = dev->data->dev_conf.rxmode.max_rx_pkt_len;
 	if (dev->data->dev_conf.rxmode.jumbo_frame == 1) {
diff --git a/lib/librte_pmd_i40e/i40e_logs.h b/lib/librte_pmd_i40e/i40e_logs.h
index f991dd2..b78e2a1 100644
--- a/lib/librte_pmd_i40e/i40e_logs.h
+++ b/lib/librte_pmd_i40e/i40e_logs.h
@@ -36,8 +36,8 @@
 
 #ifdef RTE_LIBRTE_I40E_DEBUG_INIT
 #define PMD_INIT_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
-#define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
+	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
+#define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>\n")
 #else
 #define PMD_INIT_LOG(level, fmt, args...) do { } while(0)
 #define PMD_INIT_FUNC_TRACE() do { } while(0)
@@ -45,28 +45,28 @@
 
 #ifdef RTE_LIBRTE_I40E_DEBUG_RX
 #define PMD_RX_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
 #else
 #define PMD_RX_LOG(level, fmt, args...) do { } while(0)
 #endif
 
 #ifdef RTE_LIBRTE_I40E_DEBUG_TX
 #define PMD_TX_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
 #else
 #define PMD_TX_LOG(level, fmt, args...) do { } while(0)
 #endif
 
 #ifdef RTE_LIBRTE_I40E_DEBUG_TX_FREE
 #define PMD_TX_FREE_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
 #else
 #define PMD_TX_FREE_LOG(level, fmt, args...) do { } while(0)
 #endif
 
 #ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
 #define PMD_DRV_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
 #else
 #define PMD_DRV_LOG(level, fmt, args...) do { } while(0)
 #endif
diff --git a/lib/librte_pmd_i40e/i40e_pf.c b/lib/librte_pmd_i40e/i40e_pf.c
index e8b154d..0a726d6 100644
--- a/lib/librte_pmd_i40e/i40e_pf.c
+++ b/lib/librte_pmd_i40e/i40e_pf.c
@@ -439,7 +439,7 @@ i40e_pf_host_process_cmd_config_vsi_queues(struct i40e_pf_vf *vf,
 		/* Apply VF RX queue setting to HMC */
 		if (i40e_pf_host_hmc_config_rxq(hw, vf, &qpair[i].rxq)
 			!= I40E_SUCCESS) {
-			PMD_DRV_LOG(ERR, "Configure RX queue HMC failed");
+			PMD_DRV_LOG(ERR, "Configure RX queue HMC failed\n");
 			ret = I40E_ERR_PARAM;
 			goto send_msg;
 		}
@@ -447,7 +447,7 @@ i40e_pf_host_process_cmd_config_vsi_queues(struct i40e_pf_vf *vf,
 		/* Apply VF TX queue setting to HMC */
 		if (i40e_pf_host_hmc_config_txq(hw, vf, &qpair[i].txq)
 			!= I40E_SUCCESS) {
-			PMD_DRV_LOG(ERR, "Configure TX queue HMC failed");
+			PMD_DRV_LOG(ERR, "Configure TX queue HMC failed\n");
 			ret = I40E_ERR_PARAM;
 			goto send_msg;
 		}
diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
index f153844..d592ad6 100644
--- a/lib/librte_pmd_i40e/i40e_rxtx.c
+++ b/lib/librte_pmd_i40e/i40e_rxtx.c
@@ -507,7 +507,7 @@ i40e_xmit_cleanup(struct i40e_tx_queue *txq)
 	if (!(txd[desc_to_clean_to].cmd_type_offset_bsz &
 		rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))) {
 		PMD_TX_FREE_LOG(DEBUG, "TX descriptor %4u is not done "
-			"(port=%d queue=%d)", desc_to_clean_to,
+				"(port=%d queue=%d)\n", desc_to_clean_to,
 				txq->port_id, txq->queue_id);
 		return -1;
 	}
@@ -1219,7 +1219,7 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 		if (txq->nb_tx_used >= txq->tx_rs_thresh) {
 			PMD_TX_FREE_LOG(DEBUG,
 					"Setting RS bit on TXD id="
-					"%4u (port=%d queue=%d)",
+					"%4u (port=%d queue=%d)\n",
 					tx_last, txq->port_id, txq->queue_id);
 
 			td_cmd |= I40E_TX_DESC_CMD_RS;
@@ -1236,7 +1236,7 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 end_of_tx:
 	rte_wmb();
 
-	PMD_TX_LOG(DEBUG, "port_id=%u queue_id=%u tx_tail=%u nb_tx=%u",
+	PMD_TX_LOG(DEBUG, "port_id=%u queue_id=%u tx_tail=%u nb_tx=%u\n",
 		   (unsigned) txq->port_id, (unsigned) txq->queue_id,
 		   (unsigned) tx_id, (unsigned) nb_tx);
 
-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 06/11] i40e: always log init messages
  2014-08-26 14:09 [dpdk-dev] [PATCH 00/11] cleanup logs in main PMDs David Marchand
                   ` (4 preceding siblings ...)
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 05/11] i40e: clean log messages David Marchand
@ 2014-08-26 14:09 ` David Marchand
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 07/11] i40e: add log messages when rx bulk mode is not usable David Marchand
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: David Marchand @ 2014-08-26 14:09 UTC (permalink / raw)
  To: dev

'init' messages should always be logged and filtered at runtime by rte_log.
All the more so as these messages are not in the datapath.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_pmd_i40e/i40e_logs.h |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/librte_pmd_i40e/i40e_logs.h b/lib/librte_pmd_i40e/i40e_logs.h
index b78e2a1..1430a32 100644
--- a/lib/librte_pmd_i40e/i40e_logs.h
+++ b/lib/librte_pmd_i40e/i40e_logs.h
@@ -34,12 +34,13 @@
 #ifndef _I40E_LOGS_H_
 #define _I40E_LOGS_H_
 
-#ifdef RTE_LIBRTE_I40E_DEBUG_INIT
 #define PMD_INIT_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
+	rte_log(RTE_LOG_ ## level, RTE_LOGTYPE_PMD, \
+		"PMD: %s(): " fmt, __func__, ##args)
+
+#ifdef RTE_LIBRTE_I40E_DEBUG_INIT
 #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>\n")
 #else
-#define PMD_INIT_LOG(level, fmt, args...) do { } while(0)
 #define PMD_INIT_FUNC_TRACE() do { } while(0)
 #endif
 
-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 07/11] i40e: add log messages when rx bulk mode is not usable
  2014-08-26 14:09 [dpdk-dev] [PATCH 00/11] cleanup logs in main PMDs David Marchand
                   ` (5 preceding siblings ...)
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 06/11] i40e: always log init messages David Marchand
@ 2014-08-26 14:09 ` David Marchand
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 08/11] e1000: clean log messages David Marchand
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: David Marchand @ 2014-08-26 14:09 UTC (permalink / raw)
  To: dev

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_pmd_i40e/i40e_rxtx.c |   29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
index d592ad6..de3701d 100644
--- a/lib/librte_pmd_i40e/i40e_rxtx.c
+++ b/lib/librte_pmd_i40e/i40e_rxtx.c
@@ -537,15 +537,34 @@ check_rx_burst_bulk_alloc_preconditions(__rte_unused struct i40e_rx_queue *rxq)
 	int ret = 0;
 
 #ifdef RTE_LIBRTE_I40E_RX_ALLOW_BULK_ALLOC
-	if (!(rxq->rx_free_thresh >= RTE_PMD_I40E_RX_MAX_BURST))
+	if (!(rxq->rx_free_thresh >= RTE_PMD_I40E_RX_MAX_BURST)) {
+		PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions: "
+			     "rxq->rx_free_thresh=%d, "
+			     "RTE_PMD_I40E_RX_MAX_BURST=%d\n",
+			     rxq->rx_free_thresh, RTE_PMD_I40E_RX_MAX_BURST);
 		ret = -EINVAL;
-	else if (!(rxq->rx_free_thresh < rxq->nb_rx_desc))
+	} else if (!(rxq->rx_free_thresh < rxq->nb_rx_desc)) {
+		PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions: "
+			     "rxq->rx_free_thresh=%d, "
+			     "rxq->nb_rx_desc=%d\n",
+			     rxq->rx_free_thresh, rxq->nb_rx_desc);
 		ret = -EINVAL;
-	else if (!(rxq->nb_rx_desc % rxq->rx_free_thresh) == 0)
+	} else if (!(rxq->nb_rx_desc % rxq->rx_free_thresh) == 0) {
+		PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions: "
+			     "rxq->nb_rx_desc=%d, "
+			     "rxq->rx_free_thresh=%d\n",
+			     rxq->nb_rx_desc, rxq->rx_free_thresh);
 		ret = -EINVAL;
-	else if (!(rxq->nb_rx_desc < (I40E_MAX_RING_DESC -
-				RTE_PMD_I40E_RX_MAX_BURST)))
+	} else if (!(rxq->nb_rx_desc < (I40E_MAX_RING_DESC -
+				RTE_PMD_I40E_RX_MAX_BURST))) {
+		PMD_INIT_LOG(DEBUG, "Rx Burst Bulk Alloc Preconditions: "
+			     "rxq->nb_rx_desc=%d, "
+			     "I40E_MAX_RING_DESC=%d, "
+			     "RTE_PMD_I40E_RX_MAX_BURST=%d\n",
+			     rxq->nb_rx_desc, I40E_MAX_RING_DESC,
+			     RTE_PMD_I40E_RX_MAX_BURST);
 		ret = -EINVAL;
+	}
 #else
 	ret = -EINVAL;
 #endif
-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 08/11] e1000: clean log messages
  2014-08-26 14:09 [dpdk-dev] [PATCH 00/11] cleanup logs in main PMDs David Marchand
                   ` (6 preceding siblings ...)
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 07/11] i40e: add log messages when rx bulk mode is not usable David Marchand
@ 2014-08-26 14:09 ` David Marchand
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 09/11] e1000: always log init messages David Marchand
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: David Marchand @ 2014-08-26 14:09 UTC (permalink / raw)
  To: dev

Clean log messages:
- remove superfluous \n in log macros and add some \n where needed,
- remove leading \n in some messages,
- split multi lines messages,
- introduce PMD_INIT_FUNC_TRACE and replace some PMD_INIT_LOG(DEBUG, "func\n").

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_pmd_e1000/e1000_logs.h |   12 +++---
 lib/librte_pmd_e1000/em_ethdev.c  |   47 +++++++++++------------
 lib/librte_pmd_e1000/em_rxtx.c    |   35 ++++++++---------
 lib/librte_pmd_e1000/igb_ethdev.c |   75 ++++++++++++++++++-------------------
 lib/librte_pmd_e1000/igb_rxtx.c   |    9 +++--
 5 files changed, 85 insertions(+), 93 deletions(-)

diff --git a/lib/librte_pmd_e1000/e1000_logs.h b/lib/librte_pmd_e1000/e1000_logs.h
index b6b3bb7..02f5930 100644
--- a/lib/librte_pmd_e1000/e1000_logs.h
+++ b/lib/librte_pmd_e1000/e1000_logs.h
@@ -36,35 +36,37 @@
 
 #ifdef RTE_LIBRTE_E1000_DEBUG_INIT
 #define PMD_INIT_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
+#define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>\n")
 #else
 #define PMD_INIT_LOG(level, fmt, args...) do { } while(0)
+#define PMD_INIT_FUNC_TRACE() do { } while(0)
 #endif
 
 #ifdef RTE_LIBRTE_E1000_DEBUG_RX
 #define PMD_RX_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
 #else
 #define PMD_RX_LOG(level, fmt, args...) do { } while(0)
 #endif
 
 #ifdef RTE_LIBRTE_E1000_DEBUG_TX
 #define PMD_TX_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
 #else
 #define PMD_TX_LOG(level, fmt, args...) do { } while(0)
 #endif
 
 #ifdef RTE_LIBRTE_E1000_DEBUG_TX_FREE
 #define PMD_TX_FREE_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
 #else
 #define PMD_TX_FREE_LOG(level, fmt, args...) do { } while(0)
 #endif
 
 #ifdef RTE_LIBRTE_E1000_DEBUG_DRIVER
 #define PMD_DRV_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
+	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
 #else
 #define PMD_DRV_LOG(level, fmt, args...) do { } while(0)
 #endif
diff --git a/lib/librte_pmd_e1000/em_ethdev.c b/lib/librte_pmd_e1000/em_ethdev.c
index 4555294..f359774 100644
--- a/lib/librte_pmd_e1000/em_ethdev.c
+++ b/lib/librte_pmd_e1000/em_ethdev.c
@@ -249,9 +249,9 @@ eth_em_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 	if (e1000_setup_init_funcs(hw, TRUE) != E1000_SUCCESS ||
 			em_hw_init(hw) != 0) {
 		PMD_INIT_LOG(ERR, "port_id %d vendorID=0x%x deviceID=0x%x: "
-			"failed to init HW",
-			eth_dev->data->port_id, pci_dev->id.vendor_id,
-			pci_dev->id.device_id);
+			     "failed to init HW\n",
+			     eth_dev->data->port_id, pci_dev->id.vendor_id,
+			     pci_dev->id.device_id);
 		return -(ENODEV);
 	}
 
@@ -260,8 +260,8 @@ eth_em_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 			hw->mac.rar_entry_count, 0);
 	if (eth_dev->data->mac_addrs == NULL) {
 		PMD_INIT_LOG(ERR, "Failed to allocate %d bytes needed to "
-			"store MAC addresses",
-			ETHER_ADDR_LEN * hw->mac.rar_entry_count);
+			     "store MAC addresses\n",
+			     ETHER_ADDR_LEN * hw->mac.rar_entry_count);
 		return -(ENOMEM);
 	}
 
@@ -350,7 +350,7 @@ em_hw_init(struct e1000_hw *hw)
 		 */
 		diag = e1000_validate_nvm_checksum(hw);
 		if (diag < 0) {
-			PMD_INIT_LOG(ERR, "EEPROM checksum invalid");
+			PMD_INIT_LOG(ERR, "EEPROM checksum invalid\n");
 			goto error;
 		}
 	}
@@ -358,14 +358,14 @@ em_hw_init(struct e1000_hw *hw)
 	/* Read the permanent MAC address out of the EEPROM */
 	diag = e1000_read_mac_addr(hw);
 	if (diag != 0) {
-		PMD_INIT_LOG(ERR, "EEPROM error while reading MAC address");
+		PMD_INIT_LOG(ERR, "EEPROM error while reading MAC address\n");
 		goto error;
 	}
 
 	/* Now initialize the hardware */
 	diag = em_hardware_init(hw);
 	if (diag != 0) {
-		PMD_INIT_LOG(ERR, "Hardware initialization failed");
+		PMD_INIT_LOG(ERR, "Hardware initialization failed\n");
 		goto error;
 	}
 
@@ -375,7 +375,7 @@ em_hw_init(struct e1000_hw *hw)
 	diag = e1000_check_reset_block(hw);
 	if (diag < 0) {
 		PMD_INIT_LOG(ERR, "PHY reset is blocked due to "
-			"SOL/IDER session");
+			     "SOL/IDER session\n");
 	}
 	return (0);
 
@@ -390,11 +390,10 @@ eth_em_configure(struct rte_eth_dev *dev)
 	struct e1000_interrupt *intr =
 		E1000_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 
-	PMD_INIT_LOG(DEBUG, ">>");
-
+	PMD_INIT_FUNC_TRACE();
 	intr->flags |= E1000_FLAG_NEED_LINK_UPDATE;
+	PMD_INIT_FUNC_TRACE();
 
-	PMD_INIT_LOG(DEBUG, "<<");
 	return (0);
 }
 
@@ -453,7 +452,7 @@ eth_em_start(struct rte_eth_dev *dev)
 		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	int ret, mask;
 
-	PMD_INIT_LOG(DEBUG, ">>");
+	PMD_INIT_FUNC_TRACE();
 
 	eth_em_stop(dev);
 
@@ -478,7 +477,7 @@ eth_em_start(struct rte_eth_dev *dev)
 
 	/* Initialize the hardware */
 	if (em_hardware_init(hw)) {
-		PMD_INIT_LOG(ERR, "Unable to initialize the hardware");
+		PMD_INIT_LOG(ERR, "Unable to initialize the hardware\n");
 		return (-EIO);
 	}
 
@@ -491,7 +490,7 @@ eth_em_start(struct rte_eth_dev *dev)
 
 	ret = eth_em_rx_init(dev);
 	if (ret) {
-		PMD_INIT_LOG(ERR, "Unable to initialize RX hardware");
+		PMD_INIT_LOG(ERR, "Unable to initialize RX hardware\n");
 		em_dev_clear_queues(dev);
 		return ret;
 	}
@@ -562,7 +561,7 @@ eth_em_start(struct rte_eth_dev *dev)
 	if (dev->data->dev_conf.intr_conf.lsc != 0) {
 		ret = eth_em_interrupt_setup(dev);
 		if (ret) {
-			PMD_INIT_LOG(ERR, "Unable to setup interrupts");
+			PMD_INIT_LOG(ERR, "Unable to setup interrupts\n");
 			em_dev_clear_queues(dev);
 			return ret;
 		}
@@ -1305,11 +1304,9 @@ eth_em_interrupt_action(struct rte_eth_dev *dev)
 		PMD_INIT_LOG(INFO, " Port %d: Link Down\n",
 					dev->data->port_id);
 	}
-	PMD_INIT_LOG(INFO, "PCI Address: %04d:%02d:%02d:%d",
-				dev->pci_dev->addr.domain,
-				dev->pci_dev->addr.bus,
-				dev->pci_dev->addr.devid,
-				dev->pci_dev->addr.function);
+	PMD_INIT_LOG(INFO, "PCI Address: %04d:%02d:%02d:%d\n",
+		     dev->pci_dev->addr.domain, dev->pci_dev->addr.bus,
+		     dev->pci_dev->addr.devid, dev->pci_dev->addr.function);
 	tctl = E1000_READ_REG(hw, E1000_TCTL);
 	rctl = E1000_READ_REG(hw, E1000_RCTL);
 	if (link.link_status) {
@@ -1429,14 +1426,14 @@ eth_em_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 	if (fc_conf->autoneg != hw->mac.autoneg)
 		return -ENOTSUP;
 	rx_buf_size = em_get_rx_buffer_size(hw);
-	PMD_INIT_LOG(DEBUG, "Rx packet buffer size = 0x%x \n", rx_buf_size);
+	PMD_INIT_LOG(DEBUG, "Rx packet buffer size = 0x%x\n", rx_buf_size);
 
 	/* At least reserve one Ethernet frame for watermark */
 	max_high_water = rx_buf_size - ETHER_MAX_LEN;
 	if ((fc_conf->high_water > max_high_water) ||
 		(fc_conf->high_water < fc_conf->low_water)) {
-		PMD_INIT_LOG(ERR, "e1000 incorrect high/low water value \n");
-		PMD_INIT_LOG(ERR, "high water must <= 0x%x \n", max_high_water);
+		PMD_INIT_LOG(ERR, "e1000 incorrect high/low water value\n");
+		PMD_INIT_LOG(ERR, "high water must <= 0x%x\n", max_high_water);
 		return (-EINVAL);
 	}
 
@@ -1466,7 +1463,7 @@ eth_em_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 		return 0;
 	}
 
-	PMD_INIT_LOG(ERR, "e1000_setup_link_generic = 0x%x \n", err);
+	PMD_INIT_LOG(ERR, "e1000_setup_link_generic = 0x%x\n", err);
 	return (-EIO);
 }
 
diff --git a/lib/librte_pmd_e1000/em_rxtx.c b/lib/librte_pmd_e1000/em_rxtx.c
index f254858..fd082f8 100644
--- a/lib/librte_pmd_e1000/em_rxtx.c
+++ b/lib/librte_pmd_e1000/em_rxtx.c
@@ -317,10 +317,8 @@ em_xmit_cleanup(struct em_tx_queue *txq)
 	desc_to_clean_to = sw_ring[desc_to_clean_to].last_id;
 	if (! (txr[desc_to_clean_to].upper.fields.status & E1000_TXD_STAT_DD))
 	{
-		PMD_TX_FREE_LOG(DEBUG,
-				"TX descriptor %4u is not done"
-				"(port=%d queue=%d)",
-				desc_to_clean_to,
+		PMD_TX_FREE_LOG(DEBUG, "TX descriptor %4u is not done"
+				"(port=%d queue=%d)\n", desc_to_clean_to,
 				txq->port_id, txq->queue_id);
 		/* Failed to clean any descriptors, better luck next time */
 		return -(1);
@@ -334,11 +332,10 @@ em_xmit_cleanup(struct em_tx_queue *txq)
 		nb_tx_to_clean = (uint16_t)(desc_to_clean_to -
 						last_desc_cleaned);
 
-	PMD_TX_FREE_LOG(DEBUG,
-			"Cleaning %4u TX descriptors: %4u to %4u "
-			"(port=%d queue=%d)",
-			nb_tx_to_clean, last_desc_cleaned, desc_to_clean_to,
-			txq->port_id, txq->queue_id);
+	PMD_TX_FREE_LOG(DEBUG, "Cleaning %4u TX descriptors: %4u to %4u "
+			"(port=%d queue=%d)\n", nb_tx_to_clean,
+			last_desc_cleaned, desc_to_clean_to, txq->port_id,
+			txq->queue_id);
 
 	/*
 	 * The last descriptor to clean is done, so that means all the
@@ -464,10 +461,9 @@ eth_em_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		 * nb_used better be less than or equal to txq->tx_rs_thresh
 		 */
 		while (unlikely (nb_used > txq->nb_tx_free)) {
-			PMD_TX_FREE_LOG(DEBUG,
-					"Not enough free TX descriptors "
+			PMD_TX_FREE_LOG(DEBUG, "Not enough free TX descriptors "
 					"nb_used=%4u nb_free=%4u "
-					"(port=%d queue=%d)",
+					"(port=%d queue=%d)\n",
 					nb_used, txq->nb_tx_free,
 					txq->port_id, txq->queue_id);
 
@@ -588,9 +584,8 @@ eth_em_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 
 		/* Set RS bit only on threshold packets' last descriptor */
 		if (txq->nb_tx_used >= txq->tx_rs_thresh) {
-			PMD_TX_FREE_LOG(DEBUG,
-					"Setting RS bit on TXD id="
-					"%4u (port=%d queue=%d)",
+			PMD_TX_FREE_LOG(DEBUG, "Setting RS bit on TXD id=%4u "
+					"(port=%d queue=%d)\n",
 					tx_last, txq->port_id, txq->queue_id);
 
 			cmd_type_len |= E1000_TXD_CMD_RS;
@@ -606,9 +601,9 @@ end_of_tx:
 	/*
 	 * Set the Transmit Descriptor Tail (TDT)
 	 */
-	PMD_TX_LOG(DEBUG, "port_id=%u queue_id=%u tx_tail=%u nb_tx=%u",
-		(unsigned) txq->port_id, (unsigned) txq->queue_id,
-		(unsigned) tx_id, (unsigned) nb_tx);
+	PMD_TX_LOG(DEBUG, "port_id=%u queue_id=%u tx_tail=%u nb_tx=%u\n",
+		   (unsigned) txq->port_id, (unsigned) txq->queue_id,
+		   (unsigned) tx_id, (unsigned) nb_tx);
 	E1000_PCI_REG_WRITE(txq->tdt_reg_addr, tx_id);
 	txq->tx_tail = tx_id;
 
@@ -712,7 +707,7 @@ eth_em_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		 * to happen by sending specific "back-pressure" flow control
 		 * frames to its peer(s).
 		 */
-		PMD_RX_LOG(DEBUG, "\nport_id=%u queue_id=%u rx_id=%u "
+		PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_id=%u "
 			"status=0x%x pkt_len=%u\n",
 			(unsigned) rxq->port_id, (unsigned) rxq->queue_id,
 			(unsigned) rx_id, (unsigned) status,
@@ -892,7 +887,7 @@ eth_em_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		 * to happen by sending specific "back-pressure" flow control
 		 * frames to its peer(s).
 		 */
-		PMD_RX_LOG(DEBUG, "\nport_id=%u queue_id=%u rx_id=%u "
+		PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_id=%u "
 			"status=0x%x data_len=%u\n",
 			(unsigned) rxq->port_id, (unsigned) rxq->queue_id,
 			(unsigned) rx_id, (unsigned) status,
diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c
index 3187d92..4d91aa7 100644
--- a/lib/librte_pmd_e1000/igb_ethdev.c
+++ b/lib/librte_pmd_e1000/igb_ethdev.c
@@ -509,7 +509,7 @@ eth_igb_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 		 * if it fails a second time its a real issue.
 		 */
 		if (e1000_validate_nvm_checksum(hw) < 0) {
-			PMD_INIT_LOG(ERR, "EEPROM checksum invalid");
+			PMD_INIT_LOG(ERR, "EEPROM checksum invalid\n");
 			error = -EIO;
 			goto err_late;
 		}
@@ -517,7 +517,7 @@ eth_igb_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 
 	/* Read the permanent MAC address out of the EEPROM */
 	if (e1000_read_mac_addr(hw) != 0) {
-		PMD_INIT_LOG(ERR, "EEPROM error while reading MAC address");
+		PMD_INIT_LOG(ERR, "EEPROM error while reading MAC address\n");
 		error = -EIO;
 		goto err_late;
 	}
@@ -527,8 +527,8 @@ eth_igb_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 		ETHER_ADDR_LEN * hw->mac.rar_entry_count, 0);
 	if (eth_dev->data->mac_addrs == NULL) {
 		PMD_INIT_LOG(ERR, "Failed to allocate %d bytes needed to "
-						"store MAC addresses",
-				ETHER_ADDR_LEN * hw->mac.rar_entry_count);
+			     "store MAC addresses\n",
+			     ETHER_ADDR_LEN * hw->mac.rar_entry_count);
 		error = -ENOMEM;
 		goto err_late;
 	}
@@ -541,7 +541,7 @@ eth_igb_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 
 	/* Now initialize the hardware */
 	if (igb_hardware_init(hw) != 0) {
-		PMD_INIT_LOG(ERR, "Hardware initialization failed");
+		PMD_INIT_LOG(ERR, "Hardware initialization failed\n");
 		rte_free(eth_dev->data->mac_addrs);
 		eth_dev->data->mac_addrs = NULL;
 		error = -ENODEV;
@@ -552,7 +552,7 @@ eth_igb_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 	/* Indicate SOL/IDER usage */
 	if (e1000_check_reset_block(hw) < 0) {
 		PMD_INIT_LOG(ERR, "PHY reset is blocked due to"
-					"SOL/IDER session");
+			     "SOL/IDER session\n");
 	}
 
 	/* initialize PF if max_vfs not zero */
@@ -597,7 +597,7 @@ eth_igbvf_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 		E1000_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
 	int diag;
 
-	PMD_INIT_LOG(DEBUG, "eth_igbvf_dev_init");
+	PMD_INIT_FUNC_TRACE();
 
 	eth_dev->dev_ops = &igbvf_eth_dev_ops;
 	eth_dev->rx_pkt_burst = &eth_igb_recv_pkts;
@@ -621,8 +621,8 @@ eth_igbvf_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 	/* Initialize the shared code (base driver) */
 	diag = e1000_setup_init_funcs(hw, TRUE);
 	if (diag != 0) {
-		PMD_INIT_LOG(ERR, "Shared code init failed for igbvf: %d",
-			diag);
+		PMD_INIT_LOG(ERR, "Shared code init failed for igbvf: %d\n",
+			     diag);
 		return -EIO;
 	}
 
@@ -638,10 +638,9 @@ eth_igbvf_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 	eth_dev->data->mac_addrs = rte_zmalloc("igbvf", ETHER_ADDR_LEN *
 		hw->mac.rar_entry_count, 0);
 	if (eth_dev->data->mac_addrs == NULL) {
-		PMD_INIT_LOG(ERR,
-			"Failed to allocate %d bytes needed to store MAC "
-			"addresses",
-			ETHER_ADDR_LEN * hw->mac.rar_entry_count);
+		PMD_INIT_LOG(ERR, "Failed to allocate %d bytes needed to "
+			     "store MAC addresses\n",
+			     ETHER_ADDR_LEN * hw->mac.rar_entry_count);
 		return -ENOMEM;
 	}
 
@@ -649,7 +648,7 @@ eth_igbvf_dev_init(__attribute__((unused)) struct eth_driver *eth_drv,
 	ether_addr_copy((struct ether_addr *) hw->mac.perm_addr,
 			&eth_dev->data->mac_addrs[0]);
 
-	PMD_INIT_LOG(DEBUG, "\nport %d vendorID=0x%x deviceID=0x%x "
+	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x "
 			"mac.type=%s\n",
 			eth_dev->data->port_id, pci_dev->id.vendor_id,
 			pci_dev->id.device_id,
@@ -719,11 +718,9 @@ eth_igb_configure(struct rte_eth_dev *dev)
 	struct e1000_interrupt *intr =
 		E1000_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 
-	PMD_INIT_LOG(DEBUG, ">>");
-
+	PMD_INIT_FUNC_TRACE();
 	intr->flags |= E1000_FLAG_NEED_LINK_UPDATE;
-
-	PMD_INIT_LOG(DEBUG, "<<");
+	PMD_INIT_FUNC_TRACE();
 
 	return (0);
 }
@@ -736,7 +733,7 @@ eth_igb_start(struct rte_eth_dev *dev)
 	int ret, i, mask;
 	uint32_t ctrl_ext;
 
-	PMD_INIT_LOG(DEBUG, ">>");
+	PMD_INIT_FUNC_TRACE();
 
 	/* Power up the phy. Needed to make the link go Up */
 	e1000_power_up_phy(hw);
@@ -758,7 +755,7 @@ eth_igb_start(struct rte_eth_dev *dev)
 
 	/* Initialize the hardware */
 	if (igb_hardware_init(hw)) {
-		PMD_INIT_LOG(ERR, "Unable to initialize the hardware");
+		PMD_INIT_LOG(ERR, "Unable to initialize the hardware\n");
 		return (-EIO);
 	}
 
@@ -781,7 +778,7 @@ eth_igb_start(struct rte_eth_dev *dev)
 	/* This can fail when allocating mbufs for descriptor rings */
 	ret = eth_igb_rx_init(dev);
 	if (ret) {
-		PMD_INIT_LOG(ERR, "Unable to initialize RX hardware");
+		PMD_INIT_LOG(ERR, "Unable to initialize RX hardware\n");
 		igb_dev_clear_queues(dev);
 		return ret;
 	}
@@ -1797,11 +1794,11 @@ eth_igb_interrupt_action(struct rte_eth_dev *dev)
 			PMD_INIT_LOG(INFO, " Port %d: Link Down\n",
 						dev->data->port_id);
 		}
-		PMD_INIT_LOG(INFO, "PCI Address: %04d:%02d:%02d:%d",
-					dev->pci_dev->addr.domain,
-					dev->pci_dev->addr.bus,
-					dev->pci_dev->addr.devid,
-					dev->pci_dev->addr.function);
+		PMD_INIT_LOG(INFO, "PCI Address: %04d:%02d:%02d:%d\n",
+			     dev->pci_dev->addr.domain,
+			     dev->pci_dev->addr.bus,
+			     dev->pci_dev->addr.devid,
+			     dev->pci_dev->addr.function);
 		tctl = E1000_READ_REG(hw, E1000_TCTL);
 		rctl = E1000_READ_REG(hw, E1000_RCTL);
 		if (link.link_status) {
@@ -1922,14 +1919,14 @@ eth_igb_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 	if (fc_conf->autoneg != hw->mac.autoneg)
 		return -ENOTSUP;
 	rx_buf_size = igb_get_rx_buffer_size(hw);
-	PMD_INIT_LOG(DEBUG, "Rx packet buffer size = 0x%x \n", rx_buf_size);
+	PMD_INIT_LOG(DEBUG, "Rx packet buffer size = 0x%x\n", rx_buf_size);
 
 	/* At least reserve one Ethernet frame for watermark */
 	max_high_water = rx_buf_size - ETHER_MAX_LEN;
 	if ((fc_conf->high_water > max_high_water) ||
 		(fc_conf->high_water < fc_conf->low_water)) {
-		PMD_INIT_LOG(ERR, "e1000 incorrect high/low water value \n");
-		PMD_INIT_LOG(ERR, "high water must <=  0x%x \n", max_high_water);
+		PMD_INIT_LOG(ERR, "e1000 incorrect high/low water value\n");
+		PMD_INIT_LOG(ERR, "high water must <=  0x%x\n", max_high_water);
 		return (-EINVAL);
 	}
 
@@ -1959,7 +1956,7 @@ eth_igb_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 		return 0;
 	}
 
-	PMD_INIT_LOG(ERR, "e1000_setup_link_generic = 0x%x \n", err);
+	PMD_INIT_LOG(ERR, "e1000_setup_link_generic = 0x%x\n", err);
 	return (-EIO);
 }
 
@@ -1994,7 +1991,7 @@ eth_igb_rar_clear(struct rte_eth_dev *dev, uint32_t index)
 static void
 igbvf_intr_disable(struct e1000_hw *hw)
 {
-	PMD_INIT_LOG(DEBUG, "igbvf_intr_disable");
+	PMD_INIT_FUNC_TRACE();
 
 	/* Clear interrupt mask to stop from interrupts being generated */
 	E1000_WRITE_REG(hw, E1000_EIMC, 0xFFFF);
@@ -2076,7 +2073,7 @@ igbvf_dev_configure(struct rte_eth_dev *dev)
 {
 	struct rte_eth_conf* conf = &dev->data->dev_conf;
 
-	PMD_INIT_LOG(DEBUG, "\nConfigured Virtual Function port id: %d\n",
+	PMD_INIT_LOG(DEBUG, "Configured Virtual Function port id: %d\n",
 		dev->data->port_id);
 
 	/*
@@ -2105,7 +2102,7 @@ igbvf_dev_start(struct rte_eth_dev *dev)
 		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	int ret;
 
-	PMD_INIT_LOG(DEBUG, "igbvf_dev_start");
+	PMD_INIT_FUNC_TRACE();
 
 	hw->mac.ops.reset_hw(hw);
 
@@ -2117,7 +2114,7 @@ igbvf_dev_start(struct rte_eth_dev *dev)
 	/* This can fail when allocating mbufs for descriptor rings */
 	ret = eth_igbvf_rx_init(dev);
 	if (ret) {
-		PMD_INIT_LOG(ERR, "Unable to initialize RX hardware");
+		PMD_INIT_LOG(ERR, "Unable to initialize RX hardware\n");
 		igb_dev_clear_queues(dev);
 		return ret;
 	}
@@ -2128,7 +2125,7 @@ igbvf_dev_start(struct rte_eth_dev *dev)
 static void
 igbvf_dev_stop(struct rte_eth_dev *dev)
 {
-	PMD_INIT_LOG(DEBUG, "igbvf_dev_stop");
+	PMD_INIT_FUNC_TRACE();
 
 	igbvf_stop_adapter(dev);
 
@@ -2146,7 +2143,7 @@ igbvf_dev_close(struct rte_eth_dev *dev)
 {
 	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-	PMD_INIT_LOG(DEBUG, "igbvf_dev_close");
+	PMD_INIT_FUNC_TRACE();
 
 	e1000_reset_hw(hw);
 
@@ -2202,12 +2199,12 @@ igbvf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
 	uint32_t vid_bit = 0;
 	int ret = 0;
 
-	PMD_INIT_LOG(DEBUG, "igbvf_vlan_filter_set");
+	PMD_INIT_FUNC_TRACE();
 
 	/*vind is not used in VF driver, set to 0, check ixgbe_set_vfta_vf*/
 	ret = igbvf_set_vfta(hw, vlan_id, !!on);
 	if(ret){
-		PMD_INIT_LOG(ERR, "Unable to set VF vlan");
+		PMD_INIT_LOG(ERR, "Unable to set VF vlan\n");
 		return ret;
 	}
 	vid_idx = (uint32_t) ((vlan_id >> 5) & 0x7F);
@@ -2431,7 +2428,7 @@ eth_igb_add_ethertype_filter(struct rte_eth_dev *dev, uint16_t index,
 
 	if (filter->priority_en) {
 		PMD_INIT_LOG(ERR, "vlan and priority (%d) is not supported"
-			" in E1000.", filter->priority);
+			     " in E1000.\n", filter->priority);
 		return -EINVAL;
 	}
 
diff --git a/lib/librte_pmd_e1000/igb_rxtx.c b/lib/librte_pmd_e1000/igb_rxtx.c
index 977c4a2..9a34710 100644
--- a/lib/librte_pmd_e1000/igb_rxtx.c
+++ b/lib/librte_pmd_e1000/igb_rxtx.c
@@ -555,7 +555,7 @@ eth_igb_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	 * Set the Transmit Descriptor Tail (TDT).
 	 */
 	E1000_PCI_REG_WRITE(txq->tdt_reg_addr, tx_id);
-	PMD_TX_LOG(DEBUG, "port_id=%u queue_id=%u tx_tail=%u nb_tx=%u",
+	PMD_TX_LOG(DEBUG, "port_id=%u queue_id=%u tx_tail=%u nb_tx=%u\n",
 		   (unsigned) txq->port_id, (unsigned) txq->queue_id,
 		   (unsigned) tx_id, (unsigned) nb_tx);
 	txq->tx_tail = tx_id;
@@ -697,7 +697,7 @@ eth_igb_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		 * to happen by sending specific "back-pressure" flow control
 		 * frames to its peer(s).
 		 */
-		PMD_RX_LOG(DEBUG, "\nport_id=%u queue_id=%u rx_id=%u "
+		PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_id=%u "
 			   "staterr=0x%x pkt_len=%u\n",
 			   (unsigned) rxq->port_id, (unsigned) rxq->queue_id,
 			   (unsigned) rx_id, (unsigned) staterr,
@@ -881,7 +881,7 @@ eth_igb_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		 * to happen by sending specific "back-pressure" flow control
 		 * frames to its peer(s).
 		 */
-		PMD_RX_LOG(DEBUG, "\nport_id=%u queue_id=%u rx_id=%u "
+		PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_id=%u "
 			   "staterr=0x%x data_len=%u\n",
 			   (unsigned) rxq->port_id, (unsigned) rxq->queue_id,
 			   (unsigned) rx_id, (unsigned) staterr,
@@ -1741,7 +1741,8 @@ igb_vmdq_rx_hw_configure(struct rte_eth_dev *dev)
 	uint32_t mrqc, vt_ctl, vmolr, rctl;
 	int i;
 
-	PMD_INIT_LOG(DEBUG, ">>");
+	PMD_INIT_FUNC_TRACE();
+
 	hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	cfg = &dev->data->dev_conf.rx_adv_conf.vmdq_rx_conf;
 
-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 09/11] e1000: always log init messages
  2014-08-26 14:09 [dpdk-dev] [PATCH 00/11] cleanup logs in main PMDs David Marchand
                   ` (7 preceding siblings ...)
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 08/11] e1000: clean log messages David Marchand
@ 2014-08-26 14:09 ` David Marchand
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 10/11] e1000: add a message when forcing scatter mode David Marchand
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 11/11] eal: set log level from command line David Marchand
  10 siblings, 0 replies; 17+ messages in thread
From: David Marchand @ 2014-08-26 14:09 UTC (permalink / raw)
  To: dev

'init' messages should always be logged and filtered at runtime by rte_log.
All the more so as these messages are not in the datapath.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_pmd_e1000/e1000_logs.h |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/librte_pmd_e1000/e1000_logs.h b/lib/librte_pmd_e1000/e1000_logs.h
index 02f5930..791705c 100644
--- a/lib/librte_pmd_e1000/e1000_logs.h
+++ b/lib/librte_pmd_e1000/e1000_logs.h
@@ -34,12 +34,13 @@
 #ifndef _E1000_LOGS_H_
 #define _E1000_LOGS_H_
 
-#ifdef RTE_LIBRTE_E1000_DEBUG_INIT
 #define PMD_INIT_LOG(level, fmt, args...) \
-	RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
+	rte_log(RTE_LOG_ ## level, RTE_LOGTYPE_PMD, \
+		"PMD: %s(): " fmt, __func__, ##args)
+
+#ifdef RTE_LIBRTE_E1000_DEBUG_INIT
 #define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>\n")
 #else
-#define PMD_INIT_LOG(level, fmt, args...) do { } while(0)
 #define PMD_INIT_FUNC_TRACE() do { } while(0)
 #endif
 
-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 10/11] e1000: add a message when forcing scatter mode
  2014-08-26 14:09 [dpdk-dev] [PATCH 00/11] cleanup logs in main PMDs David Marchand
                   ` (8 preceding siblings ...)
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 09/11] e1000: always log init messages David Marchand
@ 2014-08-26 14:09 ` David Marchand
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 11/11] eal: set log level from command line David Marchand
  10 siblings, 0 replies; 17+ messages in thread
From: David Marchand @ 2014-08-26 14:09 UTC (permalink / raw)
  To: dev

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_pmd_e1000/em_rxtx.c  |    4 ++++
 lib/librte_pmd_e1000/igb_rxtx.c |   14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/lib/librte_pmd_e1000/em_rxtx.c b/lib/librte_pmd_e1000/em_rxtx.c
index fd082f8..19d18e7 100644
--- a/lib/librte_pmd_e1000/em_rxtx.c
+++ b/lib/librte_pmd_e1000/em_rxtx.c
@@ -1703,6 +1703,8 @@ eth_em_rx_init(struct rte_eth_dev *dev)
 		 */
 		if (dev->data->dev_conf.rxmode.jumbo_frame ||
 				rctl_bsize < ETHER_MAX_LEN) {
+			if (!dev->data->scattered_rx)
+				PMD_INIT_LOG(DEBUG, "forcing scatter mode\n");
 			dev->rx_pkt_burst =
 				(eth_rx_burst_t)eth_em_recv_scattered_pkts;
 			dev->data->scattered_rx = 1;
@@ -1710,6 +1712,8 @@ eth_em_rx_init(struct rte_eth_dev *dev)
 	}
 
 	if (dev->data->dev_conf.rxmode.enable_scatter) {
+		if (!dev->data->scattered_rx)
+			PMD_INIT_LOG(DEBUG, "forcing scatter mode\n");
 		dev->rx_pkt_burst = eth_em_recv_scattered_pkts;
 		dev->data->scattered_rx = 1;
 	}
diff --git a/lib/librte_pmd_e1000/igb_rxtx.c b/lib/librte_pmd_e1000/igb_rxtx.c
index 9a34710..ddcd84c 100644
--- a/lib/librte_pmd_e1000/igb_rxtx.c
+++ b/lib/librte_pmd_e1000/igb_rxtx.c
@@ -1980,6 +1980,9 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 			/* It adds dual VLAN length for supporting dual VLAN */
 			if ((dev->data->dev_conf.rxmode.max_rx_pkt_len +
 						2 * VLAN_TAG_SIZE) > buf_size){
+				if (!dev->data->scattered_rx)
+					PMD_INIT_LOG(DEBUG,
+						     "forcing scatter mode\n");
 				dev->rx_pkt_burst = eth_igb_recv_scattered_pkts;
 				dev->data->scattered_rx = 1;
 			}
@@ -1989,6 +1992,8 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 			 */
 			if ((rctl_bsize == 0) || (rctl_bsize > buf_size))
 				rctl_bsize = buf_size;
+			if (!dev->data->scattered_rx)
+				PMD_INIT_LOG(DEBUG, "forcing scatter mode\n");
 			dev->rx_pkt_burst = eth_igb_recv_scattered_pkts;
 			dev->data->scattered_rx = 1;
 		}
@@ -2010,6 +2015,8 @@ eth_igb_rx_init(struct rte_eth_dev *dev)
 	}
 
 	if (dev->data->dev_conf.rxmode.enable_scatter) {
+		if (!dev->data->scattered_rx)
+			PMD_INIT_LOG(DEBUG, "forcing scatter mode\n");
 		dev->rx_pkt_burst = eth_igb_recv_scattered_pkts;
 		dev->data->scattered_rx = 1;
 	}
@@ -2244,6 +2251,9 @@ eth_igbvf_rx_init(struct rte_eth_dev *dev)
 			/* It adds dual VLAN length for supporting dual VLAN */
 			if ((dev->data->dev_conf.rxmode.max_rx_pkt_len +
 						2 * VLAN_TAG_SIZE) > buf_size){
+				if (!dev->data->scattered_rx)
+					PMD_INIT_LOG(DEBUG,
+						     "forcing scatter mode\n");
 				dev->rx_pkt_burst = eth_igb_recv_scattered_pkts;
 				dev->data->scattered_rx = 1;
 			}
@@ -2253,6 +2263,8 @@ eth_igbvf_rx_init(struct rte_eth_dev *dev)
 			 */
 			if ((rctl_bsize == 0) || (rctl_bsize > buf_size))
 				rctl_bsize = buf_size;
+			if (!dev->data->scattered_rx)
+				PMD_INIT_LOG(DEBUG, "forcing scatter mode\n");
 			dev->rx_pkt_burst = eth_igb_recv_scattered_pkts;
 			dev->data->scattered_rx = 1;
 		}
@@ -2284,6 +2296,8 @@ eth_igbvf_rx_init(struct rte_eth_dev *dev)
 	}
 
 	if (dev->data->dev_conf.rxmode.enable_scatter) {
+		if (!dev->data->scattered_rx)
+			PMD_INIT_LOG(DEBUG, "forcing scatter mode\n");
 		dev->rx_pkt_burst = eth_igb_recv_scattered_pkts;
 		dev->data->scattered_rx = 1;
 	}
-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-dev] [PATCH 11/11] eal: set log level from command line
  2014-08-26 14:09 [dpdk-dev] [PATCH 00/11] cleanup logs in main PMDs David Marchand
                   ` (9 preceding siblings ...)
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 10/11] e1000: add a message when forcing scatter mode David Marchand
@ 2014-08-26 14:09 ` David Marchand
  10 siblings, 0 replies; 17+ messages in thread
From: David Marchand @ 2014-08-26 14:09 UTC (permalink / raw)
  To: dev

Add a --log-level option to set the default eal log level.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal.c                    |   43 ++++++++++++++++++++
 .../bsdapp/eal/include/eal_internal_cfg.h          |    1 +
 lib/librte_eal/linuxapp/eal/eal.c                  |   43 ++++++++++++++++++++
 .../linuxapp/eal/include/eal_internal_cfg.h        |    1 +
 4 files changed, 88 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 38c6cfc..96b54e3 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -94,6 +94,7 @@
 #define OPT_PCI_BLACKLIST "pci-blacklist"
 #define OPT_VDEV        "vdev"
 #define OPT_SYSLOG      "syslog"
+#define OPT_LOG_LEVEL   "log-level"
 
 #define RTE_EAL_BLACKLIST_SIZE	0x100
 
@@ -311,6 +312,7 @@ eal_usage(const char *prgname)
 	       "  -v           : Display version information on startup\n"
 	       "  -m MB        : memory to allocate\n"
 	       "  -r NUM       : force number of memory ranks (don't detect)\n"
+	       "  --"OPT_LOG_LEVEL"  : set default log level\n"
 	       "  --"OPT_PROC_TYPE"  : type of this process\n"
 	       "  --"OPT_PCI_BLACKLIST", -b: add a PCI device in black list.\n"
 	       "               Prevent EAL from using this PCI device. The argument\n"
@@ -458,6 +460,28 @@ eal_parse_syslog(const char *facility)
 	return -1;
 }
 
+static int
+eal_parse_log_level(const char *level, uint32_t *log_level)
+{
+	char *end;
+	unsigned long tmp;
+
+	errno = 0;
+	tmp = strtoul(level, &end, 0);
+
+	/* check for errors */
+	if ((errno != 0) || (level[0] == '\0') ||
+	    end == NULL || (*end != '\0'))
+		return -1;
+
+	/* log_level is a uint32_t */
+	if (tmp >= UINT32_MAX)
+		return -1;
+
+	*log_level = tmp;
+	return 0;
+}
+
 static inline size_t
 eal_get_hugepage_mem_size(void)
 {
@@ -512,6 +536,7 @@ eal_parse_args(int argc, char **argv)
 		{OPT_PCI_BLACKLIST, 1, 0, 0},
 		{OPT_VDEV, 1, 0, 0},
 		{OPT_SYSLOG, 1, NULL, 0},
+		{OPT_LOG_LEVEL, 1, NULL, 0},
 		{0, 0, 0, 0}
 	};
 
@@ -524,6 +549,8 @@ eal_parse_args(int argc, char **argv)
 	internal_config.hugepage_dir = NULL;
 	internal_config.force_sockets = 0;
 	internal_config.syslog_facility = LOG_DAEMON;
+	/* default value from build option */
+	internal_config.log_level = RTE_LOG_LEVEL;
 #ifdef RTE_LIBEAL_USE_HPET
 	internal_config.no_hpet = 0;
 #else
@@ -671,6 +698,19 @@ eal_parse_args(int argc, char **argv)
 					return -1;
 				}
 			}
+			else if (!strcmp(lgopts[option_index].name,
+					 OPT_LOG_LEVEL)) {
+				uint32_t log;
+
+				if (eal_parse_log_level(optarg, &log) < 0) {
+					RTE_LOG(ERR, EAL,
+						"invalid parameters for --"
+						OPT_LOG_LEVEL "\n");
+					eal_usage(prgname);
+					return -1;
+				}
+				internal_config.log_level = log;
+			}
 			break;
 
 		default:
@@ -811,6 +851,9 @@ rte_eal_init(int argc, char **argv)
 	if (fctret < 0)
 		exit(1);
 
+	/* set log level as early as possible */
+	rte_set_log_level(internal_config.log_level);
+
 	if (internal_config.no_hugetlbfs == 0 &&
 			internal_config.process_type != RTE_PROC_SECONDARY &&
 			eal_hugepage_info_init() < 0)
diff --git a/lib/librte_eal/bsdapp/eal/include/eal_internal_cfg.h b/lib/librte_eal/bsdapp/eal/include/eal_internal_cfg.h
index 2d06c7f..24cefc2 100644
--- a/lib/librte_eal/bsdapp/eal/include/eal_internal_cfg.h
+++ b/lib/librte_eal/bsdapp/eal/include/eal_internal_cfg.h
@@ -75,6 +75,7 @@ struct internal_config {
 	volatile uint64_t socket_mem[RTE_MAX_NUMA_NODES]; /**< amount of memory per socket */
 	uintptr_t base_virtaddr;          /**< base address to try and reserve memory from */
 	volatile int syslog_facility;	  /**< facility passed to openlog() */
+	volatile uint32_t log_level;	  /**< default log level */
 	const char *hugefile_prefix;      /**< the base filename of hugetlbfs files */
 	const char *hugepage_dir;         /**< specific hugetlbfs directory to use */
 
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index ad43914..874209b 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -97,6 +97,7 @@
 #define OPT_PCI_BLACKLIST "pci-blacklist"
 #define OPT_VDEV        "vdev"
 #define OPT_SYSLOG      "syslog"
+#define OPT_LOG_LEVEL   "log-level"
 #define OPT_BASE_VIRTADDR   "base-virtaddr"
 #define OPT_XEN_DOM0    "xen-dom0"
 #define OPT_CREATE_UIO_DEV "create-uio-dev"
@@ -386,6 +387,7 @@ eal_usage(const char *prgname)
 	       "  --"OPT_XEN_DOM0" : support application running on Xen Domain0 "
 			   "without hugetlbfs\n"
 	       "  --"OPT_SYSLOG"     : set syslog facility\n"
+	       "  --"OPT_LOG_LEVEL"  : set default log level\n"
 	       "  --"OPT_SOCKET_MEM" : memory to allocate on specific \n"
 		   "                 sockets (use comma separated values)\n"
 	       "  --"OPT_HUGE_DIR"   : directory where hugetlbfs is mounted\n"
@@ -550,6 +552,28 @@ eal_parse_syslog(const char *facility)
 }
 
 static int
+eal_parse_log_level(const char *level, uint32_t *log_level)
+{
+	char *end;
+	unsigned long tmp;
+
+	errno = 0;
+	tmp = strtoul(level, &end, 0);
+
+	/* check for errors */
+	if ((errno != 0) || (level[0] == '\0') ||
+	    end == NULL || (*end != '\0'))
+		return -1;
+
+	/* log_level is a uint32_t */
+	if (tmp >= UINT32_MAX)
+		return -1;
+
+	*log_level = tmp;
+	return 0;
+}
+
+static int
 eal_parse_socket_mem(char *socket_mem)
 {
 	char * arg[RTE_MAX_NUMA_NODES];
@@ -701,6 +725,7 @@ eal_parse_args(int argc, char **argv)
 		{OPT_PCI_BLACKLIST, 1, 0, 0},
 		{OPT_VDEV, 1, 0, 0},
 		{OPT_SYSLOG, 1, NULL, 0},
+		{OPT_LOG_LEVEL, 1, NULL, 0},
 		{OPT_VFIO_INTR, 1, NULL, 0},
 		{OPT_BASE_VIRTADDR, 1, 0, 0},
 		{OPT_XEN_DOM0, 0, 0, 0},
@@ -718,6 +743,8 @@ eal_parse_args(int argc, char **argv)
 	internal_config.hugepage_dir = NULL;
 	internal_config.force_sockets = 0;
 	internal_config.syslog_facility = LOG_DAEMON;
+	/* default value from build option */
+	internal_config.log_level = RTE_LOG_LEVEL;
 	internal_config.xen_dom0_support = 0;
 	/* if set to NONE, interrupt mode is determined automatically */
 	internal_config.vfio_intr_mode = RTE_INTR_MODE_NONE;
@@ -890,6 +917,19 @@ eal_parse_args(int argc, char **argv)
 					return -1;
 				}
 			}
+			else if (!strcmp(lgopts[option_index].name,
+					 OPT_LOG_LEVEL)) {
+				uint32_t log;
+
+				if (eal_parse_log_level(optarg, &log) < 0) {
+					RTE_LOG(ERR, EAL,
+						"invalid parameters for --"
+						OPT_LOG_LEVEL "\n");
+					eal_usage(prgname);
+					return -1;
+				}
+				internal_config.log_level = log;
+			}
 			else if (!strcmp(lgopts[option_index].name, OPT_BASE_VIRTADDR)) {
 				if (eal_parse_base_virtaddr(optarg) < 0) {
 					RTE_LOG(ERR, EAL, "invalid parameter for --"
@@ -1056,6 +1096,9 @@ rte_eal_init(int argc, char **argv)
 	if (fctret < 0)
 		exit(1);
 
+	/* set log level as early as possible */
+	rte_set_log_level(internal_config.log_level);
+
 	if (internal_config.no_hugetlbfs == 0 &&
 			internal_config.process_type != RTE_PROC_SECONDARY &&
 			internal_config.xen_dom0_support == 0 &&
diff --git a/lib/librte_eal/linuxapp/eal/include/eal_internal_cfg.h b/lib/librte_eal/linuxapp/eal/include/eal_internal_cfg.h
index 498ade2..8749390 100644
--- a/lib/librte_eal/linuxapp/eal/include/eal_internal_cfg.h
+++ b/lib/librte_eal/linuxapp/eal/include/eal_internal_cfg.h
@@ -77,6 +77,7 @@ struct internal_config {
 	volatile uint64_t socket_mem[RTE_MAX_NUMA_NODES]; /**< amount of memory per socket */
 	uintptr_t base_virtaddr;          /**< base address to try and reserve memory from */
 	volatile int syslog_facility;	  /**< facility passed to openlog() */
+	volatile uint32_t log_level;	  /**< default log level */
 	/** default interrupt mode for VFIO */
 	volatile enum rte_intr_mode vfio_intr_mode;
 	const char *hugefile_prefix;      /**< the base filename of hugetlbfs files */
-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 01/11] ixgbe: clean log messages
  2014-08-26 14:09 ` [dpdk-dev] [PATCH 01/11] ixgbe: clean log messages David Marchand
@ 2014-08-26 14:23   ` Jay Rolette
  2014-08-26 14:55     ` David Marchand
  0 siblings, 1 reply; 17+ messages in thread
From: Jay Rolette @ 2014-08-26 14:23 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

Why are you adding newlines to log message strings? Shouldn't that be up to
whatever the messages end up getting routed to?

Jay


On Tue, Aug 26, 2014 at 9:09 AM, David Marchand <david.marchand@6wind.com>
wrote:

> Clean log messages:
> - remove superfluous \n in log macros and add some \n where needed,
> - remove leading \n in some messages,
> - split multi lines messages,
> - replace some PMD_INIT_LOG(DEBUG, "some_func\n") with
> PMD_INIT_FUNC_TRACE().
>
> Signed-off-by: David Marchand <david.marchand@6wind.com>
> ---
>  lib/librte_pmd_ixgbe/ixgbe_ethdev.c |   99
> +++++++++++++++++------------------
>  lib/librte_pmd_ixgbe/ixgbe_fdir.c   |   12 ++---
>  lib/librte_pmd_ixgbe/ixgbe_logs.h   |   12 ++---
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.c   |   14 ++---
>  4 files changed, 68 insertions(+), 69 deletions(-)
>
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> index 59122a1..ac00323 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> @@ -737,7 +737,7 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct
> eth_driver *eth_drv,
>  #endif /* RTE_NIC_BYPASS */
>
>         if (diag != IXGBE_SUCCESS) {
> -               PMD_INIT_LOG(ERR, "Shared code init failed: %d", diag);
> +               PMD_INIT_LOG(ERR, "Shared code init failed: %d\n", diag);
>                 return -EIO;
>         }
>
> @@ -763,7 +763,7 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct
> eth_driver *eth_drv,
>         /* Make sure we have a good EEPROM before we read from it */
>         diag = ixgbe_validate_eeprom_checksum(hw, &csum);
>         if (diag != IXGBE_SUCCESS) {
> -               PMD_INIT_LOG(ERR, "The EEPROM checksum is not valid: %d",
> diag);
> +               PMD_INIT_LOG(ERR, "The EEPROM checksum is not valid:
> %d\n", diag);
>                 return -EIO;
>         }
>
> @@ -791,13 +791,14 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct
> eth_driver *eth_drv,
>         if (diag == IXGBE_ERR_EEPROM_VERSION) {
>                 PMD_INIT_LOG(ERR, "This device is a pre-production
> adapter/"
>                     "LOM.  Please be aware there may be issues associated "
> -                   "with your hardware.\n If you are experiencing
> problems "
> +                   "with your hardware.\n");
> +               PMD_INIT_LOG(ERR, "If you are experiencing problems "
>                     "please contact your Intel or hardware representative "
>                     "who provided you with this hardware.\n");
>         } else if (diag == IXGBE_ERR_SFP_NOT_SUPPORTED)
>                 PMD_INIT_LOG(ERR, "Unsupported SFP+ Module\n");
>         if (diag) {
> -               PMD_INIT_LOG(ERR, "Hardware Initialization Failure: %d",
> diag);
> +               PMD_INIT_LOG(ERR, "Hardware Initialization Failure: %d\n",
> diag);
>                 return -EIO;
>         }
>
> @@ -813,7 +814,7 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct
> eth_driver *eth_drv,
>         if (eth_dev->data->mac_addrs == NULL) {
>                 PMD_INIT_LOG(ERR,
>                         "Failed to allocate %u bytes needed to store "
> -                       "MAC addresses",
> +                       "MAC addresses\n",
>                         ETHER_ADDR_LEN * hw->mac.num_rar_entries);
>                 return -ENOMEM;
>         }
> @@ -826,7 +827,8 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct
> eth_driver *eth_drv,
>                         IXGBE_VMDQ_NUM_UC_MAC, 0);
>         if (eth_dev->data->hash_mac_addrs == NULL) {
>                 PMD_INIT_LOG(ERR,
> -                       "Failed to allocate %d bytes needed to store MAC
> addresses",
> +                       "Failed to allocate %d bytes needed to store "
> +                       "MAC addresses\n",
>                         ETHER_ADDR_LEN * IXGBE_VMDQ_NUM_UC_MAC);
>                 return -ENOMEM;
>         }
> @@ -850,14 +852,14 @@ eth_ixgbe_dev_init(__attribute__((unused)) struct
> eth_driver *eth_drv,
>
>         if (ixgbe_is_sfp(hw) && hw->phy.sfp_type !=
> ixgbe_sfp_type_not_present)
>                 PMD_INIT_LOG(DEBUG,
> -                            "MAC: %d, PHY: %d, SFP+: %d<n",
> +                            "MAC: %d, PHY: %d, SFP+: %d\n",
>                              (int) hw->mac.type, (int) hw->phy.type,
>                              (int) hw->phy.sfp_type);
>         else
>                 PMD_INIT_LOG(DEBUG, "MAC: %d, PHY: %d\n",
>                              (int) hw->mac.type, (int) hw->phy.type);
>
> -       PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x",
> +       PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x\n",
>                         eth_dev->data->port_id, pci_dev->id.vendor_id,
>                         pci_dev->id.device_id);
>
> @@ -933,7 +935,7 @@ eth_ixgbevf_dev_init(__attribute__((unused)) struct
> eth_driver *eth_drv,
>
> IXGBE_DEV_PRIVATE_TO_HWSTRIP_BITMAP(eth_dev->data->dev_private);
>         struct ether_addr *perm_addr = (struct ether_addr *)
> hw->mac.perm_addr;
>
> -       PMD_INIT_LOG(DEBUG, "eth_ixgbevf_dev_init");
> +       PMD_INIT_FUNC_TRACE();
>
>         eth_dev->dev_ops = &ixgbevf_eth_dev_ops;
>         eth_dev->rx_pkt_burst = &ixgbe_recv_pkts;
> @@ -963,7 +965,8 @@ eth_ixgbevf_dev_init(__attribute__((unused)) struct
> eth_driver *eth_drv,
>         /* Initialize the shared code (base driver) */
>         diag = ixgbe_init_shared_code(hw);
>         if (diag != IXGBE_SUCCESS) {
> -               PMD_INIT_LOG(ERR, "Shared code init failed for ixgbevf:
> %d", diag);
> +               PMD_INIT_LOG(ERR, "Shared code init failed for ixgbevf:
> %d\n",
> +                            diag);
>                 return -EIO;
>         }
>
> @@ -982,7 +985,7 @@ eth_ixgbevf_dev_init(__attribute__((unused)) struct
> eth_driver *eth_drv,
>          * In this case, assign a random MAC address.
>          */
>         if ((diag != IXGBE_SUCCESS) && (diag !=
> IXGBE_ERR_INVALID_MAC_ADDR)) {
> -               PMD_INIT_LOG(ERR, "VF Initialization Failure: %d", diag);
> +               PMD_INIT_LOG(ERR, "VF Initialization Failure: %d\n", diag);
>                 return (diag);
>         }
>
> @@ -998,7 +1001,7 @@ eth_ixgbevf_dev_init(__attribute__((unused)) struct
> eth_driver *eth_drv,
>         if (eth_dev->data->mac_addrs == NULL) {
>                 PMD_INIT_LOG(ERR,
>                         "Failed to allocate %u bytes needed to store "
> -                       "MAC addresses",
> +                       "MAC addresses\n",
>                         ETHER_ADDR_LEN * hw->mac.num_rar_entries);
>                 return -ENOMEM;
>         }
> @@ -1034,11 +1037,12 @@ eth_ixgbevf_dev_init(__attribute__((unused))
> struct eth_driver *eth_drv,
>                         break;
>
>                 default:
> -                       PMD_INIT_LOG(ERR, "VF Initialization Failure: %d",
> diag);
> +                       PMD_INIT_LOG(ERR, "VF Initialization Failure:
> %d\n",
> +                                    diag);
>                         return (-EIO);
>         }
>
> -       PMD_INIT_LOG(DEBUG, "\nport %d vendorID=0x%x deviceID=0x%x
> mac.type=%s\n",
> +       PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x
> mac.type=%s\n",
>                          eth_dev->data->port_id, pci_dev->id.vendor_id,
> pci_dev->id.device_id,
>                          "ixgbe_mac_82599_vf");
>
> @@ -1207,7 +1211,7 @@ ixgbe_vlan_hw_strip_disable(struct rte_eth_dev *dev,
> uint16_t queue)
>
>         if (hw->mac.type == ixgbe_mac_82598EB) {
>                 /* No queue level support */
> -               PMD_INIT_LOG(INFO, "82598EB not support queue level hw
> strip");
> +               PMD_INIT_LOG(INFO, "82598EB not support queue level hw
> strip\n");
>                 return;
>         }
>         else {
> @@ -1231,7 +1235,7 @@ ixgbe_vlan_hw_strip_enable(struct rte_eth_dev *dev,
> uint16_t queue)
>
>         if (hw->mac.type == ixgbe_mac_82598EB) {
>                 /* No queue level supported */
> -               PMD_INIT_LOG(INFO, "82598EB not support queue level hw
> strip");
> +               PMD_INIT_LOG(INFO, "82598EB not support queue level hw
> strip\n");
>                 return;
>         }
>         else {
> @@ -1543,7 +1547,7 @@ skip_link_setup:
>         return (0);
>
>  error:
> -       PMD_INIT_LOG(ERR, "failure in ixgbe_dev_start(): %d", err);
> +       PMD_INIT_LOG(ERR, "failure in ixgbe_dev_start(): %d\n", err);
>         ixgbe_dev_clear_queues(dev);
>         return -EIO;
>  }
> @@ -1599,10 +1603,8 @@ ixgbe_dev_set_link_up(struct rte_eth_dev *dev)
>  #ifdef RTE_NIC_BYPASS
>                 if (hw->device_id == IXGBE_DEV_ID_82599_BYPASS) {
>                         /* Not suported in bypass mode */
> -                       PMD_INIT_LOG(ERR,
> -                               "\nSet link up is not supported "
> -                               "by device id 0x%x\n",
> -                               hw->device_id);
> +                       PMD_INIT_LOG(ERR, "Set link up is not supported "
> +                                    "by device id 0x%x\n", hw->device_id);
>                         return -ENOTSUP;
>                 }
>  #endif
> @@ -1611,8 +1613,8 @@ ixgbe_dev_set_link_up(struct rte_eth_dev *dev)
>                 return 0;
>         }
>
> -       PMD_INIT_LOG(ERR, "\nSet link up is not supported by device id
> 0x%x\n",
> -               hw->device_id);
> +       PMD_INIT_LOG(ERR, "Set link up is not supported by device id
> 0x%x\n",
> +                    hw->device_id);
>         return -ENOTSUP;
>  }
>
> @@ -1628,10 +1630,8 @@ ixgbe_dev_set_link_down(struct rte_eth_dev *dev)
>  #ifdef RTE_NIC_BYPASS
>                 if (hw->device_id == IXGBE_DEV_ID_82599_BYPASS) {
>                         /* Not suported in bypass mode */
> -                       PMD_INIT_LOG(ERR,
> -                               "\nSet link down is not supported "
> -                               "by device id 0x%x\n",
> -                                hw->device_id);
> +                       PMD_INIT_LOG(ERR, "Set link down is not supported "
> +                                    "by device id 0x%x\n", hw->device_id);
>                         return -ENOTSUP;
>                 }
>  #endif
> @@ -1640,9 +1640,8 @@ ixgbe_dev_set_link_down(struct rte_eth_dev *dev)
>                 return 0;
>         }
>
> -       PMD_INIT_LOG(ERR,
> -               "\nSet link down is not supported by device id 0x%x\n",
> -                hw->device_id);
> +       PMD_INIT_LOG(ERR, "Set link down is not supported by device id
> 0x%x\n",
> +                    hw->device_id);
>         return -ENOTSUP;
>  }
>
> @@ -2113,7 +2112,7 @@ ixgbe_dev_interrupt_get_status(struct rte_eth_dev
> *dev)
>
>         /* read-on-clear nic registers here */
>         eicr = IXGBE_READ_REG(hw, IXGBE_EICR);
> -       PMD_DRV_LOG(INFO, "eicr %x", eicr);
> +       PMD_DRV_LOG(INFO, "eicr %x\n", eicr);
>
>         intr->flags = 0;
>         if (eicr & IXGBE_EICR_LSC) {
> @@ -2145,16 +2144,16 @@ ixgbe_dev_link_status_print(struct rte_eth_dev
> *dev)
>         memset(&link, 0, sizeof(link));
>         rte_ixgbe_dev_atomic_read_link_status(dev, &link);
>         if (link.link_status) {
> -               PMD_INIT_LOG(INFO, "Port %d: Link Up - speed %u Mbps - %s",
> +               PMD_INIT_LOG(INFO, "Port %d: Link Up - speed %u Mbps -
> %s\n",
>                                         (int)(dev->data->port_id),
>                                         (unsigned)link.link_speed,
>                         link.link_duplex == ETH_LINK_FULL_DUPLEX ?
>                                         "full-duplex" : "half-duplex");
>         } else {
> -               PMD_INIT_LOG(INFO, " Port %d: Link Down",
> +               PMD_INIT_LOG(INFO, " Port %d: Link Down\n",
>                                 (int)(dev->data->port_id));
>         }
> -       PMD_INIT_LOG(INFO, "PCI Address: %04d:%02d:%02d:%d",
> +       PMD_INIT_LOG(INFO, "PCI Address: %04d:%02d:%02d:%d\n",
>                                 dev->pci_dev->addr.domain,
>                                 dev->pci_dev->addr.bus,
>                                 dev->pci_dev->addr.devid,
> @@ -2211,9 +2210,9 @@ ixgbe_dev_interrupt_action(struct rte_eth_dev *dev)
>         if (intr_enable_delay) {
>                 if (rte_eal_alarm_set(timeout * 1000,
>                                       ixgbe_dev_interrupt_delayed_handler,
> (void*)dev) < 0)
> -                       PMD_DRV_LOG(ERR, "Error setting alarm");
> +                       PMD_DRV_LOG(ERR, "Error setting alarm\n");
>         } else {
> -               PMD_DRV_LOG(DEBUG, "enable intr immediately");
> +               PMD_DRV_LOG(DEBUG, "enable intr immediately\n");
>                 ixgbe_enable_intr(dev);
>                 rte_intr_enable(&(dev->pci_dev->intr_handle));
>         }
> @@ -2371,7 +2370,7 @@ ixgbe_flow_ctrl_set(struct rte_eth_dev *dev, struct
> rte_eth_fc_conf *fc_conf)
>         if (fc_conf->autoneg != !hw->fc.disable_fc_autoneg)
>                 return -ENOTSUP;
>         rx_buf_size = IXGBE_READ_REG(hw, IXGBE_RXPBSIZE(0));
> -       PMD_INIT_LOG(DEBUG, "Rx packet buffer size = 0x%x \n",
> rx_buf_size);
> +       PMD_INIT_LOG(DEBUG, "Rx packet buffer size = 0x%x\n", rx_buf_size);
>
>         /*
>          * At least reserve one Ethernet frame for watermark
> @@ -2413,7 +2412,7 @@ ixgbe_flow_ctrl_set(struct rte_eth_dev *dev, struct
> rte_eth_fc_conf *fc_conf)
>                 return 0;
>         }
>
> -       PMD_INIT_LOG(ERR, "ixgbe_fc_enable = 0x%x \n", err);
> +       PMD_INIT_LOG(ERR, "ixgbe_fc_enable = 0x%x\n", err);
>         return -EIO;
>  }
>
> @@ -2593,7 +2592,7 @@ ixgbe_priority_flow_ctrl_set(struct rte_eth_dev
> *dev, struct rte_eth_pfc_conf *p
>         ixgbe_dcb_unpack_map_cee(dcb_config, IXGBE_DCB_RX_CONFIG, map);
>         tc_num = map[pfc_conf->priority];
>         rx_buf_size = IXGBE_READ_REG(hw, IXGBE_RXPBSIZE(tc_num));
> -       PMD_INIT_LOG(DEBUG, "Rx packet buffer size = 0x%x \n",
> rx_buf_size);
> +       PMD_INIT_LOG(DEBUG, "Rx packet buffer size = 0x%x\n", rx_buf_size);
>         /*
>          * At least reserve one Ethernet frame for watermark
>          * high_water/low_water in kilo bytes for ixgbe
> @@ -2618,7 +2617,7 @@ ixgbe_priority_flow_ctrl_set(struct rte_eth_dev
> *dev, struct rte_eth_pfc_conf *p
>         if ((err == IXGBE_SUCCESS) || (err == IXGBE_ERR_FC_NOT_NEGOTIATED))
>                 return 0;
>
> -       PMD_INIT_LOG(ERR, "ixgbe_dcb_pfc_enable = 0x%x \n", err);
> +       PMD_INIT_LOG(ERR, "ixgbe_dcb_pfc_enable = 0x%x\n", err);
>         return -EIO;
>  }
>
> @@ -2765,7 +2764,7 @@ ixgbe_dev_mtu_set(struct rte_eth_dev *dev, uint16_t
> mtu)
>  static void
>  ixgbevf_intr_disable(struct ixgbe_hw *hw)
>  {
> -       PMD_INIT_LOG(DEBUG, "ixgbevf_intr_disable");
> +       PMD_INIT_FUNC_TRACE();
>
>         /* Clear interrupt mask to stop from interrupts being generated */
>         IXGBE_WRITE_REG(hw, IXGBE_VTEIMC, IXGBE_VF_IRQ_CLEAR_MASK);
> @@ -2778,8 +2777,8 @@ ixgbevf_dev_configure(struct rte_eth_dev *dev)
>  {
>         struct rte_eth_conf* conf = &dev->data->dev_conf;
>
> -       PMD_INIT_LOG(DEBUG, "\nConfigured Virtual Function port id: %d\n",
> -               dev->data->port_id);
> +       PMD_INIT_LOG(DEBUG, "Configured Virtual Function port id: %d\n",
> +                    dev->data->port_id);
>
>         /*
>          * VF has no ability to enable/disable HW CRC
> @@ -2807,7 +2806,7 @@ ixgbevf_dev_start(struct rte_eth_dev *dev)
>                 IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>         int err, mask = 0;
>
> -       PMD_INIT_LOG(DEBUG, "ixgbevf_dev_start");
> +       PMD_INIT_FUNC_TRACE();
>
>         hw->mac.ops.reset_hw(hw);
>
> @@ -2842,7 +2841,7 @@ ixgbevf_dev_stop(struct rte_eth_dev *dev)
>  {
>         struct ixgbe_hw *hw =
> IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>
> -       PMD_INIT_LOG(DEBUG, "ixgbevf_dev_stop");
> +       PMD_INIT_FUNC_TRACE();
>
>         hw->adapter_stopped = TRUE;
>         ixgbe_stop_adapter(hw);
> @@ -2861,7 +2860,7 @@ ixgbevf_dev_close(struct rte_eth_dev *dev)
>  {
>         struct ixgbe_hw *hw =
> IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>
> -       PMD_INIT_LOG(DEBUG, "ixgbevf_dev_close");
> +       PMD_INIT_FUNC_TRACE();
>
>         ixgbe_reset_hw(hw);
>
> @@ -2908,7 +2907,7 @@ ixgbevf_vlan_filter_set(struct rte_eth_dev *dev,
> uint16_t vlan_id, int on)
>         /* vind is not used in VF driver, set to 0, check
> ixgbe_set_vfta_vf */
>         ret = ixgbe_set_vfta(hw, vlan_id, 0, !!on);
>         if(ret){
> -               PMD_INIT_LOG(ERR, "Unable to set VF vlan");
> +               PMD_INIT_LOG(ERR, "Unable to set VF vlan\n");
>                 return ret;
>         }
>         vid_idx = (uint32_t) ((vlan_id >> 5) & 0x7F);
> @@ -3477,7 +3476,7 @@ ixgbevf_add_mac_addr(struct rte_eth_dev *dev, struct
> ether_addr *mac_addr,
>         diag = ixgbevf_set_uc_addr_vf(hw, 2, mac_addr->addr_bytes);
>         if (diag == 0)
>                 return;
> -       PMD_DRV_LOG(ERR, "Unable to add MAC address - diag=%d", diag);
> +       PMD_DRV_LOG(ERR, "Unable to add MAC address - diag=%d\n", diag);
>  }
>
>  static void
> @@ -3517,7 +3516,7 @@ ixgbevf_remove_mac_addr(struct rte_eth_dev *dev,
> uint32_t index)
>                         PMD_DRV_LOG(ERR,
>                                     "Adding again MAC address "
>                                     "%02x:%02x:%02x:%02x:%02x:%02x failed "
> -                                   "diag=%d",
> +                                   "diag=%d\n",
>                                     mac_addr->addr_bytes[0],
>                                     mac_addr->addr_bytes[1],
>                                     mac_addr->addr_bytes[2],
> @@ -3806,7 +3805,7 @@ ixgbe_add_5tuple_filter(struct rte_eth_dev *dev,
> uint16_t index,
>                 return -EINVAL;  /* filter index is out of range. */
>
>         if (filter->tcp_flags) {
> -               PMD_INIT_LOG(INFO, "82599EB not tcp flags in 5tuple");
> +               PMD_INIT_LOG(INFO, "82599EB not tcp flags in 5tuple\n");
>                 return -EINVAL;
>         }
>
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_fdir.c
> b/lib/librte_pmd_ixgbe/ixgbe_fdir.c
> index 6c0a530..c78a450 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_fdir.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_fdir.c
> @@ -139,7 +139,7 @@ configure_fdir_flags(struct rte_fdir_conf *conf,
> uint32_t *fdirctrl)
>                 break;
>         default:
>                 /* bad value */
> -               PMD_INIT_LOG(ERR, "Invalid fdir_conf->pballoc value");
> +               PMD_INIT_LOG(ERR, "Invalid fdir_conf->pballoc value\n");
>                 return -EINVAL;
>         };
>
> @@ -158,7 +158,7 @@ configure_fdir_flags(struct rte_fdir_conf *conf,
> uint32_t *fdirctrl)
>                 break;
>         default:
>                 /* bad value */
> -               PMD_INIT_LOG(ERR, "Invalid fdir_conf->status value");
> +               PMD_INIT_LOG(ERR, "Invalid fdir_conf->status value\n");
>                 return -EINVAL;
>         };
>
> @@ -395,7 +395,7 @@ fdir_filter_to_atr_input(struct rte_fdir_filter
> *fdir_filter,
>         if ((fdir_filter->l4type == RTE_FDIR_L4TYPE_SCTP ||
>                         fdir_filter->l4type == RTE_FDIR_L4TYPE_NONE) &&
>                         (fdir_filter->port_src || fdir_filter->port_dst)) {
> -               PMD_INIT_LOG(ERR, "Invalid fdir_filter");
> +               PMD_INIT_LOG(ERR, "Invalid fdir_filter\n");
>                 return -EINVAL;
>         }
>
> @@ -420,7 +420,7 @@ fdir_filter_to_atr_input(struct rte_fdir_filter
> *fdir_filter,
>                 input->formatted.flow_type = IXGBE_ATR_FLOW_TYPE_IPV4;
>                 break;
>         default:
> -               PMD_INIT_LOG(ERR, " Error on l4type input");
> +               PMD_INIT_LOG(ERR, " Error on l4type input\n");
>                 return -EINVAL;
>         }
>
> @@ -524,7 +524,7 @@ fdir_erase_filter_82599(struct ixgbe_hw *hw,
>         }
>
>         if (!retry_count) {
> -               PMD_INIT_LOG(ERR, "Timeout querying for flow director
> filter");
> +               PMD_INIT_LOG(ERR, "Timeout querying for flow director
> filter\n");
>                 err = -EIO;
>         }
>
> @@ -678,7 +678,7 @@ ixgbe_fdir_set_masks(struct rte_eth_dev *dev, struct
> rte_fdir_masks *fdir_masks)
>
>         err = ixgbe_reinit_fdir_tables_82599(hw);
>         if (err) {
> -               PMD_INIT_LOG(ERR, "reinit of fdir tables failed");
> +               PMD_INIT_LOG(ERR, "reinit of fdir tables failed\n");
>                 return -EIO;
>         }
>
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_logs.h
> b/lib/librte_pmd_ixgbe/ixgbe_logs.h
> index 9f0a684..22d8cfb 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_logs.h
> +++ b/lib/librte_pmd_ixgbe/ixgbe_logs.h
> @@ -36,8 +36,8 @@
>
>  #ifdef RTE_LIBRTE_IXGBE_DEBUG_INIT
>  #define PMD_INIT_LOG(level, fmt, args...) \
> -       RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
> -#define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>")
> +       RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
> +#define PMD_INIT_FUNC_TRACE() PMD_INIT_LOG(DEBUG, " >>\n")
>  #else
>  #define PMD_INIT_LOG(level, fmt, args...) do { } while(0)
>  #define PMD_INIT_FUNC_TRACE() do { } while(0)
> @@ -45,28 +45,28 @@
>
>  #ifdef RTE_LIBRTE_IXGBE_DEBUG_RX
>  #define PMD_RX_LOG(level, fmt, args...) \
> -       RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
> +       RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
>  #else
>  #define PMD_RX_LOG(level, fmt, args...) do { } while(0)
>  #endif
>
>  #ifdef RTE_LIBRTE_IXGBE_DEBUG_TX
>  #define PMD_TX_LOG(level, fmt, args...) \
> -       RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
> +       RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
>  #else
>  #define PMD_TX_LOG(level, fmt, args...) do { } while(0)
>  #endif
>
>  #ifdef RTE_LIBRTE_IXGBE_DEBUG_TX_FREE
>  #define PMD_TX_FREE_LOG(level, fmt, args...) \
> -       RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
> +       RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
>  #else
>  #define PMD_TX_FREE_LOG(level, fmt, args...) do { } while(0)
>  #endif
>
>  #ifdef RTE_LIBRTE_IXGBE_DEBUG_DRIVER
>  #define PMD_DRV_LOG(level, fmt, args...) \
> -       RTE_LOG(level, PMD, "%s(): " fmt "\n", __func__, ## args)
> +       RTE_LOG(level, PMD, "%s(): " fmt, __func__, ## args)
>  #else
>  #define PMD_DRV_LOG(level, fmt, args...) do { } while(0)
>  #endif
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index dfc2076..cbec821 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -492,7 +492,7 @@ ixgbe_xmit_cleanup(struct igb_tx_queue *txq)
>         {
>                 PMD_TX_FREE_LOG(DEBUG,
>                                 "TX descriptor %4u is not done"
> -                               "(port=%d queue=%d)",
> +                               "(port=%d queue=%d)\n",
>                                 desc_to_clean_to,
>                                 txq->port_id, txq->queue_id);
>                 /* Failed to clean any descriptors, better luck next time
> */
> @@ -509,7 +509,7 @@ ixgbe_xmit_cleanup(struct igb_tx_queue *txq)
>
>         PMD_TX_FREE_LOG(DEBUG,
>                         "Cleaning %4u TX descriptors: %4u to %4u "
> -                       "(port=%d queue=%d)",
> +                       "(port=%d queue=%d)\n",
>                         nb_tx_to_clean, last_desc_cleaned,
> desc_to_clean_to,
>                         txq->port_id, txq->queue_id);
>
> @@ -630,7 +630,7 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf
> **tx_pkts,
>                         PMD_TX_FREE_LOG(DEBUG,
>                                         "Not enough free TX descriptors "
>                                         "nb_used=%4u nb_free=%4u "
> -                                       "(port=%d queue=%d)",
> +                                       "(port=%d queue=%d)\n",
>                                         nb_used, txq->nb_tx_free,
>                                         txq->port_id, txq->queue_id);
>
> @@ -650,7 +650,7 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf
> **tx_pkts,
>                                         "performance."
>                                         "nb_used=%4u nb_free=%4u "
>                                         "tx_rs_thresh=%4u. "
> -                                       "(port=%d queue=%d)",
> +                                       "(port=%d queue=%d)\n",
>                                         nb_used, txq->nb_tx_free,
>                                         txq->tx_rs_thresh,
>                                         txq->port_id, txq->queue_id);
> @@ -782,7 +782,7 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf
> **tx_pkts,
>                 if (txq->nb_tx_used >= txq->tx_rs_thresh) {
>                         PMD_TX_FREE_LOG(DEBUG,
>                                         "Setting RS bit on TXD id="
> -                                       "%4u (port=%d queue=%d)",
> +                                       "%4u (port=%d queue=%d)\n",
>                                         tx_last, txq->port_id,
> txq->queue_id);
>
>                         cmd_type_len |= IXGBE_TXD_CMD_RS;
> @@ -798,7 +798,7 @@ end_of_tx:
>         /*
>          * Set the Transmit Descriptor Tail (TDT)
>          */
> -       PMD_TX_LOG(DEBUG, "port_id=%u queue_id=%u tx_tail=%u nb_tx=%u",
> +       PMD_TX_LOG(DEBUG, "port_id=%u queue_id=%u tx_tail=%u nb_tx=%u\n",
>                    (unsigned) txq->port_id, (unsigned) txq->queue_id,
>                    (unsigned) tx_id, (unsigned) nb_tx);
>         IXGBE_PCI_REG_WRITE(txq->tdt_reg_addr, tx_id);
> @@ -1383,7 +1383,7 @@ ixgbe_recv_scattered_pkts(void *rx_queue, struct
> rte_mbuf **rx_pkts,
>                  * to happen by sending specific "back-pressure" flow
> control
>                  * frames to its peer(s).
>                  */
> -               PMD_RX_LOG(DEBUG, "\nport_id=%u queue_id=%u rx_id=%u "
> +               PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_id=%u "
>                            "staterr=0x%x data_len=%u\n",
>                            (unsigned) rxq->port_id, (unsigned)
> rxq->queue_id,
>                            (unsigned) rx_id, (unsigned) staterr,
> --
> 1.7.10.4
>
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 01/11] ixgbe: clean log messages
  2014-08-26 14:23   ` Jay Rolette
@ 2014-08-26 14:55     ` David Marchand
  2014-08-27 13:53       ` Jay Rolette
  0 siblings, 1 reply; 17+ messages in thread
From: David Marchand @ 2014-08-26 14:55 UTC (permalink / raw)
  To: Jay Rolette; +Cc: dev

Hello Jay,

On Tue, Aug 26, 2014 at 4:23 PM, Jay Rolette <rolette@infiniteio.com> wrote:

> Why are you adding newlines to log message strings? Shouldn't that be up
> to whatever the messages end up getting routed to?
>

Actually, I wanted to have consistent log formats in the PMDs so that the
log messages displayed at runtime are aligned (and not bouncing with \n
before or after the log message).
There was two solutions from my point of view :
- either always add a \n in the log macro and remove all trailing \n
- do the opposite

I preferred the latter as it let users of the macro set the message format
as they want.


Before this change :

Configuring Port 0 (socket 1)
PMD: ixgbe_dev_tx_queue_setup(): sw_ring=0x7f3e59cd8080
hw_ring=0x7f3e59d02080 dma_addr=0x727702080

PMD: ixgbe_dev_tx_queue_setup(): Using simple tx code path

PMD: ixgbe_dev_tx_queue_setup(): Vector tx enabled.

PMD: ixgbe_dev_rx_queue_setup(): sw_ring=0x7f3e59cd57c0
hw_ring=0x7f3e59d12080 dma_addr=0x727712080

PMD: ixgbe_dev_rx_queue_setup(): Rx Burst Bulk Alloc Preconditions are
satisfied. Rx Burst Bulk Alloc function will be used on port=0, queue=0.

PMD: ixgbe_dev_rx_queue_setup(): Vector rx enabled, please make sure RX
burst size no less than 32.


After this change :

Configuring Port 0 (socket 1)
PMD: ixgbe_dev_tx_queue_setup(): sw_ring=0x7fd47ecd8080
hw_ring=0x7fd47ed02080 dma_addr=0x727702080
PMD: ixgbe_dev_tx_queue_setup(): Using simple tx code path
PMD: ixgbe_dev_tx_queue_setup(): Vector tx enabled.
PMD: ixgbe_dev_rx_queue_setup(): sw_ring=0x7fd47ecd57c0
hw_ring=0x7fd47ed12080 dma_addr=0x727712080
PMD: ixgbe_dev_rx_queue_setup(): Rx Burst Bulk Alloc Preconditions are
satisfied. Rx Burst Bulk Alloc function will be used on port=0, queue=0.
PMD: ixgbe_dev_rx_queue_setup(): Vector rx enabled, please make sure RX
burst size no less than 32.
Port 0: 90:E2:BA:29:DF:58


-- 
David Marchand

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 01/11] ixgbe: clean log messages
  2014-08-26 14:55     ` David Marchand
@ 2014-08-27 13:53       ` Jay Rolette
  2014-08-27 18:06         ` Stephen Hemminger
  0 siblings, 1 reply; 17+ messages in thread
From: Jay Rolette @ 2014-08-27 13:53 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

Hi David,

The updated output is definitely an improvement, but if you'll go with the
first solution you described (adding \n in the log macro), it works much
better for products using DPDK. For developer use, I think it ends up being
a wash either way.

At least for my product (embedded network appliance), we want to capture
anything that is logging in syslog. The log macros in DPDK make that very
reasonable to do, but if there are embedded newlines, it ends up screwing
up log parsing when someone wants to process logs later.

We end up having to remove all of those newlines, which makes it harder to
merge as new releases coming out.

I'm assuming most products have similar requirements for logging. That's at
least been the case for the products I've been involved with over the years.

If the PMDs are using PMD_LOG as a replacement macro for debugging
printf's, I can see where this might be a little more pain, but having
PMD_LOG is a lot more useful it if easily integrates with central logging
facilities.

Thanks,
Jay


On Tue, Aug 26, 2014 at 9:55 AM, David Marchand <david.marchand@6wind.com>
wrote:

> Hello Jay,
>
> On Tue, Aug 26, 2014 at 4:23 PM, Jay Rolette <rolette@infiniteio.com>
> wrote:
>
>> Why are you adding newlines to log message strings? Shouldn't that be up
>> to whatever the messages end up getting routed to?
>>
>
> Actually, I wanted to have consistent log formats in the PMDs so that the
> log messages displayed at runtime are aligned (and not bouncing with \n
> before or after the log message).
> There was two solutions from my point of view :
> - either always add a \n in the log macro and remove all trailing \n
> - do the opposite
>
> I preferred the latter as it let users of the macro set the message format
> as they want.
>
>
> Before this change :
>
> Configuring Port 0 (socket 1)
> PMD: ixgbe_dev_tx_queue_setup(): sw_ring=0x7f3e59cd8080
> hw_ring=0x7f3e59d02080 dma_addr=0x727702080
>
> PMD: ixgbe_dev_tx_queue_setup(): Using simple tx code path
>
> PMD: ixgbe_dev_tx_queue_setup(): Vector tx enabled.
>
> PMD: ixgbe_dev_rx_queue_setup(): sw_ring=0x7f3e59cd57c0
> hw_ring=0x7f3e59d12080 dma_addr=0x727712080
>
> PMD: ixgbe_dev_rx_queue_setup(): Rx Burst Bulk Alloc Preconditions are
> satisfied. Rx Burst Bulk Alloc function will be used on port=0, queue=0.
>
> PMD: ixgbe_dev_rx_queue_setup(): Vector rx enabled, please make sure RX
> burst size no less than 32.
>
>
> After this change :
>
> Configuring Port 0 (socket 1)
> PMD: ixgbe_dev_tx_queue_setup(): sw_ring=0x7fd47ecd8080
> hw_ring=0x7fd47ed02080 dma_addr=0x727702080
> PMD: ixgbe_dev_tx_queue_setup(): Using simple tx code path
> PMD: ixgbe_dev_tx_queue_setup(): Vector tx enabled.
> PMD: ixgbe_dev_rx_queue_setup(): sw_ring=0x7fd47ecd57c0
> hw_ring=0x7fd47ed12080 dma_addr=0x727712080
> PMD: ixgbe_dev_rx_queue_setup(): Rx Burst Bulk Alloc Preconditions are
> satisfied. Rx Burst Bulk Alloc function will be used on port=0, queue=0.
> PMD: ixgbe_dev_rx_queue_setup(): Vector rx enabled, please make sure RX
> burst size no less than 32.
> Port 0: 90:E2:BA:29:DF:58
>
>
> --
> David Marchand
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 01/11] ixgbe: clean log messages
  2014-08-27 13:53       ` Jay Rolette
@ 2014-08-27 18:06         ` Stephen Hemminger
  2014-08-29  7:45           ` David Marchand
  0 siblings, 1 reply; 17+ messages in thread
From: Stephen Hemminger @ 2014-08-27 18:06 UTC (permalink / raw)
  To: Jay Rolette; +Cc: dev

On Wed, 27 Aug 2014 08:53:46 -0500
Jay Rolette <rolette@infiniteio.com> wrote:

> The updated output is definitely an improvement, but if you'll go with the
> first solution you described (adding \n in the log macro), it works much
> better for products using DPDK. For developer use, I think it ends up being
> a wash either way.

Also for driver consistency, all drivers must do the same thing. I.e the PMD_INIT_LOG
macro should either add or not add newline in the same way in all drivers.
I fixed virtio and vmxnet3 by just taking extra newlines out of messages
and leaving the macro alone.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH 01/11] ixgbe: clean log messages
  2014-08-27 18:06         ` Stephen Hemminger
@ 2014-08-29  7:45           ` David Marchand
  0 siblings, 0 replies; 17+ messages in thread
From: David Marchand @ 2014-08-29  7:45 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Wed, Aug 27, 2014 at 8:06 PM, Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Wed, 27 Aug 2014 08:53:46 -0500
> Jay Rolette <rolette@infiniteio.com> wrote:
>
> > The updated output is definitely an improvement, but if you'll go with
> the
> > first solution you described (adding \n in the log macro), it works much
> > better for products using DPDK. For developer use, I think it ends up
> being
> > a wash either way.
>
> Also for driver consistency, all drivers must do the same thing. I.e the
> PMD_INIT_LOG
> macro should either add or not add newline in the same way in all drivers.
> I fixed virtio and vmxnet3 by just taking extra newlines out of messages
> and leaving the macro alone.
>

Not sure that we can talk about consistency at the moment ... (all the more
so as if you only fixed two pmds).
Anyway.


If we go with this approach, there is still something that won't work.
ixgbe shared code uses DEBUGOUT* macros which are called a lot in this
shared code.
The macros call sites use trailing \n.
I suppose I will have a big NO from Intel guys if I remove the trailing \n,
so I suppose we can use an alternate macro that calls RTE_LOG rather than
PMD_DRV_LOG in ixgbe_os_dep.h.

I will also remove any reference to DEBUGOUT* in non-shared code.
>From my point of view, these macro are only here for compat with shared
code.


By the way, did you look at the other patches of this series ?

I want to send a v2 later (maybe not today), so any comment on the other
patches are welcome.


-- 
David Marchand

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2014-08-29  7:41 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-26 14:09 [dpdk-dev] [PATCH 00/11] cleanup logs in main PMDs David Marchand
2014-08-26 14:09 ` [dpdk-dev] [PATCH 01/11] ixgbe: clean log messages David Marchand
2014-08-26 14:23   ` Jay Rolette
2014-08-26 14:55     ` David Marchand
2014-08-27 13:53       ` Jay Rolette
2014-08-27 18:06         ` Stephen Hemminger
2014-08-29  7:45           ` David Marchand
2014-08-26 14:09 ` [dpdk-dev] [PATCH 02/11] ixgbe: always log init messages David Marchand
2014-08-26 14:09 ` [dpdk-dev] [PATCH 03/11] ixgbe: add a message when forcing scatter mode David Marchand
2014-08-26 14:09 ` [dpdk-dev] [PATCH 04/11] ixgbe: add log messages when rx bulk mode is not usable David Marchand
2014-08-26 14:09 ` [dpdk-dev] [PATCH 05/11] i40e: clean log messages David Marchand
2014-08-26 14:09 ` [dpdk-dev] [PATCH 06/11] i40e: always log init messages David Marchand
2014-08-26 14:09 ` [dpdk-dev] [PATCH 07/11] i40e: add log messages when rx bulk mode is not usable David Marchand
2014-08-26 14:09 ` [dpdk-dev] [PATCH 08/11] e1000: clean log messages David Marchand
2014-08-26 14:09 ` [dpdk-dev] [PATCH 09/11] e1000: always log init messages David Marchand
2014-08-26 14:09 ` [dpdk-dev] [PATCH 10/11] e1000: add a message when forcing scatter mode David Marchand
2014-08-26 14:09 ` [dpdk-dev] [PATCH 11/11] eal: set log level from command line David Marchand

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).