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

[Jason Wang]

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 <address@hidden>
>>>>> ---
>>>>>    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.