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

[Tetsuya Mukawa]

On 2015/12/24 13:00, Yuanhan Liu wrote:
> 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 <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. 
>> 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

Thanks. Yes, I agree we need to provides such information through
"struct virtio_net".
And the callback handler needs to handle the vhost specific structure
directly.

Probably it's difficult to remove "struct virtio_net" from callback
handler of DPDK app.
This is the point I want to describe.

Tetsuya,