Re: [Qemu-devel] [PATCH v4 02/11] init/cleanup of netfilter object

[Yang Hongyang <yanghy@cn.fujitsu.com>]

On 08/06/2015 03:43 PM, Jason Wang wrote:
> On 08/06/2015 03:35 PM, Yang Hongyang wrote:
>> On 08/06/2015 03:29 PM, Jason Wang wrote:
>>>
>>>
>>> On 08/06/2015 03:22 PM, Yang Hongyang wrote:
>>>> On 08/06/2015 03:07 PM, Jason Wang wrote:
>>>>>
>>>>>
>>>>> On 08/04/2015 04:30 PM, Yang Hongyang wrote:
>>>>>> QTAILQ_ENTRY global_list but used by filter layer, so that we can
>>>>>> manage all filters together.
>>>>>> QTAILQ_ENTRY next used by netdev, filter belongs to the specific
>>>>>> netdev is
>>>>>> in this queue.
>>>>>> This is mostly the same with init/cleanup of netdev object.
>>>>>>
>>>>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>>>>> ---
>>>>>>     include/net/filter.h    |  39 +++++++++++++
>>>>>>     include/net/net.h       |   1 +
>>>>>>     include/qemu/typedefs.h |   1 +
>>>>>>     net/filter.c            | 147
>>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>     net/net.c               |   1 +
>>>>>>     qapi-schema.json        |  37 ++++++++++++
>>>>>>     6 files changed, 226 insertions(+)
>>>>>>
>>>>>> diff --git a/include/net/filter.h b/include/net/filter.h
>>>>>> index 4242ded..9aafe08 100644
>>>>>> --- a/include/net/filter.h
>>>>>> +++ b/include/net/filter.h
>>>>>> @@ -9,7 +9,46 @@
>>>>>>     #define QEMU_NET_FILTER_H
>>>>>>
>>>>>>     #include "qemu-common.h"
>>>>>> +#include "qemu/typedefs.h"
>>>>>> +
>>>>>> +/* the netfilter chain */
>>>>>> +enum {
>>>>>> +    NET_FILTER_IN,
>>>>>> +    NET_FILTER_OUT,
>>>>>> +    NET_FILTER_ALL,
>>>>>> +};
>>>>>> +
>>>>>> +typedef void (FilterCleanup) (NetFilterState *);
>>>>>> +/*
>>>>>> + * Return:
>>>>>> + *   0: finished handling the packet, we should continue
>>>>>> + *   size: filter stolen this packet, we stop pass this packet
>>>>>> further
>>>>>> + */
>>>>>> +typedef ssize_t (FilterReceiveIOV)(NetFilterState *, NetClientState
>>>>>> *sender,
>>>>>> +                                   unsigned flags, const struct
>>>>>> iovec *, int);
>>>>>
>>>>> Please name all parameters.
>>>>
>>>> Ok, thanks.
>>>>
>>>>>
>>>>>> +
>>>>>> +typedef struct NetFilterInfo {
>>>>>> +    NetFilterOptionsKind type;
>>>>>> +    size_t size;
>>>>>> +    FilterCleanup *cleanup;
>>>>>> +    FilterReceiveIOV *receive_iov;
>>>>>> +} NetFilterInfo;
>>>>>> +
>>>>>> +struct NetFilterState {
>>>>>> +    NetFilterInfo *info;
>>>>>> +    char *model;
>>>>>
>>>>> Looks like model is never used?
>>>>
>>>> It can be used when we want to find filters by model. For example,
>>>> when we need to find all "buffer" filter, and release all buffered
>>>> packets.
>>>
>>> But this is not implemented in this series.
>>
>> true.
>>
>>> And I don't get why you need
>>> a such command. Management should keep track of this.
>>
>> There's a patch in v1 which introduce an API use this, and I dropped that
>> because there's no user currently, but that API I suppose to be used
>> by MC or COLO which can do periodic checkpoint, to release packets.
>> the API is filter_buffer_release_all() IIRC...
>
> I see, let's start with simple case to make review and merging easier.
> You can re-introduce them when it has real users.

So do I need to remove model also?

>
> .
>

-- 
Thanks,
Yang.