Re: [Qemu-devel] [PATCH] eepro100: Add support for multiple individual addresses (multiple IA)

[Stefan Weil <weil@mail.berlios.de>]

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<mst@redhat.com>
>> Signed-off-by: Stefan Weil<weil@mail.berlios.de>
>> ---
>>   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?

I doubt that there will be a large gain in runtime speed by this change.

See also hw/ne2000.c and hw/rtl8139.c - they use similar code.
So any change should be applied to those nics, too.

Regards,
Stefan