qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define


From: Yang Hongyang
Subject: Re: [Qemu-devel] [PATCH v9 05/10] move out net queue structs define
Date: Thu, 3 Sep 2015 00:18:18 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0



On 09/02/2015 09:02 PM, Stefan Hajnoczi wrote:
On Wed, Sep 02, 2015 at 09:49:18AM +0800, Yang Hongyang wrote:
On 09/01/2015 10:43 PM, Stefan Hajnoczi wrote:
On Tue, Sep 01, 2015 at 05:06:18PM +0800, Yang Hongyang wrote:
This will be used by the next patch in this series.

Signed-off-by: Yang Hongyang <address@hidden>
Reviewed-by: Thomas Huth <address@hidden>
---
  include/net/queue.h | 19 +++++++++++++++++++
  net/queue.c         | 19 -------------------
  2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/include/net/queue.h b/include/net/queue.h
index fc02b33..1d65e47 100644
--- a/include/net/queue.h
+++ b/include/net/queue.h
@@ -31,6 +31,25 @@ typedef struct NetQueue NetQueue;

  typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret);

+struct NetPacket {
+    QTAILQ_ENTRY(NetPacket) entry;
+    NetClientState *sender;
+    unsigned flags;
+    int size;
+    NetPacketSent *sent_cb;
+    uint8_t data[0];
+};
+
+struct NetQueue {
+    void *opaque;
+    uint32_t nq_maxlen;
+    uint32_t nq_count;
+
+    QTAILQ_HEAD(packets, NetPacket) packets;
+
+    unsigned delivering:1;
+};
+

Why is it necessary to expose both structs?

In next patch, we add a API to pass the packet to next filter:
ssize_t qemu_netfilter_pass_to_next(NetFilterState *nf, NetPacket *packet);
which will access the internals of NetPacket.

and in buffer filter, we need the NetQueue to Queue packets, and also to
access queue->packets(pass this packets to next filter).

Can you do these things by introducing net queue APIs instead of
exposing the struct?

It's a C anti-pattern to expose structs.  It makes code complex because
now the net queue code no longer contains all the assumptions about
NetQueue/NetPacket fields.  People modifying the code now also need to
go into the net filter code to figure out how this struct works.

Exposing the fields also leads to code duplication since every user will
reimplement similar stuff if they access the fields directly instead of
using a function interface.

I can't think of a interface that no need to access the NetPacket internals
to archive the needs. Except I do not reuse the NetPacket and NetQueue,
implement a Queue myself. But that will be more complex.
So in order to not expose those structs, I shall add lots of public get
functions to use these internals, like:
Qemu_NetQueue_get_xxx
Qemu_NetPacket_get_xxx

I'd be very happy if you could give me a better suggestion on this.

Thanks!


Stefan
.


--
Thanks,
Yang.



reply via email to

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