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 12:09:10 +0900 Message-ID: <567B61D6.1090806@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> <20151222034158.GH18863@yliu-dev.sh.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, ann.zhuangyanying@huawei.com To: Rich Lane , Yuanhan Liu Return-path: Received: from mail-pf0-f175.google.com (mail-pf0-f175.google.com [209.85.192.175]) by dpdk.org (Postfix) with ESMTP id ADE015A9D for ; Thu, 24 Dec 2015 04:09:10 +0100 (CET) Received: by mail-pf0-f175.google.com with SMTP id e65so9111688pfe.1 for ; Wed, 23 Dec 2015 19:09:10 -0800 (PST) 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 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. 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? Thanks, Tetsuya