Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev

[Hailiang Zhang <zhang.zhanghailiang@huawei.com>]

On 2016/2/1 17:49, Jason Wang wrote:
>
>
> On 02/01/2016 05:39 PM, Hailiang Zhang wrote:
>> On 2016/2/1 17:18, Jason Wang wrote:
>>>
>>>
>>> On 02/01/2016 04:21 PM, Hailiang Zhang wrote:
>>>>>>>>>
>>>>>>>>> Instead of this, I wonder maybe it's better to:
>>>>>>>>>
>>>>>>>>> - store the default filter property into a pointer to string
>>>>>>>>
>>>>>>>> Do you mean, pass a string parameter which stores the filter
>>>>>>>> property
>>>>>>>> instead of
>>>>>>>> assemble it in this helper ?
>>>>>>>
>>>>>>> Yes. E.g just a global string which could be changed by any
>>>>>>> subsystem.
>>>>>>> E.g colo may change it to
>>>>>>> "filter-buffer,interval=0,status=disable". But
>>>>>>> filter ids need to be generated automatically.
>>>>>>>
>>>>>>
>>>>>> Got it. Then we don't need the global default_netfilter_type[] in
>>>>>> patch 5,
>>>>>> Just use this global string instead ?
>>>>>>
>>>>>>>>
>>>>>>>>> - colo code may change the pointer to
>>>>>>>>> "filter-buffer,status=disable"
>>>>>>>>>
>>>>>>>>
>>>>>>>>> Then, there's no need for lots of codes above:
>>>>>>>>> - no need a "is_default" parameter in netdev_add_filter which
>>>>>>>>> does not
>>>>>>>>> scale consider we may want to have more property in the future
>>>>>>>>> - no need to hacking like "qemu_filter_opts"
>>>>>>>>
>>>>>>>> Yes, we can use qemu_find_opts("object") instead of it.
>>>>>>>>
>>>>>>>>> - no need to have a special flag like "is_default"
>>>>>>>>>
>>>>>>>>
>>>>>>>> But we have to distinguish the default filter from the common
>>>>>>>> filter, use the name (id) to distinguish it ?
>>>>>>>
>>>>>>> What's the reason that you want to distinguish default filters from
>>>>>>> others?
>>>>>>>
>>>>>>
>>>>>> The default filters will be used by COLO or MC, (In COLO, we will
>>>>>> use it
>>>>>> to control packets buffering/releasing).
>>>>>> For COLO, we don't want to control (use) other filters that added by
>>>>>> users.
>>>>>
>>>>> I think Jason's point is that COLO is a manager, you can add the
>>>>> filter
>>>>> to netdev when doing COLO, so the only difference between COLO's
>>>>> default
>>>>
>>>> Er, then we came back to the original question, 'is it necessary to
>>>> add each netdev
>>>> a default filter ?'
>>>
>>> The question could be extended to:
>>>
>>> 1) Do we need a default filter? I think the answer is yes, but of course
>>> COLO can work even without this.
>>
>> Yes, after colo-proxy is realized, we can switch to colo-proxy
>> (It should have the capability of buffer and release packets directly).
>> But for now, we want to merge COLO prototype without colo-proxy, the COLO
>> prototype should have the basic capability.
>
> Right, I see.
>
>> Just like Remus or
>> Micro-checkpointing. It is based on the default buffer-filter to
>> control net
>> packets.
>>
>>> 2) Do we want to implement COLO on top of default filter? If yes, as you
>>> suggest, we may record the ids of the default filter and do what ever we
>>
>> Yes, we need it.
>
> Or just as I reply, all buffer filters (with zero interval) could be
> tracked by itself. So as you see, several ways could go. It's your call
> to choose one of them.
>

OK, got it.

>>
>>> what. If not, COLO need codes to go through each netdev and add filter
>>> itself (hotplug is not supported). Or you want management to do this,
>>> then even hotplug could be supported.
>>>
>>
>> We also want to support hotplug during VM is in COLO state in the future.
>> (For this point, I'm not quite sure if this usage case is really exist.)
>>
>> Thanks,
>> Hailiang
>
> Support hotplug should be useful I think. But I'm also ok if you don't
> want to consider for it now.
>

Thanks very much.

Hailiang