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: Thu, 24 Dec 2015 11:51:05 +0800 Message-ID: <20151224035105.GA18863@yliu-dev.sh.intel.com> References: <1448355603-21275-2-git-send-email-mukawa@igel.co.jp> <20151223024453.GW18863@yliu-dev.sh.intel.com> <1689693.Nk6ErSWZrF@xps13> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org, ann.zhuangyanying@huawei.com To: Thomas Monjalon Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id BF5AECE7 for ; Thu, 24 Dec 2015 04:49:34 +0100 (CET) Content-Disposition: inline In-Reply-To: <1689693.Nk6ErSWZrF@xps13> 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 Wed, Dec 23, 2015 at 11:00:15PM +0100, Thomas Monjalon wrote: > 2015-12-23 10:44, Yuanhan Liu: > > 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: > > > > > > 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. > > > > > > Again I'd ask, will vring_state_changed() be enough, when above issues > > > are resolved: vring_state_changed() will be invoked at new_device()/ > > > destroy_device(), and of course, ethtool change? > > > > > > > > > It would be sufficient. It is not a great API though, because it requires 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 actually > > > changed (enabled -> enabled and disabled -> disabled). > > > > > > If you're asking about using vring_state_changed() _instead_ of the link 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? > > Yes it is an event. So I don't understand the question. > What may be better than a specific rte_eth_event_type? The alternative is a new set of callbacks, but judging that we already have a set of callback for vhost libraray, and adding a new set to vhost pmd doesn't seem elegant to me. Therefore, I'd prefer a new eth event. --yliu