[dpdk-dev,v5,2/2] test/test: add unit test for CRC computation
Checks
Commit Message
This patch provides a set of tests for verifying the functional
correctness of 16-bit and 32-bit CRC APIs.
Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
---
test/test/Makefile | 2 +
test/test/test_crc.c | 223 +++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 225 insertions(+)
create mode 100644 test/test/test_crc.c
Comments
Hi Jasvinder,
> -----Original Message-----
> From: Singh, Jasvinder
> Sent: Tuesday, March 21, 2017 2:46 PM
> To: dev@dpdk.org
> Cc: olivier.matz@6wind.com; Doherty, Declan; De Lara Guarch, Pablo
> Subject: [PATCH v5 2/2] test/test: add unit test for CRC computation
>
> This patch provides a set of tests for verifying the functional
> correctness of 16-bit and 32-bit CRC APIs.
>
> Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> ---
> test/test/Makefile | 2 +
> test/test/test_crc.c | 223
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 225 insertions(+)
> create mode 100644 test/test/test_crc.c
>
> diff --git a/test/test/test_crc.c b/test/test/test_crc.c
> new file mode 100644
> index 0000000..2eb0bff
> --- /dev/null
> +++ b/test/test/test_crc.c
> @@ -0,0 +1,223 @@
> +
> +#include "test.h"
> +
> +#define CRC_VEC_LEN 32
Unnecessary extra tab.
> +#define CRC32_VEC_LEN1 1512
> +#define CRC32_VEC_LEN2 348
> +#define CRC16_VEC_LEN1 12
> +#define CRC16_VEC_LEN2 2
> +#define LINE_LEN 75
I would align this to the other values above.
...
> +
> +/* 16-bit CRC test vector 1*/
Extra space before "*/".
> +static const uint8_t crc16_vec1[CRC16_VEC_LEN1] = {
> + 0x0D, 0x01, 0x01, 0x23, 0x45, 0x67,
> + 0x89, 0x01, 0x23, 0x45, 0x00, 0x01,
> +};
> +
> +/* 16-bit CRC test vector 2*/
Same here.
> +static const uint8_t crc16_vec2[CRC16_VEC_LEN2] = {
> + 0x03, 0x3f,
> +};
> +/** CRC results */
> +uint32_t crc32_vec_res, crc32_vec1_res, crc32_vec2_res;
> +uint32_t crc16_vec_res, crc16_vec1_res, crc16_vec2_res;
> +
> +static int
> +crc_calc(const uint8_t *vec,
> + uint32_t vec_len,
> + enum rte_net_crc_type type)
> +{
> + /* compute CRC */
> + uint32_t ret = rte_net_crc_calc(vec, vec_len, type);
> +
> + /* dump data on console*/
Same here.
> + rte_hexdump(stdout, NULL, vec, vec_len);
I would use TEST_HEXDUMP, which dumps the stream only if debug is enabled (to avoid too much output).
> +
> + return ret;
> +}
> +
> +static void
> +crc_calc_scalar(void)
> +{
> + uint32_t i;
> + enum rte_net_crc_type type;
> + uint8_t *test_data;
> +
> + /* 32-bit ethernet CRC: scalar result */
> + type = RTE_NET_CRC32_ETH;
> + crc32_vec_res = crc_calc(crc_vec,
> + CRC_VEC_LEN,
> + type);
Remove unnecessary tabs (more of this in the rest of the file).
> +
> + /* 32-bit ethernet CRC scalar result*/
> + test_data = rte_zmalloc(NULL, CRC32_VEC_LEN1, 0);
> +
> + for (i = 0; i < CRC32_VEC_LEN1; i += 12)
> + rte_memcpy(&test_data[i], crc32_vec1, 12);
Wrong indentation (more of this in the rest of the file).
Also, why not just one memcpy call, with CRC32_VEC_LEN1.
A comment would be good to explain why.
...
> +static int
> +test_crc(void) {
Looks strange to start the parameters in a new line.
I would say you can start in the same line as the function name.
> + uint32_t i;
> + enum rte_net_crc_type type;
> + uint8_t *test_data;
> + uint32_t ret;
> +
> + /* set crc scalar mode */
Probably the references of "crc" in comments should be uppercase "CRC".
> + rte_net_crc_set_alg(RTE_NET_CRC_SCALAR);
> + crc_calc_scalar();
> +
> + /* set crc sse4.2 mode */
> + rte_net_crc_set_alg(RTE_NET_CRC_SSE42);
> +
> + /* 32-bit ethernet CRC: Test 1 */
> + type = RTE_NET_CRC32_ETH;
> +
> + ret = crc_calc(crc_vec,
> + CRC_VEC_LEN,
> + type);
> + if (ret != crc32_vec_res) {
> + printf("test_crc(32-bit): test1 failed !\n");
Extra space before "!" (keep consistency with the other printfs).
> + return -1;
> + }
> +
> + /* 32-bit ethernet CRC: Test 2 */
> + test_data = rte_zmalloc(NULL, CRC32_VEC_LEN1, 0);
Free the memory after the test (also for other calls).
Hi Pablo,
> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Tuesday, March 28, 2017 8:23 PM
> To: Singh, Jasvinder <jasvinder.singh@intel.com>; dev@dpdk.org
> Cc: olivier.matz@6wind.com; Doherty, Declan <declan.doherty@intel.com>
> Subject: RE: [PATCH v5 2/2] test/test: add unit test for CRC computation
>
> Hi Jasvinder,
>
>
> > -----Original Message-----
> > From: Singh, Jasvinder
> > Sent: Tuesday, March 21, 2017 2:46 PM
> > To: dev@dpdk.org
> > Cc: olivier.matz@6wind.com; Doherty, Declan; De Lara Guarch, Pablo
> > Subject: [PATCH v5 2/2] test/test: add unit test for CRC computation
> >
> > This patch provides a set of tests for verifying the functional
> > correctness of 16-bit and 32-bit CRC APIs.
> >
> > Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> > ---
> > test/test/Makefile | 2 +
> > test/test/test_crc.c | 223
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 225 insertions(+)
> > create mode 100644 test/test/test_crc.c
> >
>
> > diff --git a/test/test/test_crc.c b/test/test/test_crc.c new file mode
> > 100644 index 0000000..2eb0bff
> > --- /dev/null
> > +++ b/test/test/test_crc.c
> > @@ -0,0 +1,223 @@
>
>
> > +
> > +#include "test.h"
> > +
> > +#define CRC_VEC_LEN 32
>
> Unnecessary extra tab.
>
> > +#define CRC32_VEC_LEN1 1512
> > +#define CRC32_VEC_LEN2 348
> > +#define CRC16_VEC_LEN1 12
> > +#define CRC16_VEC_LEN2 2
> > +#define LINE_LEN 75
>
> I would align this to the other values above.
>
> ...
>
> > +
> > +/* 16-bit CRC test vector 1*/
>
> Extra space before "*/".
>
> > +static const uint8_t crc16_vec1[CRC16_VEC_LEN1] = {
> > + 0x0D, 0x01, 0x01, 0x23, 0x45, 0x67,
> > + 0x89, 0x01, 0x23, 0x45, 0x00, 0x01,
> > +};
> > +
> > +/* 16-bit CRC test vector 2*/
>
> Same here.
>
> > +static const uint8_t crc16_vec2[CRC16_VEC_LEN2] = {
> > + 0x03, 0x3f,
> > +};
> > +/** CRC results */
> > +uint32_t crc32_vec_res, crc32_vec1_res, crc32_vec2_res; uint32_t
> > +crc16_vec_res, crc16_vec1_res, crc16_vec2_res;
> > +
> > +static int
> > +crc_calc(const uint8_t *vec,
> > + uint32_t vec_len,
> > + enum rte_net_crc_type type)
> > +{
> > + /* compute CRC */
> > + uint32_t ret = rte_net_crc_calc(vec, vec_len, type);
> > +
> > + /* dump data on console*/
>
> Same here.
>
> > + rte_hexdump(stdout, NULL, vec, vec_len);
>
> I would use TEST_HEXDUMP, which dumps the stream only if debug is
> enabled (to avoid too much output).
>
> > +
> > + return ret;
> > +}
> > +
> > +static void
> > +crc_calc_scalar(void)
> > +{
> > + uint32_t i;
> > + enum rte_net_crc_type type;
> > + uint8_t *test_data;
> > +
> > + /* 32-bit ethernet CRC: scalar result */
> > + type = RTE_NET_CRC32_ETH;
> > + crc32_vec_res = crc_calc(crc_vec,
> > + CRC_VEC_LEN,
> > + type);
>
> Remove unnecessary tabs (more of this in the rest of the file).
>
> > +
> > + /* 32-bit ethernet CRC scalar result*/
> > + test_data = rte_zmalloc(NULL, CRC32_VEC_LEN1, 0);
> > +
> > + for (i = 0; i < CRC32_VEC_LEN1; i += 12)
> > + rte_memcpy(&test_data[i], crc32_vec1, 12);
>
> Wrong indentation (more of this in the rest of the file).
> Also, why not just one memcpy call, with CRC32_VEC_LEN1.
> A comment would be good to explain why.
>
> ...
>
> > +static int
> > +test_crc(void) {
>
> Looks strange to start the parameters in a new line.
> I would say you can start in the same line as the function name.
>
> > + uint32_t i;
> > + enum rte_net_crc_type type;
> > + uint8_t *test_data;
> > + uint32_t ret;
> > +
> > + /* set crc scalar mode */
>
> Probably the references of "crc" in comments should be uppercase "CRC".
>
> > + rte_net_crc_set_alg(RTE_NET_CRC_SCALAR);
> > + crc_calc_scalar();
> > +
> > + /* set crc sse4.2 mode */
> > + rte_net_crc_set_alg(RTE_NET_CRC_SSE42);
> > +
> > + /* 32-bit ethernet CRC: Test 1 */
> > + type = RTE_NET_CRC32_ETH;
> > +
> > + ret = crc_calc(crc_vec,
> > + CRC_VEC_LEN,
> > + type);
> > + if (ret != crc32_vec_res) {
> > + printf("test_crc(32-bit): test1 failed !\n");
>
> Extra space before "!" (keep consistency with the other printfs).
>
> > + return -1;
> > + }
> > +
> > + /* 32-bit ethernet CRC: Test 2 */
> > + test_data = rte_zmalloc(NULL, CRC32_VEC_LEN1, 0);
>
> Free the memory after the test (also for other calls).
Will correct the patch and send the next version. Thank you.
Jasvinder
@@ -160,6 +160,8 @@ SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += test_cmdline_cirbuf.c
SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += test_cmdline_string.c
SRCS-$(CONFIG_RTE_LIBRTE_CMDLINE) += test_cmdline_lib.c
+SRCS-$(CONFIG_RTE_LIBRTE_NET) += test_crc.c
+
ifeq ($(CONFIG_RTE_LIBRTE_SCHED),y)
SRCS-y += test_red.c
SRCS-y += test_sched.c
new file mode 100644
@@ -0,0 +1,223 @@
+/*-
+ * BSD LICENSE
+ *
+ * Copyright(c) 2017 Intel Corporation.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <rte_net_crc.h>
+#include <rte_malloc.h>
+#include <rte_hexdump.h>
+
+#include "test.h"
+
+#define CRC_VEC_LEN 32
+#define CRC32_VEC_LEN1 1512
+#define CRC32_VEC_LEN2 348
+#define CRC16_VEC_LEN1 12
+#define CRC16_VEC_LEN2 2
+#define LINE_LEN 75
+
+/* CRC test vector */
+static const uint8_t crc_vec[CRC_VEC_LEN] = {
+ '0', '1', '2', '3', '4', '5', '6', '7',
+ '8', '9', 'a', 'b', 'c', 'd', 'e', 'f',
+ 'g', 'h', 'i', 'j', 'A', 'B', 'C', 'D',
+ 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L',
+};
+
+/* 32-bit CRC test vector */
+static const uint8_t crc32_vec1[12] = {
+ 0xBE, 0xD7, 0x23, 0x47, 0x6B, 0x8F,
+ 0xB3, 0x14, 0x5E, 0xFB, 0x35, 0x59,
+};
+
+/* 16-bit CRC test vector 1*/
+static const uint8_t crc16_vec1[CRC16_VEC_LEN1] = {
+ 0x0D, 0x01, 0x01, 0x23, 0x45, 0x67,
+ 0x89, 0x01, 0x23, 0x45, 0x00, 0x01,
+};
+
+/* 16-bit CRC test vector 2*/
+static const uint8_t crc16_vec2[CRC16_VEC_LEN2] = {
+ 0x03, 0x3f,
+};
+/** CRC results */
+uint32_t crc32_vec_res, crc32_vec1_res, crc32_vec2_res;
+uint32_t crc16_vec_res, crc16_vec1_res, crc16_vec2_res;
+
+static int
+crc_calc(const uint8_t *vec,
+ uint32_t vec_len,
+ enum rte_net_crc_type type)
+{
+ /* compute CRC */
+ uint32_t ret = rte_net_crc_calc(vec, vec_len, type);
+
+ /* dump data on console*/
+ rte_hexdump(stdout, NULL, vec, vec_len);
+
+ return ret;
+}
+
+static void
+crc_calc_scalar(void)
+{
+ uint32_t i;
+ enum rte_net_crc_type type;
+ uint8_t *test_data;
+
+ /* 32-bit ethernet CRC: scalar result */
+ type = RTE_NET_CRC32_ETH;
+ crc32_vec_res = crc_calc(crc_vec,
+ CRC_VEC_LEN,
+ type);
+
+ /* 32-bit ethernet CRC scalar result*/
+ test_data = rte_zmalloc(NULL, CRC32_VEC_LEN1, 0);
+
+ for (i = 0; i < CRC32_VEC_LEN1; i += 12)
+ rte_memcpy(&test_data[i], crc32_vec1, 12);
+
+ crc32_vec1_res = crc_calc(test_data,
+ CRC32_VEC_LEN1,
+ type);
+
+ /* 32-bit ethernet CRC scalar result */
+ test_data = rte_zmalloc(NULL, CRC32_VEC_LEN2, 0);
+
+ for (i = 0; i < CRC32_VEC_LEN2; i += 12)
+ rte_memcpy(&test_data[i], crc32_vec1, 12);
+
+ crc32_vec2_res = crc_calc(test_data,
+ CRC32_VEC_LEN2,
+ type);
+
+ /* 16-bit CCITT CRC scalar result */
+ type = RTE_NET_CRC16_CCITT;
+ crc16_vec_res = crc_calc(crc_vec,
+ CRC_VEC_LEN,
+ type);
+
+ /* 16-bit CCITT CRC scalar result */
+ crc16_vec1_res = crc_calc(crc16_vec1,
+ CRC16_VEC_LEN1,
+ type);
+
+ /* 16-bit CCITT CRC scalar result*/
+ crc16_vec2_res = crc_calc(crc16_vec2,
+ CRC16_VEC_LEN2,
+ type);
+}
+
+static int
+test_crc(void) {
+ uint32_t i;
+ enum rte_net_crc_type type;
+ uint8_t *test_data;
+ uint32_t ret;
+
+ /* set crc scalar mode */
+ rte_net_crc_set_alg(RTE_NET_CRC_SCALAR);
+ crc_calc_scalar();
+
+ /* set crc sse4.2 mode */
+ rte_net_crc_set_alg(RTE_NET_CRC_SSE42);
+
+ /* 32-bit ethernet CRC: Test 1 */
+ type = RTE_NET_CRC32_ETH;
+
+ ret = crc_calc(crc_vec,
+ CRC_VEC_LEN,
+ type);
+ if (ret != crc32_vec_res) {
+ printf("test_crc(32-bit): test1 failed !\n");
+ return -1;
+ }
+
+ /* 32-bit ethernet CRC: Test 2 */
+ test_data = rte_zmalloc(NULL, CRC32_VEC_LEN1, 0);
+
+ for (i = 0; i < CRC32_VEC_LEN1; i += 12)
+ rte_memcpy(&test_data[i], crc32_vec1, 12);
+
+ ret = crc_calc(test_data,
+ CRC32_VEC_LEN1,
+ type);
+ if (ret != crc32_vec1_res) {
+ printf("test_crc(32-bit): test2 failed!\n");
+ return -1;
+ }
+
+ /* 32-bit ethernet CRC: Test 3 */
+ test_data = rte_zmalloc(NULL, CRC32_VEC_LEN2, 0);
+
+ for (i = 0; i < CRC32_VEC_LEN2; i += 12)
+ rte_memcpy(&test_data[i], crc32_vec1, 12);
+
+ ret = crc_calc(test_data,
+ CRC32_VEC_LEN2,
+ type);
+ if (ret != crc32_vec2_res) {
+ printf("test_crc(32-bit): test3 failed!\n");
+ return -1;
+ }
+
+ /* 16-bit CCITT CRC: Test 4 */
+ type = RTE_NET_CRC16_CCITT;
+ ret = crc_calc(crc_vec,
+ CRC_VEC_LEN,
+ type);
+ if (ret != crc16_vec_res) {
+ printf("test_crc (16-bit): test4 failed!\n");
+ return -1;
+ }
+
+ /* 16-bit CCITT CRC: Test 5 */
+ ret = crc_calc(crc16_vec1,
+ CRC16_VEC_LEN1,
+ type);
+ if (ret != crc16_vec1_res) {
+ printf("test_crc (16-bit): test5 failed!\n");
+ return -1;
+ }
+
+ /* 16-bit CCITT CRC: Test 6 */
+ ret = crc_calc(crc16_vec2,
+ CRC16_VEC_LEN2,
+ type);
+ if (ret != crc16_vec2_res) {
+ printf("test_crc (16-bit): test6 failed!\n");
+ return -1;
+ }
+
+ return 0;
+}
+
+REGISTER_TEST_COMMAND(crc_autotest, test_crc);