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 C2B3E1B14D for ; Tue, 26 Jun 2018 10:46:14 +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 1fXjcX-0003qb-6Z; Tue, 26 Jun 2018 10:46:43 +0200 Received: by droids-corp.org (sSMTP sendmail emulation); Tue, 26 Jun 2018 10:46:08 +0200 Date: Tue, 26 Jun 2018 10:46:08 +0200 From: Olivier Matz To: "Zhao1, Wei" Cc: "Zhang, Qi Z" , "dev@dpdk.org" , "Lu, Wenzhuo" Message-ID: <20180626084608.c4kfsc5ccnfwypgw@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: 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: Tue, 26 Jun 2018 08:46:14 -0000 Hi Wei, On Tue, Jun 26, 2018 at 01:38:22AM +0000, Zhao1, Wei wrote: > Hi, Olivier Matz > > Will you commit fix patch for i40e and ixgbe and em? If you think the patch are relevant, yes :) Here is a pre-version (last 5 patches): http://git.droids-corp.org/?p=dpdk.git;a=shortlog;h=refs/heads/tx-desc It still need to fix checkpatch issues, few more tests, and rebase on next-net. > And the code " dd = (desc / txq->tx_rs_thresh + 1) * txq->tx_rs_thresh - 1;" > Is only proper for tx function ixgbe_xmit_pkts_simple and ixgbe_xmit_pkts_vec (). > But not proper for ixgbe_xmit_pkts (), the RS bit set rule is different from all these two. Can you please give more detail please? Note this code, maybe you are talking about this? + /* 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; Maybe there is something better to test? Just to ensure we are on the same line, here are some more infos. === - sw advances the tail pointer - hw advances the head pointer - the software populates the ring with full buffers to be sent by the hw - head points to the in-progress descriptor. - sw writes new descriptors at tail - head == tail means that the transmit queue is empty - when the hw has processed a descriptor, it sets the DD bit if the descriptor has the RS (report status) bit. - the driver never reads the head (needs a pci transaction), instead it monitors the DD bit of a descriptor that has the RS bit txq->tx_tail: sw value for tail register txq->tx_free_thresh: free buffers if count(free descs) < this value txq->tx_rs_thresh: RS bit is set every rs_thresh descriptor txq->tx_next_dd: next desc to scan for DD bit txq->tx_next_rs: next desc to set RS bit txq->last_desc_cleaned: last descriptor that have been cleaned txq->nb_tx_free: number of free descriptors Example: |----------------------------------------------------------------| | D R R R | | ............xxxxxxxxxxxxxxxxxxxxxxxxx | | <- descs not sent yet -> | | ............xxxxxxxxxxxxxxxxxxxxxxxxx | |----------------------------------------------------------------| ^last_desc_cleaned=8 ^next_rs=47 ^next_dd=15 ^sw_tail=45 ^hw_head=20 <---- nb_used ---------> The hardware is currently processing the descriptor 20 'R' means the descriptor has the RS bit 'D' means the descriptor has the DD + RS bits 'x' are packets in txq (not sent) '.' are packet already sent but not freed by sw In this example, we have rs_thres=8. On next call to ixgbe_tx_free_bufs(), some buffers will be freed. === Let's call ixgbe_dev_tx_descriptor_status(10): - original version: desc = 45 + 10 = 55 desc = ((55 + 8 - 1) / 8) * 8 = (62 / 8) * 8 = 56 wrong because it goes in the wrong direction, and because 56 does not have the RS bit - after your patch: desc = 45 + 10 = 55 desc = (((55 / 8) + 1) * 8) - 1 = (7 * 8) - 1 = 55 wrong because it goes in the wrong direction - after my patch desc = 45 - 10 - 1 = 34 desc = (((34 / 8) + 1) * 8) - 1 = (5 * 8) - 1 = 39 looks correct Regards, Olivier