Re: [Qemu-devel] [Patch] Add -net dump option

[Anthony Liguori <aliguori@us.ibm.com>]

Tristan Gingold wrote:
> Hi,
>
> this patch add a new network pseudo nic that dump traffic to a tcpdump
> compatible file.  I have found this feature useful to debug network protocols
> with the user stack.
>
> Signed-off-by: Tristan Gingold <gingold@adacore.com>
> Tristan.
>
>
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index b2fa19e..73c133a 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -739,6 +739,15 @@ vde_switch -F -sock /tmp/myswitch
>  qemu linux.img -net nic -net vde,sock=/tmp/myswitch
>  @end example
>  
> +@item -net dump[,vlan=@var{n}][,file=@var{file}][,len=@var{len}]
> +Dump network traffic on VLAN @var{n} to file @var{file} (@file{qemu.tcpdump} by
> +default).  At most @var{len} bytes (64 by default) by packet are stored.  The
> +dump can be analyzed with tools such as tcpdump.
> +
> +For better results you'd better to use this option before the @samp{-net user}
>   

For better results, you should use this option before the @samp{-net user}

> +one as the user stack may send replies before the initial packet was propagated
>   

I'd rather you fix this properly.

> +to all receivers.
> +
>  @item -net none
>  Indicate that no network devices should be configured. It is used to
>  override the default configuration (@option{-net nic -net user}) which
>
> diff --git a/net.c b/net.c
> index 8d9b3de..872a17e 100644
> --- a/net.c
> +++ b/net.c
> @@ -1064,6 +1064,75 @@ static int net_vde_init(VLANState *vlan, const char *model,
>  }
>  #endif
>  
> +static VLANClientState *tcpdump_vc;
> +static FILE *tcpdump_file;
> +static int tcpdump_caplen;
>   

Instead of it being globals, you should have a proper state.  That way 
you can use this functionality with multiple VLANs.

> +#define TCPDUMP_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 void tcpdump_receive(void *opaque, const uint8_t *buf, int size)
> +{
> +    struct pcap_sf_pkthdr hdr;
> +    int64_t ts = muldiv64 (qemu_get_clock(vm_clock),1000000, ticks_per_sec);
> +    int caplen = size > tcpdump_caplen ? tcpdump_caplen : size;
> +
> +    hdr.ts.tv_sec = ts / 1000000000LL;
> +    hdr.ts.tv_usec = ts % 1000000;
> +    hdr.caplen = caplen;
> +    hdr.len = size;
> +    fwrite(&hdr, sizeof(hdr), 1, tcpdump_file);
> +    fwrite(buf, caplen, 1, tcpdump_file);
>   

You should have some error checking here.  Are tcpdump files always 
native endian?

Regards,

Anthony Liguori