DPDK patches and discussions
 help / color / mirror / Atom feed
From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: "Yao, Lei A" <lei.a.yao@intel.com>, "dev@dpdk.org" <dev@dpdk.org>,
	"yliu@fridaylinux.org" <yliu@fridaylinux.org>
Cc: "Horton, Remy" <remy.horton@intel.com>,
	"Bie, Tiwei" <tiwei.bie@intel.com>,
	"mst@redhat.com" <mst@redhat.com>,
	"jfreiman@redhat.com" <jfreiman@redhat.com>,
	"vkaplans@redhat.com" <vkaplans@redhat.com>,
	"jasowang@redhat.com" <jasowang@redhat.com>
Subject: Re: [dpdk-dev] [PATCH v3 17/19] vhost-user: iommu: postpone device creation until ring are mapped
Date: Thu, 2 Nov 2017 17:02:57 +0100	[thread overview]
Message-ID: <4054d863-8909-23a7-aba2-b8675cdbb4ba@redhat.com> (raw)
In-Reply-To: <8d27d7d9-9567-1a57-5a5f-760f2f117b73@redhat.com>



On 11/02/2017 09:21 AM, Maxime Coquelin wrote:
> Hi Lei,
> 
> On 11/02/2017 08:21 AM, Yao, Lei A wrote:
>>
> ...
>> Hi, Maxime > I met one issue with your patch set during the v17.11 test.
> 
> Is it with v17.11-rc2 or -rc1?
> 
>> The test scenario is following,
>> 1.    Bind one NIC, use test-pmd set vhost-user with 2 queue
>> usertools/dpdk-devbind.py --bind=igb_uio 0000:05:00.0
>> ./x86_64-native-linuxapp-gcc/app/testpmd -c 0xe -n 4 --socket-mem 
>> 1024,1024 \
>> --vdev 'net_vhost0,iface=vhost-net,queues=2' - -i --rxq=2 --txq=2 
>> --nb-cores=2 --rss-ip
>> 2.    Launch qemu with  virtio device which has 2 queue
>> 3.    In VM, launch testpmd with virtio-pmd using only 1 queue.
>> x86_64-native-linuxapp-gcc/app/testpmd -c 0x07 -n 3 - -i 
>> --txqflags=0xf01 \
>> --rxq=1 --txq=1 --rss-ip --nb-cores=1
>>
>> First,
>> commit 09927b5249694bad1c094d3068124673722e6b8f
>> vhost: translate ring addresses when IOMMU enabled
>> The patch causes no traffic in PVP test. but link status is still up 
>> in vhost-user.
>>
>> Second,
>> eefac9536a901a1f0bb52aa3b6fec8f375f09190
>> vhost: postpone device creation until rings are mapped
>> The patch causes link status "down" in vhost-user.

I reproduced this one, and understand why link status remains down.
My series did fixed a potential issue Michael raised, that the vring
addresses should only interpreted once the ring is enabled.
When VHOST_USER_F_PROTOCOL_FEATURES is negotiated, the rings addrs are
translated when ring is enabled via VHOST_USER_SET_VRING_ENABLE.
When not negotiated, the ring is considered started enabled, so
translation is done at VHOST_USER_SET_VRING_KICK time.

In your case, protocol features are negotiated, so the ring addresses
are translated at enable time. The problem is that the code considers
the device is ready once addresses for all the rings are translated.
But since only the first pair of rings is used, it never happens, and
the link remains down.

One of the reason this check is done is to avoid starting the PMD
threads before the addresses are translated in case of NUMA
reallocation, as virtqueues and virtio-net device structs can be
reallocated on a different node.

I think the right fix would be to only perform NUMA reallocation for
vring 0, as today we would end-up reallocating virtio-net struct
mulitple time if VQs are on different NUMA nodes.

Doing that, we could then consider the device is ready if vring 0 is
enabled and its ring addresses are translated, and if other vrings have
been kicked.

I'll post a patch shortly implementing this idea.

Thanks,
Maxime

  reply	other threads:[~2017-11-02 16:03 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-05  8:36 [dpdk-dev] [PATCH v3 00/19] Vhost-user: Implement device IOTLB support Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 01/19] Revert "vhost: workaround MQ fails to startup" Maxime Coquelin
2017-11-01 17:11   ` Kavanagh, Mark B
2017-11-02  9:40     ` Maxime Coquelin
2017-11-03 13:05       ` Yuanhan Liu
2017-11-03 14:28         ` Maxime Coquelin
2017-11-06 12:00           ` Yuanhan Liu
2017-11-06 12:07             ` Maxime Coquelin
2017-11-06 12:24               ` Yuanhan Liu
2017-11-06 12:50                 ` Maxime Coquelin
2017-11-06 13:36                   ` Yuanhan Liu
2017-11-03 15:34       ` Thomas Monjalon
2017-11-03 16:31         ` Kavanagh, Mark B
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 02/19] vhost: make error handling consistent in rx path Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 03/19] vhost: prepare send_vhost_message() to slave requests Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 04/19] vhost: add support to slave requests channel Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 05/19] vhost: declare missing IOMMU-related definitions for old kernels Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 06/19] vhost: add iotlb helper functions Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 07/19] vhost: iotlb: add pending miss request list and helpers Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 08/19] vhost-user: add support to IOTLB miss slave requests Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 09/19] vhost: initialize vrings IOTLB caches Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 10/19] vhost-user: handle IOTLB update and invalidate requests Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 11/19] vhost: introduce guest IOVA to backend VA helper Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 12/19] vhost: use the guest IOVA to host " Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 13/19] vhost: enable rings at the right time Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 14/19] vhost: don't dereference invalid dev pointer after its reallocation Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 15/19] vhost: postpone rings addresses translation Maxime Coquelin
2017-10-13  1:47   ` Yao, Lei A
2017-10-13  7:32     ` Maxime Coquelin
2017-10-13  7:55       ` Yao, Lei A
2017-10-13  7:56         ` Maxime Coquelin
2017-10-16  5:59         ` Yao, Lei A
2017-10-16  6:23           ` Yao, Lei A
2017-10-16  9:47             ` Maxime Coquelin
2017-10-16 10:47               ` Maxime Coquelin
2017-10-17  1:24                 ` Yao, Lei A
2017-10-17  8:06                   ` Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 16/19] vhost-user: translate ring addresses when IOMMU enabled Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 17/19] vhost-user: iommu: postpone device creation until ring are mapped Maxime Coquelin
2017-11-02  7:21   ` Yao, Lei A
2017-11-02  8:21     ` Maxime Coquelin
2017-11-02 16:02       ` Maxime Coquelin [this message]
2017-11-03  8:25         ` Maxime Coquelin
2017-11-03 15:15           ` Michael S. Tsirkin
2017-11-03 15:54             ` Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 18/19] vhost: iommu: Invalidate vring in case of matching IOTLB invalidate Maxime Coquelin
2017-10-05  8:36 ` [dpdk-dev] [PATCH v3 19/19] vhost: enable IOMMU support Maxime Coquelin
2017-10-06  6:24 ` [dpdk-dev] [PATCH v3 00/19] Vhost-user: Implement device IOTLB support Yuanhan Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4054d863-8909-23a7-aba2-b8675cdbb4ba@redhat.com \
    --to=maxime.coquelin@redhat.com \
    --cc=dev@dpdk.org \
    --cc=jasowang@redhat.com \
    --cc=jfreiman@redhat.com \
    --cc=lei.a.yao@intel.com \
    --cc=mst@redhat.com \
    --cc=remy.horton@intel.com \
    --cc=tiwei.bie@intel.com \
    --cc=vkaplans@redhat.com \
    --cc=yliu@fridaylinux.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).