QEMU network probes

Please review the probe addition in QEMU for the network module, note
that I added probes for send & receive Ethernet frames for rtl8139 so
far but not other network devices. Also for receive, the probe would
only fire when a frame is successfully received, so any errors are
ignored.


=========================================================================

Signed-off-by: Rayson Ho <rho@redhat.com>

Rayson



On Wed, 2010-11-24 at 20:56 +0000, Stefan Hajnoczi wrote:
> On Wed, Nov 24, 2010 at 7:22 PM, Rayson Ho <rho@redhat.com> wrote:
> > I added a probe point for a conference demo, are you interested in
> > adding it in?
> >  void qemu_send_packet(VLANClientState *vc, const uint8_t *buf, int
> > size)
> >  {
> > +     trace_qemu_send_packet(buf, size);
> >      qemu_send_packet_async(vc, buf, size, NULL);
> >  }
> 
> Cool, I recently talked to Prerna Saxena about improving tracing
> coverage.  The net subsystem is one of the areas where we need to add
> trace events.
> 
> Please include the VLANClientState *vc pointer in the trace so sent
> packets can be correlated to their network clients.
> 
> Want to send a patch to qemu-devel?
> 
> Stefan

On Wed, Feb 9, 2011 at 7:27 PM, Rayson Ho <rho@redhat.com> wrote:
> Please review the probe addition in QEMU for the network module, note
> that I added probes for send & receive Ethernet frames for rtl8139 so
> far but not other network devices. Also for receive, the probe would
> only fire when a frame is successfully received, so any errors are
> ignored.

I don't know the network subsystem well so I hope others can comment on that.

> =========================================================================
>
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index a22530c..df04f59 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -53,6 +53,7 @@
>  #include "net.h"
>  #include "loader.h"
>  #include "sysemu.h"
> +#include "trace.h"
>
>  /* debug RTL8139 card */
>  //#define DEBUG_RTL8139 1
> @@ -1160,6 +1161,8 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
>         rtl8139_update_irq(s);
>     }
>
> +    trace_rtl8139_do_receive(nc, buf, size_, do_interrupt);
> +
>     return size_;
>  }
>

The rtl8139 has a lot of DEBUG_PRINT().  Have you thought about
converting those?

> @@ -1742,6 +1745,8 @@ static uint32_t rtl8139_RxConfig_read(RTL8139State *s)
>
>  static void rtl8139_transfer_frame(RTL8139State *s, const uint8_t *buf, int size, int do_interrupt)
>  {
> +    trace_rtl8139_transfer_frame(s, buf, size, do_interrupt);
> +
>     if (!size)
>     {
>         DEBUG_PRINT(("RTL8139: +++ empty ethernet frame\n"));
> diff --git a/net.c b/net.c
> index 9ba5be2..ba74f74 100644
> --- a/net.c
> +++ b/net.c
> @@ -36,6 +36,7 @@
>  #include "qemu-common.h"
>  #include "qemu_socket.h"
>  #include "hw/qdev.h"
> +#include "trace.h"
>
>  static QTAILQ_HEAD(, VLANState) vlans;
>  static QTAILQ_HEAD(, VLANClientState) non_vlan_clients;
> @@ -559,6 +560,7 @@ ssize_t qemu_send_packet_async(VLANClientState *sender,
>
>  void qemu_send_packet(VLANClientState *vc, const uint8_t *buf, int size)
>  {
> +    trace_qemu_send_packet(vc, buf, size);
>     qemu_send_packet_async(vc, buf, size, NULL);
>  }
>

All qemu_send_*() functions go through
qemu_send_packet_async_with_flags().  Lets put the trace event there
(there's already a printf), and include the sender, queue, buf, size,
and return value.  That way it's possible to trace all net traffic
including non-rtl8139.

> diff --git a/trace-events b/trace-events
> index e6138ea..76a8431 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -254,3 +254,10 @@ disable spice_vmc_write(ssize_t out, int len) "spice wrottn %lu of requested %zd
>  disable spice_vmc_read(int bytes, int len) "spice read %lu of requested %zd"
>  disable spice_vmc_register_interface(void *scd) "spice vmc registered interface %p"
>  disable spice_vmc_unregister_interface(void *scd) "spice vmc unregistered interface %p"
> +
> +# net.c
> +disable qemu_send_packet(void *vc, const uint8_t *buf, int size) "client-state: %p, buf %p size %d"

Please use "vc %p buf %p size %d" without ':' or ','.  Some trace
events have been merged which do not follow the convention but I'd
like to try keeping the output consistent whenever possible.

Stefan