From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yuanhan Liu Subject: Re: [PATCH v5 1/3] vhost: Add callback and private data for vhost PMD Date: Wed, 23 Dec 2015 10:44:53 +0800 Message-ID: <20151223024453.GW18863@yliu-dev.sh.intel.com> References: <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> <20151222034158.GH18863@yliu-dev.sh.intel.com> <20151222054710.GK18863@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable Cc: dev@dpdk.org, ann.zhuangyanying@huawei.com To: Rich Lane , Thomas Monjalon Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 3B361B62 for ; Wed, 23 Dec 2015 03:43:51 +0100 (CET) Content-Disposition: inline In-Reply-To: List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, Dec 22, 2015 at 01:38:29AM -0800, Rich Lane wrote: > On Mon, Dec 21, 2015 at 9:47 PM, Yuanhan Liu > wrote: >=20 > On Mon, Dec 21, 2015 at 08:47:28PM -0800, Rich Lane wrote: > > The queue state change callback is the one new API that needs to = be > > added because > > normal NICs don't have this behavior. >=20 > Again I'd ask, will vring_state_changed() be enough, when above iss= ues > are resolved: vring_state_changed() will be invoked at new_device()= / > destroy_device(), and of course, ethtool change? >=20 >=20 > It would be sufficient. It is not a great API though, because it requir= es the > application to do the conversion from struct virtio_net to a DPDK port = number, > and from a virtqueue index to a DPDK queue id and direction. Also, the = current > implementation often makes this callback when the vring state has not a= ctually > changed (enabled -> enabled and disabled -> disabled). >=20 > If you're asking about using vring_state_changed() _instead_ of the lin= k status > event and rte_eth_dev_socket_id(), No, I like the idea of link status event and rte_eth_dev_socket_id(); I was just wondering why a new API is needed. Both Tetsuya and I were thinking to leverage the link status event to represent the queue stats change (triggered by vring_state_changed()) as well, so that we don't need to introduce another eth event. However, I'd agree that it's better if we could have a new dedicate event. Thomas, here is some background for you. For vhost pmd and linux virtio-net combo, the queue can be dynamically changed by ethtool, therefore, the application wishes to have another eth event, say RTE_ETH_EVENT_QUEUE_STATE_CHANGE, so that the application can add/remove corresponding queue to the datapath when that happens. What do you think of that? > then yes, it still works. I'd only consider > that a stopgap until the real ethdev APIs are implemented. >=20 > I'd suggest to add=A0RTE_ETH_EVENT_QUEUE_STATE_CHANGE rather than > create another callback registration API. >=20 > Perhaps we could merge the basic PMD which I think is pretty solid and = then > continue the API discussion with patches to it. Perhaps, but let's see how hard it could be for the new eth event discussion then. --yliu