Patch Comment List
List patch comments
GET /api/patches/274/comments/?format=api
[ { "id": 621, "web_url": "http://patches.dpdk.org/comment/621/", "msgid": "<CADNuJVpLLfiC_ROBkYQguHUGTgXNC1Bdu7HwveXcA23_Q3twxA@mail.gmail.com>", "list_archive_url": "https://inbox.dpdk.org/dev/CADNuJVpLLfiC_ROBkYQguHUGTgXNC1Bdu7HwveXcA23_Q3twxA@mail.gmail.com", "date": "2014-09-02T13:43:55", "subject": "Re: [dpdk-dev] [PATCH v2 01/17] ixgbe: use the right debug macro", "submitter": { "id": 61, "url": "http://patches.dpdk.org/api/people/61/?format=api", "name": "Jay Rolette", "email": "rolette@infiniteio.com" }, "content": "Hi David,\n\nA couple of minor comments inline below. Looks good otherwise.\n\nJay\n\n\nOn Mon, Sep 1, 2014 at 5:24 AM, David Marchand <david.marchand@6wind.com>\nwrote:\n\n> - We should not use DEBUGOUT*/DEBUGFUNC macros in non-shared code.\n> These macros come as compat wrappers for shared code.\n> - We should avoid calling RTE_LOG directly as pmd provides a wrapper for\n> logs.\n>\n> Signed-off-by: David Marchand <david.marchand@6wind.com>\n> ---\n> lib/librte_pmd_ixgbe/ixgbe_82599_bypass.c | 14 ++++----\n> lib/librte_pmd_ixgbe/ixgbe_bypass.c | 26 +++++++-------\n> lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 27 +++++++--------\n> lib/librte_pmd_ixgbe/ixgbe_pf.c | 4 +--\n> lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 53\n> +++++++++++++++--------------\n> 5 files changed, 63 insertions(+), 61 deletions(-)\n>\n> diff --git a/lib/librte_pmd_ixgbe/ixgbe_82599_bypass.c\n> b/lib/librte_pmd_ixgbe/ixgbe_82599_bypass.c\n> index 0f0000c..2623419 100644\n> --- a/lib/librte_pmd_ixgbe/ixgbe_82599_bypass.c\n> +++ b/lib/librte_pmd_ixgbe/ixgbe_82599_bypass.c\n> @@ -63,7 +63,7 @@ ixgbe_set_fiber_fixed_speed(struct ixgbe_hw *hw,\n> ixgbe_link_speed speed)\n> rs = IXGBE_SFF_SOFT_RS_SELECT_1G;\n> break;\n> default:\n> - DEBUGOUT(\"Invalid fixed module speed\\n\");\n> + PMD_DRV_LOG(\"Invalid fixed module speed\");\n> return;\n> }\n>\n> @@ -72,7 +72,7 @@ ixgbe_set_fiber_fixed_speed(struct ixgbe_hw *hw,\n> ixgbe_link_speed speed)\n> IXGBE_I2C_EEPROM_DEV_ADDR2,\n> &eeprom_data);\n> if (status) {\n> - DEBUGOUT(\"Failed to read Rx Rate Select RS0\\n\");\n> + PMD_DRV_LOG(\"Failed to read Rx Rate Select RS0\");\n> goto out;\n> }\n>\n> @@ -82,7 +82,7 @@ ixgbe_set_fiber_fixed_speed(struct ixgbe_hw *hw,\n> ixgbe_link_speed speed)\n> IXGBE_I2C_EEPROM_DEV_ADDR2,\n> eeprom_data);\n> if (status) {\n> - DEBUGOUT(\"Failed to write Rx Rate Select RS0\\n\");\n> + PMD_DRV_LOG(\"Failed to write Rx Rate Select RS0\");\n> goto out;\n> }\n>\n> @@ -91,7 +91,7 @@ ixgbe_set_fiber_fixed_speed(struct ixgbe_hw *hw,\n> ixgbe_link_speed speed)\n> IXGBE_I2C_EEPROM_DEV_ADDR2,\n> &eeprom_data);\n> if (status) {\n> - DEBUGOUT(\"Failed to read Rx Rate Select RS1\\n\");\n> + PMD_DRV_LOG(\"Failed to read Rx Rate Select RS1\");\n> goto out;\n> }\n>\n> @@ -101,7 +101,7 @@ ixgbe_set_fiber_fixed_speed(struct ixgbe_hw *hw,\n> ixgbe_link_speed speed)\n> IXGBE_I2C_EEPROM_DEV_ADDR2,\n> eeprom_data);\n> if (status) {\n> - DEBUGOUT(\"Failed to write Rx Rate Select RS1\\n\");\n> + PMD_DRV_LOG(\"Failed to write Rx Rate Select RS1\");\n> goto out;\n> }\n> out:\n> @@ -130,7 +130,7 @@ ixgbe_setup_mac_link_multispeed_fixed_fiber(struct\n> ixgbe_hw *hw,\n> bool link_up = false;\n> bool negotiation;\n>\n> - DEBUGFUNC(\"\");\n> + PMD_INIT_FUNC_TRACE();\n>\n> /* Mask off requested but non-supported speeds */\n> status = ixgbe_get_link_capabilities(hw, &link_speed,\n> &negotiation);\n> @@ -261,7 +261,7 @@ ixgbe_bypass_get_media_type(struct ixgbe_hw *hw)\n> {\n> enum ixgbe_media_type media_type;\n>\n> - DEBUGFUNC(\"\");\n> + PMD_INIT_FUNC_TRACE();\n>\n> if (hw->device_id == IXGBE_DEV_ID_82599_BYPASS) {\n> media_type = ixgbe_media_type_fiber;\n> diff --git a/lib/librte_pmd_ixgbe/ixgbe_bypass.c\n> b/lib/librte_pmd_ixgbe/ixgbe_bypass.c\n> index 1d21dc0..1a980b8 100644\n> --- a/lib/librte_pmd_ixgbe/ixgbe_bypass.c\n> +++ b/lib/librte_pmd_ixgbe/ixgbe_bypass.c\n> @@ -40,20 +40,20 @@\n> #define BYPASS_STATUS_OFF_MASK 3\n>\n> /* Macros to check for invlaid function pointers. */\n> -#define FUNC_PTR_OR_ERR_RET(func, retval) do { \\\n> - if ((func) == NULL) { \\\n> - DEBUGOUT(\"%s:%d function not supported\\n\", \\\n> - __func__, __LINE__); \\\n> - return (retval); \\\n> - } \\\n> +#define FUNC_PTR_OR_ERR_RET(func, retval) do { \\\n> + if ((func) == NULL) { \\\n> + PMD_DRV_LOG(\"%s:%d function not supported\", \\\n> + __func__, __LINE__); \\\n> + return retval; \\\n>\nNeed to keep the parens around retval in your macro\n\n\n> + } \\\n> } while(0)\n>\n> -#define FUNC_PTR_OR_RET(func) do { \\\n> - if ((func) == NULL) { \\\n> - DEBUGOUT(\"%s:%d function not supported\\n\", \\\n> - __func__, __LINE__); \\\n> - return; \\\n> - } \\\n> +#define FUNC_PTR_OR_RET(func) do { \\\n> + if ((func) == NULL) { \\\n> + PMD_DRV_LOG(\"%s:%d function not supported\", \\\n> + __func__, __LINE__); \\\n> + return; \\\n> + } \\\n> } while(0)\n>\n>\n> @@ -114,7 +114,7 @@ ixgbe_bypass_init(struct rte_eth_dev *dev)\n> /* Only allow BYPASS ops on the first port */\n> if (hw->device_id != IXGBE_DEV_ID_82599_BYPASS ||\n> hw->bus.func != 0) {\n> - DEBUGOUT(\"bypass function is not supported on that\n> device\\n\");\n> + PMD_DRV_LOG(\"bypass function is not supported on that\n> device\");\n> return;\n> }\n>\n> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c\n> b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c\n> index 59122a1..a8a7ed6 100644\n> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c\n> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c\n> @@ -667,7 +667,7 @@ ixgbe_swfw_lock_reset(struct ixgbe_hw *hw)\n> */\n> mask = IXGBE_GSSR_PHY0_SM << hw->bus.func;\n> if (ixgbe_acquire_swfw_semaphore(hw, mask) < 0) {\n> - DEBUGOUT1(\"SWFW phy%d lock released\", hw->bus.func);\n> + PMD_DRV_LOG(DEBUG, \"SWFW phy%d lock released\",\n> hw->bus.func);\n> }\n> ixgbe_release_swfw_semaphore(hw, mask);\n>\n> @@ -679,7 +679,7 @@ ixgbe_swfw_lock_reset(struct ixgbe_hw *hw)\n> */\n> mask = IXGBE_GSSR_EEP_SM | IXGBE_GSSR_MAC_CSR_SM |\n> IXGBE_GSSR_SW_MNG_SM;\n> if (ixgbe_acquire_swfw_semaphore(hw, mask) < 0) {\n> - DEBUGOUT(\"SWFW common locks released\");\n> + PMD_DRV_LOG(DEBUG, \"SWFW common locks released\");\n> }\n> ixgbe_release_swfw_semaphore(hw, mask);\n> }\n> @@ -1012,16 +1012,15 @@ eth_ixgbevf_dev_init(__attribute__((unused))\n> struct eth_driver *eth_drv,\n> eth_dev->data->mac_addrs = NULL;\n> return diag;\n> }\n> - RTE_LOG(INFO, PMD,\n> - \"\\tVF MAC address not assigned by Host PF\\n\"\n> - \"\\tAssign randomly generated MAC address \"\n> - \"%02x:%02x:%02x:%02x:%02x:%02x\\n\",\n> - perm_addr->addr_bytes[0],\n> - perm_addr->addr_bytes[1],\n> - perm_addr->addr_bytes[2],\n> - perm_addr->addr_bytes[3],\n> - perm_addr->addr_bytes[4],\n> - perm_addr->addr_bytes[5]);\n> + PMD_INIT_LOG(INFO, \"\\tVF MAC address not assigned by Host\n> PF\");\n> + PMD_INIT_LOG(INFO, \"\\tAssign randomly generated MAC\n> address \"\n> + \"%02x:%02x:%02x:%02x:%02x:%02x\",\n> + perm_addr->addr_bytes[0],\n> + perm_addr->addr_bytes[1],\n> + perm_addr->addr_bytes[2],\n> + perm_addr->addr_bytes[3],\n> + perm_addr->addr_bytes[4],\n> + perm_addr->addr_bytes[5]);\n> }\n>\n> /* Copy the permanent MAC address */\n> @@ -1090,7 +1089,7 @@ rte_ixgbe_pmd_init(const char *name __rte_unused,\n> const char *params __rte_unuse\n> static int\n> rte_ixgbevf_pmd_init(const char *name __rte_unused, const char *param\n> __rte_unused)\n> {\n> - DEBUGFUNC(\"rte_ixgbevf_pmd_init\");\n> + PMD_INIT_FUNC_TRACE();\n>\n> rte_eth_driver_register(&rte_ixgbevf_pmd);\n> return (0);\n> @@ -2515,7 +2514,7 @@ ixgbe_dcb_pfc_enable_generic(struct ixgbe_hw\n> *hw,uint8_t tc_num)\n> fccfg_reg |= IXGBE_FCCFG_TFCE_PRIORITY;\n> break;\n> default:\n> - DEBUGOUT(\"Flow control param set incorrectly\\n\");\n> + PMD_DRV_LOG(DEBUG, \"Flow control param set incorrectly\");\n> ret_val = IXGBE_ERR_CONFIG;\n> goto out;\n> break;\n> diff --git a/lib/librte_pmd_ixgbe/ixgbe_pf.c\n> b/lib/librte_pmd_ixgbe/ixgbe_pf.c\n> index 170944d..59fb58b 100644\n> --- a/lib/librte_pmd_ixgbe/ixgbe_pf.c\n> +++ b/lib/librte_pmd_ixgbe/ixgbe_pf.c\n> @@ -478,7 +478,7 @@ ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev,\n> uint16_t vf)\n>\n> retval = ixgbe_read_mbx(hw, msgbuf, mbx_size, vf);\n> if (retval) {\n> - RTE_LOG(ERR, PMD, \"Error mbx recv msg from VF %d\\n\", vf);\n> + PMD_DRV_LOG(ERR, \"Error mbx recv msg from VF %d\", vf);\n> return retval;\n> }\n>\n> @@ -511,7 +511,7 @@ ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev,\n> uint16_t vf)\n> retval = ixgbe_vf_set_vlan(dev, vf, msgbuf);\n> break;\n> default:\n> - RTE_LOG(DEBUG, PMD, \"Unhandled Msg %8.8x\\n\", (unsigned)\n> msgbuf[0]);\n> + PMD_DRV_LOG(DEBUG, \"Unhandled Msg %8.8x\",\n> (unsigned)msgbuf[0]);\n> retval = IXGBE_ERR_MBX;\n> break;\n> }\n> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c\n> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c\n> index dfc2076..46962bc 100644\n> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c\n> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c\n> @@ -1765,33 +1765,36 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev *dev,\n> tx_free_thresh = (uint16_t)((tx_conf->tx_free_thresh) ?\n> tx_conf->tx_free_thresh : DEFAULT_TX_FREE_THRESH);\n> if (tx_rs_thresh >= (nb_desc - 2)) {\n> - RTE_LOG(ERR, PMD, \"tx_rs_thresh must be less than the\n> number \"\n> - \"of TX descriptors minus 2. (tx_rs_thresh=%u\n> port=%d \"\n> - \"queue=%d)\\n\", (unsigned int)tx_rs_thresh,\n> - (int)dev->data->port_id, (int)queue_idx);\n> + PMD_INIT_LOG(ERR, \"tx_rs_thresh must be less than the\n> number \"\n> + \"of TX descriptors minus 2. (tx_rs_thresh=%u \"\n> + \"port=%d queue=%d)\\n\", (unsigned\n> int)tx_rs_thresh,\n>\nEmbedded newline on this log message. I noticed you have \\n still on all of\nthe PMD_INIT_LOG() calls following. Intended?\n\n\n> + (int)dev->data->port_id, (int)queue_idx);\n> return -(EINVAL);\n> }\n> if (tx_free_thresh >= (nb_desc - 3)) {\n> - RTE_LOG(ERR, PMD, \"tx_rs_thresh must be less than the \"\n> - \"tx_free_thresh must be less than the number of TX\n> \"\n> - \"descriptors minus 3. (tx_free_thresh=%u port=%d \"\n> - \"queue=%d)\\n\", (unsigned\n> int)tx_free_thresh,\n> - (int)dev->data->port_id, (int)queue_idx);\n> + PMD_INIT_LOG(ERR, \"tx_rs_thresh must be less than the \"\n> + \"tx_free_thresh must be less than the number\n> of \"\n> + \"TX descriptors minus 3. (tx_free_thresh=%u \"\n> + \"port=%d queue=%d)\\n\",\n>\nEmbedded newline\n\n\n> + (unsigned int)tx_free_thresh,\n> + (int)dev->data->port_id, (int)queue_idx);\n> return -(EINVAL);\n> }\n> if (tx_rs_thresh > tx_free_thresh) {\n> - RTE_LOG(ERR, PMD, \"tx_rs_thresh must be less than or equal\n> to \"\n> - \"tx_free_thresh. (tx_free_thresh=%u\n> tx_rs_thresh=%u \"\n> - \"port=%d queue=%d)\\n\", (unsigned\n> int)tx_free_thresh,\n> - (unsigned int)tx_rs_thresh,\n> (int)dev->data->port_id,\n> - (int)queue_idx);\n> + PMD_INIT_LOG(ERR, \"tx_rs_thresh must be less than or equal\n> to \"\n> + \"tx_free_thresh. (tx_free_thresh=%u \"\n> + \"tx_rs_thresh=%u port=%d queue=%d)\\n\",\n>\nEmbedded newline\n\n\n> + (unsigned int)tx_free_thresh,\n> + (unsigned int)tx_rs_thresh,\n> + (int)dev->data->port_id,\n> + (int)queue_idx);\n> return -(EINVAL);\n> }\n> if ((nb_desc % tx_rs_thresh) != 0) {\n> - RTE_LOG(ERR, PMD, \"tx_rs_thresh must be a divisor of the \"\n> - \"number of TX descriptors. (tx_rs_thresh=%u\n> port=%d \"\n> - \"queue=%d)\\n\", (unsigned int)tx_rs_thresh,\n> - (int)dev->data->port_id, (int)queue_idx);\n> + PMD_INIT_LOG(ERR, \"tx_rs_thresh must be a divisor of the \"\n> + \"number of TX descriptors. (tx_rs_thresh=%u \"\n> + \"port=%d queue=%d)\\n\", (unsigned\n> int)tx_rs_thresh,\n>\nEmbedded newline\n\n\n> + (int)dev->data->port_id, (int)queue_idx);\n> return -(EINVAL);\n> }\n>\n> @@ -1802,10 +1805,10 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev *dev,\n> * accumulates WTHRESH descriptors.\n> */\n> if ((tx_rs_thresh > 1) && (tx_conf->tx_thresh.wthresh != 0)) {\n> - RTE_LOG(ERR, PMD, \"TX WTHRESH must be set to 0 if \"\n> - \"tx_rs_thresh is greater than 1. (tx_rs_thresh=%u \"\n> - \"port=%d queue=%d)\\n\", (unsigned int)tx_rs_thresh,\n> - (int)dev->data->port_id, (int)queue_idx);\n> + PMD_INIT_LOG(ERR, \"TX WTHRESH must be set to 0 if \"\n> + \"tx_rs_thresh is greater than 1.\n> (tx_rs_thresh=%u \"\n> + \"port=%d queue=%d)\\n\", (unsigned\n> int)tx_rs_thresh,\n>\nEmbedded newline\n\n\n> + (int)dev->data->port_id, (int)queue_idx);\n> return -(EINVAL);\n> }\n>\n> @@ -3279,7 +3282,7 @@ ixgbe_dev_mq_rx_configure(struct rte_eth_dev *dev)\n> IXGBE_WRITE_REG(hw, IXGBE_MRQC,\n> IXGBE_MRQC_VMDQRT8TCEN);\n> break;\n> default:\n> - RTE_LOG(ERR, PMD, \"invalid pool number in IOV\n> mode\\n\");\n> + PMD_INIT_LOG(ERR, \"invalid pool number in IOV\n> mode\\n\");\n>\nEmbedded newline\n\n\n> }\n> }\n>\n> @@ -3332,7 +3335,7 @@ ixgbe_dev_mq_tx_configure(struct rte_eth_dev *dev)\n> break;\n> default:\n> mtqc = IXGBE_MTQC_64Q_1PB;\n> - RTE_LOG(ERR, PMD, \"invalid pool number in IOV\n> mode\\n\");\n> + PMD_INIT_LOG(ERR, \"invalid pool number in IOV\n> mode\\n\");\n>\nEmbedded newline\n\n\n> }\n> IXGBE_WRITE_REG(hw, IXGBE_MTQC, mtqc);\n> }\n> @@ -3595,7 +3598,7 @@ ixgbe_dev_tx_init(struct rte_eth_dev *dev)\n> static inline void\n> ixgbe_setup_loopback_link_82599(struct ixgbe_hw *hw)\n> {\n> - DEBUGFUNC(\"ixgbe_setup_loopback_link_82599\");\n> + PMD_INIT_FUNC_TRACE();\n>\n> if (ixgbe_verify_lesm_fw_enabled_82599(hw)) {\n> if (hw->mac.ops.acquire_swfw_sync(hw,\n> IXGBE_GSSR_MAC_CSR_SM) !=\n> --\n> 1.7.10.4\n>\n>", "headers": { "Return-Path": "<rolette@infiniteio.com>", "Received": [ "from mail-yh0-f53.google.com (mail-yh0-f53.google.com\n\t[209.85.213.53]) by dpdk.org (Postfix) with ESMTP id EB4F5683B\n\tfor <dev@dpdk.org>; Tue, 2 Sep 2014 15:39:23 +0200 (CEST)", "by mail-yh0-f53.google.com with SMTP id a41so4249849yho.12\n\tfor <dev@dpdk.org>; Tue, 02 Sep 2014 06:43:55 -0700 (PDT)", "by 10.170.96.213 with HTTP; Tue, 2 Sep 2014 06:43:55 -0700 (PDT)" ], "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20130820;\n\th=x-gm-message-state:mime-version:in-reply-to:references:date\n\t:message-id:subject:from:to:cc:content-type;\n\tbh=kI2jaxZJ5WNYJUZUrH+bgyxM9kEvOCss8yTS/wCECfs=;\n\tb=cvGVqsaGMPBwHuTpI0PE2lMt9J0PSL1fk++Ojn7g9zTNIU2uWyCoCyvDF73CGP8P6a\n\tabce6J0hD03GHoryOSGfhVx2DoLhvCQnyk0XaYIRAjhijkYxbTdCPht202avhnfzIufS\n\tDsR8uOJTVMAgtpl/K0/4w98bYyXMzT3+dxtZEXX690hZIUIUvgpOh6pxxtXavCLn5+yb\n\ttTYIDKpZIgBQx3uNv1ozaSDAySnAbm6sHY3WidXGnlSiTUxUSzW5j2H3t1q7EAv/tdke\n\tB2MCcqq7vpZKENo/k9rFO0yg1tq4EDwvFt1/XBTsw0JrvGm7MvdgIOf9+rMCWVUDAI94\n\t7tiw==", "X-Gm-Message-State": "ALoCoQnT5IYvrDzjTjwAZhlSkD1ovrhWK779pfNRXTc+l4+WGW0cxeexpeNHbhdtaXY/3mpPKHVl", "MIME-Version": "1.0", "X-Received": "by 10.236.117.37 with SMTP id i25mr25624655yhh.85.1409665435617; \n\tTue, 02 Sep 2014 06:43:55 -0700 (PDT)", "In-Reply-To": "<1409567080-27083-2-git-send-email-david.marchand@6wind.com>", "References": "<1409567080-27083-1-git-send-email-david.marchand@6wind.com>\n\t<1409567080-27083-2-git-send-email-david.marchand@6wind.com>", "Date": "Tue, 2 Sep 2014 08:43:55 -0500", "Message-ID": "<CADNuJVpLLfiC_ROBkYQguHUGTgXNC1Bdu7HwveXcA23_Q3twxA@mail.gmail.com>", "From": "Jay Rolette <rolette@infiniteio.com>", "To": "David Marchand <david.marchand@6wind.com>", "Content-Type": "text/plain; charset=UTF-8", "X-Content-Filtered-By": "Mailman/MimeDel 2.1.15", "Cc": "\"dev@dpdk.org\" <dev@dpdk.org>", "Subject": "Re: [dpdk-dev] [PATCH v2 01/17] ixgbe: use the right debug macro", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "X-List-Received-Date": "Tue, 02 Sep 2014 13:39:24 -0000" }, "addressed": null }, { "id": 622, "web_url": "http://patches.dpdk.org/comment/622/", "msgid": "<CALwxeUuC2O-YqB8yG01ZiBE2C7qQeCR2Km2gQ0i1fk+CHqXJ2w@mail.gmail.com>", "list_archive_url": "https://inbox.dpdk.org/dev/CALwxeUuC2O-YqB8yG01ZiBE2C7qQeCR2Km2gQ0i1fk+CHqXJ2w@mail.gmail.com", "date": "2014-09-02T14:16:55", "subject": "Re: [dpdk-dev] [PATCH v2 01/17] ixgbe: use the right debug macro", "submitter": { "id": 3, "url": "http://patches.dpdk.org/api/people/3/?format=api", "name": "David Marchand", "email": "david.marchand@6wind.com" }, "content": "Hello Jay,\n\n\nOn Tue, Sep 2, 2014 at 3:43 PM, Jay Rolette <rolette@infiniteio.com> wrote:\n\n>\n> On Mon, Sep 1, 2014 at 5:24 AM, David Marchand <david.marchand@6wind.com>\n> wrote:\n>\n>>\n>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_bypass.c\n>> b/lib/librte_pmd_ixgbe/ixgbe_bypass.c\n>> index 1d21dc0..1a980b8 100644\n>> --- a/lib/librte_pmd_ixgbe/ixgbe_bypass.c\n>> +++ b/lib/librte_pmd_ixgbe/ixgbe_bypass.c\n>> @@ -40,20 +40,20 @@\n>> #define BYPASS_STATUS_OFF_MASK 3\n>>\n>> /* Macros to check for invlaid function pointers. */\n>> -#define FUNC_PTR_OR_ERR_RET(func, retval) do { \\\n>> - if ((func) == NULL) { \\\n>> - DEBUGOUT(\"%s:%d function not supported\\n\", \\\n>> - __func__, __LINE__); \\\n>> - return (retval); \\\n>> - } \\\n>> +#define FUNC_PTR_OR_ERR_RET(func, retval) do { \\\n>> + if ((func) == NULL) { \\\n>> + PMD_DRV_LOG(\"%s:%d function not supported\", \\\n>> + __func__, __LINE__); \\\n>> + return retval; \\\n>>\n> Need to keep the parens around retval in your macro\n>\n\nActually, checkpatch complained about this.\nSo I can keep the parenthesis, but then I don't want Thomas to tell me my\npatch does not pass checkpatch :-)\n\n\n\n> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c\n>> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c\n>> index dfc2076..46962bc 100644\n>> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c\n>> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c\n>> @@ -1765,33 +1765,36 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev *dev,\n>> tx_free_thresh = (uint16_t)((tx_conf->tx_free_thresh) ?\n>> tx_conf->tx_free_thresh : DEFAULT_TX_FREE_THRESH);\n>> if (tx_rs_thresh >= (nb_desc - 2)) {\n>> - RTE_LOG(ERR, PMD, \"tx_rs_thresh must be less than the\n>> number \"\n>> - \"of TX descriptors minus 2. (tx_rs_thresh=%u\n>> port=%d \"\n>> - \"queue=%d)\\n\", (unsigned int)tx_rs_thresh,\n>> - (int)dev->data->port_id, (int)queue_idx);\n>> + PMD_INIT_LOG(ERR, \"tx_rs_thresh must be less than the\n>> number \"\n>> + \"of TX descriptors minus 2. (tx_rs_thresh=%u\n>> \"\n>> + \"port=%d queue=%d)\\n\", (unsigned\n>> int)tx_rs_thresh,\n>>\n> Embedded newline on this log message. I noticed you have \\n still on all\n> of the PMD_INIT_LOG() calls following. Intended?\n>\n\nYep intended, a patch after this removes all \\n (idem for the other\ncomments).\n\nThanks.", "headers": { "Return-Path": "<david.marchand@6wind.com>", "Received": [ "from mail-ob0-f178.google.com (mail-ob0-f178.google.com\n\t[209.85.214.178]) by dpdk.org (Postfix) with ESMTP id A7D9C58EE\n\tfor <dev@dpdk.org>; Tue, 2 Sep 2014 16:12:24 +0200 (CEST)", "by mail-ob0-f178.google.com with SMTP id uy5so4812967obc.23\n\tfor <dev@dpdk.org>; Tue, 02 Sep 2014 07:16:56 -0700 (PDT)", "by 10.202.73.74 with HTTP; Tue, 2 Sep 2014 07:16:55 -0700 (PDT)" ], "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20130820;\n\th=x-gm-message-state:mime-version:in-reply-to:references:date\n\t:message-id:subject:from:to:cc:content-type;\n\tbh=EuzF4a6QJHcy8CdJX9nheVXlYNKSgTQTboAmzB6hluw=;\n\tb=hSCo9pd1DrgN8tWMcXTb1/DBvV+9AsxYmVCUsUTNwAG1VuKTtPv7jNMFeiXHWZkDnN\n\tgMh862KABJW8YpAttpG/1HlWlKyF/dhhaWh3vniwkTe1f8Q/f7ovramYpuYEOSkpUixE\n\tCypRN7d9lbAqlgbYL7gNCaVGRgEiVWm4bRPFBbPHZjn5lq3kk/zqF3OSkpfGKEfDzdgM\n\thCUj+kmN+MroCYB2D9w7Q/2BJ1MuzxQGiD7RTWKd1ya6GTlPkCCKfN9vMLuLdw+A2JHu\n\t8lVKvP3rPVeMyQqd1wpVBPvz5QRFYQCcE5xmaCRChdeEUkoz92ypbDFQ/rKRt7bZGDSr\n\tKarw==", "X-Gm-Message-State": "ALoCoQls4bAGcISWBJWFX/zpr8q9RCVwEiU9R8IcpvczNH0DOK1qbOY52d9z1tHd3R3hkZSgqljh", "MIME-Version": "1.0", "X-Received": "by 10.60.156.198 with SMTP id wg6mr2353099oeb.69.1409667415844; \n\tTue, 02 Sep 2014 07:16:55 -0700 (PDT)", "In-Reply-To": "<CADNuJVpLLfiC_ROBkYQguHUGTgXNC1Bdu7HwveXcA23_Q3twxA@mail.gmail.com>", "References": "<1409567080-27083-1-git-send-email-david.marchand@6wind.com>\n\t<1409567080-27083-2-git-send-email-david.marchand@6wind.com>\n\t<CADNuJVpLLfiC_ROBkYQguHUGTgXNC1Bdu7HwveXcA23_Q3twxA@mail.gmail.com>", "Date": "Tue, 2 Sep 2014 16:16:55 +0200", "Message-ID": "<CALwxeUuC2O-YqB8yG01ZiBE2C7qQeCR2Km2gQ0i1fk+CHqXJ2w@mail.gmail.com>", "From": "David Marchand <david.marchand@6wind.com>", "To": "Jay Rolette <rolette@infiniteio.com>", "Content-Type": "text/plain; charset=UTF-8", "X-Content-Filtered-By": "Mailman/MimeDel 2.1.15", "Cc": "\"dev@dpdk.org\" <dev@dpdk.org>", "Subject": "Re: [dpdk-dev] [PATCH v2 01/17] ixgbe: use the right debug macro", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "X-List-Received-Date": "Tue, 02 Sep 2014 14:12:25 -0000" }, "addressed": null }, { "id": 623, "web_url": "http://patches.dpdk.org/comment/623/", "msgid": "<3333103.135tnuTvaT@xps13>", "list_archive_url": "https://inbox.dpdk.org/dev/3333103.135tnuTvaT@xps13", "date": "2014-09-02T14:21:18", "subject": "Re: [dpdk-dev] [PATCH v2 01/17] ixgbe: use the right debug macro", "submitter": { "id": 1, "url": "http://patches.dpdk.org/api/people/1/?format=api", "name": "Thomas Monjalon", "email": "thomas.monjalon@6wind.com" }, "content": "2014-09-02 16:16, David Marchand:\n> >> /* Macros to check for invlaid function pointers. */\n\nInvlaid is an invalid word ;)\n\n> >> -#define FUNC_PTR_OR_ERR_RET(func, retval) do { \\\n> >> - if ((func) == NULL) { \\\n> >> - DEBUGOUT(\"%s:%d function not supported\\n\", \\\n> >> - __func__, __LINE__); \\\n> >> - return (retval); \\\n> >> - } \\\n> >> +#define FUNC_PTR_OR_ERR_RET(func, retval) do { \\\n> >> + if ((func) == NULL) { \\\n> >> + PMD_DRV_LOG(\"%s:%d function not supported\", \\\n> >> + __func__, __LINE__); \\\n> >> + return retval; \\\n> >>\n> > Need to keep the parens around retval in your macro\n> \n> Actually, checkpatch complained about this.\n> So I can keep the parenthesis, but then I don't want Thomas to tell me my\n> patch does not pass checkpatch :-)\n\nYou're right, I care about checkpatch :)\nI don't see a case where parens are needed with return. Please give an example.", "headers": { "Return-Path": "<thomas.monjalon@6wind.com>", "Received": [ "from mail-we0-f180.google.com (mail-we0-f180.google.com\n\t[74.125.82.180]) by dpdk.org (Postfix) with ESMTP id 18D2858EE\n\tfor <dev@dpdk.org>; Tue, 2 Sep 2014 16:16:53 +0200 (CEST)", "by mail-we0-f180.google.com with SMTP id w61so6991210wes.25\n\tfor <dev@dpdk.org>; Tue, 02 Sep 2014 07:21:25 -0700 (PDT)", "from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136])\n\tby mx.google.com with ESMTPSA id\n\tgr3sm35521370wib.12.2014.09.02.07.21.24 for <multiple recipients>\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tTue, 02 Sep 2014 07:21:24 -0700 (PDT)" ], "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20130820;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:organization\n\t:user-agent:in-reply-to:references:mime-version\n\t:content-transfer-encoding:content-type;\n\tbh=fEmoplTKxf4B+TiJoXexmDNZKUkLWyE00RoqaM7IDFs=;\n\tb=X9Lm08RfINuAkZWtQBIFzQUjgFjYOSc1L+cdQqLE2ehDKU9rRoLivhCxSOU4hwETLe\n\tF5/6Ws4XLlnHKplEN7GClI1RhM0CPIl/VXChVQSmEh0H/wH5DcCds1leuJIivqy6l9Lq\n\tgjp7PUWVuoRT8CdxpwyahNASWP7/eQiCHuhbpPF2UAYjGSdGwux06ZgJcN8AbL5UwGVU\n\toxmyIo5nDyssG94RFNsA7r8P0uqoKobXtfEx39KR3yUyRIcaid3GvGf7J+dKqfx0HlrM\n\tX0kouvJwTymA/+nNTqDSl24pLmTDskGM20a4SdFDeAwINbGD2DlCzyQ+QILNRVVYaF/t\n\tCCgQ==", "X-Gm-Message-State": "ALoCoQk/TukLq0hkfn+P3dwTlzhV+SeYsjqaV4uWZgjj0VFTLE4U6x+syW0LVZcEIQEMQUbwE9RH", "X-Received": "by 10.180.184.20 with SMTP id eq20mr16207690wic.61.1409667685314;\n\tTue, 02 Sep 2014 07:21:25 -0700 (PDT)", "From": "Thomas Monjalon <thomas.monjalon@6wind.com>", "To": "Jay Rolette <rolette@infiniteio.com>", "Date": "Tue, 02 Sep 2014 16:21:18 +0200", "Message-ID": "<3333103.135tnuTvaT@xps13>", "Organization": "6WIND", "User-Agent": "KMail/4.13.3 (Linux/3.15.8-1-ARCH; KDE/4.13.3; x86_64; ; )", "In-Reply-To": "<CALwxeUuC2O-YqB8yG01ZiBE2C7qQeCR2Km2gQ0i1fk+CHqXJ2w@mail.gmail.com>", "References": "<1409567080-27083-1-git-send-email-david.marchand@6wind.com>\n\t<CADNuJVpLLfiC_ROBkYQguHUGTgXNC1Bdu7HwveXcA23_Q3twxA@mail.gmail.com>\n\t<CALwxeUuC2O-YqB8yG01ZiBE2C7qQeCR2Km2gQ0i1fk+CHqXJ2w@mail.gmail.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "7Bit", "Content-Type": "text/plain; charset=\"us-ascii\"", "Cc": "dev@dpdk.org", "Subject": "Re: [dpdk-dev] [PATCH v2 01/17] ixgbe: use the right debug macro", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "X-List-Received-Date": "Tue, 02 Sep 2014 14:16:53 -0000" }, "addressed": null }, { "id": 625, "web_url": "http://patches.dpdk.org/comment/625/", "msgid": "<CADNuJVpAYrDCN_VsjygbA4K=Jh6-e5BAxYK-v+RNu2nR7UV8Ow@mail.gmail.com>", "list_archive_url": "https://inbox.dpdk.org/dev/CADNuJVpAYrDCN_VsjygbA4K=Jh6-e5BAxYK-v+RNu2nR7UV8Ow@mail.gmail.com", "date": "2014-09-02T17:57:05", "subject": "Re: [dpdk-dev] [PATCH v2 01/17] ixgbe: use the right debug macro", "submitter": { "id": 61, "url": "http://patches.dpdk.org/api/people/61/?format=api", "name": "Jay Rolette", "email": "rolette@infiniteio.com" }, "content": "On Tue, Sep 2, 2014 at 9:21 AM, Thomas Monjalon <thomas.monjalon@6wind.com>\nwrote:\n\n> >> -#define FUNC_PTR_OR_ERR_RET(func, retval) do { \\\n> > >> - if ((func) == NULL) { \\\n> > >> - DEBUGOUT(\"%s:%d function not supported\\n\", \\\n> > >> - __func__, __LINE__); \\\n> > >> - return (retval); \\\n> > >> - } \\\n> > >> +#define FUNC_PTR_OR_ERR_RET(func, retval) do { \\\n> > >> + if ((func) == NULL) { \\\n> > >> + PMD_DRV_LOG(\"%s:%d function not supported\", \\\n> > >> + __func__, __LINE__); \\\n> > >> + return retval; \\\n> > >>\n> > > Need to keep the parens around retval in your macro\n> >\n> > Actually, checkpatch complained about this.\n> > So I can keep the parenthesis, but then I don't want Thomas to tell me my\n> > patch does not pass checkpatch :-)\n>\n> You're right, I care about checkpatch :)\n> I don't see a case where parens are needed with return. Please give an\n> example.\n\n\nLooking at it again, in this specific case you are correct. It is good\nhygiene to always use parens around macro arguments, but this specific case\nis ok without. It does add a small bit of danger if retval ends up getting\nused elsewhere in the macro, but that's about it. Probably not worth\nredoing the patch over that.\n\nJay", "headers": { "Return-Path": "<rolette@infiniteio.com>", "Received": [ "from mail-yh0-f53.google.com (mail-yh0-f53.google.com\n\t[209.85.213.53]) by dpdk.org (Postfix) with ESMTP id 3669A5941\n\tfor <dev@dpdk.org>; Tue, 2 Sep 2014 19:52:33 +0200 (CEST)", "by mail-yh0-f53.google.com with SMTP id a41so4467321yho.26\n\tfor <dev@dpdk.org>; Tue, 02 Sep 2014 10:57:05 -0700 (PDT)", "by 10.170.96.213 with HTTP; Tue, 2 Sep 2014 10:57:05 -0700 (PDT)" ], "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20130820;\n\th=x-gm-message-state:mime-version:in-reply-to:references:date\n\t:message-id:subject:from:to:cc:content-type;\n\tbh=yrZB8Ly2Ei2jmM6KLr1PwyfdbJny7bTrrqYbyZ+vXtI=;\n\tb=duEHlPZ4Q7B+1jrZOKDlEOCQWSd9An2QodCbNDfR6lxhcdr5IsQRbRDtxZ4s2d1xIP\n\tIF0fk/g89q/nmWv1FOQsRlIaayWY1JzXiN0eGyzpQypaSHbnYbNg/KtUfFXdpKtMGJND\n\tCm0epvHdXLrlW3QMOopYl0G+4mSueC3SvkcqhJ1BEb29sRaOte2VrP1M+0a2uty+7WtY\n\tHmAnv2UqVKWQVmqN5ODcuYL5eUaJwpi9heMUZOXEQ3pVpbL+Kp4WvFTGvqYKymaXdnG8\n\tDqfWZ6UlozxvCCH9yyJeT4Kf9rFlSpDDdI3NRcuHmFg73dHfYqGRjhFrtQ0OgZoUZuaq\n\tle0w==", "X-Gm-Message-State": "ALoCoQkByaUY+L87/s3xoBTN3814KNSbkWBUL4wup1fpkXEg9IhcvyRe3IRsZqha0+LNuY6HVFb6", "MIME-Version": "1.0", "X-Received": "by 10.236.66.142 with SMTP id h14mr14617375yhd.104.1409680625344;\n\tTue, 02 Sep 2014 10:57:05 -0700 (PDT)", "In-Reply-To": "<3333103.135tnuTvaT@xps13>", "References": "<1409567080-27083-1-git-send-email-david.marchand@6wind.com>\n\t<CADNuJVpLLfiC_ROBkYQguHUGTgXNC1Bdu7HwveXcA23_Q3twxA@mail.gmail.com>\n\t<CALwxeUuC2O-YqB8yG01ZiBE2C7qQeCR2Km2gQ0i1fk+CHqXJ2w@mail.gmail.com>\n\t<3333103.135tnuTvaT@xps13>", "Date": "Tue, 2 Sep 2014 12:57:05 -0500", "Message-ID": "<CADNuJVpAYrDCN_VsjygbA4K=Jh6-e5BAxYK-v+RNu2nR7UV8Ow@mail.gmail.com>", "From": "Jay Rolette <rolette@infiniteio.com>", "To": "Thomas Monjalon <thomas.monjalon@6wind.com>", "Content-Type": "text/plain; charset=UTF-8", "X-Content-Filtered-By": "Mailman/MimeDel 2.1.15", "Cc": "\"dev@dpdk.org\" <dev@dpdk.org>", "Subject": "Re: [dpdk-dev] [PATCH v2 01/17] ixgbe: use the right debug macro", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "X-List-Received-Date": "Tue, 02 Sep 2014 17:52:33 -0000" }, "addressed": null } ]