From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-we0-f170.google.com (mail-we0-f170.google.com [74.125.82.170]) by dpdk.org (Postfix) with ESMTP id 6FF985A69 for ; Mon, 9 Mar 2015 13:53:39 +0100 (CET) Received: by wevl61 with SMTP id l61so9024819wev.10 for ; Mon, 09 Mar 2015 05:53:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :subject:references:in-reply-to:content-type; bh=t+yMyYqV/jlS+wnlZ2Tzm1Oo2zPT5dxij3TuZQP8rDQ=; b=A6X3SvSLMtHOyAy+lY3cOx+IuGuEnjkEY0i9T2nCZrRrN7917Zmh53ckKGFNtfAkWT tUrBLzZzrJLnmp93Qjs8emGL0eBBrY5nAmgzDGxJEbsc11/W35bJQPfnv6YxbvJL6rct 94IwQ7Lbt2O8I36ogFqz3T8FNBGBykFEMZMv10lZfQFLcVVGFrbBZyKwLroj1vbyVZI+ eeeo4eM7ta0c59vLkAzkcSYJF/ziZs5nGWaJ7Y3b7bW5ktgxijJDOr917f/J/q4+uV21 gs29IuaAbi/nLcTFi3utZbHakzGRq5n/vNHMVw2aREhbQdx3r+URRwuSgfnRV6hIJGq4 1q8A== X-Gm-Message-State: ALoCoQmejxChw3aSnSBY/OEs/B/tWjm9nZg/P2pmn62UAS7Vjc2w76dQUyPmb5+wHLcFTAqhMbkN X-Received: by 10.194.6.70 with SMTP id y6mr56300690wjy.97.1425905619213; Mon, 09 Mar 2015 05:53:39 -0700 (PDT) Received: from [10.0.0.2] ([109.65.117.109]) by mx.google.com with ESMTPSA id hj10sm28154083wjc.48.2015.03.09.05.53.38 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 09 Mar 2015 05:53:38 -0700 (PDT) Message-ID: <54FD97D1.5010500@cloudius-systems.com> Date: Mon, 09 Mar 2015 14:53:37 +0200 From: Vlad Zolotarov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: "Ananyev, Konstantin" , "dev@dpdk.org" References: <1425896433-12452-1-git-send-email-vladz@cloudius-systems.com> <1425896433-12452-2-git-send-email-vladz@cloudius-systems.com> <2601191342CEEE43887BDE71AB977258213F4B37@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB977258213F4B37@irsmsx105.ger.corp.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups 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: Mon, 09 Mar 2015 12:53:39 -0000 On 03/09/15 12:49, Ananyev, Konstantin wrote: > >> -----Original Message----- >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Vlad Zolotarov >> Sent: Monday, March 09, 2015 10:21 AM >> To: dev@dpdk.org >> Subject: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups >> >> - Removed the not needed casting. >> - ixgbe_dev_rx_init(): shorten the lines by defining a local alias variable to access >> &dev->data->dev_conf.rxmode. >> >> Signed-off-by: Vlad Zolotarov >> --- >> lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 27 ++++++++++++--------------- >> 1 file changed, 12 insertions(+), 15 deletions(-) >> >> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c >> index 72c65df..609b5fd 100644 >> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c >> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c >> @@ -1032,8 +1032,7 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq) >> int diag, i; >> >> /* allocate buffers in bulk directly into the S/W ring */ >> - alloc_idx = (uint16_t)(rxq->rx_free_trigger - >> - (rxq->rx_free_thresh - 1)); >> + alloc_idx = rxq->rx_free_trigger - (rxq->rx_free_thresh - 1); > I think all these extra casts came in to keep icc 12.* compiling without warnings. > I am agree that they are unnecessary. > Though if we still have to support icc 12.* we either need to keep them, or find > some other way to keep it happy. Fix icc maybe? I'm sorry, but what do I miss here? Both alloc_idx, rxq->rx_free_trigger and rxq->rx_free_thresh are uint16_t So, according to C standard the result is also uint16_t thus no casting is needed: the result of a subtraction generating a negative number in an unsigned type is well-defined: 1. [...] A computation involving unsigned operands can never overflow, because a result that cannot be represented by the resulting unsigned integer type is reduced modulo the number that is one greater than the largest value that can be represented by the resulting type. (ISO/IEC 9899:1999 (E) §6.2.5/9) Could u, pls., be more specific and send here the error generated by icc after my patches? thanks, vlad > Konstantin > >> rxep = &rxq->sw_ring[alloc_idx]; >> diag = rte_mempool_get_bulk(rxq->mb_pool, (void *)rxep, >> rxq->rx_free_thresh); >> @@ -1061,10 +1060,9 @@ ixgbe_rx_alloc_bufs(struct igb_rx_queue *rxq) >> IXGBE_PCI_REG_WRITE(rxq->rdt_reg_addr, rxq->rx_free_trigger); >> >> /* update state of internal queue structure */ >> - rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_trigger + >> - rxq->rx_free_thresh); >> + rxq->rx_free_trigger = rxq->rx_free_trigger + rxq->rx_free_thresh; >> if (rxq->rx_free_trigger >= rxq->nb_rx_desc) >> - rxq->rx_free_trigger = (uint16_t)(rxq->rx_free_thresh - 1); >> + rxq->rx_free_trigger = rxq->rx_free_thresh - 1; >> >> /* no errors */ >> return 0; >> @@ -3560,6 +3558,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> uint32_t rxcsum; >> uint16_t buf_size; >> uint16_t i; >> + struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode; >> >> PMD_INIT_FUNC_TRACE(); >> hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); >> @@ -3582,7 +3581,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> * Configure CRC stripping, if any. >> */ >> hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0); >> - if (dev->data->dev_conf.rxmode.hw_strip_crc) >> + if (rx_conf->hw_strip_crc) >> hlreg0 |= IXGBE_HLREG0_RXCRCSTRP; >> else >> hlreg0 &= ~IXGBE_HLREG0_RXCRCSTRP; >> @@ -3590,11 +3589,11 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> /* >> * Configure jumbo frame support, if any. >> */ >> - if (dev->data->dev_conf.rxmode.jumbo_frame == 1) { >> + if (rx_conf->jumbo_frame == 1) { >> hlreg0 |= IXGBE_HLREG0_JUMBOEN; >> maxfrs = IXGBE_READ_REG(hw, IXGBE_MAXFRS); >> maxfrs &= 0x0000FFFF; >> - maxfrs |= (dev->data->dev_conf.rxmode.max_rx_pkt_len << 16); >> + maxfrs |= (rx_conf->max_rx_pkt_len << 16); >> IXGBE_WRITE_REG(hw, IXGBE_MAXFRS, maxfrs); >> } else >> hlreg0 &= ~IXGBE_HLREG0_JUMBOEN; >> @@ -3618,9 +3617,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> * Reset crc_len in case it was changed after queue setup by a >> * call to configure. >> */ >> - rxq->crc_len = (uint8_t) >> - ((dev->data->dev_conf.rxmode.hw_strip_crc) ? 0 : >> - ETHER_CRC_LEN); >> + rxq->crc_len = rx_conf->hw_strip_crc ? 0 : ETHER_CRC_LEN; >> >> /* Setup the Base and Length of the Rx Descriptor Rings */ >> bus_addr = rxq->rx_ring_phys_addr; >> @@ -3638,7 +3635,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> /* >> * Configure Header Split >> */ >> - if (dev->data->dev_conf.rxmode.header_split) { >> + if (rx_conf->header_split) { >> if (hw->mac.type == ixgbe_mac_82599EB) { >> /* Must setup the PSRTYPE register */ >> uint32_t psrtype; >> @@ -3648,7 +3645,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> IXGBE_PSRTYPE_IPV6HDR; >> IXGBE_WRITE_REG(hw, IXGBE_PSRTYPE(rxq->reg_idx), psrtype); >> } >> - srrctl = ((dev->data->dev_conf.rxmode.split_hdr_size << >> + srrctl = ((rx_conf->split_hdr_size << >> IXGBE_SRRCTL_BSIZEHDRSIZE_SHIFT) & >> IXGBE_SRRCTL_BSIZEHDR_MASK); >> srrctl |= IXGBE_SRRCTL_DESCTYPE_HDR_SPLIT_ALWAYS; >> @@ -3699,7 +3696,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> */ >> rxcsum = IXGBE_READ_REG(hw, IXGBE_RXCSUM); >> rxcsum |= IXGBE_RXCSUM_PCSD; >> - if (dev->data->dev_conf.rxmode.hw_ip_checksum) >> + if (rx_conf->hw_ip_checksum) >> rxcsum |= IXGBE_RXCSUM_IPPCSE; >> else >> rxcsum &= ~IXGBE_RXCSUM_IPPCSE; >> @@ -3709,7 +3706,7 @@ ixgbe_dev_rx_init(struct rte_eth_dev *dev) >> if (hw->mac.type == ixgbe_mac_82599EB || >> hw->mac.type == ixgbe_mac_X540) { >> rdrxctl = IXGBE_READ_REG(hw, IXGBE_RDRXCTL); >> - if (dev->data->dev_conf.rxmode.hw_strip_crc) >> + if (rx_conf->hw_strip_crc) >> rdrxctl |= IXGBE_RDRXCTL_CRCSTRIP; >> else >> rdrxctl &= ~IXGBE_RDRXCTL_CRCSTRIP; >> -- >> 2.1.0