From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=51861 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P1MYN-0005Uj-Kl for qemu-devel@nongnu.org; Thu, 30 Sep 2010 13:04:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1P1MYJ-0001XB-6Z for qemu-devel@nongnu.org; Thu, 30 Sep 2010 13:04:19 -0400 Received: from mail-ew0-f45.google.com ([209.85.215.45]:44707) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1P1MYJ-0001Wv-2A for qemu-devel@nongnu.org; Thu, 30 Sep 2010 13:04:15 -0400 Received: by ewy27 with SMTP id 27so1106573ewy.4 for ; Thu, 30 Sep 2010 10:04:14 -0700 (PDT) Date: Thu, 30 Sep 2010 19:04:10 +0200 From: "Edgar E. Iglesias" Subject: Re: [Qemu-devel] [PATCH] eepro100: Add support for multiple individual addresses (multiple IA) Message-ID: <20100930170410.GA10823@laped.lan> References: <4CA38D83.9040106@mail.berlios.de> <1285790395-25460-1-git-send-email-weil@mail.berlios.de> <20100929203010.GA7472@laped.lan> <4CA4BE9E.6020401@mail.berlios.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4CA4BE9E.6020401@mail.berlios.de> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: QEMU Developers , "Michael S. Tsirkin" On Thu, Sep 30, 2010 at 06:45:18PM +0200, Stefan Weil wrote: > Am 29.09.2010 22:30, schrieb Edgar E. Iglesias: > > On Wed, Sep 29, 2010 at 09:59:55PM +0200, Stefan Weil wrote: > > > >> I reviewed the latest sources of Linux, FreeBSD and NetBSD. > >> They all reset the multiple IA bit (multi_ia in BSD) to zero, > >> but I did not find code which sets this bit to one > >> (like it is done by some routers). > >> > >> Running Windows guests also did not set this bit. > >> > >> Intel's Open Source Software Developer Manual does not > >> give much information on the semantics related to this bit, > >> so I had to guess how it works. The guess was good enough > >> to make the router emulation work. > >> > >> > >> Related changes in this patch: > >> * Update naming and documentation of the internal hash register. > >> It is not limited to multicast, but also used for multiple IA. > >> * Dump complete configuration register when debug traces are enabled. > >> * Debug output when multiple IA bit is set during CmdConfigure. > >> * Debug output when frames are received because multiple IA bit is set, > >> or when they are ignored although it is set. > >> > >> Cc: Michael S. Tsirkin > >> Signed-off-by: Stefan Weil > >> --- > >> hw/eepro100.c | 30 +++++++++++++++++++++--------- > >> 1 files changed, 21 insertions(+), 9 deletions(-) > >> > >> diff --git a/hw/eepro100.c b/hw/eepro100.c > >> index 2b75c8f..5f6dcb6 100644 > >> --- a/hw/eepro100.c > >> +++ b/hw/eepro100.c > >> @@ -219,7 +219,8 @@ typedef enum { > >> > >> typedef struct { > >> PCIDevice dev; > >> - uint8_t mult[8]; /* multicast mask array */ > >> + /* Hash register (multicast mask array, multiple individual addresses). */ > >> + uint8_t mult[8]; > >> > > > > Nitpick: > > It seems to me like if mult is only used internally. If so, why not > > represent the hash filter with an uint64_t? > > > > Then you can replace the current memsets with ->mult = 0 and simplify > > the match logic. > > > > > > > [snip] > > That's correct. Is it possible to change mult in the VMStateDescription > from VMSTATE_BUFFER to VMSTATE_UINT64 without loosing > compatibility? Ah, probably not. My guess is that you'd run into endianess issues. The current code is always little-endian but the savevm/loadvm code would expect big endian. Better to save the impovement for another time when the vmstate description needs to be updated for other reasons.. Cheers