From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tetsuya Mukawa Subject: Re: [PATCH v5 2/3] vhost: Add VHOST PMD Date: Fri, 18 Dec 2015 18:25:47 +0900 Message-ID: <5673D11B.7030801@igel.co.jp> References: <1447392031-24970-3-git-send-email-mukawa@igel.co.jp> <1448355603-21275-1-git-send-email-mukawa@igel.co.jp> <1448355603-21275-3-git-send-email-mukawa@igel.co.jp> <20151218074512.GA18863@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, ann.zhuangyanying@huawei.com, yuanhan.liu@intel.com To: Yuanhan Liu , "Xie, Huawei" Return-path: Received: from mail-pf0-f180.google.com (mail-pf0-f180.google.com [209.85.192.180]) by dpdk.org (Postfix) with ESMTP id 2F8638D8D for ; Fri, 18 Dec 2015 10:25:51 +0100 (CET) Received: by mail-pf0-f180.google.com with SMTP id n128so28540059pfn.0 for ; Fri, 18 Dec 2015 01:25:51 -0800 (PST) In-Reply-To: <20151218074512.GA18863@yliu-dev.sh.intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 2015/12/18 16:45, Yuanhan Liu wrote: > On Tue, Nov 24, 2015 at 06:00:02PM +0900, Tetsuya Mukawa wrote: >> +static uint16_t >> +eth_vhost_rx(void *q, struct rte_mbuf **bufs, uint16_t nb_bufs) >> +{ >> + struct vhost_queue *r = q; >> + uint16_t i, nb_rx = 0; >> + >> + if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0)) >> + return 0; >> + >> + rte_atomic32_set(&r->while_queuing, 1); >> + >> + if (unlikely(rte_atomic32_read(&r->allow_queuing) == 0)) >> + goto out; > I'm not quite famililar with PMD driver yet, but as far as I know, > rte_eth_rx/tx_burst() does not provide any garantee on concurrent > dequeuing/queuing. If that's true, vhost pmd could (and should) > not do that, to keep the consistency. Yes you are correct, but this checking isn't for that purpose. I guess there is no rule that DPDK application should not call rte_eth_rx/tx_burst() when link status is down. So the application may call rte_eth_rx/tx_burst() even when vhost backend connection isn't established. Not to call rte_vhost_dequeue_burst() in the case, above variables are used. Tetsuya > On the other hand, rte_vhost_dequeue/enqueue_burst() already has > such support since the beginning: above check is redundant then. > However, FYI, Huawei has just (internally) proposed to remove > it, as he thinks that's application's duty. > > So, in neither way, we should not do that. > > --yliu