[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH COLO-Frame v13 34/39] net/filter-buffer: Add def
From: |
Jason Wang |
Subject: |
Re: [Qemu-devel] [PATCH COLO-Frame v13 34/39] net/filter-buffer: Add default filter-buffer for each netdev |
Date: |
Tue, 19 Jan 2016 11:19:30 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 |
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 <address@hidden>
> Cc: Jason Wang <address@hidden>
> Cc: Yang Hongyang <address@hidden>
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;
> }
>