From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 067AE432B5; Mon, 6 Nov 2023 07:44:02 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BB6E9402B5; Mon, 6 Nov 2023 07:44:01 +0100 (CET) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id 66069402BA for ; Mon, 6 Nov 2023 07:43:59 +0100 (CET) Received: from dggpeml100024.china.huawei.com (unknown [172.30.72.53]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4SP1tB74VmzPnnl; Mon, 6 Nov 2023 14:39:46 +0800 (CST) Received: from [10.67.121.161] (10.67.121.161) by dggpeml100024.china.huawei.com (7.185.36.115) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.31; Mon, 6 Nov 2023 14:43:20 +0800 Subject: Re: [PATCH v3 3/3] test/dma: add SG copy tests To: Gowrishankar Muthukrishnan , CC: , Vamsi Attunuru , Amit Prakash Shukla , Vidya Sagar Velumuri , Kevin Laatz , Bruce Richardson References: <7c61824f1ed1e720c974d1baadfa7c55578c0c4f.1699025423.git.gmuthukrishn@marvell.com> From: fengchengwen Message-ID: Date: Mon, 6 Nov 2023 14:43:20 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <7c61824f1ed1e720c974d1baadfa7c55578c0c4f.1699025423.git.gmuthukrishn@marvell.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.121.161] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) To dggpeml100024.china.huawei.com (7.185.36.115) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi Gowrishankar, On 2023/11/3 23:38, Gowrishankar Muthukrishnan wrote: > Add scatter-gather copy tests. > > Signed-off-by: Vidya Sagar Velumuri > Signed-off-by: Gowrishankar Muthukrishnan > --- > app/test/test_dmadev.c | 132 +++++++++++++++++++++++++++++- > app/test/test_dmadev_api.c | 163 ++++++++++++++++++++++++++++++++++--- > app/test/test_dmadev_api.h | 2 + > 3 files changed, 283 insertions(+), 14 deletions(-) > > diff --git a/app/test/test_dmadev.c b/app/test/test_dmadev.c > index 780941fc1e..a2f3a7f999 100644 > --- a/app/test/test_dmadev.c > +++ b/app/test/test_dmadev.c > @@ -19,7 +19,7 @@ > #define ERR_RETURN(...) do { print_err(__func__, __LINE__, __VA_ARGS__); return -1; } while (0) > > #define TEST_RINGSIZE 512 > -#define COPY_LEN 1024 > +#define COPY_LEN 1032 The test MAX_SG_NUM is limit 4, so it could be 1/2/3/4 segment, and 1032 can both div 1/2/3/4, but 1024 couldn't I think this is why change 1024->1032. Suggest add some comment about it. > > static struct rte_mempool *pool; > static int16_t test_dev_id; > @@ -396,6 +396,120 @@ test_stop_start(int16_t dev_id, uint16_t vchan) > return 0; > } > > +static int > +test_enqueue_sg_copies(int16_t dev_id, uint16_t vchan) > +{ > + unsigned int src_len, dst_len, n_sge, len, i, j, k; > + char orig_src[COPY_LEN], orig_dst[COPY_LEN]; > + struct rte_dma_info info = { 0 }; > + enum rte_dma_status_code status; > + uint16_t id, n_src, n_dst; > + > + if (rte_dma_info_get(dev_id, &info) < 0) > + ERR_RETURN("Failed to get dev info"); > + > + n_sge = RTE_MIN(info.max_sges, TEST_SG_MAX); > + len = COPY_LEN; > + > + for (n_src = 1; n_src <= n_sge; n_src++) { > + src_len = len / n_src; > + for (n_dst = 1; n_dst <= n_sge; n_dst++) { > + dst_len = len / n_dst; If the len % [1~n_dst] not zero, how to process it ? I see, it was ensured by adjust copy_len value. Suggest add comments for it. Also suggest extra above to one function. > + > + struct rte_dma_sge sg_src[n_sge], sg_dst[n_sge]; > + struct rte_mbuf *src[n_sge], *dst[n_sge]; > + char *src_data[n_sge], *dst_data[n_sge]; > + > + for (i = 0 ; i < COPY_LEN; i++) > + orig_src[i] = rte_rand() & 0xFF; > + > + memset(orig_dst, 0, COPY_LEN); > + > + for (i = 0; i < n_src; i++) { > + src[i] = rte_pktmbuf_alloc(pool); > + RTE_ASSERT(src[i] != NULL); > + sg_src[i].addr = rte_pktmbuf_iova(src[i]); > + sg_src[i].length = src_len; > + src_data[i] = rte_pktmbuf_mtod(src[i], char *); > + } > + > + for (k = 0; k < n_dst; k++) { > + dst[k] = rte_pktmbuf_alloc(pool); > + RTE_ASSERT(dst[k] != NULL); > + sg_dst[k].addr = rte_pktmbuf_iova(dst[k]); > + sg_dst[k].length = dst_len; > + dst_data[k] = rte_pktmbuf_mtod(dst[k], char *); > + } > + > + for (i = 0; i < n_src; i++) { > + for (j = 0; j < src_len; j++) > + src_data[i][j] = orig_src[i * src_len + j]; > + } > + > + for (k = 0; k < n_dst; k++) > + memset(dst_data[k], 0, dst_len); > + > + printf("\tsrc segs: %2d [seg len: %4d] - dst segs: %2d [seg len : %4d]\n", > + n_src, src_len, n_dst, dst_len); > + > + id = rte_dma_copy_sg(dev_id, vchan, sg_src, sg_dst, n_src, n_dst, > + RTE_DMA_OP_FLAG_SUBMIT); > + > + if (id != id_count) > + ERR_RETURN("Error with rte_dma_copy_sg, got %u, expected %u\n", > + id, id_count); > + > + /* Give time for copy to finish, then check it was done */ > + await_hw(dev_id, vchan); > + > + for (k = 0; k < n_dst; k++) > + memcpy((&orig_dst[0] + k * dst_len), dst_data[k], dst_len); > + > + if (memcmp(orig_src, orig_dst, COPY_LEN)) > + ERR_RETURN("Data mismatch"); > + > + /* Verify completion */ > + id = ~id; > + if (rte_dma_completed(dev_id, vchan, 1, &id, NULL) != 1) > + ERR_RETURN("Error with rte_dma_completed\n"); > + > + /* Verify expected index(id_count) */ > + if (id != id_count) > + ERR_RETURN("Error:incorrect job id received, %u [expected %u]\n", > + id, id_count); > + > + /* Check for completed and id when no job done */ > + id = ~id; > + if (rte_dma_completed(dev_id, vchan, 1, &id, NULL) != 0) > + ERR_RETURN("Error with rte_dma_completed when no job done\n"); > + > + if (id != id_count) > + ERR_RETURN("Error:incorrect job id received when no job done, %u [expected %u]\n", > + id, id_count); > + > + /* Check for completed_status and id when no job done */ > + id = ~id; > + if (rte_dma_completed_status(dev_id, vchan, 1, &id, &status) != 0) > + ERR_RETURN("Error with rte_dma_completed_status when no job done\n"); > + if (id != id_count) > + ERR_RETURN("Error:incorrect job id received when no job done, %u [expected %u]\n", > + id, 0); > + > + for (i = 0; i < n_src; i++) > + rte_pktmbuf_free(src[i]); > + for (i = 0; i < n_dst; i++) > + rte_pktmbuf_free(dst[i]); > + > + /* Verify that completion returns nothing more */ > + if (rte_dma_completed(dev_id, 0, 1, NULL, NULL) != 0) > + ERR_RETURN("Error with rte_dma_completed in empty check\n"); already verify this, no need do more > + > + id_count++; > + } > + } > + return 0; > +} > + > /* Failure handling test cases - global macros and variables for those tests*/ > #define COMP_BURST_SZ 16 > #define OPT_FENCE(idx) ((fence && idx == 8) ? RTE_DMA_OP_FLAG_FENCE : 0) > @@ -1003,7 +1117,7 @@ test_dmadev_setup(void) > TEST_RINGSIZE * 2, /* n == num elements */ > 32, /* cache size */ > 0, /* priv size */ > - 2048, /* data room size */ > + COPY_LEN + RTE_PKTMBUF_HEADROOM, /* data room size */ > info.numa_node); > if (pool == NULL) > ERR_RETURN("Error with mempool creation\n"); > @@ -1026,6 +1140,7 @@ test_dmadev_instance(int16_t dev_id) > #define CHECK_ERRS true > enum { > TEST_COPY = 0, > + TEST_COPY_SG, > TEST_START, > TEST_BURST, > TEST_ERR, > @@ -1060,6 +1175,13 @@ test_dmadev_instance(int16_t dev_id) > param[TEST_COPY].vchan = vchan; > param[TEST_COPY].check_err_stats = CHECK_ERRS; > > + param[TEST_COPY_SG].printable = "copy"; should be SG copy. > + param[TEST_COPY_SG].test_fn = test_enqueue_sg_copies; > + param[TEST_COPY_SG].iterations = 1; > + param[TEST_COPY_SG].dev_id = dev_id; > + param[TEST_COPY_SG].vchan = vchan; > + param[TEST_COPY_SG].check_err_stats = CHECK_ERRS; > + > param[TEST_START].printable = "stop-start"; > param[TEST_START].test_fn = test_stop_start; > param[TEST_START].iterations = 1; > @@ -1122,6 +1244,12 @@ test_dmadev_instance(int16_t dev_id) > /* run tests stopping/starting devices and check jobs still work after restart */ > ts->unit_test_cases[TEST_START].enabled = 1; > > + /* run SG test cases */ > + if ((dev_info.dev_capa & RTE_DMA_CAPA_OPS_COPY_SG) == 0) > + printf("DMA Dev %u: No SG support, skipping SG copy tests\n", dev_id); > + else > + ts->unit_test_cases[TEST_COPY_SG].enabled = 1; suggest wrap it as test_dmadev_sg_copy_setup, just like test_dmadev_burst_setup > + > /* run some burst capacity tests */ > ts->unit_test_cases[TEST_BURST].setup = test_dmadev_burst_setup; > ts->unit_test_cases[TEST_BURST].enabled = 1; > diff --git a/app/test/test_dmadev_api.c b/app/test/test_dmadev_api.c > index aa07d2b359..37e43c9336 100644 > --- a/app/test/test_dmadev_api.c > +++ b/app/test/test_dmadev_api.c > @@ -10,6 +10,7 @@ > #include > > #include "test.h" > +#include "test_dmadev_api.h" > > extern int test_dma_api(uint16_t dev_id); > > @@ -21,36 +22,62 @@ static int16_t invalid_dev_id; > > static char *src; > static char *dst; > +static char *src_sg[TEST_SG_MAX]; > +static char *dst_sg[TEST_SG_MAX]; > > static int > testsuite_setup(void) > { > invalid_dev_id = -1; > - > - src = rte_malloc("dmadev_test_src", TEST_MEMCPY_SIZE, 0); > - if (src == NULL) > - return -ENOMEM; > - dst = rte_malloc("dmadev_test_dst", TEST_MEMCPY_SIZE, 0); > - if (dst == NULL) { > - rte_free(src); > - src = NULL; > - return -ENOMEM; > + int i, rc = 0; > + > + for (i = 0; i < TEST_SG_MAX; i++) { > + src_sg[i] = rte_malloc("dmadev_test_src", TEST_MEMCPY_SIZE, 0); > + if (src_sg[i] == NULL) { > + rc = -ENOMEM; > + goto exit; > + } > + > + dst_sg[i] = rte_malloc("dmadev_test_dst", TEST_MEMCPY_SIZE, 0); > + if (dst_sg[i] == NULL) { > + rte_free(src_sg[i]); > + src_sg[i] = NULL; > + rc = -ENOMEM; > + goto exit; > + } > } > > + src = src_sg[0]; > + dst = dst_sg[0]; > + > /* Set dmadev log level to critical to suppress unnecessary output > * during API tests. > */ > rte_log_set_level_pattern("lib.dmadev", RTE_LOG_CRIT); > > - return 0; > + return rc; > +exit: > + while (--i >= 0) { > + rte_free(src_sg[i]); > + rte_free(dst_sg[i]); > + } > + > + return rc; > } > > static void > testsuite_teardown(void) > { > - rte_free(src); > + int i; > + > + for (i = 0; i < TEST_SG_MAX; i++) { > + rte_free(src_sg[i]); > + src_sg[i] = NULL; > + rte_free(dst_sg[i]); > + dst_sg[i] = NULL; > + } > + > src = NULL; > - rte_free(dst); > dst = NULL; > /* Ensure the dmadev is stopped. */ > rte_dma_stop(test_dev_id); > @@ -437,6 +464,37 @@ verify_memory(void) > return 0; > } > > +static void > +sg_memory_setup(int n) > +{ > + int i, j; > + > + for (i = 0; i < n; i++) { > + for (j = 0; j < TEST_MEMCPY_SIZE; j++) > + src_sg[i][j] = (char)j; > + > + memset(dst_sg[i], 0, TEST_MEMCPY_SIZE); > + } > +} > + > +static int > +sg_memory_verify(int n) > +{ > + int i, j; > + > + for (i = 0; i < n; i++) { > + for (j = 0; j < TEST_MEMCPY_SIZE; j++) { > + if (src_sg[i][j] == dst_sg[i][j]) > + continue; > + > + RTE_TEST_ASSERT_EQUAL(src_sg[i][j], dst_sg[i][j], "Failed to copy memory, %d %d", > + src_sg[i][j], dst_sg[i][j]); > + } > + } > + > + return 0; > +} > + > static int > test_dma_completed(void) > { > @@ -551,6 +609,86 @@ test_dma_completed_status(void) > return TEST_SUCCESS; > } > > +static int > +test_dma_sg(void) > +{ > + struct rte_dma_sge src_sge[TEST_SG_MAX], dst_sge[TEST_SG_MAX]; > + struct rte_dma_info dev_info = { 0 }; > + uint16_t last_idx = -1; > + bool has_error = true; > + int n_sge, i, ret; > + uint16_t cpl_ret; > + > + ret = rte_dma_info_get(test_dev_id, &dev_info); > + RTE_TEST_ASSERT_SUCCESS(ret, "Failed to obtain device info, %d", ret); > + > + if ((dev_info.dev_capa & RTE_DMA_CAPA_OPS_COPY_SG) == 0) > + return TEST_SKIPPED; > + > + n_sge = RTE_MIN(dev_info.max_sges, TEST_SG_MAX); > + > + ret = setup_vchan(1); > + RTE_TEST_ASSERT_SUCCESS(ret, "Failed to setup one vchan, %d", ret); > + > + ret = rte_dma_start(test_dev_id); > + RTE_TEST_ASSERT_SUCCESS(ret, "Failed to start, %d", ret); > + > + for (i = 0; i < n_sge; i++) { > + src_sge[i].addr = rte_malloc_virt2iova(src_sg[i]); > + src_sge[i].length = TEST_MEMCPY_SIZE; > + dst_sge[i].addr = rte_malloc_virt2iova(dst_sg[i]); > + dst_sge[i].length = TEST_MEMCPY_SIZE; > + } > + > + sg_memory_setup(n_sge); > + > + /* Check enqueue without submit */ > + ret = rte_dma_copy_sg(test_dev_id, 0, src_sge, dst_sge, n_sge, n_sge, 0); > + RTE_TEST_ASSERT_EQUAL(ret, 0, "Failed to enqueue copy, %d", ret); > + > + rte_delay_us_sleep(TEST_WAIT_US_VAL); > + > + cpl_ret = rte_dma_completed(test_dev_id, 0, 1, &last_idx, &has_error); > + RTE_TEST_ASSERT_EQUAL(cpl_ret, 0, "Failed to get completed"); > + > + /* Check DMA submit */ > + ret = rte_dma_submit(test_dev_id, 0); > + RTE_TEST_ASSERT_SUCCESS(ret, "Failed to submit, %d", ret); > + > + rte_delay_us_sleep(TEST_WAIT_US_VAL); > + > + cpl_ret = rte_dma_completed(test_dev_id, 0, 1, &last_idx, &has_error); > + RTE_TEST_ASSERT_EQUAL(cpl_ret, 1, "Failed to get completed"); > + RTE_TEST_ASSERT_EQUAL(last_idx, 0, "Last idx should be zero, %u", last_idx); > + RTE_TEST_ASSERT_EQUAL(has_error, false, "Should have no error"); > + > + ret = sg_memory_verify(n_sge); > + RTE_TEST_ASSERT_SUCCESS(ret, "Failed to verify memory"); > + > + sg_memory_setup(n_sge); > + > + /* Check for enqueue with submit */ > + ret = rte_dma_copy_sg(test_dev_id, 0, src_sge, dst_sge, n_sge, n_sge, > + RTE_DMA_OP_FLAG_SUBMIT); > + RTE_TEST_ASSERT_EQUAL(ret, 1, "Failed to enqueue copy, %d", ret); > + > + rte_delay_us_sleep(TEST_WAIT_US_VAL); > + > + cpl_ret = rte_dma_completed(test_dev_id, 0, 1, &last_idx, &has_error); > + RTE_TEST_ASSERT_EQUAL(cpl_ret, 1, "Failed to get completed"); > + RTE_TEST_ASSERT_EQUAL(last_idx, 1, "Last idx should be 1, %u", last_idx); > + RTE_TEST_ASSERT_EQUAL(has_error, false, "Should have no error"); > + > + ret = sg_memory_verify(n_sge); > + RTE_TEST_ASSERT_SUCCESS(ret, "Failed to verify memory"); > + > + /* Stop dmadev to make sure dmadev to a known state */ > + ret = rte_dma_stop(test_dev_id); > + RTE_TEST_ASSERT_SUCCESS(ret, "Failed to stop, %d", ret); > + > + return TEST_SUCCESS; > +} > + > static struct unit_test_suite dma_api_testsuite = { > .suite_name = "DMA API Test Suite", > .setup = testsuite_setup, > @@ -568,6 +706,7 @@ static struct unit_test_suite dma_api_testsuite = { > TEST_CASE(test_dma_dump), > TEST_CASE(test_dma_completed), > TEST_CASE(test_dma_completed_status), > + TEST_CASE(test_dma_sg), > TEST_CASES_END() > } > }; > diff --git a/app/test/test_dmadev_api.h b/app/test/test_dmadev_api.h > index 33fbc5bd41..10ab925f80 100644 > --- a/app/test/test_dmadev_api.h > +++ b/app/test/test_dmadev_api.h > @@ -2,4 +2,6 @@ > * Copyright(c) 2021 HiSilicon Limited > */ > > +#define TEST_SG_MAX 4 suggest TEST_MAX_SGES which corresponding dev_info.max_sges Thanks Chengwen > + > int test_dma_api(uint16_t dev_id); >