From: "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
To: "Kovacevic, Marko" <marko.kovacevic@intel.com>
Cc: "akhil.goyal@nxp.com" <akhil.goyal@nxp.com>,
"Daly, Lee" <lee.daly@intel.com>,
"Jozwiak, TomaszX" <tomaszx.jozwiak@intel.com>,
"O'Hare, Cathal" <cathal.ohare@intel.com>,
"Trahe, Fiona" <fiona.trahe@intel.com>,
"Kovacevic, Marko" <marko.kovacevic@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2 1/2] test/compress: add out of space test
Date: Wed, 9 Jan 2019 16:47:42 +0000 [thread overview]
Message-ID: <E115CCD9D858EF4F90C690B0DCB4D897803616A9@irsmsx112.ger.corp.intel.com> (raw)
In-Reply-To: <20181220145824.37223-1-marko.kovacevic@intel.com>
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 <lee.daly@intel.com>; Jozwiak, TomaszX
> <tomaszx.jozwiak@intel.com>; O'Hare, Cathal <cathal.ohare@intel.com>;
> Trahe, Fiona <fiona.trahe@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>
> Subject: [dpdk-dev] [PATCH v2 1/2] test/compress: add out of space test
>
> From: "Kovacevic, Marko" <marko.kovacevic@intel.com>
>
> 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.
>
> Signed-off-by: Marko Kovacevic <marko.kovacevic@intel.com>
> Acked-by: Lee Daly <lee.daly@intel.com>
>
> ---
> V2:
> Added check flag to return proper status to user
> ---
> test/test/test_compressdev.c | 130
> +++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 126 insertions(+), 4 deletions(-)
>
> 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 test 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.
>
> const char *
> huffman_type_strings[] = {
> @@ -734,8 +737,9 @@ test_deflate_comp_decomp(const char * const
> test_bufs[],
>
> if (sgl) {
> for (i = 0; i < num_bufs; i++) {
> - data_size = strlen(test_bufs[i]) *
> - COMPRESS_BUF_SIZE_RATIO;
> + out_of_space ? data_size = OUT_OF_SPACE_BUF :
> + (data_size = 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 = ZLIB_COMPRESS and out_of_space = 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[],
>
> } else {
> for (i = 0; i < num_bufs; i++) {
> - data_size = strlen(test_bufs[i]) *
> - COMPRESS_BUF_SIZE_RATIO;
> + out_of_space ? data_size = OUT_OF_SPACE_BUF :
> + (data_size = 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 = 0; i < num_bufs; i++) {
> + if(out_of_space) {
> + if (ops_processed[i]->status ==
> +
> 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 terminated",
the test is actually passing (this is a negative test), so there is no error.
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 = 0;
I think instead of using out_of_space as an output, ret_status can be changed to 0
and just exit the function successfully.
If status is not the expected, then ret_status = -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 = OOS_TERMINATED and return an error in that case.
Then, after the loop, if out_of_space = 1, just go to exit.
> + goto exit;
> + }
> + }
> +
> if (ops_processed[i]->status !=
> 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 = 0; i < num_bufs; i++) {
> + if(out_of_space) {
> + if (ops_processed[i]->status ==
> +
> RTE_COMP_OP_STATUS_OUT_OF_SPACE_TERMINATED) {
> + RTE_LOG(ERR, USER1,
> + "Some operations were not successful\n");
> + out_of_space = 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 !=
> 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;
> }
>
> +static int
> +test_compressdev_out_of_space_buffer(void)
> +{
> + struct comp_testsuite_params *ts_params = &testsuite_params;
> + const char *test_buffer;
> + int ret;
> + uint16_t i = 0;
> + const struct rte_compressdev_capabilities *capab;
> + out_of_space = 1;
> +
> + capab = rte_compressdev_capability_get(0,
> RTE_COMP_ALGO_DEFLATE);
> + TEST_ASSERT(capab != NULL, "Failed to retrieve device capabilities");
> +
> + if ((capab->comp_feature_flags & RTE_COMP_FF_HUFFMAN_FIXED)
> == 0)
> + return -ENOTSUP;
> +
> + struct rte_comp_xform *compress_xform =
> + rte_malloc(NULL, sizeof(struct rte_comp_xform), 0);
> +
> + if (compress_xform == NULL) {
> + RTE_LOG(ERR, USER1,
> + "Compress xform could not be created\n");
> + ret = TEST_FAILED;
> + goto exit;
> + }
> +
> + test_buffer = 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) == 0 && out_of_space == 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.
next prev parent reply other threads:[~2019-01-09 16:47 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-14 15:33 [dpdk-dev] [PATCH v1 " Marko Kovacevic
2018-12-14 15:33 ` [dpdk-dev] [PATCH v1 2/2] test/compress: add varied buffer input/outputs Marko Kovacevic
2018-12-14 15:56 ` Daly, Lee
2018-12-14 15:55 ` [dpdk-dev] [PATCH v1 1/2] test/compress: add out of space test Daly, Lee
2018-12-20 14:58 ` [dpdk-dev] [PATCH v2 " Marko Kovacevic
2018-12-20 14:58 ` [dpdk-dev] [PATCH v2 2/2] test/compress: add varied buffer input/outputs Marko Kovacevic
2018-12-21 0:44 ` Trahe, Fiona
2019-01-09 17:02 ` De Lara Guarch, Pablo
2018-12-21 0:41 ` [dpdk-dev] [PATCH v2 1/2] test/compress: add out of space test Trahe, Fiona
2019-01-09 16:47 ` De Lara Guarch, Pablo [this message]
2019-01-11 15:08 ` [dpdk-dev] [PATCH v3 0/3] Compression Unit Tests Kovacevic, Marko
2019-01-11 15:08 ` [dpdk-dev] [PATCH v3 1/3] test/compress: refactor main test function Kovacevic, Marko
2019-01-11 16:52 ` [dpdk-dev] [PATCH v4 0/3] Compression Unit Tests Kovacevic, Marko
2019-01-11 16:52 ` [dpdk-dev] [PATCH v4 1/3] test/compress: refactor main test function Kovacevic, Marko
2019-01-17 10:19 ` [dpdk-dev] [PATCH v5 0/3] Compression Unit Tests Kovacevic, Marko
2019-01-17 10:19 ` [dpdk-dev] [PATCH v5 1/3] test/compress: refactor main test function Kovacevic, Marko
2019-01-17 10:19 ` [dpdk-dev] [PATCH v5 2/3] test/compress: add out of space test Kovacevic, Marko
2019-01-17 10:19 ` [dpdk-dev] [PATCH v5 3/3] test/compress: add varied buffer input/outputs Kovacevic, Marko
2019-01-18 0:06 ` [dpdk-dev] [PATCH v5 0/3] Compression Unit Tests Thomas Monjalon
2019-01-11 16:52 ` [dpdk-dev] [PATCH v4 2/3] test/compress: add out of space test Kovacevic, Marko
2019-01-16 17:16 ` De Lara Guarch, Pablo
2019-01-11 16:52 ` [dpdk-dev] [PATCH v4 3/3] test/compress: add varied buffer input/outputs Kovacevic, Marko
2019-01-16 17:21 ` De Lara Guarch, Pablo
2019-01-11 15:08 ` [dpdk-dev] [PATCH v3 2/3] test/compress: add out of space test Kovacevic, Marko
2019-01-11 15:09 ` [dpdk-dev] [PATCH v3 3/3] test/compress: add varied buffer input/outputs Kovacevic, Marko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E115CCD9D858EF4F90C690B0DCB4D897803616A9@irsmsx112.ger.corp.intel.com \
--to=pablo.de.lara.guarch@intel.com \
--cc=akhil.goyal@nxp.com \
--cc=cathal.ohare@intel.com \
--cc=dev@dpdk.org \
--cc=fiona.trahe@intel.com \
--cc=lee.daly@intel.com \
--cc=marko.kovacevic@intel.com \
--cc=tomaszx.jozwiak@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).