From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 0D04E5A4F for ; Tue, 22 Dec 2015 05:35:41 +0100 (CET) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 21 Dec 2015 20:35:34 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,462,1444719600"; d="scan'208";a="876474351" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49]) by orsmga002.jf.intel.com with ESMTP; 21 Dec 2015 20:35:33 -0800 Date: Tue, 22 Dec 2015 12:36:45 +0800 From: Yuanhan Liu To: Tetsuya Mukawa , "Xie, Huawei" Message-ID: <20151222043645.GI18863@yliu-dev.sh.intel.com> 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> <56775F82.3090306@igel.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56775F82.3090306@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: Tue, 22 Dec 2015 04:35:42 -0000 On Mon, Dec 21, 2015 at 11:10:10AM +0900, Tetsuya Mukawa wrote: > nes: 168 > > On 2015/12/19 3:01, Rich Lane wrote: > > I'm using the vhost callbacks and struct virtio_net with the vhost PMD in a > > few ways: > > > > 1. new_device/destroy_device: Link state change (will be covered by the > > link status interrupt). > > 2. new_device: Add first queue to datapath. > > 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). > > 5. new_device and struct virtio_net: Determine NUMA node of the VM. > > > > The vring_state_changed callback is necessary because the VM might not be > > using the maximum number of RX queues. If I boot Linux in the VM it will > > start out using one RX queue, which can be changed with ethtool. The DPDK > > app in the host needs to be notified that it can start sending traffic to > > the new queue. > > > > The vring_state_changed callback is also useful for guest TX queues to > > avoid reading from an inactive queue. > > > > API I'd like to have: > > > > 1. Link status interrupt. > > 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. > > 3. Per-queue or per-device NUMA node info. > > Hi Rich and Yuanhan, > > As Rich described, some users needs more information when the interrupts > comes. > And the virtio_net structure contains the information. > > I guess it's very similar to interrupt handling of normal hardware. > First, a interrupt comes, then an interrupt handler checks status > register of the device to know actually what was happened. > In vhost PMD case, reading status register equals reading virtio_net > structure. > > So how about below specification? > > 1. The link status interrupt of vhost PMD will occurs when new_device, > destroy_device and vring_state_changed events are happened. > 2. Vhost PMD provides a function to let the users know virtio_net > structure of the interrupted port. > (Probably almost same as "rte_eth_vhost_portid2vdev" that I described > in "[PATCH v5 3/3] vhost: Add helper function to convert port id to > virtio device pointer") That is one option, to wrapper everything into the link status interrupt handler, and let it to query the virtio_net structure to know what exactly happened, and then take the proper action. With that, we could totally get rid of the "two sets of vhost callbacks". The interface is a also clean. However, there is a drawback: it's not that extensible: what if vhost introduces a new vhost callback, and it does not literally belong to a link status interrupt event? The another options is to introduce a new set of callbacks (not based on vhost, but based on vhost-pmd as I suggested before). Here we could rename the callback to make it looks more reasonable to a pmd driver, say, remove the new_device()/destroy_device() and combine them as one callback: link_status_changed. The good thing about that is it's extensible; we could easily add a new callback when there is a new one at vhost. However, it's not that clean as the first one. Besides that, two sets of callback for vhost is always weird to me. Thoughts, comments? --yliu