[Qemu-devel] [PATCH RFC 0/4] Dumping traffic when using netdev devices

[Qemu-devel] [PATCH RFC 0/4] Dumping traffic when using netdev devices

[Miguel Di Ciurcio Filho]

Hello,

This is a prototype suggestion. I mostly copied and pasted the code from
net/dump.c into net.c and made some adjustments. There is no command line
parsing involved yet, just the internals and small changes in net/tap.c and
net/slirp.c do make the thing work.

In my tests, using tap as backend, e1000 as a guest device and running iperf from
guest to host, the overhead of dumping the traffic caused a loss of around 30%
of performance.

I opened the dumped files in wireshark and they looked fine. When using slirp
all requests were dumped fine too.

Bugs/limitations:
- I have no clue on how to deal with tap+vhost, is it necessary?
- When using virtio-net, I'm not sure how to handle iovec when vnet_hdr=on
- Create a function to add dump to a netdev on the fly, is it necessary?
- Handle cleanups

Miguel Di Ciurcio Filho (4):
  net/dump: Make pcap structures public
  net: Introduce NetClientDump and auxiliary functions
  net/tap: Suggested support for NetClientDump
  net/slirp: Suggested support for NetClientDump

 net.c         |   90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 net.h         |    8 +++++
 net/dump.c    |   21 -------------
 net/dump.h    |   21 +++++++++++++
 net/slirp.c   |    3 ++
 net/tap.c     |    3 ++
 qemu-common.h |    1 +
 7 files changed, 126 insertions(+), 21 deletions(-)

[Qemu-devel] [PATCH RFC 1/4] net/dump: Make pcap structures public

[Miguel Di Ciurcio Filho]

Signed-off-by: Miguel Di Ciurcio Filho <miguel.filho@gmail.com>
---
 net/dump.c |   21 ---------------------
 net/dump.h |   21 +++++++++++++++++++++
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index 6db7ecf..8eebacd 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -34,27 +34,6 @@ typedef struct DumpState {
     int pcap_caplen;
 } DumpState;
 
-#define PCAP_MAGIC 0xa1b2c3d4
-
-struct pcap_file_hdr {
-    uint32_t magic;
-    uint16_t version_major;
-    uint16_t version_minor;
-    int32_t thiszone;
-    uint32_t sigfigs;
-    uint32_t snaplen;
-    uint32_t linktype;
-};
-
-struct pcap_sf_pkthdr {
-    struct {
-        int32_t tv_sec;
-        int32_t tv_usec;
-    } ts;
-    uint32_t caplen;
-    uint32_t len;
-};
-
 static ssize_t dump_receive(VLANClientState *nc, const uint8_t *buf, size_t size)
 {
     DumpState *s = DO_UPCAST(DumpState, nc, nc);
diff --git a/net/dump.h b/net/dump.h
index fdc91ad..e9133da 100644
--- a/net/dump.h
+++ b/net/dump.h
@@ -27,6 +27,27 @@
 #include "net.h"
 #include "qemu-common.h"
 
+#define PCAP_MAGIC 0xa1b2c3d4
+
+struct pcap_file_hdr {
+    uint32_t magic;
+    uint16_t version_major;
+    uint16_t version_minor;
+    int32_t thiszone;
+    uint32_t sigfigs;
+    uint32_t snaplen;
+    uint32_t linktype;
+};
+
+struct pcap_sf_pkthdr {
+    struct {
+        int32_t tv_sec;
+        int32_t tv_usec;
+    } ts;
+    uint32_t caplen;
+    uint32_t len;
+};
+
 int net_init_dump(QemuOpts *opts, Monitor *mon,
                   const char *name, VLANState *vlan);
 
-- 
1.7.1

[Qemu-devel] [PATCH RFC 2/4] net: Introduce NetClientDump and auxiliary functions

[Miguel Di Ciurcio Filho]

Signed-off-by: Miguel Di Ciurcio Filho <miguel.filho@gmail.com>
---
 net.c         |   90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 net.h         |    8 +++++
 qemu-common.h |    1 +
 3 files changed, 99 insertions(+), 0 deletions(-)

diff --git a/net.c b/net.c
index 8ddf872..6f125f8 100644
--- a/net.c
+++ b/net.c
@@ -42,6 +42,36 @@ static QTAILQ_HEAD(, VLANClientState) non_vlan_clients;
 
 int default_net = 1;
 
+static void pcap_dump(NetClientDump *net_client_dump, const uint8_t *buf, size_t size)
+{
+    struct pcap_sf_pkthdr hdr;
+    int64_t ts;
+    int caplen;
+
+    if (!net_client_dump) {
+        return;
+    }
+
+    /* Early return in case of previous error. */
+    if (net_client_dump->fd < 0) {
+        return;
+    }
+
+    ts = muldiv64(qemu_get_clock(vm_clock), 1000000, get_ticks_per_sec());
+    caplen = size > net_client_dump->pcap_caplen ? net_client_dump->pcap_caplen : size;
+
+    hdr.ts.tv_sec = ts / 1000000;
+    hdr.ts.tv_usec = ts % 1000000;
+    hdr.caplen = caplen;
+    hdr.len = size;
+    if (write(net_client_dump->fd, &hdr, sizeof(hdr)) != sizeof(hdr) ||
+        write(net_client_dump->fd, buf, caplen) != caplen) {
+        error_report("Error writing pcap dump, closing descriptor.");
+        close(net_client_dump->fd);
+        net_client_dump->fd = -1;
+    }
+}
+
 /***********************************************************/
 /* network device redirectors */
 
@@ -495,6 +525,7 @@ static ssize_t qemu_send_packet_async_with_flags(VLANClientState *sender,
                                                  NetPacketSent *sent_cb)
 {
     NetQueue *queue;
+    NetClientDump *dump;
 
 #ifdef DEBUG_NET
     printf("qemu_send_packet_async:\n");
@@ -507,6 +538,14 @@ static ssize_t qemu_send_packet_async_with_flags(VLANClientState *sender,
 
     if (sender->peer) {
         queue = sender->peer->send_queue;
+        if (sender->dump || sender->peer->dump) {
+            if (sender->dump) {
+                dump = sender->dump;
+            } else {
+                dump = sender->peer->dump;
+            }
+            pcap_dump(dump, buf, size);
+        }
     } else {
         queue = sender->vlan->send_queue;
     }
@@ -621,6 +660,8 @@ ssize_t qemu_sendv_packet_async(VLANClientState *sender,
                                 NetPacketSent *sent_cb)
 {
     NetQueue *queue;
+    NetClientDump *dump;
+    int i;
 
     if (sender->link_down || (!sender->peer && !sender->vlan)) {
         return calc_iov_length(iov, iovcnt);
@@ -628,6 +669,17 @@ ssize_t qemu_sendv_packet_async(VLANClientState *sender,
 
     if (sender->peer) {
         queue = sender->peer->send_queue;
+        if (sender->dump || sender->peer->dump) {
+            if (sender->dump) {
+                dump = sender->dump;
+            } else {
+                dump = sender->peer->dump;
+            }
+            /* XXX handle vnet_hdr headers or dump will be corrupt */
+            for (i = 0; i < iovcnt; i++) {
+                pcap_dump(dump, (const uint8_t*)iov[i].iov_base, iov[i].iov_len);
+            }
+        }
     } else {
         queue = sender->vlan->send_queue;
     }
@@ -1347,6 +1399,44 @@ static int net_init_netdev(QemuOpts *opts, void *dummy)
     return net_client_init(NULL, opts, 1);
 }
 
+NetClientDump *net_client_create_dump(const char *filename, int len) {
+    NetClientDump *net_client_dump;
+    struct pcap_file_hdr hdr;
+    int fd;
+
+    fd = open(filename, O_CREAT | O_WRONLY | O_BINARY, 0644);
+    if (fd < 0) {
+        error_report("Cannot create dump file: %s", filename);
+        return NULL;
+    }
+
+    if (!len) {
+        len = 65536;
+    }
+
+    hdr.magic = PCAP_MAGIC;
+    hdr.version_major = 2;
+    hdr.version_minor = 4;
+    hdr.thiszone = 0;
+    hdr.sigfigs = 0;
+    hdr.snaplen = len;
+    hdr.linktype = 1;
+
+    if (write(fd, &hdr, sizeof(hdr)) < sizeof(hdr)) {
+        error_report("Error writing dump file error: %s", strerror(errno));
+        close(fd);
+        return NULL;
+    }
+
+    net_client_dump = qemu_malloc(sizeof(NetClientDump));
+    net_client_dump->fd = fd;
+    net_client_dump->pcap_caplen = len;
+    pstrcpy(net_client_dump->filename, sizeof(char[128]), filename);
+
+    return net_client_dump;
+
+}
+
 int net_init_clients(void)
 {
     if (default_net) {
diff --git a/net.h b/net.h
index 518cf9c..9a95db5 100644
--- a/net.h
+++ b/net.h
@@ -55,6 +55,12 @@ typedef struct NetClientInfo {
     NetPoll *poll;
 } NetClientInfo;
 
+struct NetClientDump {
+    int fd;
+    int pcap_caplen;
+    char filename[128];
+};
+
 struct VLANClientState {
     NetClientInfo *info;
     int link_down;
@@ -66,6 +72,7 @@ struct VLANClientState {
     char *name;
     char info_str[256];
     unsigned receive_disabled : 1;
+    NetClientDump *dump;
 };
 
 typedef struct NICState {
@@ -81,6 +88,7 @@ struct VLANState {
     NetQueue *send_queue;
 };
 
+NetClientDump *net_client_create_dump(const char *filename, int len);
 VLANState *qemu_find_vlan(int id, int allocate);
 VLANClientState *qemu_find_netdev(const char *id);
 VLANClientState *qemu_new_net_client(NetClientInfo *info,
diff --git a/qemu-common.h b/qemu-common.h
index 3fb2f0b..7354775 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -212,6 +212,7 @@ typedef struct CharDriverState CharDriverState;
 typedef struct MACAddr MACAddr;
 typedef struct VLANState VLANState;
 typedef struct VLANClientState VLANClientState;
+typedef struct NetClientDump NetClientDump;
 typedef struct i2c_bus i2c_bus;
 typedef struct i2c_slave i2c_slave;
 typedef struct SMBusDevice SMBusDevice;
-- 
1.7.1

[Qemu-devel] [PATCH RFC 3/4] net/tap: Suggested support for NetClientDump

[Miguel Di Ciurcio Filho]

Signed-off-by: Miguel Di Ciurcio Filho <miguel.filho@gmail.com>
---
 net/tap.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index 0147dab..5bd069b 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -442,6 +442,9 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan
         return -1;
     }
 
+    /* XXX: Handle dump and dumplen here */
+    s->nc.dump = net_client_create_dump("/tmp/tap0.dump", 0);
+
     if (tap_set_sndbuf(s->fd, opts) < 0) {
         return -1;
     }
-- 
1.7.1

[Qemu-devel] [PATCH RFC 4/4] net/slirp: Suggested support for NetClientDump

[Miguel Di Ciurcio Filho]

Signed-off-by: Miguel Di Ciurcio Filho <miguel.filho@gmail.com>
---
 net/slirp.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index b41c60a..5823699 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -240,6 +240,9 @@ static int net_slirp_init(VLANState *vlan, const char *model,
 
     nc = qemu_new_net_client(&net_slirp_info, vlan, NULL, model, name);
 
+    /* XXX: Handle dump and dumplen here */
+    nc->dump = net_client_create_dump("/tmp/slirp.pcap", 0);
+
     snprintf(nc->info_str, sizeof(nc->info_str),
              "net=%s, restricted=%c", inet_ntoa(net), restricted ? 'y' : 'n');
 
-- 
1.7.1

[Qemu-devel] Re: [PATCH RFC 0/4] Dumping traffic when using netdev devices

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2193 bytes --]

Miguel Di Ciurcio Filho wrote:
> Hello,
> 
> This is a prototype suggestion. I mostly copied and pasted the code from
> net/dump.c into net.c and made some adjustments. There is no command line
> parsing involved yet, just the internals and small changes in net/tap.c and
> net/slirp.c do make the thing work.
> 
> In my tests, using tap as backend, e1000 as a guest device and running iperf from
> guest to host, the overhead of dumping the traffic caused a loss of around 30%
> of performance.

No surprise, dump writes are synchronous. The day this is actually
hurting someone, we may see asynchronous patches.

> 
> I opened the dumped files in wireshark and they looked fine. When using slirp
> all requests were dumped fine too.
> 
> Bugs/limitations:
> - I have no clue on how to deal with tap+vhost, is it necessary?

I don't think so. Do you can still do "tcpdump -i tap0" in that
scenario, don't you? Then definitely not.

> - When using virtio-net, I'm not sure how to handle iovec when vnet_hdr=on

Let the sending peer report (offset field or callback) where to find the
payload in a frame.

That channel - or a separate one - could also be used to detect if a
peer supports dumping at all (vhost...). Then no peer code need to be
extended with dump management code, all could be moved into net.c

> - Create a function to add dump to a netdev on the fly, is it necessary?

Yes. That way you don't need to shutdown&restart your VM just to enable
or disable dumping - quite important in certain tests.

> - Handle cleanups
> 
> Miguel Di Ciurcio Filho (4):
>   net/dump: Make pcap structures public
>   net: Introduce NetClientDump and auxiliary functions
>   net/tap: Suggested support for NetClientDump
>   net/slirp: Suggested support for NetClientDump
> 
>  net.c         |   90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  net.h         |    8 +++++
>  net/dump.c    |   21 -------------
>  net/dump.h    |   21 +++++++++++++
>  net/slirp.c   |    3 ++
>  net/tap.c     |    3 ++
>  qemu-common.h |    1 +
>  7 files changed, 126 insertions(+), 21 deletions(-)
> 

Thanks for picking this up!

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: [PATCH RFC 0/4] Dumping traffic when using netdev devices

[Miguel Di Ciurcio Filho]

On Fri, Jul 16, 2010 at 3:34 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
>
>> - When using virtio-net, I'm not sure how to handle iovec when vnet_hdr=on
>
> Let the sending peer report (offset field or callback) where to find the
> payload in a frame.
>

Looking further, do you mean what net.c:vc_sendv_compat() does?

> That channel - or a separate one - could also be used to detect if a
> peer supports dumping at all (vhost...). Then no peer code need to be
> extended with dump management code, all could be moved into net.c
>

What do you mean by channel? (sorry, I'm kinda new around here :-)
I don't see how to put everything into net.c. Options parsing/setting
go inside each backend that adds a NetClientDump instance (just a few
lines as seen in my patch on net/tap.c as example), and when packets
come through net.c (qemu_deliver_packet_*()) we get them, right?

Regards,

Miguel

[Qemu-devel] Re: [PATCH RFC 0/4] Dumping traffic when using netdev devices

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1701 bytes --]

Miguel Di Ciurcio Filho wrote:
> On Fri, Jul 16, 2010 at 3:34 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
>>> - When using virtio-net, I'm not sure how to handle iovec when vnet_hdr=on
>> Let the sending peer report (offset field or callback) where to find the
>> payload in a frame.
>>
> 
> Looking further, do you mean what net.c:vc_sendv_compat() does?

No. This is for converting vectored into single-buffer requests in case
the network client supports only the latter interface.

My proposal is to let net.c ask the sending peer to point to the payload
inside the frame, either by picking up some VLANClientState::dump_offset
(the peer would have to set during init) or invoking a new callback
function in NetClientInfo.

> 
>> That channel - or a separate one - could also be used to detect if a
>> peer supports dumping at all (vhost...). Then no peer code need to be
>> extended with dump management code, all could be moved into net.c
>>
> 
> What do you mean by channel? (sorry, I'm kinda new around here :-)

Channel was meant in an abstract way. Think of a flag in VLANClientState
that indicates if dumping is supported by the backend. Should be set
during initialization once the operation mode is decided. Or set
VLANClientState::dump_offset to -1 to indicate "nothing to dump here".

> I don't see how to put everything into net.c. Options parsing/setting
> go inside each backend that adds a NetClientDump instance (just a few
> lines as seen in my patch on net/tap.c as example), and when packets
> come through net.c (qemu_deliver_packet_*()) we get them, right?

What speaks against parsing and processing generic netdev options in net.c?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: [PATCH RFC 0/4] Dumping traffic when using netdev devices

[Anthony Liguori]

On 07/15/2010 03:22 PM, Miguel Di Ciurcio Filho wrote:
> Hello,
>
> This is a prototype suggestion. I mostly copied and pasted the code from
> net/dump.c into net.c and made some adjustments. There is no command line
> parsing involved yet, just the internals and small changes in net/tap.c and
> net/slirp.c do make the thing work.
>
> In my tests, using tap as backend, e1000 as a guest device and running iperf from
> guest to host, the overhead of dumping the traffic caused a loss of around 30%
> of performance.
>
> I opened the dumped files in wireshark and they looked fine. When using slirp
> all requests were dumped fine too.
>    

A less invasive way to do this would be to chain netdev devices.

Basically:

-netdev tap,fd=X,id=foo
-netdev dump,file=foo.pcap,netdev=foo,id=bar
-net nic,model=virtio,netdev=bar

I think this has some clear advantages to this architecturally.  From a 
user perspective, the only loss is that you have to add the dump device 
at startup (you can still enable/disable capture dynamically).

Regards,

Anthony Liguori

> Bugs/limitations:
> - I have no clue on how to deal with tap+vhost, is it necessary?
> - When using virtio-net, I'm not sure how to handle iovec when vnet_hdr=on
> - Create a function to add dump to a netdev on the fly, is it necessary?
> - Handle cleanups
>
> Miguel Di Ciurcio Filho (4):
>    net/dump: Make pcap structures public
>    net: Introduce NetClientDump and auxiliary functions
>    net/tap: Suggested support for NetClientDump
>    net/slirp: Suggested support for NetClientDump
>
>   net.c         |   90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   net.h         |    8 +++++
>   net/dump.c    |   21 -------------
>   net/dump.h    |   21 +++++++++++++
>   net/slirp.c   |    3 ++
>   net/tap.c     |    3 ++
>   qemu-common.h |    1 +
>   7 files changed, 126 insertions(+), 21 deletions(-)
>
>    

Re: [Qemu-devel] Re: [PATCH RFC 0/4] Dumping traffic when using netdev devices

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 07/15/2010 03:22 PM, Miguel Di Ciurcio Filho wrote:
>> Hello,
>>
>> This is a prototype suggestion. I mostly copied and pasted the code from
>> net/dump.c into net.c and made some adjustments. There is no command line
>> parsing involved yet, just the internals and small changes in net/tap.c and
>> net/slirp.c do make the thing work.
>>
>> In my tests, using tap as backend, e1000 as a guest device and running iperf from
>> guest to host, the overhead of dumping the traffic caused a loss of around 30%
>> of performance.
>>
>> I opened the dumped files in wireshark and they looked fine. When using slirp
>> all requests were dumped fine too.
>>    
>
> A less invasive way to do this would be to chain netdev devices.
>
> Basically:
>
> -netdev tap,fd=X,id=foo
> -netdev dump,file=foo.pcap,netdev=foo,id=bar
> -net nic,model=virtio,netdev=bar

Is this really less invasive?  It breaks the simple 1:1 relationship
between NIC and network backend.  All the code dealing with
VLANClientState member peer needs to be touched.  For instance, this is
the code to connect peers, in qemu_new_net_client():

        if (peer) {
            assert(!peer->peer);
            vc->peer = peer;
            peer->peer = vc;
        }

Possibly worth it if we had a number of different things we want to
insert between the end points, but I don't see that right now.

> I think this has some clear advantages to this architecturally.  From
> a user perspective, the only loss is that you have to add the dump
> device at startup (you can still enable/disable capture dynamically).

I don't like this restriction at all.

Re: [Qemu-devel] Re: [PATCH RFC 0/4] Dumping traffic when using netdev devices

[Anthony Liguori]

On 07/16/2010 10:41 AM, Markus Armbruster wrote:
> Anthony Liguori<anthony@codemonkey.ws>  writes:
>
>    
>> On 07/15/2010 03:22 PM, Miguel Di Ciurcio Filho wrote:
>>      
>>> Hello,
>>>
>>> This is a prototype suggestion. I mostly copied and pasted the code from
>>> net/dump.c into net.c and made some adjustments. There is no command line
>>> parsing involved yet, just the internals and small changes in net/tap.c and
>>> net/slirp.c do make the thing work.
>>>
>>> In my tests, using tap as backend, e1000 as a guest device and running iperf from
>>> guest to host, the overhead of dumping the traffic caused a loss of around 30%
>>> of performance.
>>>
>>> I opened the dumped files in wireshark and they looked fine. When using slirp
>>> all requests were dumped fine too.
>>>
>>>        
>> A less invasive way to do this would be to chain netdev devices.
>>
>> Basically:
>>
>> -netdev tap,fd=X,id=foo
>> -netdev dump,file=foo.pcap,netdev=foo,id=bar
>> -net nic,model=virtio,netdev=bar
>>      
> Is this really less invasive?  It breaks the simple 1:1 relationship
> between NIC and network backend.  All the code dealing with
> VLANClientState member peer needs to be touched.  For instance, this is
> the code to connect peers, in qemu_new_net_client():
>
>          if (peer) {
>              assert(!peer->peer);
>              vc->peer = peer;
>              peer->peer = vc;
>          }
>    

The peering code should all disappear.  I thought that's the whole point 
of this exercise?

I think the main advantage is we avoid adding special logic to handle 
dumping.  If we never have a case like this again, then perhaps it 
doesn't matter.

> Possibly worth it if we had a number of different things we want to
> insert between the end points, but I don't see that right now.
>
>    
>> I think this has some clear advantages to this architecturally.  From
>> a user perspective, the only loss is that you have to add the dump
>> device at startup (you can still enable/disable capture dynamically).
>>      
> I don't like this restriction at all.
>    

I don't either but I don't think it's a deal breaker.  I'm really open 
to either approach but I just wanted to make sure this one was considered.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH RFC 0/4] Dumping traffic when using netdev devices

[Miguel Di Ciurcio Filho]

On Fri, Jul 16, 2010 at 1:14 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 07/16/2010 10:41 AM, Markus Armbruster wrote:
>>> A less invasive way to do this would be to chain netdev devices.
>>>
>>> Basically:
>>>
>>> -netdev tap,fd=X,id=foo
>>> -netdev dump,file=foo.pcap,netdev=foo,id=bar
>>> -net nic,model=virtio,netdev=bar
>>>
>>
>> Is this really less invasive?  It breaks the simple 1:1 relationship
>> between NIC and network backend.  All the code dealing with
>> VLANClientState member peer needs to be touched.  For instance, this is
>> the code to connect peers, in qemu_new_net_client():
>>
>>         if (peer) {
>>             assert(!peer->peer);
>>             vc->peer = peer;
>>             peer->peer = vc;
>>         }
>>
>
> The peering code should all disappear.  I thought that's the whole point of
> this exercise?
>

My intent was to take small steps towards dropping the vlan code, and
one strong dependency for that is to have some facility equivalent to
dump.

For now I believe a solution as simple as possible would be better.

Regards,

Miguel

[Qemu-devel] Re: [PATCH RFC 0/4] Dumping traffic when using netdev devices

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1447 bytes --]

Anthony Liguori wrote:
> On 07/15/2010 03:22 PM, Miguel Di Ciurcio Filho wrote:
>> Hello,
>>
>> This is a prototype suggestion. I mostly copied and pasted the code from
>> net/dump.c into net.c and made some adjustments. There is no command line
>> parsing involved yet, just the internals and small changes in
>> net/tap.c and
>> net/slirp.c do make the thing work.
>>
>> In my tests, using tap as backend, e1000 as a guest device and running
>> iperf from
>> guest to host, the overhead of dumping the traffic caused a loss of
>> around 30%
>> of performance.
>>
>> I opened the dumped files in wireshark and they looked fine. When
>> using slirp
>> all requests were dumped fine too.
>>    
> 
> A less invasive way to do this would be to chain netdev devices.
> 
> Basically:
> 
> -netdev tap,fd=X,id=foo
> -netdev dump,file=foo.pcap,netdev=foo,id=bar
> -net nic,model=virtio,netdev=bar
> 
> I think this has some clear advantages to this architecturally.  From a
> user perspective, the only loss is that you have to add the dump device
> at startup (you can still enable/disable capture dynamically).

At least I tend to forget this, and then you already have a VM running
without that chain. And it looks like dynamic reconfiguration of the
backend (netdev_del/add) implies hot-removing/adding the frontend as
well. So this is also no option if the guest does not support that. A
bit unhandy.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]