Re: [dpdk-dev] dpdk/vpp and cross-version migration for vhost

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 11/24/2016 04:24 PM, Kavanagh, Mark B wrote:
>>
>> On 11/24/2016 12:47 PM, Maxime Coquelin wrote:
>>>
>>>
>>> On 11/24/2016 01:33 PM, Yuanhan Liu wrote:
>>>> On Thu, Nov 24, 2016 at 09:30:49AM +0000, Kevin Traynor wrote:
>>>>>> On 11/24/2016 06:31 AM, Yuanhan Liu wrote:
>>>>>>>> On Tue, Nov 22, 2016 at 04:53:05PM +0200, Michael S. Tsirkin wrote:
>>>>>>>>>>>>>> You keep assuming that you have the VM started first and
>>>>>>>>>>>>>> figure out things afterwards, but this does not work.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Think about a cluster of machines. You want to start a VM in
>>>>>>>>>>>>>> a way that will ensure compatibility with all hosts
>>>>>>>>>>>>>> in a cluster.
>>>>>>>>>>>>
>>>>>>>>>>>> I see. I was more considering about the case when the dst
>>>>>>>>>>>> host (including the qemu and dpdk combo) is given, and
>>>>>>>>>>>> then determine whether it will be a successfull migration
>>>>>>>>>>>> or not.
>>>>>>>>>>>>
>>>>>>>>>>>> And you are asking that we need to know which host could
>>>>>>>>>>>> be a good candidate before starting the migration. In such
>>>>>>>>>>>> case, we indeed need some inputs from both the qemu and
>>>>>>>>>>>> vhost-user backend.
>>>>>>>>>>>>
>>>>>>>>>>>> For DPDK, I think it could be simple, just as you said, it
>>>>>>>>>>>> could be either a tiny script, or even a macro defined in
>>>>>>>>>>>> the source code file (we extend it every time we add a
>>>>>>>>>>>> new feature) to let the libvirt to read it. Or something
>>>>>>>>>>>> else.
>>>>>>>>>>
>>>>>>>>>> There's the issue of APIs that tweak features as Maxime
>>>>>>>>>> suggested.
>>>>>>>>
>>>>>>>> Yes, it's a good point.
>>>>>>>>
>>>>>>>>>> Maybe the only thing to do is to deprecate it,
>>>>>>>>
>>>>>>>> Looks like so.
>>>>>>>>
>>>>>>>>>> but I feel some way for application to pass info into
>>>>>>>>>> guest might be benefitial.
>>>>>>>>
>>>>>>>> The two APIs are just for tweaking feature bits DPDK supports
>>>>>> before
>>>>>>>> any device got connected. It's another way to disable some features
>>>>>>>> (the another obvious way is to through QEMU command lines).
>>>>>>>>
>>>>>>>> IMO, it's bit handy only in a case like: we have bunch of VMs.
>>>>>> Instead
>>>>>>>> of disabling something though qemu one by one, we could disable it
>>>>>>>> once in DPDK.
>>>>>>>>
>>>>>>>> But I doubt the useful of it. It's only used in DPDK's vhost
>>>>>> example
>>>>>>>> after all. Nor is it used in vhost pmd, neither is it used in OVS.
>>>>>>
>>>>>> rte_vhost_feature_disable() is currently used in OVS,
>>>>> lib/netdev-dpdk.c
>>>> Hmmm. I must have checked very old code ...
>>>>>>
>>>>>> netdev_dpdk_vhost_class_init(void)
>>>>>> {
>>>>>>     static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>>>>>>
>>>>>>     /* This function can be called for different classes.  The
>>>>>> initialization
>>>>>>      * needs to be done only once */
>>>>>>     if (ovsthread_once_start(&once)) {
>>>>>>         rte_vhost_driver_callback_register(&virtio_net_device_ops);
>>>>>>         rte_vhost_feature_disable(1ULL << VIRTIO_NET_F_HOST_TSO4
>>>>>>                                   | 1ULL << VIRTIO_NET_F_HOST_TSO6
>>>>>>                                   | 1ULL << VIRTIO_NET_F_CSUM);
>>>> I saw the commit introduced such change, but it tells no reason why
>>>> it was added.
>>>
>>> I'm also interested to know the reason.
>>
>> I can't remember off hand, added Mark K or Michal W who should be able
>> to shed some light on it.
>
> DPDK v16.04 added support for vHost User TSO; as such, by default, TSO is advertised to guest devices as an available feature during feature negotiation with QEMU.
> However, while the vHost user backend sets up the majority of the mbuf fields that are required for TSO, there is still a reliance on the associated DPDK application (i.e. in this case OvS-DPDK) to set the remaining flags and/or offsets. Since OvS-DPDK doesn't currently provide that functionality, it is necessary to explicitly disable TSO; otherwise, undefined behaviour will ensue.
Thanks Mark for the clarification.

In this case, maybe we could add a DPDK build option to disable Vhost's
TSO support, that would be selected for OVS packages?

Does that sound reasonable?

Cheers,
Maxime

>>
>>> In any case, I think this is something that can/should be managed by
>>> the management tool, which  should disable it in cmd parameters.
>>>
>>> Kevin, do you agree?
>>
>> I think best to find out the reason first. Because if no reason to
>> disable in the code, then no need to debate!
>>
>>>
>>> Cheers,
>>> Maxime
>