From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <tiwei.bie@intel.com>
Received: from mga11.intel.com (mga11.intel.com [192.55.52.93])
 by dpdk.org (Postfix) with ESMTP id D4B3E7CE2;
 Fri,  8 Dec 2017 03:18:16 +0100 (CET)
Received: from orsmga007.jf.intel.com ([10.7.209.58])
 by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 07 Dec 2017 18:18:15 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.45,375,1508828400"; 
   d="scan'208";a="1000980"
Received: from deepin-15.sh.intel.com (HELO debian-xvivbkq) ([10.67.104.165])
 by orsmga007.jf.intel.com with ESMTP; 07 Dec 2017 18:18:12 -0800
Date: Fri, 8 Dec 2017 10:17:44 +0800
From: Tiwei Bie <tiwei.bie@intel.com>
To: Olivier MATZ <olivier.matz@6wind.com>
Cc: maxime.coquelin@redhat.com, yliu@fridaylinux.org,
 stephen@networkplumber.org, dev@dpdk.org, stable@dpdk.org,
 antonio.fischetti@intel.com
Message-ID: <20171208021744.7hoobdsao5xx4b2q@debian-xvivbkq>
References: <20170831134015.1383-1-olivier.matz@6wind.com>
 <20170907121347.16208-1-olivier.matz@6wind.com>
 <20170907121347.16208-7-olivier.matz@6wind.com>
 <20171206052528.xmxjxfkkurq7hhsp@debian-xvivbkq>
 <20171207141443.msbs6oqmwjav2lpk@glumotte.dev.6wind.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20171207141443.msbs6oqmwjav2lpk@glumotte.dev.6wind.com>
User-Agent: NeoMutt/20170609 (1.8.3)
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v2 06/10] net/virtio: fix queue
 setup consistency
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/stable>,
 <mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/stable>,
 <mailto:stable-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 08 Dec 2017 02:18:17 -0000

Hi Olivier,

On Thu, Dec 07, 2017 at 03:14:44PM +0100, Olivier MATZ wrote:
> On Wed, Dec 06, 2017 at 01:25:29PM +0800, Tiwei Bie wrote:
> > Hi Maxime and Olivier:
> > 
> > On Thu, Sep 07, 2017 at 02:13:43PM +0200, Olivier Matz wrote:
> > [...]
> > > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> > > index 8eee3ff80..c7888f103 100644
> > > --- a/drivers/net/virtio/virtio_ethdev.c
> > > +++ b/drivers/net/virtio/virtio_ethdev.c
> > > @@ -1737,6 +1737,19 @@ virtio_dev_start(struct rte_eth_dev *dev)
> > >  	struct virtnet_rx *rxvq;
> > >  	struct virtnet_tx *txvq __rte_unused;
> > >  	struct virtio_hw *hw = dev->data->dev_private;
> > > +	int ret;
> > > +
> > > +	/* Finish the initialization of the queues */
> > > +	for (i = 0; i < dev->data->nb_rx_queues; i++) {
> > > +		ret = virtio_dev_rx_queue_setup_finish(dev, i);
> > > +		if (ret < 0)
> > > +			return ret;
> > > +	}
> > 
> > I'm trying to fix an issue [1] reported by Antonio. And during
> > the debugging, I found that vector Rx of virtio PMD has been
> > broken (when doing port stop/start) since below two patches were
> > applied:
> > 
> > 25bf7a0b0936 ("vhost: make error handling consistent in Rx path")
> >    -- needed on the Tx side (testpmd/vhost-pmd in below test)
> > efc83a1e7fc3 ("net/virtio: fix queue setup consistency")
> >    -- needed on the Rx side (testpmd/virtio-user in below test)
> 
> Just to be sure I understand properly: each of these 2 patches
> break a different part your test case?
> 

Thank you for looking into this! ;-)

I mean the above test case won't pass when we have both
of them applied. And the first patch changes the Tx side,
and the second one changes the Rx side.

I haven't done thorough analysis on the first patch, so
I'm not sure what would be affected in the non-mergeable
Rx and vector Rx of virtio-PMD after changing the error
handling in vhost.

But I think there is something wrong with this patch (i.e.
the second patch). From my understanding, it seems that
virtio_rxq_rearm_vec() has an assumption that each time
it's called, the starting 'desc_idx' should be multiple
times of RTE_VIRTIO_VPMD_RX_REARM_THRESH (or 0). After
introducing virtio_dev_rx_queue_setup_finish() in device
start, the rxq will be fully refilled no matter where
the 'desc_idx' is after a device stop/start. And it could
break such assumption.

> I tried to reproduce your test case (the working case first):
> - on 0c4f909c17 (the commit before the efc83a1e7fc3)
> - without the patch disabling mergeable Rx
> 
> No packet is received. Am I doing something wrong? Please see the
> log:
> 
>    cd /root/dpdk.org
>    git checkout -b test 0c4f909c17
>    rm -rf build && make config T=x86_64-native-linuxapp-gcc && make -j32
>    insmod build/kmod/igb_uio.ko
>    echo 1000 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
>    echo 1000 > /sys/devices/system/node/node1/hugepages/hugepages-2048kB/nr_hugepages

Sorry, I forgot to mention that, 1G hugepage is required
to use virtio-user (2M hugepage won't work). For more
details about it, you could refer to the "Limitations"
section in below doc:

http://dpdk.org/doc/guides/howto/virtio_user_for_container_networking.html#limitations

Best regards,
Tiwei Bie