Re: [Qemu-devel] [PATCHv2 5/5] sunhme: switch sunhme_receive() over to use net_crc32_le()

[Philippe Mathieu-Daudé]

On Wed, Dec 6, 2017 at 12:34 AM, Philippe Mathieu-Daudé <address@hidden> wrote:
> Hi Mark,
>
> On 12/05/2017 05:17 AM, Mark Cave-Ayland wrote:
>> Signed-off-by: Mark Cave-Ayland <address@hidden>
>> ---
>>  hw/net/sunhme.c | 25 +------------------------
>>  1 file changed, 1 insertion(+), 24 deletions(-)
>>
>> diff --git a/hw/net/sunhme.c b/hw/net/sunhme.c
>> index b1efa1b88d..df66e2630c 100644
>> --- a/hw/net/sunhme.c
>> +++ b/hw/net/sunhme.c
>> @@ -698,29 +698,6 @@ static inline void sunhme_set_rx_ring_nr(SunHMEState 
>> *s, int i)
>>      s->erxregs[HME_ERXI_RING >> 2] = ring;
>>  }
>>
>> -#define POLYNOMIAL_LE 0xedb88320
>> -static uint32_t sunhme_crc32_le(const uint8_t *p, int len)
>> -{
>> -    uint32_t crc;
>> -    int carry, i, j;
>> -    uint8_t b;
>> -
>> -    crc = 0xffffffff;
>> -    for (i = 0; i < len; i++) {
>> -        b = *p++;
>> -        for (j = 0; j < 8; j++) {
>> -            carry = (crc & 0x1) ^ (b & 0x01);
>> -            crc >>= 1;
>> -            b >>= 1;
>> -            if (carry) {
>> -                crc = crc ^ POLYNOMIAL_LE;
>> -            }
>> -        }
>> -    }
>> -
>> -    return crc;
>> -}
>> -
>>  #define MIN_BUF_SIZE 60
>>
>>  static ssize_t sunhme_receive(NetClientState *nc, const uint8_t *buf,
>> @@ -761,7 +738,7 @@ static ssize_t sunhme_receive(NetClientState *nc, const 
>> uint8_t *buf,
>>              trace_sunhme_rx_filter_bcast_match();
>>          } else if (s->macregs[HME_MACI_RXCFG >> 2] & HME_MAC_RXCFG_HENABLE) 
>> {
>>              /* Didn't match local address, check hash filter */
>> -            int mcast_idx = sunhme_crc32_le(buf, 6) >> 26;
>
> This could be:
>
>                int mcast_idx = compute_mcast_idx_le(buf);
>
> With:
>
> unsigned compute_mcast_idx_le(const uint8_t *ep)
> {
>     return net_crc32(ep, ETH_ALEN) >> 26;

Oops not ETH_ALEN but MAC_ALEN ;) from sizeof(MACAddr)

> }
>
> Anyway:
> Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
>
>> +            int mcast_idx = net_crc32_le(buf, 6) >> 26;
>>              if (!(s->macregs[(HME_MACI_HASHTAB0 >> 2) - (mcast_idx >> 4)] &
>>                      (1 << (mcast_idx & 0xf)))) {
>>                  /* Didn't match hash filter */
>>