DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: dev@dpdk.org
Subject: [dpdk-dev] [PATCH] igb/ixgbe: fix index overflow when resetting 4096 queue rings
Date: Fri, 15 Nov 2013 14:19:08 +0100	[thread overview]
Message-ID: <1384521548-7729-1-git-send-email-thomas.monjalon@6wind.com> (raw)

Rings are resetted with a loop because memset cannot be used without
issuing a warning about volatile casting.
The index of the loop was a 16-bit variable which is is sufficient for
ring entries number but not for the byte size of the whole ring.
The overflow happens when rings are configured for 4096 entries
(descriptor size is 16 bytes). The result is an endless loop.

It is fixed by indexing ring entries and resetting all bytes of the entry
with a simple assignment.
The descriptor initializer is zeroed thanks to its static declaration.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 lib/librte_pmd_e1000/igb_rxtx.c   |   14 ++++++--------
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c |   10 ++++++----
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/lib/librte_pmd_e1000/igb_rxtx.c b/lib/librte_pmd_e1000/igb_rxtx.c
index a09b774..f785d9f 100644
--- a/lib/librte_pmd_e1000/igb_rxtx.c
+++ b/lib/librte_pmd_e1000/igb_rxtx.c
@@ -1134,16 +1134,15 @@ igb_reset_tx_queue_stat(struct igb_tx_queue *txq)
 static void
 igb_reset_tx_queue(struct igb_tx_queue *txq, struct rte_eth_dev *dev)
 {
+	static const union e1000_adv_tx_desc zeroed_desc;
 	struct igb_tx_entry *txe = txq->sw_ring;
-	uint32_t size;
 	uint16_t i, prev;
 	struct e1000_hw *hw;
 
 	hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	size = sizeof(union e1000_adv_tx_desc) * txq->nb_tx_desc;
 	/* Zero out HW ring memory */
-	for (i = 0; i < size; i++) {
-		((volatile char *)txq->tx_ring)[i] = 0;
+	for (i = 0; i < txq->nb_tx_desc; i++) {
+		txq->tx_ring[i] = zeroed_desc;
 	}
 
 	/* Initialize ring entries */
@@ -1297,13 +1296,12 @@ eth_igb_rx_queue_release(void *rxq)
 static void
 igb_reset_rx_queue(struct igb_rx_queue *rxq)
 {
-	unsigned size;
+	static const union e1000_adv_rx_desc zeroed_desc;
 	unsigned i;
 
 	/* Zero out HW ring memory */
-	size = sizeof(union e1000_adv_rx_desc) * rxq->nb_rx_desc;
-	for (i = 0; i < size; i++) {
-		((volatile char *)rxq->rx_ring)[i] = 0;
+	for (i = 0; i < rxq->nb_rx_desc; i++) {
+		rxq->rx_ring[i] = zeroed_desc;
 	}
 
 	rxq->rx_tail = 0;
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 39d794d..0f7be95 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -1799,12 +1799,13 @@ ixgbe_dev_tx_queue_release(void *txq)
 static void
 ixgbe_reset_tx_queue(struct igb_tx_queue *txq)
 {
+	static const union ixgbe_adv_tx_desc zeroed_desc;
 	struct igb_tx_entry *txe = txq->sw_ring;
 	uint16_t prev, i;
 
 	/* Zero out HW ring memory */
-	for (i = 0; i < sizeof(union ixgbe_adv_tx_desc) * txq->nb_tx_desc; i++) {
-		((volatile char *)txq->tx_ring)[i] = 0;
+	for (i = 0; i < txq->nb_tx_desc; i++) {
+		txq->tx_ring[i] = zeroed_desc;
 	}
 
 	/* Initialize SW ring entries */
@@ -2093,6 +2094,7 @@ check_rx_burst_bulk_alloc_preconditions(__rte_unused struct igb_rx_queue *rxq)
 static void
 ixgbe_reset_rx_queue(struct igb_rx_queue *rxq)
 {
+	static const union ixgbe_adv_rx_desc zeroed_desc;
 	unsigned i;
 	uint16_t len;
 
@@ -2120,8 +2122,8 @@ ixgbe_reset_rx_queue(struct igb_rx_queue *rxq)
 	 * the H/W ring so look-ahead logic in Rx Burst bulk alloc function
 	 * reads extra memory as zeros.
 	 */
-	for (i = 0; i < len * sizeof(union ixgbe_adv_rx_desc); i++) {
-		((volatile char *)rxq->rx_ring)[i] = 0;
+	for (i = 0; i < len; i++) {
+		rxq->rx_ring[i] = zeroed_desc;
 	}
 
 #ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC
-- 
1.7.10.4

             reply	other threads:[~2013-11-15 13:18 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-15 13:19 Thomas Monjalon [this message]
2013-11-15 13:26 ` Ivan Boule
2013-11-15 15:15   ` Thomas Monjalon

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=1384521548-7729-1-git-send-email-thomas.monjalon@6wind.com \
    --to=thomas.monjalon@6wind.com \
    --cc=dev@dpdk.org \
    /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).