[dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian cpu

[dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian cpu

[xuelin.shi]

From: Xuelin Shi <xuelin.shi@freescale.com>

enforce rules of the cpu and ixgbe exchange data.
1. cpu use data owned by ixgbe must use rte_le_to_cpu_xx(...)
2. cpu fill data to ixgbe must use rte_cpu_to_le_xx(...)
3. checking pci status with converted constant.

Signed-off-by: Xuelin Shi <xuelin.shi@freescale.com>
---
change for v3:
   check pci status with converted constant to avoid performance penalty.
   remove tmp variable.

 lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 89 ++++++++++++++++++++++++---------------
 1 file changed, 56 insertions(+), 33 deletions(-)

diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 9da2c7e..6e508ec 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -129,7 +129,7 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq)
 
 	/* check DD bit on threshold descriptor */
 	status = txq->tx_ring[txq->tx_next_dd].wb.status;
-	if (! (status & IXGBE_ADVTXD_STAT_DD))
+	if (!(status & rte_cpu_to_le_32(IXGBE_ADVTXD_STAT_DD)))
 		return 0;
 
 	/*
@@ -174,11 +174,14 @@ tx4(volatile union ixgbe_adv_tx_desc *txdp, struct rte_mbuf **pkts)
 		pkt_len = (*pkts)->data_len;
 
 		/* write data to descriptor */
-		txdp->read.buffer_addr = buf_dma_addr;
+		txdp->read.buffer_addr = rte_cpu_to_le_64(buf_dma_addr);
+
 		txdp->read.cmd_type_len =
-				((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
+			rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
+
 		txdp->read.olinfo_status =
-				(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
+			rte_cpu_to_le_32(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
+
 		rte_prefetch0(&(*pkts)->pool);
 	}
 }
@@ -194,11 +197,14 @@ tx1(volatile union ixgbe_adv_tx_desc *txdp, struct rte_mbuf **pkts)
 	pkt_len = (*pkts)->data_len;
 
 	/* write data to descriptor */
-	txdp->read.buffer_addr = buf_dma_addr;
+	txdp->read.buffer_addr = rte_cpu_to_le_64(buf_dma_addr);
+
 	txdp->read.cmd_type_len =
-			((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
+			rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
+
 	txdp->read.olinfo_status =
-			(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
+			rte_cpu_to_le_32(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
+
 	rte_prefetch0(&(*pkts)->pool);
 }
 
@@ -285,7 +291,7 @@ tx_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		 * a divisor of the ring size
 		 */
 		tx_r[txq->tx_next_rs].read.cmd_type_len |=
-			rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
+				rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
 		txq->tx_next_rs = (uint16_t)(txq->tx_rs_thresh - 1);
 
 		txq->tx_tail = 0;
@@ -304,7 +310,7 @@ tx_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	 */
 	if (txq->tx_tail > txq->tx_next_rs) {
 		tx_r[txq->tx_next_rs].read.cmd_type_len |=
-			rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
+				rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
 		txq->tx_next_rs = (uint16_t)(txq->tx_next_rs +
 						txq->tx_rs_thresh);
 		if (txq->tx_next_rs >= txq->nb_tx_desc)
@@ -505,6 +511,7 @@ ixgbe_xmit_cleanup(struct ixgbe_tx_queue *txq)
 	uint16_t nb_tx_desc = txq->nb_tx_desc;
 	uint16_t desc_to_clean_to;
 	uint16_t nb_tx_to_clean;
+	uint32_t stat;
 
 	/* Determine the last descriptor needing to be cleaned */
 	desc_to_clean_to = (uint16_t)(last_desc_cleaned + txq->tx_rs_thresh);
@@ -513,7 +520,9 @@ ixgbe_xmit_cleanup(struct ixgbe_tx_queue *txq)
 
 	/* Check to make sure the last descriptor to clean is done */
 	desc_to_clean_to = sw_ring[desc_to_clean_to].last_id;
-	if (! (txr[desc_to_clean_to].wb.status & IXGBE_TXD_STAT_DD))
+
+	stat = txr[desc_to_clean_to].wb.status;
+	if (!(stat & rte_cpu_to_le_32(IXGBE_TXD_STAT_DD)))
 	{
 		PMD_TX_FREE_LOG(DEBUG,
 				"TX descriptor %4u is not done"
@@ -801,12 +810,14 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			 */
 			slen = m_seg->data_len;
 			buf_dma_addr = RTE_MBUF_DATA_DMA_ADDR(m_seg);
+
 			txd->read.buffer_addr =
-				rte_cpu_to_le_64(buf_dma_addr);
+					rte_cpu_to_le_64(buf_dma_addr);
 			txd->read.cmd_type_len =
-				rte_cpu_to_le_32(cmd_type_len | slen);
+					rte_cpu_to_le_32(cmd_type_len | slen);
 			txd->read.olinfo_status =
-				rte_cpu_to_le_32(olinfo_status);
+					rte_cpu_to_le_32(olinfo_status);
+
 			txe->last_id = tx_last;
 			tx_id = txe->next_id;
 			txe = txn;
@@ -946,14 +957,16 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
 	uint64_t pkt_flags;
 	int s[LOOK_AHEAD], nb_dd;
 	int i, j, nb_rx = 0;
+	uint32_t stat;
 
 
 	/* get references to current descriptor and S/W ring entry */
 	rxdp = &rxq->rx_ring[rxq->rx_tail];
 	rxep = &rxq->sw_ring[rxq->rx_tail];
 
+	stat = rxdp->wb.upper.status_error;
 	/* check to make sure there is at least 1 packet to receive */
-	if (! (rxdp->wb.upper.status_error & IXGBE_RXDADV_STAT_DD))
+	if (!(stat & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
 		return 0;
 
 	/*
@@ -965,7 +978,7 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
 	{
 		/* Read desc statuses backwards to avoid race condition */
 		for (j = LOOK_AHEAD-1; j >= 0; --j)
-			s[j] = rxdp[j].wb.upper.status_error;
+			s[j] = rte_le_to_cpu_32(rxdp[j].wb.upper.status_error);
 
 		/* Compute how many status bits were set */
 		nb_dd = 0;
@@ -977,27 +990,32 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
 		/* Translate descriptor info to mbuf format */
 		for (j = 0; j < nb_dd; ++j) {
 			mb = rxep[j].mbuf;
-			pkt_len = (uint16_t)(rxdp[j].wb.upper.length - rxq->crc_len);
+			pkt_len = rte_le_to_cpu_16(rxdp[j].wb.upper.length) -
+				  rxq->crc_len;
+
 			mb->data_len = pkt_len;
 			mb->pkt_len = pkt_len;
-			mb->vlan_tci = rxdp[j].wb.upper.vlan;
 			mb->vlan_tci = rte_le_to_cpu_16(rxdp[j].wb.upper.vlan);
 
 			/* convert descriptor fields to rte mbuf flags */
 			pkt_flags  = rx_desc_hlen_type_rss_to_pkt_flags(
-					rxdp[j].wb.lower.lo_dword.data);
+			    rte_le_to_cpu_32(rxdp[j].wb.lower.lo_dword.data));
+
 			/* reuse status field from scan list */
 			pkt_flags |= rx_desc_status_to_pkt_flags(s[j]);
 			pkt_flags |= rx_desc_error_to_pkt_flags(s[j]);
 			mb->ol_flags = pkt_flags;
 
 			if (likely(pkt_flags & PKT_RX_RSS_HASH))
-				mb->hash.rss = rxdp[j].wb.lower.hi_dword.rss;
+				mb->hash.rss = rte_le_to_cpu_32(
+				    rxdp[j].wb.lower.hi_dword.rss);
 			else if (pkt_flags & PKT_RX_FDIR) {
-				mb->hash.fdir.hash =
-					(uint16_t)((rxdp[j].wb.lower.hi_dword.csum_ip.csum)
-						& IXGBE_ATR_HASH_MASK);
-				mb->hash.fdir.id = rxdp[j].wb.lower.hi_dword.csum_ip.ip_id;
+				mb->hash.fdir.hash = rte_le_to_cpu_16(
+				    rxdp[j].wb.lower.hi_dword.csum_ip.csum) &
+				    IXGBE_ATR_HASH_MASK;
+
+				mb->hash.fdir.id = rte_le_to_cpu_16(
+				    rxdp[j].wb.lower.hi_dword.csum_ip.ip_id);
 			}
 		}
 
@@ -1221,7 +1239,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		 */
 		rxdp = &rx_ring[rx_id];
 		staterr = rxdp->wb.upper.status_error;
-		if (! (staterr & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
+		if (!(staterr & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
 			break;
 		rxd = *rxdp;
 
@@ -1325,12 +1343,15 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		rxm->ol_flags = pkt_flags;
 
 		if (likely(pkt_flags & PKT_RX_RSS_HASH))
-			rxm->hash.rss = rxd.wb.lower.hi_dword.rss;
+			rxm->hash.rss = rte_le_to_cpu_32(
+						rxd.wb.lower.hi_dword.rss);
 		else if (pkt_flags & PKT_RX_FDIR) {
-			rxm->hash.fdir.hash =
-				(uint16_t)((rxd.wb.lower.hi_dword.csum_ip.csum)
-					   & IXGBE_ATR_HASH_MASK);
-			rxm->hash.fdir.id = rxd.wb.lower.hi_dword.csum_ip.ip_id;
+			rxm->hash.fdir.hash = rte_le_to_cpu_16(
+					rxd.wb.lower.hi_dword.csum_ip.csum) &
+					IXGBE_ATR_HASH_MASK;
+
+			rxm->hash.fdir.id = rte_le_to_cpu_16(
+					rxd.wb.lower.hi_dword.csum_ip.ip_id);
 		}
 		/*
 		 * Store the mbuf address into the next entry of the array
@@ -1413,7 +1434,7 @@ ixgbe_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 		 */
 		rxdp = &rx_ring[rx_id];
 		staterr = rxdp->wb.upper.status_error;
-		if (! (staterr & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
+		if (!(staterr & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
 			break;
 		rxd = *rxdp;
 
@@ -1742,7 +1763,7 @@ ixgbe_reset_tx_queue(struct ixgbe_tx_queue *txq)
 	prev = (uint16_t) (txq->nb_tx_desc - 1);
 	for (i = 0; i < txq->nb_tx_desc; i++) {
 		volatile union ixgbe_adv_tx_desc *txd = &txq->tx_ring[i];
-		txd->wb.status = IXGBE_TXD_STAT_DD;
+		txd->wb.status = rte_cpu_to_le_32(IXGBE_TXD_STAT_DD);
 		txe[i].mbuf = NULL;
 		txe[i].last_id = i;
 		txe[prev].next_id = i;
@@ -2293,7 +2314,8 @@ ixgbe_dev_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
 	rxdp = &(rxq->rx_ring[rxq->rx_tail]);
 
 	while ((desc < rxq->nb_rx_desc) &&
-		(rxdp->wb.upper.status_error & IXGBE_RXDADV_STAT_DD)) {
+		(rte_le_to_cpu_32(rxdp->wb.upper.status_error) &
+				  IXGBE_RXDADV_STAT_DD)) {
 		desc += IXGBE_RXQ_SCAN_INTERVAL;
 		rxdp += IXGBE_RXQ_SCAN_INTERVAL;
 		if (rxq->rx_tail + desc >= rxq->nb_rx_desc)
@@ -2318,7 +2340,8 @@ ixgbe_dev_rx_descriptor_done(void *rx_queue, uint16_t offset)
 		desc -= rxq->nb_rx_desc;
 
 	rxdp = &rxq->rx_ring[desc];
-	return !!(rxdp->wb.upper.status_error & IXGBE_RXDADV_STAT_DD);
+	return !!(rxdp->wb.upper.status_error &
+			rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD));
 }
 
 void
-- 
1.9.1

Re: [dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian cpu

[Butler, Siobhan A]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of
> xuelin.shi@freescale.com
> Sent: Tuesday, March 31, 2015 8:26 AM
> To: Ananyev, Konstantin
> Cc: dev@dpdk.org; Xuelin Shi
> Subject: [dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian cpu
> 
> From: Xuelin Shi <xuelin.shi@freescale.com>
> 
> enforce rules of the cpu and ixgbe exchange data.
> 1. cpu use data owned by ixgbe must use rte_le_to_cpu_xx(...) 2. cpu fill
> data to ixgbe must use rte_cpu_to_le_xx(...) 3. checking pci status with
> converted constant.
> 
> Signed-off-by: Xuelin Shi <xuelin.shi@freescale.com>
> ---
> change for v3:
>    check pci status with converted constant to avoid performance penalty.
>    remove tmp variable.
> 
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 89 ++++++++++++++++++++++++-------
> --------
>  1 file changed, 56 insertions(+), 33 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index 9da2c7e..6e508ec 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -129,7 +129,7 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq)
> 
>  	/* check DD bit on threshold descriptor */
>  	status = txq->tx_ring[txq->tx_next_dd].wb.status;
> -	if (! (status & IXGBE_ADVTXD_STAT_DD))
> +	if (!(status & rte_cpu_to_le_32(IXGBE_ADVTXD_STAT_DD)))
>  		return 0;
> 
>  	/*
> @@ -174,11 +174,14 @@ tx4(volatile union ixgbe_adv_tx_desc *txdp, struct
> rte_mbuf **pkts)
>  		pkt_len = (*pkts)->data_len;
> 
>  		/* write data to descriptor */
> -		txdp->read.buffer_addr = buf_dma_addr;
> +		txdp->read.buffer_addr =
> rte_cpu_to_le_64(buf_dma_addr);
> +
>  		txdp->read.cmd_type_len =
> -				((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> +			rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS |
> pkt_len);
> +
>  		txdp->read.olinfo_status =
> -				(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> +			rte_cpu_to_le_32(pkt_len <<
> IXGBE_ADVTXD_PAYLEN_SHIFT);
> +
>  		rte_prefetch0(&(*pkts)->pool);
>  	}
>  }
> @@ -194,11 +197,14 @@ tx1(volatile union ixgbe_adv_tx_desc *txdp, struct
> rte_mbuf **pkts)
>  	pkt_len = (*pkts)->data_len;
> 
>  	/* write data to descriptor */
> -	txdp->read.buffer_addr = buf_dma_addr;
> +	txdp->read.buffer_addr = rte_cpu_to_le_64(buf_dma_addr);
> +
>  	txdp->read.cmd_type_len =
> -			((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> +			rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS |
> pkt_len);
> +
>  	txdp->read.olinfo_status =
> -			(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> +			rte_cpu_to_le_32(pkt_len <<
> IXGBE_ADVTXD_PAYLEN_SHIFT);
> +
>  	rte_prefetch0(&(*pkts)->pool);
>  }
> 
> @@ -285,7 +291,7 @@ tx_xmit_pkts(void *tx_queue, struct rte_mbuf
> **tx_pkts,
>  		 * a divisor of the ring size
>  		 */
>  		tx_r[txq->tx_next_rs].read.cmd_type_len |=
> -			rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> +
> 	rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
>  		txq->tx_next_rs = (uint16_t)(txq->tx_rs_thresh - 1);
> 
>  		txq->tx_tail = 0;
> @@ -304,7 +310,7 @@ tx_xmit_pkts(void *tx_queue, struct rte_mbuf
> **tx_pkts,
>  	 */
>  	if (txq->tx_tail > txq->tx_next_rs) {
>  		tx_r[txq->tx_next_rs].read.cmd_type_len |=
> -			rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> +
> 	rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
>  		txq->tx_next_rs = (uint16_t)(txq->tx_next_rs +
>  						txq->tx_rs_thresh);
>  		if (txq->tx_next_rs >= txq->nb_tx_desc) @@ -505,6 +511,7
> @@ ixgbe_xmit_cleanup(struct ixgbe_tx_queue *txq)
>  	uint16_t nb_tx_desc = txq->nb_tx_desc;
>  	uint16_t desc_to_clean_to;
>  	uint16_t nb_tx_to_clean;
> +	uint32_t stat;
> 
>  	/* Determine the last descriptor needing to be cleaned */
>  	desc_to_clean_to = (uint16_t)(last_desc_cleaned + txq-
> >tx_rs_thresh); @@ -513,7 +520,9 @@ ixgbe_xmit_cleanup(struct
> ixgbe_tx_queue *txq)
> 
>  	/* Check to make sure the last descriptor to clean is done */
>  	desc_to_clean_to = sw_ring[desc_to_clean_to].last_id;
> -	if (! (txr[desc_to_clean_to].wb.status & IXGBE_TXD_STAT_DD))
> +
> +	stat = txr[desc_to_clean_to].wb.status;
> +	if (!(stat & rte_cpu_to_le_32(IXGBE_TXD_STAT_DD)))
>  	{
>  		PMD_TX_FREE_LOG(DEBUG,
>  				"TX descriptor %4u is not done"
> @@ -801,12 +810,14 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf
> **tx_pkts,
>  			 */
>  			slen = m_seg->data_len;
>  			buf_dma_addr =
> RTE_MBUF_DATA_DMA_ADDR(m_seg);
> +
>  			txd->read.buffer_addr =
> -				rte_cpu_to_le_64(buf_dma_addr);
> +					rte_cpu_to_le_64(buf_dma_addr);
>  			txd->read.cmd_type_len =
> -				rte_cpu_to_le_32(cmd_type_len | slen);
> +					rte_cpu_to_le_32(cmd_type_len |
> slen);
>  			txd->read.olinfo_status =
> -				rte_cpu_to_le_32(olinfo_status);
> +					rte_cpu_to_le_32(olinfo_status);
> +
>  			txe->last_id = tx_last;
>  			tx_id = txe->next_id;
>  			txe = txn;
> @@ -946,14 +957,16 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue
> *rxq)
>  	uint64_t pkt_flags;
>  	int s[LOOK_AHEAD], nb_dd;
>  	int i, j, nb_rx = 0;
> +	uint32_t stat;
> 
> 
>  	/* get references to current descriptor and S/W ring entry */
>  	rxdp = &rxq->rx_ring[rxq->rx_tail];
>  	rxep = &rxq->sw_ring[rxq->rx_tail];
> 
> +	stat = rxdp->wb.upper.status_error;
>  	/* check to make sure there is at least 1 packet to receive */
> -	if (! (rxdp->wb.upper.status_error & IXGBE_RXDADV_STAT_DD))
> +	if (!(stat & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
>  		return 0;
> 
>  	/*
> @@ -965,7 +978,7 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
>  	{
>  		/* Read desc statuses backwards to avoid race condition */
>  		for (j = LOOK_AHEAD-1; j >= 0; --j)
> -			s[j] = rxdp[j].wb.upper.status_error;
> +			s[j] =
> rte_le_to_cpu_32(rxdp[j].wb.upper.status_error);
> 
>  		/* Compute how many status bits were set */
>  		nb_dd = 0;
> @@ -977,27 +990,32 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue
> *rxq)
>  		/* Translate descriptor info to mbuf format */
>  		for (j = 0; j < nb_dd; ++j) {
>  			mb = rxep[j].mbuf;
> -			pkt_len = (uint16_t)(rxdp[j].wb.upper.length - rxq-
> >crc_len);
> +			pkt_len =
> rte_le_to_cpu_16(rxdp[j].wb.upper.length) -
> +				  rxq->crc_len;
> +
>  			mb->data_len = pkt_len;
>  			mb->pkt_len = pkt_len;
> -			mb->vlan_tci = rxdp[j].wb.upper.vlan;
>  			mb->vlan_tci =
> rte_le_to_cpu_16(rxdp[j].wb.upper.vlan);
> 
>  			/* convert descriptor fields to rte mbuf flags */
>  			pkt_flags  = rx_desc_hlen_type_rss_to_pkt_flags(
> -					rxdp[j].wb.lower.lo_dword.data);
> +
> rte_le_to_cpu_32(rxdp[j].wb.lower.lo_dword.data));
> +
>  			/* reuse status field from scan list */
>  			pkt_flags |= rx_desc_status_to_pkt_flags(s[j]);
>  			pkt_flags |= rx_desc_error_to_pkt_flags(s[j]);
>  			mb->ol_flags = pkt_flags;
> 
>  			if (likely(pkt_flags & PKT_RX_RSS_HASH))
> -				mb->hash.rss =
> rxdp[j].wb.lower.hi_dword.rss;
> +				mb->hash.rss = rte_le_to_cpu_32(
> +				    rxdp[j].wb.lower.hi_dword.rss);
>  			else if (pkt_flags & PKT_RX_FDIR) {
> -				mb->hash.fdir.hash =
> -
> 	(uint16_t)((rxdp[j].wb.lower.hi_dword.csum_ip.csum)
> -						& IXGBE_ATR_HASH_MASK);
> -				mb->hash.fdir.id =
> rxdp[j].wb.lower.hi_dword.csum_ip.ip_id;
> +				mb->hash.fdir.hash = rte_le_to_cpu_16(
> +				    rxdp[j].wb.lower.hi_dword.csum_ip.csum)
> &
> +				    IXGBE_ATR_HASH_MASK;
> +
> +				mb->hash.fdir.id = rte_le_to_cpu_16(
> +				    rxdp[j].wb.lower.hi_dword.csum_ip.ip_id);
>  			}
>  		}
> 
> @@ -1221,7 +1239,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf
> **rx_pkts,
>  		 */
>  		rxdp = &rx_ring[rx_id];
>  		staterr = rxdp->wb.upper.status_error;
> -		if (! (staterr &
> rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
> +		if (!(staterr & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
>  			break;
>  		rxd = *rxdp;
> 
> @@ -1325,12 +1343,15 @@ ixgbe_recv_pkts(void *rx_queue, struct
> rte_mbuf **rx_pkts,
>  		rxm->ol_flags = pkt_flags;
> 
>  		if (likely(pkt_flags & PKT_RX_RSS_HASH))
> -			rxm->hash.rss = rxd.wb.lower.hi_dword.rss;
> +			rxm->hash.rss = rte_le_to_cpu_32(
> +						rxd.wb.lower.hi_dword.rss);
>  		else if (pkt_flags & PKT_RX_FDIR) {
> -			rxm->hash.fdir.hash =
> -
> 	(uint16_t)((rxd.wb.lower.hi_dword.csum_ip.csum)
> -					   & IXGBE_ATR_HASH_MASK);
> -			rxm->hash.fdir.id =
> rxd.wb.lower.hi_dword.csum_ip.ip_id;
> +			rxm->hash.fdir.hash = rte_le_to_cpu_16(
> +
> 	rxd.wb.lower.hi_dword.csum_ip.csum) &
> +					IXGBE_ATR_HASH_MASK;
> +
> +			rxm->hash.fdir.id = rte_le_to_cpu_16(
> +
> 	rxd.wb.lower.hi_dword.csum_ip.ip_id);
>  		}
>  		/*
>  		 * Store the mbuf address into the next entry of the array
> @@ -1413,7 +1434,7 @@ ixgbe_recv_scattered_pkts(void *rx_queue, struct
> rte_mbuf **rx_pkts,
>  		 */
>  		rxdp = &rx_ring[rx_id];
>  		staterr = rxdp->wb.upper.status_error;
> -		if (! (staterr &
> rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
> +		if (!(staterr & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
>  			break;
>  		rxd = *rxdp;
> 
> @@ -1742,7 +1763,7 @@ ixgbe_reset_tx_queue(struct ixgbe_tx_queue
> *txq)
>  	prev = (uint16_t) (txq->nb_tx_desc - 1);
>  	for (i = 0; i < txq->nb_tx_desc; i++) {
>  		volatile union ixgbe_adv_tx_desc *txd = &txq->tx_ring[i];
> -		txd->wb.status = IXGBE_TXD_STAT_DD;
> +		txd->wb.status = rte_cpu_to_le_32(IXGBE_TXD_STAT_DD);
>  		txe[i].mbuf = NULL;
>  		txe[i].last_id = i;
>  		txe[prev].next_id = i;
> @@ -2293,7 +2314,8 @@ ixgbe_dev_rx_queue_count(struct rte_eth_dev
> *dev, uint16_t rx_queue_id)
>  	rxdp = &(rxq->rx_ring[rxq->rx_tail]);
> 
>  	while ((desc < rxq->nb_rx_desc) &&
> -		(rxdp->wb.upper.status_error & IXGBE_RXDADV_STAT_DD))
> {
> +		(rte_le_to_cpu_32(rxdp->wb.upper.status_error) &
> +				  IXGBE_RXDADV_STAT_DD)) {
>  		desc += IXGBE_RXQ_SCAN_INTERVAL;
>  		rxdp += IXGBE_RXQ_SCAN_INTERVAL;
>  		if (rxq->rx_tail + desc >= rxq->nb_rx_desc) @@ -2318,7
> +2340,8 @@ ixgbe_dev_rx_descriptor_done(void *rx_queue, uint16_t
> offset)
>  		desc -= rxq->nb_rx_desc;
> 
>  	rxdp = &rxq->rx_ring[desc];
> -	return !!(rxdp->wb.upper.status_error &
> IXGBE_RXDADV_STAT_DD);
> +	return !!(rxdp->wb.upper.status_error &
> +			rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD));
>  }
> 
>  void
> --
> 1.9.1
Hi Xuelin,
I haven't had a chance to test the patches yet but if its ok with everyone can we defer on this until after the 2.0 package as it would be risky to make the change right before the release without full testing?
Thanks
Siobhan 
<siobhan.a.butler@intel.com>

Re: [dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian cpu

[Xuelin Shi]

Hi,

OK, I'm fine to defer it.

Thanks,
Xuelin Shi

> -----Original Message-----
> From: Butler, Siobhan A [mailto:siobhan.a.butler@intel.com]
> Sent: Friday, April 03, 2015 04:18
> To: Shi Xuelin-B29237; Ananyev, Konstantin
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian
> cpu
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of
> > xuelin.shi@freescale.com
> > Sent: Tuesday, March 31, 2015 8:26 AM
> > To: Ananyev, Konstantin
> > Cc: dev@dpdk.org; Xuelin Shi
> > Subject: [dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian
> > cpu
> >
> > From: Xuelin Shi <xuelin.shi@freescale.com>
> >
> > enforce rules of the cpu and ixgbe exchange data.
> > 1. cpu use data owned by ixgbe must use rte_le_to_cpu_xx(...) 2. cpu
> > fill data to ixgbe must use rte_cpu_to_le_xx(...) 3. checking pci
> > status with converted constant.
> >
> > Signed-off-by: Xuelin Shi <xuelin.shi@freescale.com>
> > ---
> > change for v3:
> >    check pci status with converted constant to avoid performance
> penalty.
> >    remove tmp variable.
> >
> >  lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 89
> > ++++++++++++++++++++++++-------
> > --------
> >  1 file changed, 56 insertions(+), 33 deletions(-)
> >
> > diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > index 9da2c7e..6e508ec 100644
> > --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> > @@ -129,7 +129,7 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq)
> >
> >  	/* check DD bit on threshold descriptor */
> >  	status = txq->tx_ring[txq->tx_next_dd].wb.status;
> > -	if (! (status & IXGBE_ADVTXD_STAT_DD))
> > +	if (!(status & rte_cpu_to_le_32(IXGBE_ADVTXD_STAT_DD)))
> >  		return 0;
> >
> >  	/*
> > @@ -174,11 +174,14 @@ tx4(volatile union ixgbe_adv_tx_desc *txdp,
> > struct rte_mbuf **pkts)
> >  		pkt_len = (*pkts)->data_len;
> >
> >  		/* write data to descriptor */
> > -		txdp->read.buffer_addr = buf_dma_addr;
> > +		txdp->read.buffer_addr =
> > rte_cpu_to_le_64(buf_dma_addr);
> > +
> >  		txdp->read.cmd_type_len =
> > -				((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> > +			rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS |
> > pkt_len);
> > +
> >  		txdp->read.olinfo_status =
> > -				(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> > +			rte_cpu_to_le_32(pkt_len <<
> > IXGBE_ADVTXD_PAYLEN_SHIFT);
> > +
> >  		rte_prefetch0(&(*pkts)->pool);
> >  	}
> >  }
> > @@ -194,11 +197,14 @@ tx1(volatile union ixgbe_adv_tx_desc *txdp,
> > struct rte_mbuf **pkts)
> >  	pkt_len = (*pkts)->data_len;
> >
> >  	/* write data to descriptor */
> > -	txdp->read.buffer_addr = buf_dma_addr;
> > +	txdp->read.buffer_addr = rte_cpu_to_le_64(buf_dma_addr);
> > +
> >  	txdp->read.cmd_type_len =
> > -			((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> > +			rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS |
> > pkt_len);
> > +
> >  	txdp->read.olinfo_status =
> > -			(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> > +			rte_cpu_to_le_32(pkt_len <<
> > IXGBE_ADVTXD_PAYLEN_SHIFT);
> > +
> >  	rte_prefetch0(&(*pkts)->pool);
> >  }
> >
> > @@ -285,7 +291,7 @@ tx_xmit_pkts(void *tx_queue, struct rte_mbuf
> > **tx_pkts,
> >  		 * a divisor of the ring size
> >  		 */
> >  		tx_r[txq->tx_next_rs].read.cmd_type_len |=
> > -			rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> > +
> > 	rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> >  		txq->tx_next_rs = (uint16_t)(txq->tx_rs_thresh - 1);
> >
> >  		txq->tx_tail = 0;
> > @@ -304,7 +310,7 @@ tx_xmit_pkts(void *tx_queue, struct rte_mbuf
> > **tx_pkts,
> >  	 */
> >  	if (txq->tx_tail > txq->tx_next_rs) {
> >  		tx_r[txq->tx_next_rs].read.cmd_type_len |=
> > -			rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> > +
> > 	rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> >  		txq->tx_next_rs = (uint16_t)(txq->tx_next_rs +
> >  						txq->tx_rs_thresh);
> >  		if (txq->tx_next_rs >= txq->nb_tx_desc) @@ -505,6 +511,7 @@
> > ixgbe_xmit_cleanup(struct ixgbe_tx_queue *txq)
> >  	uint16_t nb_tx_desc = txq->nb_tx_desc;
> >  	uint16_t desc_to_clean_to;
> >  	uint16_t nb_tx_to_clean;
> > +	uint32_t stat;
> >
> >  	/* Determine the last descriptor needing to be cleaned */
> >  	desc_to_clean_to = (uint16_t)(last_desc_cleaned + txq-
> > >tx_rs_thresh); @@ -513,7 +520,9 @@ ixgbe_xmit_cleanup(struct
> > ixgbe_tx_queue *txq)
> >
> >  	/* Check to make sure the last descriptor to clean is done */
> >  	desc_to_clean_to = sw_ring[desc_to_clean_to].last_id;
> > -	if (! (txr[desc_to_clean_to].wb.status & IXGBE_TXD_STAT_DD))
> > +
> > +	stat = txr[desc_to_clean_to].wb.status;
> > +	if (!(stat & rte_cpu_to_le_32(IXGBE_TXD_STAT_DD)))
> >  	{
> >  		PMD_TX_FREE_LOG(DEBUG,
> >  				"TX descriptor %4u is not done"
> > @@ -801,12 +810,14 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf
> > **tx_pkts,
> >  			 */
> >  			slen = m_seg->data_len;
> >  			buf_dma_addr =
> > RTE_MBUF_DATA_DMA_ADDR(m_seg);
> > +
> >  			txd->read.buffer_addr =
> > -				rte_cpu_to_le_64(buf_dma_addr);
> > +					rte_cpu_to_le_64(buf_dma_addr);
> >  			txd->read.cmd_type_len =
> > -				rte_cpu_to_le_32(cmd_type_len | slen);
> > +					rte_cpu_to_le_32(cmd_type_len |
> > slen);
> >  			txd->read.olinfo_status =
> > -				rte_cpu_to_le_32(olinfo_status);
> > +					rte_cpu_to_le_32(olinfo_status);
> > +
> >  			txe->last_id = tx_last;
> >  			tx_id = txe->next_id;
> >  			txe = txn;
> > @@ -946,14 +957,16 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue
> > *rxq)
> >  	uint64_t pkt_flags;
> >  	int s[LOOK_AHEAD], nb_dd;
> >  	int i, j, nb_rx = 0;
> > +	uint32_t stat;
> >
> >
> >  	/* get references to current descriptor and S/W ring entry */
> >  	rxdp = &rxq->rx_ring[rxq->rx_tail];
> >  	rxep = &rxq->sw_ring[rxq->rx_tail];
> >
> > +	stat = rxdp->wb.upper.status_error;
> >  	/* check to make sure there is at least 1 packet to receive */
> > -	if (! (rxdp->wb.upper.status_error & IXGBE_RXDADV_STAT_DD))
> > +	if (!(stat & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
> >  		return 0;
> >
> >  	/*
> > @@ -965,7 +978,7 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
> >  	{
> >  		/* Read desc statuses backwards to avoid race condition */
> >  		for (j = LOOK_AHEAD-1; j >= 0; --j)
> > -			s[j] = rxdp[j].wb.upper.status_error;
> > +			s[j] =
> > rte_le_to_cpu_32(rxdp[j].wb.upper.status_error);
> >
> >  		/* Compute how many status bits were set */
> >  		nb_dd = 0;
> > @@ -977,27 +990,32 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue
> > *rxq)
> >  		/* Translate descriptor info to mbuf format */
> >  		for (j = 0; j < nb_dd; ++j) {
> >  			mb = rxep[j].mbuf;
> > -			pkt_len = (uint16_t)(rxdp[j].wb.upper.length - rxq-
> > >crc_len);
> > +			pkt_len =
> > rte_le_to_cpu_16(rxdp[j].wb.upper.length) -
> > +				  rxq->crc_len;
> > +
> >  			mb->data_len = pkt_len;
> >  			mb->pkt_len = pkt_len;
> > -			mb->vlan_tci = rxdp[j].wb.upper.vlan;
> >  			mb->vlan_tci =
> > rte_le_to_cpu_16(rxdp[j].wb.upper.vlan);
> >
> >  			/* convert descriptor fields to rte mbuf flags */
> >  			pkt_flags  = rx_desc_hlen_type_rss_to_pkt_flags(
> > -					rxdp[j].wb.lower.lo_dword.data);
> > +
> > rte_le_to_cpu_32(rxdp[j].wb.lower.lo_dword.data));
> > +
> >  			/* reuse status field from scan list */
> >  			pkt_flags |= rx_desc_status_to_pkt_flags(s[j]);
> >  			pkt_flags |= rx_desc_error_to_pkt_flags(s[j]);
> >  			mb->ol_flags = pkt_flags;
> >
> >  			if (likely(pkt_flags & PKT_RX_RSS_HASH))
> > -				mb->hash.rss =
> > rxdp[j].wb.lower.hi_dword.rss;
> > +				mb->hash.rss = rte_le_to_cpu_32(
> > +				    rxdp[j].wb.lower.hi_dword.rss);
> >  			else if (pkt_flags & PKT_RX_FDIR) {
> > -				mb->hash.fdir.hash =
> > -
> > 	(uint16_t)((rxdp[j].wb.lower.hi_dword.csum_ip.csum)
> > -						& IXGBE_ATR_HASH_MASK);
> > -				mb->hash.fdir.id =
> > rxdp[j].wb.lower.hi_dword.csum_ip.ip_id;
> > +				mb->hash.fdir.hash = rte_le_to_cpu_16(
> > +				    rxdp[j].wb.lower.hi_dword.csum_ip.csum)
> > &
> > +				    IXGBE_ATR_HASH_MASK;
> > +
> > +				mb->hash.fdir.id = rte_le_to_cpu_16(
> > +				    rxdp[j].wb.lower.hi_dword.csum_ip.ip_id);
> >  			}
> >  		}
> >
> > @@ -1221,7 +1239,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf
> > **rx_pkts,
> >  		 */
> >  		rxdp = &rx_ring[rx_id];
> >  		staterr = rxdp->wb.upper.status_error;
> > -		if (! (staterr &
> > rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
> > +		if (!(staterr & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
> >  			break;
> >  		rxd = *rxdp;
> >
> > @@ -1325,12 +1343,15 @@ ixgbe_recv_pkts(void *rx_queue, struct
> > rte_mbuf **rx_pkts,
> >  		rxm->ol_flags = pkt_flags;
> >
> >  		if (likely(pkt_flags & PKT_RX_RSS_HASH))
> > -			rxm->hash.rss = rxd.wb.lower.hi_dword.rss;
> > +			rxm->hash.rss = rte_le_to_cpu_32(
> > +						rxd.wb.lower.hi_dword.rss);
> >  		else if (pkt_flags & PKT_RX_FDIR) {
> > -			rxm->hash.fdir.hash =
> > -
> > 	(uint16_t)((rxd.wb.lower.hi_dword.csum_ip.csum)
> > -					   & IXGBE_ATR_HASH_MASK);
> > -			rxm->hash.fdir.id =
> > rxd.wb.lower.hi_dword.csum_ip.ip_id;
> > +			rxm->hash.fdir.hash = rte_le_to_cpu_16(
> > +
> > 	rxd.wb.lower.hi_dword.csum_ip.csum) &
> > +					IXGBE_ATR_HASH_MASK;
> > +
> > +			rxm->hash.fdir.id = rte_le_to_cpu_16(
> > +
> > 	rxd.wb.lower.hi_dword.csum_ip.ip_id);
> >  		}
> >  		/*
> >  		 * Store the mbuf address into the next entry of the array @@
> > -1413,7 +1434,7 @@ ixgbe_recv_scattered_pkts(void *rx_queue, struct
> > rte_mbuf **rx_pkts,
> >  		 */
> >  		rxdp = &rx_ring[rx_id];
> >  		staterr = rxdp->wb.upper.status_error;
> > -		if (! (staterr &
> > rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
> > +		if (!(staterr & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
> >  			break;
> >  		rxd = *rxdp;
> >
> > @@ -1742,7 +1763,7 @@ ixgbe_reset_tx_queue(struct ixgbe_tx_queue
> > *txq)
> >  	prev = (uint16_t) (txq->nb_tx_desc - 1);
> >  	for (i = 0; i < txq->nb_tx_desc; i++) {
> >  		volatile union ixgbe_adv_tx_desc *txd = &txq->tx_ring[i];
> > -		txd->wb.status = IXGBE_TXD_STAT_DD;
> > +		txd->wb.status = rte_cpu_to_le_32(IXGBE_TXD_STAT_DD);
> >  		txe[i].mbuf = NULL;
> >  		txe[i].last_id = i;
> >  		txe[prev].next_id = i;
> > @@ -2293,7 +2314,8 @@ ixgbe_dev_rx_queue_count(struct rte_eth_dev
> > *dev, uint16_t rx_queue_id)
> >  	rxdp = &(rxq->rx_ring[rxq->rx_tail]);
> >
> >  	while ((desc < rxq->nb_rx_desc) &&
> > -		(rxdp->wb.upper.status_error & IXGBE_RXDADV_STAT_DD))
> > {
> > +		(rte_le_to_cpu_32(rxdp->wb.upper.status_error) &
> > +				  IXGBE_RXDADV_STAT_DD)) {
> >  		desc += IXGBE_RXQ_SCAN_INTERVAL;
> >  		rxdp += IXGBE_RXQ_SCAN_INTERVAL;
> >  		if (rxq->rx_tail + desc >= rxq->nb_rx_desc) @@ -2318,7
> > +2340,8 @@ ixgbe_dev_rx_descriptor_done(void *rx_queue, uint16_t
> > offset)
> >  		desc -= rxq->nb_rx_desc;
> >
> >  	rxdp = &rxq->rx_ring[desc];
> > -	return !!(rxdp->wb.upper.status_error &
> > IXGBE_RXDADV_STAT_DD);
> > +	return !!(rxdp->wb.upper.status_error &
> > +			rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD));
> >  }
> >
> >  void
> > --
> > 1.9.1
> Hi Xuelin,
> I haven't had a chance to test the patches yet but if its ok with
> everyone can we defer on this until after the 2.0 package as it would be
> risky to make the change right before the release without full testing?
> Thanks
> Siobhan
> <siobhan.a.butler@intel.com>

Re: [dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian cpu

[Ananyev, Konstantin]

Hi,

> -----Original Message-----
> From: xuelin.shi@freescale.com [mailto:xuelin.shi@freescale.com]
> Sent: Tuesday, March 31, 2015 8:26 AM
> To: Ananyev, Konstantin
> Cc: dev@dpdk.org; thomas.monjalon@6wind.com; Xuelin Shi
> Subject: [PATCH v3] ixgbe: fix data access on big endian cpu
> 
> From: Xuelin Shi <xuelin.shi@freescale.com>
> 
> enforce rules of the cpu and ixgbe exchange data.
> 1. cpu use data owned by ixgbe must use rte_le_to_cpu_xx(...)
> 2. cpu fill data to ixgbe must use rte_cpu_to_le_xx(...)
> 3. checking pci status with converted constant.
> 
> Signed-off-by: Xuelin Shi <xuelin.shi@freescale.com>
> ---
> change for v3:
>    check pci status with converted constant to avoid performance penalty.
>    remove tmp variable.
> 
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 89 ++++++++++++++++++++++++---------------
>  1 file changed, 56 insertions(+), 33 deletions(-)
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index 9da2c7e..6e508ec 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -129,7 +129,7 @@ ixgbe_tx_free_bufs(struct ixgbe_tx_queue *txq)
> 
>  	/* check DD bit on threshold descriptor */
>  	status = txq->tx_ring[txq->tx_next_dd].wb.status;
> -	if (! (status & IXGBE_ADVTXD_STAT_DD))
> +	if (!(status & rte_cpu_to_le_32(IXGBE_ADVTXD_STAT_DD)))
>  		return 0;
> 
>  	/*
> @@ -174,11 +174,14 @@ tx4(volatile union ixgbe_adv_tx_desc *txdp, struct rte_mbuf **pkts)
>  		pkt_len = (*pkts)->data_len;
> 
>  		/* write data to descriptor */
> -		txdp->read.buffer_addr = buf_dma_addr;
> +		txdp->read.buffer_addr = rte_cpu_to_le_64(buf_dma_addr);
> +
>  		txdp->read.cmd_type_len =
> -				((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> +			rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> +
>  		txdp->read.olinfo_status =
> -				(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> +			rte_cpu_to_le_32(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> +
>  		rte_prefetch0(&(*pkts)->pool);
>  	}
>  }
> @@ -194,11 +197,14 @@ tx1(volatile union ixgbe_adv_tx_desc *txdp, struct rte_mbuf **pkts)
>  	pkt_len = (*pkts)->data_len;
> 
>  	/* write data to descriptor */
> -	txdp->read.buffer_addr = buf_dma_addr;
> +	txdp->read.buffer_addr = rte_cpu_to_le_64(buf_dma_addr);
> +
>  	txdp->read.cmd_type_len =
> -			((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> +			rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> +
>  	txdp->read.olinfo_status =
> -			(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> +			rte_cpu_to_le_32(pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> +
>  	rte_prefetch0(&(*pkts)->pool);
>  }
> 
> @@ -285,7 +291,7 @@ tx_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>  		 * a divisor of the ring size
>  		 */
>  		tx_r[txq->tx_next_rs].read.cmd_type_len |=
> -			rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> +				rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
>  		txq->tx_next_rs = (uint16_t)(txq->tx_rs_thresh - 1);
> 
>  		txq->tx_tail = 0;
> @@ -304,7 +310,7 @@ tx_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>  	 */
>  	if (txq->tx_tail > txq->tx_next_rs) {
>  		tx_r[txq->tx_next_rs].read.cmd_type_len |=
> -			rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> +				rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
>  		txq->tx_next_rs = (uint16_t)(txq->tx_next_rs +
>  						txq->tx_rs_thresh);
>  		if (txq->tx_next_rs >= txq->nb_tx_desc)
> @@ -505,6 +511,7 @@ ixgbe_xmit_cleanup(struct ixgbe_tx_queue *txq)
>  	uint16_t nb_tx_desc = txq->nb_tx_desc;
>  	uint16_t desc_to_clean_to;
>  	uint16_t nb_tx_to_clean;
> +	uint32_t stat;
> 
>  	/* Determine the last descriptor needing to be cleaned */
>  	desc_to_clean_to = (uint16_t)(last_desc_cleaned + txq->tx_rs_thresh);
> @@ -513,7 +520,9 @@ ixgbe_xmit_cleanup(struct ixgbe_tx_queue *txq)
> 
>  	/* Check to make sure the last descriptor to clean is done */
>  	desc_to_clean_to = sw_ring[desc_to_clean_to].last_id;
> -	if (! (txr[desc_to_clean_to].wb.status & IXGBE_TXD_STAT_DD))
> +
> +	stat = txr[desc_to_clean_to].wb.status;
> +	if (!(stat & rte_cpu_to_le_32(IXGBE_TXD_STAT_DD)))
>  	{
>  		PMD_TX_FREE_LOG(DEBUG,
>  				"TX descriptor %4u is not done"
> @@ -801,12 +810,14 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>  			 */
>  			slen = m_seg->data_len;
>  			buf_dma_addr = RTE_MBUF_DATA_DMA_ADDR(m_seg);
> +
>  			txd->read.buffer_addr =
> -				rte_cpu_to_le_64(buf_dma_addr);
> +					rte_cpu_to_le_64(buf_dma_addr);
>  			txd->read.cmd_type_len =
> -				rte_cpu_to_le_32(cmd_type_len | slen);
> +					rte_cpu_to_le_32(cmd_type_len | slen);
>  			txd->read.olinfo_status =
> -				rte_cpu_to_le_32(olinfo_status);
> +					rte_cpu_to_le_32(olinfo_status);
> +

Not sure, what had changed here?

>  			txe->last_id = tx_last;
>  			tx_id = txe->next_id;
>  			txe = txn;
> @@ -946,14 +957,16 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
>  	uint64_t pkt_flags;
>  	int s[LOOK_AHEAD], nb_dd;
>  	int i, j, nb_rx = 0;
> +	uint32_t stat;
> 
> 
>  	/* get references to current descriptor and S/W ring entry */
>  	rxdp = &rxq->rx_ring[rxq->rx_tail];
>  	rxep = &rxq->sw_ring[rxq->rx_tail];
> 
> +	stat = rxdp->wb.upper.status_error;
>  	/* check to make sure there is at least 1 packet to receive */
> -	if (! (rxdp->wb.upper.status_error & IXGBE_RXDADV_STAT_DD))
> +	if (!(stat & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
>  		return 0;
> 
>  	/*
> @@ -965,7 +978,7 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
>  	{
>  		/* Read desc statuses backwards to avoid race condition */
>  		for (j = LOOK_AHEAD-1; j >= 0; --j)
> -			s[j] = rxdp[j].wb.upper.status_error;
> +			s[j] = rte_le_to_cpu_32(rxdp[j].wb.upper.status_error);
> 
>  		/* Compute how many status bits were set */
>  		nb_dd = 0;
> @@ -977,27 +990,32 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue *rxq)
>  		/* Translate descriptor info to mbuf format */
>  		for (j = 0; j < nb_dd; ++j) {
>  			mb = rxep[j].mbuf;
> -			pkt_len = (uint16_t)(rxdp[j].wb.upper.length - rxq->crc_len);
> +			pkt_len = rte_le_to_cpu_16(rxdp[j].wb.upper.length) -
> +				  rxq->crc_len;
> +
>  			mb->data_len = pkt_len;
>  			mb->pkt_len = pkt_len;
> -			mb->vlan_tci = rxdp[j].wb.upper.vlan;
>  			mb->vlan_tci = rte_le_to_cpu_16(rxdp[j].wb.upper.vlan);
> 
>  			/* convert descriptor fields to rte mbuf flags */
>  			pkt_flags  = rx_desc_hlen_type_rss_to_pkt_flags(
> -					rxdp[j].wb.lower.lo_dword.data);
> +			    rte_le_to_cpu_32(rxdp[j].wb.lower.lo_dword.data));
> +
>  			/* reuse status field from scan list */
>  			pkt_flags |= rx_desc_status_to_pkt_flags(s[j]);
>  			pkt_flags |= rx_desc_error_to_pkt_flags(s[j]);
>  			mb->ol_flags = pkt_flags;
> 
>  			if (likely(pkt_flags & PKT_RX_RSS_HASH))
> -				mb->hash.rss = rxdp[j].wb.lower.hi_dword.rss;
> +				mb->hash.rss = rte_le_to_cpu_32(
> +				    rxdp[j].wb.lower.hi_dword.rss);
>  			else if (pkt_flags & PKT_RX_FDIR) {
> -				mb->hash.fdir.hash =
> -					(uint16_t)((rxdp[j].wb.lower.hi_dword.csum_ip.csum)
> -						& IXGBE_ATR_HASH_MASK);
> -				mb->hash.fdir.id = rxdp[j].wb.lower.hi_dword.csum_ip.ip_id;
> +				mb->hash.fdir.hash = rte_le_to_cpu_16(
> +				    rxdp[j].wb.lower.hi_dword.csum_ip.csum) &
> +				    IXGBE_ATR_HASH_MASK;
> +
> +				mb->hash.fdir.id = rte_le_to_cpu_16(
> +				    rxdp[j].wb.lower.hi_dword.csum_ip.ip_id);
>  			}
>  		}
> 
> @@ -1221,7 +1239,7 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  		 */
>  		rxdp = &rx_ring[rx_id];
>  		staterr = rxdp->wb.upper.status_error;
> -		if (! (staterr & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
> +		if (!(staterr & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
>  			break;
>  		rxd = *rxdp;
> 
> @@ -1325,12 +1343,15 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  		rxm->ol_flags = pkt_flags;
> 
>  		if (likely(pkt_flags & PKT_RX_RSS_HASH))
> -			rxm->hash.rss = rxd.wb.lower.hi_dword.rss;
> +			rxm->hash.rss = rte_le_to_cpu_32(
> +						rxd.wb.lower.hi_dword.rss);
>  		else if (pkt_flags & PKT_RX_FDIR) {
> -			rxm->hash.fdir.hash =
> -				(uint16_t)((rxd.wb.lower.hi_dword.csum_ip.csum)
> -					   & IXGBE_ATR_HASH_MASK);
> -			rxm->hash.fdir.id = rxd.wb.lower.hi_dword.csum_ip.ip_id;
> +			rxm->hash.fdir.hash = rte_le_to_cpu_16(
> +					rxd.wb.lower.hi_dword.csum_ip.csum) &
> +					IXGBE_ATR_HASH_MASK;
> +
> +			rxm->hash.fdir.id = rte_le_to_cpu_16(
> +					rxd.wb.lower.hi_dword.csum_ip.ip_id);
>  		}
>  		/*
>  		 * Store the mbuf address into the next entry of the array
> @@ -1413,7 +1434,7 @@ ixgbe_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
>  		 */
>  		rxdp = &rx_ring[rx_id];
>  		staterr = rxdp->wb.upper.status_error;
> -		if (! (staterr & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
> +		if (!(staterr & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
>  			break;
>  		rxd = *rxdp;
> 
> @@ -1742,7 +1763,7 @@ ixgbe_reset_tx_queue(struct ixgbe_tx_queue *txq)
>  	prev = (uint16_t) (txq->nb_tx_desc - 1);
>  	for (i = 0; i < txq->nb_tx_desc; i++) {
>  		volatile union ixgbe_adv_tx_desc *txd = &txq->tx_ring[i];
> -		txd->wb.status = IXGBE_TXD_STAT_DD;
> +		txd->wb.status = rte_cpu_to_le_32(IXGBE_TXD_STAT_DD);
>  		txe[i].mbuf = NULL;
>  		txe[i].last_id = i;
>  		txe[prev].next_id = i;
> @@ -2293,7 +2314,8 @@ ixgbe_dev_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
>  	rxdp = &(rxq->rx_ring[rxq->rx_tail]);
> 
>  	while ((desc < rxq->nb_rx_desc) &&
> -		(rxdp->wb.upper.status_error & IXGBE_RXDADV_STAT_DD)) {
> +		(rte_le_to_cpu_32(rxdp->wb.upper.status_error) &
> +				  IXGBE_RXDADV_STAT_DD)) {

Why not ' rxdp->wb.upper.status_error & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)'?
To keep it consistent with rest of the changes?

Konstantin

>  		desc += IXGBE_RXQ_SCAN_INTERVAL;
>  		rxdp += IXGBE_RXQ_SCAN_INTERVAL;
>  		if (rxq->rx_tail + desc >= rxq->nb_rx_desc)
> @@ -2318,7 +2340,8 @@ ixgbe_dev_rx_descriptor_done(void *rx_queue, uint16_t offset)
>  		desc -= rxq->nb_rx_desc;
> 
>  	rxdp = &rxq->rx_ring[desc];
> -	return !!(rxdp->wb.upper.status_error & IXGBE_RXDADV_STAT_DD);
> +	return !!(rxdp->wb.upper.status_error &
> +			rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD));
>  }
> 
>  void
> --
> 1.9.1

Re: [dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian cpu

[Thomas Monjalon]

Xuelin, have you given up with that patch?

2015-04-14 23:11, Ananyev, Konstantin:
> From: xuelin.shi@freescale.com [mailto:xuelin.shi@freescale.com]
> >  			txd->read.olinfo_status =
> > -				rte_cpu_to_le_32(olinfo_status);
> > +					rte_cpu_to_le_32(olinfo_status);
> > +
> 
> Not sure, what had changed here?

> > @@ -2293,7 +2314,8 @@ ixgbe_dev_rx_queue_count(struct rte_eth_dev *dev, uint16_t rx_queue_id)
> >  	rxdp = &(rxq->rx_ring[rxq->rx_tail]);
> > 
> >  	while ((desc < rxq->nb_rx_desc) &&
> > -		(rxdp->wb.upper.status_error & IXGBE_RXDADV_STAT_DD)) {
> > +		(rte_le_to_cpu_32(rxdp->wb.upper.status_error) &
> > +				  IXGBE_RXDADV_STAT_DD)) {
> 
> Why not ' rxdp->wb.upper.status_error & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)'?
> To keep it consistent with rest of the changes?

Re: [dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian cpu

[Xuelin Shi]

Hi Thomas,

I have worked out the new version based on dpdk v2 codebase to address the comments.
	http://www.dpdk.org/dev/patchwork/patch/6449/

thanks,
Xuelin Shi

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, July 10, 2015 22:52
> To: Shi Xuelin-B29237
> Cc: Ananyev, Konstantin; dev@dpdk.org
> Subject: Re: [PATCH v3] ixgbe: fix data access on big endian cpu
> 
> Xuelin, have you given up with that patch?
> 
> 2015-04-14 23:11, Ananyev, Konstantin:
> > From: xuelin.shi@freescale.com [mailto:xuelin.shi@freescale.com]
> > >  			txd->read.olinfo_status =
> > > -				rte_cpu_to_le_32(olinfo_status);
> > > +					rte_cpu_to_le_32(olinfo_status);
> > > +
> >
> > Not sure, what had changed here?
> 
> > > @@ -2293,7 +2314,8 @@ ixgbe_dev_rx_queue_count(struct rte_eth_dev
> *dev, uint16_t rx_queue_id)
> > >  	rxdp = &(rxq->rx_ring[rxq->rx_tail]);
> > >
> > >  	while ((desc < rxq->nb_rx_desc) &&
> > > -		(rxdp->wb.upper.status_error & IXGBE_RXDADV_STAT_DD)) {
> > > +		(rte_le_to_cpu_32(rxdp->wb.upper.status_error) &
> > > +				  IXGBE_RXDADV_STAT_DD)) {
> >
> > Why not ' rxdp->wb.upper.status_error &
> rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)'?
> > To keep it consistent with rest of the changes?
> 
>