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

[Jason Wang]

On 07/30/2015 05:04 PM, Yang Hongyang wrote:
>
>
> On 07/30/2015 04:40 PM, Jason Wang wrote:
>>
>>
>> On 07/30/2015 02:47 PM, Yang Hongyang wrote:
>>> On 07/30/2015 01:13 PM, Jason Wang wrote:
>>> [...]
>>>>> +
>>>>> +#include "net/filter.h"
>>>>> +#include "net/queue.h"
>>>>> +#include "filters.h"
>>>>> +#include "qemu-common.h"
>>>>> +#include "qemu/error-report.h"
>>>>> +
>>>>> +typedef struct FILTERBUFFERState {
>>>>> +    NetFilterState nf;
>>>>> +    NetClientState dummy; /* used to send buffered packets */
>>>>
>>>> Why need this? Couldn't we just infer this from NetFilterState?
>>>
>>> Because we use existing API qemu_send_packet_async/raw to send
>>> packet, it takes an NetClientState as the first argument sender,
>>> and use sender->peer->incoming_queue as the dest queue, so in order to
>>> make this API work, we need to use this dummy NC and init it's
>>> peer to our dest(which is the network backend)
>>> Another way is to call qemu_net_queue_send(netdev->incoming_queue,...)
>>> directly, we still need a NetClientState *sender param, can not
>>> use NetFilterState.
>>
>> I think this is my meaning. Use NetFilterState->netdev.
>
> Problem is NetFilterState->netdev is our destination, we need a sender...
> if we use this, packet will be sent back to NIC...
>

I see, then NetFilterState->netdev->peer is sender. But I think it's
better to track sender instead of destination in this case. Something
like dummy NC is not elegant.

>>
>>> This dummy NC also been checked in filter_buffer_receive to avoid
>>> buffering
>>> packet been sent by ourself.
>>>
>>
>> I don't get why this is needed. Who is going to queue a packet in dummy
>> NC, consider it was not peered by any others?
>
> There's nothing in the dummy NC except the dummy->peer =
> NetFilterState->netdev
> This dummy NC only used to as a sender param of the existing APIs
> which send
> packets. When a buffered packet been sent, we shouldn't buffer it
> again, we
> cann't use any existing NC (packet->sender or NetFilterState->netdev)
> as the sender because otherwise we can't distinguish if the packet is
> a buffered
> packet sent by ourself. 

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?