From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tetsuya Mukawa Subject: Re: [PATCH v5 1/3] vhost: Add callback and private data for vhost PMD Date: Thu, 24 Dec 2015 13:07:19 +0900 Message-ID: <567B6F77.8010909@igel.co.jp> References: <1448355603-21275-2-git-send-email-mukawa@igel.co.jp> <20151223024453.GW18863@yliu-dev.sh.intel.com> <1689693.Nk6ErSWZrF@xps13> <20151224035105.GA18863@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, ann.zhuangyanying@huawei.com To: Yuanhan Liu , Thomas Monjalon , Rich Lane Return-path: Received: from mail-pa0-f45.google.com (mail-pa0-f45.google.com [209.85.220.45]) by dpdk.org (Postfix) with ESMTP id 849815A0A for ; Thu, 24 Dec 2015 05:07:19 +0100 (CET) Received: by mail-pa0-f45.google.com with SMTP id jx14so115682681pad.2 for ; Wed, 23 Dec 2015 20:07:19 -0800 (PST) In-Reply-To: <20151224035105.GA18863@yliu-dev.sh.intel.com> 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 2015/12/24 12:51, Yuanhan Liu wrote: > 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 I am ok to have one more event type. BTW, I have questions about numa_node. I guess "rte_eth_dev_socket_id()" can only return numa_node of specified port. If multiple queues are used in one device(port), can we say all queues are always in same numa_node? If the answer is no, I am still not sure we can remove "struct virtio_net" from DPDK application callback handling. I agree we can add RTE_ETH_EVENT_QUEUE_STATE_CHANGE for interrupt notice. But current ethdev APIs may not be able to hide vhost specific properties, then the callback hander needs to handle "struct virtio_net" directly. Thanks, Tetsuya