Message ID | 1451372975-5048-35-git-send-email-zhang.zhanghailiang@huawei.com |
---|---|
State | New |
Headers | show |
Hi Jason, Could you please help reviewing the filter part of this series ? Thanks, Hailiang On 2015/12/29 15:09, zhanghailiang wrote: > We add each netdev (except vhost-net) a default filter-buffer, > which will be used for COLO or Micro-checkpoint to buffer VM's packets. > The name of default filter-buffer is 'nop'. > For the default filter-buffer, it will not buffer any packets in default. > So it has no side effect for the netdev. > > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > Cc: Jason Wang <jasowang@redhat.com> > Cc: Yang Hongyang <hongyang.yang@easystack.cn> > --- > v12: > - Skip vhost-net when add default filter > - Don't go through filter layer if the filter is disabled. > v11: > - New patch > --- > include/net/filter.h | 10 +++++++ > net/filter-buffer.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > net/filter.c | 6 +++- > net/net.c | 12 ++++++++ > 4 files changed, 109 insertions(+), 1 deletion(-) > > diff --git a/include/net/filter.h b/include/net/filter.h > index 2deda36..40aa38c 100644 > --- a/include/net/filter.h > +++ b/include/net/filter.h > @@ -56,6 +56,8 @@ struct NetFilterState { > NetClientState *netdev; > NetFilterDirection direction; > char info_str[256]; > + bool is_default; > + bool enabled; > QTAILQ_ENTRY(NetFilterState) next; > }; > > @@ -74,4 +76,12 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, > int iovcnt, > void *opaque); > > +static inline bool qemu_need_skip_netfilter(NetFilterState *nf) > +{ > + return nf->enabled ? false : true; > +} > + > +void netdev_add_default_filter_buffer(const char *netdev_id, > + NetFilterDirection direction, > + Error **errp); > #endif /* QEMU_NET_FILTER_H */ > diff --git a/net/filter-buffer.c b/net/filter-buffer.c > index 57be149..9cf3544 100644 > --- a/net/filter-buffer.c > +++ b/net/filter-buffer.c > @@ -14,6 +14,13 @@ > #include "qapi/qmp/qerror.h" > #include "qapi-visit.h" > #include "qom/object.h" > +#include "net/net.h" > +#include "qapi/qmp/qdict.h" > +#include "qapi/qmp-output-visitor.h" > +#include "qapi/qmp-input-visitor.h" > +#include "monitor/monitor.h" > +#include "qmp-commands.h" > +#include "net/vhost_net.h" > > #define TYPE_FILTER_BUFFER "filter-buffer" > > @@ -102,6 +109,7 @@ static void filter_buffer_cleanup(NetFilterState *nf) > static void filter_buffer_setup(NetFilterState *nf, Error **errp) > { > FilterBufferState *s = FILTER_BUFFER(nf); > + char *path = object_get_canonical_path_component(OBJECT(nf)); > > /* > * We may want to accept zero interval when VM FT solutions like MC > @@ -114,6 +122,14 @@ static void filter_buffer_setup(NetFilterState *nf, Error **errp) > } > > s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf); > + nf->is_default = !strcmp(path, "nop"); > + /* > + * For the default buffer filter, it will be disabled by default, > + * So it will not buffer any packets. > + */ > + if (nf->is_default) { > + nf->enabled = false; > + } > if (s->interval) { > timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL, > filter_buffer_release_timer, nf); > @@ -163,6 +179,72 @@ out: > error_propagate(errp, local_err); > } > > +/* > +* This will be used by COLO or MC FT, for which they will need > +* to buffer the packets of VM's net devices, Here we add a default > +* buffer filter for each netdev. The name of default buffer filter is > +* 'nop' > +*/ > +void netdev_add_default_filter_buffer(const char *netdev_id, > + NetFilterDirection direction, > + Error **errp) > +{ > + QmpOutputVisitor *qov; > + QmpInputVisitor *qiv; > + Visitor *ov, *iv; > + QObject *obj = NULL; > + QDict *qdict; > + void *dummy = NULL; > + const char *id = "nop"; > + char *queue = g_strdup(NetFilterDirection_lookup[direction]); > + NetClientState *nc = qemu_find_netdev(netdev_id); > + Error *err = NULL; > + > + /* FIXME: Not support multiple queues */ > + if (!nc || nc->queue_index > 1) { > + g_free(queue); > + return; > + } > + /* Not support vhost-net */ > + if (get_vhost_net(nc)) { > + g_free(queue); > + return; > + } > + qov = qmp_output_visitor_new(); > + ov = qmp_output_get_visitor(qov); > + visit_start_struct(ov, &dummy, NULL, NULL, 0, &err); > + if (err) { > + goto out; > + } > + visit_type_str(ov, &nc->name, "netdev", &err); > + if (err) { > + goto out; > + } > + visit_type_str(ov, &queue, "queue", &err); > + if (err) { > + goto out; > + } > + visit_end_struct(ov, &err); > + if (err) { > + goto out; > + } > + obj = qmp_output_get_qobject(qov); > + g_assert(obj != NULL); > + qdict = qobject_to_qdict(obj); > + qmp_output_visitor_cleanup(qov); > + > + qiv = qmp_input_visitor_new(obj); > + iv = qmp_input_get_visitor(qiv); > + object_add(TYPE_FILTER_BUFFER, id, qdict, iv, &err); > + qmp_input_visitor_cleanup(qiv); > + qobject_decref(obj); > +out: > + g_free(queue); > + if (err) { > + error_propagate(errp, err); > + } > +} > + > static void filter_buffer_init(Object *obj) > { > object_property_add(obj, "interval", "int", > diff --git a/net/filter.c b/net/filter.c > index 1365bad..0b1e408 100644 > --- a/net/filter.c > +++ b/net/filter.c > @@ -163,7 +163,8 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) > } > > nf->netdev = ncs[0]; > - > + nf->is_default = false; > + nf->enabled = true; > if (nfc->setup) { > nfc->setup(nf, &local_err); > if (local_err) { > @@ -190,6 +191,9 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) > g_free(info); > } > object_property_iter_free(iter); > + info = g_strdup_printf(",status=%s", nf->enabled ? "on" : "off"); > + g_strlcat(nf->info_str, info, sizeof(nf->info_str)); > + g_free(info); > } > > static void netfilter_finalize(Object *obj) > diff --git a/net/net.c b/net/net.c > index 87dd356..fd53cfc 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -581,6 +581,10 @@ static ssize_t filter_receive_iov(NetClientState *nc, > NetFilterState *nf = NULL; > > QTAILQ_FOREACH(nf, &nc->filters, next) { > + /* Don't go through filter if it is off */ > + if (qemu_need_skip_netfilter(nf)) { > + continue; > + } > ret = qemu_netfilter_receive(nf, direction, sender, flags, iov, > iovcnt, sent_cb); > if (ret) { > @@ -1028,6 +1032,14 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp) > } > return -1; > } > + > + if (is_netdev) { > + const Netdev *netdev = object; > + > + netdev_add_default_filter_buffer(netdev->id, > + NET_FILTER_DIRECTION_RX, > + errp); > + } > return 0; > } > >
ping? The other part of this series have been fully reviewed except this net related part. We hope COLO frame could be merged in qemu 2.6, so please help us. Any comments will be welcomed, thanks. zhanghailiang On 2016/1/11 9:26, Hailiang Zhang wrote: > Hi Jason, > > Could you please help reviewing the filter part of this series ? > > Thanks, > Hailiang > > On 2015/12/29 15:09, zhanghailiang wrote: >> We add each netdev (except vhost-net) a default filter-buffer, >> which will be used for COLO or Micro-checkpoint to buffer VM's packets. >> The name of default filter-buffer is 'nop'. >> For the default filter-buffer, it will not buffer any packets in default. >> So it has no side effect for the netdev. >> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> Cc: Jason Wang <jasowang@redhat.com> >> Cc: Yang Hongyang <hongyang.yang@easystack.cn> >> --- >> v12: >> - Skip vhost-net when add default filter >> - Don't go through filter layer if the filter is disabled. >> v11: >> - New patch >> --- >> include/net/filter.h | 10 +++++++ >> net/filter-buffer.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++ >> net/filter.c | 6 +++- >> net/net.c | 12 ++++++++ >> 4 files changed, 109 insertions(+), 1 deletion(-) >> >> diff --git a/include/net/filter.h b/include/net/filter.h >> index 2deda36..40aa38c 100644 >> --- a/include/net/filter.h >> +++ b/include/net/filter.h >> @@ -56,6 +56,8 @@ struct NetFilterState { >> NetClientState *netdev; >> NetFilterDirection direction; >> char info_str[256]; >> + bool is_default; >> + bool enabled; >> QTAILQ_ENTRY(NetFilterState) next; >> }; >> >> @@ -74,4 +76,12 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, >> int iovcnt, >> void *opaque); >> >> +static inline bool qemu_need_skip_netfilter(NetFilterState *nf) >> +{ >> + return nf->enabled ? false : true; >> +} >> + >> +void netdev_add_default_filter_buffer(const char *netdev_id, >> + NetFilterDirection direction, >> + Error **errp); >> #endif /* QEMU_NET_FILTER_H */ >> diff --git a/net/filter-buffer.c b/net/filter-buffer.c >> index 57be149..9cf3544 100644 >> --- a/net/filter-buffer.c >> +++ b/net/filter-buffer.c >> @@ -14,6 +14,13 @@ >> #include "qapi/qmp/qerror.h" >> #include "qapi-visit.h" >> #include "qom/object.h" >> +#include "net/net.h" >> +#include "qapi/qmp/qdict.h" >> +#include "qapi/qmp-output-visitor.h" >> +#include "qapi/qmp-input-visitor.h" >> +#include "monitor/monitor.h" >> +#include "qmp-commands.h" >> +#include "net/vhost_net.h" >> >> #define TYPE_FILTER_BUFFER "filter-buffer" >> >> @@ -102,6 +109,7 @@ static void filter_buffer_cleanup(NetFilterState *nf) >> static void filter_buffer_setup(NetFilterState *nf, Error **errp) >> { >> FilterBufferState *s = FILTER_BUFFER(nf); >> + char *path = object_get_canonical_path_component(OBJECT(nf)); >> >> /* >> * We may want to accept zero interval when VM FT solutions like MC >> @@ -114,6 +122,14 @@ static void filter_buffer_setup(NetFilterState *nf, Error **errp) >> } >> >> s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf); >> + nf->is_default = !strcmp(path, "nop"); >> + /* >> + * For the default buffer filter, it will be disabled by default, >> + * So it will not buffer any packets. >> + */ >> + if (nf->is_default) { >> + nf->enabled = false; >> + } >> if (s->interval) { >> timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL, >> filter_buffer_release_timer, nf); >> @@ -163,6 +179,72 @@ out: >> error_propagate(errp, local_err); >> } >> >> +/* >> +* This will be used by COLO or MC FT, for which they will need >> +* to buffer the packets of VM's net devices, Here we add a default >> +* buffer filter for each netdev. The name of default buffer filter is >> +* 'nop' >> +*/ >> +void netdev_add_default_filter_buffer(const char *netdev_id, >> + NetFilterDirection direction, >> + Error **errp) >> +{ >> + QmpOutputVisitor *qov; >> + QmpInputVisitor *qiv; >> + Visitor *ov, *iv; >> + QObject *obj = NULL; >> + QDict *qdict; >> + void *dummy = NULL; >> + const char *id = "nop"; >> + char *queue = g_strdup(NetFilterDirection_lookup[direction]); >> + NetClientState *nc = qemu_find_netdev(netdev_id); >> + Error *err = NULL; >> + >> + /* FIXME: Not support multiple queues */ >> + if (!nc || nc->queue_index > 1) { >> + g_free(queue); >> + return; >> + } >> + /* Not support vhost-net */ >> + if (get_vhost_net(nc)) { >> + g_free(queue); >> + return; >> + } >> + qov = qmp_output_visitor_new(); >> + ov = qmp_output_get_visitor(qov); >> + visit_start_struct(ov, &dummy, NULL, NULL, 0, &err); >> + if (err) { >> + goto out; >> + } >> + visit_type_str(ov, &nc->name, "netdev", &err); >> + if (err) { >> + goto out; >> + } >> + visit_type_str(ov, &queue, "queue", &err); >> + if (err) { >> + goto out; >> + } >> + visit_end_struct(ov, &err); >> + if (err) { >> + goto out; >> + } >> + obj = qmp_output_get_qobject(qov); >> + g_assert(obj != NULL); >> + qdict = qobject_to_qdict(obj); >> + qmp_output_visitor_cleanup(qov); >> + >> + qiv = qmp_input_visitor_new(obj); >> + iv = qmp_input_get_visitor(qiv); >> + object_add(TYPE_FILTER_BUFFER, id, qdict, iv, &err); >> + qmp_input_visitor_cleanup(qiv); >> + qobject_decref(obj); >> +out: >> + g_free(queue); >> + if (err) { >> + error_propagate(errp, err); >> + } >> +} >> + >> static void filter_buffer_init(Object *obj) >> { >> object_property_add(obj, "interval", "int", >> diff --git a/net/filter.c b/net/filter.c >> index 1365bad..0b1e408 100644 >> --- a/net/filter.c >> +++ b/net/filter.c >> @@ -163,7 +163,8 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) >> } >> >> nf->netdev = ncs[0]; >> - >> + nf->is_default = false; >> + nf->enabled = true; >> if (nfc->setup) { >> nfc->setup(nf, &local_err); >> if (local_err) { >> @@ -190,6 +191,9 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) >> g_free(info); >> } >> object_property_iter_free(iter); >> + info = g_strdup_printf(",status=%s", nf->enabled ? "on" : "off"); >> + g_strlcat(nf->info_str, info, sizeof(nf->info_str)); >> + g_free(info); >> } >> >> static void netfilter_finalize(Object *obj) >> diff --git a/net/net.c b/net/net.c >> index 87dd356..fd53cfc 100644 >> --- a/net/net.c >> +++ b/net/net.c >> @@ -581,6 +581,10 @@ static ssize_t filter_receive_iov(NetClientState *nc, >> NetFilterState *nf = NULL; >> >> QTAILQ_FOREACH(nf, &nc->filters, next) { >> + /* Don't go through filter if it is off */ >> + if (qemu_need_skip_netfilter(nf)) { >> + continue; >> + } >> ret = qemu_netfilter_receive(nf, direction, sender, flags, iov, >> iovcnt, sent_cb); >> if (ret) { >> @@ -1028,6 +1032,14 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp) >> } >> return -1; >> } >> + >> + if (is_netdev) { >> + const Netdev *netdev = object; >> + >> + netdev_add_default_filter_buffer(netdev->id, >> + NET_FILTER_DIRECTION_RX, >> + errp); >> + } >> return 0; >> } >> >> >
On 12/29/2015 03:09 PM, zhanghailiang wrote: > We add each netdev (except vhost-net) a default filter-buffer, > which will be used for COLO or Micro-checkpoint to buffer VM's packets. > The name of default filter-buffer is 'nop'. > For the default filter-buffer, it will not buffer any packets in default. > So it has no side effect for the netdev. > > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > Cc: Jason Wang <jasowang@redhat.com> > Cc: Yang Hongyang <hongyang.yang@easystack.cn> This patch did three things: 1) the ability to enable or disable a netfilter 2) the ability to add a default filter 3) default filter attaching for filter-buffer Better to split them into separate small patches. And several questions: For 1), I'm not sure this is real needed, we can in fact disable a filter by removing it. For 2), Instead of a specific code just for filter buffer, I think we need a generic method for an arbitrary filter to be used as default. And if we can achieve 2), 3) is not needed any more. > --- > v12: > - Skip vhost-net when add default filter > - Don't go through filter layer if the filter is disabled. > v11: > - New patch > --- > include/net/filter.h | 10 +++++++ > net/filter-buffer.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > net/filter.c | 6 +++- > net/net.c | 12 ++++++++ > 4 files changed, 109 insertions(+), 1 deletion(-) > > diff --git a/include/net/filter.h b/include/net/filter.h > index 2deda36..40aa38c 100644 > --- a/include/net/filter.h > +++ b/include/net/filter.h > @@ -56,6 +56,8 @@ struct NetFilterState { > NetClientState *netdev; > NetFilterDirection direction; > char info_str[256]; > + bool is_default; > + bool enabled; > QTAILQ_ENTRY(NetFilterState) next; > }; > > @@ -74,4 +76,12 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, > int iovcnt, > void *opaque); > > +static inline bool qemu_need_skip_netfilter(NetFilterState *nf) > +{ > + return nf->enabled ? false : true; > +} > + > +void netdev_add_default_filter_buffer(const char *netdev_id, > + NetFilterDirection direction, > + Error **errp); > #endif /* QEMU_NET_FILTER_H */ > diff --git a/net/filter-buffer.c b/net/filter-buffer.c > index 57be149..9cf3544 100644 > --- a/net/filter-buffer.c > +++ b/net/filter-buffer.c > @@ -14,6 +14,13 @@ > #include "qapi/qmp/qerror.h" > #include "qapi-visit.h" > #include "qom/object.h" > +#include "net/net.h" > +#include "qapi/qmp/qdict.h" > +#include "qapi/qmp-output-visitor.h" > +#include "qapi/qmp-input-visitor.h" > +#include "monitor/monitor.h" > +#include "qmp-commands.h" > +#include "net/vhost_net.h" > > #define TYPE_FILTER_BUFFER "filter-buffer" > > @@ -102,6 +109,7 @@ static void filter_buffer_cleanup(NetFilterState *nf) > static void filter_buffer_setup(NetFilterState *nf, Error **errp) > { > FilterBufferState *s = FILTER_BUFFER(nf); > + char *path = object_get_canonical_path_component(OBJECT(nf)); > > /* > * We may want to accept zero interval when VM FT solutions like MC > @@ -114,6 +122,14 @@ static void filter_buffer_setup(NetFilterState *nf, Error **errp) > } > > s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf); > + nf->is_default = !strcmp(path, "nop"); > + /* > + * For the default buffer filter, it will be disabled by default, > + * So it will not buffer any packets. > + */ > + if (nf->is_default) { > + nf->enabled = false; > + } > if (s->interval) { > timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL, > filter_buffer_release_timer, nf); > @@ -163,6 +179,72 @@ out: > error_propagate(errp, local_err); > } > > +/* > +* This will be used by COLO or MC FT, for which they will need > +* to buffer the packets of VM's net devices, Here we add a default > +* buffer filter for each netdev. The name of default buffer filter is > +* 'nop' > +*/ > +void netdev_add_default_filter_buffer(const char *netdev_id, > + NetFilterDirection direction, > + Error **errp) > +{ Need a more generic way to add an arbitrary filter as default. E.g during netdev init, query if there's a default and do the initialization there. > + QmpOutputVisitor *qov; > + QmpInputVisitor *qiv; > + Visitor *ov, *iv; > + QObject *obj = NULL; > + QDict *qdict; > + void *dummy = NULL; > + const char *id = "nop"; > + char *queue = g_strdup(NetFilterDirection_lookup[direction]); > + NetClientState *nc = qemu_find_netdev(netdev_id); > + Error *err = NULL; > + > + /* FIXME: Not support multiple queues */ > + if (!nc || nc->queue_index > 1) { > + g_free(queue); > + return; > + } > + /* Not support vhost-net */ > + if (get_vhost_net(nc)) { > + g_free(queue); > + return; > + } > + qov = qmp_output_visitor_new(); > + ov = qmp_output_get_visitor(qov); > + visit_start_struct(ov, &dummy, NULL, NULL, 0, &err); > + if (err) { > + goto out; > + } > + visit_type_str(ov, &nc->name, "netdev", &err); > + if (err) { > + goto out; > + } > + visit_type_str(ov, &queue, "queue", &err); > + if (err) { > + goto out; > + } > + visit_end_struct(ov, &err); > + if (err) { > + goto out; > + } > + obj = qmp_output_get_qobject(qov); > + g_assert(obj != NULL); > + qdict = qobject_to_qdict(obj); > + qmp_output_visitor_cleanup(qov); > + > + qiv = qmp_input_visitor_new(obj); > + iv = qmp_input_get_visitor(qiv); > + object_add(TYPE_FILTER_BUFFER, id, qdict, iv, &err); > + qmp_input_visitor_cleanup(qiv); > + qobject_decref(obj); > +out: > + g_free(queue); > + if (err) { > + error_propagate(errp, err); > + } > +} > + > static void filter_buffer_init(Object *obj) > { > object_property_add(obj, "interval", "int", > diff --git a/net/filter.c b/net/filter.c > index 1365bad..0b1e408 100644 > --- a/net/filter.c > +++ b/net/filter.c > @@ -163,7 +163,8 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) > } > > nf->netdev = ncs[0]; > - > + nf->is_default = false; > + nf->enabled = true; If we really need something like enabled, need more code for user to control this bit. > if (nfc->setup) { > nfc->setup(nf, &local_err); > if (local_err) { > @@ -190,6 +191,9 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) > g_free(info); > } > object_property_iter_free(iter); > + info = g_strdup_printf(",status=%s", nf->enabled ? "on" : "off"); > + g_strlcat(nf->info_str, info, sizeof(nf->info_str)); > + g_free(info); > } > > static void netfilter_finalize(Object *obj) > diff --git a/net/net.c b/net/net.c > index 87dd356..fd53cfc 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -581,6 +581,10 @@ static ssize_t filter_receive_iov(NetClientState *nc, > NetFilterState *nf = NULL; > > QTAILQ_FOREACH(nf, &nc->filters, next) { > + /* Don't go through filter if it is off */ > + if (qemu_need_skip_netfilter(nf)) { > + continue; > + } Better move the above to qemu_netfilter_receive() > ret = qemu_netfilter_receive(nf, direction, sender, flags, iov, > iovcnt, sent_cb); > if (ret) { > @@ -1028,6 +1032,14 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp) > } > return -1; > } > + > + if (is_netdev) { > + const Netdev *netdev = object; > + > + netdev_add_default_filter_buffer(netdev->id, > + NET_FILTER_DIRECTION_RX, > + errp); Several issues here: - Generic code should know nothing about a specific kind of netfilter. So the default filter should be pointed out by user instead of hard-coding it like this. - Need to deal with the netdev hot add. It should still has the default filter. > + } > return 0; > } >
Hi Jason, Thanks for your review. On 2016/1/19 11:19, Jason Wang wrote: > > > On 12/29/2015 03:09 PM, zhanghailiang wrote: >> We add each netdev (except vhost-net) a default filter-buffer, >> which will be used for COLO or Micro-checkpoint to buffer VM's packets. >> The name of default filter-buffer is 'nop'. >> For the default filter-buffer, it will not buffer any packets in default. >> So it has no side effect for the netdev. >> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> Cc: Jason Wang <jasowang@redhat.com> >> Cc: Yang Hongyang <hongyang.yang@easystack.cn> > > This patch did three things: > > 1) the ability to enable or disable a netfilter > 2) the ability to add a default filter > 3) default filter attaching for filter-buffer > > Better to split them into separate small patches. > > And several questions: > > For 1), I'm not sure this is real needed, we can in fact disable a > filter by removing it. If we do like this, do we also need to _enable_ the buffer filter by add it dynamically instead of attaching the default filter ? Just like what we do in V10 ? (In that series, you think have a default filter may be better. The main reason for that is to support hot-add nic. Since we didn't support hot-add nic during COLO, it will be OK to add default filter dynamically) > For 2), Instead of a specific code just for filter buffer, I think we > need a generic method for an arbitrary filter to be used as default. Good idea. > And if we can achieve 2), 3) is not needed any more. > >> --- >> v12: >> - Skip vhost-net when add default filter >> - Don't go through filter layer if the filter is disabled. >> v11: >> - New patch >> --- >> include/net/filter.h | 10 +++++++ >> net/filter-buffer.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++ >> net/filter.c | 6 +++- >> net/net.c | 12 ++++++++ >> 4 files changed, 109 insertions(+), 1 deletion(-) >> >> diff --git a/include/net/filter.h b/include/net/filter.h >> index 2deda36..40aa38c 100644 >> --- a/include/net/filter.h >> +++ b/include/net/filter.h >> @@ -56,6 +56,8 @@ struct NetFilterState { >> NetClientState *netdev; >> NetFilterDirection direction; >> char info_str[256]; >> + bool is_default; >> + bool enabled; >> QTAILQ_ENTRY(NetFilterState) next; >> }; >> >> @@ -74,4 +76,12 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, >> int iovcnt, >> void *opaque); >> >> +static inline bool qemu_need_skip_netfilter(NetFilterState *nf) >> +{ >> + return nf->enabled ? false : true; >> +} >> + >> +void netdev_add_default_filter_buffer(const char *netdev_id, >> + NetFilterDirection direction, >> + Error **errp); >> #endif /* QEMU_NET_FILTER_H */ >> diff --git a/net/filter-buffer.c b/net/filter-buffer.c >> index 57be149..9cf3544 100644 >> --- a/net/filter-buffer.c >> +++ b/net/filter-buffer.c >> @@ -14,6 +14,13 @@ >> #include "qapi/qmp/qerror.h" >> #include "qapi-visit.h" >> #include "qom/object.h" >> +#include "net/net.h" >> +#include "qapi/qmp/qdict.h" >> +#include "qapi/qmp-output-visitor.h" >> +#include "qapi/qmp-input-visitor.h" >> +#include "monitor/monitor.h" >> +#include "qmp-commands.h" >> +#include "net/vhost_net.h" >> >> #define TYPE_FILTER_BUFFER "filter-buffer" >> >> @@ -102,6 +109,7 @@ static void filter_buffer_cleanup(NetFilterState *nf) >> static void filter_buffer_setup(NetFilterState *nf, Error **errp) >> { >> FilterBufferState *s = FILTER_BUFFER(nf); >> + char *path = object_get_canonical_path_component(OBJECT(nf)); >> >> /* >> * We may want to accept zero interval when VM FT solutions like MC >> @@ -114,6 +122,14 @@ static void filter_buffer_setup(NetFilterState *nf, Error **errp) >> } >> >> s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf); >> + nf->is_default = !strcmp(path, "nop"); >> + /* >> + * For the default buffer filter, it will be disabled by default, >> + * So it will not buffer any packets. >> + */ >> + if (nf->is_default) { >> + nf->enabled = false; >> + } >> if (s->interval) { >> timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL, >> filter_buffer_release_timer, nf); >> @@ -163,6 +179,72 @@ out: >> error_propagate(errp, local_err); >> } >> >> +/* >> +* This will be used by COLO or MC FT, for which they will need >> +* to buffer the packets of VM's net devices, Here we add a default >> +* buffer filter for each netdev. The name of default buffer filter is >> +* 'nop' >> +*/ >> +void netdev_add_default_filter_buffer(const char *netdev_id, >> + NetFilterDirection direction, >> + Error **errp) >> +{ > > Need a more generic way to add an arbitrary filter as default. E.g > during netdev init, query if there's a default and do the initialization > there. > We call it in net_client_init1(), i don't find a better place to call it, what's your suggestion ? >> + QmpOutputVisitor *qov; >> + QmpInputVisitor *qiv; >> + Visitor *ov, *iv; >> + QObject *obj = NULL; >> + QDict *qdict; >> + void *dummy = NULL; >> + const char *id = "nop"; >> + char *queue = g_strdup(NetFilterDirection_lookup[direction]); >> + NetClientState *nc = qemu_find_netdev(netdev_id); >> + Error *err = NULL; >> + >> + /* FIXME: Not support multiple queues */ >> + if (!nc || nc->queue_index > 1) { >> + g_free(queue); >> + return; >> + } >> + /* Not support vhost-net */ >> + if (get_vhost_net(nc)) { >> + g_free(queue); >> + return; >> + } >> + qov = qmp_output_visitor_new(); >> + ov = qmp_output_get_visitor(qov); >> + visit_start_struct(ov, &dummy, NULL, NULL, 0, &err); >> + if (err) { >> + goto out; >> + } >> + visit_type_str(ov, &nc->name, "netdev", &err); >> + if (err) { >> + goto out; >> + } >> + visit_type_str(ov, &queue, "queue", &err); >> + if (err) { >> + goto out; >> + } >> + visit_end_struct(ov, &err); >> + if (err) { >> + goto out; >> + } >> + obj = qmp_output_get_qobject(qov); >> + g_assert(obj != NULL); >> + qdict = qobject_to_qdict(obj); >> + qmp_output_visitor_cleanup(qov); >> + >> + qiv = qmp_input_visitor_new(obj); >> + iv = qmp_input_get_visitor(qiv); >> + object_add(TYPE_FILTER_BUFFER, id, qdict, iv, &err); >> + qmp_input_visitor_cleanup(qiv); >> + qobject_decref(obj); >> +out: >> + g_free(queue); >> + if (err) { >> + error_propagate(errp, err); >> + } >> +} >> + >> static void filter_buffer_init(Object *obj) >> { >> object_property_add(obj, "interval", "int", >> diff --git a/net/filter.c b/net/filter.c >> index 1365bad..0b1e408 100644 >> --- a/net/filter.c >> +++ b/net/filter.c >> @@ -163,7 +163,8 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) >> } >> >> nf->netdev = ncs[0]; >> - >> + nf->is_default = false; >> + nf->enabled = true; > > If we really need something like enabled, need more code for user to > control this bit. > OK, i will fix it. >> if (nfc->setup) { >> nfc->setup(nf, &local_err); >> if (local_err) { >> @@ -190,6 +191,9 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) >> g_free(info); >> } >> object_property_iter_free(iter); >> + info = g_strdup_printf(",status=%s", nf->enabled ? "on" : "off"); >> + g_strlcat(nf->info_str, info, sizeof(nf->info_str)); >> + g_free(info); >> } >> >> static void netfilter_finalize(Object *obj) >> diff --git a/net/net.c b/net/net.c >> index 87dd356..fd53cfc 100644 >> --- a/net/net.c >> +++ b/net/net.c >> @@ -581,6 +581,10 @@ static ssize_t filter_receive_iov(NetClientState *nc, >> NetFilterState *nf = NULL; >> >> QTAILQ_FOREACH(nf, &nc->filters, next) { >> + /* Don't go through filter if it is off */ >> + if (qemu_need_skip_netfilter(nf)) { >> + continue; >> + } > > Better move the above to qemu_netfilter_receive() > OK. >> ret = qemu_netfilter_receive(nf, direction, sender, flags, iov, >> iovcnt, sent_cb); >> if (ret) { >> @@ -1028,6 +1032,14 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp) >> } >> return -1; >> } >> + >> + if (is_netdev) { >> + const Netdev *netdev = object; >> + >> + netdev_add_default_filter_buffer(netdev->id, >> + NET_FILTER_DIRECTION_RX, >> + errp); > > Several issues here: > > - Generic code should know nothing about a specific kind of netfilter. > So the default filter should be pointed out by user instead of > hard-coding it like this. > - Need to deal with the netdev hot add. It should still has the default > filter. > OK, then, here just answer my above question, we still need to add the default filter, so we still need the ability to enable or disable a netfilter. :) Thanks, Hailiang >> + } >> return 0; >> } >> > > > . >
On 01/19/2016 04:39 PM, Hailiang Zhang wrote: > Hi Jason, > > Thanks for your review. > > On 2016/1/19 11:19, Jason Wang wrote: >> >> >> On 12/29/2015 03:09 PM, zhanghailiang wrote: >>> We add each netdev (except vhost-net) a default filter-buffer, >>> which will be used for COLO or Micro-checkpoint to buffer VM's packets. >>> The name of default filter-buffer is 'nop'. >>> For the default filter-buffer, it will not buffer any packets in >>> default. >>> So it has no side effect for the netdev. >>> >>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >>> Cc: Jason Wang <jasowang@redhat.com> >>> Cc: Yang Hongyang <hongyang.yang@easystack.cn> >> >> This patch did three things: >> >> 1) the ability to enable or disable a netfilter >> 2) the ability to add a default filter >> 3) default filter attaching for filter-buffer >> >> Better to split them into separate small patches. >> >> And several questions: >> >> For 1), I'm not sure this is real needed, we can in fact disable a >> filter by removing it. > > If we do like this, do we also need to _enable_ the buffer filter by > add it dynamically instead of attaching the default filter ? > Just like what we do in V10 ? > (In that series, you think have a default filter may be better. > The main reason for that is to support > hot-add nic. Since we didn't support hot-add nic during COLO, > it will be OK to add default filter dynamically) Aha, right. So rethinking of this, enabling/disabling a filter looks ok to me. > >> For 2), Instead of a specific code just for filter buffer, I think we >> need a generic method for an arbitrary filter to be used as default. > > Good idea. > >> And if we can achieve 2), 3) is not needed any more. >> >>> --- >>> v12: >>> - Skip vhost-net when add default filter >>> - Don't go through filter layer if the filter is disabled. >>> v11: >>> - New patch >>> --- >>> include/net/filter.h | 10 +++++++ >>> net/filter-buffer.c | 82 >>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> net/filter.c | 6 +++- >>> net/net.c | 12 ++++++++ >>> 4 files changed, 109 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/net/filter.h b/include/net/filter.h >>> index 2deda36..40aa38c 100644 >>> --- a/include/net/filter.h >>> +++ b/include/net/filter.h >>> @@ -56,6 +56,8 @@ struct NetFilterState { >>> NetClientState *netdev; >>> NetFilterDirection direction; >>> char info_str[256]; >>> + bool is_default; >>> + bool enabled; >>> QTAILQ_ENTRY(NetFilterState) next; >>> }; >>> >>> @@ -74,4 +76,12 @@ ssize_t >>> qemu_netfilter_pass_to_next(NetClientState *sender, >>> int iovcnt, >>> void *opaque); >>> >>> +static inline bool qemu_need_skip_netfilter(NetFilterState *nf) >>> +{ >>> + return nf->enabled ? false : true; >>> +} >>> + >>> +void netdev_add_default_filter_buffer(const char *netdev_id, >>> + NetFilterDirection direction, >>> + Error **errp); >>> #endif /* QEMU_NET_FILTER_H */ >>> diff --git a/net/filter-buffer.c b/net/filter-buffer.c >>> index 57be149..9cf3544 100644 >>> --- a/net/filter-buffer.c >>> +++ b/net/filter-buffer.c >>> @@ -14,6 +14,13 @@ >>> #include "qapi/qmp/qerror.h" >>> #include "qapi-visit.h" >>> #include "qom/object.h" >>> +#include "net/net.h" >>> +#include "qapi/qmp/qdict.h" >>> +#include "qapi/qmp-output-visitor.h" >>> +#include "qapi/qmp-input-visitor.h" >>> +#include "monitor/monitor.h" >>> +#include "qmp-commands.h" >>> +#include "net/vhost_net.h" >>> >>> #define TYPE_FILTER_BUFFER "filter-buffer" >>> >>> @@ -102,6 +109,7 @@ static void filter_buffer_cleanup(NetFilterState >>> *nf) >>> static void filter_buffer_setup(NetFilterState *nf, Error **errp) >>> { >>> FilterBufferState *s = FILTER_BUFFER(nf); >>> + char *path = object_get_canonical_path_component(OBJECT(nf)); >>> >>> /* >>> * We may want to accept zero interval when VM FT solutions >>> like MC >>> @@ -114,6 +122,14 @@ static void filter_buffer_setup(NetFilterState >>> *nf, Error **errp) >>> } >>> >>> s->incoming_queue = >>> qemu_new_net_queue(qemu_netfilter_pass_to_next, nf); >>> + nf->is_default = !strcmp(path, "nop"); >>> + /* >>> + * For the default buffer filter, it will be disabled by default, >>> + * So it will not buffer any packets. >>> + */ >>> + if (nf->is_default) { >>> + nf->enabled = false; >>> + } >>> if (s->interval) { >>> timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL, >>> filter_buffer_release_timer, nf); >>> @@ -163,6 +179,72 @@ out: >>> error_propagate(errp, local_err); >>> } >>> >>> +/* >>> +* This will be used by COLO or MC FT, for which they will need >>> +* to buffer the packets of VM's net devices, Here we add a default >>> +* buffer filter for each netdev. The name of default buffer filter is >>> +* 'nop' >>> +*/ >>> +void netdev_add_default_filter_buffer(const char *netdev_id, >>> + NetFilterDirection direction, >>> + Error **errp) >>> +{ >> >> Need a more generic way to add an arbitrary filter as default. E.g >> during netdev init, query if there's a default and do the initialization >> there. >> > > We call it in net_client_init1(), i don't find a better place to call it, > what's your suggestion ? net_client_init1() is ok, just need a generic way. E.g a string which stores the default filter and its parameters which could be queried by default filter initialization function. > >>> + QmpOutputVisitor *qov; >>> + QmpInputVisitor *qiv; >>> + Visitor *ov, *iv; >>> + QObject *obj = NULL; >>> + QDict *qdict; >>> + void *dummy = NULL; >>> + const char *id = "nop"; >>> + char *queue = g_strdup(NetFilterDirection_lookup[direction]); >>> + NetClientState *nc = qemu_find_netdev(netdev_id); >>> + Error *err = NULL; >>> + >>> + /* FIXME: Not support multiple queues */ >>> + if (!nc || nc->queue_index > 1) { >>> + g_free(queue); >>> + return; >>> + } >>> + /* Not support vhost-net */ >>> + if (get_vhost_net(nc)) { >>> + g_free(queue); >>> + return; >>> + } >>> + qov = qmp_output_visitor_new(); >>> + ov = qmp_output_get_visitor(qov); >>> + visit_start_struct(ov, &dummy, NULL, NULL, 0, &err); >>> + if (err) { >>> + goto out; >>> + } >>> + visit_type_str(ov, &nc->name, "netdev", &err); >>> + if (err) { >>> + goto out; >>> + } >>> + visit_type_str(ov, &queue, "queue", &err); >>> + if (err) { >>> + goto out; >>> + } >>> + visit_end_struct(ov, &err); >>> + if (err) { >>> + goto out; >>> + } >>> + obj = qmp_output_get_qobject(qov); >>> + g_assert(obj != NULL); >>> + qdict = qobject_to_qdict(obj); >>> + qmp_output_visitor_cleanup(qov); >>> + >>> + qiv = qmp_input_visitor_new(obj); >>> + iv = qmp_input_get_visitor(qiv); >>> + object_add(TYPE_FILTER_BUFFER, id, qdict, iv, &err); >>> + qmp_input_visitor_cleanup(qiv); >>> + qobject_decref(obj); >>> +out: >>> + g_free(queue); >>> + if (err) { >>> + error_propagate(errp, err); >>> + } >>> +} >>> + >>> static void filter_buffer_init(Object *obj) >>> { >>> object_property_add(obj, "interval", "int", >>> diff --git a/net/filter.c b/net/filter.c >>> index 1365bad..0b1e408 100644 >>> --- a/net/filter.c >>> +++ b/net/filter.c >>> @@ -163,7 +163,8 @@ static void netfilter_complete(UserCreatable >>> *uc, Error **errp) >>> } >>> >>> nf->netdev = ncs[0]; >>> - >>> + nf->is_default = false; >>> + nf->enabled = true; >> >> If we really need something like enabled, need more code for user to >> control this bit. >> > > OK, i will fix it. Or if you want to minimize the change set. You can make something like enabled locally to filter buffer, and let it to be used by colo only. > >>> if (nfc->setup) { >>> nfc->setup(nf, &local_err); >>> if (local_err) { >>> @@ -190,6 +191,9 @@ static void netfilter_complete(UserCreatable >>> *uc, Error **errp) >>> g_free(info); >>> } >>> object_property_iter_free(iter); >>> + info = g_strdup_printf(",status=%s", nf->enabled ? "on" : "off"); >>> + g_strlcat(nf->info_str, info, sizeof(nf->info_str)); >>> + g_free(info); >>> } >>> >>> static void netfilter_finalize(Object *obj) >>> diff --git a/net/net.c b/net/net.c >>> index 87dd356..fd53cfc 100644 >>> --- a/net/net.c >>> +++ b/net/net.c >>> @@ -581,6 +581,10 @@ static ssize_t >>> filter_receive_iov(NetClientState *nc, >>> NetFilterState *nf = NULL; >>> >>> QTAILQ_FOREACH(nf, &nc->filters, next) { >>> + /* Don't go through filter if it is off */ >>> + if (qemu_need_skip_netfilter(nf)) { >>> + continue; >>> + } >> >> Better move the above to qemu_netfilter_receive() >> > > OK. > >>> ret = qemu_netfilter_receive(nf, direction, sender, flags, >>> iov, >>> iovcnt, sent_cb); >>> if (ret) { >>> @@ -1028,6 +1032,14 @@ static int net_client_init1(const void >>> *object, int is_netdev, Error **errp) >>> } >>> return -1; >>> } >>> + >>> + if (is_netdev) { >>> + const Netdev *netdev = object; >>> + >>> + netdev_add_default_filter_buffer(netdev->id, >>> + NET_FILTER_DIRECTION_RX, >>> + errp); >> >> Several issues here: >> >> - Generic code should know nothing about a specific kind of netfilter. >> So the default filter should be pointed out by user instead of >> hard-coding it like this. >> - Need to deal with the netdev hot add. It should still has the default >> filter. >> > > OK, then, here just answer my above question, we still need to add the > default filter, > so we still need the ability to enable or disable a netfilter. :) Right. So the issue is just to have a way to let an arbitrary filter to be default :). Thanks > > Thanks, > Hailiang > >>> + } >>> return 0; >>> } >>> >> >> >> . >> > > >
On 2016/1/20 10:39, Jason Wang wrote: > > > On 01/19/2016 04:39 PM, Hailiang Zhang wrote: >> Hi Jason, >> >> Thanks for your review. >> >> On 2016/1/19 11:19, Jason Wang wrote: >>> >>> >>> On 12/29/2015 03:09 PM, zhanghailiang wrote: >>>> We add each netdev (except vhost-net) a default filter-buffer, >>>> which will be used for COLO or Micro-checkpoint to buffer VM's packets. >>>> The name of default filter-buffer is 'nop'. >>>> For the default filter-buffer, it will not buffer any packets in >>>> default. >>>> So it has no side effect for the netdev. >>>> >>>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >>>> Cc: Jason Wang <jasowang@redhat.com> >>>> Cc: Yang Hongyang <hongyang.yang@easystack.cn> >>> >>> This patch did three things: >>> >>> 1) the ability to enable or disable a netfilter >>> 2) the ability to add a default filter >>> 3) default filter attaching for filter-buffer >>> >>> Better to split them into separate small patches. >>> >>> And several questions: >>> >>> For 1), I'm not sure this is real needed, we can in fact disable a >>> filter by removing it. >> >> If we do like this, do we also need to _enable_ the buffer filter by >> add it dynamically instead of attaching the default filter ? >> Just like what we do in V10 ? >> (In that series, you think have a default filter may be better. >> The main reason for that is to support >> hot-add nic. Since we didn't support hot-add nic during COLO, >> it will be OK to add default filter dynamically) > > Aha, right. So rethinking of this, enabling/disabling a filter looks ok > to me. > >> >>> For 2), Instead of a specific code just for filter buffer, I think we >>> need a generic method for an arbitrary filter to be used as default. >> >> Good idea. >> >>> And if we can achieve 2), 3) is not needed any more. >>> >>>> --- >>>> v12: >>>> - Skip vhost-net when add default filter >>>> - Don't go through filter layer if the filter is disabled. >>>> v11: >>>> - New patch >>>> --- >>>> include/net/filter.h | 10 +++++++ >>>> net/filter-buffer.c | 82 >>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> net/filter.c | 6 +++- >>>> net/net.c | 12 ++++++++ >>>> 4 files changed, 109 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/include/net/filter.h b/include/net/filter.h >>>> index 2deda36..40aa38c 100644 >>>> --- a/include/net/filter.h >>>> +++ b/include/net/filter.h >>>> @@ -56,6 +56,8 @@ struct NetFilterState { >>>> NetClientState *netdev; >>>> NetFilterDirection direction; >>>> char info_str[256]; >>>> + bool is_default; >>>> + bool enabled; >>>> QTAILQ_ENTRY(NetFilterState) next; >>>> }; >>>> >>>> @@ -74,4 +76,12 @@ ssize_t >>>> qemu_netfilter_pass_to_next(NetClientState *sender, >>>> int iovcnt, >>>> void *opaque); >>>> >>>> +static inline bool qemu_need_skip_netfilter(NetFilterState *nf) >>>> +{ >>>> + return nf->enabled ? false : true; >>>> +} >>>> + >>>> +void netdev_add_default_filter_buffer(const char *netdev_id, >>>> + NetFilterDirection direction, >>>> + Error **errp); >>>> #endif /* QEMU_NET_FILTER_H */ >>>> diff --git a/net/filter-buffer.c b/net/filter-buffer.c >>>> index 57be149..9cf3544 100644 >>>> --- a/net/filter-buffer.c >>>> +++ b/net/filter-buffer.c >>>> @@ -14,6 +14,13 @@ >>>> #include "qapi/qmp/qerror.h" >>>> #include "qapi-visit.h" >>>> #include "qom/object.h" >>>> +#include "net/net.h" >>>> +#include "qapi/qmp/qdict.h" >>>> +#include "qapi/qmp-output-visitor.h" >>>> +#include "qapi/qmp-input-visitor.h" >>>> +#include "monitor/monitor.h" >>>> +#include "qmp-commands.h" >>>> +#include "net/vhost_net.h" >>>> >>>> #define TYPE_FILTER_BUFFER "filter-buffer" >>>> >>>> @@ -102,6 +109,7 @@ static void filter_buffer_cleanup(NetFilterState >>>> *nf) >>>> static void filter_buffer_setup(NetFilterState *nf, Error **errp) >>>> { >>>> FilterBufferState *s = FILTER_BUFFER(nf); >>>> + char *path = object_get_canonical_path_component(OBJECT(nf)); >>>> >>>> /* >>>> * We may want to accept zero interval when VM FT solutions >>>> like MC >>>> @@ -114,6 +122,14 @@ static void filter_buffer_setup(NetFilterState >>>> *nf, Error **errp) >>>> } >>>> >>>> s->incoming_queue = >>>> qemu_new_net_queue(qemu_netfilter_pass_to_next, nf); >>>> + nf->is_default = !strcmp(path, "nop"); >>>> + /* >>>> + * For the default buffer filter, it will be disabled by default, >>>> + * So it will not buffer any packets. >>>> + */ >>>> + if (nf->is_default) { >>>> + nf->enabled = false; >>>> + } >>>> if (s->interval) { >>>> timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL, >>>> filter_buffer_release_timer, nf); >>>> @@ -163,6 +179,72 @@ out: >>>> error_propagate(errp, local_err); >>>> } >>>> >>>> +/* >>>> +* This will be used by COLO or MC FT, for which they will need >>>> +* to buffer the packets of VM's net devices, Here we add a default >>>> +* buffer filter for each netdev. The name of default buffer filter is >>>> +* 'nop' >>>> +*/ >>>> +void netdev_add_default_filter_buffer(const char *netdev_id, >>>> + NetFilterDirection direction, >>>> + Error **errp) >>>> +{ >>> >>> Need a more generic way to add an arbitrary filter as default. E.g >>> during netdev init, query if there's a default and do the initialization >>> there. >>> >> >> We call it in net_client_init1(), i don't find a better place to call it, >> what's your suggestion ? > > net_client_init1() is ok, just need a generic way. E.g a string which > stores the default filter and its parameters which could be queried by > default filter initialization function. > Hmm, i'm still confused about how to realize this, do you mean change this helper to a more generic function ? Just like : void netdev_add_filter(const char *netdev_id, const char *filter_type, char *id, Error **errp) Could you please give me more detail about how to realize it ? :) >> >>>> + QmpOutputVisitor *qov; >>>> + QmpInputVisitor *qiv; >>>> + Visitor *ov, *iv; >>>> + QObject *obj = NULL; >>>> + QDict *qdict; >>>> + void *dummy = NULL; >>>> + const char *id = "nop"; >>>> + char *queue = g_strdup(NetFilterDirection_lookup[direction]); >>>> + NetClientState *nc = qemu_find_netdev(netdev_id); >>>> + Error *err = NULL; >>>> + >>>> + /* FIXME: Not support multiple queues */ >>>> + if (!nc || nc->queue_index > 1) { >>>> + g_free(queue); >>>> + return; >>>> + } >>>> + /* Not support vhost-net */ >>>> + if (get_vhost_net(nc)) { >>>> + g_free(queue); >>>> + return; >>>> + } >>>> + qov = qmp_output_visitor_new(); >>>> + ov = qmp_output_get_visitor(qov); >>>> + visit_start_struct(ov, &dummy, NULL, NULL, 0, &err); >>>> + if (err) { >>>> + goto out; >>>> + } >>>> + visit_type_str(ov, &nc->name, "netdev", &err); >>>> + if (err) { >>>> + goto out; >>>> + } >>>> + visit_type_str(ov, &queue, "queue", &err); >>>> + if (err) { >>>> + goto out; >>>> + } >>>> + visit_end_struct(ov, &err); >>>> + if (err) { >>>> + goto out; >>>> + } >>>> + obj = qmp_output_get_qobject(qov); >>>> + g_assert(obj != NULL); >>>> + qdict = qobject_to_qdict(obj); >>>> + qmp_output_visitor_cleanup(qov); >>>> + >>>> + qiv = qmp_input_visitor_new(obj); >>>> + iv = qmp_input_get_visitor(qiv); >>>> + object_add(TYPE_FILTER_BUFFER, id, qdict, iv, &err); >>>> + qmp_input_visitor_cleanup(qiv); >>>> + qobject_decref(obj); >>>> +out: >>>> + g_free(queue); >>>> + if (err) { >>>> + error_propagate(errp, err); >>>> + } >>>> +} >>>> + >>>> static void filter_buffer_init(Object *obj) >>>> { >>>> object_property_add(obj, "interval", "int", >>>> diff --git a/net/filter.c b/net/filter.c >>>> index 1365bad..0b1e408 100644 >>>> --- a/net/filter.c >>>> +++ b/net/filter.c >>>> @@ -163,7 +163,8 @@ static void netfilter_complete(UserCreatable >>>> *uc, Error **errp) >>>> } >>>> >>>> nf->netdev = ncs[0]; >>>> - >>>> + nf->is_default = false; >>>> + nf->enabled = true; >>> >>> If we really need something like enabled, need more code for user to >>> control this bit. >>> >> >> OK, i will fix it. > > Or if you want to minimize the change set. You can make something like > enabled locally to filter buffer, and let it to be used by colo only. > I have convert it to use object property to set/get the value, is that OK ? Thanks, Hailiang >> >>>> if (nfc->setup) { >>>> nfc->setup(nf, &local_err); >>>> if (local_err) { >>>> @@ -190,6 +191,9 @@ static void netfilter_complete(UserCreatable >>>> *uc, Error **errp) >>>> g_free(info); >>>> } >>>> object_property_iter_free(iter); >>>> + info = g_strdup_printf(",status=%s", nf->enabled ? "on" : "off"); >>>> + g_strlcat(nf->info_str, info, sizeof(nf->info_str)); >>>> + g_free(info); >>>> } >>>> >>>> static void netfilter_finalize(Object *obj) >>>> diff --git a/net/net.c b/net/net.c >>>> index 87dd356..fd53cfc 100644 >>>> --- a/net/net.c >>>> +++ b/net/net.c >>>> @@ -581,6 +581,10 @@ static ssize_t >>>> filter_receive_iov(NetClientState *nc, >>>> NetFilterState *nf = NULL; >>>> >>>> QTAILQ_FOREACH(nf, &nc->filters, next) { >>>> + /* Don't go through filter if it is off */ >>>> + if (qemu_need_skip_netfilter(nf)) { >>>> + continue; >>>> + } >>> >>> Better move the above to qemu_netfilter_receive() >>> >> >> OK. >> >>>> ret = qemu_netfilter_receive(nf, direction, sender, flags, >>>> iov, >>>> iovcnt, sent_cb); >>>> if (ret) { >>>> @@ -1028,6 +1032,14 @@ static int net_client_init1(const void >>>> *object, int is_netdev, Error **errp) >>>> } >>>> return -1; >>>> } >>>> + >>>> + if (is_netdev) { >>>> + const Netdev *netdev = object; >>>> + >>>> + netdev_add_default_filter_buffer(netdev->id, >>>> + NET_FILTER_DIRECTION_RX, >>>> + errp); >>> >>> Several issues here: >>> >>> - Generic code should know nothing about a specific kind of netfilter. >>> So the default filter should be pointed out by user instead of >>> hard-coding it like this. >>> - Need to deal with the netdev hot add. It should still has the default >>> filter. >>> >> >> OK, then, here just answer my above question, we still need to add the >> default filter, >> so we still need the ability to enable or disable a netfilter. :) > > Right. So the issue is just to have a way to let an arbitrary filter to > be default :). > > Thanks > >> >> Thanks, >> Hailiang >> >>>> + } >>>> return 0; >>>> } >>>> >>> >>> >>> . >>> >> >> >> > > > . >
On 01/20/2016 03:14 PM, Hailiang Zhang wrote: > On 2016/1/20 10:39, Jason Wang wrote: >> >> >> On 01/19/2016 04:39 PM, Hailiang Zhang wrote: >>> Hi Jason, >>> >>> Thanks for your review. >>> >>> On 2016/1/19 11:19, Jason Wang wrote: >>>> >>>> >>>> On 12/29/2015 03:09 PM, zhanghailiang wrote: >>>>> We add each netdev (except vhost-net) a default filter-buffer, >>>>> which will be used for COLO or Micro-checkpoint to buffer VM's >>>>> packets. >>>>> The name of default filter-buffer is 'nop'. >>>>> For the default filter-buffer, it will not buffer any packets in >>>>> default. >>>>> So it has no side effect for the netdev. >>>>> >>>>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >>>>> Cc: Jason Wang <jasowang@redhat.com> >>>>> Cc: Yang Hongyang <hongyang.yang@easystack.cn> >>>> >>>> This patch did three things: >>>> >>>> 1) the ability to enable or disable a netfilter >>>> 2) the ability to add a default filter >>>> 3) default filter attaching for filter-buffer >>>> >>>> Better to split them into separate small patches. >>>> >>>> And several questions: >>>> >>>> For 1), I'm not sure this is real needed, we can in fact disable a >>>> filter by removing it. >>> >>> If we do like this, do we also need to _enable_ the buffer filter by >>> add it dynamically instead of attaching the default filter ? >>> Just like what we do in V10 ? >>> (In that series, you think have a default filter may be better. >>> The main reason for that is to support >>> hot-add nic. Since we didn't support hot-add nic during COLO, >>> it will be OK to add default filter dynamically) >> >> Aha, right. So rethinking of this, enabling/disabling a filter looks ok >> to me. >> >>> >>>> For 2), Instead of a specific code just for filter buffer, I think we >>>> need a generic method for an arbitrary filter to be used as default. >>> >>> Good idea. >>> >>>> And if we can achieve 2), 3) is not needed any more. >>>> >>>>> --- >>>>> v12: >>>>> - Skip vhost-net when add default filter >>>>> - Don't go through filter layer if the filter is disabled. >>>>> v11: >>>>> - New patch >>>>> --- >>>>> include/net/filter.h | 10 +++++++ >>>>> net/filter-buffer.c | 82 >>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> net/filter.c | 6 +++- >>>>> net/net.c | 12 ++++++++ >>>>> 4 files changed, 109 insertions(+), 1 deletion(-) >>>>> [...] >>>>> +/* >>>>> +* This will be used by COLO or MC FT, for which they will need >>>>> +* to buffer the packets of VM's net devices, Here we add a default >>>>> +* buffer filter for each netdev. The name of default buffer >>>>> filter is >>>>> +* 'nop' >>>>> +*/ >>>>> +void netdev_add_default_filter_buffer(const char *netdev_id, >>>>> + NetFilterDirection direction, >>>>> + Error **errp) >>>>> +{ >>>> >>>> Need a more generic way to add an arbitrary filter as default. E.g >>>> during netdev init, query if there's a default and do the >>>> initialization >>>> there. >>>> >>> >>> We call it in net_client_init1(), i don't find a better place to >>> call it, >>> what's your suggestion ? >> >> net_client_init1() is ok, just need a generic way. E.g a string which >> stores the default filter and its parameters which could be queried by >> default filter initialization function. >> > > Hmm, i'm still confused about how to realize this, do you mean change > this helper > to a more generic function ? Just like : > void netdev_add_filter(const char *netdev_id, > const char *filter_type, char *id, > Error **errp) > > Could you please give me more detail about how to realize it ? :) I mean you need to know the type of default filter before calling netdev_add_filter(). May need a global pointer for this, and colo may set this during initialization. And in the future, we may allow this to be set from cli. > >>> >>>>> + QmpOutputVisitor *qov; >>>>> + QmpInputVisitor *qiv; >>>>> + Visitor *ov, *iv; >>>>> + QObject *obj = NULL; >>>>> + QDict *qdict; >>>>> + void *dummy = NULL; >>>>> + const char *id = "nop"; >>>>> + char *queue = g_strdup(NetFilterDirection_lookup[direction]); >>>>> + NetClientState *nc = qemu_find_netdev(netdev_id); >>>>> + Error *err = NULL; >>>>> + >>>>> + /* FIXME: Not support multiple queues */ >>>>> + if (!nc || nc->queue_index > 1) { >>>>> + g_free(queue); >>>>> + return; >>>>> + } >>>>> + /* Not support vhost-net */ >>>>> + if (get_vhost_net(nc)) { >>>>> + g_free(queue); >>>>> + return; >>>>> + } >>>>> + qov = qmp_output_visitor_new(); >>>>> + ov = qmp_output_get_visitor(qov); >>>>> + visit_start_struct(ov, &dummy, NULL, NULL, 0, &err); >>>>> + if (err) { >>>>> + goto out; >>>>> + } >>>>> + visit_type_str(ov, &nc->name, "netdev", &err); >>>>> + if (err) { >>>>> + goto out; >>>>> + } >>>>> + visit_type_str(ov, &queue, "queue", &err); >>>>> + if (err) { >>>>> + goto out; >>>>> + } >>>>> + visit_end_struct(ov, &err); >>>>> + if (err) { >>>>> + goto out; >>>>> + } >>>>> + obj = qmp_output_get_qobject(qov); >>>>> + g_assert(obj != NULL); >>>>> + qdict = qobject_to_qdict(obj); >>>>> + qmp_output_visitor_cleanup(qov); >>>>> + >>>>> + qiv = qmp_input_visitor_new(obj); >>>>> + iv = qmp_input_get_visitor(qiv); >>>>> + object_add(TYPE_FILTER_BUFFER, id, qdict, iv, &err); >>>>> + qmp_input_visitor_cleanup(qiv); >>>>> + qobject_decref(obj); >>>>> +out: >>>>> + g_free(queue); >>>>> + if (err) { >>>>> + error_propagate(errp, err); >>>>> + } >>>>> +} >>>>> + >>>>> static void filter_buffer_init(Object *obj) >>>>> { >>>>> object_property_add(obj, "interval", "int", >>>>> diff --git a/net/filter.c b/net/filter.c >>>>> index 1365bad..0b1e408 100644 >>>>> --- a/net/filter.c >>>>> +++ b/net/filter.c >>>>> @@ -163,7 +163,8 @@ static void netfilter_complete(UserCreatable >>>>> *uc, Error **errp) >>>>> } >>>>> >>>>> nf->netdev = ncs[0]; >>>>> - >>>>> + nf->is_default = false; >>>>> + nf->enabled = true; >>>> >>>> If we really need something like enabled, need more code for user to >>>> control this bit. >>>> >>> >>> OK, i will fix it. >> >> Or if you want to minimize the change set. You can make something like >> enabled locally to filter buffer, and let it to be used by colo only. >> > > I have convert it to use object property to set/get the value, is that > OK ? > > Thanks, > Hailiang It's ok.
On 2016/1/20 17:15, Jason Wang wrote: > > > On 01/20/2016 03:14 PM, Hailiang Zhang wrote: >> On 2016/1/20 10:39, Jason Wang wrote: >>> >>> >>> On 01/19/2016 04:39 PM, Hailiang Zhang wrote: >>>> Hi Jason, >>>> >>>> Thanks for your review. >>>> >>>> On 2016/1/19 11:19, Jason Wang wrote: >>>>> >>>>> >>>>> On 12/29/2015 03:09 PM, zhanghailiang wrote: >>>>>> We add each netdev (except vhost-net) a default filter-buffer, >>>>>> which will be used for COLO or Micro-checkpoint to buffer VM's >>>>>> packets. >>>>>> The name of default filter-buffer is 'nop'. >>>>>> For the default filter-buffer, it will not buffer any packets in >>>>>> default. >>>>>> So it has no side effect for the netdev. >>>>>> >>>>>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >>>>>> Cc: Jason Wang <jasowang@redhat.com> >>>>>> Cc: Yang Hongyang <hongyang.yang@easystack.cn> >>>>> >>>>> This patch did three things: >>>>> >>>>> 1) the ability to enable or disable a netfilter >>>>> 2) the ability to add a default filter >>>>> 3) default filter attaching for filter-buffer >>>>> >>>>> Better to split them into separate small patches. >>>>> >>>>> And several questions: >>>>> >>>>> For 1), I'm not sure this is real needed, we can in fact disable a >>>>> filter by removing it. >>>> >>>> If we do like this, do we also need to _enable_ the buffer filter by >>>> add it dynamically instead of attaching the default filter ? >>>> Just like what we do in V10 ? >>>> (In that series, you think have a default filter may be better. >>>> The main reason for that is to support >>>> hot-add nic. Since we didn't support hot-add nic during COLO, >>>> it will be OK to add default filter dynamically) >>> >>> Aha, right. So rethinking of this, enabling/disabling a filter looks ok >>> to me. >>> >>>> >>>>> For 2), Instead of a specific code just for filter buffer, I think we >>>>> need a generic method for an arbitrary filter to be used as default. >>>> >>>> Good idea. >>>> >>>>> And if we can achieve 2), 3) is not needed any more. >>>>> >>>>>> --- >>>>>> v12: >>>>>> - Skip vhost-net when add default filter >>>>>> - Don't go through filter layer if the filter is disabled. >>>>>> v11: >>>>>> - New patch >>>>>> --- >>>>>> include/net/filter.h | 10 +++++++ >>>>>> net/filter-buffer.c | 82 >>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>>> net/filter.c | 6 +++- >>>>>> net/net.c | 12 ++++++++ >>>>>> 4 files changed, 109 insertions(+), 1 deletion(-) >>>>>> > > [...] > >>>>>> +/* >>>>>> +* This will be used by COLO or MC FT, for which they will need >>>>>> +* to buffer the packets of VM's net devices, Here we add a default >>>>>> +* buffer filter for each netdev. The name of default buffer >>>>>> filter is >>>>>> +* 'nop' >>>>>> +*/ >>>>>> +void netdev_add_default_filter_buffer(const char *netdev_id, >>>>>> + NetFilterDirection direction, >>>>>> + Error **errp) >>>>>> +{ >>>>> >>>>> Need a more generic way to add an arbitrary filter as default. E.g >>>>> during netdev init, query if there's a default and do the >>>>> initialization >>>>> there. >>>>> >>>> >>>> We call it in net_client_init1(), i don't find a better place to >>>> call it, >>>> what's your suggestion ? >>> >>> net_client_init1() is ok, just need a generic way. E.g a string which >>> stores the default filter and its parameters which could be queried by >>> default filter initialization function. >>> >> >> Hmm, i'm still confused about how to realize this, do you mean change >> this helper >> to a more generic function ? Just like : >> void netdev_add_filter(const char *netdev_id, >> const char *filter_type, char *id, >> Error **errp) >> >> Could you please give me more detail about how to realize it ? :) > > I mean you need to know the type of default filter before calling > netdev_add_filter(). May need a global pointer for this, and colo may > set this during initialization. And in the future, we may allow this to > be set from cli. > Got it, i will fix it, thanks! >> >>>> >>>>>> + QmpOutputVisitor *qov; >>>>>> + QmpInputVisitor *qiv; >>>>>> + Visitor *ov, *iv; >>>>>> + QObject *obj = NULL; >>>>>> + QDict *qdict; >>>>>> + void *dummy = NULL; >>>>>> + const char *id = "nop"; >>>>>> + char *queue = g_strdup(NetFilterDirection_lookup[direction]); >>>>>> + NetClientState *nc = qemu_find_netdev(netdev_id); >>>>>> + Error *err = NULL; >>>>>> + >>>>>> + /* FIXME: Not support multiple queues */ >>>>>> + if (!nc || nc->queue_index > 1) { >>>>>> + g_free(queue); >>>>>> + return; >>>>>> + } >>>>>> + /* Not support vhost-net */ >>>>>> + if (get_vhost_net(nc)) { >>>>>> + g_free(queue); >>>>>> + return; >>>>>> + } >>>>>> + qov = qmp_output_visitor_new(); >>>>>> + ov = qmp_output_get_visitor(qov); >>>>>> + visit_start_struct(ov, &dummy, NULL, NULL, 0, &err); >>>>>> + if (err) { >>>>>> + goto out; >>>>>> + } >>>>>> + visit_type_str(ov, &nc->name, "netdev", &err); >>>>>> + if (err) { >>>>>> + goto out; >>>>>> + } >>>>>> + visit_type_str(ov, &queue, "queue", &err); >>>>>> + if (err) { >>>>>> + goto out; >>>>>> + } >>>>>> + visit_end_struct(ov, &err); >>>>>> + if (err) { >>>>>> + goto out; >>>>>> + } >>>>>> + obj = qmp_output_get_qobject(qov); >>>>>> + g_assert(obj != NULL); >>>>>> + qdict = qobject_to_qdict(obj); >>>>>> + qmp_output_visitor_cleanup(qov); >>>>>> + >>>>>> + qiv = qmp_input_visitor_new(obj); >>>>>> + iv = qmp_input_get_visitor(qiv); >>>>>> + object_add(TYPE_FILTER_BUFFER, id, qdict, iv, &err); >>>>>> + qmp_input_visitor_cleanup(qiv); >>>>>> + qobject_decref(obj); >>>>>> +out: >>>>>> + g_free(queue); >>>>>> + if (err) { >>>>>> + error_propagate(errp, err); >>>>>> + } >>>>>> +} >>>>>> + >>>>>> static void filter_buffer_init(Object *obj) >>>>>> { >>>>>> object_property_add(obj, "interval", "int", >>>>>> diff --git a/net/filter.c b/net/filter.c >>>>>> index 1365bad..0b1e408 100644 >>>>>> --- a/net/filter.c >>>>>> +++ b/net/filter.c >>>>>> @@ -163,7 +163,8 @@ static void netfilter_complete(UserCreatable >>>>>> *uc, Error **errp) >>>>>> } >>>>>> >>>>>> nf->netdev = ncs[0]; >>>>>> - >>>>>> + nf->is_default = false; >>>>>> + nf->enabled = true; >>>>> >>>>> If we really need something like enabled, need more code for user to >>>>> control this bit. >>>>> >>>> >>>> OK, i will fix it. >>> >>> Or if you want to minimize the change set. You can make something like >>> enabled locally to filter buffer, and let it to be used by colo only. >>> >> >> I have convert it to use object property to set/get the value, is that >> OK ? >> >> Thanks, >> Hailiang > > It's ok. > > > . >
diff --git a/include/net/filter.h b/include/net/filter.h index 2deda36..40aa38c 100644 --- a/include/net/filter.h +++ b/include/net/filter.h @@ -56,6 +56,8 @@ struct NetFilterState { NetClientState *netdev; NetFilterDirection direction; char info_str[256]; + bool is_default; + bool enabled; QTAILQ_ENTRY(NetFilterState) next; }; @@ -74,4 +76,12 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, int iovcnt, void *opaque); +static inline bool qemu_need_skip_netfilter(NetFilterState *nf) +{ + return nf->enabled ? false : true; +} + +void netdev_add_default_filter_buffer(const char *netdev_id, + NetFilterDirection direction, + Error **errp); #endif /* QEMU_NET_FILTER_H */ diff --git a/net/filter-buffer.c b/net/filter-buffer.c index 57be149..9cf3544 100644 --- a/net/filter-buffer.c +++ b/net/filter-buffer.c @@ -14,6 +14,13 @@ #include "qapi/qmp/qerror.h" #include "qapi-visit.h" #include "qom/object.h" +#include "net/net.h" +#include "qapi/qmp/qdict.h" +#include "qapi/qmp-output-visitor.h" +#include "qapi/qmp-input-visitor.h" +#include "monitor/monitor.h" +#include "qmp-commands.h" +#include "net/vhost_net.h" #define TYPE_FILTER_BUFFER "filter-buffer" @@ -102,6 +109,7 @@ static void filter_buffer_cleanup(NetFilterState *nf) static void filter_buffer_setup(NetFilterState *nf, Error **errp) { FilterBufferState *s = FILTER_BUFFER(nf); + char *path = object_get_canonical_path_component(OBJECT(nf)); /* * We may want to accept zero interval when VM FT solutions like MC @@ -114,6 +122,14 @@ static void filter_buffer_setup(NetFilterState *nf, Error **errp) } s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf); + nf->is_default = !strcmp(path, "nop"); + /* + * For the default buffer filter, it will be disabled by default, + * So it will not buffer any packets. + */ + if (nf->is_default) { + nf->enabled = false; + } if (s->interval) { timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL, filter_buffer_release_timer, nf); @@ -163,6 +179,72 @@ out: error_propagate(errp, local_err); } +/* +* This will be used by COLO or MC FT, for which they will need +* to buffer the packets of VM's net devices, Here we add a default +* buffer filter for each netdev. The name of default buffer filter is +* 'nop' +*/ +void netdev_add_default_filter_buffer(const char *netdev_id, + NetFilterDirection direction, + Error **errp) +{ + QmpOutputVisitor *qov; + QmpInputVisitor *qiv; + Visitor *ov, *iv; + QObject *obj = NULL; + QDict *qdict; + void *dummy = NULL; + const char *id = "nop"; + char *queue = g_strdup(NetFilterDirection_lookup[direction]); + NetClientState *nc = qemu_find_netdev(netdev_id); + Error *err = NULL; + + /* FIXME: Not support multiple queues */ + if (!nc || nc->queue_index > 1) { + g_free(queue); + return; + } + /* Not support vhost-net */ + if (get_vhost_net(nc)) { + g_free(queue); + return; + } + qov = qmp_output_visitor_new(); + ov = qmp_output_get_visitor(qov); + visit_start_struct(ov, &dummy, NULL, NULL, 0, &err); + if (err) { + goto out; + } + visit_type_str(ov, &nc->name, "netdev", &err); + if (err) { + goto out; + } + visit_type_str(ov, &queue, "queue", &err); + if (err) { + goto out; + } + visit_end_struct(ov, &err); + if (err) { + goto out; + } + obj = qmp_output_get_qobject(qov); + g_assert(obj != NULL); + qdict = qobject_to_qdict(obj); + qmp_output_visitor_cleanup(qov); + + qiv = qmp_input_visitor_new(obj); + iv = qmp_input_get_visitor(qiv); + object_add(TYPE_FILTER_BUFFER, id, qdict, iv, &err); + qmp_input_visitor_cleanup(qiv); + qobject_decref(obj); +out: + g_free(queue); + if (err) { + error_propagate(errp, err); + } +} + static void filter_buffer_init(Object *obj) { object_property_add(obj, "interval", "int", diff --git a/net/filter.c b/net/filter.c index 1365bad..0b1e408 100644 --- a/net/filter.c +++ b/net/filter.c @@ -163,7 +163,8 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) } nf->netdev = ncs[0]; - + nf->is_default = false; + nf->enabled = true; if (nfc->setup) { nfc->setup(nf, &local_err); if (local_err) { @@ -190,6 +191,9 @@ static void netfilter_complete(UserCreatable *uc, Error **errp) g_free(info); } object_property_iter_free(iter); + info = g_strdup_printf(",status=%s", nf->enabled ? "on" : "off"); + g_strlcat(nf->info_str, info, sizeof(nf->info_str)); + g_free(info); } static void netfilter_finalize(Object *obj) diff --git a/net/net.c b/net/net.c index 87dd356..fd53cfc 100644 --- a/net/net.c +++ b/net/net.c @@ -581,6 +581,10 @@ static ssize_t filter_receive_iov(NetClientState *nc, NetFilterState *nf = NULL; QTAILQ_FOREACH(nf, &nc->filters, next) { + /* Don't go through filter if it is off */ + if (qemu_need_skip_netfilter(nf)) { + continue; + } ret = qemu_netfilter_receive(nf, direction, sender, flags, iov, iovcnt, sent_cb); if (ret) { @@ -1028,6 +1032,14 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp) } return -1; } + + if (is_netdev) { + const Netdev *netdev = object; + + netdev_add_default_filter_buffer(netdev->id, + NET_FILTER_DIRECTION_RX, + errp); + } return 0; }
We add each netdev (except vhost-net) a default filter-buffer, which will be used for COLO or Micro-checkpoint to buffer VM's packets. The name of default filter-buffer is 'nop'. For the default filter-buffer, it will not buffer any packets in default. So it has no side effect for the netdev. Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> Cc: Jason Wang <jasowang@redhat.com> Cc: Yang Hongyang <hongyang.yang@easystack.cn> --- v12: - Skip vhost-net when add default filter - Don't go through filter layer if the filter is disabled. v11: - New patch --- include/net/filter.h | 10 +++++++ net/filter-buffer.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++ net/filter.c | 6 +++- net/net.c | 12 ++++++++ 4 files changed, 109 insertions(+), 1 deletion(-)