Re: [Qemu-devel] [PATCH] eepro100: Fix multicast regression

[Jason Wang]

On 04/11/2012 03:26 AM, Michael S. Tsirkin wrote:
> On Tue, Apr 10, 2012 at 08:48:54PM +0200, Stefan Weil wrote:
> > Commit 7fc8d918b9674c3e9233d6d25da2457345d414a0 removed code from
> > eepro100.c and replaced it by different code: the code in net.c
> > returns bits 31...26, but eepro100 needs bits 7...2.
> >
> > This patch partially reverts 7fc8d918b9674c3e9233d6d25da2457345d414a0.
> > To avoid future problems, I renamed the function and changed the comment.
> >
> > Cc: Michael S. Tsirkin<address@hidden>
> > Signed-off-by: Stefan Weil<address@hidden>
> Cc Jason who wrote this originally.

Yes,  I miss the fact that eepro100 useshh a modified version of 
compute_mcast_idx(). Thanks for catching this.

Acked-by: Jason Wang <address@hidden>
> > ---
> >   hw/eepro100.c |   28 ++++++++++++++++++++++++++--
> >   1 files changed, 26 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/eepro100.c b/hw/eepro100.c
> > index 02e6f7e..6279ae3 100644
> > --- a/hw/eepro100.c
> > +++ b/hw/eepro100.c
> > @@ -322,8 +322,32 @@ static const uint16_t eepro100_mdi_mask[] = {
> >       0xffff, 0xffff, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
> >   };
> >
> > +#define POLYNOMIAL 0x04c11db6
> > +
> >   static E100PCIDeviceInfo *eepro100_get_class(EEPRO100State *s);
> >
> > +/* From FreeBSD (locally modified). */
> > +static unsigned e100_compute_mcast_idx(const uint8_t *ep)
> > +{
> > +    uint32_t crc;
> > +    int carry, i, j;
> > +    uint8_t b;
> > +
> > +    crc = 0xffffffff;
> > +    for (i = 0; i<  6; i++) {
> > +        b = *ep++;
> > +        for (j = 0; j<  8; j++) {
> > +            carry = ((crc&  0x80000000L) ? 1 : 0) ^ (b&  0x01);
> > +            crc<<= 1;
> > +            b>>= 1;
> > +            if (carry) {
> > +                crc = ((crc ^ POLYNOMIAL) | carry);
> > +            }
> > +        }
> > +    }
> > +    return (crc&  BITS(7, 2))>>  2;
> > +}
> > +
> >   /* Read a 16 bit control/status (CSR) register. */
> >   static uint16_t e100_read_reg2(EEPRO100State *s, E100RegisterOffset addr)
> >   {
> > @@ -823,7 +847,7 @@ static void set_multicast_list(EEPRO100State *s)
> >           uint8_t multicast_addr[6];
> >           pci_dma_read(&s->dev, s->cb_address + 10 + i, multicast_addr, 6);
> >           TRACE(OTHER, logout("multicast entry %s\n", nic_dump(multicast_addr, 
> > 6)));
> > -        unsigned mcast_idx = compute_mcast_idx(multicast_addr);
> > +        unsigned mcast_idx = e100_compute_mcast_idx(multicast_addr);
> >           assert(mcast_idx<  64);
> >           s->mult[mcast_idx>>  3] |= (1<<  (mcast_idx&  7));
> >       }
> > @@ -1650,7 +1674,7 @@ static ssize_t nic_receive(VLANClientState *nc, const 
> > uint8_t * buf, size_t size
> >           if (s->configuration[21]&  BIT(3)) {
> >             /* Multicast all bit is set, receive all multicast frames. */
> >           } else {
> > -          unsigned mcast_idx = compute_mcast_idx(buf);
> > +          unsigned mcast_idx = e100_compute_mcast_idx(buf);
> >             assert(mcast_idx<  64);
> >             if (s->mult[mcast_idx>>  3]&  (1<<  (mcast_idx&  7))) {
> >               /* Multicast frame is allowed in hash table. */
> > --
> > 1.7.9