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

[Jason Wang]

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.

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