From: Vlad Zolotarov <vladz@cloudius-systems.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups
Date: Mon, 09 Mar 2015 14:53:37 +0200 [thread overview]
Message-ID: <54FD97D1.5010500@cloudius-systems.com> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB977258213F4B37@irsmsx105.ger.corp.intel.com>
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 <vladz@cloudius-systems.com>
>> ---
>> 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
next prev parent reply other threads:[~2015-03-09 12:53 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-09 10:20 [dpdk-dev] [PATCH v5 0/3]This series adds the missing flow for enabling the LRO in the ethdev and Vlad Zolotarov
2015-03-09 10:20 ` [dpdk-dev] [PATCH v5 1/3] ixgbe: Cleanups Vlad Zolotarov
2015-03-09 10:49 ` Ananyev, Konstantin
2015-03-09 11:09 ` Pawel Wodkowski
2015-03-09 11:29 ` Ananyev, Konstantin
2015-03-09 15:57 ` Vlad Zolotarov
2015-03-09 16:39 ` Ananyev, Konstantin
2015-03-09 17:13 ` Vladislav Zolotarov
2015-03-09 18:00 ` Mcnamara, John
2015-03-09 18:21 ` Vladislav Zolotarov
2015-03-09 18:21 ` Vladislav Zolotarov
[not found] ` <2601191342CEEE43887BDE71AB977258213F4F13@irsmsx105.ger.corp.intel.com>
2015-03-09 19:13 ` Ananyev, Konstantin
2015-03-09 19:32 ` Vlad Zolotarov
2015-03-09 19:36 ` Ananyev, Konstantin
2015-03-09 12:53 ` Vlad Zolotarov [this message]
2015-03-09 10:20 ` [dpdk-dev] [PATCH v5 2/3] ixgbe: Code refactoring Vlad Zolotarov
2015-03-09 10:20 ` [dpdk-dev] [PATCH v5 3/3] ixgbe: Add LRO support Vlad Zolotarov
2015-03-09 10:46 ` [dpdk-dev] [PATCH v5 0/3]This series adds the missing flow for enabling the LRO in the ethdev and Vlad Zolotarov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54FD97D1.5010500@cloudius-systems.com \
--to=vladz@cloudius-systems.com \
--cc=dev@dpdk.org \
--cc=konstantin.ananyev@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).