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.
> 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