From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36796) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQAgu-0003Sn-PF for qemu-devel@nongnu.org; Mon, 01 Feb 2016 04:22:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aQAgr-0005hv-Ii for qemu-devel@nongnu.org; Mon, 01 Feb 2016 04:22:36 -0500 Received: from szxga02-in.huawei.com ([119.145.14.65]:53849) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQAgr-0005hU-0Q for qemu-devel@nongnu.org; Mon, 01 Feb 2016 04:22:33 -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> From: Hailiang Zhang Message-ID: <56AF23BB.8000203@huawei.com> Date: Mon, 1 Feb 2016 17:22:03 +0800 MIME-Version: 1.0 In-Reply-To: <56AF1F8E.2020604@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: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). 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 >> For COLO, we don't want to control (use) other filters that added by >> users. >> >> Thanks, >> Hailiang >> >>> Thanks >>> >>>> >>>> Thanks, >>>> Hailiang >>>> >>>>> Thoughts? >>>>> >>>>>> + opts = qemu_opts_parse_noisily(&qemu_filter_opts, >>>>>> + optarg, false); >>>>>> + if (!opts) { >>>>>> + error_report("Failed to parse param '%s'", optarg); >>>>>> + exit(1); >>>>>> + } >>>>>> + g_free(optarg); >>>>>> + if (object_create(NULL, opts, &err) < 0) { >>>>>> + error_report("Failed to create object"); >>>>>> + goto out_clean; >>>>>> + } >>>>>> + filter_set_default_flag(id, is_default, &err); >>>>>> + >>>>>> +out_clean: >>>>>> + qemu_opts_del(opts); >>>>>> + if (err) { >>>>>> + error_propagate(errp, err); >>>>>> + } >>>>>> +} >>>>>> + >>>>>> static void netfilter_finalize(Object *obj) >>>>>> { >>>>>> NetFilterState *nf = NETFILTER(obj); >>>>> >>>>> >>>>> . >>>>> >>>> >>>> >>> >>> >>> . >>> >> >> >> > > > . >