From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f172.google.com (mail-pf0-f172.google.com [209.85.192.172]) by dpdk.org (Postfix) with ESMTP id 89B6AC13A for ; Mon, 7 Dec 2015 19:10:48 +0100 (CET) Received: by pfdd184 with SMTP id d184so68556972pfd.3 for ; Mon, 07 Dec 2015 10:10:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; bh=9ccCPfN5i2BcFcdxrvUzzvTVGIyPQ5ZvMJAywOSbSOk=; b=m6j3Vd30fKovVj/m2SIgA7Ix65VSiqH3DuGwhs7E0RjxsFnVW+E7IUx//kTQcsqYv2 EnDvm2VZTTF3xhu5tMxqMVCx1E5DkT/0pko2Gd0BxIjOVwJ0ec0VJoLpmgNhUa0GV+mk rulOwitM51uI2fabu4jj5Ag7mSTWFbmzLmbV3iCa2FWM+9/NgDuYFwDFOio0cFJ1RhXR 2w+Z3k5nYYkNm8ZNWbsR2I2gquxVb0/8tOPy5bfwd2u2kMXGMazxKjNFY3TxuJN1QvUw S0d5nbLCxPw//cMk558AH3c2vdohjq0UmCcdmfTAoMu6cjmU27fAHWeUOa2ufXaHjtv9 qfog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-type:content-transfer-encoding; bh=9ccCPfN5i2BcFcdxrvUzzvTVGIyPQ5ZvMJAywOSbSOk=; b=A5B5jTyYfAKFlbx+OnG2YRn42m3WXa+MgGGwAuijB95PFcOmNPkQkV5d2ZKgXn1ADt yDZKX1vVv4Uv145nYerUj2qylJNnOWWPpWTK4rtZggAOfFgIIZIUPm65Q9X3pU6GnC+3 xvJ7QIraWTcSvOvevFrLu3xdqs56WDnjIWwNomOTRc72jfyf3oowq3oy+zGmR9Zn/RwF b5+4oN90L9tmkTkv4jeUL4WqzDiTANmtLdoZ2vqzLklEXPTf4k5nxaF/1hKY+U7gq9Nv yIG9J6MiXovd/43OWc5XSkqQ9wc5U8WC8M0MltaSUKYD+hOZn4DFUmHK0kRTQRs5yCh6 M6Eg== X-Gm-Message-State: ALoCoQnelSa7hF9hzbq6jxPOLcprUeEq7fV7mqDC5ES0YIhmyvt4VXY5jHemqyHwczxiSBij9h+o X-Received: by 10.98.13.195 with SMTP id 64mr45870747pfn.147.1449511847829; Mon, 07 Dec 2015 10:10:47 -0800 (PST) Received: from xeon-e3 (static-50-53-82-155.bvtn.or.frontiernet.net. [50.53.82.155]) by smtp.gmail.com with ESMTPSA id c14sm36019534pfd.38.2015.12.07.10.10.47 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 07 Dec 2015 10:10:47 -0800 (PST) Date: Sat, 5 Dec 2015 11:50:07 -0800 From: Stephen Hemminger To: Yuanhan Liu Message-ID: <20151205115007.2230a16a@xeon-e3> In-Reply-To: <20151204032858.GB29571@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> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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: Mon, 07 Dec 2015 18:10:48 -0000 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. > > (And yes, it has nothing to do with your patch, I just found it we > can rename it to a better name to improve the code readability a bit. > If you agree, would you submit a patch, or should I do it?) > > > > > - /*Positive value indicates it need free vring descriptors */ > > + /* Positive value indicates it need free vring descriptors */ > > if (unlikely(need > 0)) { > > nb_used = VIRTQUEUE_NUSED(txvq); > > virtio_rmb(); > > need = RTE_MIN(need, (int)nb_used); > > > > virtio_xmit_cleanup(txvq, need); > > - need = (int)tx_pkts[nb_tx]->nb_segs - > > - txvq->vq_free_cnt + 1; > > + need = txm->nb_segs - txvq->vq_free_cnt + 1; > > + if (unlikely(need > 0)) { > > + PMD_TX_LOG(ERR, > > + "No free tx descriptors to transmit"); > > + break; > > + } > ^ > > Minor nit: I found a leading white space there. Hmm. I didn't see it in checkpatch.