2018-07-17 16:05:35

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 0/6] 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

--
2.14.1


2018-07-17 16:05:36

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [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

diff --git a/include/linux/crc32poly.h b/include/linux/crc32poly.h
new file mode 100644
index 000000000000..7ad5aa92d3c7
--- /dev/null
+++ b/include/linux/crc32poly.h
@@ -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 */
diff --git a/lib/crc32.c b/lib/crc32.c
index 2ef20fe84b69..341c54cb4edf 100644
--- a/lib/crc32.c
+++ b/lib/crc32.c
@@ -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>
diff --git a/lib/crc32defs.h b/lib/crc32defs.h
index cb275a28a750..0c8fb5923e7e 100644
--- a/lib/crc32defs.h
+++ b/lib/crc32defs.h
@@ -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
diff --git a/lib/gen_crc32table.c b/lib/gen_crc32table.c
index 8f26660ea10a..34c3bc826f45 100644
--- a/lib/gen_crc32table.c
+++ b/lib/gen_crc32table.c
@@ -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>
--
2.14.1

2018-07-17 16:05:37

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 2/6] lib/crc: Use consistent naming for CRC-32 polynomials

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(-)

diff --git a/include/linux/crc32poly.h b/include/linux/crc32poly.h
index 7ad5aa92d3c7..62c4b7790a28 100644
--- a/include/linux/crc32poly.h
+++ b/include/linux/crc32poly.h
@@ -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.
diff --git a/lib/crc32.c b/lib/crc32.c
index 341c54cb4edf..a6c9afafc8c8 100644
--- a/lib/crc32.c
+++ b/lib/crc32.c
@@ -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);
diff --git a/lib/gen_crc32table.c b/lib/gen_crc32table.c
index 34c3bc826f45..f755b997b967 100644
--- a/lib/gen_crc32table.c
+++ b/lib/gen_crc32table.c
@@ -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];
}
--
2.14.1

2018-07-17 16:05:38

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 3/6] crypto: stm32_crc32 - Use existing define with polynomial

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(-)

diff --git a/drivers/crypto/stm32/stm32_crc32.c b/drivers/crypto/stm32/stm32_crc32.c
index 040bed5e7725..29d2095d9dfd 100644
--- a/drivers/crypto/stm32/stm32_crc32.c
+++ b/drivers/crypto/stm32/stm32_crc32.c
@@ -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;
--
2.14.1

2018-07-17 16:05:39

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 4/6] net: ethernet: Use existing define with polynomial

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(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
index e107e180e2c8..1e929a1e4ca7 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-dev.c
@@ -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;
diff --git a/drivers/net/ethernet/apple/bmac.c b/drivers/net/ethernet/apple/bmac.c
index 5a655d289dd5..024998d6d8c6 100644
--- a/drivers/net/ethernet/apple/bmac.c
+++ b/drivers/net/ethernet/apple/bmac.c
@@ -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;
}
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 0a796d5ec893..3d526d99ac59 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -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;
}
}

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index c729665107f5..a7cb378ef881 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -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);
}
}

diff --git a/drivers/net/ethernet/freescale/fs_enet/fec.h b/drivers/net/ethernet/freescale/fs_enet/fec.h
index 7832db71dcb9..1dbee5d898b3 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fec.h
+++ b/drivers/net/ethernet/freescale/fs_enet/fec.h
@@ -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.
diff --git a/drivers/net/ethernet/freescale/fs_enet/mac-fec.c b/drivers/net/ethernet/freescale/fs_enet/mac-fec.c
index 1fc27c97e3b2..05093e5fc9dd 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mac-fec.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mac-fec.c
@@ -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;
}
}
diff --git a/drivers/net/ethernet/micrel/ks8851_mll.c b/drivers/net/ethernet/micrel/ks8851_mll.c
index f3e9dd47b56f..0e9719fbc624 100644
--- a/drivers/net/ethernet/micrel/ks8851_mll.c
+++ b/drivers/net/ethernet/micrel/ks8851_mll.c
@@ -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)
{
diff --git a/drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c b/drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c
index 458a7844260a..99d86e39ff54 100644
--- a/drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c
+++ b/drivers/net/ethernet/synopsys/dwc-xlgmac-hw.c
@@ -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;
--
2.14.1

2018-07-17 16:05:40

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 5/6] staging: rtl: Use existing define with polynomial

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(-)

diff --git a/drivers/staging/rtl8712/rtl871x_security.c b/drivers/staging/rtl8712/rtl871x_security.c
index 7bc74d7d8a3a..1075eacdb441 100644
--- a/drivers/staging/rtl8712/rtl871x_security.c
+++ b/drivers/staging/rtl8712/rtl871x_security.c
@@ -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]);
diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c
index 612277a555d2..6c8ac9e86c9f 100644
--- a/drivers/staging/rtl8723bs/core/rtw_security.c
+++ b/drivers/staging/rtl8723bs/core/rtw_security.c
@@ -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];

--
2.14.1

2018-07-17 16:05:41

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 6/6] lib: Use existing define with polynomial

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(-)

diff --git a/lib/decompress_bunzip2.c b/lib/decompress_bunzip2.c
index 0234361b24b8..7c4932eed748 100644
--- a/lib/decompress_bunzip2.c
+++ b/lib/decompress_bunzip2.c
@@ -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;
}

diff --git a/lib/xz/xz_crc32.c b/lib/xz/xz_crc32.c
index 34532d14fd4c..25a5d87e2e4c 100644
--- a/lib/xz/xz_crc32.c
+++ b/lib/xz/xz_crc32.c
@@ -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;
--
2.14.1

2018-07-18 00:12:58

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH 0/6] lib/crc32: treewide: Use existing define with polynomial

Hi Krzysztof,

On Tue, Jul 17, 2018 at 06:05:35PM +0200, Krzysztof Kozlowski wrote:
> 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
>

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

2018-07-18 06:33:56

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 0/6] lib/crc32: treewide: Use existing define with polynomial

On 18 July 2018 at 02:12, Eric Biggers <[email protected]> wrote:
> Hi Krzysztof,
>
> On Tue, Jul 17, 2018 at 06:05:35PM +0200, Krzysztof Kozlowski wrote:
>> 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
>>
>
> 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

2018-07-27 16:05:13

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/6] lib/crc32: treewide: Use existing define with polynomial

On Tue, Jul 17, 2018 at 06:05:35PM +0200, Krzysztof Kozlowski wrote:
> 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

All applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt