diff mbox series

[PATCHv2,5/5] sunhme: switch sunhme_receive() over to use net_crc32_le()

Message ID 20171205081744.6563-6-mark.cave-ayland@ilande.co.uk
State New
Headers show
Series net: introduce common net_crc32() and net_crc32_le() functions | expand

Commit Message

Mark Cave-Ayland Dec. 5, 2017, 8:17 a.m. UTC 
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/net/sunhme.c | 25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)

Comments

Eric Blake Dec. 5, 2017, 2:34 p.m. UTC | #1 
On 12/05/2017 02:17 AM, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/net/sunhme.c | 25 +------------------------
>  1 file changed, 1 insertion(+), 24 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Philippe Mathieu-Daudé Dec. 6, 2017, 3:34 a.m. UTC | #2 
Hi Mark,

On 12/05/2017 05:17 AM, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  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;
}

Anyway:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +            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 */
>
Philippe Mathieu-Daudé Dec. 6, 2017, 3:40 a.m. UTC | #3 
On Wed, Dec 6, 2017 at 12:34 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Hi Mark,
>
> On 12/05/2017 05:17 AM, Mark Cave-Ayland wrote:
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  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é <f4bug@amsat.org>
>
>> +            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 */
>>
Mark Cave-Ayland Dec. 7, 2017, 5:17 a.m. UTC | #4 
On 06/12/17 03:34, Philippe Mathieu-Daudé wrote:

> Hi Mark,
> 
> On 12/05/2017 05:17 AM, Mark Cave-Ayland wrote:
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>   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;
> }

It looks like Stefan is suggesting in his comments that the inline 
approach is better, so I'll try to keep that style for the moment.

I do like the use of ETH_ALEN though so I'll add that in to the next 
version.

> Anyway:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
>> +            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 */
>>

ATB,

Mark.
diff mbox series

Patch

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;
+            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 */