From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 2540F11F5 for ; Tue, 8 Dec 2015 02:53:14 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga103.jf.intel.com with ESMTP; 07 Dec 2015 17:53:15 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,397,1444719600"; d="scan'208";a="868790622" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49]) by fmsmga002.fm.intel.com with ESMTP; 07 Dec 2015 17:53:14 -0800 Date: Tue, 8 Dec 2015 09:54:10 +0800 From: Yuanhan Liu To: Stephen Hemminger Message-ID: <20151208015410.GG29571@yliu-dev.sh.intel.com> References: <1449191574-14629-1-git-send-email-stephen@networkplumber.org> <1449191574-14629-3-git-send-email-stephen@networkplumber.org> <20151204032858.GB29571@yliu-dev.sh.intel.com> <20151205115007.2230a16a@xeon-e3> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151205115007.2230a16a@xeon-e3> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 2/2] virtio: clean up space checks on xmit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 Dec 2015 01:53:15 -0000 On Sat, Dec 05, 2015 at 11:50:07AM -0800, Stephen Hemminger wrote: > int error; > > > > > > @@ -846,58 +845,49 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts) > > > if (likely(nb_used > txvq->vq_nentries - txvq->vq_free_thresh)) > > > virtio_xmit_cleanup(txvq, nb_used); > > > > > > - nb_tx = 0; > > > - > > > - while (nb_tx < nb_pkts) { > > > + for (nb_tx = 0; nb_tx < nb_pkts; nb_tx++) { > > > + struct rte_mbuf *txm = tx_pkts[nb_tx]; > > > /* Need one more descriptor for virtio header. */ > > > - int need = tx_pkts[nb_tx]->nb_segs - txvq->vq_free_cnt + 1; > > > + int need = txm->nb_segs - txvq->vq_free_cnt + 1; > > > > While reviewing the code, I found the var name `need' is not properly > > taken. Maybe `need_cleanup' is better, and it's better to be defined > > as bool type. What do you think of that? > > The variable need indicates how many more buffers are needed to > complete the transmit. In later patches, there is a variable slots > so: > needed = slots - free > > So if needed is positive, then more buffers are needed than available > and transmit is blocked. If needed is negative then there is free > space available. Yeah, I knew that. And there is a comment for that (thanks for the explanation anyway!): /* Positive value indicates it need free vring descriptors */ I mean, if a var name well taken, we could avoid comments like above. However, for this case, I simply overlooked that virtio_xmit_cleanup() takes `need' as the input, so that I thought `need' is just a boolean var to check if we need to get few more free spaces. And that's how my above suggestion comes. So, sorry for the noisy. --yliu