* [Qemu-devel] [PATCH RFC v2 0/5] Netfilter: Add each netdev a default filter @ 2016-01-27 8:29 zhanghailiang 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 1/5] net/filter: Add a 'status' property for filter object zhanghailiang ` (4 more replies) 0 siblings, 5 replies; 33+ messages in thread From: zhanghailiang @ 2016-01-27 8:29 UTC (permalink / raw) To: qemu-devel; +Cc: jasowang, zhanghailiang, zhangchen.fnst, hongyang.yang This series is a prerequisite for COLO, here we add each netdev a default buffer filter, it is disabled by default, and has no side effect for delivering packets in net layer. Note: this series is based on patch '[PATCH v2] net/filter: Fix the output information for command 'info network' v2: - Drop the patch 'net/filter: prevent the default filter to be deleted' (Jason) - Re-implement netdev_add_filter() by re-using object_object() (Jason) - Send patch 'net/filter: Fix the output information for command 'info network' as an independent one. (Jason) zhanghailiang (5): net/filter: Add a 'status' property for filter object vl: Make object_create() public net/filter: Introduce a helper to add a filter to the netdev filter-buffer: Accept zero interval net/filter: Add a default filter to each netdev include/net/filter.h | 12 +++++ include/qemu-common.h | 2 + net/filter-buffer.c | 10 ---- net/filter.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++ net/net.c | 23 ++++++++++ vl.c | 4 +- 6 files changed, 164 insertions(+), 12 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH RFC v2 1/5] net/filter: Add a 'status' property for filter object 2016-01-27 8:29 [Qemu-devel] [PATCH RFC v2 0/5] Netfilter: Add each netdev a default filter zhanghailiang @ 2016-01-27 8:29 ` zhanghailiang 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 2/5] vl: Make object_create() public zhanghailiang ` (3 subsequent siblings) 4 siblings, 0 replies; 33+ messages in thread From: zhanghailiang @ 2016-01-27 8:29 UTC (permalink / raw) To: qemu-devel; +Cc: jasowang, zhanghailiang, zhangchen.fnst, hongyang.yang With this property, users can control if this filter is 'enable' or 'disable'. The default behavior for filter is enabled. We will skip the disabled filter when delivering packets in net layer. Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> --- v2: - Squash previous patch 3 into this patch (Jason's suggestion) --- include/net/filter.h | 1 + net/filter.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/include/net/filter.h b/include/net/filter.h index 5639976..af3c53c 100644 --- a/include/net/filter.h +++ b/include/net/filter.h @@ -55,6 +55,7 @@ struct NetFilterState { char *netdev_id; NetClientState *netdev; NetFilterDirection direction; + bool enabled; QTAILQ_ENTRY(NetFilterState) next; }; diff --git a/net/filter.c b/net/filter.c index 5adcec8..d08a2be 100644 --- a/net/filter.c +++ b/net/filter.c @@ -16,6 +16,11 @@ #include "qom/object_interfaces.h" #include "qemu/iov.h" +static inline bool qemu_need_skip_netfilter(NetFilterState *nf) +{ + return nf->enabled ? false : true; +} + ssize_t qemu_netfilter_receive(NetFilterState *nf, NetFilterDirection direction, NetClientState *sender, @@ -24,6 +29,10 @@ ssize_t qemu_netfilter_receive(NetFilterState *nf, int iovcnt, NetPacketSent *sent_cb) { + /* Don't go through the filter if it is disabled */ + if (qemu_need_skip_netfilter(nf)) { + return 0; + } if (nf->direction == direction || nf->direction == NET_FILTER_DIRECTION_ALL) { return NETFILTER_GET_CLASS(OBJECT(nf))->receive_iov( @@ -116,8 +125,41 @@ static void netfilter_set_direction(Object *obj, int direction, Error **errp) nf->direction = direction; } +static char *netfilter_get_status(Object *obj, Error **errp) +{ + NetFilterState *nf = NETFILTER(obj); + + if (nf->enabled) { + return g_strdup("enable"); + } else { + return g_strdup("disable"); + } +} + +static void netfilter_set_status(Object *obj, const char *str, Error **errp) +{ + NetFilterState *nf = NETFILTER(obj); + + if (!strcmp(str, "enable")) { + nf->enabled = true; + } else if (!strcmp(str, "disable")) { + nf->enabled = false; + } else { + error_setg(errp, "Invalid value for netfilter status, " + "should be 'enable' or 'disable'"); + } +} + static void netfilter_init(Object *obj) { + NetFilterState *nf = NETFILTER(obj); + + /* + * If not configured with 'status' property, the default status + * for netfilter will be enabled. + */ + nf->enabled = true; + object_property_add_str(obj, "netdev", netfilter_get_netdev_id, netfilter_set_netdev_id, NULL); @@ -125,6 +167,9 @@ static void netfilter_init(Object *obj) NetFilterDirection_lookup, netfilter_get_direction, netfilter_set_direction, NULL); + object_property_add_str(obj, "status", + netfilter_get_status, netfilter_set_status, + NULL); } static void netfilter_complete(UserCreatable *uc, Error **errp) -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH RFC v2 2/5] vl: Make object_create() public 2016-01-27 8:29 [Qemu-devel] [PATCH RFC v2 0/5] Netfilter: Add each netdev a default filter zhanghailiang 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 1/5] net/filter: Add a 'status' property for filter object zhanghailiang @ 2016-01-27 8:29 ` zhanghailiang 2016-02-01 3:05 ` Jason Wang 2016-02-01 10:41 ` Daniel P. Berrange 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev zhanghailiang ` (2 subsequent siblings) 4 siblings, 2 replies; 33+ messages in thread From: zhanghailiang @ 2016-01-27 8:29 UTC (permalink / raw) To: qemu-devel Cc: zhanghailiang, zhangchen.fnst, jasowang, Paolo Bonzini, hongyang.yang Make the helper object_create() public and fix its first parameter to accept NULL value. Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> Cc: Paolo Bonzini <pbonzini@redhat.com> --- v2: - New patch --- include/qemu-common.h | 2 ++ vl.c | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/qemu-common.h b/include/qemu-common.h index 22b010c..52cf4fd 100644 --- a/include/qemu-common.h +++ b/include/qemu-common.h @@ -500,4 +500,6 @@ int parse_debug_env(const char *name, int max, int initial); const char *qemu_ether_ntoa(const MACAddr *mac); void page_size_init(void); +int object_create(void *opaque, QemuOpts *opts, Error **errp); + #endif diff --git a/vl.c b/vl.c index f043009..b21335e 100644 --- a/vl.c +++ b/vl.c @@ -2819,7 +2819,7 @@ static bool object_create_delayed(const char *type) } -static int object_create(void *opaque, QemuOpts *opts, Error **errp) +int object_create(void *opaque, QemuOpts *opts, Error **errp) { Error *err = NULL; char *type = NULL; @@ -2842,7 +2842,7 @@ static int object_create(void *opaque, QemuOpts *opts, Error **errp) if (err) { goto out; } - if (!type_predicate(type)) { + if (type_predicate && !type_predicate(type)) { goto out; } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 2/5] vl: Make object_create() public 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 2/5] vl: Make object_create() public zhanghailiang @ 2016-02-01 3:05 ` Jason Wang 2016-02-01 6:19 ` Hailiang Zhang 2016-02-01 10:41 ` Daniel P. Berrange 1 sibling, 1 reply; 33+ messages in thread From: Jason Wang @ 2016-02-01 3:05 UTC (permalink / raw) To: zhanghailiang, qemu-devel; +Cc: Paolo Bonzini, zhangchen.fnst, hongyang.yang On 01/27/2016 04:29 PM, zhanghailiang wrote: > Make the helper object_create() public and fix its first > parameter to accept NULL value. Looks not very nice. Maybe pass a new predicate func for sanity check it better. > > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > --- > v2: > - New patch > --- > include/qemu-common.h | 2 ++ > vl.c | 4 ++-- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/include/qemu-common.h b/include/qemu-common.h > index 22b010c..52cf4fd 100644 > --- a/include/qemu-common.h > +++ b/include/qemu-common.h > @@ -500,4 +500,6 @@ int parse_debug_env(const char *name, int max, int initial); > const char *qemu_ether_ntoa(const MACAddr *mac); > void page_size_init(void); > > +int object_create(void *opaque, QemuOpts *opts, Error **errp); > + > #endif > diff --git a/vl.c b/vl.c > index f043009..b21335e 100644 > --- a/vl.c > +++ b/vl.c > @@ -2819,7 +2819,7 @@ static bool object_create_delayed(const char *type) > } > > > -static int object_create(void *opaque, QemuOpts *opts, Error **errp) > +int object_create(void *opaque, QemuOpts *opts, Error **errp) > { > Error *err = NULL; > char *type = NULL; > @@ -2842,7 +2842,7 @@ static int object_create(void *opaque, QemuOpts *opts, Error **errp) > if (err) { > goto out; > } > - if (!type_predicate(type)) { > + if (type_predicate && !type_predicate(type)) { > goto out; > } > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 2/5] vl: Make object_create() public 2016-02-01 3:05 ` Jason Wang @ 2016-02-01 6:19 ` Hailiang Zhang 2016-02-01 7:27 ` Jason Wang 0 siblings, 1 reply; 33+ messages in thread From: Hailiang Zhang @ 2016-02-01 6:19 UTC (permalink / raw) To: Jason Wang, qemu-devel Cc: Paolo Bonzini, peter.huangpeng, zhangchen.fnst, hongyang.yang On 2016/2/1 11:05, Jason Wang wrote: > > > On 01/27/2016 04:29 PM, zhanghailiang wrote: >> Make the helper object_create() public and fix its first >> parameter to accept NULL value. > > Looks not very nice. Maybe pass a new predicate func for sanity check it > better. > OK, but here is it better to check if the predicate func is NULL ? Thanks, Hailiang >> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> --- >> v2: >> - New patch >> --- >> include/qemu-common.h | 2 ++ >> vl.c | 4 ++-- >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/include/qemu-common.h b/include/qemu-common.h >> index 22b010c..52cf4fd 100644 >> --- a/include/qemu-common.h >> +++ b/include/qemu-common.h >> @@ -500,4 +500,6 @@ int parse_debug_env(const char *name, int max, int initial); >> const char *qemu_ether_ntoa(const MACAddr *mac); >> void page_size_init(void); >> >> +int object_create(void *opaque, QemuOpts *opts, Error **errp); >> + >> #endif >> diff --git a/vl.c b/vl.c >> index f043009..b21335e 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2819,7 +2819,7 @@ static bool object_create_delayed(const char *type) >> } >> >> >> -static int object_create(void *opaque, QemuOpts *opts, Error **errp) >> +int object_create(void *opaque, QemuOpts *opts, Error **errp) >> { >> Error *err = NULL; >> char *type = NULL; >> @@ -2842,7 +2842,7 @@ static int object_create(void *opaque, QemuOpts *opts, Error **errp) >> if (err) { >> goto out; >> } >> - if (!type_predicate(type)) { >> + if (type_predicate && !type_predicate(type)) { >> goto out; >> } >> > > > . > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 2/5] vl: Make object_create() public 2016-02-01 6:19 ` Hailiang Zhang @ 2016-02-01 7:27 ` Jason Wang 2016-02-01 7:34 ` Hailiang Zhang 0 siblings, 1 reply; 33+ messages in thread From: Jason Wang @ 2016-02-01 7:27 UTC (permalink / raw) To: Hailiang Zhang, qemu-devel Cc: Paolo Bonzini, peter.huangpeng, zhangchen.fnst, hongyang.yang On 02/01/2016 02:19 PM, Hailiang Zhang wrote: > On 2016/2/1 11:05, Jason Wang wrote: >> >> >> On 01/27/2016 04:29 PM, zhanghailiang wrote: >>> Make the helper object_create() public and fix its first >>> parameter to accept NULL value. >> >> Looks not very nice. Maybe pass a new predicate func for sanity check it >> better. >> > > OK, but here is it better to check if the predicate func is NULL ? > > Thanks, > Hailiang Not sure, but if you stick to check against NULL, need a separate patch. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 2/5] vl: Make object_create() public 2016-02-01 7:27 ` Jason Wang @ 2016-02-01 7:34 ` Hailiang Zhang 0 siblings, 0 replies; 33+ messages in thread From: Hailiang Zhang @ 2016-02-01 7:34 UTC (permalink / raw) To: Jason Wang, qemu-devel Cc: Paolo Bonzini, peter.huangpeng, zhangchen.fnst, hongyang.yang On 2016/2/1 15:27, Jason Wang wrote: > > > On 02/01/2016 02:19 PM, Hailiang Zhang wrote: >> On 2016/2/1 11:05, Jason Wang wrote: >>> >>> >>> On 01/27/2016 04:29 PM, zhanghailiang wrote: >>>> Make the helper object_create() public and fix its first >>>> parameter to accept NULL value. >>> >>> Looks not very nice. Maybe pass a new predicate func for sanity check it >>> better. >>> >> >> OK, but here is it better to check if the predicate func is NULL ? >> >> Thanks, >> Hailiang > > Not sure, but if you stick to check against NULL, need a separate patch. > OK, i will drop this unnecessary check and add sanity check in next version, thanks. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 2/5] vl: Make object_create() public 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 2/5] vl: Make object_create() public zhanghailiang 2016-02-01 3:05 ` Jason Wang @ 2016-02-01 10:41 ` Daniel P. Berrange 2016-02-01 10:44 ` Hailiang Zhang 1 sibling, 1 reply; 33+ messages in thread From: Daniel P. Berrange @ 2016-02-01 10:41 UTC (permalink / raw) To: zhanghailiang Cc: Paolo Bonzini, jasowang, qemu-devel, zhangchen.fnst, hongyang.yang On Wed, Jan 27, 2016 at 04:29:37PM +0800, zhanghailiang wrote: > Make the helper object_create() public and fix its first > parameter to accept NULL value. > > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > --- > v2: > - New patch > --- > include/qemu-common.h | 2 ++ > vl.c | 4 ++-- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/include/qemu-common.h b/include/qemu-common.h > index 22b010c..52cf4fd 100644 > --- a/include/qemu-common.h > +++ b/include/qemu-common.h > @@ -500,4 +500,6 @@ int parse_debug_env(const char *name, int max, int initial); > const char *qemu_ether_ntoa(const MACAddr *mac); > void page_size_init(void); > > +int object_create(void *opaque, QemuOpts *opts, Error **errp); > + > #endif > diff --git a/vl.c b/vl.c > index f043009..b21335e 100644 > --- a/vl.c > +++ b/vl.c > @@ -2819,7 +2819,7 @@ static bool object_create_delayed(const char *type) > } > > > -static int object_create(void *opaque, QemuOpts *opts, Error **errp) > +int object_create(void *opaque, QemuOpts *opts, Error **errp) > { > Error *err = NULL; > char *type = NULL; > @@ -2842,7 +2842,7 @@ static int object_create(void *opaque, QemuOpts *opts, Error **errp) > if (err) { > goto out; > } > - if (!type_predicate(type)) { > + if (type_predicate && !type_predicate(type)) { > goto out; > } No, please don't do this - your later patch should *not* be using object_create, it should use object_new_with_props. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 2/5] vl: Make object_create() public 2016-02-01 10:41 ` Daniel P. Berrange @ 2016-02-01 10:44 ` Hailiang Zhang 0 siblings, 0 replies; 33+ messages in thread From: Hailiang Zhang @ 2016-02-01 10:44 UTC (permalink / raw) To: Daniel P. Berrange Cc: zhangchen.fnst, jasowang, peter.huangpeng, qemu-devel, Paolo Bonzini, hongyang.yang On 2016/2/1 18:41, Daniel P. Berrange wrote: > On Wed, Jan 27, 2016 at 04:29:37PM +0800, zhanghailiang wrote: >> Make the helper object_create() public and fix its first >> parameter to accept NULL value. >> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> --- >> v2: >> - New patch >> --- >> include/qemu-common.h | 2 ++ >> vl.c | 4 ++-- >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/include/qemu-common.h b/include/qemu-common.h >> index 22b010c..52cf4fd 100644 >> --- a/include/qemu-common.h >> +++ b/include/qemu-common.h >> @@ -500,4 +500,6 @@ int parse_debug_env(const char *name, int max, int initial); >> const char *qemu_ether_ntoa(const MACAddr *mac); >> void page_size_init(void); >> >> +int object_create(void *opaque, QemuOpts *opts, Error **errp); >> + >> #endif >> diff --git a/vl.c b/vl.c >> index f043009..b21335e 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -2819,7 +2819,7 @@ static bool object_create_delayed(const char *type) >> } >> >> >> -static int object_create(void *opaque, QemuOpts *opts, Error **errp) >> +int object_create(void *opaque, QemuOpts *opts, Error **errp) >> { >> Error *err = NULL; >> char *type = NULL; >> @@ -2842,7 +2842,7 @@ static int object_create(void *opaque, QemuOpts *opts, Error **errp) >> if (err) { >> goto out; >> } >> - if (!type_predicate(type)) { >> + if (type_predicate && !type_predicate(type)) { >> goto out; >> } > > No, please don't do this - your later patch should *not* be using > object_create, it should use object_new_with_props. > Er, i didn't notice this helper before, i will look into it. Thanks, Hailiang > Regards, > Daniel > ^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-01-27 8:29 [Qemu-devel] [PATCH RFC v2 0/5] Netfilter: Add each netdev a default filter zhanghailiang 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 1/5] net/filter: Add a 'status' property for filter object zhanghailiang 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 2/5] vl: Make object_create() public zhanghailiang @ 2016-01-27 8:29 ` zhanghailiang 2016-02-01 3:14 ` Jason Wang 2016-02-01 10:43 ` Daniel P. Berrange 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 4/5] filter-buffer: Accept zero interval zhanghailiang 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 5/5] net/filter: Add a default filter to each netdev zhanghailiang 4 siblings, 2 replies; 33+ messages in thread From: zhanghailiang @ 2016-01-27 8:29 UTC (permalink / raw) To: qemu-devel; +Cc: jasowang, zhanghailiang, zhangchen.fnst, hongyang.yang 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 <zhang.zhanghailiang@huawei.com> --- 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(+) diff --git a/include/net/filter.h b/include/net/filter.h index af3c53c..ee1c024 100644 --- a/include/net/filter.h +++ b/include/net/filter.h @@ -55,6 +55,7 @@ struct NetFilterState { char *netdev_id; NetClientState *netdev; NetFilterDirection direction; + bool is_default; bool enabled; QTAILQ_ENTRY(NetFilterState) next; }; @@ -74,4 +75,10 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, int iovcnt, void *opaque); +void netdev_add_filter(const char *netdev_id, + const char *filter_type, + const char *id, + bool is_default, + Error **errp); + #endif /* QEMU_NET_FILTER_H */ diff --git a/net/filter.c b/net/filter.c index d08a2be..dc7aa9b 100644 --- a/net/filter.c +++ b/net/filter.c @@ -214,6 +214,86 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next); } +QemuOptsList qemu_filter_opts = { + .name = "default-filter", + .head = QTAILQ_HEAD_INITIALIZER(qemu_filter_opts.head), + .desc = { + { + .name = "qom-type", + .type = QEMU_OPT_STRING, + },{ + .name = "id", + .type = QEMU_OPT_STRING, + },{ + .name = "netdev", + .type = QEMU_OPT_STRING, + },{ + .name = "status", + .type = QEMU_OPT_STRING, + }, + { /* end of list */ } + }, +}; + +static void filter_set_default_flag(const char *id, + bool is_default, + Error **errp) +{ + Object *obj, *container; + NetFilterState *nf; + + container = object_get_objects_root(); + obj = object_resolve_path_component(container, id); + if (!obj) { + error_setg(errp, "object id not found"); + return; + } + nf = NETFILTER(obj); + nf->is_default = is_default; +} + +void netdev_add_filter(const char *netdev_id, + const char *filter_type, + const char *id, + bool is_default, + Error **errp) +{ + NetClientState *nc = qemu_find_netdev(netdev_id); + char *optarg; + QemuOpts *opts = NULL; + Error *err = NULL; + + /* FIXME: Not support multiple queues */ + if (!nc || nc->queue_index > 1) { + return; + } + /* Not support vhost-net */ + if (get_vhost_net(nc)) { + return; + } + + optarg = g_strdup_printf("qom-type=%s,id=%s,netdev=%s,status=%s", + filter_type, id, netdev_id, is_default ? "disable" : "enable"); + 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); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev zhanghailiang @ 2016-02-01 3:14 ` Jason Wang 2016-02-01 6:13 ` Hailiang Zhang 2016-02-01 10:43 ` Daniel P. Berrange 1 sibling, 1 reply; 33+ messages in thread From: Jason Wang @ 2016-02-01 3:14 UTC (permalink / raw) To: zhanghailiang, qemu-devel; +Cc: zhangchen.fnst, hongyang.yang 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 <zhang.zhanghailiang@huawei.com> > --- > 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(+) > > diff --git a/include/net/filter.h b/include/net/filter.h > index af3c53c..ee1c024 100644 > --- a/include/net/filter.h > +++ b/include/net/filter.h > @@ -55,6 +55,7 @@ struct NetFilterState { > char *netdev_id; > NetClientState *netdev; > NetFilterDirection direction; > + bool is_default; > bool enabled; > QTAILQ_ENTRY(NetFilterState) next; > }; > @@ -74,4 +75,10 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, > int iovcnt, > void *opaque); > > +void netdev_add_filter(const char *netdev_id, > + const char *filter_type, > + const char *id, > + bool is_default, > + Error **errp); > + > #endif /* QEMU_NET_FILTER_H */ > diff --git a/net/filter.c b/net/filter.c > index d08a2be..dc7aa9b 100644 > --- a/net/filter.c > +++ b/net/filter.c > @@ -214,6 +214,86 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) > QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next); > } > > +QemuOptsList qemu_filter_opts = { > + .name = "default-filter", > + .head = QTAILQ_HEAD_INITIALIZER(qemu_filter_opts.head), > + .desc = { > + { > + .name = "qom-type", > + .type = QEMU_OPT_STRING, > + },{ > + .name = "id", > + .type = QEMU_OPT_STRING, > + },{ > + .name = "netdev", > + .type = QEMU_OPT_STRING, > + },{ > + .name = "status", > + .type = QEMU_OPT_STRING, > + }, > + { /* end of list */ } > + }, > +}; > + > +static void filter_set_default_flag(const char *id, > + bool is_default, > + Error **errp) > +{ > + Object *obj, *container; > + NetFilterState *nf; > + > + container = object_get_objects_root(); > + obj = object_resolve_path_component(container, id); > + if (!obj) { > + error_setg(errp, "object id not found"); > + return; > + } > + nf = NETFILTER(obj); > + nf->is_default = is_default; > +} > + > +void netdev_add_filter(const char *netdev_id, > + const char *filter_type, > + const char *id, > + bool is_default, > + Error **errp) > +{ > + NetClientState *nc = qemu_find_netdev(netdev_id); > + char *optarg; > + QemuOpts *opts = NULL; > + Error *err = NULL; > + > + /* FIXME: Not support multiple queues */ > + if (!nc || nc->queue_index > 1) { > + return; > + } > + /* Not support vhost-net */ > + if (get_vhost_net(nc)) { > + return; > + } > + > + 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 - 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" - no need to have a special flag like "is_default" 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); ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 3:14 ` Jason Wang @ 2016-02-01 6:13 ` Hailiang Zhang 2016-02-01 7:46 ` Jason Wang 0 siblings, 1 reply; 33+ messages in thread From: Hailiang Zhang @ 2016-02-01 6:13 UTC (permalink / raw) To: Jason Wang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst, hongyang.yang 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 <zhang.zhanghailiang@huawei.com> >> --- >> 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(+) >> >> diff --git a/include/net/filter.h b/include/net/filter.h >> index af3c53c..ee1c024 100644 >> --- a/include/net/filter.h >> +++ b/include/net/filter.h >> @@ -55,6 +55,7 @@ struct NetFilterState { >> char *netdev_id; >> NetClientState *netdev; >> NetFilterDirection direction; >> + bool is_default; >> bool enabled; >> QTAILQ_ENTRY(NetFilterState) next; >> }; >> @@ -74,4 +75,10 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, >> int iovcnt, >> void *opaque); >> >> +void netdev_add_filter(const char *netdev_id, >> + const char *filter_type, >> + const char *id, >> + bool is_default, >> + Error **errp); >> + >> #endif /* QEMU_NET_FILTER_H */ >> diff --git a/net/filter.c b/net/filter.c >> index d08a2be..dc7aa9b 100644 >> --- a/net/filter.c >> +++ b/net/filter.c >> @@ -214,6 +214,86 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) >> QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next); >> } >> >> +QemuOptsList qemu_filter_opts = { >> + .name = "default-filter", >> + .head = QTAILQ_HEAD_INITIALIZER(qemu_filter_opts.head), >> + .desc = { >> + { >> + .name = "qom-type", >> + .type = QEMU_OPT_STRING, >> + },{ >> + .name = "id", >> + .type = QEMU_OPT_STRING, >> + },{ >> + .name = "netdev", >> + .type = QEMU_OPT_STRING, >> + },{ >> + .name = "status", >> + .type = QEMU_OPT_STRING, >> + }, >> + { /* end of list */ } >> + }, >> +}; >> + >> +static void filter_set_default_flag(const char *id, >> + bool is_default, >> + Error **errp) >> +{ >> + Object *obj, *container; >> + NetFilterState *nf; >> + >> + container = object_get_objects_root(); >> + obj = object_resolve_path_component(container, id); >> + if (!obj) { >> + error_setg(errp, "object id not found"); >> + return; >> + } >> + nf = NETFILTER(obj); >> + nf->is_default = is_default; >> +} >> + >> +void netdev_add_filter(const char *netdev_id, >> + const char *filter_type, >> + const char *id, >> + bool is_default, >> + Error **errp) >> +{ >> + NetClientState *nc = qemu_find_netdev(netdev_id); >> + char *optarg; >> + QemuOpts *opts = NULL; >> + Error *err = NULL; >> + >> + /* FIXME: Not support multiple queues */ >> + if (!nc || nc->queue_index > 1) { >> + return; >> + } >> + /* Not support vhost-net */ >> + if (get_vhost_net(nc)) { >> + return; >> + } >> + >> + 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 ? > - 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 ? 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); > > > . > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 6:13 ` Hailiang Zhang @ 2016-02-01 7:46 ` Jason Wang 2016-02-01 7:56 ` Hailiang Zhang 0 siblings, 1 reply; 33+ messages in thread From: Jason Wang @ 2016-02-01 7:46 UTC (permalink / raw) To: Hailiang Zhang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst, hongyang.yang 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 <zhang.zhanghailiang@huawei.com> >>> --- >>> 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(+) >>> >>> diff --git a/include/net/filter.h b/include/net/filter.h >>> index af3c53c..ee1c024 100644 >>> --- a/include/net/filter.h >>> +++ b/include/net/filter.h >>> @@ -55,6 +55,7 @@ struct NetFilterState { >>> char *netdev_id; >>> NetClientState *netdev; >>> NetFilterDirection direction; >>> + bool is_default; >>> bool enabled; >>> QTAILQ_ENTRY(NetFilterState) next; >>> }; >>> @@ -74,4 +75,10 @@ ssize_t >>> qemu_netfilter_pass_to_next(NetClientState *sender, >>> int iovcnt, >>> void *opaque); >>> >>> +void netdev_add_filter(const char *netdev_id, >>> + const char *filter_type, >>> + const char *id, >>> + bool is_default, >>> + Error **errp); >>> + >>> #endif /* QEMU_NET_FILTER_H */ >>> diff --git a/net/filter.c b/net/filter.c >>> index d08a2be..dc7aa9b 100644 >>> --- a/net/filter.c >>> +++ b/net/filter.c >>> @@ -214,6 +214,86 @@ static void netfilter_complete(UserCreatable >>> *uc, Error **errp) >>> QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next); >>> } >>> >>> +QemuOptsList qemu_filter_opts = { >>> + .name = "default-filter", >>> + .head = QTAILQ_HEAD_INITIALIZER(qemu_filter_opts.head), >>> + .desc = { >>> + { >>> + .name = "qom-type", >>> + .type = QEMU_OPT_STRING, >>> + },{ >>> + .name = "id", >>> + .type = QEMU_OPT_STRING, >>> + },{ >>> + .name = "netdev", >>> + .type = QEMU_OPT_STRING, >>> + },{ >>> + .name = "status", >>> + .type = QEMU_OPT_STRING, >>> + }, >>> + { /* end of list */ } >>> + }, >>> +}; >>> + >>> +static void filter_set_default_flag(const char *id, >>> + bool is_default, >>> + Error **errp) >>> +{ >>> + Object *obj, *container; >>> + NetFilterState *nf; >>> + >>> + container = object_get_objects_root(); >>> + obj = object_resolve_path_component(container, id); >>> + if (!obj) { >>> + error_setg(errp, "object id not found"); >>> + return; >>> + } >>> + nf = NETFILTER(obj); >>> + nf->is_default = is_default; >>> +} >>> + >>> +void netdev_add_filter(const char *netdev_id, >>> + const char *filter_type, >>> + const char *id, >>> + bool is_default, >>> + Error **errp) >>> +{ >>> + NetClientState *nc = qemu_find_netdev(netdev_id); >>> + char *optarg; >>> + QemuOpts *opts = NULL; >>> + Error *err = NULL; >>> + >>> + /* FIXME: Not support multiple queues */ >>> + if (!nc || nc->queue_index > 1) { >>> + return; >>> + } >>> + /* Not support vhost-net */ >>> + if (get_vhost_net(nc)) { >>> + return; >>> + } >>> + >>> + 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. > >> - 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? 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); >> >> >> . >> > > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 7:46 ` Jason Wang @ 2016-02-01 7:56 ` Hailiang Zhang 2016-02-01 8:05 ` Yang Hongyang 2016-02-01 9:04 ` Jason Wang 0 siblings, 2 replies; 33+ messages in thread From: Hailiang Zhang @ 2016-02-01 7:56 UTC (permalink / raw) To: Jason Wang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst, hongyang.yang 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 <zhang.zhanghailiang@huawei.com> >>>> --- >>>> 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(+) >>>> >>>> diff --git a/include/net/filter.h b/include/net/filter.h >>>> index af3c53c..ee1c024 100644 >>>> --- a/include/net/filter.h >>>> +++ b/include/net/filter.h >>>> @@ -55,6 +55,7 @@ struct NetFilterState { >>>> char *netdev_id; >>>> NetClientState *netdev; >>>> NetFilterDirection direction; >>>> + bool is_default; >>>> bool enabled; >>>> QTAILQ_ENTRY(NetFilterState) next; >>>> }; >>>> @@ -74,4 +75,10 @@ ssize_t >>>> qemu_netfilter_pass_to_next(NetClientState *sender, >>>> int iovcnt, >>>> void *opaque); >>>> >>>> +void netdev_add_filter(const char *netdev_id, >>>> + const char *filter_type, >>>> + const char *id, >>>> + bool is_default, >>>> + Error **errp); >>>> + >>>> #endif /* QEMU_NET_FILTER_H */ >>>> diff --git a/net/filter.c b/net/filter.c >>>> index d08a2be..dc7aa9b 100644 >>>> --- a/net/filter.c >>>> +++ b/net/filter.c >>>> @@ -214,6 +214,86 @@ static void netfilter_complete(UserCreatable >>>> *uc, Error **errp) >>>> QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next); >>>> } >>>> >>>> +QemuOptsList qemu_filter_opts = { >>>> + .name = "default-filter", >>>> + .head = QTAILQ_HEAD_INITIALIZER(qemu_filter_opts.head), >>>> + .desc = { >>>> + { >>>> + .name = "qom-type", >>>> + .type = QEMU_OPT_STRING, >>>> + },{ >>>> + .name = "id", >>>> + .type = QEMU_OPT_STRING, >>>> + },{ >>>> + .name = "netdev", >>>> + .type = QEMU_OPT_STRING, >>>> + },{ >>>> + .name = "status", >>>> + .type = QEMU_OPT_STRING, >>>> + }, >>>> + { /* end of list */ } >>>> + }, >>>> +}; >>>> + >>>> +static void filter_set_default_flag(const char *id, >>>> + bool is_default, >>>> + Error **errp) >>>> +{ >>>> + Object *obj, *container; >>>> + NetFilterState *nf; >>>> + >>>> + container = object_get_objects_root(); >>>> + obj = object_resolve_path_component(container, id); >>>> + if (!obj) { >>>> + error_setg(errp, "object id not found"); >>>> + return; >>>> + } >>>> + nf = NETFILTER(obj); >>>> + nf->is_default = is_default; >>>> +} >>>> + >>>> +void netdev_add_filter(const char *netdev_id, >>>> + const char *filter_type, >>>> + const char *id, >>>> + bool is_default, >>>> + Error **errp) >>>> +{ >>>> + NetClientState *nc = qemu_find_netdev(netdev_id); >>>> + char *optarg; >>>> + QemuOpts *opts = NULL; >>>> + Error *err = NULL; >>>> + >>>> + /* FIXME: Not support multiple queues */ >>>> + if (!nc || nc->queue_index > 1) { >>>> + return; >>>> + } >>>> + /* Not support vhost-net */ >>>> + if (get_vhost_net(nc)) { >>>> + return; >>>> + } >>>> + >>>> + 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, 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. 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); >>> >>> >>> . >>> >> >> > > > . > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 7:56 ` Hailiang Zhang @ 2016-02-01 8:05 ` Yang Hongyang 2016-02-01 8:21 ` Hailiang Zhang 2016-02-01 9:04 ` Jason Wang 1 sibling, 1 reply; 33+ messages in thread From: Yang Hongyang @ 2016-02-01 8:05 UTC (permalink / raw) To: Hailiang Zhang, Jason Wang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst 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 <zhang.zhanghailiang@huawei.com> >>>>> --- >>>>> 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(+) >>>>> >>>>> diff --git a/include/net/filter.h b/include/net/filter.h >>>>> index af3c53c..ee1c024 100644 >>>>> --- a/include/net/filter.h >>>>> +++ b/include/net/filter.h >>>>> @@ -55,6 +55,7 @@ struct NetFilterState { >>>>> char *netdev_id; >>>>> NetClientState *netdev; >>>>> NetFilterDirection direction; >>>>> + bool is_default; >>>>> bool enabled; >>>>> QTAILQ_ENTRY(NetFilterState) next; >>>>> }; >>>>> @@ -74,4 +75,10 @@ ssize_t >>>>> qemu_netfilter_pass_to_next(NetClientState *sender, >>>>> int iovcnt, >>>>> void *opaque); >>>>> >>>>> +void netdev_add_filter(const char *netdev_id, >>>>> + const char *filter_type, >>>>> + const char *id, >>>>> + bool is_default, >>>>> + Error **errp); >>>>> + >>>>> #endif /* QEMU_NET_FILTER_H */ >>>>> diff --git a/net/filter.c b/net/filter.c >>>>> index d08a2be..dc7aa9b 100644 >>>>> --- a/net/filter.c >>>>> +++ b/net/filter.c >>>>> @@ -214,6 +214,86 @@ static void netfilter_complete(UserCreatable >>>>> *uc, Error **errp) >>>>> QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next); >>>>> } >>>>> >>>>> +QemuOptsList qemu_filter_opts = { >>>>> + .name = "default-filter", >>>>> + .head = QTAILQ_HEAD_INITIALIZER(qemu_filter_opts.head), >>>>> + .desc = { >>>>> + { >>>>> + .name = "qom-type", >>>>> + .type = QEMU_OPT_STRING, >>>>> + },{ >>>>> + .name = "id", >>>>> + .type = QEMU_OPT_STRING, >>>>> + },{ >>>>> + .name = "netdev", >>>>> + .type = QEMU_OPT_STRING, >>>>> + },{ >>>>> + .name = "status", >>>>> + .type = QEMU_OPT_STRING, >>>>> + }, >>>>> + { /* end of list */ } >>>>> + }, >>>>> +}; >>>>> + >>>>> +static void filter_set_default_flag(const char *id, >>>>> + bool is_default, >>>>> + Error **errp) >>>>> +{ >>>>> + Object *obj, *container; >>>>> + NetFilterState *nf; >>>>> + >>>>> + container = object_get_objects_root(); >>>>> + obj = object_resolve_path_component(container, id); >>>>> + if (!obj) { >>>>> + error_setg(errp, "object id not found"); >>>>> + return; >>>>> + } >>>>> + nf = NETFILTER(obj); >>>>> + nf->is_default = is_default; >>>>> +} >>>>> + >>>>> +void netdev_add_filter(const char *netdev_id, >>>>> + const char *filter_type, >>>>> + const char *id, >>>>> + bool is_default, >>>>> + Error **errp) >>>>> +{ >>>>> + NetClientState *nc = qemu_find_netdev(netdev_id); >>>>> + char *optarg; >>>>> + QemuOpts *opts = NULL; >>>>> + Error *err = NULL; >>>>> + >>>>> + /* FIXME: Not support multiple queues */ >>>>> + if (!nc || nc->queue_index > 1) { >>>>> + return; >>>>> + } >>>>> + /* Not support vhost-net */ >>>>> + if (get_vhost_net(nc)) { >>>>> + return; >>>>> + } >>>>> + >>>>> + 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, > 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 filter and other filter is that COLO's filter (for example buffer filter) is added by COLO, and the other filter is added by user specifing -filter. > > 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); >>>> >>>> >>>> . >>>> >>> >>> >> >> >> . >> > > > -- Thanks, Yang ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 8:05 ` Yang Hongyang @ 2016-02-01 8:21 ` Hailiang Zhang 2016-02-01 9:18 ` Jason Wang 0 siblings, 1 reply; 33+ messages in thread From: Hailiang Zhang @ 2016-02-01 8:21 UTC (permalink / raw) To: Yang Hongyang, Jason Wang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst On 2016/2/1 16:05, Yang Hongyang 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 <zhang.zhanghailiang@huawei.com> >>>>>> --- >>>>>> 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(+) >>>>>> >>>>>> diff --git a/include/net/filter.h b/include/net/filter.h >>>>>> index af3c53c..ee1c024 100644 >>>>>> --- a/include/net/filter.h >>>>>> +++ b/include/net/filter.h >>>>>> @@ -55,6 +55,7 @@ struct NetFilterState { >>>>>> char *netdev_id; >>>>>> NetClientState *netdev; >>>>>> NetFilterDirection direction; >>>>>> + bool is_default; >>>>>> bool enabled; >>>>>> QTAILQ_ENTRY(NetFilterState) next; >>>>>> }; >>>>>> @@ -74,4 +75,10 @@ ssize_t >>>>>> qemu_netfilter_pass_to_next(NetClientState *sender, >>>>>> int iovcnt, >>>>>> void *opaque); >>>>>> >>>>>> +void netdev_add_filter(const char *netdev_id, >>>>>> + const char *filter_type, >>>>>> + const char *id, >>>>>> + bool is_default, >>>>>> + Error **errp); >>>>>> + >>>>>> #endif /* QEMU_NET_FILTER_H */ >>>>>> diff --git a/net/filter.c b/net/filter.c >>>>>> index d08a2be..dc7aa9b 100644 >>>>>> --- a/net/filter.c >>>>>> +++ b/net/filter.c >>>>>> @@ -214,6 +214,86 @@ static void netfilter_complete(UserCreatable >>>>>> *uc, Error **errp) >>>>>> QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next); >>>>>> } >>>>>> >>>>>> +QemuOptsList qemu_filter_opts = { >>>>>> + .name = "default-filter", >>>>>> + .head = QTAILQ_HEAD_INITIALIZER(qemu_filter_opts.head), >>>>>> + .desc = { >>>>>> + { >>>>>> + .name = "qom-type", >>>>>> + .type = QEMU_OPT_STRING, >>>>>> + },{ >>>>>> + .name = "id", >>>>>> + .type = QEMU_OPT_STRING, >>>>>> + },{ >>>>>> + .name = "netdev", >>>>>> + .type = QEMU_OPT_STRING, >>>>>> + },{ >>>>>> + .name = "status", >>>>>> + .type = QEMU_OPT_STRING, >>>>>> + }, >>>>>> + { /* end of list */ } >>>>>> + }, >>>>>> +}; >>>>>> + >>>>>> +static void filter_set_default_flag(const char *id, >>>>>> + bool is_default, >>>>>> + Error **errp) >>>>>> +{ >>>>>> + Object *obj, *container; >>>>>> + NetFilterState *nf; >>>>>> + >>>>>> + container = object_get_objects_root(); >>>>>> + obj = object_resolve_path_component(container, id); >>>>>> + if (!obj) { >>>>>> + error_setg(errp, "object id not found"); >>>>>> + return; >>>>>> + } >>>>>> + nf = NETFILTER(obj); >>>>>> + nf->is_default = is_default; >>>>>> +} >>>>>> + >>>>>> +void netdev_add_filter(const char *netdev_id, >>>>>> + const char *filter_type, >>>>>> + const char *id, >>>>>> + bool is_default, >>>>>> + Error **errp) >>>>>> +{ >>>>>> + NetClientState *nc = qemu_find_netdev(netdev_id); >>>>>> + char *optarg; >>>>>> + QemuOpts *opts = NULL; >>>>>> + Error *err = NULL; >>>>>> + >>>>>> + /* FIXME: Not support multiple queues */ >>>>>> + if (!nc || nc->queue_index > 1) { >>>>>> + return; >>>>>> + } >>>>>> + /* Not support vhost-net */ >>>>>> + if (get_vhost_net(nc)) { >>>>>> + return; >>>>>> + } >>>>>> + >>>>>> + 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, >> 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 ?' If we add the a filter to netdev when doing COLO, it will be added dynamically, Here we want to add each netdev a default filter while launch QEMU (no matter if this VM will go into COLO or not), just to support hot-add NIC for VM while in COLO lifetime. > filter and other filter is that COLO's filter (for example buffer > filter) is added by COLO, and the other filter is added by user > specifing -filter. > >> >> 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); >>>>> >>>>> >>>>> . >>>>> >>>> >>>> >>> >>> >>> . >>> >> >> >> > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 8:21 ` Hailiang Zhang @ 2016-02-01 9:18 ` Jason Wang 2016-02-01 9:39 ` Hailiang Zhang 0 siblings, 1 reply; 33+ messages in thread From: Jason Wang @ 2016-02-01 9:18 UTC (permalink / raw) To: Hailiang Zhang, Yang Hongyang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst 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. 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 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. Any thoughts? > If we add the a filter to netdev when doing COLO, it will be added > dynamically, > Here we want to add each netdev a default filter while launch QEMU > (no matter if this VM will go into COLO or not), > just to support hot-add NIC for VM while in COLO lifetime. Yes. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 9:18 ` Jason Wang @ 2016-02-01 9:39 ` Hailiang Zhang 2016-02-01 9:49 ` Jason Wang 0 siblings, 1 reply; 33+ messages in thread From: Hailiang Zhang @ 2016-02-01 9:39 UTC (permalink / raw) To: Jason Wang, Yang Hongyang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst 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. 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. > 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 > Any thoughts? > >> If we add the a filter to netdev when doing COLO, it will be added >> dynamically, >> Here we want to add each netdev a default filter while launch QEMU >> (no matter if this VM will go into COLO or not), >> just to support hot-add NIC for VM while in COLO lifetime. > > Yes. > > > > > . > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 9:39 ` Hailiang Zhang @ 2016-02-01 9:49 ` Jason Wang 2016-02-01 10:41 ` Hailiang Zhang 0 siblings, 1 reply; 33+ messages in thread From: Jason Wang @ 2016-02-01 9:49 UTC (permalink / raw) To: Hailiang Zhang, Yang Hongyang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 9:49 ` Jason Wang @ 2016-02-01 10:41 ` Hailiang Zhang 0 siblings, 0 replies; 33+ messages in thread From: Hailiang Zhang @ 2016-02-01 10:41 UTC (permalink / raw) To: Jason Wang, Yang Hongyang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst 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 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 7:56 ` Hailiang Zhang 2016-02-01 8:05 ` Yang Hongyang @ 2016-02-01 9:04 ` Jason Wang 2016-02-01 9:22 ` Hailiang Zhang 1 sibling, 1 reply; 33+ messages in thread From: Jason Wang @ 2016-02-01 9:04 UTC (permalink / raw) To: Hailiang Zhang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst, hongyang.yang 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 <zhang.zhanghailiang@huawei.com> >>>>> --- >>>>> 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? > 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); >>>> >>>> >>>> . >>>> >>> >>> >> >> >> . >> > > > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 9:04 ` Jason Wang @ 2016-02-01 9:22 ` Hailiang Zhang 2016-02-01 9:42 ` Jason Wang 0 siblings, 1 reply; 33+ messages in thread From: Hailiang Zhang @ 2016-02-01 9:22 UTC (permalink / raw) To: Jason Wang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst, hongyang.yang 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 <zhang.zhanghailiang@huawei.com> >>>>>> --- >>>>>> 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); >>>>> >>>>> >>>>> . >>>>> >>>> >>>> >>> >>> >>> . >>> >> >> >> > > > . > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 9:22 ` Hailiang Zhang @ 2016-02-01 9:42 ` Jason Wang 2016-02-01 10:40 ` Hailiang Zhang 2016-02-01 12:21 ` Hailiang Zhang 0 siblings, 2 replies; 33+ messages in thread From: Jason Wang @ 2016-02-01 9:42 UTC (permalink / raw) To: Hailiang Zhang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst, hongyang.yang 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 <zhang.zhanghailiang@huawei.com> >>>>>>> --- >>>>>>> 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. > 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 9:42 ` Jason Wang @ 2016-02-01 10:40 ` Hailiang Zhang 2016-02-05 6:19 ` Jason Wang 2016-02-01 12:21 ` Hailiang Zhang 1 sibling, 1 reply; 33+ messages in thread From: Hailiang Zhang @ 2016-02-01 10:40 UTC (permalink / raw) To: Jason Wang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst, hongyang.yang 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 <zhang.zhanghailiang@huawei.com> >>>>>>>> --- >>>>>>>> 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. > > > . > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 10:40 ` Hailiang Zhang @ 2016-02-05 6:19 ` Jason Wang 2016-02-05 7:01 ` Hailiang Zhang 0 siblings, 1 reply; 33+ messages in thread From: Jason Wang @ 2016-02-05 6:19 UTC (permalink / raw) To: Hailiang Zhang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst, hongyang.yang On 02/01/2016 06:40 PM, Hailiang Zhang wrote: > 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 <zhang.zhanghailiang@huawei.com> >>>>>>>>> --- >>>>>>>>> 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. Then I see minor issues: 1) Looks like we should prevent user other than COLO from using zero interval, neither from cli nor 'qom-set'. 2) For the zero interval filter used by COLO, we should not allow user to change the value of interval. So I was thinking a new type which is based on current filter-buffer whose interval is invisible to user. > 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. >> >> >> . >> > > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-05 6:19 ` Jason Wang @ 2016-02-05 7:01 ` Hailiang Zhang 2016-02-05 7:40 ` Jason Wang 0 siblings, 1 reply; 33+ messages in thread From: Hailiang Zhang @ 2016-02-05 7:01 UTC (permalink / raw) To: Jason Wang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst, hongyang.yang On 2016/2/5 14:19, Jason Wang wrote: > > > On 02/01/2016 06:40 PM, Hailiang Zhang wrote: >> 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 <zhang.zhanghailiang@huawei.com> >>>>>>>>>> --- >>>>>>>>>> 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. > > Then I see minor issues: > Great, you are still online :) > 1) Looks like we should prevent user other than COLO from using zero > interval, neither from cli nor 'qom-set'. > 2) For the zero interval filter used by COLO, we should not allow user > to change the value of interval. > > So I was thinking a new type which is based on current filter-buffer > whose interval is invisible to user. > It seems that, we have prevented this case, if we set the interval to zero through qom-set, it will report error: "Property 'filter-buffer.interval' requires a positive value", also if launch qemu with CLI "-object filter-buffer,id=f0,netdev=hn0,queue=rx,interval=0", it also reported the error "Property 'filter-buffer.interval' requires a positive value". This is because we judge this value in filter_buffer_set_interval(), but the 'interval' property is optional, users can use ignore this property while launch qemu and its default value will be zero, so we still can't use this value to identify default filter. Other choices to drop the troublesome 'is_default' flag is, 1) use a list to store all the default filters, add them to the list in netdev_add_default_filters(). but we need add a new member 'QTAILQ_ENTRY(NetFilterState) internal_next' to struct NetFilterState, The benefit of this scheme is, it will more easy to traverse and control the default filters, we don't have to traverse all netfilters to identify them. 2) Use the name to identify them, more the name rule for default filter is '[netdev->id]nop'. Which one do you think is more acceptable ? Or any other suggestions ? Thanks, Hailiang >> 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. >>> >>> >>> . >>> >> >> > > > . > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-05 7:01 ` Hailiang Zhang @ 2016-02-05 7:40 ` Jason Wang 2016-02-05 8:29 ` Hailiang Zhang 0 siblings, 1 reply; 33+ messages in thread From: Jason Wang @ 2016-02-05 7:40 UTC (permalink / raw) To: Hailiang Zhang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst, hongyang.yang On 02/05/2016 03:01 PM, Hailiang Zhang wrote: > On 2016/2/5 14:19, Jason Wang wrote: >> >> >> On 02/01/2016 06:40 PM, Hailiang Zhang wrote: >>> 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 <zhang.zhanghailiang@huawei.com> >>>>>>>>>>> --- >>>>>>>>>>> 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. >> >> Then I see minor issues: >> > > Great, you are still online :) Right :) > >> 1) Looks like we should prevent user other than COLO from using zero >> interval, neither from cli nor 'qom-set'. >> 2) For the zero interval filter used by COLO, we should not allow user >> to change the value of interval. >> >> So I was thinking a new type which is based on current filter-buffer >> whose interval is invisible to user. >> > > It seems that, we have prevented this case, if we set the interval > to zero through qom-set, it will report error: > "Property 'filter-buffer.interval' requires a positive value", also if > launch qemu with CLI "-object > filter-buffer,id=f0,netdev=hn0,queue=rx,interval=0", > it also reported the error "Property 'filter-buffer.interval' requires > a positive value". > > This is because we judge this value in filter_buffer_set_interval(), > but the 'interval' property is optional, users can use ignore this > property while launch > qemu and its default value will be zero, so we still can't use this > value to identify default > filter. Cool, good to know this. > > Other choices to drop the troublesome 'is_default' flag is, > 1) use a list to store all the default filters, add them to the list in > netdev_add_default_filters(). but we need add a new member > 'QTAILQ_ENTRY(NetFilterState) internal_next' to struct NetFilterState, > The benefit of this scheme is, it will more easy to traverse and control > the default filters, we don't have to traverse all netfilters to > identify them. This is pretty similar to what I suggested above. The only difference is that you propose to track this at filter layer instead of buffer filter implementation. Think hard about this. Consider: 1) Buffer is the only user for something like default filter and there's no plan for exporting default filter configuration through cli (at least in short term). 2) All default filter buffer needs to be tracked in a list for COLO to buffer/relase packets. Adding the ability of default filter is good but may need more thoughts. For example, cli design and to be generic enough for all kinds of filter (you may want to generate filename dynamically for dump as a default filter which sounds bad). To reduce the extra efforts and converge this soon , maybe we could do something more simpler. - Instead of a explicit default filter, using something like notifier/callback in net_dev_init(). Then COLO can register a callback for this notifier. Each time a netdev is created, COLO will be notified and can do whatever it wants (E.g adding a buffer filter with zero interval). - With above, there's no need to care about the zero interval setting from cli. Then filter buffer can track all filters with zero interval, and export helpers for COLO to buffer or release the packet. Thoughts? Thanks > 2) Use the name to identify them, more the name rule for default > filter is > '[netdev->id]nop'. > > Which one do you think is more acceptable ? Or any other suggestions ? > > Thanks, > Hailiang > >>> 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. >>>> >>>> >>>> . >>>> >>> >>> >> >> >> . >> > > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-05 7:40 ` Jason Wang @ 2016-02-05 8:29 ` Hailiang Zhang 0 siblings, 0 replies; 33+ messages in thread From: Hailiang Zhang @ 2016-02-05 8:29 UTC (permalink / raw) To: Jason Wang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst, hongyang.yang On 2016/2/5 15:40, Jason Wang wrote: > > > On 02/05/2016 03:01 PM, Hailiang Zhang wrote: >> On 2016/2/5 14:19, Jason Wang wrote: >>> >>> >>> On 02/01/2016 06:40 PM, Hailiang Zhang wrote: >>>> 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 <zhang.zhanghailiang@huawei.com> >>>>>>>>>>>> --- >>>>>>>>>>>> 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. >>> >>> Then I see minor issues: >>> >> >> Great, you are still online :) > > Right :) > >> >>> 1) Looks like we should prevent user other than COLO from using zero >>> interval, neither from cli nor 'qom-set'. >>> 2) For the zero interval filter used by COLO, we should not allow user >>> to change the value of interval. >>> >>> So I was thinking a new type which is based on current filter-buffer >>> whose interval is invisible to user. >>> >> >> It seems that, we have prevented this case, if we set the interval >> to zero through qom-set, it will report error: >> "Property 'filter-buffer.interval' requires a positive value", also if >> launch qemu with CLI "-object >> filter-buffer,id=f0,netdev=hn0,queue=rx,interval=0", >> it also reported the error "Property 'filter-buffer.interval' requires >> a positive value". >> >> This is because we judge this value in filter_buffer_set_interval(), >> but the 'interval' property is optional, users can use ignore this >> property while launch >> qemu and its default value will be zero, so we still can't use this >> value to identify default >> filter. > > Cool, good to know this. > >> >> Other choices to drop the troublesome 'is_default' flag is, >> 1) use a list to store all the default filters, add them to the list in >> netdev_add_default_filters(). but we need add a new member >> 'QTAILQ_ENTRY(NetFilterState) internal_next' to struct NetFilterState, >> The benefit of this scheme is, it will more easy to traverse and control >> the default filters, we don't have to traverse all netfilters to >> identify them. > > This is pretty similar to what I suggested above. The only difference > is that you propose to track this at filter layer instead of buffer > filter implementation. > > Think hard about this. Consider: > > 1) Buffer is the only user for something like default filter and there's > no plan for exporting default filter configuration through cli (at least > in short term). > 2) All default filter buffer needs to be tracked in a list for COLO to > buffer/relase packets. > > Adding the ability of default filter is good but may need more thoughts. > For example, cli design and to be generic enough for all kinds of filter > (you may want to generate filename dynamically for dump as a default > filter which sounds bad). To reduce the extra efforts and converge this > soon , maybe we could do something more simpler. > Totally agree, and for now, it seems that only COLO use the buffer filter. After colo-proxy is merged, COLO will switch to colo-proxy. So realizing it in a simple way is a good idea. > - Instead of a explicit default filter, using something like > notifier/callback in net_dev_init(). Then COLO can register a callback > for this notifier. Each time a netdev is created, COLO will be notified > and can do whatever it wants (E.g adding a buffer filter with zero > interval). Yes, in this way, we can still support hot-add netfilter, and can also handle it with the later colo-proxy. > - With above, there's no need to care about the zero interval setting > from cli. Then filter buffer can track all filters with zero interval, > and export helpers for COLO to buffer or release the packet. > > Thank you very much for your patience :) > Thoughts? > > Thanks > >> 2) Use the name to identify them, more the name rule for default >> filter is >> '[netdev->id]nop'. >> >> Which one do you think is more acceptable ? Or any other suggestions ? >> >> Thanks, >> Hailiang >> >>>> 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. >>>>> >>>>> >>>>> . >>>>> >>>> >>>> >>> >>> >>> . >>> >> >> > > > . > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 9:42 ` Jason Wang 2016-02-01 10:40 ` Hailiang Zhang @ 2016-02-01 12:21 ` Hailiang Zhang 1 sibling, 0 replies; 33+ messages in thread From: Hailiang Zhang @ 2016-02-01 12:21 UTC (permalink / raw) To: Jason Wang, qemu-devel; +Cc: peter.huangpeng, zhangchen.fnst, hongyang.yang Hi Jason, 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 <zhang.zhanghailiang@huawei.com> >>>>>>>> --- >>>>>>>> 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. > Danies recommended using object_new_with_props() instead of object_create(), which will avoids the need of format + parse process. It makes the codes more clean. I have fixed it in v3. I kept the 'is_default' member for struct NetFilterState. Because comparing with finding the default filter by the special name ([netdev->id][nop]), it seems to be more easy for COLO to find/control the default filter with this flag. (We added a new helper qemu_foreach_netfilter() in COLO frame, and we traverse these filters directly without passing any netdev related info.) But if you think it is still not acceptable to add such a member, i'll remove it in next verison ;) 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. > > > . > ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev zhanghailiang 2016-02-01 3:14 ` Jason Wang @ 2016-02-01 10:43 ` Daniel P. Berrange 2016-02-01 10:57 ` Hailiang Zhang 1 sibling, 1 reply; 33+ messages in thread From: Daniel P. Berrange @ 2016-02-01 10:43 UTC (permalink / raw) To: zhanghailiang; +Cc: jasowang, qemu-devel, zhangchen.fnst, hongyang.yang On Wed, Jan 27, 2016 at 04:29:38PM +0800, 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 <zhang.zhanghailiang@huawei.com> > --- > 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(+) > +void netdev_add_filter(const char *netdev_id, > + const char *filter_type, > + const char *id, > + bool is_default, > + Error **errp) > +{ > + NetClientState *nc = qemu_find_netdev(netdev_id); > + char *optarg; > + QemuOpts *opts = NULL; > + Error *err = NULL; > + > + /* FIXME: Not support multiple queues */ > + if (!nc || nc->queue_index > 1) { > + return; > + } > + /* Not support vhost-net */ > + if (get_vhost_net(nc)) { > + return; > + } > + > + optarg = g_strdup_printf("qom-type=%s,id=%s,netdev=%s,status=%s", > + filter_type, id, netdev_id, is_default ? "disable" : "enable"); > + opts = qemu_opts_parse_noisily(&qemu_filter_opts, > + optarg, false); Formatting a string and then immediately parsing it again is totally crazy, not least because you're not taking care to do escaping of special characters like ',' in the string parameters. > + 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; > + } Don't use object_create() - use object_new_with_props() which avoids the need to format + parse the string above. ie do object_new_with_props(filter_type, object_get_objects_root(), id, &err, "netdev", netdev_id, "status", is_default ? "disable" : "enable", NULL); Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev 2016-02-01 10:43 ` Daniel P. Berrange @ 2016-02-01 10:57 ` Hailiang Zhang 0 siblings, 0 replies; 33+ messages in thread From: Hailiang Zhang @ 2016-02-01 10:57 UTC (permalink / raw) To: Daniel P. Berrange Cc: jasowang, hongyang.yang, peter.huangpeng, zhangchen.fnst, qemu-devel On 2016/2/1 18:43, Daniel P. Berrange wrote: > On Wed, Jan 27, 2016 at 04:29:38PM +0800, 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 <zhang.zhanghailiang@huawei.com> >> --- >> 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(+) > >> +void netdev_add_filter(const char *netdev_id, >> + const char *filter_type, >> + const char *id, >> + bool is_default, >> + Error **errp) >> +{ >> + NetClientState *nc = qemu_find_netdev(netdev_id); >> + char *optarg; >> + QemuOpts *opts = NULL; >> + Error *err = NULL; >> + >> + /* FIXME: Not support multiple queues */ >> + if (!nc || nc->queue_index > 1) { >> + return; >> + } >> + /* Not support vhost-net */ >> + if (get_vhost_net(nc)) { >> + return; >> + } >> + >> + optarg = g_strdup_printf("qom-type=%s,id=%s,netdev=%s,status=%s", >> + filter_type, id, netdev_id, is_default ? "disable" : "enable"); >> + opts = qemu_opts_parse_noisily(&qemu_filter_opts, >> + optarg, false); > > Formatting a string and then immediately parsing it again is totally > crazy, not least because you're not taking care to do escaping of > special characters like ',' in the string parameters. > Got it. >> + 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; >> + } > > Don't use object_create() - use object_new_with_props() which avoids > the need to format + parse the string above. ie do > > object_new_with_props(filter_type, > object_get_objects_root(), > id, > &err, > "netdev", netdev_id, > "status", is_default ? "disable" : "enable", > NULL); > > Ha, that's really a good idea, i will fix it like that in next version. Thank you very much. Hailiang > Regards, > Daniel > ^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH RFC v2 4/5] filter-buffer: Accept zero interval 2016-01-27 8:29 [Qemu-devel] [PATCH RFC v2 0/5] Netfilter: Add each netdev a default filter zhanghailiang ` (2 preceding siblings ...) 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev zhanghailiang @ 2016-01-27 8:29 ` zhanghailiang 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 5/5] net/filter: Add a default filter to each netdev zhanghailiang 4 siblings, 0 replies; 33+ messages in thread From: zhanghailiang @ 2016-01-27 8:29 UTC (permalink / raw) To: qemu-devel; +Cc: jasowang, zhanghailiang, zhangchen.fnst, hongyang.yang We may want to accept zero interval when VM FT solutions like MC or COLO use this filter to release packets on demand. Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn> --- net/filter-buffer.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/net/filter-buffer.c b/net/filter-buffer.c index 57be149..12e0c87 100644 --- a/net/filter-buffer.c +++ b/net/filter-buffer.c @@ -103,16 +103,6 @@ static void filter_buffer_setup(NetFilterState *nf, Error **errp) { FilterBufferState *s = FILTER_BUFFER(nf); - /* - * We may want to accept zero interval when VM FT solutions like MC - * or COLO use this filter to release packets on demand. - */ - if (!s->interval) { - error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "interval", - "a non-zero interval"); - return; - } - s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf); if (s->interval) { timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL, -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH RFC v2 5/5] net/filter: Add a default filter to each netdev 2016-01-27 8:29 [Qemu-devel] [PATCH RFC v2 0/5] Netfilter: Add each netdev a default filter zhanghailiang ` (3 preceding siblings ...) 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 4/5] filter-buffer: Accept zero interval zhanghailiang @ 2016-01-27 8:29 ` zhanghailiang 4 siblings, 0 replies; 33+ messages in thread From: zhanghailiang @ 2016-01-27 8:29 UTC (permalink / raw) To: qemu-devel; +Cc: jasowang, zhanghailiang, zhangchen.fnst, hongyang.yang We add each netdev a default buffer filter, and the default buffer filter is disabled, so it has no side effect for packets delivering in qemu net layer. The default buffer filter can be used by COLO or Micro-checkpoint, The reason we add the default filter is we hope to support hot add network during COLO state in future. Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> --- v2: - Add codes that generate id automatically for default filter (Jason's suggestion) - Some other minor fixes. --- include/net/filter.h | 4 ++++ net/net.c | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/include/net/filter.h b/include/net/filter.h index ee1c024..e443f9c 100644 --- a/include/net/filter.h +++ b/include/net/filter.h @@ -22,6 +22,10 @@ #define NETFILTER_CLASS(klass) \ OBJECT_CLASS_CHECK(NetFilterClass, (klass), TYPE_NETFILTER) +#define DEFAULT_FILTER_TYPE "nop" + +#define TYPE_FILTER_BUFFER "filter-buffer" + typedef void (FilterSetup) (NetFilterState *nf, Error **errp); typedef void (FilterCleanup) (NetFilterState *nf); /* diff --git a/net/net.c b/net/net.c index a49af48..12c13f9 100644 --- a/net/net.c +++ b/net/net.c @@ -77,6 +77,12 @@ const char *host_net_devices[] = { int default_net = 1; +/* + * TODO: Export this with an option for users to control + * this with comand line ? + */ +char default_netfilter_type[16] = TYPE_FILTER_BUFFER; + /***********************************************************/ /* network device redirectors */ @@ -1029,6 +1035,23 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp) } return -1; } + + if (is_netdev) { + const Netdev *netdev = object; + char default_name[128]; + + snprintf(default_name, sizeof(default_name), + "%s%s0", netdev->id, DEFAULT_FILTER_TYPE); + /* + * Here we add each netdev a default filter, + * it will disabled by default, Users can enable it when necessary. + */ + netdev_add_filter(netdev->id, + default_netfilter_type, + default_name, + true, + errp); + } return 0; } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 33+ messages in thread
end of thread, other threads:[~2016-02-05 8:30 UTC | newest] Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-01-27 8:29 [Qemu-devel] [PATCH RFC v2 0/5] Netfilter: Add each netdev a default filter zhanghailiang 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 1/5] net/filter: Add a 'status' property for filter object zhanghailiang 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 2/5] vl: Make object_create() public zhanghailiang 2016-02-01 3:05 ` Jason Wang 2016-02-01 6:19 ` Hailiang Zhang 2016-02-01 7:27 ` Jason Wang 2016-02-01 7:34 ` Hailiang Zhang 2016-02-01 10:41 ` Daniel P. Berrange 2016-02-01 10:44 ` Hailiang Zhang 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 3/5] net/filter: Introduce a helper to add a filter to the netdev zhanghailiang 2016-02-01 3:14 ` Jason Wang 2016-02-01 6:13 ` Hailiang Zhang 2016-02-01 7:46 ` Jason Wang 2016-02-01 7:56 ` Hailiang Zhang 2016-02-01 8:05 ` Yang Hongyang 2016-02-01 8:21 ` Hailiang Zhang 2016-02-01 9:18 ` Jason Wang 2016-02-01 9:39 ` Hailiang Zhang 2016-02-01 9:49 ` Jason Wang 2016-02-01 10:41 ` Hailiang Zhang 2016-02-01 9:04 ` Jason Wang 2016-02-01 9:22 ` Hailiang Zhang 2016-02-01 9:42 ` Jason Wang 2016-02-01 10:40 ` Hailiang Zhang 2016-02-05 6:19 ` Jason Wang 2016-02-05 7:01 ` Hailiang Zhang 2016-02-05 7:40 ` Jason Wang 2016-02-05 8:29 ` Hailiang Zhang 2016-02-01 12:21 ` Hailiang Zhang 2016-02-01 10:43 ` Daniel P. Berrange 2016-02-01 10:57 ` Hailiang Zhang 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 4/5] filter-buffer: Accept zero interval zhanghailiang 2016-01-27 8:29 ` [Qemu-devel] [PATCH RFC v2 5/5] net/filter: Add a default filter to each netdev zhanghailiang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.