From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <xuelin.shi@freescale.com>
Received: from na01-by2-obe.outbound.protection.outlook.com
(mail-by2on0115.outbound.protection.outlook.com [207.46.100.115])
by dpdk.org (Postfix) with ESMTP id D80566A95
for <dev@dpdk.org>; Fri, 3 Apr 2015 04:02:52 +0200 (CEST)
Received: from BY1PR03MB1339.namprd03.prod.outlook.com (25.162.109.21) by
BY1PR03MB1337.namprd03.prod.outlook.com (25.162.109.19) with Microsoft SMTP
Server (TLS) id 15.1.130.23; Fri, 3 Apr 2015 02:02:50 +0000
Received: from BY1PR03MB1339.namprd03.prod.outlook.com ([25.162.109.21]) by
BY1PR03MB1339.namprd03.prod.outlook.com ([25.162.109.21]) with mapi id
15.01.0125.002; Fri, 3 Apr 2015 02:02:50 +0000
From: Xuelin Shi <xuelin.shi@freescale.com>
To: "Butler, Siobhan A" <siobhan.a.butler@intel.com>
Thread-Topic: [dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian cpu
Thread-Index: AQHQa4sLnIjQ9ACIrkSlHDCefxY1b506LSQAgABezKA=
Date: Fri, 3 Apr 2015 02:02:49 +0000
Message-ID: <BY1PR03MB1339E77C780895D7F3D3662B86F10@BY1PR03MB1339.namprd03.prod.outlook.com>
References: <1427786750-30308-1-git-send-email-xuelin.shi@freescale.com>
<0C5AFCA4B3408848ADF2A3073F7D8CC86D587220@IRSMSX109.ger.corp.intel.com>
In-Reply-To: <0C5AFCA4B3408848ADF2A3073F7D8CC86D587220@IRSMSX109.ger.corp.intel.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [192.158.241.86]
authentication-results: intel.com; dkim=none (message not signed)
header.d=none;
x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BY1PR03MB1337;
x-forefront-antispam-report: BMV:1; SFV:NSPM;
SFS:(10019020)(6009001)(13464003)(51704005)(377454003)(164054003)(46102003)(110136001)(2950100001)(102836002)(2900100001)(19580405001)(122556002)(86362001)(40100003)(74316001)(76576001)(19580395003)(62966003)(77156002)(76176999)(54356999)(50986999)(66066001)(106116001)(99286002)(2656002)(33656002)(92566002)(87936001);
DIR:OUT; SFP:1102; SCL:1; SRVR:BY1PR03MB1337;
H:BY1PR03MB1339.namprd03.prod.outlook.com; FPR:; SPF:None; MLV:sfv; LANG:en;
x-microsoft-antispam-prvs: <BY1PR03MB1337D9266329DE7051A6820486F10@BY1PR03MB1337.namprd03.prod.outlook.com>
x-exchange-antispam-report-test: UriScan:;
x-exchange-antispam-report-cfa-test: BCL:0; PCL:0;
RULEID:(601004)(5002010)(5005006); SRVR:BY1PR03MB1337; BCL:0; PCL:0; RULEID:;
SRVR:BY1PR03MB1337;
x-forefront-prvs: 05352A48BE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: freescale.com
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Apr 2015 02:02:49.7821 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 710a03f5-10f6-4d38-9ff4-a80b81da590d
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR03MB1337
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v3] ixgbe: fix data access on big endian cpu
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 03 Apr 2015 02:02:53 -0000
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
>=20
>=20
>=20
> > -----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 =3D 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 =3D (*pkts)->data_len;
> >
> > /* write data to descriptor */
> > - txdp->read.buffer_addr =3D buf_dma_addr;
> > + txdp->read.buffer_addr =3D
> > rte_cpu_to_le_64(buf_dma_addr);
> > +
> > txdp->read.cmd_type_len =3D
> > - ((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> > + rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS |
> > pkt_len);
> > +
> > txdp->read.olinfo_status =3D
> > - (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 =3D (*pkts)->data_len;
> >
> > /* write data to descriptor */
> > - txdp->read.buffer_addr =3D buf_dma_addr;
> > + txdp->read.buffer_addr =3D rte_cpu_to_le_64(buf_dma_addr);
> > +
> > txdp->read.cmd_type_len =3D
> > - ((uint32_t)DCMD_DTYP_FLAGS | pkt_len);
> > + rte_cpu_to_le_32((uint32_t)DCMD_DTYP_FLAGS |
> > pkt_len);
> > +
> > txdp->read.olinfo_status =3D
> > - (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 |=3D
> > - rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> > +
> > rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> > txq->tx_next_rs =3D (uint16_t)(txq->tx_rs_thresh - 1);
> >
> > txq->tx_tail =3D 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 |=3D
> > - rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> > +
> > rte_cpu_to_le_32(IXGBE_ADVTXD_DCMD_RS);
> > txq->tx_next_rs =3D (uint16_t)(txq->tx_next_rs +
> > txq->tx_rs_thresh);
> > if (txq->tx_next_rs >=3D txq->nb_tx_desc) @@ -505,6 +511,7 @@
> > ixgbe_xmit_cleanup(struct ixgbe_tx_queue *txq)
> > uint16_t nb_tx_desc =3D 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 =3D (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 =3D sw_ring[desc_to_clean_to].last_id;
> > - if (! (txr[desc_to_clean_to].wb.status & IXGBE_TXD_STAT_DD))
> > +
> > + stat =3D 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 =3D m_seg->data_len;
> > buf_dma_addr =3D
> > RTE_MBUF_DATA_DMA_ADDR(m_seg);
> > +
> > txd->read.buffer_addr =3D
> > - rte_cpu_to_le_64(buf_dma_addr);
> > + rte_cpu_to_le_64(buf_dma_addr);
> > txd->read.cmd_type_len =3D
> > - rte_cpu_to_le_32(cmd_type_len | slen);
> > + rte_cpu_to_le_32(cmd_type_len |
> > slen);
> > txd->read.olinfo_status =3D
> > - rte_cpu_to_le_32(olinfo_status);
> > + rte_cpu_to_le_32(olinfo_status);
> > +
> > txe->last_id =3D tx_last;
> > tx_id =3D txe->next_id;
> > txe =3D 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 =3D 0;
> > + uint32_t stat;
> >
> >
> > /* get references to current descriptor and S/W ring entry */
> > rxdp =3D &rxq->rx_ring[rxq->rx_tail];
> > rxep =3D &rxq->sw_ring[rxq->rx_tail];
> >
> > + stat =3D 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 =3D LOOK_AHEAD-1; j >=3D 0; --j)
> > - s[j] =3D rxdp[j].wb.upper.status_error;
> > + s[j] =3D
> > rte_le_to_cpu_32(rxdp[j].wb.upper.status_error);
> >
> > /* Compute how many status bits were set */
> > nb_dd =3D 0;
> > @@ -977,27 +990,32 @@ ixgbe_rx_scan_hw_ring(struct ixgbe_rx_queue
> > *rxq)
> > /* Translate descriptor info to mbuf format */
> > for (j =3D 0; j < nb_dd; ++j) {
> > mb =3D rxep[j].mbuf;
> > - pkt_len =3D (uint16_t)(rxdp[j].wb.upper.length - rxq-
> > >crc_len);
> > + pkt_len =3D
> > rte_le_to_cpu_16(rxdp[j].wb.upper.length) -
> > + rxq->crc_len;
> > +
> > mb->data_len =3D pkt_len;
> > mb->pkt_len =3D pkt_len;
> > - mb->vlan_tci =3D rxdp[j].wb.upper.vlan;
> > mb->vlan_tci =3D
> > rte_le_to_cpu_16(rxdp[j].wb.upper.vlan);
> >
> > /* convert descriptor fields to rte mbuf flags */
> > pkt_flags =3D 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 |=3D rx_desc_status_to_pkt_flags(s[j]);
> > pkt_flags |=3D rx_desc_error_to_pkt_flags(s[j]);
> > mb->ol_flags =3D pkt_flags;
> >
> > if (likely(pkt_flags & PKT_RX_RSS_HASH))
> > - mb->hash.rss =3D
> > rxdp[j].wb.lower.hi_dword.rss;
> > + mb->hash.rss =3D rte_le_to_cpu_32(
> > + rxdp[j].wb.lower.hi_dword.rss);
> > else if (pkt_flags & PKT_RX_FDIR) {
> > - mb->hash.fdir.hash =3D
> > -
> > (uint16_t)((rxdp[j].wb.lower.hi_dword.csum_ip.csum)
> > - & IXGBE_ATR_HASH_MASK);
> > - mb->hash.fdir.id =3D
> > rxdp[j].wb.lower.hi_dword.csum_ip.ip_id;
> > + mb->hash.fdir.hash =3D rte_le_to_cpu_16(
> > + rxdp[j].wb.lower.hi_dword.csum_ip.csum)
> > &
> > + IXGBE_ATR_HASH_MASK;
> > +
> > + mb->hash.fdir.id =3D 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 =3D &rx_ring[rx_id];
> > staterr =3D 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 =3D *rxdp;
> >
> > @@ -1325,12 +1343,15 @@ ixgbe_recv_pkts(void *rx_queue, struct
> > rte_mbuf **rx_pkts,
> > rxm->ol_flags =3D pkt_flags;
> >
> > if (likely(pkt_flags & PKT_RX_RSS_HASH))
> > - rxm->hash.rss =3D rxd.wb.lower.hi_dword.rss;
> > + rxm->hash.rss =3D rte_le_to_cpu_32(
> > + rxd.wb.lower.hi_dword.rss);
> > else if (pkt_flags & PKT_RX_FDIR) {
> > - rxm->hash.fdir.hash =3D
> > -
> > (uint16_t)((rxd.wb.lower.hi_dword.csum_ip.csum)
> > - & IXGBE_ATR_HASH_MASK);
> > - rxm->hash.fdir.id =3D
> > rxd.wb.lower.hi_dword.csum_ip.ip_id;
> > + rxm->hash.fdir.hash =3D rte_le_to_cpu_16(
> > +
> > rxd.wb.lower.hi_dword.csum_ip.csum) &
> > + IXGBE_ATR_HASH_MASK;
> > +
> > + rxm->hash.fdir.id =3D 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 =3D &rx_ring[rx_id];
> > staterr =3D 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 =3D *rxdp;
> >
> > @@ -1742,7 +1763,7 @@ ixgbe_reset_tx_queue(struct ixgbe_tx_queue
> > *txq)
> > prev =3D (uint16_t) (txq->nb_tx_desc - 1);
> > for (i =3D 0; i < txq->nb_tx_desc; i++) {
> > volatile union ixgbe_adv_tx_desc *txd =3D &txq->tx_ring[i];
> > - txd->wb.status =3D IXGBE_TXD_STAT_DD;
> > + txd->wb.status =3D rte_cpu_to_le_32(IXGBE_TXD_STAT_DD);
> > txe[i].mbuf =3D NULL;
> > txe[i].last_id =3D i;
> > txe[prev].next_id =3D i;
> > @@ -2293,7 +2314,8 @@ ixgbe_dev_rx_queue_count(struct rte_eth_dev
> > *dev, uint16_t rx_queue_id)
> > rxdp =3D &(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 +=3D IXGBE_RXQ_SCAN_INTERVAL;
> > rxdp +=3D IXGBE_RXQ_SCAN_INTERVAL;
> > if (rxq->rx_tail + desc >=3D rxq->nb_rx_desc) @@ -2318,7
> > +2340,8 @@ ixgbe_dev_rx_descriptor_done(void *rx_queue, uint16_t
> > offset)
> > desc -=3D rxq->nb_rx_desc;
> >
> > rxdp =3D &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>