[3/3] test/reorder: add cases to cover new API

Message ID 20230120102146.4035460-4-vfialko@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series reorder: introduce new APIs |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/github-robot: build fail github build: failed
ci/iol-aarch64-unit-testing fail Testing issues
ci/iol-testing fail Testing issues
ci/iol-x86_64-unit-testing fail Testing issues
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing fail Unit Testing FAIL

Commit Message

Volodymyr Fialko Jan. 20, 2023, 10:21 a.m. UTC
  Add new test cases to cover `rte_reorder_drain_up_to_seqn` and
`rte_reorder_min_seqn_set`.

Signed-off-by: Volodymyr Fialko <vfialko@marvell.com>
---
 app/test/test_reorder.c | 160 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 160 insertions(+)
  

Comments

Volodymyr Fialko Jan. 20, 2023, 12:43 p.m. UTC | #1
Depends-on: series-26425 ("lib/reorder: fix drain/free issues")

Test failure is caused by this bug in drain/api test - http://patches.dpdk.org/project/dpdk/list/?series=26425
It wasn't seen before seen before because drain was the last test, but now following test receive pointer to same packet twice due to double free.

> -----Original Message-----
> From: Volodymyr Fialko <vfialko@marvell.com>
> Sent: Friday, January 20, 2023 11:22 AM
> To: dev@dpdk.org; Reshma Pattan <reshma.pattan@intel.com>
> Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Anoob Joseph <anoobj@marvell.com>;
> Volodymyr Fialko <vfialko@marvell.com>
> Subject: [PATCH 3/3] test/reorder: add cases to cover new API
> 
> Add new test cases to cover `rte_reorder_drain_up_to_seqn` and `rte_reorder_min_seqn_set`.
> 
> Signed-off-by: Volodymyr Fialko <vfialko@marvell.com>
> ---
>  app/test/test_reorder.c | 160 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 160 insertions(+)
> 
> diff --git a/app/test/test_reorder.c b/app/test/test_reorder.c index f0714a5c18..c345a72e0c 100644
> --- a/app/test/test_reorder.c
> +++ b/app/test/test_reorder.c
> @@ -335,6 +335,164 @@ test_reorder_drain(void)
>  	return ret;
>  }
> 
> +static void
> +buffer_to_reorder_move(struct rte_mbuf **mbuf, struct
> +rte_reorder_buffer *b) {
> +	rte_reorder_insert(b, *mbuf);
> +	*mbuf = NULL;
> +}
> +
> +static int
> +test_reorder_drain_up_to_seqn(void)
> +{
> +	struct rte_mempool *p = test_params->p;
> +	struct rte_reorder_buffer *b = NULL;
> +	const unsigned int num_bufs = 10;
> +	const unsigned int size = 4;
> +	unsigned int i, cnt;
> +	int ret = 0;
> +
> +	struct rte_mbuf *bufs[num_bufs];
> +	struct rte_mbuf *robufs[num_bufs];
> +
> +	/* initialize all robufs to NULL */
> +	memset(robufs, 0, sizeof(robufs));
> +
> +	/* This would create a reorder buffer instance consisting of:
> +	 * reorder_seq = 0
> +	 * ready_buf: RB[size] = {NULL, NULL, NULL, NULL}
> +	 * order_buf: OB[size] = {NULL, NULL, NULL, NULL}
> +	 */
> +	b = rte_reorder_create("test_drain_up_to_seqn", rte_socket_id(), size);
> +	TEST_ASSERT_NOT_NULL(b, "Failed to create reorder buffer");
> +
> +	for (i = 0; i < num_bufs; i++) {
> +		bufs[i] = rte_pktmbuf_alloc(p);
> +		TEST_ASSERT_NOT_NULL(bufs[i], "Packet allocation failed\n");
> +		*rte_reorder_seqn(bufs[i]) = i;
> +	}
> +
> +	/* Insert packet with seqn 1 and 3:
> +	 * RB[] = {NULL, NULL, NULL, NULL}
> +	 * OB[] = {1, 2, 3, NULL}
> +	 */
> +	buffer_to_reorder_move(&bufs[1], b);
> +	buffer_to_reorder_move(&bufs[2], b);
> +	buffer_to_reorder_move(&bufs[3], b);
> +	/* Draining 1, 2 */
> +	cnt = rte_reorder_drain_up_to_seqn(b, robufs, num_bufs, 3);
> +	if (cnt != 2) {
> +		printf("%s:%d:%d: number of expected packets not drained\n",
> +				__func__, __LINE__, cnt);
> +		ret = -1;
> +		goto exit;
> +	}
> +	for (i = 0; i < 2; i++)
> +		rte_pktmbuf_free(robufs[i]);
> +	memset(robufs, 0, sizeof(robufs));
> +
> +	/* Insert more packets
> +	 * RB[] = {NULL, NULL, NULL, NULL}
> +	 * OB[] = {3, 4, NULL, 6}
> +	 */
> +	buffer_to_reorder_move(&bufs[4], b);
> +	buffer_to_reorder_move(&bufs[6], b);
> +	/* Insert more packets to utilize Ready buffer
> +	 * RB[] = {3, NULL, 5, 6}
> +	 * OB[] = {NULL, NULL, 8, NULL}
> +	 */
> +	buffer_to_reorder_move(&bufs[8], b);
> +
> +	/* Drain 3 and 5 */
> +	cnt = rte_reorder_drain_up_to_seqn(b, robufs, num_bufs, 6);
> +	if (cnt != 2) {
> +		printf("%s:%d:%d: number of expected packets not drained\n",
> +				__func__, __LINE__, cnt);
> +		ret = -1;
> +		goto exit;
> +	}
> +	for (i = 0; i < 2; i++)
> +		rte_pktmbuf_free(robufs[i]);
> +	memset(robufs, 0, sizeof(robufs));
> +
> +	ret = 0;
> +exit:
> +	rte_reorder_free(b);
> +	for (i = 0; i < num_bufs; i++) {
> +		rte_pktmbuf_free(bufs[i]);
> +		rte_pktmbuf_free(robufs[i]);
> +	}
> +	return ret;
> +}
> +
> +static int
> +test_reorder_set_seqn(void)
> +{
> +	struct rte_mempool *p = test_params->p;
> +	struct rte_reorder_buffer *b = NULL;
> +	const unsigned int num_bufs = 7;
> +	const unsigned int size = 4;
> +	unsigned int i;
> +	int ret = 0;
> +
> +	struct rte_mbuf *bufs[num_bufs];
> +
> +	/* This would create a reorder buffer instance consisting of:
> +	 * reorder_seq = 0
> +	 * ready_buf: RB[size] = {NULL, NULL, NULL, NULL}
> +	 * order_buf: OB[size] = {NULL, NULL, NULL, NULL}
> +	 */
> +	b = rte_reorder_create("test_min_seqn_set", rte_socket_id(), size);
> +	TEST_ASSERT_NOT_NULL(b, "Failed to create reorder buffer");
> +
> +	for (i = 0; i < num_bufs; i++) {
> +		bufs[i] = rte_pktmbuf_alloc(p);
> +		if (bufs[i] == NULL) {
> +			printf("Packet allocation failed\n");
> +			goto exit;
> +		}
> +		*rte_reorder_seqn(bufs[i]) = i;
> +	}
> +
> +	ret = rte_reorder_min_seqn_set(b, 5);
> +	if (ret != 0) {
> +		printf("%s:%d: Error in setting min sequence number\n", __func__, __LINE__);
> +		ret = -1;
> +		goto exit;
> +	}
> +
> +	ret = rte_reorder_insert(b, bufs[0]);
> +	if (ret >= 0) {
> +		printf("%s:%d: Insertion with value less the min seq number\n", __func__, __LINE__);
> +		ret = -1;
> +		goto exit;
> +	}
> +
> +	ret = rte_reorder_insert(b, bufs[5]);
> +	if (ret != 0) {
> +		printf("%s:%d: Error inserting packet with valid seqn\n", __func__, __LINE__);
> +		ret = -1;
> +		goto exit;
> +	}
> +	bufs[5] = NULL;
> +
> +	ret = rte_reorder_min_seqn_set(b, 0);
> +	if (ret >= 0) {
> +		printf("%s:%d: Error in setting min sequence number with non-empty buffer\n",
> +				__func__, __LINE__);
> +		ret = -1;
> +		goto exit;
> +	}
> +
> +	ret = 0;
> +exit:
> +	rte_reorder_free(b);
> +	for (i = 0; i < num_bufs; i++)
> +		rte_pktmbuf_free(bufs[i]);
> +
> +	return ret;
> +}
> +
>  static int
>  test_setup(void)
>  {
> @@ -385,6 +543,8 @@ static struct unit_test_suite reorder_test_suite  = {
>  		TEST_CASE(test_reorder_free),
>  		TEST_CASE(test_reorder_insert),
>  		TEST_CASE(test_reorder_drain),
> +		TEST_CASE(test_reorder_drain_up_to_seqn),
> +		TEST_CASE(test_reorder_set_seqn),
>  		TEST_CASES_END()
>  	}
>  };
> --
> 2.34.1
  
Thomas Monjalon Feb. 19, 2023, 11:32 p.m. UTC | #2
20/01/2023 11:21, Volodymyr Fialko:
> Add new test cases to cover `rte_reorder_drain_up_to_seqn` and
> `rte_reorder_min_seqn_set`.

Instead of doing a separate patch, please squash the tests
when adding the new API functions in patches 1 & 2.
  

Patch

diff --git a/app/test/test_reorder.c b/app/test/test_reorder.c
index f0714a5c18..c345a72e0c 100644
--- a/app/test/test_reorder.c
+++ b/app/test/test_reorder.c
@@ -335,6 +335,164 @@  test_reorder_drain(void)
 	return ret;
 }
 
+static void
+buffer_to_reorder_move(struct rte_mbuf **mbuf, struct rte_reorder_buffer *b)
+{
+	rte_reorder_insert(b, *mbuf);
+	*mbuf = NULL;
+}
+
+static int
+test_reorder_drain_up_to_seqn(void)
+{
+	struct rte_mempool *p = test_params->p;
+	struct rte_reorder_buffer *b = NULL;
+	const unsigned int num_bufs = 10;
+	const unsigned int size = 4;
+	unsigned int i, cnt;
+	int ret = 0;
+
+	struct rte_mbuf *bufs[num_bufs];
+	struct rte_mbuf *robufs[num_bufs];
+
+	/* initialize all robufs to NULL */
+	memset(robufs, 0, sizeof(robufs));
+
+	/* This would create a reorder buffer instance consisting of:
+	 * reorder_seq = 0
+	 * ready_buf: RB[size] = {NULL, NULL, NULL, NULL}
+	 * order_buf: OB[size] = {NULL, NULL, NULL, NULL}
+	 */
+	b = rte_reorder_create("test_drain_up_to_seqn", rte_socket_id(), size);
+	TEST_ASSERT_NOT_NULL(b, "Failed to create reorder buffer");
+
+	for (i = 0; i < num_bufs; i++) {
+		bufs[i] = rte_pktmbuf_alloc(p);
+		TEST_ASSERT_NOT_NULL(bufs[i], "Packet allocation failed\n");
+		*rte_reorder_seqn(bufs[i]) = i;
+	}
+
+	/* Insert packet with seqn 1 and 3:
+	 * RB[] = {NULL, NULL, NULL, NULL}
+	 * OB[] = {1, 2, 3, NULL}
+	 */
+	buffer_to_reorder_move(&bufs[1], b);
+	buffer_to_reorder_move(&bufs[2], b);
+	buffer_to_reorder_move(&bufs[3], b);
+	/* Draining 1, 2 */
+	cnt = rte_reorder_drain_up_to_seqn(b, robufs, num_bufs, 3);
+	if (cnt != 2) {
+		printf("%s:%d:%d: number of expected packets not drained\n",
+				__func__, __LINE__, cnt);
+		ret = -1;
+		goto exit;
+	}
+	for (i = 0; i < 2; i++)
+		rte_pktmbuf_free(robufs[i]);
+	memset(robufs, 0, sizeof(robufs));
+
+	/* Insert more packets
+	 * RB[] = {NULL, NULL, NULL, NULL}
+	 * OB[] = {3, 4, NULL, 6}
+	 */
+	buffer_to_reorder_move(&bufs[4], b);
+	buffer_to_reorder_move(&bufs[6], b);
+	/* Insert more packets to utilize Ready buffer
+	 * RB[] = {3, NULL, 5, 6}
+	 * OB[] = {NULL, NULL, 8, NULL}
+	 */
+	buffer_to_reorder_move(&bufs[8], b);
+
+	/* Drain 3 and 5 */
+	cnt = rte_reorder_drain_up_to_seqn(b, robufs, num_bufs, 6);
+	if (cnt != 2) {
+		printf("%s:%d:%d: number of expected packets not drained\n",
+				__func__, __LINE__, cnt);
+		ret = -1;
+		goto exit;
+	}
+	for (i = 0; i < 2; i++)
+		rte_pktmbuf_free(robufs[i]);
+	memset(robufs, 0, sizeof(robufs));
+
+	ret = 0;
+exit:
+	rte_reorder_free(b);
+	for (i = 0; i < num_bufs; i++) {
+		rte_pktmbuf_free(bufs[i]);
+		rte_pktmbuf_free(robufs[i]);
+	}
+	return ret;
+}
+
+static int
+test_reorder_set_seqn(void)
+{
+	struct rte_mempool *p = test_params->p;
+	struct rte_reorder_buffer *b = NULL;
+	const unsigned int num_bufs = 7;
+	const unsigned int size = 4;
+	unsigned int i;
+	int ret = 0;
+
+	struct rte_mbuf *bufs[num_bufs];
+
+	/* This would create a reorder buffer instance consisting of:
+	 * reorder_seq = 0
+	 * ready_buf: RB[size] = {NULL, NULL, NULL, NULL}
+	 * order_buf: OB[size] = {NULL, NULL, NULL, NULL}
+	 */
+	b = rte_reorder_create("test_min_seqn_set", rte_socket_id(), size);
+	TEST_ASSERT_NOT_NULL(b, "Failed to create reorder buffer");
+
+	for (i = 0; i < num_bufs; i++) {
+		bufs[i] = rte_pktmbuf_alloc(p);
+		if (bufs[i] == NULL) {
+			printf("Packet allocation failed\n");
+			goto exit;
+		}
+		*rte_reorder_seqn(bufs[i]) = i;
+	}
+
+	ret = rte_reorder_min_seqn_set(b, 5);
+	if (ret != 0) {
+		printf("%s:%d: Error in setting min sequence number\n", __func__, __LINE__);
+		ret = -1;
+		goto exit;
+	}
+
+	ret = rte_reorder_insert(b, bufs[0]);
+	if (ret >= 0) {
+		printf("%s:%d: Insertion with value less the min seq number\n", __func__, __LINE__);
+		ret = -1;
+		goto exit;
+	}
+
+	ret = rte_reorder_insert(b, bufs[5]);
+	if (ret != 0) {
+		printf("%s:%d: Error inserting packet with valid seqn\n", __func__, __LINE__);
+		ret = -1;
+		goto exit;
+	}
+	bufs[5] = NULL;
+
+	ret = rte_reorder_min_seqn_set(b, 0);
+	if (ret >= 0) {
+		printf("%s:%d: Error in setting min sequence number with non-empty buffer\n",
+				__func__, __LINE__);
+		ret = -1;
+		goto exit;
+	}
+
+	ret = 0;
+exit:
+	rte_reorder_free(b);
+	for (i = 0; i < num_bufs; i++)
+		rte_pktmbuf_free(bufs[i]);
+
+	return ret;
+}
+
 static int
 test_setup(void)
 {
@@ -385,6 +543,8 @@  static struct unit_test_suite reorder_test_suite  = {
 		TEST_CASE(test_reorder_free),
 		TEST_CASE(test_reorder_insert),
 		TEST_CASE(test_reorder_drain),
+		TEST_CASE(test_reorder_drain_up_to_seqn),
+		TEST_CASE(test_reorder_set_seqn),
 		TEST_CASES_END()
 	}
 };