Skip to contentSkip to conversation

lib/crc32: treewide: Use existing define with polynomial

PATCH In review Krzysztof Kozlowski 3 participants 10 messages Jul 17, 2018 – Jul 27, 2018

Subsystems (13)
Networking Network drivers Staging Library code +9 more subsystems
lore
Share & tools
lore

Krzysztof Kozlowski

author
#0 lore raw

cover lib/crc32: treewide: Use existing define with polynomial

Hi,

Kernel defines same polynomial for CRC-32 in few places. This is unnecessary duplication of the same value. Also this might be error-prone for future code - every driver will define the polynomial again.

This is an attempt to unify definition of polynomial. Few obvious hard-coded locations are fixed with define.

All series depend on each 1/6 and 2/6.

This could be merged in two different merge windows (1st lib/crc and then the rest) or taken through one tree.

It would be nice to get some testing. Only generic lib/crc, bunzip, xz_crc32 and Freescale's Ethernet driver were tested on HW. Rest got just different builds.

Best regards,
Krzysztof

Krzysztof Kozlowski (6):
lib/crc: Move polynomial definition to separate header lib/crc: Use consistent naming for CRC-32 polynomials
crypto: stm32_crc32 - Use existing define with polynomial
net: ethernet: Use existing define with polynomial
staging: rtl: Use existing define with polynomial
lib: Use existing define with polynomial

 drivers/crypto/stm32/stm32_crc32.c               | 11 ++++-------
 drivers/net/ethernet/amd/xgbe/xgbe-dev.c         |  4 ++--
 drivers/net/ethernet/apple/bmac.c                |  8 ++------
 drivers/net/ethernet/broadcom/tg3.c              |  3 ++-
 drivers/net/ethernet/freescale/fec_main.c        |  4 ++--
 drivers/net/ethernet/freescale/fs_enet/fec.h     |  3 ---
 drivers/net/ethernet/freescale/fs_enet/mac-fec.c |  3 ++-
 drivers/net/ethernet/micrel/ks8851_mll.c         |  3 ++-
 drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c    |  4 ++--
 drivers/staging/rtl8712/rtl871x_security.c       |  5 ++---
 drivers/staging/rtl8723bs/core/rtw_security.c    |  5 ++---
 include/linux/crc32poly.h                        | 20 ++++++++++++++++++++
 lib/crc32.c                                      | 11 ++++++-----
 lib/crc32defs.h                                  | 14 --------------
 lib/decompress_bunzip2.c                         |  3 ++-
 lib/gen_crc32table.c                             |  5 +++--
 lib/xz/xz_crc32.c                                |  3 ++-
 17 files changed, 55 insertions(+), 54 deletions(-)
 create mode 100644 include/linux/crc32poly.h
Signature · 1 line
2.14.1
author
#1 lore raw

PATCH 1/6 lib/crc: Move polynomial definition to separate header

Allow other drivers and parts of kernel to use the same define for CRC32 polynomial, instead of duplicating it in many places. This code does not bring any functional changes, except moving existing code.

Signed-off-by Krzysztof Kozlowski <[email protected]>
---
 include/linux/crc32poly.h | 20 ++++++++++++++++++++
 lib/crc32.c               |  1 +
 lib/crc32defs.h           | 14 --------------
 lib/gen_crc32table.c      |  1 +
 4 files changed, 22 insertions(+), 14 deletions(-)
 create mode 100644 include/linux/crc32poly.h
include/linux/crc32poly.h new file +20 -0
new file mode 100644
index 000000000000..7ad5aa92d3c7
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_CRC32_POLY_H
+#define _LINUX_CRC32_POLY_H
+
+/*
+ * There are multiple 16-bit CRC polynomials in common use, but this is
+ * *the* standard CRC-32 polynomial, first popularized by Ethernet.
+ * x^32+x^26+x^23+x^22+x^16+x^12+x^11+x^10+x^8+x^7+x^5+x^4+x^2+x^1+x^0
+ */
+#define CRCPOLY_LE 0xedb88320
+#define CRCPOLY_BE 0x04c11db7
+
+/*
+ * This is the CRC32c polynomial, as outlined by Castagnoli.
+ * x^32+x^28+x^27+x^26+x^25+x^23+x^22+x^20+x^19+x^18+x^14+x^13+x^11+x^10+x^9+
+ * x^8+x^6+x^0
+ */
+#define CRC32C_POLY_LE 0x82F63B78
+
+#endif /* _LINUX_CRC32_POLY_H */
lib/crc32.c +1 -0
index 2ef20fe84b69..341c54cb4edf 100644
@@ -27,6 +27,7 @@
 /* see: Documentation/crc32.txt for a description of algorithms */
 
 #include <linux/crc32.h>
+#include <linux/crc32poly.h>
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/sched.h>
lib/crc32defs.h +0 -14
index cb275a28a750..0c8fb5923e7e 100644
@@ -1,18 +1,4 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-/*
- * There are multiple 16-bit CRC polynomials in common use, but this is
- * *the* standard CRC-32 polynomial, first popularized by Ethernet.
- * x^32+x^26+x^23+x^22+x^16+x^12+x^11+x^10+x^8+x^7+x^5+x^4+x^2+x^1+x^0
- */
-#define CRCPOLY_LE 0xedb88320
-#define CRCPOLY_BE 0x04c11db7
-
-/*
- * This is the CRC32c polynomial, as outlined by Castagnoli.
- * x^32+x^28+x^27+x^26+x^25+x^23+x^22+x^20+x^19+x^18+x^14+x^13+x^11+x^10+x^9+
- * x^8+x^6+x^0
- */
-#define CRC32C_POLY_LE 0x82F63B78
 
 /* Try to choose an implementation variant via Kconfig */
 #ifdef CONFIG_CRC32_SLICEBY8
lib/gen_crc32table.c +1 -0
index 8f26660ea10a..34c3bc826f45 100644
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <stdio.h>
+#include "../include/linux/crc32poly.h"
 #include "../include/generated/autoconf.h"
 #include "crc32defs.h"
 #include <inttypes.h>
Signature · 1 line
2.14.1
author
#2 lore raw

PATCH 2/6 lib/crc: Use consistent naming for CRC-32 polynomials

In reply to Krzysztof Kozlowski

Header was defining CRCPOLY_LE/BE and CRC32C_POLY_LE but in fact all of them are CRC-32 polynomials so use consistent naming.

Signed-off-by Krzysztof Kozlowski <[email protected]>
---
 include/linux/crc32poly.h |  4 ++--
 lib/crc32.c               | 10 +++++-----
 lib/gen_crc32table.c      |  4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)
include/linux/crc32poly.h +2 -2
index 7ad5aa92d3c7..62c4b7790a28 100644
@@ -7,8 +7,8 @@
  * *the* standard CRC-32 polynomial, first popularized by Ethernet.
  * x^32+x^26+x^23+x^22+x^16+x^12+x^11+x^10+x^8+x^7+x^5+x^4+x^2+x^1+x^0
  */
-#define CRCPOLY_LE 0xedb88320
-#define CRCPOLY_BE 0x04c11db7
+#define CRC32_POLY_LE 0xedb88320
+#define CRC32_POLY_BE 0x04c11db7
 
 /*
  * This is the CRC32c polynomial, as outlined by Castagnoli.
lib/crc32.c +5 -5
index 341c54cb4edf..a6c9afafc8c8 100644
@@ -185,7 +185,7 @@ static inline u32 __pure crc32_le_generic(u32 crc, unsigned char const *p,
 #if CRC_LE_BITS == 1
 u32 __pure crc32_le(u32 crc, unsigned char const *p, size_t len)
 {
-	return crc32_le_generic(crc, p, len, NULL, CRCPOLY_LE);
+	return crc32_le_generic(crc, p, len, NULL, CRC32_POLY_LE);
 }
 u32 __pure __crc32c_le(u32 crc, unsigned char const *p, size_t len)
 {

@@ -195,7 +195,7 @@ u32 __pure __crc32c_le(u32 crc, unsigned char const *p, size_t len)
 u32 __pure crc32_le(u32 crc, unsigned char const *p, size_t len)
 {
 	return crc32_le_generic(crc, p, len,
-			(const u32 (*)[256])crc32table_le, CRCPOLY_LE);
+			(const u32 (*)[256])crc32table_le, CRC32_POLY_LE);
 }
 u32 __pure __crc32c_le(u32 crc, unsigned char const *p, size_t len)
 {

@@ -269,7 +269,7 @@ static u32 __attribute_const__ crc32_generic_shift(u32 crc, size_t len,
 
 u32 __attribute_const__ crc32_le_shift(u32 crc, size_t len)
 {
-	return crc32_generic_shift(crc, len, CRCPOLY_LE);
+	return crc32_generic_shift(crc, len, CRC32_POLY_LE);
 }
 
 u32 __attribute_const__ __crc32c_le_shift(u32 crc, size_t len)

@@ -331,13 +331,13 @@ static inline u32 __pure crc32_be_generic(u32 crc, unsigned char const *p,
 #if CRC_LE_BITS == 1
 u32 __pure crc32_be(u32 crc, unsigned char const *p, size_t len)
 {
-	return crc32_be_generic(crc, p, len, NULL, CRCPOLY_BE);
+	return crc32_be_generic(crc, p, len, NULL, CRC32_POLY_BE);
 }
 #else
 u32 __pure crc32_be(u32 crc, unsigned char const *p, size_t len)
 {
 	return crc32_be_generic(crc, p, len,
-			(const u32 (*)[256])crc32table_be, CRCPOLY_BE);
+			(const u32 (*)[256])crc32table_be, CRC32_POLY_BE);
 }
 #endif
 EXPORT_SYMBOL(crc32_be);
lib/gen_crc32table.c +2 -2
index 34c3bc826f45..f755b997b967 100644
@@ -58,7 +58,7 @@ static void crc32init_le_generic(const uint32_t polynomial,
 
 static void crc32init_le(void)
 {
-	crc32init_le_generic(CRCPOLY_LE, crc32table_le);
+	crc32init_le_generic(CRC32_POLY_LE, crc32table_le);
 }
 
 static void crc32cinit_le(void)

@@ -77,7 +77,7 @@ static void crc32init_be(void)
 	crc32table_be[0][0] = 0;
 
 	for (i = 1; i < BE_TABLE_SIZE; i <<= 1) {
-		crc = (crc << 1) ^ ((crc & 0x80000000) ? CRCPOLY_BE : 0);
+		crc = (crc << 1) ^ ((crc & 0x80000000) ? CRC32_POLY_BE : 0);
 		for (j = 0; j < i; j++)
 			crc32table_be[0][i + j] = crc ^ crc32table_be[0][j];
 	}
Signature · 1 line
2.14.1
author
#3 lore raw

PATCH 3/6 crypto: stm32_crc32 - Use existing define with polynomial

In reply to Krzysztof Kozlowski

Do not define again the polynomial but use header with existing define.

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
Not tested

It would be nice to get some testing. Only generic lib/crc, bunzip, xz_crc32 and Freescale's Ethernet driver were tested on HW. Rest got just different builds.

 drivers/crypto/stm32/stm32_crc32.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)
drivers/crypto/stm32/stm32_crc32.c +4 -7
index 040bed5e7725..29d2095d9dfd 100644
@@ -6,6 +6,7 @@
 
 #include <linux/bitrev.h>
 #include <linux/clk.h>
+#include <linux/crc32poly.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>

@@ -30,10 +31,6 @@
 #define CRC_CR_REVERSE          (BIT(7) | BIT(6) | BIT(5))
 #define CRC_INIT_DEFAULT        0xFFFFFFFF
 
-/* Polynomial reversed */
-#define POLY_CRC32              0xEDB88320
-#define POLY_CRC32C             0x82F63B78
-
 #define CRC_AUTOSUSPEND_DELAY	50
 
 struct stm32_crc {

@@ -70,7 +67,7 @@ static int stm32_crc32_cra_init(struct crypto_tfm *tfm)
 	struct stm32_crc_ctx *mctx = crypto_tfm_ctx(tfm);
 
 	mctx->key = CRC_INIT_DEFAULT;
-	mctx->poly = POLY_CRC32;
+	mctx->poly = CRC32_POLY_LE;
 	return 0;
 }
 

@@ -79,7 +76,7 @@ static int stm32_crc32c_cra_init(struct crypto_tfm *tfm)
 	struct stm32_crc_ctx *mctx = crypto_tfm_ctx(tfm);
 
 	mctx->key = CRC_INIT_DEFAULT;
-	mctx->poly = POLY_CRC32C;
+	mctx->poly = CRC32C_POLY_LE;
 	return 0;
 }
 

@@ -188,7 +185,7 @@ static int stm32_crc_final(struct shash_desc *desc, u8 *out)
 	struct stm32_crc_ctx *mctx = crypto_shash_ctx(desc->tfm);
 
 	/* Send computed CRC */
-	put_unaligned_le32(mctx->poly == POLY_CRC32C ?
+	put_unaligned_le32(mctx->poly == CRC32C_POLY_LE ?
 			   ~ctx->partial : ctx->partial, out);
 
 	return 0;
Signature · 1 line
2.14.1
author
#4 lore raw

PATCH 4/6 net: ethernet: Use existing define with polynomial

In reply to Krzysztof Kozlowski

Do not define again the polynomial but use header with existing define.

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
Only Freescale FEC was tested

It would be nice to get some testing. Only generic lib/crc, bunzip, xz_crc32 and Freescale's Ethernet driver were tested on HW. Rest got just different builds.

 drivers/net/ethernet/amd/xgbe/xgbe-dev.c         | 4 ++--
 drivers/net/ethernet/apple/bmac.c                | 8 ++------
 drivers/net/ethernet/broadcom/tg3.c              | 3 ++-
 drivers/net/ethernet/freescale/fec_main.c        | 4 ++--
 drivers/net/ethernet/freescale/fs_enet/fec.h     | 3 ---
 drivers/net/ethernet/freescale/fs_enet/mac-fec.c | 3 ++-
 drivers/net/ethernet/micrel/ks8851_mll.c         | 3 ++-
 drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c    | 4 ++--
 8 files changed, 14 insertions(+), 18 deletions(-)
drivers/net/ethernet/amd/xgbe/xgbe-dev.c +2 -2
index e107e180e2c8..1e929a1e4ca7 100644
@@ -119,6 +119,7 @@
 #include <linux/clk.h>
 #include <linux/bitrev.h>
 #include <linux/crc32.h>
+#include <linux/crc32poly.h>
 
 #include "xgbe.h"
 #include "xgbe-common.h"

@@ -887,7 +888,6 @@ static int xgbe_disable_rx_vlan_filtering(struct xgbe_prv_data *pdata)
 
 static u32 xgbe_vid_crc32_le(__le16 vid_le)
 {
-	u32 poly = 0xedb88320;	/* CRCPOLY_LE */
 	u32 crc = ~0;
 	u32 temp = 0;
 	unsigned char *data = (unsigned char *)&vid_le;

@@ -904,7 +904,7 @@ static u32 xgbe_vid_crc32_le(__le16 vid_le)
 		data_byte >>= 1;
 
 		if (temp)
-			crc ^= poly;
+			crc ^= CRC32_POLY_LE;
 	}
 
 	return crc;
drivers/net/ethernet/apple/bmac.c +2 -6
index 5a655d289dd5..024998d6d8c6 100644
@@ -19,6 +19,7 @@
 #include <linux/init.h>
 #include <linux/spinlock.h>
 #include <linux/crc32.h>
+#include <linux/crc32poly.h>
 #include <linux/bitrev.h>
 #include <linux/ethtool.h>
 #include <linux/slab.h>

@@ -37,11 +38,6 @@
 #define trunc_page(x)	((void *)(((unsigned long)(x)) & ~((unsigned long)(PAGE_SIZE - 1))))
 #define round_page(x)	trunc_page(((unsigned long)(x)) + ((unsigned long)(PAGE_SIZE - 1)))
 
-/*
- * CRC polynomial - used in working out multicast filter bits.
- */
-#define ENET_CRCPOLY 0x04c11db7
-
 /* switch to use multicast code lifted from sunhme driver */
 #define SUNHME_MULTICAST
 

@@ -838,7 +834,7 @@ crc416(unsigned int curval, unsigned short nxtval)
 		next = next >> 1;
 
 		/* do the XOR */
-		if (high_crc_set ^ low_data_set) cur = cur ^ ENET_CRCPOLY;
+		if (high_crc_set ^ low_data_set) cur = cur ^ CRC32_POLY_BE;
 	}
 	return cur;
 }
drivers/net/ethernet/broadcom/tg3.c +2 -1
index 0a796d5ec893..3d526d99ac59 100644
@@ -50,6 +50,7 @@
 #include <linux/ssb/ssb_driver_gige.h>
 #include <linux/hwmon.h>
 #include <linux/hwmon-sysfs.h>
+#include <linux/crc32poly.h>
 
 #include <net/checksum.h>
 #include <net/ip.h>

@@ -9709,7 +9710,7 @@ static inline u32 calc_crc(unsigned char *buf, int len)
 			reg >>= 1;
 
 			if (tmp)
-				reg ^= 0xedb88320;
+				reg ^= CRC32_POLY_LE;
 		}
 	}
drivers/net/ethernet/freescale/fec_main.c +2 -2
index c729665107f5..a7cb378ef881 100644
@@ -48,6 +48,7 @@
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/clk.h>
+#include <linux/crc32poly.h>
 #include <linux/platform_device.h>
 #include <linux/mdio.h>
 #include <linux/phy.h>

@@ -2949,7 +2950,6 @@ fec_enet_close(struct net_device *ndev)
  */
 
 #define FEC_HASH_BITS	6		/* #bits in hash */
-#define CRC32_POLY	0xEDB88320
 
 static void set_multicast_list(struct net_device *ndev)
 {

@@ -2989,7 +2989,7 @@ static void set_multicast_list(struct net_device *ndev)
 			data = ha->addr[i];
 			for (bit = 0; bit < 8; bit++, data >>= 1) {
 				crc = (crc >> 1) ^
-				(((crc ^ data) & 1) ? CRC32_POLY : 0);
+				(((crc ^ data) & 1) ? CRC32_POLY_LE : 0);
 			}
 		}
drivers/net/ethernet/freescale/fs_enet/fec.h +0 -3
index 7832db71dcb9..1dbee5d898b3 100644
@@ -2,9 +2,6 @@
 #ifndef FS_ENET_FEC_H
 #define FS_ENET_FEC_H
 
-/* CRC polynomium used by the FEC for the multicast group filtering */
-#define FEC_CRC_POLY   0x04C11DB7
-
 #define FEC_MAX_MULTICAST_ADDRS	64
 
 /* Interrupt events/masks.
drivers/net/ethernet/freescale/fs_enet/mac-fec.c +2 -1
index 1fc27c97e3b2..05093e5fc9dd 100644
@@ -18,6 +18,7 @@
 #include <linux/string.h>
 #include <linux/ptrace.h>
 #include <linux/errno.h>
+#include <linux/crc32poly.h>
 #include <linux/ioport.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>

@@ -187,7 +188,7 @@ static void set_multicast_one(struct net_device *dev, const u8 *mac)
 			msb = crc >> 31;
 			crc <<= 1;
 			if (msb ^ (byte & 0x1))
-				crc ^= FEC_CRC_POLY;
+				crc ^= CRC32_POLY_BE;
 			byte >>= 1;
 		}
 	}
drivers/net/ethernet/micrel/ks8851_mll.c +2 -1
index f3e9dd47b56f..0e9719fbc624 100644
@@ -30,6 +30,7 @@
 #include <linux/ethtool.h>
 #include <linux/cache.h>
 #include <linux/crc32.h>
+#include <linux/crc32poly.h>
 #include <linux/mii.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>

@@ -1078,7 +1079,7 @@ static void ks_stop_rx(struct ks_net *ks)
 
 }  /* ks_stop_rx */
 
-static unsigned long const ethernet_polynomial = 0x04c11db7U;
+static unsigned long const ethernet_polynomial = CRC32_POLY_BE;
 
 static unsigned long ether_gen_crc(int length, u8 *data)
 {
drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c +2 -2
index 458a7844260a..99d86e39ff54 100644
@@ -20,6 +20,7 @@
 #include <linux/clk.h>
 #include <linux/bitrev.h>
 #include <linux/crc32.h>
+#include <linux/crc32poly.h>
 #include <linux/dcbnl.h>
 
 #include "dwc-xlgmac.h"

@@ -193,7 +194,6 @@ static u32 xlgmac_vid_crc32_le(__le16 vid_le)
 {
 	unsigned char *data = (unsigned char *)&vid_le;
 	unsigned char data_byte = 0;
-	u32 poly = 0xedb88320;
 	u32 crc = ~0;
 	u32 temp = 0;
 	int i, bits;

@@ -208,7 +208,7 @@ static u32 xlgmac_vid_crc32_le(__le16 vid_le)
 		data_byte >>= 1;
 
 		if (temp)
-			crc ^= poly;
+			crc ^= CRC32_POLY_LE;
 	}
 
 	return crc;
Signature · 1 line
2.14.1
author
#5 lore raw

PATCH 5/6 staging: rtl: Use existing define with polynomial

In reply to Krzysztof Kozlowski

Do not define again the polynomial but use header with existing define.

Signed-off-by Krzysztof Kozlowski <[email protected]>

---
Not tested

It would be nice to get some testing. Only generic lib/crc, bunzip, xz_crc32 and Freescale's Ethernet driver were tested on HW. Rest got just different builds.

 drivers/staging/rtl8712/rtl871x_security.c    | 5 ++---
 drivers/staging/rtl8723bs/core/rtw_security.c | 5 ++---
 2 files changed, 4 insertions(+), 6 deletions(-)
drivers/staging/rtl8712/rtl871x_security.c +2 -3
index 7bc74d7d8a3a..1075eacdb441 100644
@@ -40,6 +40,7 @@
 #include <linux/uaccess.h>
 #include <asm/byteorder.h>
 #include <linux/atomic.h>
+#include <linux/crc32poly.h>
 #include <linux/semaphore.h>
 
 #include "osdep_service.h"

@@ -49,8 +50,6 @@
 
 /* =====WEP related===== */
 
-#define CRC32_POLY 0x04c11db7
-
 struct arc4context {
 	u32 x;
 	u32 y;

@@ -135,7 +134,7 @@ static void crc32_init(void)
 	for (i = 0; i < 256; ++i) {
 		k = crc32_reverseBit((u8)i);
 		for (c = ((u32)k) << 24, j = 8; j > 0; --j)
-			c = c & 0x80000000 ? (c << 1) ^ CRC32_POLY : (c << 1);
+			c = c & 0x80000000 ? (c << 1) ^ CRC32_POLY_BE : (c << 1);
 		p1 = (u8 *)&crc32_table[i];
 		p1[0] = crc32_reverseBit(p[3]);
 		p1[1] = crc32_reverseBit(p[2]);
drivers/staging/rtl8723bs/core/rtw_security.c +2 -3
index 612277a555d2..6c8ac9e86c9f 100644
@@ -6,6 +6,7 @@
  ******************************************************************************/
 #define  _RTW_SECURITY_C_
 
+#include <linux/crc32poly.h>
 #include <drv_types.h>
 #include <rtw_debug.h>
 

@@ -87,8 +88,6 @@ const char *security_type_str(u8 value)
 
 /* WEP related ===== */
 
-#define CRC32_POLY 0x04c11db7
-
 struct arc4context {
 	u32 x;
 	u32 y;

@@ -178,7 +177,7 @@ static void crc32_init(void)
 		for (i = 0; i < 256; ++i) {
 			k = crc32_reverseBit((u8)i);
 			for (c = ((u32)k) << 24, j = 8; j > 0; --j) {
-				c = c & 0x80000000 ? (c << 1) ^ CRC32_POLY : (c << 1);
+				c = c & 0x80000000 ? (c << 1) ^ CRC32_POLY_BE : (c << 1);
 			}
 			p1 = (u8 *)&crc32_table[i];
Signature · 1 line
2.14.1
author
#6 lore raw

PATCH 6/6 lib: Use existing define with polynomial

In reply to Krzysztof Kozlowski

Do not define again the polynomial but use header with existing define.

Signed-off-by Krzysztof Kozlowski <[email protected]>
---
 lib/decompress_bunzip2.c | 3 ++-
 lib/xz/xz_crc32.c        | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)
lib/decompress_bunzip2.c +2 -1
index 0234361b24b8..7c4932eed748 100644
@@ -51,6 +51,7 @@
 #endif /* STATIC */
 
 #include <linux/decompress/mm.h>
+#include <linux/crc32poly.h>
 
 #ifndef INT_MAX
 #define INT_MAX 0x7fffffff

@@ -654,7 +655,7 @@ static int INIT start_bunzip(struct bunzip_data **bdp, void *inbuf, long len,
 	for (i = 0; i < 256; i++) {
 		c = i << 24;
 		for (j = 8; j; j--)
-			c = c&0x80000000 ? (c << 1)^0x04c11db7 : (c << 1);
+			c = c&0x80000000 ? (c << 1)^(CRC32_POLY_BE) : (c << 1);
 		bd->crc32Table[i] = c;
 	}
lib/xz/xz_crc32.c +2 -1
index 34532d14fd4c..25a5d87e2e4c 100644
@@ -15,6 +15,7 @@
  * but they are bigger and use more memory for the lookup table.
  */
 
+#include <linux/crc32poly.h>
 #include "xz_private.h"
 
 /*

@@ -29,7 +30,7 @@ STATIC_RW_DATA uint32_t xz_crc32_table[256];
 
 XZ_EXTERN void xz_crc32_init(void)
 {
-	const uint32_t poly = 0xEDB88320;
+	const uint32_t poly = CRC32_POLY_LE;
 
 	uint32_t i;
 	uint32_t j;
Signature · 1 line
2.14.1
#7 lore raw
In reply to Krzysztof Kozlowski

Hi Krzysztof,

Quoted 41 lines · Krzysztof Kozlowski

Hi,

Kernel defines same polynomial for CRC-32 in few places. This is unnecessary duplication of the same value. Also this might be error-prone for future code - every driver will define the polynomial again.

This is an attempt to unify definition of polynomial. Few obvious hard-coded locations are fixed with define.

All series depend on each 1/6 and 2/6.

This could be merged in two different merge windows (1st lib/crc and then the rest) or taken through one tree.

It would be nice to get some testing. Only generic lib/crc, bunzip, xz_crc32 and Freescale's Ethernet driver were tested on HW. Rest got just different builds.

Best regards,
Krzysztof

Krzysztof Kozlowski (6):
lib/crc: Move polynomial definition to separate header lib/crc: Use consistent naming for CRC-32 polynomials
crypto: stm32_crc32 - Use existing define with polynomial
net: ethernet: Use existing define with polynomial
staging: rtl: Use existing define with polynomial
lib: Use existing define with polynomial

 drivers/crypto/stm32/stm32_crc32.c               | 11 ++++-------
 drivers/net/ethernet/amd/xgbe/xgbe-dev.c         |  4 ++--
 drivers/net/ethernet/apple/bmac.c                |  8 ++------
 drivers/net/ethernet/broadcom/tg3.c              |  3 ++-
 drivers/net/ethernet/freescale/fec_main.c        |  4 ++--
 drivers/net/ethernet/freescale/fs_enet/fec.h     |  3 ---
 drivers/net/ethernet/freescale/fs_enet/mac-fec.c |  3 ++-
 drivers/net/ethernet/micrel/ks8851_mll.c         |  3 ++-
 drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c    |  4 ++--
 drivers/staging/rtl8712/rtl871x_security.c       |  5 ++---
 drivers/staging/rtl8723bs/core/rtw_security.c    |  5 ++---
Quoted 9 lines
 include/linux/crc32poly.h                        | 20 ++++++++++++++++++++
 lib/crc32.c                                      | 11 ++++++-----
 lib/crc32defs.h                                  | 14 --------------
 lib/decompress_bunzip2.c                         |  3 ++-
 lib/gen_crc32table.c                             |  5 +++--
 lib/xz/xz_crc32.c                                |  3 ++-
 17 files changed, 55 insertions(+), 54 deletions(-)
 create mode 100644 include/linux/crc32poly.h

Did you check whether any of these users can be converted to use the CRC implementations in lib/, so they wouldn't need the polynomial definition themselves?

- Eric

author
#8 lore raw
Quoted 54 lines · Eric Biggers

Hi Krzysztof,

Quoted 41 lines · Krzysztof Kozlowski

Hi,

Kernel defines same polynomial for CRC-32 in few places. This is unnecessary duplication of the same value. Also this might be error-prone for future code - every driver will define the polynomial again.

This is an attempt to unify definition of polynomial. Few obvious hard-coded locations are fixed with define.

All series depend on each 1/6 and 2/6.

This could be merged in two different merge windows (1st lib/crc and then the rest) or taken through one tree.

It would be nice to get some testing. Only generic lib/crc, bunzip, xz_crc32 and Freescale's Ethernet driver were tested on HW. Rest got just different builds.

Best regards,
Krzysztof

Krzysztof Kozlowski (6):
lib/crc: Move polynomial definition to separate header lib/crc: Use consistent naming for CRC-32 polynomials
crypto: stm32_crc32 - Use existing define with polynomial
net: ethernet: Use existing define with polynomial
staging: rtl: Use existing define with polynomial
lib: Use existing define with polynomial

 drivers/crypto/stm32/stm32_crc32.c               | 11 ++++-------
 drivers/net/ethernet/amd/xgbe/xgbe-dev.c         |  4 ++--
 drivers/net/ethernet/apple/bmac.c                |  8 ++------
 drivers/net/ethernet/broadcom/tg3.c              |  3 ++-
 drivers/net/ethernet/freescale/fec_main.c        |  4 ++--
 drivers/net/ethernet/freescale/fs_enet/fec.h     |  3 ---
 drivers/net/ethernet/freescale/fs_enet/mac-fec.c |  3 ++-
 drivers/net/ethernet/micrel/ks8851_mll.c         |  3 ++-
 drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c    |  4 ++--
 drivers/staging/rtl8712/rtl871x_security.c       |  5 ++---
 drivers/staging/rtl8723bs/core/rtw_security.c    |  5 ++---
Quoted 9 lines
 include/linux/crc32poly.h                        | 20 ++++++++++++++++++++
 lib/crc32.c                                      | 11 ++++++-----
 lib/crc32defs.h                                  | 14 --------------
 lib/decompress_bunzip2.c                         |  3 ++-
 lib/gen_crc32table.c                             |  5 +++--
 lib/xz/xz_crc32.c                                |  3 ++-
 17 files changed, 55 insertions(+), 54 deletions(-)
 create mode 100644 include/linux/crc32poly.h
Quoted 3 lines

Did you check whether any of these users can be converted to use the CRC implementations in lib/, so they wouldn't need the polynomial definition themselves?

I did not check but that's interesting point... The Ethernet drivers (xgbe, tg3, fec, ks8851, dwc-xlgmac) look like could be converted to generic implementation. The apple/bmac looks weird. The rtl WiFi drivers in long term can be converted to use generic lib80211 for encryption (see commit 0d4876f4e977 ("staging:r8188eu: Use lib80211 to encrypt (TKIP) tx frames")) but that is much bigger task. The remaining use the polynomials in different aspect:
1. XZ and BUNZIP use it to create CRC tables - probably generic gen_crc32table.c could be used,
2. stm32_crc32.c uses it to initialize HW CRC accelerator.

I can work on Freescale FEC, xz and bunzip code because these I can test but I would prefer to do it as follow up.

Best regards,
Krzysztof

maintainer
#9 lore raw
In reply to Krzysztof Kozlowski
Quoted 44 lines · Krzysztof Kozlowski

Hi,

Kernel defines same polynomial for CRC-32 in few places. This is unnecessary duplication of the same value. Also this might be error-prone for future code - every driver will define the polynomial again.

This is an attempt to unify definition of polynomial. Few obvious hard-coded locations are fixed with define.

All series depend on each 1/6 and 2/6.

This could be merged in two different merge windows (1st lib/crc and then the rest) or taken through one tree.

It would be nice to get some testing. Only generic lib/crc, bunzip, xz_crc32 and Freescale's Ethernet driver were tested on HW. Rest got just different builds.

Best regards,
Krzysztof

Krzysztof Kozlowski (6):
lib/crc: Move polynomial definition to separate header lib/crc: Use consistent naming for CRC-32 polynomials
crypto: stm32_crc32 - Use existing define with polynomial
net: ethernet: Use existing define with polynomial
staging: rtl: Use existing define with polynomial
lib: Use existing define with polynomial

 drivers/crypto/stm32/stm32_crc32.c               | 11 ++++-------
 drivers/net/ethernet/amd/xgbe/xgbe-dev.c         |  4 ++--
 drivers/net/ethernet/apple/bmac.c                |  8 ++------
 drivers/net/ethernet/broadcom/tg3.c              |  3 ++-
 drivers/net/ethernet/freescale/fec_main.c        |  4 ++--
 drivers/net/ethernet/freescale/fs_enet/fec.h     |  3 ---
 drivers/net/ethernet/freescale/fs_enet/mac-fec.c |  3 ++-
 drivers/net/ethernet/micrel/ks8851_mll.c         |  3 ++-
 drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c    |  4 ++--
 drivers/staging/rtl8712/rtl871x_security.c       |  5 ++---
 drivers/staging/rtl8723bs/core/rtw_security.c    |  5 ++---
Quoted 8 lines
 include/linux/crc32poly.h                        | 20 ++++++++++++++++++++
 lib/crc32.c                                      | 11 ++++++-----
 lib/crc32defs.h                                  | 14 --------------
 lib/decompress_bunzip2.c                         |  3 ++-
 lib/gen_crc32table.c                             |  5 +++--
 lib/xz/xz_crc32.c                                |  3 ++-
 17 files changed, 55 insertions(+), 54 deletions(-)
 create mode 100644 include/linux/crc32poly.h

All applied. Thanks.

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt