Re: [Qemu-devel] QEMU network probes

[Stefan Hajnoczi <stefanha at gmail dot com>]

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