Re: [Qemu-devel] [PATCH v11 09/12] netfilter: add a netbuffer filter

[Markus Armbruster]

Jason Wang <address@hidden> writes:

> On 09/25/2015 11:07 PM, Markus Armbruster wrote:
>> Yang Hongyang <address@hidden> writes:
>>
>>> On 09/24/2015 05:12 PM, Markus Armbruster wrote:
>>>> Yang Hongyang <address@hidden> writes:
>>>>
>>>>> This filter is to buffer/release packets, this feature can be used
>>>>> when using MicroCheckpointing, or other Remus like VM FT solutions, you
>>>> What's "Remus"?
>
> [...]
>
>>>>
>>>>> +
>>>>> +static void filter_buffer_release_timer(void *opaque)
>>>>> +{
>>>>> +    NetFilterState *nf = opaque;
>>>>> +    FilterBufferState *s = FILTER_BUFFER(nf);
>>>> Style nit: blank line between declarations and statements, please.
>>>>
>>>>> +    filter_buffer_flush(nf);
>>>> Is purging correct here?
>> When the timer expires, we flush as many buffered packets as we can,
>> then throw away the rest.  Why throw them away?  Shouldn't we leave them
>> in the buffer, and only throw away packets when the buffer is full?
>
> May need a "FIXME" or "TODO" here. I think this is for simplicity. We
> could queue the packet if the receiver or next filter could not receive
> packets. But currently there's no way for the next filter or recivier to
> notify us that it can receive more packet. This could be done in the future.

Good enough for me.  Make it FIXME if purging packets is actually wrong,
else TODO.