[PATCH 1/2] raw/skeleton: fix selftest

[PATCH 1/2] raw/skeleton: fix selftest

[David Marchand]

ASan reported issues in this driver.

rte_rawdev_obj_t context object points at a uint16_t.
skeleton_rawdev_enqueue_bufs() and skeleton_rawdev_dequeue_bufs() were
incorrectly casting to an int.

The enqueue/dequeue selftest had a leak on the enqueued string and was
wrong in passing a rte_rawdev_buf pointer array.
Fix this by allocating buffers on the stack and check that returned
string is the expected one.

Bugzilla ID: 1175
Fixes: 61c592a8d035 ("raw/skeleton: introduce skeleton rawdev driver")
Fixes: 55ca1b0f2151 ("raw/skeleton: add test cases")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/raw/skeleton/skeleton_rawdev.c      |  4 +-
 drivers/raw/skeleton/skeleton_rawdev_test.c | 46 +++++++++------------
 2 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c
index 53fe49f936..6e99d35536 100644
--- a/drivers/raw/skeleton/skeleton_rawdev.c
+++ b/drivers/raw/skeleton/skeleton_rawdev.c
@@ -428,7 +428,7 @@ static int skeleton_rawdev_enqueue_bufs(struct rte_rawdev *dev,
 	 * help in complex implementation which require more information than
 	 * just an integer - for example, a queue-pair.
 	 */
-	q_id = *((int *)context);
+	q_id = *((uint16_t *)context);
 
 	for (i = 0; i < count; i++)
 		queue_buf[q_id].bufs[i] = buffers[i]->buf_addr;
@@ -450,7 +450,7 @@ static int skeleton_rawdev_dequeue_bufs(struct rte_rawdev *dev,
 	 * help in complex implementation which require more information than
 	 * just an integer - for example, a queue-pair.
 	 */
-	q_id = *((int *)context);
+	q_id = *((uint16_t *)context);
 
 	for (i = 0; i < count; i++)
 		buffers[i]->buf_addr = queue_buf[q_id].bufs[i];
diff --git a/drivers/raw/skeleton/skeleton_rawdev_test.c b/drivers/raw/skeleton/skeleton_rawdev_test.c
index ca15c49990..b7a7f623aa 100644
--- a/drivers/raw/skeleton/skeleton_rawdev_test.c
+++ b/drivers/raw/skeleton/skeleton_rawdev_test.c
@@ -370,40 +370,34 @@ static int
 test_rawdev_enqdeq(void)
 {
 	int ret;
-	unsigned int count = 1;
 	uint16_t queue_id = 0;
-	struct rte_rawdev_buf buffers[1];
-	struct rte_rawdev_buf *deq_buffers = NULL;
-
-	buffers[0].buf_addr = malloc(strlen(TEST_DEV_NAME) + 3);
-	if (!buffers[0].buf_addr)
-		goto cleanup;
-	snprintf(buffers[0].buf_addr, strlen(TEST_DEV_NAME) + 2, "%s%d",
+	struct rte_rawdev_buf buffer;
+	struct rte_rawdev_buf *buffers[1];
+	struct rte_rawdev_buf deq_buffer;
+	struct rte_rawdev_buf *deq_buffers[1];
+
+	buffers[0] = &buffer;
+	buffer.buf_addr = malloc(strlen(TEST_DEV_NAME) + 3);
+	if (!buffer.buf_addr)
+		return TEST_FAILED;
+	snprintf(buffer.buf_addr, strlen(TEST_DEV_NAME) + 2, "%s%d",
 		 TEST_DEV_NAME, 0);
 
-	ret = rte_rawdev_enqueue_buffers(test_dev_id,
-					 (struct rte_rawdev_buf **)&buffers,
-					 count, &queue_id);
-	RTE_TEST_ASSERT_EQUAL((unsigned int)ret, count,
+	ret = rte_rawdev_enqueue_buffers(test_dev_id, buffers,
+					 RTE_DIM(buffers), &queue_id);
+	RTE_TEST_ASSERT_EQUAL((unsigned int)ret, RTE_DIM(buffers),
 			      "Unable to enqueue buffers");
 
-	deq_buffers = malloc(sizeof(struct rte_rawdev_buf) * count);
-	if (!deq_buffers)
-		goto cleanup;
-
-	ret = rte_rawdev_dequeue_buffers(test_dev_id,
-					(struct rte_rawdev_buf **)&deq_buffers,
-					count, &queue_id);
-	RTE_TEST_ASSERT_EQUAL((unsigned int)ret, count,
+	deq_buffers[0] = &deq_buffer;
+	ret = rte_rawdev_dequeue_buffers(test_dev_id, deq_buffers,
+					RTE_DIM(deq_buffers), &queue_id);
+	RTE_TEST_ASSERT_EQUAL((unsigned int)ret, RTE_DIM(buffers),
 			      "Unable to dequeue buffers");
+	RTE_TEST_ASSERT_EQUAL(deq_buffers[0]->buf_addr, buffers[0]->buf_addr,
+			      "Did not retrieve expected object");
 
-	free(deq_buffers);
-
+	free(buffer.buf_addr);
 	return TEST_SUCCESS;
-cleanup:
-	free(buffers[0].buf_addr);
-
-	return TEST_FAILED;
 }
 
 static void skeldev_test_run(int (*setup)(void),
-- 
2.39.2

[PATCH 2/2] test: add rawdev to fast tests

[David Marchand]

The rawdev unit test can be run in the CI.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/meson.build | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index 5d78814878..6624b478b4 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -315,7 +315,6 @@ driver_test_names = [
         'cryptodev_sw_zuc_autotest',
         'cryptodev_uadk_autotest',
         'dmadev_autotest',
-        'rawdev_autotest',
 ]
 
 dump_test_names = []
@@ -418,6 +417,10 @@ if dpdk_conf.has('RTE_NET_NULL')
     test_sources += 'test_vdev.c'
     fast_tests += [['vdev_autotest', true, true]]
 endif
+if dpdk_conf.has('RTE_RAW_SKELETON')
+    test_deps += 'raw_skeleton'
+    fast_tests += [['rawdev_autotest', true, true]]
+endif
 
 if dpdk_conf.has('RTE_HAS_LIBPCAP')
     ext_deps += pcap_dep
-- 
2.39.2

RE: [PATCH 1/2] raw/skeleton: fix selftest

[Jiang, YuX]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, March 10, 2023 10:26 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Sachin Saxena <sachin.saxena@nxp.com>; Hemant
> Agrawal <hemant.agrawal@nxp.com>; Shreyansh Jain
> <shreyansh.jain@nxp.com>
> Subject: [PATCH 1/2] raw/skeleton: fix selftest
> 
> ASan reported issues in this driver.
> 
> rte_rawdev_obj_t context object points at a uint16_t.
> skeleton_rawdev_enqueue_bufs() and skeleton_rawdev_dequeue_bufs() were
> incorrectly casting to an int.
> 
> The enqueue/dequeue selftest had a leak on the enqueued string and was
> wrong in passing a rte_rawdev_buf pointer array.
> Fix this by allocating buffers on the stack and check that returned string is the
> expected one.
> 
> Bugzilla ID: 1175
> Fixes: 61c592a8d035 ("raw/skeleton: introduce skeleton rawdev driver")
> Fixes: 55ca1b0f2151 ("raw/skeleton: add test cases")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
Tested-by:  Yu Jiang <yux.jiang@intel.com>

Best regards,
Yu Jiang

Re: [PATCH 1/2] raw/skeleton: fix selftest

[David Marchand]

On Fri, Mar 10, 2023 at 3:26 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> ASan reported issues in this driver.
>
> rte_rawdev_obj_t context object points at a uint16_t.
> skeleton_rawdev_enqueue_bufs() and skeleton_rawdev_dequeue_bufs() were
> incorrectly casting to an int.
>
> The enqueue/dequeue selftest had a leak on the enqueued string and was
> wrong in passing a rte_rawdev_buf pointer array.
> Fix this by allocating buffers on the stack and check that returned
> string is the expected one.
>
> Bugzilla ID: 1175

Thomas, just a note if you are the one to apply this one.
I was looking at bugzilla, and this issue was actually reported a
while back, as bz 999, so the commitlog needs some update before
applying.


> Fixes: 61c592a8d035 ("raw/skeleton: introduce skeleton rawdev driver")
> Fixes: 55ca1b0f2151 ("raw/skeleton: add test cases")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand

Re: [PATCH 1/2] raw/skeleton: fix selftest

[Hemant Agrawal]

Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>

Re: [PATCH 2/2] test: add rawdev to fast tests

[Hemant Agrawal]

Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>

Re: [PATCH 1/2] raw/skeleton: fix selftest

[David Marchand]

On Fri, Mar 10, 2023 at 3:26 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> ASan reported issues in this driver.
>
> rte_rawdev_obj_t context object points at a uint16_t.
> skeleton_rawdev_enqueue_bufs() and skeleton_rawdev_dequeue_bufs() were
> incorrectly casting to an int.
>
> The enqueue/dequeue selftest had a leak on the enqueued string and was
> wrong in passing a rte_rawdev_buf pointer array.
> Fix this by allocating buffers on the stack and check that returned
> string is the expected one.
>
Bugzilla ID: 999
> Fixes: 61c592a8d035 ("raw/skeleton: introduce skeleton rawdev driver")
> Fixes: 55ca1b0f2151 ("raw/skeleton: add test cases")
> Cc: stable@dpdk.org
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Series applied.


-- 
David Marchand