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