Re: [Qemu-devel] [PATCH COLO-Frame v14 32/40] net/filter: Introduce a helper to add a filter to the netdev

[Jason Wang <jasowang@redhat.com>]

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?