[Qemu-devel] [PATCH v3 0/4] Netfilter: Add each netdev a default filter

[Qemu-devel] [PATCH v3 0/4] Netfilter: Add each netdev a default filter

[zhanghailiang]

This series is a prerequisite for COLO, here we add each netdev
a default buffer filter, it is disabled by default, and has
no side effect for delivering packets in net layer.

Note: this series is based on patch
    '[PATCH v2] net/filter: Fix the output information for command 'info network'

v3:
 - Drop patch '[PATCH RFC v2 2/5] vl: Make object_create() public'
 - Use object_new_with_props() instead of object_create() (Daniel)
v2:
 - Drop the patch net/filter: prevent the default filter to be deleted' (Jason)
 - Re-implement netdev_add_filter() by re-using object_object() (Jason)
 - Send patch 'net/filter: Fix the output information for command 'info
   network' as an independent one. (Jason)

zhanghailiang (4):
  net/filter: Add a 'status' property for filter object
  net/filter: Introduce a helper to add a filter to the netdev
  filter-buffer: Accept zero interval
  net/filter: Add a default filter to each netdev

 include/net/filter.h | 12 ++++++++
 net/filter-buffer.c  | 10 -------
 net/filter.c         | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/net.c            | 23 +++++++++++++++
 4 files changed, 114 insertions(+), 10 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v3 1/4] net/filter: Add a 'status' property for filter object

[zhanghailiang]

With this property, users can control if this filter is 'enable'
or 'disable'. The default behavior for filter is enabled.

We will skip the disabled filter when delivering packets in net layer.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
v2:
 - Squash previous patch 3 into this patch (Jason's suggestion)
---
 include/net/filter.h |  1 +
 net/filter.c         | 45 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/include/net/filter.h b/include/net/filter.h
index 5639976..af3c53c 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -55,6 +55,7 @@ struct NetFilterState {
     char *netdev_id;
     NetClientState *netdev;
     NetFilterDirection direction;
+    bool enabled;
     QTAILQ_ENTRY(NetFilterState) next;
 };
 
diff --git a/net/filter.c b/net/filter.c
index 5adcec8..d08a2be 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -16,6 +16,11 @@
 #include "qom/object_interfaces.h"
 #include "qemu/iov.h"
 
+static inline bool qemu_need_skip_netfilter(NetFilterState *nf)
+{
+    return nf->enabled ? false : true;
+}
+
 ssize_t qemu_netfilter_receive(NetFilterState *nf,
                                NetFilterDirection direction,
                                NetClientState *sender,
@@ -24,6 +29,10 @@ ssize_t qemu_netfilter_receive(NetFilterState *nf,
                                int iovcnt,
                                NetPacketSent *sent_cb)
 {
+    /* Don't go through the filter if it is disabled */
+    if (qemu_need_skip_netfilter(nf)) {
+        return 0;
+    }
     if (nf->direction == direction ||
         nf->direction == NET_FILTER_DIRECTION_ALL) {
         return NETFILTER_GET_CLASS(OBJECT(nf))->receive_iov(
@@ -116,8 +125,41 @@ static void netfilter_set_direction(Object *obj, int direction, Error **errp)
     nf->direction = direction;
 }
 
+static char *netfilter_get_status(Object *obj, Error **errp)
+{
+    NetFilterState *nf = NETFILTER(obj);
+
+    if (nf->enabled) {
+        return g_strdup("enable");
+    } else {
+        return g_strdup("disable");
+    }
+}
+
+static void netfilter_set_status(Object *obj, const char *str, Error **errp)
+{
+    NetFilterState *nf = NETFILTER(obj);
+
+    if (!strcmp(str, "enable")) {
+        nf->enabled = true;
+    } else if (!strcmp(str, "disable")) {
+        nf->enabled = false;
+    } else {
+        error_setg(errp, "Invalid value for netfilter status, "
+                         "should be 'enable' or 'disable'");
+    }
+}
+
 static void netfilter_init(Object *obj)
 {
+    NetFilterState *nf = NETFILTER(obj);
+
+    /*
+    * If not configured with 'status' property, the default status
+    * for netfilter will be enabled.
+    */
+    nf->enabled = true;
+
     object_property_add_str(obj, "netdev",
                             netfilter_get_netdev_id, netfilter_set_netdev_id,
                             NULL);
@@ -125,6 +167,9 @@ static void netfilter_init(Object *obj)
                              NetFilterDirection_lookup,
                              netfilter_get_direction, netfilter_set_direction,
                              NULL);
+    object_property_add_str(obj, "status",
+                            netfilter_get_status, netfilter_set_status,
+                            NULL);
 }
 
 static void netfilter_complete(UserCreatable *uc, Error **errp)
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 2/4] net/filter: Introduce a helper to add a filter to the netdev

[zhanghailiang]

We add a new helper function netdev_add_filter(), this function
can help adding a filter object to a netdev.
Besides, we add a is_default member for struct NetFilterState
to indicate whether the filter is default or not.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
v3:
 - Use object_new_with_props() instead of object_create()
  (Daniel's suggestion)
v2:
 - Re-implement netdev_add_filter() by re-using object_create()
  (Jason's suggestion)
---
 include/net/filter.h |  7 +++++++
 net/filter.c         | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/include/net/filter.h b/include/net/filter.h
index af3c53c..661abef 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -55,6 +55,7 @@ struct NetFilterState {
     char *netdev_id;
     NetClientState *netdev;
     NetFilterDirection direction;
+    bool is_default;
     bool enabled;
     QTAILQ_ENTRY(NetFilterState) next;
 };
@@ -74,4 +75,10 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
                                     int iovcnt,
                                     void *opaque);
 
+void netdev_add_filter(const char *netdev_id,
+                       const char *filter_type,
+                       const char *filter_id,
+                       bool is_default,
+                       Error **errp);
+
 #endif /* QEMU_NET_FILTER_H */
diff --git a/net/filter.c b/net/filter.c
index d08a2be..7f9df57 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -214,6 +214,40 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
     QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
 }
 
+void netdev_add_filter(const char *netdev_id,
+                       const char *filter_type,
+                       const char *filter_id,
+                       bool is_default,
+                       Error **errp)
+{
+    NetClientState *nc = qemu_find_netdev(netdev_id);
+    Object *filter;
+    NetFilterState *nf;
+    Error *local_err = NULL;
+
+    /* FIXME: Not support multiple queues */
+    if (!nc || nc->queue_index > 1) {
+        return;
+    }
+    /* Not support vhost-net */
+    if (get_vhost_net(nc)) {
+        return;
+    }
+    filter = object_new_with_props(filter_type,
+                        object_get_objects_root(),
+                        filter_id,
+                        &local_err,
+                        "netdev", netdev_id,
+                        "status", is_default ? "disable" : "enable",
+                        NULL);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    nf = NETFILTER(filter);
+    nf->is_default = is_default;
+}
+
 static void netfilter_finalize(Object *obj)
 {
     NetFilterState *nf = NETFILTER(obj);
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 3/4] filter-buffer: Accept zero interval

[zhanghailiang]

We may want to accept zero interval when VM FT solutions like MC
or COLO use this filter to release packets on demand.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn>
---
 net/filter-buffer.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/net/filter-buffer.c b/net/filter-buffer.c
index 57be149..12e0c87 100644
--- a/net/filter-buffer.c
+++ b/net/filter-buffer.c
@@ -103,16 +103,6 @@ static void filter_buffer_setup(NetFilterState *nf, Error **errp)
 {
     FilterBufferState *s = FILTER_BUFFER(nf);
 
-    /*
-     * We may want to accept zero interval when VM FT solutions like MC
-     * or COLO use this filter to release packets on demand.
-     */
-    if (!s->interval) {
-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "interval",
-                   "a non-zero interval");
-        return;
-    }
-
     s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
     if (s->interval) {
         timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL,
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 4/4] net/filter: Add a default filter to each netdev

[zhanghailiang]

We add each netdev a default buffer filter, and
the default buffer filter is disabled, so it has
no side effect for packets delivering in qemu net layer.

The default buffer filter can be used by COLO or Micro-checkpoint,
The reason we add the default filter is we hope to support
hot add network during COLO state in future.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
v2:
- Add codes that generate id automatically for default filter
  (Jason's suggestion)
- Some other minor fixes.
---
 include/net/filter.h |  4 ++++
 net/net.c            | 23 +++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/include/net/filter.h b/include/net/filter.h
index 661abef..8f5fa31 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -22,6 +22,10 @@
 #define NETFILTER_CLASS(klass) \
     OBJECT_CLASS_CHECK(NetFilterClass, (klass), TYPE_NETFILTER)
 
+#define DEFAULT_FILTER_TYPE "nop"
+
+#define TYPE_FILTER_BUFFER "filter-buffer"
+
 typedef void (FilterSetup) (NetFilterState *nf, Error **errp);
 typedef void (FilterCleanup) (NetFilterState *nf);
 /*
diff --git a/net/net.c b/net/net.c
index a49af48..998e90a 100644
--- a/net/net.c
+++ b/net/net.c
@@ -77,6 +77,12 @@ const char *host_net_devices[] = {
 
 int default_net = 1;
 
+/*
+ * TODO: Export this with an option for users to control
+ * this with comand line ?
+ */
+char default_netfilter_type[16] = TYPE_FILTER_BUFFER;
+
 /***********************************************************/
 /* network device redirectors */
 
@@ -1029,6 +1035,23 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
         }
         return -1;
     }
+
+    if (is_netdev) {
+        const Netdev *netdev = object;
+        char default_name[128];
+
+        snprintf(default_name, sizeof(default_name),
+                "%s%s", netdev->id, DEFAULT_FILTER_TYPE);
+        /*
+        * Here we add each netdev a default filter,
+        * it will disabled by default, Users can enable it when necessary.
+        */
+        netdev_add_filter(netdev->id,
+                          default_netfilter_type,
+                          default_name,
+                          true,
+                          errp);
+    }
     return 0;
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 0/4] Netfilter: Add each netdev a default filter

[Yang Hongyang]

If we have to add a default filter, then I have a suggestion only
for this series:
1. Add a nop filter. filter-nop.c
2. Add a "default-filter=xxx" property to -netdev, if not specified,
    default to nop.

On 02/01/2016 08:01 PM, zhanghailiang wrote:
> This series is a prerequisite for COLO, here we add each netdev
> a default buffer filter, it is disabled by default, and has
> no side effect for delivering packets in net layer.
>
> Note: this series is based on patch
>      '[PATCH v2] net/filter: Fix the output information for command 'info network'
>
> v3:
>   - Drop patch '[PATCH RFC v2 2/5] vl: Make object_create() public'
>   - Use object_new_with_props() instead of object_create() (Daniel)
> v2:
>   - Drop the patch net/filter: prevent the default filter to be deleted' (Jason)
>   - Re-implement netdev_add_filter() by re-using object_object() (Jason)
>   - Send patch 'net/filter: Fix the output information for command 'info
>     network' as an independent one. (Jason)
>
> zhanghailiang (4):
>    net/filter: Add a 'status' property for filter object
>    net/filter: Introduce a helper to add a filter to the netdev
>    filter-buffer: Accept zero interval
>    net/filter: Add a default filter to each netdev
>
>   include/net/filter.h | 12 ++++++++
>   net/filter-buffer.c  | 10 -------
>   net/filter.c         | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   net/net.c            | 23 +++++++++++++++
>   4 files changed, 114 insertions(+), 10 deletions(-)
>

-- 
Thanks,
Yang

Re: [Qemu-devel] [PATCH v3 2/4] net/filter: Introduce a helper to add a filter to the netdev

[Yang Hongyang]

On 02/01/2016 08:01 PM, zhanghailiang wrote:
> We add a new helper function netdev_add_filter(), this function
> can help adding a filter object to a netdev.
> Besides, we add a is_default member for struct NetFilterState
> to indicate whether the filter is default or not.
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
> v3:
>   - Use object_new_with_props() instead of object_create()
>    (Daniel's suggestion)
> v2:
>   - Re-implement netdev_add_filter() by re-using object_create()
>    (Jason's suggestion)
> ---
>   include/net/filter.h |  7 +++++++
>   net/filter.c         | 34 ++++++++++++++++++++++++++++++++++
>   2 files changed, 41 insertions(+)
>
> diff --git a/include/net/filter.h b/include/net/filter.h
> index af3c53c..661abef 100644
> --- a/include/net/filter.h
> +++ b/include/net/filter.h
> @@ -55,6 +55,7 @@ struct NetFilterState {
>       char *netdev_id;
>       NetClientState *netdev;
>       NetFilterDirection direction;
> +    bool is_default;

You added a member, but seems did not initialize it. although the
default behavior of a struct init is 0, a explicit is_defuault=false
is better IMO.

>       bool enabled;
>       QTAILQ_ENTRY(NetFilterState) next;
>   };
> @@ -74,4 +75,10 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
>                                       int iovcnt,
>                                       void *opaque);
>
> +void netdev_add_filter(const char *netdev_id,
> +                       const char *filter_type,
> +                       const char *filter_id,
> +                       bool is_default,
> +                       Error **errp);
> +
>   #endif /* QEMU_NET_FILTER_H */
> diff --git a/net/filter.c b/net/filter.c
> index d08a2be..7f9df57 100644
> --- a/net/filter.c
> +++ b/net/filter.c
> @@ -214,6 +214,40 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
>       QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
>   }
>
> +void netdev_add_filter(const char *netdev_id,
> +                       const char *filter_type,
> +                       const char *filter_id,
> +                       bool is_default,
> +                       Error **errp)

We could assume all calls to this API is adding default filter.
so is_default is useness.

> +{
> +    NetClientState *nc = qemu_find_netdev(netdev_id);
> +    Object *filter;
> +    NetFilterState *nf;
> +    Error *local_err = NULL;
> +
> +    /* FIXME: Not support multiple queues */
> +    if (!nc || nc->queue_index > 1) {
> +        return;
> +    }
> +    /* Not support vhost-net */
> +    if (get_vhost_net(nc)) {
> +        return;
> +    }
> +    filter = object_new_with_props(filter_type,
> +                        object_get_objects_root(),
> +                        filter_id,
> +                        &local_err,
> +                        "netdev", netdev_id,
> +                        "status", is_default ? "disable" : "enable",
> +                        NULL);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +    nf = NETFILTER(filter);
> +    nf->is_default = is_default;
> +}
> +
>   static void netfilter_finalize(Object *obj)
>   {
>       NetFilterState *nf = NETFILTER(obj);
>

-- 
Thanks,
Yang

Re: [Qemu-devel] [PATCH v3 4/4] net/filter: Add a default filter to each netdev

[Yang Hongyang]

On 02/01/2016 08:01 PM, zhanghailiang wrote:
> We add each netdev a default buffer filter, and
> the default buffer filter is disabled, so it has
> no side effect for packets delivering in qemu net layer.
>
> The default buffer filter can be used by COLO or Micro-checkpoint,
> The reason we add the default filter is we hope to support
> hot add network during COLO state in future.
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
> v2:
> - Add codes that generate id automatically for default filter
>    (Jason's suggestion)
> - Some other minor fixes.
> ---
>   include/net/filter.h |  4 ++++
>   net/net.c            | 23 +++++++++++++++++++++++
>   2 files changed, 27 insertions(+)
>
> diff --git a/include/net/filter.h b/include/net/filter.h
> index 661abef..8f5fa31 100644
> --- a/include/net/filter.h
> +++ b/include/net/filter.h
> @@ -22,6 +22,10 @@
>   #define NETFILTER_CLASS(klass) \
>       OBJECT_CLASS_CHECK(NetFilterClass, (klass), TYPE_NETFILTER)
>
> +#define DEFAULT_FILTER_TYPE "nop"
> +
> +#define TYPE_FILTER_BUFFER "filter-buffer"
> +
>   typedef void (FilterSetup) (NetFilterState *nf, Error **errp);
>   typedef void (FilterCleanup) (NetFilterState *nf);
>   /*
> diff --git a/net/net.c b/net/net.c
> index a49af48..998e90a 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -77,6 +77,12 @@ const char *host_net_devices[] = {
>
>   int default_net = 1;
>
> +/*
> + * TODO: Export this with an option for users to control
> + * this with comand line ?
> + */
> +char default_netfilter_type[16] = TYPE_FILTER_BUFFER;
> +
>   /***********************************************************/
>   /* network device redirectors */
>
> @@ -1029,6 +1035,23 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>           }
>           return -1;
>       }
> +
> +    if (is_netdev) {

If as I said earlier add a 'defult-filter' property to netdev, we
can use that property here to init default filter.

> +        const Netdev *netdev = object;
> +        char default_name[128];
> +
> +        snprintf(default_name, sizeof(default_name),
> +                "%s%s", netdev->id, DEFAULT_FILTER_TYPE);

name is "nop"

> +        /*
> +        * Here we add each netdev a default filter,
> +        * it will disabled by default, Users can enable it when necessary.
> +        */
> +        netdev_add_filter(netdev->id,
> +                          default_netfilter_type,
> +                          default_name,

type is filter-buffer, but name is "nop"?

> +                          true,
> +                          errp);
> +    }
>       return 0;
>   }
>
>

-- 
Thanks,
Yang

Re: [Qemu-devel] [PATCH v3 0/4] Netfilter: Add each netdev a default filter

[Hailiang Zhang]

Hi Hongyang,

On 2016/2/2 11:11, Yang Hongyang wrote:
> If we have to add a default filter, then I have a suggestion only
> for this series:
> 1. Add a nop filter. filter-nop.c

I'm not quite sure if we really need to add a new type 'nop' filter,
because it is only a special case for buffer-filter, where
the process of releasing packets is not controlled by the timer.
(Maybe we can use this to distinguish if the buffer-filter is default or not.
If a buffer-filter is not attached with a timer, then it could be considered as
the default filter)

> 2. Add a "default-filter=xxx" property to -netdev, if not specified,
>     default to nop.
>

Hmm, yes, Jason also suggested that, export an option for the netdev
to control the type of default filter. I have added a 'TODO' tag in
patch 4, we will do that in the future.

Thanks,
Hailiang


> On 02/01/2016 08:01 PM, zhanghailiang wrote:
>> This series is a prerequisite for COLO, here we add each netdev
>> a default buffer filter, it is disabled by default, and has
>> no side effect for delivering packets in net layer.
>>
>> Note: this series is based on patch
>>      '[PATCH v2] net/filter: Fix the output information for command 'info network'
>>
>> v3:
>>   - Drop patch '[PATCH RFC v2 2/5] vl: Make object_create() public'
>>   - Use object_new_with_props() instead of object_create() (Daniel)
>> v2:
>>   - Drop the patch net/filter: prevent the default filter to be deleted' (Jason)
>>   - Re-implement netdev_add_filter() by re-using object_object() (Jason)
>>   - Send patch 'net/filter: Fix the output information for command 'info
>>     network' as an independent one. (Jason)
>>
>> zhanghailiang (4):
>>    net/filter: Add a 'status' property for filter object
>>    net/filter: Introduce a helper to add a filter to the netdev
>>    filter-buffer: Accept zero interval
>>    net/filter: Add a default filter to each netdev
>>
>>   include/net/filter.h | 12 ++++++++
>>   net/filter-buffer.c  | 10 -------
>>   net/filter.c         | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   net/net.c            | 23 +++++++++++++++
>>   4 files changed, 114 insertions(+), 10 deletions(-)
>>
>

Re: [Qemu-devel] [PATCH v3 2/4] net/filter: Introduce a helper to add a filter to the netdev

[Hailiang Zhang]

On 2016/2/2 11:14, Yang Hongyang wrote:
>
>
> On 02/01/2016 08:01 PM, zhanghailiang wrote:
>> We add a new helper function netdev_add_filter(), this function
>> can help adding a filter object to a netdev.
>> Besides, we add a is_default member for struct NetFilterState
>> to indicate whether the filter is default or not.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>> v3:
>>   - Use object_new_with_props() instead of object_create()
>>    (Daniel's suggestion)
>> v2:
>>   - Re-implement netdev_add_filter() by re-using object_create()
>>    (Jason's suggestion)
>> ---
>>   include/net/filter.h |  7 +++++++
>>   net/filter.c         | 34 ++++++++++++++++++++++++++++++++++
>>   2 files changed, 41 insertions(+)
>>
>> diff --git a/include/net/filter.h b/include/net/filter.h
>> index af3c53c..661abef 100644
>> --- a/include/net/filter.h
>> +++ b/include/net/filter.h
>> @@ -55,6 +55,7 @@ struct NetFilterState {
>>       char *netdev_id;
>>       NetClientState *netdev;
>>       NetFilterDirection direction;
>> +    bool is_default;
>
> You added a member, but seems did not initialize it. although the
> default behavior of a struct init is 0, a explicit is_defuault=false
> is better IMO.
>

OK, i will fix it in next version.

>>       bool enabled;
>>       QTAILQ_ENTRY(NetFilterState) next;
>>   };
>> @@ -74,4 +75,10 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
>>                                       int iovcnt,
>>                                       void *opaque);
>>
>> +void netdev_add_filter(const char *netdev_id,
>> +                       const char *filter_type,
>> +                       const char *filter_id,
>> +                       bool is_default,
>> +                       Error **errp);
>> +
>>   #endif /* QEMU_NET_FILTER_H */
>> diff --git a/net/filter.c b/net/filter.c
>> index d08a2be..7f9df57 100644
>> --- a/net/filter.c
>> +++ b/net/filter.c
>> @@ -214,6 +214,40 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
>>       QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
>>   }
>>
>> +void netdev_add_filter(const char *netdev_id,
>> +                       const char *filter_type,
>> +                       const char *filter_id,
>> +                       bool is_default,
>> +                       Error **errp)
>
> We could assume all calls to this API is adding default filter.
> so is_default is useness.
>

Make sense, i will rename this API to netdev_add_default_filter()
and drop the is_default parameter.

Thanks,
Hailiang

>> +{
>> +    NetClientState *nc = qemu_find_netdev(netdev_id);
>> +    Object *filter;
>> +    NetFilterState *nf;
>> +    Error *local_err = NULL;
>> +
>> +    /* FIXME: Not support multiple queues */
>> +    if (!nc || nc->queue_index > 1) {
>> +        return;
>> +    }
>> +    /* Not support vhost-net */
>> +    if (get_vhost_net(nc)) {
>> +        return;
>> +    }
>> +    filter = object_new_with_props(filter_type,
>> +                        object_get_objects_root(),
>> +                        filter_id,
>> +                        &local_err,
>> +                        "netdev", netdev_id,
>> +                        "status", is_default ? "disable" : "enable",
>> +                        NULL);
>> +    if (local_err) {
>> +        error_propagate(errp, local_err);
>> +        return;
>> +    }
>> +    nf = NETFILTER(filter);
>> +    nf->is_default = is_default;
>> +}
>> +
>>   static void netfilter_finalize(Object *obj)
>>   {
>>       NetFilterState *nf = NETFILTER(obj);
>>
>

Re: [Qemu-devel] [PATCH v3 4/4] net/filter: Add a default filter to each netdev

[Hailiang Zhang]

On 2016/2/2 11:16, Yang Hongyang wrote:
>
>
> On 02/01/2016 08:01 PM, zhanghailiang wrote:
>> We add each netdev a default buffer filter, and
>> the default buffer filter is disabled, so it has
>> no side effect for packets delivering in qemu net layer.
>>
>> The default buffer filter can be used by COLO or Micro-checkpoint,
>> The reason we add the default filter is we hope to support
>> hot add network during COLO state in future.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>> v2:
>> - Add codes that generate id automatically for default filter
>>    (Jason's suggestion)
>> - Some other minor fixes.
>> ---
>>   include/net/filter.h |  4 ++++
>>   net/net.c            | 23 +++++++++++++++++++++++
>>   2 files changed, 27 insertions(+)
>>
>> diff --git a/include/net/filter.h b/include/net/filter.h
>> index 661abef..8f5fa31 100644
>> --- a/include/net/filter.h
>> +++ b/include/net/filter.h
>> @@ -22,6 +22,10 @@
>>   #define NETFILTER_CLASS(klass) \
>>       OBJECT_CLASS_CHECK(NetFilterClass, (klass), TYPE_NETFILTER)
>>
>> +#define DEFAULT_FILTER_TYPE "nop"
>> +
>> +#define TYPE_FILTER_BUFFER "filter-buffer"
>> +
>>   typedef void (FilterSetup) (NetFilterState *nf, Error **errp);
>>   typedef void (FilterCleanup) (NetFilterState *nf);
>>   /*
>> diff --git a/net/net.c b/net/net.c
>> index a49af48..998e90a 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -77,6 +77,12 @@ const char *host_net_devices[] = {
>>
>>   int default_net = 1;
>>
>> +/*
>> + * TODO: Export this with an option for users to control
>> + * this with comand line ?
>> + */
>> +char default_netfilter_type[16] = TYPE_FILTER_BUFFER;
>> +
>>   /***********************************************************/
>>   /* network device redirectors */
>>
>> @@ -1029,6 +1035,23 @@ static int net_client_init1(const void *object, int is_netdev, Error **errp)
>>           }
>>           return -1;
>>       }
>> +
>> +    if (is_netdev) {
>
> If as I said earlier add a 'defult-filter' property to netdev, we
> can use that property here to init default filter.
>
>> +        const Netdev *netdev = object;
>> +        char default_name[128];
>> +
>> +        snprintf(default_name, sizeof(default_name),
>> +                "%s%s", netdev->id, DEFAULT_FILTER_TYPE);
>
> name is "nop"
>

No, we shouldn't use the same name for all default filters,
we need to generate the name automatically,
Or there will be an error: " attempt to add duplicate property 'nop' to object".

Here, i rename DEFAULT_FILTER_TYPE to DEFAULT_FILTER_ID,
it is only an identification for default filter.

Thanks,
Hailiang

>> +        /*
>> +        * Here we add each netdev a default filter,
>> +        * it will disabled by default, Users can enable it when necessary.
>> +        */
>> +        netdev_add_filter(netdev->id,
>> +                          default_netfilter_type,
>> +                          default_name,
>
> type is filter-buffer, but name is "nop"?
>
>> +                          true,
>> +                          errp);
>> +    }
>>       return 0;
>>   }
>>
>>
>