From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 217DB2BDF for ; Mon, 25 Jun 2018 16:30:47 +0200 (CEST) Received: from rsa59-2-82-233-193-189.fbx.proxad.net ([82.233.193.189] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1fXSWP-0001ja-92; Mon, 25 Jun 2018 16:31:15 +0200 Received: by droids-corp.org (sSMTP sendmail emulation); Mon, 25 Jun 2018 16:30:40 +0200 Date: Mon, 25 Jun 2018 16:30:40 +0200 From: Olivier Matz To: Wei Zhao , Qi Z Zhang Cc: dev@dpdk.org, Wenzhuo Lu Message-ID: <20180625143040.55wb5smx4kzshtqe@platinum> References: <20180625142057.6296-1-olivier.matz@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180625142057.6296-1-olivier.matz@6wind.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [RFC] net/ixgbe: fix Tx descriptor status api X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Jun 2018 14:30:47 -0000 Removing wrong mails from the To and Cc. Sorry. On Mon, Jun 25, 2018 at 04:20:57PM +0200, Olivier Matz wrote: > The Tx descriptor status api was not behaving as expected. This API is > used to inspect the content of the descriptors in the Tx ring to > determine the length of the Tx queue. > > Since the software advances the tail pointer and the hardware advances > the head pointer, the Tx queue is located before txq->tx_tail in the > ring. Therefore, a call to rte_eth_tx_descriptor_status(..., offset=20) > should inspect the 20th descriptor before the tail, not after. > > As before, we still need to take care about only checking descriptors > that have the RS bit. > > Additionally, we can avoid an access to the ring if offset is greater or > equal to nb_tx_desc - nb_tx_free. > > Fixes: a2919e13d95e ("net/ixgbe: implement descriptor status API") > Signed-off-by: Olivier Matz > Signed-off-by: Didier Pallard > --- > > Hi Wei, Hi Qi, > > We also recently found some issues in Tx descriptor status API for ixgbe, > i40, e1000, igb. I'm preparing a clean patchset for all of them. > > Here is already the patch for ixgbe, please let me know what you think. > > The API comment of rte_eth_tx_descriptor_status() is incorrect and should > be fixed too. The reference descriptor (when offset = 0) is not where the > next packet will be sent, but where the latest packet has been enqueued. > > Regards, > Olivier > > > > drivers/net/ixgbe/ixgbe_rxtx.c | 45 +++++++++++++++++++++++++++++++----------- > drivers/net/ixgbe/ixgbe_rxtx.h | 1 + > 2 files changed, 34 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c > index 3e13d26ae..384587cc6 100644 > --- a/drivers/net/ixgbe/ixgbe_rxtx.c > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c > @@ -2606,10 +2606,15 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev *dev, > hw->mac.type == ixgbe_mac_X540_vf || > hw->mac.type == ixgbe_mac_X550_vf || > hw->mac.type == ixgbe_mac_X550EM_x_vf || > - hw->mac.type == ixgbe_mac_X550EM_a_vf) > + hw->mac.type == ixgbe_mac_X550EM_a_vf) { > txq->tdt_reg_addr = IXGBE_PCI_REG_ADDR(hw, IXGBE_VFTDT(queue_idx)); > - else > + txq->tdh_reg_addr = IXGBE_PCI_REG_ADDR(hw, > + IXGBE_VFTDH(queue_idx)); > + } else { > txq->tdt_reg_addr = IXGBE_PCI_REG_ADDR(hw, IXGBE_TDT(txq->reg_idx)); > + txq->tdh_reg_addr = IXGBE_PCI_REG_ADDR(hw, > + IXGBE_TDH(txq->reg_idx)); > + } > > txq->tx_ring_phys_addr = tz->iova; > txq->tx_ring = (union ixgbe_adv_tx_desc *) tz->addr; > @@ -3140,22 +3145,38 @@ ixgbe_dev_tx_descriptor_status(void *tx_queue, uint16_t offset) > { > struct ixgbe_tx_queue *txq = tx_queue; > volatile uint32_t *status; > - uint32_t desc; > + int32_t desc, dd; > > if (unlikely(offset >= txq->nb_tx_desc)) > return -EINVAL; > + if (offset >= txq->nb_tx_desc - txq->nb_tx_free) > + return RTE_ETH_TX_DESC_DONE; > + > + desc = txq->tx_tail - offset - 1; > + if (desc < 0) > + desc += txq->nb_tx_desc; > > - desc = txq->tx_tail + offset; > - /* go to next desc that has the RS bit */ > - desc = ((desc + txq->tx_rs_thresh - 1) / txq->tx_rs_thresh) * > - txq->tx_rs_thresh; > - if (desc >= txq->nb_tx_desc) { > - desc -= txq->nb_tx_desc; > - if (desc >= txq->nb_tx_desc) > - desc -= txq->nb_tx_desc; > + /* offset is too small, no other way than reading PCI reg */ > + if (unlikely(offset < txq->tx_rs_thresh)) { > + int16_t tx_head, queue_size; > + tx_head = ixgbe_read_addr(txq->tdh_reg_addr); > + queue_size = txq->tx_tail - tx_head; > + if (queue_size < 0) > + queue_size += txq->nb_tx_desc; > + return queue_size > offset ? RTE_ETH_TX_DESC_FULL : > + RTE_ETH_TX_DESC_DONE; > } > > - status = &txq->tx_ring[desc].wb.status; > + /* index of the dd bit to look at */ > + dd = (desc / txq->tx_rs_thresh + 1) * txq->tx_rs_thresh - 1; > + > + /* In full featured mode, RS bit is only set in the last descriptor */ > + /* of a multisegments packet */ > + if (!((txq->offloads == 0) && > + (txq->tx_rs_thresh >= RTE_PMD_IXGBE_TX_MAX_BURST))) > + dd = txq->sw_ring[dd].last_id; > + > + status = &txq->tx_ring[dd].wb.status; > if (*status & rte_cpu_to_le_32(IXGBE_ADVTXD_STAT_DD)) > return RTE_ETH_TX_DESC_DONE; > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h > index 39378f754..384f6324d 100644 > --- a/drivers/net/ixgbe/ixgbe_rxtx.h > +++ b/drivers/net/ixgbe/ixgbe_rxtx.h > @@ -201,6 +201,7 @@ struct ixgbe_tx_queue { > struct ixgbe_tx_entry_v *sw_ring_v; /**< address of SW ring for vector PMD */ > }; > volatile uint32_t *tdt_reg_addr; /**< Address of TDT register. */ > + volatile uint32_t *tdh_reg_addr; /**< Address of TDH register. */ > uint16_t nb_tx_desc; /**< number of TX descriptors. */ > uint16_t tx_tail; /**< current value of TDT reg. */ > /**< Start freeing TX buffers if there are less free descriptors than > -- > 2.11.0 >