net: ethernet: fs-enet: Use generic CRC32 implementation
Subsystems (3)
PATCH 1/1 net: ethernet: fs-enet: Use generic CRC32 implementation
Use generic kernel CRC32 implementation because it:
1. Should be faster (uses lookup tables),
2. Removes duplicated CRC generation code,
3. Uses well-proven algorithm instead of coding it one more time.
---
Not tested on hardware.
--- drivers/net/ethernet/freescale/fs_enet/mac-fec.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-)
drivers/net/ethernet/freescale/fs_enet/mac-fec.c +4 -14
@@ -18,6 +18,7 @@#include <linux/string.h> #include <linux/ptrace.h> #include <linux/errno.h> +#include <linux/crc32.h> #include <linux/ioport.h> #include <linux/interrupt.h> #include <linux/delay.h>
@@ -176,21 +177,10 @@ static void set_multicast_start(struct net_device *dev)static void set_multicast_one (struct net_device *dev,const u8 *mac) {struct fs_enet_private *fep =netdev_priv (dev); -int temp, hash_index, i, j; +int temp, hash_index;u32 crc, csrVal; -u8 byte, msb; - - crc =0xffffffff ; -for (i =0 ; i <6 ; i++) { - byte = mac[i]; -for (j =0 ; j <8 ; j++) { - msb = crc >>31 ; - crc <<=1 ; -if (msb ^ (byte &0x1 )) - crc ^=FEC_CRC_POLY ; - byte >>=1 ; - } - } + + crc =ether_crc (6 , mac); temp = (crc &0x3f ) >>1 ; hash_index = ((temp & 0x01) << 4) |
Signature · 1 line
2.14.1
From: Krzysztof Kozlowski
Quoted 3 lines
Sent: 23 July 2018 17:20
Use generic kernel CRC32 implementation because it:
1. Should be faster (uses lookup tables),
Are you sure?
The lookup tables are unlikely to be in the data cache and the 6 cache misses kill performance.
(Not that it particularly matters when setting up multicast hash tables).
Quoted 2 lines
2. Removes duplicated CRC generation code,
3. Uses well-proven algorithm instead of coding it one more time.
...
Quoted 2 lines
Not tested on hardware.
Have you verified that the old and new functions give the same result for a few mac addresses?
It is very easy to use the wrong bits in crc calculations or generate the output in the wrong bit order.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Quoted 9 lines · David Laight
From: Krzysztof Kozlowski
Quoted 3 lines
Sent: 23 July 2018 17:20
Use generic kernel CRC32 implementation because it:
1. Should be faster (uses lookup tables),Are you sure?
The lookup tables are unlikely to be in the data cache and the 6 cache misses kill performance.
(Not that it particularly matters when setting up multicast hash tables).
Good point, so this statement should be rather "Could be faster"... I did not run any performance tests so this is not backed up by any data.
I think the main benefit is rather easier code maintenance by removing duplicated, custom code.
Quoted 10 lines
Quoted 2 lines
2. Removes duplicated CRC generation code,
3. Uses well-proven algorithm instead of coding it one more time....
Quoted 2 lines
Not tested on hardware.
Have you verified that the old and new functions give the same result for a few mac addresses?
It is very easy to use the wrong bits in crc calculations or generate the output in the wrong bit order.
I copied the original code and new one onto a different driver and run this in a loop for thousands of data input (although not all possible MAC combinations). The output was the same. I agree however that real testing would be important.
Best regards,
Krzysztof
From: Krzysztof Kozlowski
Quoted 1 line
Sent: 24 July 2018 12:12
...
Quoted 11 lines
Quoted 6 lines
Quoted 1 line
Not tested on hardware.
Have you verified that the old and new functions give the same result for a few mac addresses?
It is very easy to use the wrong bits in crc calculations or generate the output in the wrong bit order.I copied the original code and new one onto a different driver and run this in a loop for thousands of data input (although not all possible MAC combinations). The output was the same. I agree however that real testing would be important.
Since CRC are linear you only need to check that each input bit generates the correct output.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
From: Krzysztof Kozlowski <[email protected]>
Date: Mon, 23 Jul 2018 18:20:20 +0200
Quoted 7 lines
Use generic kernel CRC32 implementation because it:
1. Should be faster (uses lookup tables),
2. Removes duplicated CRC generation code,
3. Uses well-proven algorithm instead of coding it one more time.Suggested-by Eric Biggers <[email protected]>Signed-off-by Krzysztof Kozlowski <[email protected]>
Applied.