From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <rich.lane@bigswitch.com>
Received: from mail-vk0-f45.google.com (mail-vk0-f45.google.com
 [209.85.213.45]) by dpdk.org (Postfix) with ESMTP id 23D708F9E
 for <dev@dpdk.org>; Mon, 28 Dec 2015 23:00:00 +0100 (CET)
Received: by mail-vk0-f45.google.com with SMTP id a188so185849424vkc.0
 for <dev@dpdk.org>; Mon, 28 Dec 2015 14:00:00 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=bigswitch-com.20150623.gappssmtp.com; s=20150623;
 h=mime-version:in-reply-to:references:date:message-id:subject:from:to
 :cc:content-type;
 bh=W740lYcC/DHWAIn1jKFLHkf+wOiYMObhPduGc2mAGJY=;
 b=BwdPBb1HJquO3fLOCR8/QIld01/ILNIEHON10Wqke+YMjfi+e8OQYIGmAm6BMV8QD1
 72KrftlnWCxpDmnKLwVtNYfD+PXnkm6BYQy9iA2tbXjNQ6TR6diDBpOSeu5XkKt6lodp
 HinxpnzYW6Sw7IGBD5desCJ/DhX5FFm2hK60wGSu2dHqH1pnVySMEAD/I1yQmYJxBZNk
 ZeirNkCmDQgp7/HtbxEJdiFvivXWeiP0gMN7DNq2OuaRC04RR/rBQIX4A0VxMP07BUHA
 U75akIM5R4AZU9AE2xeXeE8+ce29l2NPsq6VXL2m5/SYbnLkwCHxmhMuVS/TZj0v5ENY
 C5DQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20130820;
 h=x-gm-message-state:mime-version:in-reply-to:references:date
 :message-id:subject:from:to:cc:content-type;
 bh=W740lYcC/DHWAIn1jKFLHkf+wOiYMObhPduGc2mAGJY=;
 b=ebNynA66MDW4GxDl4efgRiSla9IZ+CmNrP1ifWXdbNLDJ87WpwSYmPloapyYNcUCPZ
 QdYjZDf+9d6M/ex0nuZ5rJKXh8E1PyHYoWpjCw8ZUoWabkJw5Eq4/tuRkwSnctJPCIJR
 yeXQWkjGHIvnZz44y/Bmi+rPGrTJjFS/eNk7XEhfQCG26exPgsbmNGaE7k3Dm6sqDH9m
 mQZ4E1ABbIlW8QFcER/rS5aByskF93GpHx5I2oV2Mim9kogLghOt0enCE7srcCFbKSWG
 o+uD28RcLFRqU3wbc+orIWFQjFdOP2Lz9QGREqN3Z6YbhFblbyCfQNgqhphef+3tS8pa
 ScQw==
X-Gm-Message-State: ALoCoQkfJ7O6JKgMB1njhoSau1rtTzrV8hVF+o/fw3V5dCBCL8bbVdkqCxx6/1oproqzPmM0666hn60N3A+QGBBZktJrNTvDxORddaEKADi2YclIHG7fdls=
MIME-Version: 1.0
X-Received: by 10.31.162.22 with SMTP id l22mr37587620vke.76.1451339999548;
 Mon, 28 Dec 2015 13:59:59 -0800 (PST)
Received: by 10.31.129.9 with HTTP; Mon, 28 Dec 2015 13:59:59 -0800 (PST)
In-Reply-To: <567BA5B9.5090006@igel.co.jp>
References: <1447392031-24970-3-git-send-email-mukawa@igel.co.jp>
 <1448355603-21275-1-git-send-email-mukawa@igel.co.jp>
 <1448355603-21275-2-git-send-email-mukawa@igel.co.jp>
 <20151217114223.GC29571@yliu-dev.sh.intel.com>
 <56737A5E.1030803@igel.co.jp>
 <20151218041536.GI29571@yliu-dev.sh.intel.com>
 <56738B5C.1030206@igel.co.jp>
 <CAGSMBPPGs8b3sGAn6A2ETaau70oeChouTOnj=Ah2sgjz_1p_nA@mail.gmail.com>
 <20151222034158.GH18863@yliu-dev.sh.intel.com>
 <CAGSMBPMHhCOmRJ9aQEmdtBeBBcokztmdHyXC5BBnQ-rXF0OkYg@mail.gmail.com>
 <567B61D6.1090806@igel.co.jp>
 <CAGSMBPMeHByvncdZWbi0mCc2B1CczH005jKQvewe9fwNMFD_uQ@mail.gmail.com>
 <567BA5B9.5090006@igel.co.jp>
Date: Mon, 28 Dec 2015 13:59:59 -0800
Message-ID: <CAGSMBPMk6Mp9=NTECQBGLb5axDH6eN5k-U19X_mLzbUMiLf9Vw@mail.gmail.com>
From: Rich Lane <rich.lane@bigswitch.com>
To: Tetsuya Mukawa <mukawa@igel.co.jp>
Content-Type: text/plain; charset=UTF-8
X-Content-Filtered-By: Mailman/MimeDel 2.1.15
Cc: dev@dpdk.org, ann.zhuangyanying@huawei.com
Subject: Re: [dpdk-dev] [PATCH v5 1/3] vhost: Add callback and private data
 for vhost PMD
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 28 Dec 2015 22:00:00 -0000

On Wed, Dec 23, 2015 at 11:58 PM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
>
> Hi Rich and Yuanhan,
>
> I guess we have 2 implementations here.
>
> 1. rte_eth_vhost_get_queue_event() returns each event.
> 2. rte_eth_vhost_get_queue_status() returns current status of the queues.
>
> I guess option "2" is more generic manner to handle interrupts from
> device driver.
> In the case of option "1", if DPDK application doesn't call
> rte_eth_vhost_get_queue_event(), the vhost PMD needs to keep all events.
> This may exhaust memory.
>

Option 1 can be implemented in constant space by only tracking the latest
state of each
queue. I pushed a rough implementation to https://github.com/rlane/dpdk
vhost-queue-callback.

One more example is current link status interrupt handling.
> Actually ethdev API just returns current status of the port.
> What do you think?
>

Option 2 adds a small burden to the application but I'm fine with this as
long as it's
thread-safe (see my comments below).


> > An issue with having the application dig into struct virtio_net is that
> it
> > can only be safely accessed from
> > a callback on the vhost thread.
>
> Here is one of example how to invoke a callback handler registered by
> DPDK application from the PMD.
>
>   _rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
>
> Above function is called by interrupt handling thread of the PMDs.
>
> Please check implementation of above function.
> A callback handler that DPDK application registers is called in
> "interrupt handling context".
> (I mean the interrupt handling thread of the PMD calls the callback
> handler of DPDK application also.)
> Anyway, I guess the callback handler of DPDK application can access to
> "struct virtio_net" safely.


> > A typical application running its control
> > plane on lcore 0 would need to
> > copy all the relevant info from struct virtio_net before sending it over.
>
> Could you please describe it more?
> Sorry, probably I don't understand correctly which restriction make you
> copy data.
> (As described above, the callback handler registered by DPDK application
> can safely access "to struct virtio_net". Does this solve the copy issue?)
>

The ethdev event callback can safely access struct virtio_net, yes. The
problem is
that a real application will likely want to handle queue state changes as
part
of its main event loop running on a separate thread. Because no other thread
can safely access struct virtio_net. the callback would need to copy the
queue
states out of struct virtio_net into another datastructure before sending
it to
the main thread.

Instead of having the callback muck around with struct virtio_net, I would
prefer
an API that I could call from any thread to get the current queue states.
This
also removes struct virtio_net from the PMD's API which I think is a win.


> > As you mentioned, queues for a single vhost port could be located on
> > different NUMA nodes. I think this
> > is an uncommon scenario but if needed you could add an API to retrieve
> the
> > NUMA node for a given port
> > and queue.
> >
>
> I agree this is very specific for vhost, because in the case of generic
> PCI device, all queues of a port are on same NUMA node.
> Anyway, because it's very specific for vhost, I am not sure we should
> add ethdev API to handle this.
>
> If we handle it by vhost PMD API, we probably have 2 options also here.
>
> 1. Extend "struct rte_eth_vhost_queue_event , and use
> rte_eth_vhost_get_queue_event() like you described.
> struct rte_eth_vhost_queue_event
> {
>         uint16_t queue_id;
>         bool rx;
>         bool enable;
> +      int socket_id;
> };
>
> 2. rte_eth_vhost_get_queue_status() returns current socket_ids of all
> queues.
>

Up to you, but I think we can skip this for the time being because it would
be unusual
for a guest to place virtqueues for one PCI device on different NUMA nodes.