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

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

On 07/30/2015 10:16 PM, Thomas Huth wrote:
> On 30/07/15 12:28, Yang Hongyang wrote:
>> On 07/30/2015 06:14 PM, Jason Wang wrote:
>>>
>>>
>>> On 07/30/2015 05:49 PM, Yang Hongyang wrote:
>>>> On 07/30/2015 05:33 PM, Jason Wang wrote:
> [...]
>>>>> I see, so the reason is you are using qemu_deliver_packet() for both
>>>>> enqueuing packet to filter and delivering packet to destination. How
>>>>> about something like:
>>>>>
>>>>> E.g for qemu_send_packet_async(), move the hook before
>>>>> qemu_send_packet_async_with_flags(). Then flush method can call
>>>>> qemu_send_packet_async_with_flags() without any issue?
>>>>
>>>> I think we can't move the hook earlier, because filters only deal
>>>> with the packets will actually been sent. for example, a dump filter.
>>>> dump packet that probably won't been sent is wrong. calling
>>>> qemu_send_packet_async() or qemu_send_packet_async_with_flags()
>>>> doesn't mean the packet is sent, if the sent_cb is not provided and
>>>> the other peer is not able to receive, the packet will be dropped.
>>>
>>> It depends on how do you define 'actually been sent' and whether or not
>>> we should have such accuracy. Packet could be dropped by various layers.
>>> Reaching receive() or receive_iov() does not mean it can be sent for
>>> sure. For example, lots of nics drop packet in their receive()
>>> implementation.
>>
>> This is true, ok, I'm convinced that we might not need to be this accurate.
>> but Thomas might have different opinion, I saw this description in his
>> dump series:
>>
>> +    /*
>> +     * Log network traffic into a dump file. Note: This should ideally
>> +     * be done after calling the ->receive() function below to make sure
>> +     * that we only log the packets that have really been sent. However,
>> +     * this does not work right with slirp networking since it immediately
>> +     * sends reply packets during the receive() function already, so we
>> +     * would get a wrong order of the packets in the dump file in that
>> case.
>> +     */
>>
>> So Thomas, what do you think of this?
>
> IMHO it should be ok if a dump captures a packet multiple times - it's
> not nice, but it could theoretically also happen on a physical line when
> a packet has to be retransmitted.

Ok, thanks! then I'll move the filter hook earlier.

>
>   Thomas
>
>
> .
>

-- 
Thanks,
Yang.