From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yh0-f41.google.com (mail-yh0-f41.google.com [209.85.213.41]) by dpdk.org (Postfix) with ESMTP id 367215947 for ; Tue, 2 Sep 2014 20:20:31 +0200 (CEST) Received: by mail-yh0-f41.google.com with SMTP id 29so4595369yhl.0 for ; Tue, 02 Sep 2014 11:25:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=4QtwQelNMF5NMgE27khecp6ZAqrtcMvW2TxsCaDRp78=; b=iJFAAnu1ZZhKBVH10QPJa8g6h4xsPhA9i3NZcWJslbSlEmiR8BZ3HRab6qfBZZEoET Dp6kTaiUdtEsASpEB5FZrhYDPOA8Zatjgs+Mf4PN7Hzn9Dt1ByMEn+2ILWoatj2OpgIa i8XOA7ob2vsnGwNCgpW6aNlAjWTKbg/re10cS11oACU5OqNu4xF9dz8Q0DPQCpQEnopg F800j12RiPD7K38cualSCmnyv3dq1RhtVN6bk3kFjarX1QaRy5wdVubG+VQOzVzC5XBC olqSRLcdm2Q16z5i1wKj1TBfYW4ivQZWVyoogBwWkBeX8bZpsvLzIGGFhMdf0BpHcBPW Ngmg== X-Gm-Message-State: ALoCoQkX+tJf3LYHopjrMpUwq9IubBW8hTWZTX5Vk0OpGowIFLk0dZQ0nQkPDGdqn82GIko/Di0j MIME-Version: 1.0 X-Received: by 10.236.120.163 with SMTP id p23mr3503068yhh.139.1409682303695; Tue, 02 Sep 2014 11:25:03 -0700 (PDT) Received: by 10.170.96.213 with HTTP; Tue, 2 Sep 2014 11:25:03 -0700 (PDT) In-Reply-To: <1409567080-27083-8-git-send-email-david.marchand@6wind.com> References: <1409567080-27083-1-git-send-email-david.marchand@6wind.com> <1409567080-27083-8-git-send-email-david.marchand@6wind.com> Date: Tue, 2 Sep 2014 13:25:03 -0500 Message-ID: From: Jay Rolette To: David Marchand Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2 07/17] i40e: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Sep 2014 18:20:31 -0000 On Mon, Sep 1, 2014 at 5:24 AM, David Marchand wrote: > - Don't use DEBUGFUNC macro in non-shared code. > - Don't use printf for logs. > - We should avoid calling RTE_LOG directly as pmd provides a wrapper for > logs. > > Signed-off-by: David Marchand > --- > lib/librte_pmd_i40e/i40e_ethdev.c | 146 > +++++++++++++++++----------------- > lib/librte_pmd_i40e/i40e_ethdev_vf.c | 2 +- > lib/librte_pmd_i40e/i40e_pf.c | 6 +- > lib/librte_pmd_i40e/i40e_rxtx.c | 64 +++++++-------- > 4 files changed, 110 insertions(+), 108 deletions(-) > > diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c > b/lib/librte_pmd_i40e/i40e_ethdev.c > index 4e65ca4..352beb1 100644 > --- a/lib/librte_pmd_i40e/i40e_ethdev.c > +++ b/lib/librte_pmd_i40e/i40e_ethdev.c > @@ -1059,24 +1059,23 @@ i40e_update_vsi_stats(struct i40e_vsi *vsi) > &oes->tx_errors, &nes->tx_errors); > vsi->offset_loaded = true; > > -#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER > - printf("***************** VSI[%u] stats start > *******************\n", > - > vsi->vsi_id); > - printf("rx_bytes: %lu\n", nes->rx_bytes); > - printf("rx_unicast: %lu\n", nes->rx_unicast); > - printf("rx_multicast: %lu\n", nes->rx_multicast); > - printf("rx_broadcast: %lu\n", nes->rx_broadcast); > - printf("rx_discards: %lu\n", nes->rx_discards); > - printf("rx_unknown_protocol: %lu\n", nes->rx_unknown_protocol); > - printf("tx_bytes: %lu\n", nes->tx_bytes); > - printf("tx_unicast: %lu\n", nes->tx_unicast); > - printf("tx_multicast: %lu\n", nes->tx_multicast); > - printf("tx_broadcast: %lu\n", nes->tx_broadcast); > - printf("tx_discards: %lu\n", nes->tx_discards); > - printf("tx_errors: %lu\n", nes->tx_errors); > - printf("***************** VSI[%u] stats end *******************\n", > - > vsi->vsi_id); > -#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */ > + PMD_DRV_LOG(DEBUG, "***************** VSI[%u] stats start > *******************\n", > + vsi->vsi_id); > + PMD_DRV_LOG(DEBUG, "rx_bytes: %lu\n", nes->rx_bytes); > + PMD_DRV_LOG(DEBUG, "rx_unicast: %lu\n", nes->rx_unicast); > + PMD_DRV_LOG(DEBUG, "rx_multicast: %lu\n", > nes->rx_multicast); > + PMD_DRV_LOG(DEBUG, "rx_broadcast: %lu\n", > nes->rx_broadcast); > + PMD_DRV_LOG(DEBUG, "rx_discards: %lu\n", nes->rx_discards); > + PMD_DRV_LOG(DEBUG, "rx_unknown_protocol: %lu\n", > + nes->rx_unknown_protocol); > + PMD_DRV_LOG(DEBUG, "tx_bytes: %lu\n", nes->tx_bytes); > + PMD_DRV_LOG(DEBUG, "tx_unicast: %lu\n", nes->tx_unicast); > + PMD_DRV_LOG(DEBUG, "tx_multicast: %lu\n", > nes->tx_multicast); > + PMD_DRV_LOG(DEBUG, "tx_broadcast: %lu\n", > nes->tx_broadcast); > + PMD_DRV_LOG(DEBUG, "tx_discards: %lu\n", nes->tx_discards); > + PMD_DRV_LOG(DEBUG, "tx_errors: %lu\n", nes->tx_errors); > + PMD_DRV_LOG(DEBUG, "***************** VSI[%u] stats end > *******************\n", > + vsi->vsi_id); > } > > /* Get all statistics of a port */ > @@ -1277,69 +1276,74 @@ i40e_dev_stats_get(struct rte_eth_dev *dev, struct > rte_eth_stats *stats) > if (pf->main_vsi) > i40e_update_vsi_stats(pf->main_vsi); > > -#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER > - printf("***************** PF stats start *******************\n"); > - printf("rx_bytes: %lu\n", ns->eth.rx_bytes); > - printf("rx_unicast: %lu\n", ns->eth.rx_unicast); > - printf("rx_multicast: %lu\n", ns->eth.rx_multicast); > - printf("rx_broadcast: %lu\n", ns->eth.rx_broadcast); > - printf("rx_discards: %lu\n", ns->eth.rx_discards); > - printf("rx_unknown_protocol: %lu\n", ns->eth.rx_unknown_protocol); > - printf("tx_bytes: %lu\n", ns->eth.tx_bytes); > - printf("tx_unicast: %lu\n", ns->eth.tx_unicast); > - printf("tx_multicast: %lu\n", ns->eth.tx_multicast); > - printf("tx_broadcast: %lu\n", ns->eth.tx_broadcast); > - printf("tx_discards: %lu\n", ns->eth.tx_discards); > - printf("tx_errors: %lu\n", ns->eth.tx_errors); > - > - printf("tx_dropped_link_down: %lu\n", > ns->tx_dropped_link_down); > - printf("crc_errors: %lu\n", ns->crc_errors); > - printf("illegal_bytes: %lu\n", ns->illegal_bytes); > - printf("error_bytes: %lu\n", ns->error_bytes); > - printf("mac_local_faults: %lu\n", ns->mac_local_faults); > - printf("mac_remote_faults: %lu\n", ns->mac_remote_faults); > - printf("rx_length_errors: %lu\n", ns->rx_length_errors); > - printf("link_xon_rx: %lu\n", ns->link_xon_rx); > - printf("link_xoff_rx: %lu\n", ns->link_xoff_rx); > + PMD_DRV_LOG(DEBUG, "***************** PF stats start > *******************\n"); > + PMD_DRV_LOG(DEBUG, "rx_bytes: %lu\n", ns->eth.rx_bytes); > + PMD_DRV_LOG(DEBUG, "rx_unicast: %lu\n", > ns->eth.rx_unicast); > + PMD_DRV_LOG(DEBUG, "rx_multicast: %lu\n", > ns->eth.rx_multicast); > + PMD_DRV_LOG(DEBUG, "rx_broadcast: %lu\n", > ns->eth.rx_broadcast); > + PMD_DRV_LOG(DEBUG, "rx_discards: %lu\n", > ns->eth.rx_discards); > + PMD_DRV_LOG(DEBUG, "rx_unknown_protocol: %lu\n", > + ns->eth.rx_unknown_protocol); > + PMD_DRV_LOG(DEBUG, "tx_bytes: %lu\n", ns->eth.tx_bytes); > + PMD_DRV_LOG(DEBUG, "tx_unicast: %lu\n", > ns->eth.tx_unicast); > + PMD_DRV_LOG(DEBUG, "tx_multicast: %lu\n", > ns->eth.tx_multicast); > + PMD_DRV_LOG(DEBUG, "tx_broadcast: %lu\n", > ns->eth.tx_broadcast); > + PMD_DRV_LOG(DEBUG, "tx_discards: %lu\n", > ns->eth.tx_discards); > + PMD_DRV_LOG(DEBUG, "tx_errors: %lu\n", > ns->eth.tx_errors); > + > + PMD_DRV_LOG(DEBUG, "tx_dropped_link_down: %lu\n", > + ns->tx_dropped_link_down); > + PMD_DRV_LOG(DEBUG, "crc_errors: %lu\n", > ns->crc_errors); > + PMD_DRV_LOG(DEBUG, "illegal_bytes: %lu\n", > + ns->illegal_bytes); > + PMD_DRV_LOG(DEBUG, "error_bytes: %lu\n", > ns->error_bytes); > + PMD_DRV_LOG(DEBUG, "mac_local_faults: %lu\n", > + ns->mac_local_faults); > + PMD_DRV_LOG(DEBUG, "mac_remote_faults: %lu\n", > + ns->mac_remote_faults); > + PMD_DRV_LOG(DEBUG, "rx_length_errors: %lu\n", > + ns->rx_length_errors); > + PMD_DRV_LOG(DEBUG, "link_xon_rx: %lu\n", > ns->link_xon_rx); > + PMD_DRV_LOG(DEBUG, "link_xoff_rx: %lu\n", > ns->link_xoff_rx); > for (i = 0; i < 8; i++) { > - printf("priority_xon_rx[%d]: %lu\n", > + PMD_DRV_LOG(DEBUG, "priority_xon_rx[%d]: %lu\n", > i, ns->priority_xon_rx[i]); > - printf("priority_xoff_rx[%d]: %lu\n", > + PMD_DRV_LOG(DEBUG, "priority_xoff_rx[%d]: %lu\n", > i, ns->priority_xoff_rx[i]); > } > - printf("link_xon_tx: %lu\n", ns->link_xon_tx); > - printf("link_xoff_tx: %lu\n", ns->link_xoff_tx); > + PMD_DRV_LOG(DEBUG, "link_xon_tx: %lu\n", > ns->link_xon_tx); > + PMD_DRV_LOG(DEBUG, "link_xoff_tx: %lu\n", > ns->link_xoff_tx); > for (i = 0; i < 8; i++) { > - printf("priority_xon_tx[%d]: %lu\n", > + PMD_DRV_LOG(DEBUG, "priority_xon_tx[%d]: %lu\n", > i, ns->priority_xon_tx[i]); > - printf("priority_xoff_tx[%d]: %lu\n", > + PMD_DRV_LOG(DEBUG, "priority_xoff_tx[%d]: %lu\n", > i, ns->priority_xoff_tx[i]); > - printf("priority_xon_2_xoff[%d]: %lu\n", > + PMD_DRV_LOG(DEBUG, "priority_xon_2_xoff[%d]: %lu\n", > i, ns->priority_xon_2_xoff[i]); > } > - printf("rx_size_64: %lu\n", ns->rx_size_64); > - printf("rx_size_127: %lu\n", ns->rx_size_127); > - printf("rx_size_255: %lu\n", ns->rx_size_255); > - printf("rx_size_511: %lu\n", ns->rx_size_511); > - printf("rx_size_1023: %lu\n", ns->rx_size_1023); > - printf("rx_size_1522: %lu\n", ns->rx_size_1522); > - printf("rx_size_big: %lu\n", ns->rx_size_big); > - printf("rx_undersize: %lu\n", ns->rx_undersize); > - printf("rx_fragments: %lu\n", ns->rx_fragments); > - printf("rx_oversize: %lu\n", ns->rx_oversize); > - printf("rx_jabber: %lu\n", ns->rx_jabber); > - printf("tx_size_64: %lu\n", ns->tx_size_64); > - printf("tx_size_127: %lu\n", ns->tx_size_127); > - printf("tx_size_255: %lu\n", ns->tx_size_255); > - printf("tx_size_511: %lu\n", ns->tx_size_511); > - printf("tx_size_1023: %lu\n", ns->tx_size_1023); > - printf("tx_size_1522: %lu\n", ns->tx_size_1522); > - printf("tx_size_big: %lu\n", ns->tx_size_big); > - printf("mac_short_packet_dropped: %lu\n", > + PMD_DRV_LOG(DEBUG, "rx_size_64: %lu\n", > ns->rx_size_64); > + PMD_DRV_LOG(DEBUG, "rx_size_127: %lu\n", > ns->rx_size_127); > + PMD_DRV_LOG(DEBUG, "rx_size_255: %lu\n", > ns->rx_size_255); > + PMD_DRV_LOG(DEBUG, "rx_size_511: %lu\n", > ns->rx_size_511); > + PMD_DRV_LOG(DEBUG, "rx_size_1023: %lu\n", > ns->rx_size_1023); > + PMD_DRV_LOG(DEBUG, "rx_size_1522: %lu\n", > ns->rx_size_1522); > + PMD_DRV_LOG(DEBUG, "rx_size_big: %lu\n", > ns->rx_size_big); > + PMD_DRV_LOG(DEBUG, "rx_undersize: %lu\n", > ns->rx_undersize); > + PMD_DRV_LOG(DEBUG, "rx_fragments: %lu\n", > ns->rx_fragments); > + PMD_DRV_LOG(DEBUG, "rx_oversize: %lu\n", > ns->rx_oversize); > + PMD_DRV_LOG(DEBUG, "rx_jabber: %lu\n", > ns->rx_jabber); > + PMD_DRV_LOG(DEBUG, "tx_size_64: %lu\n", > ns->tx_size_64); > + PMD_DRV_LOG(DEBUG, "tx_size_127: %lu\n", > ns->tx_size_127); > + PMD_DRV_LOG(DEBUG, "tx_size_255: %lu\n", > ns->tx_size_255); > + PMD_DRV_LOG(DEBUG, "tx_size_511: %lu\n", > ns->tx_size_511); > + PMD_DRV_LOG(DEBUG, "tx_size_1023: %lu\n", > ns->tx_size_1023); > + PMD_DRV_LOG(DEBUG, "tx_size_1522: %lu\n", > ns->tx_size_1522); > + PMD_DRV_LOG(DEBUG, "tx_size_big: %lu\n", > ns->tx_size_big); > + PMD_DRV_LOG(DEBUG, "mac_short_packet_dropped: %lu\n", > ns->mac_short_packet_dropped); > - printf("checksum_error: %lu\n", ns->checksum_error); > - printf("***************** PF stats end ********************\n"); > -#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */ > + PMD_DRV_LOG(DEBUG, "checksum_error: %lu\n", > + ns->checksum_error); > + PMD_DRV_LOG(DEBUG, "***************** PF stats end > ********************\n"); > } > > /* Reset the statistics */ > diff --git a/lib/librte_pmd_i40e/i40e_ethdev_vf.c > b/lib/librte_pmd_i40e/i40e_ethdev_vf.c > index d8552ad..9c1ae94 100644 > --- a/lib/librte_pmd_i40e/i40e_ethdev_vf.c > +++ b/lib/librte_pmd_i40e/i40e_ethdev_vf.c > @@ -1132,7 +1132,7 @@ static int > rte_i40evf_pmd_init(const char *name __rte_unused, > const char *params __rte_unused) > { > - DEBUGFUNC("rte_i40evf_pmd_init"); > + PMD_INIT_FUNC_TRACE(); > > rte_eth_driver_register(&rte_i40evf_pmd); > > diff --git a/lib/librte_pmd_i40e/i40e_pf.c b/lib/librte_pmd_i40e/i40e_pf.c > index e8b154d..eec291f 100644 > --- a/lib/librte_pmd_i40e/i40e_pf.c > +++ b/lib/librte_pmd_i40e/i40e_pf.c > @@ -253,10 +253,8 @@ i40e_pf_host_send_msg_to_vf(struct i40e_pf_vf *vf, > ret = i40e_aq_send_msg_to_vf(hw, abs_vf_id, opcode, retval, > msg, msglen, NULL); > if (ret) { > - PMD_DRV_LOG(ERR, "Fail to send message to VF, err %u\n", > - hw->aq.asq_last_status); > - printf("Fail to send message to VF, err %u\n", > - hw->aq.asq_last_status); > + PMD_INIT_LOG(ERR, "Fail to send message to VF, err %u\n", > + hw->aq.asq_last_status); > } > > return ret; > diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c > b/lib/librte_pmd_i40e/i40e_rxtx.c > index f153844..6987200 100644 > --- a/lib/librte_pmd_i40e/i40e_rxtx.c > +++ b/lib/librte_pmd_i40e/i40e_rxtx.c > @@ -1788,50 +1788,50 @@ i40e_dev_tx_queue_setup(struct rte_eth_dev *dev, > tx_free_thresh = (uint16_t)((tx_conf->tx_free_thresh) ? > tx_conf->tx_free_thresh : DEFAULT_TX_FREE_THRESH); > if (tx_rs_thresh >= (nb_desc - 2)) { > - RTE_LOG(ERR, PMD, "tx_rs_thresh must be less than the " > - "number of TX descriptors minus 2. " > - "(tx_rs_thresh=%u port=%d queue=%d)\n", > - (unsigned int)tx_rs_thresh, > - (int)dev->data->port_id, > - (int)queue_idx); > + PMD_INIT_LOG(ERR, "tx_rs_thresh must be less than the " > + "number of TX descriptors minus 2. " > + "(tx_rs_thresh=%u port=%d queue=%d)\n", > + (unsigned int)tx_rs_thresh, > + (int)dev->data->port_id, > + (int)queue_idx); > return I40E_ERR_PARAM; > } > if (tx_free_thresh >= (nb_desc - 3)) { > - RTE_LOG(ERR, PMD, "tx_rs_thresh must be less than the " > - "tx_free_thresh must be less than the " > - "number of TX descriptors minus 3. " > - "(tx_free_thresh=%u port=%d queue=%d)\n", > - (unsigned int)tx_free_thresh, > - (int)dev->data->port_id, > - (int)queue_idx); > + PMD_INIT_LOG(ERR, "tx_rs_thresh must be less than the " > + "tx_free_thresh must be less than the " > + "number of TX descriptors minus 3. " > + "(tx_free_thresh=%u port=%d queue=%d)\n", > + (unsigned int)tx_free_thresh, > + (int)dev->data->port_id, > + (int)queue_idx); > return I40E_ERR_PARAM; > } > if (tx_rs_thresh > tx_free_thresh) { > - RTE_LOG(ERR, PMD, "tx_rs_thresh must be less than or " > - "equal to tx_free_thresh. > (tx_free_thresh=%u" > - " tx_rs_thresh=%u port=%d queue=%d)\n", > - (unsigned > int)tx_free_thresh, > - (unsigned int)tx_rs_thresh, > - (int)dev->data->port_id, > - (int)queue_idx); > + PMD_INIT_LOG(ERR, "tx_rs_thresh must be less than or " > + "equal to tx_free_thresh. (tx_free_thresh=%u" > + " tx_rs_thresh=%u port=%d queue=%d)\n", > + (unsigned int)tx_free_thresh, > + (unsigned int)tx_rs_thresh, > + (int)dev->data->port_id, > + (int)queue_idx); > return I40E_ERR_PARAM; > } > if ((nb_desc % tx_rs_thresh) != 0) { > - RTE_LOG(ERR, PMD, "tx_rs_thresh must be a divisor of the " > - "number of TX descriptors. > (tx_rs_thresh=%u" > - " port=%d queue=%d)\n", > - (unsigned int)tx_rs_thresh, > - (int)dev->data->port_id, > - (int)queue_idx); > + PMD_INIT_LOG(ERR, "tx_rs_thresh must be a divisor of the " > + "number of TX descriptors. (tx_rs_thresh=%u" > + " port=%d queue=%d)\n", > + (unsigned int)tx_rs_thresh, > + (int)dev->data->port_id, > + (int)queue_idx); > return I40E_ERR_PARAM; > } > if ((tx_rs_thresh > 1) && (tx_conf->tx_thresh.wthresh != 0)) { > - RTE_LOG(ERR, PMD, "TX WTHRESH must be set to 0 if " > - "tx_rs_thresh is greater than 1. " > - "(tx_rs_thresh=%u port=%d queue=%d)\n", > - (unsigned int)tx_rs_thresh, > - (int)dev->data->port_id, > - (int)queue_idx); > + PMD_INIT_LOG(ERR, "TX WTHRESH must be set to 0 if " > + "tx_rs_thresh is greater than 1. " > + "(tx_rs_thresh=%u port=%d queue=%d)\n", > + (unsigned int)tx_rs_thresh, > + (int)dev->data->port_id, > + (int)queue_idx); > return I40E_ERR_PARAM; > } > > -- > 1.7.10.4 > > Reviewed-by: Jay Rolette