* [Qemu-devel] [PATCH 1/4] net: move CRC32 calculation from compute_mcast_idx() into its own net_crc32() function
2017-08-18 14:15 [Qemu-devel] [PATCH 0/4] net: introduce common net_crc32() and net_crc32_le() functions Mark Cave-Ayland
@ 2017-08-18 14:15 ` Mark Cave-Ayland
2017-08-18 16:51 ` Philippe Mathieu-Daudé
2017-08-18 14:15 ` [Qemu-devel] [PATCH 2/4] net: introduce net_crc32_le() function Mark Cave-Ayland
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Mark Cave-Ayland @ 2017-08-18 14:15 UTC (permalink / raw)
To: qemu-devel, jasowang
Separate out the standard ethernet CRC32 calculation into a new net_crc32()
function, renaming the constant POLYNOMIAL to POLYNOMIAL_BE to make it clear
that this is a big-endian CRC32 calculation.
Then remove the existing implementation from compute_mcast_idx() and call the
new function in its place.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
include/net/net.h | 3 ++-
net/net.c | 16 +++++++++++-----
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/include/net/net.h b/include/net/net.h
index 1c55a93..586098c 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -227,7 +227,8 @@ NetClientState *net_hub_port_find(int hub_id);
void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
-#define POLYNOMIAL 0x04c11db6
+#define POLYNOMIAL_BE 0x04c11db6
+uint32_t net_crc32(const uint8_t *p, int len);
unsigned compute_mcast_idx(const uint8_t *ep);
#define vmstate_offset_macaddr(_state, _field) \
diff --git a/net/net.c b/net/net.c
index 0e28099..a856907 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1568,25 +1568,31 @@ int net_client_parse(QemuOptsList *opts_list, const char *optarg)
/* From FreeBSD */
/* XXX: optimize */
-unsigned compute_mcast_idx(const uint8_t *ep)
+uint32_t net_crc32(const uint8_t *p, int len)
{
uint32_t crc;
int carry, i, j;
uint8_t b;
crc = 0xffffffff;
- for (i = 0; i < 6; i++) {
- b = *ep++;
+ for (i = 0; i < len; i++) {
+ b = *p++;
for (j = 0; j < 8; j++) {
carry = ((crc & 0x80000000L) ? 1 : 0) ^ (b & 0x01);
crc <<= 1;
b >>= 1;
if (carry) {
- crc = ((crc ^ POLYNOMIAL) | carry);
+ crc = ((crc ^ POLYNOMIAL_BE) | carry);
}
}
}
- return crc >> 26;
+
+ return crc;
+}
+
+unsigned compute_mcast_idx(const uint8_t *ep)
+{
+ return net_crc32(ep, 6) >> 26;
}
QemuOptsList qemu_netdev_opts = {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] net: move CRC32 calculation from compute_mcast_idx() into its own net_crc32() function
2017-08-18 14:15 ` [Qemu-devel] [PATCH 1/4] net: move CRC32 calculation from compute_mcast_idx() into its own net_crc32() function Mark Cave-Ayland
@ 2017-08-18 16:51 ` Philippe Mathieu-Daudé
2017-08-18 17:06 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-08-18 16:51 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, jasowang
On 08/18/2017 11:15 AM, Mark Cave-Ayland wrote:
> Separate out the standard ethernet CRC32 calculation into a new net_crc32()
> function, renaming the constant POLYNOMIAL to POLYNOMIAL_BE to make it clear
> that this is a big-endian CRC32 calculation.
>
> Then remove the existing implementation from compute_mcast_idx() and call the
> new function in its place.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> include/net/net.h | 3 ++-
> net/net.c | 16 +++++++++++-----
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 1c55a93..586098c 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -227,7 +227,8 @@ NetClientState *net_hub_port_find(int hub_id);
>
> void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
>
> -#define POLYNOMIAL 0x04c11db6
> +#define POLYNOMIAL_BE 0x04c11db6
> +uint32_t net_crc32(const uint8_t *p, int len);
> unsigned compute_mcast_idx(const uint8_t *ep);
>
> #define vmstate_offset_macaddr(_state, _field) \
> diff --git a/net/net.c b/net/net.c
> index 0e28099..a856907 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1568,25 +1568,31 @@ int net_client_parse(QemuOptsList *opts_list, const char *optarg)
>
> /* From FreeBSD */
> /* XXX: optimize */
> -unsigned compute_mcast_idx(const uint8_t *ep)
> +uint32_t net_crc32(const uint8_t *p, int len)
imho there is no point in using signed length.
> {
> uint32_t crc;
> int carry, i, j;
> uint8_t b;
>
> crc = 0xffffffff;
> - for (i = 0; i < 6; i++) {
> - b = *ep++;
> + for (i = 0; i < len; i++) {
> + b = *p++;
> for (j = 0; j < 8; j++) {
> carry = ((crc & 0x80000000L) ? 1 : 0) ^ (b & 0x01);
> crc <<= 1;
> b >>= 1;
> if (carry) {
> - crc = ((crc ^ POLYNOMIAL) | carry);
> + crc = ((crc ^ POLYNOMIAL_BE) | carry);
> }
> }
> }
> - return crc >> 26;
> +
> + return crc;
> +}
> +
> +unsigned compute_mcast_idx(const uint8_t *ep)
> +{
> + return net_crc32(ep, 6) >> 26;
while here let's use ETH_ALEN
> }
>
> QemuOptsList qemu_netdev_opts = {
>
with "uint32_t net_crc32(const uint8_t *p, size_t len);":
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] net: move CRC32 calculation from compute_mcast_idx() into its own net_crc32() function
2017-08-18 16:51 ` Philippe Mathieu-Daudé
@ 2017-08-18 17:06 ` Philippe Mathieu-Daudé
2017-08-18 18:11 ` Mark Cave-Ayland
0 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-08-18 17:06 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, jasowang
On 08/18/2017 01:51 PM, Philippe Mathieu-Daudé wrote:
> On 08/18/2017 11:15 AM, Mark Cave-Ayland wrote:
>> Separate out the standard ethernet CRC32 calculation into a new
>> net_crc32()
>> function, renaming the constant POLYNOMIAL to POLYNOMIAL_BE to make it
>> clear
>> that this is a big-endian CRC32 calculation.
>>
>> Then remove the existing implementation from compute_mcast_idx() and
>> call the
>> new function in its place.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> include/net/net.h | 3 ++-
>> net/net.c | 16 +++++++++++-----
>> 2 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/net/net.h b/include/net/net.h
>> index 1c55a93..586098c 100644
>> --- a/include/net/net.h
>> +++ b/include/net/net.h
>> @@ -227,7 +227,8 @@ NetClientState *net_hub_port_find(int hub_id);
>> void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
>> -#define POLYNOMIAL 0x04c11db6
>> +#define POLYNOMIAL_BE 0x04c11db6
>> +uint32_t net_crc32(const uint8_t *p, int len);
>> unsigned compute_mcast_idx(const uint8_t *ep);
>> #define vmstate_offset_macaddr(_state, _field) \
>> diff --git a/net/net.c b/net/net.c
>> index 0e28099..a856907 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -1568,25 +1568,31 @@ int net_client_parse(QemuOptsList *opts_list,
>> const char *optarg)
>> /* From FreeBSD */
>> /* XXX: optimize */
>> -unsigned compute_mcast_idx(const uint8_t *ep)
>> +uint32_t net_crc32(const uint8_t *p, int len)
>
> imho there is no point in using signed length.
>
>> {
>> uint32_t crc;
>> int carry, i, j;
>> uint8_t b;
>> crc = 0xffffffff;
>> - for (i = 0; i < 6; i++) {
>> - b = *ep++;
>> + for (i = 0; i < len; i++) {
>> + b = *p++;
>> for (j = 0; j < 8; j++) {
>> carry = ((crc & 0x80000000L) ? 1 : 0) ^ (b & 0x01);
>> crc <<= 1;
>> b >>= 1;
>> if (carry) {
>> - crc = ((crc ^ POLYNOMIAL) | carry);
>> + crc = ((crc ^ POLYNOMIAL_BE) | carry);
>> }
>> }
>> }
>> - return crc >> 26;
>> +
>> + return crc;
>> +}
>> +
>> +unsigned compute_mcast_idx(const uint8_t *ep)
>> +{
>> + return net_crc32(ep, 6) >> 26;
>
> while here let's use ETH_ALEN
>
reading the next patches, what do you think about:
static inline uint32_t mac_crc32_be(const uint8_t *p)
{
return net_crc32_be(ep, ETH_ALEN);
}
unsigned compute_mcast_idx(const uint8_t *ep)
{
return mac_crc32_be(p) >> 26;
}
>> }
>> QemuOptsList qemu_netdev_opts = {
>>
> with "uint32_t net_crc32(const uint8_t *p, size_t len);":
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] net: move CRC32 calculation from compute_mcast_idx() into its own net_crc32() function
2017-08-18 17:06 ` Philippe Mathieu-Daudé
@ 2017-08-18 18:11 ` Mark Cave-Ayland
0 siblings, 0 replies; 13+ messages in thread
From: Mark Cave-Ayland @ 2017-08-18 18:11 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel, jasowang
On 18/08/17 18:06, Philippe Mathieu-Daudé wrote:
> On 08/18/2017 01:51 PM, Philippe Mathieu-Daudé wrote:
>> On 08/18/2017 11:15 AM, Mark Cave-Ayland wrote:
>>> Separate out the standard ethernet CRC32 calculation into a new
>>> net_crc32()
>>> function, renaming the constant POLYNOMIAL to POLYNOMIAL_BE to make
>>> it clear
>>> that this is a big-endian CRC32 calculation.
>>>
>>> Then remove the existing implementation from compute_mcast_idx() and
>>> call the
>>> new function in its place.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> ---
>>> include/net/net.h | 3 ++-
>>> net/net.c | 16 +++++++++++-----
>>> 2 files changed, 13 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/include/net/net.h b/include/net/net.h
>>> index 1c55a93..586098c 100644
>>> --- a/include/net/net.h
>>> +++ b/include/net/net.h
>>> @@ -227,7 +227,8 @@ NetClientState *net_hub_port_find(int hub_id);
>>> void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
>>> -#define POLYNOMIAL 0x04c11db6
>>> +#define POLYNOMIAL_BE 0x04c11db6
>>> +uint32_t net_crc32(const uint8_t *p, int len);
>>> unsigned compute_mcast_idx(const uint8_t *ep);
>>> #define vmstate_offset_macaddr(_state, _field) \
>>> diff --git a/net/net.c b/net/net.c
>>> index 0e28099..a856907 100644
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>> @@ -1568,25 +1568,31 @@ int net_client_parse(QemuOptsList *opts_list,
>>> const char *optarg)
>>> /* From FreeBSD */
>>> /* XXX: optimize */
>>> -unsigned compute_mcast_idx(const uint8_t *ep)
>>> +uint32_t net_crc32(const uint8_t *p, int len)
>>
>> imho there is no point in using signed length.
>>
>>> {
>>> uint32_t crc;
>>> int carry, i, j;
>>> uint8_t b;
>>> crc = 0xffffffff;
>>> - for (i = 0; i < 6; i++) {
>>> - b = *ep++;
>>> + for (i = 0; i < len; i++) {
>>> + b = *p++;
>>> for (j = 0; j < 8; j++) {
>>> carry = ((crc & 0x80000000L) ? 1 : 0) ^ (b & 0x01);
>>> crc <<= 1;
>>> b >>= 1;
>>> if (carry) {
>>> - crc = ((crc ^ POLYNOMIAL) | carry);
>>> + crc = ((crc ^ POLYNOMIAL_BE) | carry);
>>> }
>>> }
>>> }
>>> - return crc >> 26;
>>> +
>>> + return crc;
>>> +}
>>> +
>>> +unsigned compute_mcast_idx(const uint8_t *ep)
>>> +{
>>> + return net_crc32(ep, 6) >> 26;
>>
>> while here let's use ETH_ALEN
>>
>
> reading the next patches, what do you think about:
>
> static inline uint32_t mac_crc32_be(const uint8_t *p)
> {
> return net_crc32_be(ep, ETH_ALEN);
> }
>
> unsigned compute_mcast_idx(const uint8_t *ep)
> {
> return mac_crc32_be(p) >> 26;
> }
Yes, using ETH_ALEN looks like a good idea - I can do that for the next
revision.
As for the naming of the functions, I've followed the kernel convention
that by default the CRC32 functions are big-endian unless they have a
_le suffix. I don't really feel strongly either way, so I'm happy to go
with whatever naming scheme Jason prefers.
ATB,
Mark.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 2/4] net: introduce net_crc32_le() function
2017-08-18 14:15 [Qemu-devel] [PATCH 0/4] net: introduce common net_crc32() and net_crc32_le() functions Mark Cave-Ayland
2017-08-18 14:15 ` [Qemu-devel] [PATCH 1/4] net: move CRC32 calculation from compute_mcast_idx() into its own net_crc32() function Mark Cave-Ayland
@ 2017-08-18 14:15 ` Mark Cave-Ayland
2017-08-18 16:54 ` Philippe Mathieu-Daudé
2017-08-18 14:15 ` [Qemu-devel] [PATCH 3/4] pcnet: switch lnc_mchash() over to use net_crc32_le() Mark Cave-Ayland
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Mark Cave-Ayland @ 2017-08-18 14:15 UTC (permalink / raw)
To: qemu-devel, jasowang
This provides a standard ethernet CRC32 little-endian implementation.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
include/net/net.h | 2 ++
net/net.c | 22 ++++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/include/net/net.h b/include/net/net.h
index 586098c..4afac1a 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -228,7 +228,9 @@ NetClientState *net_hub_port_find(int hub_id);
void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
#define POLYNOMIAL_BE 0x04c11db6
+#define POLYNOMIAL_LE 0xedb88320
uint32_t net_crc32(const uint8_t *p, int len);
+uint32_t net_crc32_le(const uint8_t *p, int len);
unsigned compute_mcast_idx(const uint8_t *ep);
#define vmstate_offset_macaddr(_state, _field) \
diff --git a/net/net.c b/net/net.c
index a856907..c99e705 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1590,6 +1590,28 @@ uint32_t net_crc32(const uint8_t *p, int len)
return crc;
}
+uint32_t net_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;
+}
+
unsigned compute_mcast_idx(const uint8_t *ep)
{
return net_crc32(ep, 6) >> 26;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] net: introduce net_crc32_le() function
2017-08-18 14:15 ` [Qemu-devel] [PATCH 2/4] net: introduce net_crc32_le() function Mark Cave-Ayland
@ 2017-08-18 16:54 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-08-18 16:54 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, jasowang
On 08/18/2017 11:15 AM, Mark Cave-Ayland wrote:
> This provides a standard ethernet CRC32 little-endian implementation.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> include/net/net.h | 2 ++
> net/net.c | 22 ++++++++++++++++++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 586098c..4afac1a 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -228,7 +228,9 @@ NetClientState *net_hub_port_find(int hub_id);
> void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
>
> #define POLYNOMIAL_BE 0x04c11db6
> +#define POLYNOMIAL_LE 0xedb88320
> uint32_t net_crc32(const uint8_t *p, int len);
> +uint32_t net_crc32_le(const uint8_t *p, int len);
same here, with size_t len:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> unsigned compute_mcast_idx(const uint8_t *ep);
>
> #define vmstate_offset_macaddr(_state, _field) \
> diff --git a/net/net.c b/net/net.c
> index a856907..c99e705 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1590,6 +1590,28 @@ uint32_t net_crc32(const uint8_t *p, int len)
> return crc;
> }
>
> +uint32_t net_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;
> +}
> +
> unsigned compute_mcast_idx(const uint8_t *ep)
> {
> return net_crc32(ep, 6) >> 26;
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 3/4] pcnet: switch lnc_mchash() over to use net_crc32_le()
2017-08-18 14:15 [Qemu-devel] [PATCH 0/4] net: introduce common net_crc32() and net_crc32_le() functions Mark Cave-Ayland
2017-08-18 14:15 ` [Qemu-devel] [PATCH 1/4] net: move CRC32 calculation from compute_mcast_idx() into its own net_crc32() function Mark Cave-Ayland
2017-08-18 14:15 ` [Qemu-devel] [PATCH 2/4] net: introduce net_crc32_le() function Mark Cave-Ayland
@ 2017-08-18 14:15 ` Mark Cave-Ayland
2017-08-18 16:59 ` Philippe Mathieu-Daudé
2017-08-18 14:15 ` [Qemu-devel] [PATCH 4/4] eepro100: switch e100_compute_mcast_idx() over to use net_crc32() Mark Cave-Ayland
2017-08-30 17:42 ` [Qemu-devel] [PATCH 0/4] net: introduce common net_crc32() and net_crc32_le() functions Mark Cave-Ayland
4 siblings, 1 reply; 13+ messages in thread
From: Mark Cave-Ayland @ 2017-08-18 14:15 UTC (permalink / raw)
To: qemu-devel, jasowang
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/net/pcnet.c | 16 +---------------
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index 6544553..c050993 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -522,23 +522,9 @@ static inline void pcnet_rmd_store(PCNetState *s, struct pcnet_RMD *rmd,
be16_to_cpu(hdr->ether_type)); \
} while (0)
-#define MULTICAST_FILTER_LEN 8
-
static inline uint32_t lnc_mchash(const uint8_t *ether_addr)
{
-#define LNC_POLYNOMIAL 0xEDB88320UL
- uint32_t crc = 0xFFFFFFFF;
- int idx, bit;
- uint8_t data;
-
- for (idx = 0; idx < 6; idx++) {
- for (data = *ether_addr++, bit = 0; bit < MULTICAST_FILTER_LEN; bit++) {
- crc = (crc >> 1) ^ (((crc ^ data) & 1) ? LNC_POLYNOMIAL : 0);
- data >>= 1;
- }
- }
- return crc;
-#undef LNC_POLYNOMIAL
+ return net_crc32_le(ether_addr, 6);
}
#define CRC(crc, ch) (crc = (crc >> 8) ^ crctab[(crc ^ (ch)) & 0xff])
--
1.7.10.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] pcnet: switch lnc_mchash() over to use net_crc32_le()
2017-08-18 14:15 ` [Qemu-devel] [PATCH 3/4] pcnet: switch lnc_mchash() over to use net_crc32_le() Mark Cave-Ayland
@ 2017-08-18 16:59 ` Philippe Mathieu-Daudé
2017-08-18 17:10 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-08-18 16:59 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, jasowang
On 08/18/2017 11:15 AM, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/net/pcnet.c | 16 +---------------
> 1 file changed, 1 insertion(+), 15 deletions(-)
>
> diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
> index 6544553..c050993 100644
> --- a/hw/net/pcnet.c
> +++ b/hw/net/pcnet.c
> @@ -522,23 +522,9 @@ static inline void pcnet_rmd_store(PCNetState *s, struct pcnet_RMD *rmd,
> be16_to_cpu(hdr->ether_type)); \
> } while (0)
>
> -#define MULTICAST_FILTER_LEN 8
> -
> static inline uint32_t lnc_mchash(const uint8_t *ether_addr)
> {
> -#define LNC_POLYNOMIAL 0xEDB88320UL
> - uint32_t crc = 0xFFFFFFFF;
> - int idx, bit;
> - uint8_t data;
> -
> - for (idx = 0; idx < 6; idx++) {
> - for (data = *ether_addr++, bit = 0; bit < MULTICAST_FILTER_LEN; bit++) {
> - crc = (crc >> 1) ^ (((crc ^ data) & 1) ? LNC_POLYNOMIAL : 0);
> - data >>= 1;
> - }
> - }
> - return crc;
> -#undef LNC_POLYNOMIAL
> + return net_crc32_le(ether_addr, 6);
using ETH_ALEN:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> }
>
> #define CRC(crc, ch) (crc = (crc >> 8) ^ crctab[(crc ^ (ch)) & 0xff])
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] pcnet: switch lnc_mchash() over to use net_crc32_le()
2017-08-18 16:59 ` Philippe Mathieu-Daudé
@ 2017-08-18 17:10 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-08-18 17:10 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, jasowang
On 08/18/2017 01:59 PM, Philippe Mathieu-Daudé wrote:
> On 08/18/2017 11:15 AM, Mark Cave-Ayland wrote:
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> hw/net/pcnet.c | 16 +---------------
>> 1 file changed, 1 insertion(+), 15 deletions(-)
>>
>> diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
>> index 6544553..c050993 100644
>> --- a/hw/net/pcnet.c
>> +++ b/hw/net/pcnet.c
>> @@ -522,23 +522,9 @@ static inline void pcnet_rmd_store(PCNetState *s,
>> struct pcnet_RMD *rmd,
>> be16_to_cpu(hdr->ether_type)); \
>> } while (0)
>> -#define MULTICAST_FILTER_LEN 8
>> -
>> static inline uint32_t lnc_mchash(const uint8_t *ether_addr)
>> {
>> -#define LNC_POLYNOMIAL 0xEDB88320UL
>> - uint32_t crc = 0xFFFFFFFF;
>> - int idx, bit;
>> - uint8_t data;
>> -
>> - for (idx = 0; idx < 6; idx++) {
>> - for (data = *ether_addr++, bit = 0; bit <
>> MULTICAST_FILTER_LEN; bit++) {
>> - crc = (crc >> 1) ^ (((crc ^ data) & 1) ? LNC_POLYNOMIAL :
>> 0);
>> - data >>= 1;
>> - }
>> - }
>> - return crc;
>> -#undef LNC_POLYNOMIAL
>> + return net_crc32_le(ether_addr, 6);
with previous suggestion from patch 1/4:
return mac_crc32_le(ether_addr);
>
> using ETH_ALEN:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
>> }
>> #define CRC(crc, ch) (crc = (crc >> 8) ^ crctab[(crc ^ (ch)) &
>> 0xff])
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 4/4] eepro100: switch e100_compute_mcast_idx() over to use net_crc32()
2017-08-18 14:15 [Qemu-devel] [PATCH 0/4] net: introduce common net_crc32() and net_crc32_le() functions Mark Cave-Ayland
` (2 preceding siblings ...)
2017-08-18 14:15 ` [Qemu-devel] [PATCH 3/4] pcnet: switch lnc_mchash() over to use net_crc32_le() Mark Cave-Ayland
@ 2017-08-18 14:15 ` Mark Cave-Ayland
2017-08-18 17:10 ` Philippe Mathieu-Daudé
2017-08-30 17:42 ` [Qemu-devel] [PATCH 0/4] net: introduce common net_crc32() and net_crc32_le() functions Mark Cave-Ayland
4 siblings, 1 reply; 13+ messages in thread
From: Mark Cave-Ayland @ 2017-08-18 14:15 UTC (permalink / raw)
To: qemu-devel, jasowang
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/net/eepro100.c | 19 +------------------
1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index 5a4774a..4226572 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -327,26 +327,9 @@ static const uint16_t eepro100_mdi_mask[] = {
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;
+ return (net_crc32(ep, 6) & BITS(7, 2)) >> 2;
}
/* Read a 16 bit control/status (CSR) register. */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] eepro100: switch e100_compute_mcast_idx() over to use net_crc32()
2017-08-18 14:15 ` [Qemu-devel] [PATCH 4/4] eepro100: switch e100_compute_mcast_idx() over to use net_crc32() Mark Cave-Ayland
@ 2017-08-18 17:10 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-08-18 17:10 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel, jasowang
On 08/18/2017 11:15 AM, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/net/eepro100.c | 19 +------------------
> 1 file changed, 1 insertion(+), 18 deletions(-)
>
> diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
> index 5a4774a..4226572 100644
> --- a/hw/net/eepro100.c
> +++ b/hw/net/eepro100.c
> @@ -327,26 +327,9 @@ static const uint16_t eepro100_mdi_mask[] = {
>
> 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;
> + return (net_crc32(ep, 6) & BITS(7, 2)) >> 2;
with previous suggestion from patch 1/4:
return (mac_crc32_be(ep) & BITS(7, 2)) >> 2;
then
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> }
>
> /* Read a 16 bit control/status (CSR) register. */
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 0/4] net: introduce common net_crc32() and net_crc32_le() functions
2017-08-18 14:15 [Qemu-devel] [PATCH 0/4] net: introduce common net_crc32() and net_crc32_le() functions Mark Cave-Ayland
` (3 preceding siblings ...)
2017-08-18 14:15 ` [Qemu-devel] [PATCH 4/4] eepro100: switch e100_compute_mcast_idx() over to use net_crc32() Mark Cave-Ayland
@ 2017-08-30 17:42 ` Mark Cave-Ayland
4 siblings, 0 replies; 13+ messages in thread
From: Mark Cave-Ayland @ 2017-08-30 17:42 UTC (permalink / raw)
To: qemu-devel, jasowang
On 18/08/17 15:15, Mark Cave-Ayland wrote:
> Whilst trying to debug a CRC32 endian issue for NIC multicast hash lookups, it
> struck me that it would make sense to have a common set of standard ethernet
> CRC32 functions (both little and big endian variants) in net.c.
>
> The first two patches introduce the relevant functions while the last two patches
> switch the pcnet and eepro100 drivers over to use them, allowing us to remove
> their private implementations.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>
> Mark Cave-Ayland (4):
> net: move CRC32 calculation from compute_mcast_idx() into its own
> net_crc32() function
> net: introduce net_crc32_le() function
> pcnet: switch lnc_mchash() over to use net_crc32_le()
> eepro100: switch e100_compute_mcast_idx() over to use net_crc32()
>
> hw/net/eepro100.c | 19 +------------------
> hw/net/pcnet.c | 16 +---------------
> include/net/net.h | 5 ++++-
> net/net.c | 38 +++++++++++++++++++++++++++++++++-----
> 4 files changed, 39 insertions(+), 39 deletions(-)
Hi Jason,
Any thoughts on this patchset? I now have another virtual NIC
implementation for upstream that makes use of the new net_crc32_le()
function created by this patchset and I would really like to use this
instead of copy/pasting yet another different CRC32 implementation :)
ATB,
Mark.
^ permalink raw reply [flat|nested] 13+ messages in thread