From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id AF3141B4BE for ; Wed, 9 Jan 2019 17:47:50 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 09 Jan 2019 08:47:49 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,458,1539673200"; d="scan'208";a="134428283" Received: from irsmsx151.ger.corp.intel.com ([163.33.192.59]) by fmsmga004.fm.intel.com with ESMTP; 09 Jan 2019 08:47:48 -0800 Received: from irsmsx112.ger.corp.intel.com ([169.254.1.84]) by IRSMSX151.ger.corp.intel.com ([169.254.4.172]) with mapi id 14.03.0415.000; Wed, 9 Jan 2019 16:47:43 +0000 From: "De Lara Guarch, Pablo" To: "Kovacevic, Marko" CC: "akhil.goyal@nxp.com" , "Daly, Lee" , "Jozwiak, TomaszX" , "O'Hare, Cathal" , "Trahe, Fiona" , "Kovacevic, Marko" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v2 1/2] test/compress: add out of space test Thread-Index: AQHUmHSHIBIsvqvHMkKzIJxn87aWAaWm9scA Date: Wed, 9 Jan 2019 16:47:42 +0000 Message-ID: References: <20181214153326.17356-1-marko.kovacevic@intel.com> <20181220145824.37223-1-marko.kovacevic@intel.com> In-Reply-To: <20181220145824.37223-1-marko.kovacevic@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZTQzMDQ2ZDUtNWMzNy00ZGJjLWI3MWYtZGU4NzgzMDlhN2UzIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiTEFHbzNiXC9NK0luZHBmUGoxNHBkbkFBV3VjZ3R0T0dBZXFvU2k2MzNPTzNKM2dvczBBVkJ0Z05MeWVoVXZQNUEifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2 1/2] test/compress: add out of space test X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Jan 2019 16:47:51 -0000 Hi Marko, > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Marko Kovacevic > Sent: Thursday, December 20, 2018 2:58 PM > To: dev@dpdk.org > Cc: akhil.goyal@nxp.com; Daly, Lee ; Jozwiak, TomaszX > ; O'Hare, Cathal ; > Trahe, Fiona ; Kovacevic, Marko > > Subject: [dpdk-dev] [PATCH v2 1/2] test/compress: add out of space test >=20 > From: "Kovacevic, Marko" >=20 > This patch adds new out of space testcase to check that the destination > mbuf is smaller than required for the output of compression to ensure the > driver doesn't crash and returns the valid error case. >=20 > Signed-off-by: Marko Kovacevic > Acked-by: Lee Daly >=20 > --- > V2: > Added check flag to return proper status to user > --- > test/test/test_compressdev.c | 130 > +++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 126 insertions(+), 4 deletions(-) >=20 > diff --git a/test/test/test_compressdev.c b/test/test/test_compressdev.c > index 42327dc..b2999fa 100644 > --- a/test/test/test_compressdev.c > +++ b/test/test/test_compressdev.c > @@ -41,6 +41,9 @@ > #define ZLIB_TRAILER_SIZE 4 > #define GZIP_HEADER_SIZE 10 > #define GZIP_TRAILER_SIZE 8 > +#define OUT_OF_SPACE_BUF 1 > + > +int out_of_space; I think we should avoid global variables if possible. This is a variable to change a test type, so it should go with the other te= st parameters. I get that the list of arguments is growing a lot, therefore refactoring the test_deflate_comp_decomp function is highly needed. I will send a patch doing that, so code is cleaner. >=20 > const char * > huffman_type_strings[] =3D { > @@ -734,8 +737,9 @@ test_deflate_comp_decomp(const char * const > test_bufs[], >=20 > if (sgl) { > for (i =3D 0; i < num_bufs; i++) { > - data_size =3D strlen(test_bufs[i]) * > - COMPRESS_BUF_SIZE_RATIO; > + out_of_space ? data_size =3D OUT_OF_SPACE_BUF : > + (data_size =3D strlen(test_bufs[i]) * > + COMPRESS_BUF_SIZE_RATIO); This is OK when compressing with compressdev. When testing out of place error when decompressing with compressdev, then compression (with Zlib) needs to be successful. Therefore, data_size should only be equal to OUT_OF_SPACE_BUF when zlib_dir =3D ZLIB_COMPRESS and out_of_space =3D 1. > if (prepare_sgl_bufs(NULL, comp_bufs[i], > data_size, > ts_params->small_mbuf_pool, > @@ -746,8 +750,9 @@ test_deflate_comp_decomp(const char * const > test_bufs[], >=20 > } else { > for (i =3D 0; i < num_bufs; i++) { > - data_size =3D strlen(test_bufs[i]) * > - COMPRESS_BUF_SIZE_RATIO; > + out_of_space ? data_size =3D OUT_OF_SPACE_BUF : > + (data_size =3D strlen(test_bufs[i]) * > + COMPRESS_BUF_SIZE_RATIO); > rte_pktmbuf_append(comp_bufs[i], data_size); > } > } > @@ -913,6 +918,16 @@ test_deflate_comp_decomp(const char * const > test_bufs[], > * compress operation information is needed for the decompression > stage) > */ > for (i =3D 0; i < num_bufs; i++) { > + if(out_of_space) { > + if (ops_processed[i]->status =3D=3D > + > RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED) { > + RTE_LOG(ERR, USER1, > + "Some operations were not successful\n"); I think this log should be removed, as if status is "out of space terminate= d", the test is actually passing (this is a negative test), so there is no erro= r. Instead, the log should be placed in the else part of this condition, with a message saying that the status is incorrect. > + out_of_space =3D 0; I think instead of using out_of_space as an output, ret_status can be chang= ed to 0 and just exit the function successfully. If status is not the expected, then ret_status =3D -1 (default, so no need = to change). Also, all operations should be checked if their status is the right one. Therefore, one thing you can do is check if not status =3D OOS_TERMINATED a= nd return an error in that case. Then, after the loop, if out_of_space =3D 1, just go to exit. > + goto exit; > + } > + } > + > if (ops_processed[i]->status !=3D > RTE_COMP_OP_STATUS_SUCCESS) { > RTE_LOG(ERR, USER1, > "Some operations were not successful\n"); > @@ -1110,6 +1125,16 @@ test_deflate_comp_decomp(const char * const > test_bufs[], > * compress operation information is still needed) > */ > for (i =3D 0; i < num_bufs; i++) { > + if(out_of_space) { > + if (ops_processed[i]->status =3D=3D > + > RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED) { > + RTE_LOG(ERR, USER1, > + "Some operations were not successful\n"); > + out_of_space =3D 0; > + goto exit; Same comments as above apply here. Also, data_size of the output buffers need to be modified as it is done for= compression. > + } > + } > + > if (ops_processed[i]->status !=3D > RTE_COMP_OP_STATUS_SUCCESS) { > RTE_LOG(ERR, USER1, > "Some operations were not successful\n"); > @@ -1664,6 +1689,101 @@ > test_compressdev_deflate_stateless_checksum(void) > return ret; > } >=20 > +static int > +test_compressdev_out_of_space_buffer(void) > +{ > + struct comp_testsuite_params *ts_params =3D &testsuite_params; > + const char *test_buffer; > + int ret; > + uint16_t i =3D 0; > + const struct rte_compressdev_capabilities *capab; > + out_of_space =3D 1; > + > + capab =3D rte_compressdev_capability_get(0, > RTE_COMP_ALGO_DEFLATE); > + TEST_ASSERT(capab !=3D NULL, "Failed to retrieve device capabilities"); > + > + if ((capab->comp_feature_flags & RTE_COMP_FF_HUFFMAN_FIXED) > =3D=3D 0) > + return -ENOTSUP; > + > + struct rte_comp_xform *compress_xform =3D > + rte_malloc(NULL, sizeof(struct rte_comp_xform), 0); > + > + if (compress_xform =3D=3D NULL) { > + RTE_LOG(ERR, USER1, > + "Compress xform could not be created\n"); > + ret =3D TEST_FAILED; > + goto exit; > + } > + > + test_buffer =3D compress_test_bufs[i]; > + > + /* Compress with compressdev, decompress with Zlib */ > + if (test_deflate_comp_decomp(&test_buffer, 1, > + &i, > + &ts_params->def_comp_xform, > + &ts_params->def_decomp_xform, > + 1, > + RTE_COMP_OP_STATELESS, > + 0, > + ZLIB_DECOMPRESS) =3D=3D 0 && out_of_space =3D=3D 1) { As said above, I think out_of_space does not need to be checked as an ouput= . Instead, only the return value of this function should be checked.