[dpdk-dev] [PATCH v5 1/3] vhost: Add callback and private data for vhost PMD

[Rich Lane]

On Mon, Dec 21, 2015 at 7:41 PM, Yuanhan Liu <yuanhan.liu at linux.intel.com>
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. The application
would never need
to touch struct virtio_net.