| Message ID | 1454750932-7556-32-git-send-email-zhang.zhanghailiang@huawei.com |
|---|---|
| State | New |
| Headers | show |
On 02/06/2016 05:28 PM, zhanghailiang wrote: > With this property, users can control if this filter is 'enable' > or 'disable'. The default behavior for filter is enabled. > > We will skip the disabled filter when delivering packets in net layer. > > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > Cc: Jason Wang <jasowang@redhat.com> > Cc: Yang Hongyang <hongyang.yang@easystack.cn> > --- > include/net/filter.h | 1 + > net/filter.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 46 insertions(+) > > diff --git a/include/net/filter.h b/include/net/filter.h > index 5639976..af3c53c 100644 > --- a/include/net/filter.h > +++ b/include/net/filter.h > @@ -55,6 +55,7 @@ struct NetFilterState { > char *netdev_id; > NetClientState *netdev; > NetFilterDirection direction; > + bool enabled; > QTAILQ_ENTRY(NetFilterState) next; > }; > > diff --git a/net/filter.c b/net/filter.c > index d2a514e..5551cf1 100644 > --- a/net/filter.c > +++ b/net/filter.c > @@ -17,6 +17,11 @@ > #include "qom/object_interfaces.h" > #include "qemu/iov.h" > > +static inline bool qemu_need_skip_netfilter(NetFilterState *nf) > +{ > + return nf->enabled ? false : true; > +} Suggest to rename this to qemu_netfilter_can_skip. > + > ssize_t qemu_netfilter_receive(NetFilterState *nf, > NetFilterDirection direction, > NetClientState *sender, > @@ -25,6 +30,10 @@ ssize_t qemu_netfilter_receive(NetFilterState *nf, > int iovcnt, > NetPacketSent *sent_cb) > { > + /* Don't go through the filter if it is disabled */ > + if (qemu_need_skip_netfilter(nf)) { > + return 0; > + } The code is self explained, so the commnet is useless. > if (nf->direction == direction || > nf->direction == NET_FILTER_DIRECTION_ALL) { > return NETFILTER_GET_CLASS(OBJECT(nf))->receive_iov( > @@ -134,8 +143,41 @@ static void netfilter_set_direction(Object *obj, int direction, Error **errp) > nf->direction = direction; > } > > +static char *netfilter_get_status(Object *obj, Error **errp) > +{ > + NetFilterState *nf = NETFILTER(obj); > + > + if (nf->enabled) { > + return g_strdup("enable"); > + } else { > + return g_strdup("disable"); > + } > +} > + > +static void netfilter_set_status(Object *obj, const char *str, Error **errp) > +{ > + NetFilterState *nf = NETFILTER(obj); > + > + if (!strcmp(str, "enable")) { > + nf->enabled = true; > + } else if (!strcmp(str, "disable")) { > + nf->enabled = false; Do we need a filter specific callback here to drain filter's queue? E.g for filter-buffer ,need to release all the packets that has been buffered. > + } else { > + error_setg(errp, "Invalid value for netfilter status, " > + "should be 'enable' or 'disable'"); > + } > +} > + > static void netfilter_init(Object *obj) > { > + NetFilterState *nf = NETFILTER(obj); > + > + /* > + * If not configured with 'status' property, the default status > + * for netfilter will be enabled. > + */ > + nf->enabled = true; The code is clear too, so the comment need to be moved to qemu-options.hx. > + > object_property_add_str(obj, "netdev", > netfilter_get_netdev_id, netfilter_set_netdev_id, > NULL); > @@ -143,6 +185,9 @@ static void netfilter_init(Object *obj) > NetFilterDirection_lookup, > netfilter_get_direction, netfilter_set_direction, > NULL); > + object_property_add_str(obj, "status", > + netfilter_get_status, netfilter_set_status, > + NULL); > } > > static void netfilter_complete(UserCreatable *uc, Error **errp)
Hi Jason, Happy spring festival! ;) On 2016/2/18 11:00, Jason Wang wrote: > > > On 02/06/2016 05:28 PM, zhanghailiang wrote: >> With this property, users can control if this filter is 'enable' >> or 'disable'. The default behavior for filter is enabled. >> >> We will skip the disabled filter when delivering packets in net layer. >> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> Cc: Jason Wang <jasowang@redhat.com> >> Cc: Yang Hongyang <hongyang.yang@easystack.cn> >> --- >> include/net/filter.h | 1 + >> net/filter.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 46 insertions(+) >> >> diff --git a/include/net/filter.h b/include/net/filter.h >> index 5639976..af3c53c 100644 >> --- a/include/net/filter.h >> +++ b/include/net/filter.h >> @@ -55,6 +55,7 @@ struct NetFilterState { >> char *netdev_id; >> NetClientState *netdev; >> NetFilterDirection direction; >> + bool enabled; >> QTAILQ_ENTRY(NetFilterState) next; >> }; >> >> diff --git a/net/filter.c b/net/filter.c >> index d2a514e..5551cf1 100644 >> --- a/net/filter.c >> +++ b/net/filter.c >> @@ -17,6 +17,11 @@ >> #include "qom/object_interfaces.h" >> #include "qemu/iov.h" >> >> +static inline bool qemu_need_skip_netfilter(NetFilterState *nf) >> +{ >> + return nf->enabled ? false : true; >> +} > > Suggest to rename this to qemu_netfilter_can_skip. > OK, i will fix it. >> + >> ssize_t qemu_netfilter_receive(NetFilterState *nf, >> NetFilterDirection direction, >> NetClientState *sender, >> @@ -25,6 +30,10 @@ ssize_t qemu_netfilter_receive(NetFilterState *nf, >> int iovcnt, >> NetPacketSent *sent_cb) >> { >> + /* Don't go through the filter if it is disabled */ >> + if (qemu_need_skip_netfilter(nf)) { >> + return 0; >> + } > > The code is self explained, so the commnet is useless. > OK, i will remove it. >> if (nf->direction == direction || >> nf->direction == NET_FILTER_DIRECTION_ALL) { >> return NETFILTER_GET_CLASS(OBJECT(nf))->receive_iov( >> @@ -134,8 +143,41 @@ static void netfilter_set_direction(Object *obj, int direction, Error **errp) >> nf->direction = direction; >> } >> >> +static char *netfilter_get_status(Object *obj, Error **errp) >> +{ >> + NetFilterState *nf = NETFILTER(obj); >> + >> + if (nf->enabled) { >> + return g_strdup("enable"); >> + } else { >> + return g_strdup("disable"); >> + } >> +} >> + >> +static void netfilter_set_status(Object *obj, const char *str, Error **errp) >> +{ >> + NetFilterState *nf = NETFILTER(obj); >> + >> + if (!strcmp(str, "enable")) { >> + nf->enabled = true; >> + } else if (!strcmp(str, "disable")) { >> + nf->enabled = false; > > Do we need a filter specific callback here to drain filter's queue? E.g > for filter-buffer ,need to release all the packets that has been buffered. > I don't think we need to do that here, we drain the filter's queue explicitly when we try to change the filter's status . Besides, it seems that, we didn't have a callback in filter layer to process the queued packets for different types of filters. >> + } else { >> + error_setg(errp, "Invalid value for netfilter status, " >> + "should be 'enable' or 'disable'"); >> + } >> +} >> + >> static void netfilter_init(Object *obj) >> { >> + NetFilterState *nf = NETFILTER(obj); >> + >> + /* >> + * If not configured with 'status' property, the default status >> + * for netfilter will be enabled. >> + */ >> + nf->enabled = true; > > The code is clear too, so the comment need to be moved to qemu-options.hx. > OK, i will fix it in next version. Thanks, Hailiang >> + >> object_property_add_str(obj, "netdev", >> netfilter_get_netdev_id, netfilter_set_netdev_id, >> NULL); >> @@ -143,6 +185,9 @@ static void netfilter_init(Object *obj) >> NetFilterDirection_lookup, >> netfilter_get_direction, netfilter_set_direction, >> NULL); >> + object_property_add_str(obj, "status", >> + netfilter_get_status, netfilter_set_status, >> + NULL); >> } >> >> static void netfilter_complete(UserCreatable *uc, Error **errp) > > > . >
On 02/18/2016 11:27 AM, Hailiang Zhang wrote: >>> +static void netfilter_set_status(Object *obj, const char *str, >>> Error **errp) >>> +{ >>> + NetFilterState *nf = NETFILTER(obj); >>> + >>> + if (!strcmp(str, "enable")) { >>> + nf->enabled = true; >>> + } else if (!strcmp(str, "disable")) { >>> + nf->enabled = false; >> >> Do we need a filter specific callback here to drain filter's queue? E.g >> for filter-buffer ,need to release all the packets that has been >> buffered. >> > > I don't think we need to do that here, we drain the filter's queue > explicitly > when we try to change the filter's status . I think we don't do this. E.g what happens if we disable a filter buffer? > Besides, it seems that, we didn't have a callback in filter layer to > process > the queued packets for different types of filters. Just need a type specific callback, no?
On 2016/2/23 16:34, Jason Wang wrote: > > > On 02/18/2016 11:27 AM, Hailiang Zhang wrote: >>>> +static void netfilter_set_status(Object *obj, const char *str, >>>> Error **errp) >>>> +{ >>>> + NetFilterState *nf = NETFILTER(obj); >>>> + >>>> + if (!strcmp(str, "enable")) { >>>> + nf->enabled = true; >>>> + } else if (!strcmp(str, "disable")) { >>>> + nf->enabled = false; >>> >>> Do we need a filter specific callback here to drain filter's queue? E.g >>> for filter-buffer ,need to release all the packets that has been >>> buffered. >>> >> >> I don't think we need to do that here, we drain the filter's queue >> explicitly >> when we try to change the filter's status . > > I think we don't do this. E.g what happens if we disable a filter buffer? > Ha, got it, these buffered packets will be buffered until we enable it again, we indeed need to release them while disable it. >> Besides, it seems that, we didn't have a callback in filter layer to >> process >> the queued packets for different types of filters. > > Just need a type specific callback, no? > Yes, we have change the status in filter layer, for different type of filter, they may need to do different things. we need such a callback in NetFilterClass to respond to this. Thanks, Hailaing > . >
diff --git a/include/net/filter.h b/include/net/filter.h index 5639976..af3c53c 100644 --- a/include/net/filter.h +++ b/include/net/filter.h @@ -55,6 +55,7 @@ struct NetFilterState { char *netdev_id; NetClientState *netdev; NetFilterDirection direction; + bool enabled; QTAILQ_ENTRY(NetFilterState) next; }; diff --git a/net/filter.c b/net/filter.c index d2a514e..5551cf1 100644 --- a/net/filter.c +++ b/net/filter.c @@ -17,6 +17,11 @@ #include "qom/object_interfaces.h" #include "qemu/iov.h" +static inline bool qemu_need_skip_netfilter(NetFilterState *nf) +{ + return nf->enabled ? false : true; +} + ssize_t qemu_netfilter_receive(NetFilterState *nf, NetFilterDirection direction, NetClientState *sender, @@ -25,6 +30,10 @@ ssize_t qemu_netfilter_receive(NetFilterState *nf, int iovcnt, NetPacketSent *sent_cb) { + /* Don't go through the filter if it is disabled */ + if (qemu_need_skip_netfilter(nf)) { + return 0; + } if (nf->direction == direction || nf->direction == NET_FILTER_DIRECTION_ALL) { return NETFILTER_GET_CLASS(OBJECT(nf))->receive_iov( @@ -134,8 +143,41 @@ static void netfilter_set_direction(Object *obj, int direction, Error **errp) nf->direction = direction; } +static char *netfilter_get_status(Object *obj, Error **errp) +{ + NetFilterState *nf = NETFILTER(obj); + + if (nf->enabled) { + return g_strdup("enable"); + } else { + return g_strdup("disable"); + } +} + +static void netfilter_set_status(Object *obj, const char *str, Error **errp) +{ + NetFilterState *nf = NETFILTER(obj); + + if (!strcmp(str, "enable")) { + nf->enabled = true; + } else if (!strcmp(str, "disable")) { + nf->enabled = false; + } else { + error_setg(errp, "Invalid value for netfilter status, " + "should be 'enable' or 'disable'"); + } +} + static void netfilter_init(Object *obj) { + NetFilterState *nf = NETFILTER(obj); + + /* + * If not configured with 'status' property, the default status + * for netfilter will be enabled. + */ + nf->enabled = true; + object_property_add_str(obj, "netdev", netfilter_get_netdev_id, netfilter_set_netdev_id, NULL); @@ -143,6 +185,9 @@ static void netfilter_init(Object *obj) NetFilterDirection_lookup, netfilter_get_direction, netfilter_set_direction, NULL); + object_property_add_str(obj, "status", + netfilter_get_status, netfilter_set_status, + NULL); } static void netfilter_complete(UserCreatable *uc, Error **errp)
With this property, users can control if this filter is 'enable' or 'disable'. The default behavior for filter is enabled. We will skip the disabled filter when delivering packets in net layer. Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> Cc: Jason Wang <jasowang@redhat.com> Cc: Yang Hongyang <hongyang.yang@easystack.cn> --- include/net/filter.h | 1 + net/filter.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+)