From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yuanhan Liu Subject: Re: [PATCH 0/2] net/virtio: support to turn on/off the traffic flow Date: Wed, 19 Apr 2017 10:03:19 +0800 Message-ID: <20170419020319.GQ7333@yliu-dev.sh.intel.com> References: <1490960419-16779-1-git-send-email-zhiyong.yang@intel.com> <20170406035953.GM18844@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "dev@dpdk.org" , "maxime.coquelin@redhat.com" To: "Yang, Zhiyong" Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 3D4031077 for ; Wed, 19 Apr 2017 04:06:29 +0200 (CEST) Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Mon, Apr 17, 2017 at 08:50:52AM +0000, Yang, Zhiyong wrote: > Hi, yuanhan: > Sorry for the delay reply due to my annual leave. > > > -----Original Message----- > > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] > > Sent: Thursday, April 6, 2017 12:00 PM > > To: Yang, Zhiyong > > Cc: dev@dpdk.org; maxime.coquelin@redhat.com > > Subject: Re: [PATCH 0/2] net/virtio: support to turn on/off the traffic flow > > > > On Fri, Mar 31, 2017 at 07:40:17PM +0800, Zhiyong Yang wrote: > > > Current dpdk code virtio_dev_stop only disables interrupt and marks > > > link down, When it is invoked, tx/rx traffic flows still work. This is a strange > > behavior. > > > The patchset supports the switch of flow by calling virtio_dev_start/stop. > > > > > > The implementation refers to vhost pmd. > > > > That's a difference story. Vhost pmd uses 2 vars to track the status, whereas you > > are using only one here. So why not setting/clearing "started" at dev_start/stop, > > respectively? > > Then we can check "started" at Rx/Tx functions. > > Yes, I use only one var since I think vhost pmd using two is too complex and it is unnecessary. No, it's needed. For vhost-user pmd, we can only do Rx when both below items are met: - port is started - new_device() is invoked, aka, the device is connected For that reason, two vars is used to track it. > I'm setting/clearing started at virtio_dev_start/stop, update_queuing_status is added to avoid > duplicate code. It's not about duplicate code. While we could make the var per-device, you make it per-queue. That's complex and unnecessary. > I don't understand your question. > > > > > BTW, why does it have to be atomic? > > > > Consider again. It is not necessary to use atomic here. But It seems that it doesn't have an negative effect. Hmm... that's a good reason to keep it, just because it has no negative effect? Talking about the negative effect, badly, it really has. The atomic is more expensive. --yliu