Re: [Qemu-devel] [PATCH ] lan9118: fix multicast filtering

[Aurelien Jarno]

On Fri, Sep 07, 2012 at 04:04:16PM +0100, Peter Maydell wrote:
> On 7 September 2012 15:56, Aurelien Jarno <address@hidden> wrote:
> > On Thu, Aug 23, 2012 at 05:39:39PM +0200, Aurelien Jarno wrote:
> >> The lan9118 emulation tries to compute the multicast index by calling
> >> directly the crc32() function from zlib, but fails to get the correct
> >> result.
> >>
> >> Use the common compute_mcast_idx() function instead, which gives the
> >> correct result. This fixes IPv6 support.
> >>
> >> Signed-off-by: Aurelien Jarno <address@hidden>
> >> ---
> >>  hw/lan9118.c |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/hw/lan9118.c b/hw/lan9118.c
> >> index ff0a50b..ceaf96f 100644
> >> --- a/hw/lan9118.c
> >> +++ b/hw/lan9118.c
> >> @@ -500,7 +500,7 @@ static int lan9118_filter(lan9118_state *s, const 
> >> uint8_t *addr)
> >>          }
> >>      } else {
> >>          /* Hash matching  */
> >> -        hash = (crc32(~0, addr, 6) >> 26);
> >> +        hash = compute_mcast_idx(addr);
> >>          if (hash & 0x20) {
> >>              return (s->mac_hashh >> (hash & 0x1f)) & 1;
> >>          } else {
> >
> > Ping?
> >
> > For the record the Linux kernel uses the ether_crc() function for
> > smsc911x.c, but also for 8139cp.c, 8139too.c and ethoc.c, which use
> > compute_mcast_idx() on the QEMU side.
> 
> Looks ok to me. I did check the data sheet, which helpfully doesn't
> say exactly what the CRC function is, and also the zlib docs (which
> suggest we should use something that isn't what we were doing here).
> So I guess
> 
> Reviewed-by: Peter Maydell <address@hidden>
> 
> Happy for you to commit directly or I can put it in arm-devs.next
> if you prefer.
> 

Thanks for the review, I have applied it.


-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net