diff mbox

[COLO-Frame,v14,32/40] net/filter: Introduce a helper to add a filter to the netdev

Message ID 1454750932-7556-33-git-send-email-zhang.zhanghailiang@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zhanghailiang Feb. 6, 2016, 9:28 a.m. UTC
We add a new helper function netdev_add_filter(),
this function can help adding a filter object to a netdev.

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 |  7 +++++++
 net/filter.c         | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

Comments

Jason Wang Feb. 18, 2016, 3:19 a.m. UTC | #1
On 02/06/2016 05:28 PM, zhanghailiang wrote:
> We add a new helper function netdev_add_filter(),
> this function can help adding a filter object to a netdev.
>
> 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 |  7 +++++++
>  net/filter.c         | 34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
>
> diff --git a/include/net/filter.h b/include/net/filter.h
> index af3c53c..0159080 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;

I believe we've agreed that, we will remove this flag? And it seems that
it was not used by following patches.

>      bool enabled;
>      QTAILQ_ENTRY(NetFilterState) next;
>  };
> @@ -74,4 +75,10 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
>                                      int iovcnt,
>                                      void *opaque);
>  
> +NetFilterState *netdev_add_filter(const char *netdev_id,
> +                                  const char *filter_type,
> +                                  const char *filter_id,
> +                                  bool enabled,
> +                                  Error **errp);
> +
>  #endif /* QEMU_NET_FILTER_H */
> diff --git a/net/filter.c b/net/filter.c
> index 5551cf1..dbe9399 100644
> --- a/net/filter.c
> +++ b/net/filter.c
> @@ -177,6 +177,7 @@ static void netfilter_init(Object *obj)
>      * for netfilter will be enabled.
>      */
>      nf->enabled = true;
> +    nf->is_default = false;
>  
>      object_property_add_str(obj, "netdev",
>                              netfilter_get_netdev_id, netfilter_set_netdev_id,
> @@ -232,6 +233,39 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
>      QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
>  }
>  
> +NetFilterState *netdev_add_filter(const char *netdev_id,
> +                                  const char *filter_type,
> +                                  const char *filter_id,
> +                                  bool enabled,
> +                                  Error **errp)
> +{
> +    NetClientState *nc = qemu_find_netdev(netdev_id);
> +    Object *filter;
> +    Error *local_err = NULL;
> +
> +    /* FIXME: Not support multiple queues */
> +    if (!nc || nc->queue_index > 1) {
> +        return NULL;
> +    }
> +    /* Not support vhost-net */
> +    if (get_vhost_net(nc)) {
> +        return NULL;
> +    }

We will check those in netfilter_complete(), no?

> +
> +    filter = object_new_with_props(filter_type,
> +                        object_get_objects_root(),
> +                        filter_id,
> +                        &local_err,
> +                        "netdev", netdev_id,
> +                        "status", enabled ? "enable" : "disable",
> +                        NULL);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return NULL;
> +    }
> +    return NETFILTER(filter);
> +}
> +
>  static void netfilter_finalize(Object *obj)
>  {
>      NetFilterState *nf = NETFILTER(obj);
Zhanghailiang Feb. 18, 2016, 3:30 a.m. UTC | #2
On 2016/2/18 11:19, Jason Wang wrote:
>
>
> On 02/06/2016 05:28 PM, zhanghailiang wrote:
>> We add a new helper function netdev_add_filter(),
>> this function can help adding a filter object to a netdev.
>>
>> 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 |  7 +++++++
>>   net/filter.c         | 34 ++++++++++++++++++++++++++++++++++
>>   2 files changed, 41 insertions(+)
>>
>> diff --git a/include/net/filter.h b/include/net/filter.h
>> index af3c53c..0159080 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;
>
> I believe we've agreed that, we will remove this flag? And it seems that
> it was not used by following patches.
>

Oops, i forgot to remove this useless codes. 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);
>>
>> +NetFilterState *netdev_add_filter(const char *netdev_id,
>> +                                  const char *filter_type,
>> +                                  const char *filter_id,
>> +                                  bool enabled,
>> +                                  Error **errp);
>> +
>>   #endif /* QEMU_NET_FILTER_H */
>> diff --git a/net/filter.c b/net/filter.c
>> index 5551cf1..dbe9399 100644
>> --- a/net/filter.c
>> +++ b/net/filter.c
>> @@ -177,6 +177,7 @@ static void netfilter_init(Object *obj)
>>       * for netfilter will be enabled.
>>       */
>>       nf->enabled = true;
>> +    nf->is_default = false;
>>
>>       object_property_add_str(obj, "netdev",
>>                               netfilter_get_netdev_id, netfilter_set_netdev_id,
>> @@ -232,6 +233,39 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
>>       QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
>>   }
>>
>> +NetFilterState *netdev_add_filter(const char *netdev_id,
>> +                                  const char *filter_type,
>> +                                  const char *filter_id,
>> +                                  bool enabled,
>> +                                  Error **errp)
>> +{
>> +    NetClientState *nc = qemu_find_netdev(netdev_id);
>> +    Object *filter;
>> +    Error *local_err = NULL;
>> +
>> +    /* FIXME: Not support multiple queues */
>> +    if (!nc || nc->queue_index > 1) {
>> +        return NULL;
>> +    }
>> +    /* Not support vhost-net */
>> +    if (get_vhost_net(nc)) {
>> +        return NULL;
>> +    }
>
> We will check those in netfilter_complete(), no?
>

Yes, I'd like to check this more early here.

Thanks,
hailiang

>> +
>> +    filter = object_new_with_props(filter_type,
>> +                        object_get_objects_root(),
>> +                        filter_id,
>> +                        &local_err,
>> +                        "netdev", netdev_id,
>> +                        "status", enabled ? "enable" : "disable",
>> +                        NULL);
>> +    if (local_err) {
>> +        error_propagate(errp, local_err);
>> +        return NULL;
>> +    }
>> +    return NETFILTER(filter);
>> +}
>> +
>>   static void netfilter_finalize(Object *obj)
>>   {
>>       NetFilterState *nf = NETFILTER(obj);
>
>
> .
>
Jason Wang Feb. 23, 2016, 8:36 a.m. UTC | #3
On 02/18/2016 11:30 AM, Hailiang Zhang wrote:
> On 2016/2/18 11:19, Jason Wang wrote:
>>
>>
>> On 02/06/2016 05:28 PM, zhanghailiang wrote:
>>> We add a new helper function netdev_add_filter(),
>>> this function can help adding a filter object to a netdev.
>>>
>>> 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 |  7 +++++++
>>>   net/filter.c         | 34 ++++++++++++++++++++++++++++++++++
>>>   2 files changed, 41 insertions(+)
>>>
>>> diff --git a/include/net/filter.h b/include/net/filter.h
>>> index af3c53c..0159080 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;
>>
>> I believe we've agreed that, we will remove this flag? And it seems that
>> it was not used by following patches.
>>
>
> Oops, i forgot to remove this useless codes. 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);
>>>
>>> +NetFilterState *netdev_add_filter(const char *netdev_id,
>>> +                                  const char *filter_type,
>>> +                                  const char *filter_id,
>>> +                                  bool enabled,
>>> +                                  Error **errp);
>>> +
>>>   #endif /* QEMU_NET_FILTER_H */
>>> diff --git a/net/filter.c b/net/filter.c
>>> index 5551cf1..dbe9399 100644
>>> --- a/net/filter.c
>>> +++ b/net/filter.c
>>> @@ -177,6 +177,7 @@ static void netfilter_init(Object *obj)
>>>       * for netfilter will be enabled.
>>>       */
>>>       nf->enabled = true;
>>> +    nf->is_default = false;
>>>
>>>       object_property_add_str(obj, "netdev",
>>>                               netfilter_get_netdev_id,
>>> netfilter_set_netdev_id,
>>> @@ -232,6 +233,39 @@ static void netfilter_complete(UserCreatable
>>> *uc, Error **errp)
>>>       QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
>>>   }
>>>
>>> +NetFilterState *netdev_add_filter(const char *netdev_id,
>>> +                                  const char *filter_type,
>>> +                                  const char *filter_id,
>>> +                                  bool enabled,
>>> +                                  Error **errp)
>>> +{
>>> +    NetClientState *nc = qemu_find_netdev(netdev_id);
>>> +    Object *filter;
>>> +    Error *local_err = NULL;
>>> +
>>> +    /* FIXME: Not support multiple queues */
>>> +    if (!nc || nc->queue_index > 1) {
>>> +        return NULL;
>>> +    }
>>> +    /* Not support vhost-net */
>>> +    if (get_vhost_net(nc)) {
>>> +        return NULL;
>>> +    }
>>
>> We will check those in netfilter_complete(), no?
>>
>
> Yes, I'd like to check this more early here.
>
> Thanks,
> hailiang

But what's the advantages here?
Zhanghailiang Feb. 23, 2016, 11:39 a.m. UTC | #4
On 2016/2/23 16:36, Jason Wang wrote:
>
>
> On 02/18/2016 11:30 AM, Hailiang Zhang wrote:
>> On 2016/2/18 11:19, Jason Wang wrote:
>>>
>>>
>>> On 02/06/2016 05:28 PM, zhanghailiang wrote:
>>>> We add a new helper function netdev_add_filter(),
>>>> this function can help adding a filter object to a netdev.
>>>>
>>>> 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 |  7 +++++++
>>>>    net/filter.c         | 34 ++++++++++++++++++++++++++++++++++
>>>>    2 files changed, 41 insertions(+)
>>>>
>>>> diff --git a/include/net/filter.h b/include/net/filter.h
>>>> index af3c53c..0159080 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;
>>>
>>> I believe we've agreed that, we will remove this flag? And it seems that
>>> it was not used by following patches.
>>>
>>
>> Oops, i forgot to remove this useless codes. 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);
>>>>
>>>> +NetFilterState *netdev_add_filter(const char *netdev_id,
>>>> +                                  const char *filter_type,
>>>> +                                  const char *filter_id,
>>>> +                                  bool enabled,
>>>> +                                  Error **errp);
>>>> +
>>>>    #endif /* QEMU_NET_FILTER_H */
>>>> diff --git a/net/filter.c b/net/filter.c
>>>> index 5551cf1..dbe9399 100644
>>>> --- a/net/filter.c
>>>> +++ b/net/filter.c
>>>> @@ -177,6 +177,7 @@ static void netfilter_init(Object *obj)
>>>>        * for netfilter will be enabled.
>>>>        */
>>>>        nf->enabled = true;
>>>> +    nf->is_default = false;
>>>>
>>>>        object_property_add_str(obj, "netdev",
>>>>                                netfilter_get_netdev_id,
>>>> netfilter_set_netdev_id,
>>>> @@ -232,6 +233,39 @@ static void netfilter_complete(UserCreatable
>>>> *uc, Error **errp)
>>>>        QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
>>>>    }
>>>>
>>>> +NetFilterState *netdev_add_filter(const char *netdev_id,
>>>> +                                  const char *filter_type,
>>>> +                                  const char *filter_id,
>>>> +                                  bool enabled,
>>>> +                                  Error **errp)
>>>> +{
>>>> +    NetClientState *nc = qemu_find_netdev(netdev_id);
>>>> +    Object *filter;
>>>> +    Error *local_err = NULL;
>>>> +
>>>> +    /* FIXME: Not support multiple queues */
>>>> +    if (!nc || nc->queue_index > 1) {
>>>> +        return NULL;
>>>> +    }
>>>> +    /* Not support vhost-net */
>>>> +    if (get_vhost_net(nc)) {
>>>> +        return NULL;
>>>> +    }
>>>
>>> We will check those in netfilter_complete(), no?
>>>
>>
>> Yes, I'd like to check this more early here.
>>
>> Thanks,
>> hailiang
>
> But what's the advantages here?

Nothing special, here we just want to keep silent while users startup qemu
with vhost=on, Or there will be an annoying "Vhost is not supported",
since here we try to add each netdev a default filter.

Thanks,
Hailiang
Zhanghailiang Feb. 23, 2016, 11:50 a.m. UTC | #5
On 2016/2/23 19:39, Hailiang Zhang wrote:
> On 2016/2/23 16:36, Jason Wang wrote:
>>
>>
>> On 02/18/2016 11:30 AM, Hailiang Zhang wrote:
>>> On 2016/2/18 11:19, Jason Wang wrote:
>>>>
>>>>
>>>> On 02/06/2016 05:28 PM, zhanghailiang wrote:
>>>>> We add a new helper function netdev_add_filter(),
>>>>> this function can help adding a filter object to a netdev.
>>>>>
>>>>> 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 |  7 +++++++
>>>>>    net/filter.c         | 34 ++++++++++++++++++++++++++++++++++
>>>>>    2 files changed, 41 insertions(+)
>>>>>
>>>>> diff --git a/include/net/filter.h b/include/net/filter.h
>>>>> index af3c53c..0159080 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;
>>>>
>>>> I believe we've agreed that, we will remove this flag? And it seems that
>>>> it was not used by following patches.
>>>>
>>>
>>> Oops, i forgot to remove this useless codes. 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);
>>>>>
>>>>> +NetFilterState *netdev_add_filter(const char *netdev_id,
>>>>> +                                  const char *filter_type,
>>>>> +                                  const char *filter_id,
>>>>> +                                  bool enabled,
>>>>> +                                  Error **errp);
>>>>> +
>>>>>    #endif /* QEMU_NET_FILTER_H */
>>>>> diff --git a/net/filter.c b/net/filter.c
>>>>> index 5551cf1..dbe9399 100644
>>>>> --- a/net/filter.c
>>>>> +++ b/net/filter.c
>>>>> @@ -177,6 +177,7 @@ static void netfilter_init(Object *obj)
>>>>>        * for netfilter will be enabled.
>>>>>        */
>>>>>        nf->enabled = true;
>>>>> +    nf->is_default = false;
>>>>>
>>>>>        object_property_add_str(obj, "netdev",
>>>>>                                netfilter_get_netdev_id,
>>>>> netfilter_set_netdev_id,
>>>>> @@ -232,6 +233,39 @@ static void netfilter_complete(UserCreatable
>>>>> *uc, Error **errp)
>>>>>        QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
>>>>>    }
>>>>>
>>>>> +NetFilterState *netdev_add_filter(const char *netdev_id,
>>>>> +                                  const char *filter_type,
>>>>> +                                  const char *filter_id,
>>>>> +                                  bool enabled,
>>>>> +                                  Error **errp)
>>>>> +{
>>>>> +    NetClientState *nc = qemu_find_netdev(netdev_id);
>>>>> +    Object *filter;
>>>>> +    Error *local_err = NULL;
>>>>> +
>>>>> +    /* FIXME: Not support multiple queues */
>>>>> +    if (!nc || nc->queue_index > 1) {
>>>>> +        return NULL;
>>>>> +    }
>>>>> +    /* Not support vhost-net */
>>>>> +    if (get_vhost_net(nc)) {
>>>>> +        return NULL;
>>>>> +    }
>>>>
>>>> We will check those in netfilter_complete(), no?
>>>>
>>>
>>> Yes, I'd like to check this more early here.
>>>
>>> Thanks,
>>> hailiang
>>
>> But what's the advantages here?
>
> Nothing special, here we just want to keep silent while users startup qemu
> with vhost=on, Or there will be an annoying "Vhost is not supported",
> since here we try to add each netdev a default filter.
>

Hmm, i made some mistake, we can remove the check here,
we call object_new_with_props(), with argument errp = NULL, there
won't be any complaint about vhost.
I will remove it in next version, thanks.

> Thanks,
> Hailiang
>
diff mbox

Patch

diff --git a/include/net/filter.h b/include/net/filter.h
index af3c53c..0159080 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);
 
+NetFilterState *netdev_add_filter(const char *netdev_id,
+                                  const char *filter_type,
+                                  const char *filter_id,
+                                  bool enabled,
+                                  Error **errp);
+
 #endif /* QEMU_NET_FILTER_H */
diff --git a/net/filter.c b/net/filter.c
index 5551cf1..dbe9399 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -177,6 +177,7 @@  static void netfilter_init(Object *obj)
     * for netfilter will be enabled.
     */
     nf->enabled = true;
+    nf->is_default = false;
 
     object_property_add_str(obj, "netdev",
                             netfilter_get_netdev_id, netfilter_set_netdev_id,
@@ -232,6 +233,39 @@  static void netfilter_complete(UserCreatable *uc, Error **errp)
     QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
 }
 
+NetFilterState *netdev_add_filter(const char *netdev_id,
+                                  const char *filter_type,
+                                  const char *filter_id,
+                                  bool enabled,
+                                  Error **errp)
+{
+    NetClientState *nc = qemu_find_netdev(netdev_id);
+    Object *filter;
+    Error *local_err = NULL;
+
+    /* FIXME: Not support multiple queues */
+    if (!nc || nc->queue_index > 1) {
+        return NULL;
+    }
+    /* Not support vhost-net */
+    if (get_vhost_net(nc)) {
+        return NULL;
+    }
+
+    filter = object_new_with_props(filter_type,
+                        object_get_objects_root(),
+                        filter_id,
+                        &local_err,
+                        "netdev", netdev_id,
+                        "status", enabled ? "enable" : "disable",
+                        NULL);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return NULL;
+    }
+    return NETFILTER(filter);
+}
+
 static void netfilter_finalize(Object *obj)
 {
     NetFilterState *nf = NETFILTER(obj);