[dpdk-dev,v2,06/10] net/virtio: fix queue setup consistency
Checks
Commit Message
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)
Below are the steps to reproduce the issue:
#0. Checkout the commit
# 25bf7a0b0936 was applied after efc83a1e7fc3
git checkout 25bf7a0b0936
(There is another vector Rx bug caused by rxq flushing on the
HEAD. So it's better to checkout the old commit first.)
#1. Apply below patch to disable mergeable Rx, and build DPDK
#2. Launch testpmd/vhost-pmd:
./x86_64-native-linuxapp-gcc/app/testpmd -l 1,2 \
--socket-mem 1024,1024 \
--file-prefix=vhost \
--no-pci \
--vdev=net_vhost0,iface=/tmp/socket-0,queues=1 \
-- \
--port-topology=chained \
-i \
--nb-cores=1
#3. Launch testpmd/virtio-user:
./x86_64-native-linuxapp-gcc/app/testpmd -l 5,6 \
--socket-mem 1024,1024 \
--file-prefix=virtio-user \
--no-pci \
--vdev=net_virtio_user0,path=/tmp/socket-0 \
-- \
--port-topology=chained \
-i \
--nb-cores=1 \
--disable-hw-vlan \
--txqflags=0xf01
#4. In testpmd/virtio-user run below commands:
testpmd> set fwd rxonly
testpmd> start
#5. In testpmd/vhost-pmd run below commands:
testpmd> set burst 1
testpmd> set fwd rxonly
testpmd> start tx_first 1
testpmd> stop
#6. In testpmd/virtio-user run below commands:
testpmd> stop
testpmd> port stop all
testpmd> port start all
testpmd> start
#7. In testpmd/vhost-pmd run below commands:
testpmd> set fwd txonly
testpmd> start
#8. In testpmd/virtio-user run below commands:
testpmd> show port stats all
And you will see that there is no traffic any more after
receiving a few hundred packets.
[1] http://dpdk.org/ml/archives/dev/2017-December/082983.html
Best regards,
Tiwei Bie
Comments
Hi Tiwei,
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?
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
mkdir -p /mnt/huge
mount -t hugetlbfs none /mnt/huge
# term 1: testpmd/vhost-pmd
/root/dpdk.org/build/app/testpmd -l 1,2 \
--socket-mem 512,512 \
--file-prefix=vhost \
--no-pci \
--vdev=net_vhost0,iface=/tmp/socket-0,queues=1 \
-- \
--port-topology=chained \
-i \
--nb-cores=1
# term 2: virtio-user
/root/dpdk.org/build/app/testpmd -l 5,6 \
--socket-mem 512,512 \
--file-prefix=virtio-user \
--no-pci \
--vdev=net_virtio_user0,path=/tmp/socket-0 \
-- \
--port-topology=chained \
-i \
--nb-cores=1 \
--disable-hw-vlan \
--txqflags=0xf01
testpmd> set fwd rxonly
testpmd> start
# back to term1: vhost
testpmd> set burst 1
testpmd> set fwd rxonly
testpmd> start tx_first 1
testpmd> stop
Result on term1:
---------------------- Forward statistics for port 0 ----------------------
RX-packets: 0 RX-dropped: 0 RX-total: 0
TX-packets: 0 TX-dropped: 1 TX-total: 1
----------------------------------------------------------------------------
+++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
RX-packets: 0 RX-dropped: 0 RX-total: 0
TX-packets: 0 TX-dropped: 1 TX-total: 1
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Olivier
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
@@ -65,7 +65,6 @@
1u << VIRTIO_NET_F_CSUM | \
1u << VIRTIO_NET_F_HOST_TSO4 | \
1u << VIRTIO_NET_F_HOST_TSO6 | \
- 1u << VIRTIO_NET_F_MRG_RXBUF | \
1u << VIRTIO_NET_F_MTU | \
1u << VIRTIO_RING_F_INDIRECT_DESC | \
1ULL << VIRTIO_F_VERSION_1 | \