DPDK patches and discussions
 help / color / mirror / Atom feed
From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: Matan Azrad <matan@mellanox.com>, "dev@dpdk.org" <dev@dpdk.org>,
	"chenbo.xia@intel.com" <chenbo.xia@intel.com>,
	"yong.liu@intel.com" <yong.liu@intel.com>,
	"yinan.wang@intel.com" <yinan.wang@intel.com>
Cc: Thomas Monjalon <thomas@monjalon.net>,
	"ferruh.yigit@intel.com" <ferruh.yigit@intel.com>,
	"david.marchand@redhat.com" <david.marchand@redhat.com>
Subject: Re: [dpdk-dev] [PATCH v2 3/3] net/vhost: fix interrupt mode
Date: Wed, 29 Jul 2020 11:10:24 +0200
Message-ID: <9a5eeda1-8125-9fed-7ea3-5a7ce83c9394@redhat.com> (raw)
In-Reply-To: <DB3PR0502MB4028D506887D9BE6FA909762D2700@DB3PR0502MB4028.eurprd05.prod.outlook.com>



On 7/29/20 10:43 AM, Matan Azrad wrote:
> Hi Maxime
> 
> From: Maxime Coquelin:
>> On 7/28/20 9:03 PM, Matan Azrad wrote:
>>>
>>>
>>> From: Maxime Coquelin:
>>>> At .new_device() time, only the first vring pair is now ready, other
>>>> vrings are consfigured later.
>>>>
>>>> Problem is that when application will setup and enable interrupts,
>>>> only the first queue pair Rx interrupt will be enabled.
>>>>
>>>> This patches fixes the issue by setting the number of max interrupts
>>>> to the number of Rx queues that will be later initialized. Then, as
>>>> soon as a Rx vring is ready, it removes the corresponding
>>>> uninitialized epoll event, and install a new one with the valid FD.
>>>
>>> Doesn't it race condition to the application decision?
>>> App may change the configuration per queue in any time by the app
>> control thread.
>>> The vhost PMD may change it usynchronically from the vhost control
>> thread in the vring state callback.
>>
>> Yes you are right there could be a race here,I'm looking into getting it done in
>> a safe way. Yet it is good to get the confirmation from Intel that it does fix the
>> problem on their side.
> 
> Intel case\l3fw-power is only one case, we can put out the fire now by solving specific case but we need a fix for the global usage.
> 
>> Based on David suggestion, it might be made safe by relying on
>> eth_rxq_intr_enable()/eth_rxq_intr_disable().
> 
> Yes, maybe.
> 
> One more option to adjust the vhost PMD is to do the new_device callback logic when the last queue is ready as was done before,
> By this way, the vhost PMD will use the same timing as before.

Yes, that's one option but that would not be ideal because we would
lose the benefits of what the initial series brings, and it is not a
light change that late in the release.

Trying to fix interrupts, we mainly risk to break interrupt support
which is not widely used. Trying to wait for all queue event to be
received may cause other regressions in all use-cases.

>> If we cannot solve it in a safe way, then we'll have no other choice than
>> reverting partially your patch.
> 
> I'm sure we can find a solution.
> 
> As you probably remember we did the design for the readiness series together (a long discussion in other thread),
> It came to solve an architecture issue in QEMU last versions which might affect vhost lib users in other cases.
> We took it into account that some vhost lib users should do adjustment for the new behavior.
> 
> 
> Matan 
> 
>>
>> Maxime
>>
>>> I already mentioned it in other thread on this topic but didn't get reply.
>>>
>>>> Fixes: 604052ae5395 ("net/vhost: support queue update")
>>>>
>>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>
> 


  reply	other threads:[~2020-07-29  9:10 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-28 16:50 [dpdk-dev] [PATCH v2 0/3] Fix Vhost regressions Maxime Coquelin
2020-07-28 16:50 ` [dpdk-dev] [PATCH v2 1/3] vhost: fix guest notification setting Maxime Coquelin
2020-07-28 16:50 ` [dpdk-dev] [PATCH v2 2/3] net/vhost: fix queue update Maxime Coquelin
2020-07-29  2:52   ` Xia, Chenbo
2020-07-29  6:09     ` Wang, Yinan
2020-07-28 16:50 ` [dpdk-dev] [PATCH v2 3/3] net/vhost: fix interrupt mode Maxime Coquelin
2020-07-28 19:03   ` Matan Azrad
2020-07-29  7:52     ` Maxime Coquelin
2020-07-29  8:43       ` Matan Azrad
2020-07-29  9:10         ` Maxime Coquelin [this message]
2020-07-29  6:08 ` [dpdk-dev] [PATCH v2 0/3] Fix Vhost regressions Wang, Yinan

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=9a5eeda1-8125-9fed-7ea3-5a7ce83c9394@redhat.com \
    --to=maxime.coquelin@redhat.com \
    --cc=chenbo.xia@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=matan@mellanox.com \
    --cc=thomas@monjalon.net \
    --cc=yinan.wang@intel.com \
    --cc=yong.liu@intel.com \
    /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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git