From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id C0346902 for ; Thu, 24 Dec 2015 04:59:00 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP; 23 Dec 2015 19:59:01 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,472,1444719600"; d="scan'208";a="623054700" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49]) by FMSMGA003.fm.intel.com with ESMTP; 23 Dec 2015 19:59:00 -0800 Date: Thu, 24 Dec 2015 12:00:30 +0800 From: Yuanhan Liu To: Tetsuya Mukawa Message-ID: <20151224040030.GB18863@yliu-dev.sh.intel.com> References: <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> <20151222034158.GH18863@yliu-dev.sh.intel.com> <567B61D6.1090806@igel.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <567B61D6.1090806@igel.co.jp> User-Agent: Mutt/1.5.23 (2014-03-12) 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Dec 2015 03:59:01 -0000 On Thu, Dec 24, 2015 at 12:09:10PM +0900, Tetsuya Mukawa wrote: > On 2015/12/22 13:47, Rich Lane wrote: > > On Mon, Dec 21, 2015 at 7:41 PM, Yuanhan Liu > > wrote: > > > >> On Fri, Dec 18, 2015 at 10:01:25AM -0800, Rich Lane wrote: > >>> I'm using the vhost callbacks and struct virtio_net with the vhost PMD > >> in a few > >>> ways: > >> Rich, thanks for the info! > >> > >>> 1. new_device/destroy_device: Link state change (will be covered by the > >> link > >>> status interrupt). > >>> 2. new_device: Add first queue to datapath. > >> I'm wondering why vring_state_changed() is not used, as it will also be > >> triggered at the beginning, when the default queue (the first queue) is > >> enabled. > >> > > Turns out I'd misread the code and it's already using the > > vring_state_changed callback for the > > first queue. Not sure if this is intentional but vring_state_changed is > > called for the first queue > > before new_device. > > > > > >>> 3. vring_state_changed: Add/remove queue to datapath. > >>> 4. destroy_device: Remove all queues (vring_state_changed is not called > >> when > >>> qemu is killed). > >> I had a plan to invoke vring_state_changed() to disable all vrings > >> when destroy_device() is called. > >> > > That would be good. > > > > > >>> 5. new_device and struct virtio_net: Determine NUMA node of the VM. > >> You can get the 'struct virtio_net' dev from all above callbacks. > > > > > >> 1. Link status interrupt. > >> > >> To vhost pmd, new_device()/destroy_device() equals to the link status > >> interrupt, where new_device() is a link up, and destroy_device() is link > >> down(). > >> > >> > >>> 2. New queue_state_changed callback. Unlike vring_state_changed this > >> should > >>> cover the first queue at new_device and removal of all queues at > >>> destroy_device. > >> As stated above, vring_state_changed() should be able to do that, except > >> the one on destroy_device(), which is not done yet. > >> > >>> 3. Per-queue or per-device NUMA node info. > >> You can query the NUMA node info implicitly by get_mempolicy(); check > >> numa_realloc() at lib/librte_vhost/virtio-net.c for reference. > >> > > Your suggestions are exactly how my application is already working. I was > > commenting on the > > proposed changes to the vhost PMD API. I would prefer to > > use RTE_ETH_EVENT_INTR_LSC > > and rte_eth_dev_socket_id for consistency with other NIC drivers, instead > > of these vhost-specific > > hacks. The queue state change callback is the one new API that needs to be > > added because > > normal NICs don't have this behavior. > > > > You could add another rte_eth_event_type for the queue state change > > callback, and pass the > > queue ID, RX/TX direction, and enable bit through cb_arg. > > Hi Rich, > > So far, EAL provides rte_eth_dev_callback_register() for event handling. > DPDK app can register callback handler and "callback argument". > And EAL will call callback handler with the argument. > Anyway, vhost library and PMD cannot change the argument. Yes, the event callback argument is provided from application, where it has no way to know info you mentioned above, like queue ID, RX/TX direction. For that, we may need introduce another structure to note all above info, and embed it to virtio_net struct. > I guess the callback handler will need to call ethdev APIs to know what > causes the interrupt. > Probably rte_eth_dev_socket_id() is to know numa_node, and > rte_eth_dev_info_get() is to know the number of queues. > Is this okay for your case? I don't think that's enough, and I think Rich has already given all info needed to a queue change interrupt to you. (That would also answer your questions in another email) --yliu