qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Yang Hongyang
Subject: Re: [Qemu-devel] [PATCH 09/12] netfilter: add a netbuffer filter
Date: Thu, 30 Jul 2015 17:49:27 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0

On 07/30/2015 05:33 PM, Jason Wang wrote:
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?

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.




.


--
Thanks,
Yang.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]