From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55650) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQBv7-0002Ov-5c for qemu-devel@nongnu.org; Mon, 01 Feb 2016 05:41:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aQBv3-00062h-Tq for qemu-devel@nongnu.org; Mon, 01 Feb 2016 05:41:21 -0500 Received: from szxga01-in.huawei.com ([58.251.152.64]:47069) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQBv3-0005yl-67 for qemu-devel@nongnu.org; Mon, 01 Feb 2016 05:41:17 -0500 References: <1453883380-10532-1-git-send-email-zhang.zhanghailiang@huawei.com> <1453883380-10532-4-git-send-email-zhang.zhanghailiang@huawei.com> <56AECDA8.6000402@redhat.com> <56AEF788.3080706@huawei.com> <56AF0D63.3010707@redhat.com> <56AF0FC4.7070809@huawei.com> <56AF1F8E.2020604@redhat.com> <56AF23BB.8000203@huawei.com> <56AF2890.4090304@redhat.com> From: Hailiang Zhang Message-ID: <56AF362E.9000302@huawei.com> Date: Mon, 1 Feb 2016 18:40:46 +0800 MIME-Version: 1.0 In-Reply-To: <56AF2890.4090304@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , qemu-devel@nongnu.org Cc: peter.huangpeng@huawei.com, zhangchen.fnst@cn.fujitsu.com, hongyang.yang@easystack.cn On 2016/2/1 17:42, Jason Wang wrote: > > > On 02/01/2016 05:22 PM, Hailiang Zhang wrote: >> On 2016/2/1 17:04, Jason Wang wrote: >>> >>> >>> On 02/01/2016 03:56 PM, Hailiang Zhang wrote: >>>> On 2016/2/1 15:46, Jason Wang wrote: >>>>> >>>>> >>>>> On 02/01/2016 02:13 PM, Hailiang Zhang wrote: >>>>>> On 2016/2/1 11:14, Jason Wang wrote: >>>>>>> >>>>>>> >>>>>>> On 01/27/2016 04:29 PM, zhanghailiang wrote: >>>>>>>> We add a new helper function netdev_add_filter(), this function >>>>>>>> can help adding a filter object to a netdev. >>>>>>>> Besides, we add a is_default member for struct NetFilterState >>>>>>>> to indicate whether the filter is default or not. >>>>>>>> >>>>>>>> Signed-off-by: zhanghailiang >>>>>>>> --- >>>>>>>> v2: >>>>>>>> -Re-implement netdev_add_filter() by re-using object_create() >>>>>>>> (Jason's suggestion) >>>>>>>> --- >>>>>>>> include/net/filter.h | 7 +++++ >>>>>>>> net/filter.c | 80 >>>>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>>>>> 2 files changed, 87 insertions(+) >>>>>>>> >>>>>>>> >>> >>> [...] >>> >>>>>>>> + >>>>>>>> + optarg = >>>>>>>> g_strdup_printf("qom-type=%s,id=%s,netdev=%s,status=%s", >>>>>>>> + filter_type, id, netdev_id, is_default ? "disable" : >>>>>>>> "enable" >>>>>>> >>>>>>> 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, >>> >>> Yes. >>> >>>> Just use this global string instead ? >>> >>> Right. >>> >>>> >>>>>> >>>>>>> - 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). >>> >>> A question is how will you do this? >>> >> >> Er, for COLO, we will enable all the default filter in the >> initialization stage, >> then the buffer-filter will buffer all netdev's packets, >> after doing a checkpoint, we will release all the buffered packets >> (Flush all default >> filters' packets). > > Right, that's the point. So looks several choices here: > > 1) Track each default filter explicitly, generate and record the netdev > ids for default filter by COLO. Walk through the ids list and release > the packet in each filter. > 2) Track the default filters implicitly. Link all buffer filters (with > zero interval) in a list during filter buffer initialization. And export > a helper for COLO to walk them and release packets. > > Either looks fine, but maybe 2 is easier. > 2) is a good choice, but I'm a little worry about using zero to distinguish if a filter is default filter or not, because users can use qom-set to change its value. If special flag like 'is_default' is not acceptable, then we have to use the filter ids to distinguish the default buffer-filter (here the rule of generation ids for default filter is '[netdev-id][DEFAULT_FILTER_TYPE]' Thanks, Hailiang >> If VM is failover, we will set all default filters back to disabled >> status. >> (This is a periodic mode for COLO, different from another mode, which >> we will call it >> hybrid mode, that is based on colo-proxy, which is in developing by >> zhangchen) >> >> Thanks, >> Hailiang > > Yes, I see. > > > . >