On Fri, Nov 16, 2012 at 2:55 PM, Dmitry Fleytman <dmi...@daynix.com> wrote:
> Signed-off-by: Dmitry Fleytman <dmi...@daynix.com>
> Signed-off-by: Yan Vugenfirer <y...@daynix.com>
> ---
> hw/vmware_utils.h | 24 ++--
> hw/vmxnet_debug.h | 121 +++++++++++++++++++
> hw/vmxnet_utils.c | 219 +++++++++++++++++++++++++++++++++++
> hw/vmxnet_utils.h | 340
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 692 insertions(+), 12 deletions(-)
> create mode 100644 hw/vmxnet_debug.h
> create mode 100644 hw/vmxnet_utils.c
> create mode 100644 hw/vmxnet_utils.h
>
> diff --git a/hw/vmware_utils.h b/hw/vmware_utils.h
> index d88136a..7f449eb 100644
> --- a/hw/vmware_utils.h
> +++ b/hw/vmware_utils.h
> @@ -29,21 +29,21 @@
> *
> */
> static inline void
> -vmw_shmem_read(target_phys_addr_t addr, void *buf, int len)
> +vmw_shmem_read(hwaddr addr, void *buf, int len)
> {
> VMW_SHPRN("SHMEM r: %" PRIx64 ", len: %d to %p", addr, len, buf);
> cpu_physical_memory_read(addr, buf, len);
> }
All changes to this file should be squashed with the previous patch.
> +/* #define VMXNET_DEBUG_CB */
> +#define VMXNET_DEBUG_WARNINGS
> +#define VMXNET_DEBUG_ERRORS
> +/* #define VMXNET_DEBUG_INTERRUPTS */
> +/* #define VMXNET_DEBUG_CONFIG */
> +/* #define VMXNET_DEBUG_RINGS */
> +/* #define VMXNET_DEBUG_PACKETS */
> +/* #define VMXNET_DEBUG_SHMEM_ACCESS */
> +
> +#ifdef VMXNET_DEBUG_SHMEM_ACCESS
> +#define VMW_SHPRN(fmt, ...)
> \
> + do {
> \
> + printf("[%s][SH][%s]: " fmt "\n", VMXNET_DEVICE_NAME, __func__,
> \
> + ## __VA_ARGS__);
> \
> + } while (0)
> +#else
> +#define VMW_SHPRN(fmt, ...) do {} while (0)
> +#endif
Please use QEMU tracing. It eliminates all this boilerplate and
conditional compilation. Tracing can be enabled/disabled at runtime
and works with SystemTap/DTrace. See docs/tracing.txt.
> diff --git a/hw/vmxnet_utils.h b/hw/vmxnet_utils.h
> new file mode 100644
> index 0000000..7fd9a01
> --- /dev/null
> +++ b/hw/vmxnet_utils.h
> @@ -0,0 +1,340 @@
> +/*
> + * QEMU VMWARE VMXNET* paravirtual NICs - network auxiliary code
> + *
> + * Copyright (c) 2012 Ravello Systems LTD (http://ravellosystems.com)
> + *
> + * Developed by Daynix Computing LTD (http://www.daynix.com)
> + *
> + * Authors:
> + * Dmitry Fleytman <dmi...@daynix.com>
> + * Tamir Shomer <tam...@daynix.com>
> + * Yan Vugenfirer <y...@daynix.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef _VMXNET_UTILS_H_
> +#define _VMXNET_UTILS_H_
> +
> +#include <sys/types.h>
> +#include <bswap.h>
> +#include <string.h>
> +#include <hwaddr.h>
> +#include <hw/virtio-net.h>
> +
> +struct eth_header {
> + uint8_t h_dest[ETH_ALEN]; /* destination eth addr */
> + uint8_t h_source[ETH_ALEN]; /* source ether addr */
> + uint16_t h_proto; /* packet type ID field */
> +};
Looks like it's copy-pasted stuff from /usr/include/linux/if_*.h,
/usr/include/netinet/*.h, and friends. If the system-wide headers are
included names will collide for some of the macros at least.
Did you check if the slirp/ definitions can be reused?
I'd rather we import network header definitions once in a generic
place into the source tree. That way vmxnet and other components
don't need to redefine these structs.
