Filter needs to process the event of checkpoint/failover or
other event passed by COLO frame.
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
include/net/filter.h | 5 +++++
net/filter.c | 17 +++++++++++++++++
net/net.c | 28 ++++++++++++++++++++++++++++
3 files changed, 50 insertions(+)
diff --git a/include/net/filter.h b/include/net/filter.h
index 435acd6f82..49da666ac0 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc,
typedef void (FilterStatusChanged) (NetFilterState *nf, Error **errp);
+typedef void (FilterHandleEvent) (NetFilterState *nf, int event, Error **errp);
+
typedef struct NetFilterClass {
ObjectClass parent_class;
@@ -45,6 +47,7 @@ typedef struct NetFilterClass {
FilterSetup *setup;
FilterCleanup *cleanup;
FilterStatusChanged *status_changed;
+ FilterHandleEvent *handle_event;
/* mandatory */
FilterReceiveIOV *receive_iov;
} NetFilterClass;
@@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
int iovcnt,
void *opaque);
+void colo_notify_filters_event(int event, Error **errp);
+
#endif /* QEMU_NET_FILTER_H */
diff --git a/net/filter.c b/net/filter.c
index 2fd7d7d663..0f17eba143 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -17,6 +17,8 @@
#include "net/vhost_net.h"
#include "qom/object_interfaces.h"
#include "qemu/iov.h"
+#include "net/colo.h"
+#include "migration/colo.h"
static inline bool qemu_can_skip_netfilter(NetFilterState *nf)
{
@@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj)
g_free(nf->netdev_id);
}
+static void dummy_handle_event(NetFilterState *nf, int event, Error **errp)
+{
+ switch (event) {
+ case COLO_EVENT_CHECKPOINT:
+ break;
+ case COLO_EVENT_FAILOVER:
+ object_property_set_str(OBJECT(nf), "off", "status", errp);
+ break;
+ default:
+ break;
+ }
+}
+
static void netfilter_class_init(ObjectClass *oc, void *data)
{
UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+ NetFilterClass *nfc = NETFILTER_CLASS(oc);
ucc->complete = netfilter_complete;
+ nfc->handle_event = dummy_handle_event;
}
static const TypeInfo netfilter_info = {
diff --git a/net/net.c b/net/net.c
index a77ea88fff..b4f6a2efb2 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1331,6 +1331,34 @@ void hmp_info_network(Monitor *mon, const QDict *qdict)
}
}
+void colo_notify_filters_event(int event, Error **errp)
+{
+ NetClientState *nc, *peer;
+ NetClientDriver type;
+ NetFilterState *nf;
+ NetFilterClass *nfc = NULL;
+ Error *local_err = NULL;
+
+ QTAILQ_FOREACH(nc, &net_clients, next) {
+ peer = nc->peer;
+ type = nc->info->type;
+ if (!peer || type != NET_CLIENT_DRIVER_TAP) {
+ continue;
+ }
+ QTAILQ_FOREACH(nf, &nc->filters, next) {
+ nfc = NETFILTER_GET_CLASS(OBJECT(nf));
+ if (!nfc->handle_event) {
+ continue;
+ }
+ nfc->handle_event(nf, event, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ }
+ }
+}
+
void qmp_set_link(const char *name, bool up, Error **errp)
{
NetClientState *ncs[MAX_QUEUE_NUM];
--
2.17.GIT
On 2018年08月12日 04:59, Zhang Chen wrote:
> Filter needs to process the event of checkpoint/failover or
> other event passed by COLO frame.
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
> include/net/filter.h | 5 +++++
> net/filter.c | 17 +++++++++++++++++
> net/net.c | 28 ++++++++++++++++++++++++++++
> 3 files changed, 50 insertions(+)
>
> diff --git a/include/net/filter.h b/include/net/filter.h
> index 435acd6f82..49da666ac0 100644
> --- a/include/net/filter.h
> +++ b/include/net/filter.h
> @@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc,
>
> typedef void (FilterStatusChanged) (NetFilterState *nf, Error **errp);
>
> +typedef void (FilterHandleEvent) (NetFilterState *nf, int event, Error **errp);
> +
> typedef struct NetFilterClass {
> ObjectClass parent_class;
>
> @@ -45,6 +47,7 @@ typedef struct NetFilterClass {
> FilterSetup *setup;
> FilterCleanup *cleanup;
> FilterStatusChanged *status_changed;
> + FilterHandleEvent *handle_event;
> /* mandatory */
> FilterReceiveIOV *receive_iov;
> } NetFilterClass;
> @@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
> int iovcnt,
> void *opaque);
>
> +void colo_notify_filters_event(int event, Error **errp);
> +
> #endif /* QEMU_NET_FILTER_H */
> diff --git a/net/filter.c b/net/filter.c
> index 2fd7d7d663..0f17eba143 100644
> --- a/net/filter.c
> +++ b/net/filter.c
> @@ -17,6 +17,8 @@
> #include "net/vhost_net.h"
> #include "qom/object_interfaces.h"
> #include "qemu/iov.h"
> +#include "net/colo.h"
> +#include "migration/colo.h"
>
> static inline bool qemu_can_skip_netfilter(NetFilterState *nf)
> {
> @@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj)
> g_free(nf->netdev_id);
> }
>
> +static void dummy_handle_event(NetFilterState *nf, int event, Error **errp)
> +{
It's in fact not a "dummy" handler, Maybe it's better to rename it as
"default".
> + switch (event) {
> + case COLO_EVENT_CHECKPOINT:
> + break;
> + case COLO_EVENT_FAILOVER:
> + object_property_set_str(OBJECT(nf), "off", "status", errp);
I think filter is a generic infrastructure, so it's better not have COLO
specific things like this. You can either add a generic name or have a
dedicated helper to just disable all net filters.
> + break;
> + default:
> + break;
> + }
> +}
> +
> static void netfilter_class_init(ObjectClass *oc, void *data)
> {
> UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
> + NetFilterClass *nfc = NETFILTER_CLASS(oc);
>
> ucc->complete = netfilter_complete;
> + nfc->handle_event = dummy_handle_event;
> }
>
> static const TypeInfo netfilter_info = {
> diff --git a/net/net.c b/net/net.c
> index a77ea88fff..b4f6a2efb2 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1331,6 +1331,34 @@ void hmp_info_network(Monitor *mon, const QDict *qdict)
> }
> }
>
> +void colo_notify_filters_event(int event, Error **errp)
> +{
> + NetClientState *nc, *peer;
> + NetClientDriver type;
> + NetFilterState *nf;
> + NetFilterClass *nfc = NULL;
> + Error *local_err = NULL;
> +
> + QTAILQ_FOREACH(nc, &net_clients, next) {
> + peer = nc->peer;
> + type = nc->info->type;
> + if (!peer || type != NET_CLIENT_DRIVER_TAP) {
> + continue;
> + }
The check the TAP is redundant with previous patch.
> + QTAILQ_FOREACH(nf, &nc->filters, next) {
> + nfc = NETFILTER_GET_CLASS(OBJECT(nf));
> + if (!nfc->handle_event) {
Looks like this won't happen.
Thanks
> + continue;
> + }
> + nfc->handle_event(nf, event, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + }
> + }
> +}
> +
> void qmp_set_link(const char *name, bool up, Error **errp)
> {
> NetClientState *ncs[MAX_QUEUE_NUM];
On Tue, Aug 21, 2018 at 11:30 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2018年08月12日 04:59, Zhang Chen wrote:
> > Filter needs to process the event of checkpoint/failover or
> > other event passed by COLO frame.
> >
> > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> > ---
> > include/net/filter.h | 5 +++++
> > net/filter.c | 17 +++++++++++++++++
> > net/net.c | 28 ++++++++++++++++++++++++++++
> > 3 files changed, 50 insertions(+)
> >
> > diff --git a/include/net/filter.h b/include/net/filter.h
> > index 435acd6f82..49da666ac0 100644
> > --- a/include/net/filter.h
> > +++ b/include/net/filter.h
> > @@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc,
> >
> > typedef void (FilterStatusChanged) (NetFilterState *nf, Error **errp);
> >
> > +typedef void (FilterHandleEvent) (NetFilterState *nf, int event, Error
> **errp);
> > +
> > typedef struct NetFilterClass {
> > ObjectClass parent_class;
> >
> > @@ -45,6 +47,7 @@ typedef struct NetFilterClass {
> > FilterSetup *setup;
> > FilterCleanup *cleanup;
> > FilterStatusChanged *status_changed;
> > + FilterHandleEvent *handle_event;
> > /* mandatory */
> > FilterReceiveIOV *receive_iov;
> > } NetFilterClass;
> > @@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState
> *sender,
> > int iovcnt,
> > void *opaque);
> >
> > +void colo_notify_filters_event(int event, Error **errp);
> > +
> > #endif /* QEMU_NET_FILTER_H */
> > diff --git a/net/filter.c b/net/filter.c
> > index 2fd7d7d663..0f17eba143 100644
> > --- a/net/filter.c
> > +++ b/net/filter.c
> > @@ -17,6 +17,8 @@
> > #include "net/vhost_net.h"
> > #include "qom/object_interfaces.h"
> > #include "qemu/iov.h"
> > +#include "net/colo.h"
> > +#include "migration/colo.h"
> >
> > static inline bool qemu_can_skip_netfilter(NetFilterState *nf)
> > {
> > @@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj)
> > g_free(nf->netdev_id);
> > }
> >
> > +static void dummy_handle_event(NetFilterState *nf, int event, Error
> **errp)
> > +{
>
> It's in fact not a "dummy" handler, Maybe it's better to rename it as
> "default".
>
OK, I will rename it in next version.
>
> > + switch (event) {
> > + case COLO_EVENT_CHECKPOINT:
> > + break;
> > + case COLO_EVENT_FAILOVER:
> > + object_property_set_str(OBJECT(nf), "off", "status", errp);
>
> I think filter is a generic infrastructure, so it's better not have COLO
> specific things like this. You can either add a generic name or have a
> dedicated helper to just disable all net filters.
>
Maybe we can rename it to "EVENT_CHECKPOINT" and "EVENT_FAILOVER" looks
better?
>
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > static void netfilter_class_init(ObjectClass *oc, void *data)
> > {
> > UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
> > + NetFilterClass *nfc = NETFILTER_CLASS(oc);
> >
> > ucc->complete = netfilter_complete;
> > + nfc->handle_event = dummy_handle_event;
> > }
> >
> > static const TypeInfo netfilter_info = {
> > diff --git a/net/net.c b/net/net.c
> > index a77ea88fff..b4f6a2efb2 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -1331,6 +1331,34 @@ void hmp_info_network(Monitor *mon, const QDict
> *qdict)
> > }
> > }
> >
> > +void colo_notify_filters_event(int event, Error **errp)
> > +{
> > + NetClientState *nc, *peer;
> > + NetClientDriver type;
> > + NetFilterState *nf;
> > + NetFilterClass *nfc = NULL;
> > + Error *local_err = NULL;
> > +
> > + QTAILQ_FOREACH(nc, &net_clients, next) {
> > + peer = nc->peer;
> > + type = nc->info->type;
> > + if (!peer || type != NET_CLIENT_DRIVER_TAP) {
> > + continue;
> > + }
>
> The check the TAP is redundant with previous patch.
>
OK, I will remove it.
>
> > + QTAILQ_FOREACH(nf, &nc->filters, next) {
> > + nfc = NETFILTER_GET_CLASS(OBJECT(nf));
> > + if (!nfc->handle_event) {
>
> Looks like this won't happen.
>
It will happen. like filter-mirror and filter-redirector haven't this event.
Thanks
Zhang Chen
>
> Thanks
>
> > + continue;
> > + }
> > + nfc->handle_event(nf, event, &local_err);
> > + if (local_err) {
> > + error_propagate(errp, local_err);
> > + return;
> > + }
> > + }
> > + }
> > +}
> > +
> > void qmp_set_link(const char *name, bool up, Error **errp)
> > {
> > NetClientState *ncs[MAX_QUEUE_NUM];
>
>
On 2018年08月21日 17:25, Zhang Chen wrote:
> On Tue, Aug 21, 2018 at 11:30 AM Jason Wang <jasowang@redhat.com> wrote:
>
>>
>> On 2018年08月12日 04:59, Zhang Chen wrote:
>>> Filter needs to process the event of checkpoint/failover or
>>> other event passed by COLO frame.
>>>
>>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>>> ---
>>> include/net/filter.h | 5 +++++
>>> net/filter.c | 17 +++++++++++++++++
>>> net/net.c | 28 ++++++++++++++++++++++++++++
>>> 3 files changed, 50 insertions(+)
>>>
>>> diff --git a/include/net/filter.h b/include/net/filter.h
>>> index 435acd6f82..49da666ac0 100644
>>> --- a/include/net/filter.h
>>> +++ b/include/net/filter.h
>>> @@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState *nc,
>>>
>>> typedef void (FilterStatusChanged) (NetFilterState *nf, Error **errp);
>>>
>>> +typedef void (FilterHandleEvent) (NetFilterState *nf, int event, Error
>> **errp);
>>> +
>>> typedef struct NetFilterClass {
>>> ObjectClass parent_class;
>>>
>>> @@ -45,6 +47,7 @@ typedef struct NetFilterClass {
>>> FilterSetup *setup;
>>> FilterCleanup *cleanup;
>>> FilterStatusChanged *status_changed;
>>> + FilterHandleEvent *handle_event;
>>> /* mandatory */
>>> FilterReceiveIOV *receive_iov;
>>> } NetFilterClass;
>>> @@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState
>> *sender,
>>> int iovcnt,
>>> void *opaque);
>>>
>>> +void colo_notify_filters_event(int event, Error **errp);
>>> +
>>> #endif /* QEMU_NET_FILTER_H */
>>> diff --git a/net/filter.c b/net/filter.c
>>> index 2fd7d7d663..0f17eba143 100644
>>> --- a/net/filter.c
>>> +++ b/net/filter.c
>>> @@ -17,6 +17,8 @@
>>> #include "net/vhost_net.h"
>>> #include "qom/object_interfaces.h"
>>> #include "qemu/iov.h"
>>> +#include "net/colo.h"
>>> +#include "migration/colo.h"
>>>
>>> static inline bool qemu_can_skip_netfilter(NetFilterState *nf)
>>> {
>>> @@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj)
>>> g_free(nf->netdev_id);
>>> }
>>>
>>> +static void dummy_handle_event(NetFilterState *nf, int event, Error
>> **errp)
>>> +{
>> It's in fact not a "dummy" handler, Maybe it's better to rename it as
>> "default".
>>
> OK, I will rename it in next version.
>
>
>>> + switch (event) {
>>> + case COLO_EVENT_CHECKPOINT:
>>> + break;
>>> + case COLO_EVENT_FAILOVER:
>>> + object_property_set_str(OBJECT(nf), "off", "status", errp);
>> I think filter is a generic infrastructure, so it's better not have COLO
>> specific things like this. You can either add a generic name or have a
>> dedicated helper to just disable all net filters.
>>
> Maybe we can rename it to "EVENT_CHECKPOINT" and "EVENT_FAILOVER" looks
> better?
>
I think registering notifier to COLO is better. For disabling filter, we
can have a generic dedicated helpers to do this.
Btw, I remember we allow disabling filter through qmp, if it's true, we
probably need some notification to management, but this can be done in
the future.
>>> + break;
>>> + default:
>>> + break;
>>> + }
>>> +}
>>> +
>>> static void netfilter_class_init(ObjectClass *oc, void *data)
>>> {
>>> UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
>>> + NetFilterClass *nfc = NETFILTER_CLASS(oc);
>>>
>>> ucc->complete = netfilter_complete;
>>> + nfc->handle_event = dummy_handle_event;
>>> }
>>>
>>> static const TypeInfo netfilter_info = {
>>> diff --git a/net/net.c b/net/net.c
>>> index a77ea88fff..b4f6a2efb2 100644
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>> @@ -1331,6 +1331,34 @@ void hmp_info_network(Monitor *mon, const QDict
>> *qdict)
>>> }
>>> }
>>>
>>> +void colo_notify_filters_event(int event, Error **errp)
>>> +{
>>> + NetClientState *nc, *peer;
>>> + NetClientDriver type;
>>> + NetFilterState *nf;
>>> + NetFilterClass *nfc = NULL;
>>> + Error *local_err = NULL;
>>> +
>>> + QTAILQ_FOREACH(nc, &net_clients, next) {
>>> + peer = nc->peer;
>>> + type = nc->info->type;
>>> + if (!peer || type != NET_CLIENT_DRIVER_TAP) {
>>> + continue;
>>> + }
>> The check the TAP is redundant with previous patch.
>>
> OK, I will remove it.
>
>
>>> + QTAILQ_FOREACH(nf, &nc->filters, next) {
>>> + nfc = NETFILTER_GET_CLASS(OBJECT(nf));
>>> + if (!nfc->handle_event) {
>> Looks like this won't happen.
>>
> It will happen. like filter-mirror and filter-redirector haven't this event.
But you do:
static void netfilter_class_init(ObjectClass *oc, void *data)
{
UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+ NetFilterClass *nfc = NETFILTER_CLASS(oc);
ucc->complete = netfilter_complete;
+ nfc->handle_event = dummy_handle_event;
}
?
Thanks
>
> Thanks
> Zhang Chen
>
>
>> Thanks
>>
>>> + continue;
>>> + }
>>> + nfc->handle_event(nf, event, &local_err);
>>> + if (local_err) {
>>> + error_propagate(errp, local_err);
>>> + return;
>>> + }
>>> + }
>>> + }
>>> +}
>>> +
>>> void qmp_set_link(const char *name, bool up, Error **errp)
>>> {
>>> NetClientState *ncs[MAX_QUEUE_NUM];
>>
On Wed, Aug 22, 2018 at 4:22 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2018年08月21日 17:25, Zhang Chen wrote:
> > On Tue, Aug 21, 2018 at 11:30 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> >>
> >> On 2018年08月12日 04:59, Zhang Chen wrote:
> >>> Filter needs to process the event of checkpoint/failover or
> >>> other event passed by COLO frame.
> >>>
> >>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> >>> ---
> >>> include/net/filter.h | 5 +++++
> >>> net/filter.c | 17 +++++++++++++++++
> >>> net/net.c | 28 ++++++++++++++++++++++++++++
> >>> 3 files changed, 50 insertions(+)
> >>>
> >>> diff --git a/include/net/filter.h b/include/net/filter.h
> >>> index 435acd6f82..49da666ac0 100644
> >>> --- a/include/net/filter.h
> >>> +++ b/include/net/filter.h
> >>> @@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState
> *nc,
> >>>
> >>> typedef void (FilterStatusChanged) (NetFilterState *nf, Error
> **errp);
> >>>
> >>> +typedef void (FilterHandleEvent) (NetFilterState *nf, int event, Error
> >> **errp);
> >>> +
> >>> typedef struct NetFilterClass {
> >>> ObjectClass parent_class;
> >>>
> >>> @@ -45,6 +47,7 @@ typedef struct NetFilterClass {
> >>> FilterSetup *setup;
> >>> FilterCleanup *cleanup;
> >>> FilterStatusChanged *status_changed;
> >>> + FilterHandleEvent *handle_event;
> >>> /* mandatory */
> >>> FilterReceiveIOV *receive_iov;
> >>> } NetFilterClass;
> >>> @@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState
> >> *sender,
> >>> int iovcnt,
> >>> void *opaque);
> >>>
> >>> +void colo_notify_filters_event(int event, Error **errp);
> >>> +
> >>> #endif /* QEMU_NET_FILTER_H */
> >>> diff --git a/net/filter.c b/net/filter.c
> >>> index 2fd7d7d663..0f17eba143 100644
> >>> --- a/net/filter.c
> >>> +++ b/net/filter.c
> >>> @@ -17,6 +17,8 @@
> >>> #include "net/vhost_net.h"
> >>> #include "qom/object_interfaces.h"
> >>> #include "qemu/iov.h"
> >>> +#include "net/colo.h"
> >>> +#include "migration/colo.h"
> >>>
> >>> static inline bool qemu_can_skip_netfilter(NetFilterState *nf)
> >>> {
> >>> @@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj)
> >>> g_free(nf->netdev_id);
> >>> }
> >>>
> >>> +static void dummy_handle_event(NetFilterState *nf, int event, Error
> >> **errp)
> >>> +{
> >> It's in fact not a "dummy" handler, Maybe it's better to rename it as
> >> "default".
> >>
> > OK, I will rename it in next version.
> >
> >
> >>> + switch (event) {
> >>> + case COLO_EVENT_CHECKPOINT:
> >>> + break;
> >>> + case COLO_EVENT_FAILOVER:
> >>> + object_property_set_str(OBJECT(nf), "off", "status", errp);
> >> I think filter is a generic infrastructure, so it's better not have COLO
> >> specific things like this. You can either add a generic name or have a
> >> dedicated helper to just disable all net filters.
> >>
> > Maybe we can rename it to "EVENT_CHECKPOINT" and "EVENT_FAILOVER" looks
> > better?
> >
>
> I think registering notifier to COLO is better. For disabling filter, we
> can have a generic dedicated helpers to do this.
>
If we registering notifier to COLO (in migration/colo.c), filter-rewriter
can not get the notify immediately when checkout occur.
filter-reweiter didn't know when checkpoint happened, and I think loop to
check the COLO event in filter-rewriter is a bad choice.
Any thing I misunderstand?
>
> Btw, I remember we allow disabling filter through qmp, if it's true, we
> probably need some notification to management, but this can be done in
> the future.
>
> >>> + break;
> >>> + default:
> >>> + break;
> >>> + }
> >>> +}
> >>> +
> >>> static void netfilter_class_init(ObjectClass *oc, void *data)
> >>> {
> >>> UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
> >>> + NetFilterClass *nfc = NETFILTER_CLASS(oc);
> >>>
> >>> ucc->complete = netfilter_complete;
> >>> + nfc->handle_event = dummy_handle_event;
> >>> }
> >>>
> >>> static const TypeInfo netfilter_info = {
> >>> diff --git a/net/net.c b/net/net.c
> >>> index a77ea88fff..b4f6a2efb2 100644
> >>> --- a/net/net.c
> >>> +++ b/net/net.c
> >>> @@ -1331,6 +1331,34 @@ void hmp_info_network(Monitor *mon, const QDict
> >> *qdict)
> >>> }
> >>> }
> >>>
> >>> +void colo_notify_filters_event(int event, Error **errp)
> >>> +{
> >>> + NetClientState *nc, *peer;
> >>> + NetClientDriver type;
> >>> + NetFilterState *nf;
> >>> + NetFilterClass *nfc = NULL;
> >>> + Error *local_err = NULL;
> >>> +
> >>> + QTAILQ_FOREACH(nc, &net_clients, next) {
> >>> + peer = nc->peer;
> >>> + type = nc->info->type;
> >>> + if (!peer || type != NET_CLIENT_DRIVER_TAP) {
> >>> + continue;
> >>> + }
> >> The check the TAP is redundant with previous patch.
> >>
> > OK, I will remove it.
> >
> >
> >>> + QTAILQ_FOREACH(nf, &nc->filters, next) {
> >>> + nfc = NETFILTER_GET_CLASS(OBJECT(nf));
> >>> + if (!nfc->handle_event) {
> >> Looks like this won't happen.
> >>
> > It will happen. like filter-mirror and filter-redirector haven't this
> event.
>
> But you do:
>
> static void netfilter_class_init(ObjectClass *oc, void *data)
> {
> UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
> + NetFilterClass *nfc = NETFILTER_CLASS(oc);
>
> ucc->complete = netfilter_complete;
> + nfc->handle_event = dummy_handle_event;
> }
>
> ?
>
Oh, I forgot thiat. you are right, I will remove this in next version.
Thanks
Zhang Chen
>
> Thanks
>
> >
> > Thanks
> > Zhang Chen
> >
> >
> >> Thanks
> >>
> >>> + continue;
> >>> + }
> >>> + nfc->handle_event(nf, event, &local_err);
> >>> + if (local_err) {
> >>> + error_propagate(errp, local_err);
> >>> + return;
> >>> + }
> >>> + }
> >>> + }
> >>> +}
> >>> +
> >>> void qmp_set_link(const char *name, bool up, Error **errp)
> >>> {
> >>> NetClientState *ncs[MAX_QUEUE_NUM];
> >>
>
>
On 2018年08月24日 13:57, Zhang Chen wrote:
> On Wed, Aug 22, 2018 at 4:22 PM Jason Wang<jasowang@redhat.com> wrote:
>
>> On 2018年08月21日 17:25, Zhang Chen wrote:
>>> On Tue, Aug 21, 2018 at 11:30 AM Jason Wang<jasowang@redhat.com> wrote:
>>>
>>>> On 2018年08月12日 04:59, Zhang Chen wrote:
>>>>> Filter needs to process the event of checkpoint/failover or
>>>>> other event passed by COLO frame.
>>>>>
>>>>> Signed-off-by: zhanghailiang<zhang.zhanghailiang@huawei.com>
>>>>> ---
>>>>> include/net/filter.h | 5 +++++
>>>>> net/filter.c | 17 +++++++++++++++++
>>>>> net/net.c | 28 ++++++++++++++++++++++++++++
>>>>> 3 files changed, 50 insertions(+)
>>>>>
>>>>> diff --git a/include/net/filter.h b/include/net/filter.h
>>>>> index 435acd6f82..49da666ac0 100644
>>>>> --- a/include/net/filter.h
>>>>> +++ b/include/net/filter.h
>>>>> @@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState
>> *nc,
>>>>> typedef void (FilterStatusChanged) (NetFilterState *nf, Error
>> **errp);
>>>>> +typedef void (FilterHandleEvent) (NetFilterState *nf, int event, Error
>>>> **errp);
>>>>> +
>>>>> typedef struct NetFilterClass {
>>>>> ObjectClass parent_class;
>>>>>
>>>>> @@ -45,6 +47,7 @@ typedef struct NetFilterClass {
>>>>> FilterSetup *setup;
>>>>> FilterCleanup *cleanup;
>>>>> FilterStatusChanged *status_changed;
>>>>> + FilterHandleEvent *handle_event;
>>>>> /* mandatory */
>>>>> FilterReceiveIOV *receive_iov;
>>>>> } NetFilterClass;
>>>>> @@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState
>>>> *sender,
>>>>> int iovcnt,
>>>>> void *opaque);
>>>>>
>>>>> +void colo_notify_filters_event(int event, Error **errp);
>>>>> +
>>>>> #endif /* QEMU_NET_FILTER_H */
>>>>> diff --git a/net/filter.c b/net/filter.c
>>>>> index 2fd7d7d663..0f17eba143 100644
>>>>> --- a/net/filter.c
>>>>> +++ b/net/filter.c
>>>>> @@ -17,6 +17,8 @@
>>>>> #include "net/vhost_net.h"
>>>>> #include "qom/object_interfaces.h"
>>>>> #include "qemu/iov.h"
>>>>> +#include "net/colo.h"
>>>>> +#include "migration/colo.h"
>>>>>
>>>>> static inline bool qemu_can_skip_netfilter(NetFilterState *nf)
>>>>> {
>>>>> @@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj)
>>>>> g_free(nf->netdev_id);
>>>>> }
>>>>>
>>>>> +static void dummy_handle_event(NetFilterState *nf, int event, Error
>>>> **errp)
>>>>> +{
>>>> It's in fact not a "dummy" handler, Maybe it's better to rename it as
>>>> "default".
>>>>
>>> OK, I will rename it in next version.
>>>
>>>
>>>>> + switch (event) {
>>>>> + case COLO_EVENT_CHECKPOINT:
>>>>> + break;
>>>>> + case COLO_EVENT_FAILOVER:
>>>>> + object_property_set_str(OBJECT(nf), "off", "status", errp);
>>>> I think filter is a generic infrastructure, so it's better not have COLO
>>>> specific things like this. You can either add a generic name or have a
>>>> dedicated helper to just disable all net filters.
>>>>
>>> Maybe we can rename it to "EVENT_CHECKPOINT" and "EVENT_FAILOVER" looks
>>> better?
>>>
>> I think registering notifier to COLO is better. For disabling filter, we
>> can have a generic dedicated helpers to do this.
>>
> If we registering notifier to COLO (in migration/colo.c), filter-rewriter
> can not get the notify immediately when checkout occur.
> filter-reweiter didn't know when checkpoint happened,
I may miss something, isn't COLO which triggers the checkpoint even?
> and I think loop to
> check the COLO event in filter-rewriter is a bad choice.
Well, anyway you need a loop somewhere to iterate all notifiers?
Thanks
> Any thing I misunderstand?
>
>
>
On Tue, Aug 28, 2018 at 3:19 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2018年08月24日 13:57, Zhang Chen wrote:
> > On Wed, Aug 22, 2018 at 4:22 PM Jason Wang<jasowang@redhat.com> wrote:
> >
> >> On 2018年08月21日 17:25, Zhang Chen wrote:
> >>> On Tue, Aug 21, 2018 at 11:30 AM Jason Wang<jasowang@redhat.com>
> wrote:
> >>>
> >>>> On 2018年08月12日 04:59, Zhang Chen wrote:
> >>>>> Filter needs to process the event of checkpoint/failover or
> >>>>> other event passed by COLO frame.
> >>>>>
> >>>>> Signed-off-by: zhanghailiang<zhang.zhanghailiang@huawei.com>
> >>>>> ---
> >>>>> include/net/filter.h | 5 +++++
> >>>>> net/filter.c | 17 +++++++++++++++++
> >>>>> net/net.c | 28 ++++++++++++++++++++++++++++
> >>>>> 3 files changed, 50 insertions(+)
> >>>>>
> >>>>> diff --git a/include/net/filter.h b/include/net/filter.h
> >>>>> index 435acd6f82..49da666ac0 100644
> >>>>> --- a/include/net/filter.h
> >>>>> +++ b/include/net/filter.h
> >>>>> @@ -38,6 +38,8 @@ typedef ssize_t (FilterReceiveIOV)(NetFilterState
> >> *nc,
> >>>>> typedef void (FilterStatusChanged) (NetFilterState *nf, Error
> >> **errp);
> >>>>> +typedef void (FilterHandleEvent) (NetFilterState *nf, int event,
> Error
> >>>> **errp);
> >>>>> +
> >>>>> typedef struct NetFilterClass {
> >>>>> ObjectClass parent_class;
> >>>>>
> >>>>> @@ -45,6 +47,7 @@ typedef struct NetFilterClass {
> >>>>> FilterSetup *setup;
> >>>>> FilterCleanup *cleanup;
> >>>>> FilterStatusChanged *status_changed;
> >>>>> + FilterHandleEvent *handle_event;
> >>>>> /* mandatory */
> >>>>> FilterReceiveIOV *receive_iov;
> >>>>> } NetFilterClass;
> >>>>> @@ -77,4 +80,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState
> >>>> *sender,
> >>>>> int iovcnt,
> >>>>> void *opaque);
> >>>>>
> >>>>> +void colo_notify_filters_event(int event, Error **errp);
> >>>>> +
> >>>>> #endif /* QEMU_NET_FILTER_H */
> >>>>> diff --git a/net/filter.c b/net/filter.c
> >>>>> index 2fd7d7d663..0f17eba143 100644
> >>>>> --- a/net/filter.c
> >>>>> +++ b/net/filter.c
> >>>>> @@ -17,6 +17,8 @@
> >>>>> #include "net/vhost_net.h"
> >>>>> #include "qom/object_interfaces.h"
> >>>>> #include "qemu/iov.h"
> >>>>> +#include "net/colo.h"
> >>>>> +#include "migration/colo.h"
> >>>>>
> >>>>> static inline bool qemu_can_skip_netfilter(NetFilterState *nf)
> >>>>> {
> >>>>> @@ -245,11 +247,26 @@ static void netfilter_finalize(Object *obj)
> >>>>> g_free(nf->netdev_id);
> >>>>> }
> >>>>>
> >>>>> +static void dummy_handle_event(NetFilterState *nf, int event, Error
> >>>> **errp)
> >>>>> +{
> >>>> It's in fact not a "dummy" handler, Maybe it's better to rename it as
> >>>> "default".
> >>>>
> >>> OK, I will rename it in next version.
> >>>
> >>>
> >>>>> + switch (event) {
> >>>>> + case COLO_EVENT_CHECKPOINT:
> >>>>> + break;
> >>>>> + case COLO_EVENT_FAILOVER:
> >>>>> + object_property_set_str(OBJECT(nf), "off", "status", errp);
> >>>> I think filter is a generic infrastructure, so it's better not have
> COLO
> >>>> specific things like this. You can either add a generic name or have a
> >>>> dedicated helper to just disable all net filters.
> >>>>
> >>> Maybe we can rename it to "EVENT_CHECKPOINT" and "EVENT_FAILOVER" looks
> >>> better?
> >>>
> >> I think registering notifier to COLO is better. For disabling filter, we
> >> can have a generic dedicated helpers to do this.
> >>
> > If we registering notifier to COLO (in migration/colo.c), filter-rewriter
> > can not get the notify immediately when checkout occur.
> > filter-reweiter didn't know when checkpoint happened,
>
> I may miss something, isn't COLO which triggers the checkpoint even?
>
Yes, in detail, we have two way to trigger the checkpoint:
case 1. COLO compare trigger.
case 2. periodic checkpoint mechanism trigger.
But we both solve the same problem that how to notify filter-rewriter do
checkpoint in secondary node.
Currently we do this job in that way:
case 1. primary node COLO compare(net/colo-compare.c) ----> primary node
COLO frame(migration/colo.c) ------> secondary node COLO frame
-----------------> secondary node filter-rewriter.
case 2. primary node COLO frame(migration/colo.c) ------> secondary node
COLO frame -----------------> secondary node filter-rewriter.
So, this patch just focus on "secondary node COLO frame ----------------->
secondary node filter-rewriter".
In filter-rewriter hard to get the checkpoint message from COLO frame.
Thanks
Zhang Chen
>
> > and I think loop to
> > check the COLO event in filter-rewriter is a bad choice.
>
> Well, anyway you need a loop somewhere to iterate all notifiers?
>
> Thanks
>
> > Any thing I misunderstand?
> >
> >
> >
>
>
© 2016 - 2025 Red Hat, Inc.