diff mbox series

[12/12] test/ipsec: fix build with GCC 12

Message ID 20220518101657.1230416-13-david.marchand@redhat.com (mailing list archive)
State Changes Requested
Delegated to: Thomas Monjalon
Headers show
Series Fix compilation with gcc 12 | expand

Checks

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

Commit Message

David Marchand May 18, 2022, 10:16 a.m. UTC
GCC 12 raises the following warning:

In function ‘_mm256_loadu_si256’,
    inlined from ‘rte_mov32’ at
        ../lib/eal/x86/include/rte_memcpy.h:319:9,
    inlined from ‘rte_mov128’ at
        ../lib/eal/x86/include/rte_memcpy.h:344:2,
    inlined from ‘rte_memcpy_generic’ at
        ../lib/eal/x86/include/rte_memcpy.h:438:4,
    inlined from ‘rte_memcpy’ at
        ../lib/eal/x86/include/rte_memcpy.h:882:10,
    inlined from ‘setup_test_string.constprop’ at
        ../app/test/test_ipsec.c:572:4:
/usr/lib/gcc/x86_64-redhat-linux/12/include/avxintrin.h:929:10: error:
    array subscript ‘__m256i_u[3]’ is partly outside array bounds of
    ‘const char[108]’ [-Werror=array-bounds]
  929 |   return *__P;
      |          ^~~~
../app/test/test_ipsec.c: In function ‘setup_test_string.constprop’:
../app/test/test_ipsec.c:539:12: note: at offset 96 into object
    ‘null_plain_data’ of size 108
  539 | const char null_plain_data[] =
      |            ^~~~~~~~~~~~~~~

Split copy request into copies of string lengths and remove unused
blocksize.

Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test_ipsec.c | 48 ++++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 19 deletions(-)

Comments

Medvedkin, Vladimir June 2, 2022, 6:41 p.m. UTC | #1
Hi David,

On 18/05/2022 11:16, David Marchand wrote:
> GCC 12 raises the following warning:
> 
> In function ‘_mm256_loadu_si256’,
>      inlined from ‘rte_mov32’ at
>          ../lib/eal/x86/include/rte_memcpy.h:319:9,
>      inlined from ‘rte_mov128’ at
>          ../lib/eal/x86/include/rte_memcpy.h:344:2,
>      inlined from ‘rte_memcpy_generic’ at
>          ../lib/eal/x86/include/rte_memcpy.h:438:4,
>      inlined from ‘rte_memcpy’ at
>          ../lib/eal/x86/include/rte_memcpy.h:882:10,
>      inlined from ‘setup_test_string.constprop’ at
>          ../app/test/test_ipsec.c:572:4:
> /usr/lib/gcc/x86_64-redhat-linux/12/include/avxintrin.h:929:10: error:
>      array subscript ‘__m256i_u[3]’ is partly outside array bounds of
>      ‘const char[108]’ [-Werror=array-bounds]
>    929 |   return *__P;
>        |          ^~~~
> ../app/test/test_ipsec.c: In function ‘setup_test_string.constprop’:
> ../app/test/test_ipsec.c:539:12: note: at offset 96 into object
>      ‘null_plain_data’ of size 108
>    539 | const char null_plain_data[] =
>        |            ^~~~~~~~~~~~~~~
> 
> Split copy request into copies of string lengths and remove unused
> blocksize.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>   app/test/test_ipsec.c | 48 ++++++++++++++++++++++++++-----------------
>   1 file changed, 29 insertions(+), 19 deletions(-)
> 
> diff --git a/app/test/test_ipsec.c b/app/test/test_ipsec.c
> index 8da025bf66..d7455fd021 100644
> --- a/app/test/test_ipsec.c
> +++ b/app/test/test_ipsec.c
> @@ -554,24 +554,28 @@ struct rte_ipv4_hdr ipv4_outer  = {
>   };
>   
>   static struct rte_mbuf *
> -setup_test_string(struct rte_mempool *mpool,
> -		const char *string, size_t len, uint8_t blocksize)
> +setup_test_string(struct rte_mempool *mpool, const char *string,
> +	size_t string_len, size_t len)
>   {
>   	struct rte_mbuf *m = rte_pktmbuf_alloc(mpool);
> -	size_t t_len = len - (blocksize ? (len % blocksize) : 0);
>   
>   	if (m) {
>   		memset(m->buf_addr, 0, m->buf_len);
> -		char *dst = rte_pktmbuf_append(m, t_len);
> +		char *dst = rte_pktmbuf_append(m, len);
>   
>   		if (!dst) {
>   			rte_pktmbuf_free(m);
>   			return NULL;
>   		}
> -		if (string != NULL)
> -			rte_memcpy(dst, string, t_len);
> -		else
> -			memset(dst, 0, t_len);
> +		if (string != NULL) {
> +			size_t off;
> +
> +			for (off = 0; off + string_len < len; off += string_len)

I think it should be off + string_len <= len here, because otherwise, if 
len is a multiple of string_len, the last ret_memcpy (after this loop) 
will copy 0 bytes.

> +				rte_memcpy(&dst[off], string, string_len);
> +			rte_memcpy(&dst[off], string, len % string_len);
> +		} else {
> +			memset(dst, 0, len);
> +		}
>   	}
>   
>   	return m;
> @@ -1365,7 +1369,8 @@ test_ipsec_crypto_outb_burst_null_null(int i)
>   	/* Generate input mbuf data */
>   	for (j = 0; j < num_pkts && rc == 0; j++) {
>   		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
> -			null_plain_data, test_cfg[i].pkt_sz, 0);
> +			null_plain_data, sizeof(null_plain_data),
> +			test_cfg[i].pkt_sz);
>   		if (ut_params->ibuf[j] == NULL)
>   			rc = TEST_FAILED;
>   		else {
> @@ -1483,7 +1488,8 @@ test_ipsec_inline_crypto_inb_burst_null_null(int i)
>   			/* Generate test mbuf data */
>   			ut_params->obuf[j] = setup_test_string(
>   				ts_params->mbuf_pool,
> -				null_plain_data, test_cfg[i].pkt_sz, 0);
> +				null_plain_data, sizeof(null_plain_data),
> +				test_cfg[i].pkt_sz);
>   			if (ut_params->obuf[j] == NULL)
>   				rc = TEST_FAILED;
>   		}
> @@ -1551,16 +1557,17 @@ test_ipsec_inline_proto_inb_burst_null_null(int i)
>   
>   	/* Generate inbound mbuf data */
>   	for (j = 0; j < num_pkts && rc == 0; j++) {
> -		ut_params->ibuf[j] = setup_test_string(
> -			ts_params->mbuf_pool,
> -			null_plain_data, test_cfg[i].pkt_sz, 0);
> +		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
> +			null_plain_data, sizeof(null_plain_data),
> +			test_cfg[i].pkt_sz);
>   		if (ut_params->ibuf[j] == NULL)
>   			rc = TEST_FAILED;
>   		else {
>   			/* Generate test mbuf data */
>   			ut_params->obuf[j] = setup_test_string(
>   				ts_params->mbuf_pool,
> -				null_plain_data, test_cfg[i].pkt_sz, 0);
> +				null_plain_data, sizeof(null_plain_data),
> +				test_cfg[i].pkt_sz);
>   			if (ut_params->obuf[j] == NULL)
>   				rc = TEST_FAILED;
>   		}
> @@ -1660,7 +1667,8 @@ test_ipsec_inline_crypto_outb_burst_null_null(int i)
>   	/* Generate test mbuf data */
>   	for (j = 0; j < num_pkts && rc == 0; j++) {
>   		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
> -			null_plain_data, test_cfg[i].pkt_sz, 0);
> +			null_plain_data, sizeof(null_plain_data),
> +			test_cfg[i].pkt_sz);
>   		if (ut_params->ibuf[0] == NULL)
>   			rc = TEST_FAILED;
>   
> @@ -1738,15 +1746,16 @@ test_ipsec_inline_proto_outb_burst_null_null(int i)
>   	/* Generate test mbuf data */
>   	for (j = 0; j < num_pkts && rc == 0; j++) {
>   		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
> -			null_plain_data, test_cfg[i].pkt_sz, 0);
> +			null_plain_data, sizeof(null_plain_data),
> +			test_cfg[i].pkt_sz);
>   		if (ut_params->ibuf[0] == NULL)
>   			rc = TEST_FAILED;
>   
>   		if (rc == 0) {
>   			/* Generate test tunneled mbuf data for comparison */
>   			ut_params->obuf[j] = setup_test_string(
> -					ts_params->mbuf_pool,
> -					null_plain_data, test_cfg[i].pkt_sz, 0);
> +				ts_params->mbuf_pool, null_plain_data,
> +				sizeof(null_plain_data), test_cfg[i].pkt_sz);
>   			if (ut_params->obuf[j] == NULL)
>   				rc = TEST_FAILED;
>   		}
> @@ -1815,7 +1824,8 @@ test_ipsec_lksd_proto_inb_burst_null_null(int i)
>   	for (j = 0; j < num_pkts && rc == 0; j++) {
>   		/* packet with sequence number 0 is invalid */
>   		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
> -			null_encrypted_data, test_cfg[i].pkt_sz, 0);
> +			null_encrypted_data, sizeof(null_encrypted_data),
> +			 test_cfg[i].pkt_sz);
>   		if (ut_params->ibuf[j] == NULL)
>   			rc = TEST_FAILED;
>   	}
David Marchand June 3, 2022, 7:45 a.m. UTC | #2
Hello Vladimir,

On Thu, Jun 2, 2022 at 8:42 PM Medvedkin, Vladimir
<vladimir.medvedkin@intel.com> wrote:
> >               if (!dst) {
> >                       rte_pktmbuf_free(m);
> >                       return NULL;
> >               }
> > -             if (string != NULL)
> > -                     rte_memcpy(dst, string, t_len);
> > -             else
> > -                     memset(dst, 0, t_len);
> > +             if (string != NULL) {
> > +                     size_t off;
> > +
> > +                     for (off = 0; off + string_len < len; off += string_len)
>
> I think it should be off + string_len <= len here, because otherwise, if
> len is a multiple of string_len, the last ret_memcpy (after this loop)
> will copy 0 bytes.

Changing to off + string_len <= len would trigger an oob access to dst
(by one extra byte)?
Otoh, I don't think it is an issue to have a 0-length call to rte_memcpy.

>
> > +                             rte_memcpy(&dst[off], string, string_len);
> > +                     rte_memcpy(&dst[off], string, len % string_len);
Bruce Richardson June 3, 2022, 7:56 a.m. UTC | #3
On Fri, Jun 03, 2022 at 09:45:45AM +0200, David Marchand wrote:
> Hello Vladimir,
> 
> On Thu, Jun 2, 2022 at 8:42 PM Medvedkin, Vladimir
> <vladimir.medvedkin@intel.com> wrote:
> > >               if (!dst) {
> > >                       rte_pktmbuf_free(m);
> > >                       return NULL;
> > >               }
> > > -             if (string != NULL)
> > > -                     rte_memcpy(dst, string, t_len);
> > > -             else
> > > -                     memset(dst, 0, t_len);
> > > +             if (string != NULL) {
> > > +                     size_t off;
> > > +
> > > +                     for (off = 0; off + string_len < len; off += string_len)
> >
> > I think it should be off + string_len <= len here, because otherwise, if
> > len is a multiple of string_len, the last ret_memcpy (after this loop)
> > will copy 0 bytes.
> 
> Changing to off + string_len <= len would trigger an oob access to dst
> (by one extra byte)?
> Otoh, I don't think it is an issue to have a 0-length call to rte_memcpy.
> 
Given this is test code, do we need rte_memcpy for performance over regular
libc memcpy? Does fixing the warning become any easier or clearer if libc
memcpy is used?
David Marchand June 3, 2022, 9:41 a.m. UTC | #4
On Fri, Jun 3, 2022 at 9:56 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Fri, Jun 03, 2022 at 09:45:45AM +0200, David Marchand wrote:
> > Hello Vladimir,
> >
> > On Thu, Jun 2, 2022 at 8:42 PM Medvedkin, Vladimir
> > <vladimir.medvedkin@intel.com> wrote:
> > > >               if (!dst) {
> > > >                       rte_pktmbuf_free(m);
> > > >                       return NULL;
> > > >               }
> > > > -             if (string != NULL)
> > > > -                     rte_memcpy(dst, string, t_len);
> > > > -             else
> > > > -                     memset(dst, 0, t_len);
> > > > +             if (string != NULL) {
> > > > +                     size_t off;
> > > > +
> > > > +                     for (off = 0; off + string_len < len; off += string_len)
> > >
> > > I think it should be off + string_len <= len here, because otherwise, if
> > > len is a multiple of string_len, the last ret_memcpy (after this loop)
> > > will copy 0 bytes.
> >
> > Changing to off + string_len <= len would trigger an oob access to dst
> > (by one extra byte)?
> > Otoh, I don't think it is an issue to have a 0-length call to rte_memcpy.
> >
> Given this is test code, do we need rte_memcpy for performance over regular
> libc memcpy? Does fixing the warning become any easier or clearer if libc
> memcpy is used?

There was a similar proposal in vhost/crypto code.
I am not a fan to switching to libc memcpy.
We would be waiving a potential issue in rte_memcpy itself (which
could also be a problem in how gcc understands this inlined code) or
in the rte_memcpy caller code.

Here, gcc is probably too picky.
No path currently leads to oob access on the src string.

Adding a simple hint (see simplified hunk below) seems to help gcc enough:

@@ -554,12 +554,14 @@ struct rte_ipv4_hdr ipv4_outer  = {
 };

 static struct rte_mbuf *
-setup_test_string(struct rte_mempool *mpool,
-        const char *string, size_t len, uint8_t blocksize)
+setup_test_string(struct rte_mempool *mpool, const char *string,
+    size_t string_len, size_t len, uint8_t blocksize)
 {
     struct rte_mbuf *m = rte_pktmbuf_alloc(mpool);
     size_t t_len = len - (blocksize ? (len % blocksize) : 0);

+    RTE_VERIFY(len <= string_len);
+

     if (m) {
         memset(m->buf_addr, 0, m->buf_len);
Medvedkin, Vladimir June 3, 2022, 3:57 p.m. UTC | #5
Hi David,

On 03/06/2022 10:41, David Marchand wrote:
> On Fri, Jun 3, 2022 at 9:56 AM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
>>
>> On Fri, Jun 03, 2022 at 09:45:45AM +0200, David Marchand wrote:
>>> Hello Vladimir,
>>>
>>> On Thu, Jun 2, 2022 at 8:42 PM Medvedkin, Vladimir
>>> <vladimir.medvedkin@intel.com> wrote:
>>>>>                if (!dst) {
>>>>>                        rte_pktmbuf_free(m);
>>>>>                        return NULL;
>>>>>                }
>>>>> -             if (string != NULL)
>>>>> -                     rte_memcpy(dst, string, t_len);
>>>>> -             else
>>>>> -                     memset(dst, 0, t_len);
>>>>> +             if (string != NULL) {
>>>>> +                     size_t off;
>>>>> +
>>>>> +                     for (off = 0; off + string_len < len; off += string_len)
>>>>
>>>> I think it should be off + string_len <= len here, because otherwise, if
>>>> len is a multiple of string_len, the last ret_memcpy (after this loop)
>>>> will copy 0 bytes.
>>>
>>> Changing to off + string_len <= len would trigger an oob access to dst
>>> (by one extra byte)?
>>> Otoh, I don't think it is an issue to have a 0-length call to rte_memcpy.
>>>

The problem here is that if, for example, string_len is 8 bytes and len 
is 16, then it will write only 8 bytes.

>> Given this is test code, do we need rte_memcpy for performance over regular
>> libc memcpy? Does fixing the warning become any easier or clearer if libc
>> memcpy is used?
> 
> There was a similar proposal in vhost/crypto code.
> I am not a fan to switching to libc memcpy.
> We would be waiving a potential issue in rte_memcpy itself (which
> could also be a problem in how gcc understands this inlined code) or
> in the rte_memcpy caller code.
> 
> Here, gcc is probably too picky.
> No path currently leads to oob access on the src string.
> 
> Adding a simple hint (see simplified hunk below) seems to help gcc enough:
> 
> @@ -554,12 +554,14 @@ struct rte_ipv4_hdr ipv4_outer  = {
>   };
> 
>   static struct rte_mbuf *
> -setup_test_string(struct rte_mempool *mpool,
> -        const char *string, size_t len, uint8_t blocksize)
> +setup_test_string(struct rte_mempool *mpool, const char *string,
> +    size_t string_len, size_t len, uint8_t blocksize)
>   {
>       struct rte_mbuf *m = rte_pktmbuf_alloc(mpool);
>       size_t t_len = len - (blocksize ? (len % blocksize) : 0);
> 
> +    RTE_VERIFY(len <= string_len);
> +

RTE_VERIFY looks better here to make picky GCC happy.

> 
>       if (m) {
>           memset(m->buf_addr, 0, m->buf_len);
> 
>
Stephen Hemminger June 11, 2022, 3:38 p.m. UTC | #6
On Wed, 18 May 2022 12:16:57 +0200
David Marchand <david.marchand@redhat.com> wrote:

> GCC 12 raises the following warning:
> 
> In function ‘_mm256_loadu_si256’,
>     inlined from ‘rte_mov32’ at
>         ../lib/eal/x86/include/rte_memcpy.h:319:9,
>     inlined from ‘rte_mov128’ at
>         ../lib/eal/x86/include/rte_memcpy.h:344:2,
>     inlined from ‘rte_memcpy_generic’ at
>         ../lib/eal/x86/include/rte_memcpy.h:438:4,
>     inlined from ‘rte_memcpy’ at
>         ../lib/eal/x86/include/rte_memcpy.h:882:10,
>     inlined from ‘setup_test_string.constprop’ at
>         ../app/test/test_ipsec.c:572:4:
> /usr/lib/gcc/x86_64-redhat-linux/12/include/avxintrin.h:929:10: error:
>     array subscript ‘__m256i_u[3]’ is partly outside array bounds of
>     ‘const char[108]’ [-Werror=array-bounds]
>   929 |   return *__P;
>       |          ^~~~
> ../app/test/test_ipsec.c: In function ‘setup_test_string.constprop’:
> ../app/test/test_ipsec.c:539:12: note: at offset 96 into object
>     ‘null_plain_data’ of size 108
>   539 | const char null_plain_data[] =
>       |            ^~~~~~~~~~~~~~~
> 
> Split copy request into copies of string lengths and remove unused
> blocksize.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Why is test code for ipsec bother with using rte_memcpy at all.
Instead global replace rte_memcpy() with memcpy() for the whole test.
diff mbox series

Patch

diff --git a/app/test/test_ipsec.c b/app/test/test_ipsec.c
index 8da025bf66..d7455fd021 100644
--- a/app/test/test_ipsec.c
+++ b/app/test/test_ipsec.c
@@ -554,24 +554,28 @@  struct rte_ipv4_hdr ipv4_outer  = {
 };
 
 static struct rte_mbuf *
-setup_test_string(struct rte_mempool *mpool,
-		const char *string, size_t len, uint8_t blocksize)
+setup_test_string(struct rte_mempool *mpool, const char *string,
+	size_t string_len, size_t len)
 {
 	struct rte_mbuf *m = rte_pktmbuf_alloc(mpool);
-	size_t t_len = len - (blocksize ? (len % blocksize) : 0);
 
 	if (m) {
 		memset(m->buf_addr, 0, m->buf_len);
-		char *dst = rte_pktmbuf_append(m, t_len);
+		char *dst = rte_pktmbuf_append(m, len);
 
 		if (!dst) {
 			rte_pktmbuf_free(m);
 			return NULL;
 		}
-		if (string != NULL)
-			rte_memcpy(dst, string, t_len);
-		else
-			memset(dst, 0, t_len);
+		if (string != NULL) {
+			size_t off;
+
+			for (off = 0; off + string_len < len; off += string_len)
+				rte_memcpy(&dst[off], string, string_len);
+			rte_memcpy(&dst[off], string, len % string_len);
+		} else {
+			memset(dst, 0, len);
+		}
 	}
 
 	return m;
@@ -1365,7 +1369,8 @@  test_ipsec_crypto_outb_burst_null_null(int i)
 	/* Generate input mbuf data */
 	for (j = 0; j < num_pkts && rc == 0; j++) {
 		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
-			null_plain_data, test_cfg[i].pkt_sz, 0);
+			null_plain_data, sizeof(null_plain_data),
+			test_cfg[i].pkt_sz);
 		if (ut_params->ibuf[j] == NULL)
 			rc = TEST_FAILED;
 		else {
@@ -1483,7 +1488,8 @@  test_ipsec_inline_crypto_inb_burst_null_null(int i)
 			/* Generate test mbuf data */
 			ut_params->obuf[j] = setup_test_string(
 				ts_params->mbuf_pool,
-				null_plain_data, test_cfg[i].pkt_sz, 0);
+				null_plain_data, sizeof(null_plain_data),
+				test_cfg[i].pkt_sz);
 			if (ut_params->obuf[j] == NULL)
 				rc = TEST_FAILED;
 		}
@@ -1551,16 +1557,17 @@  test_ipsec_inline_proto_inb_burst_null_null(int i)
 
 	/* Generate inbound mbuf data */
 	for (j = 0; j < num_pkts && rc == 0; j++) {
-		ut_params->ibuf[j] = setup_test_string(
-			ts_params->mbuf_pool,
-			null_plain_data, test_cfg[i].pkt_sz, 0);
+		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
+			null_plain_data, sizeof(null_plain_data),
+			test_cfg[i].pkt_sz);
 		if (ut_params->ibuf[j] == NULL)
 			rc = TEST_FAILED;
 		else {
 			/* Generate test mbuf data */
 			ut_params->obuf[j] = setup_test_string(
 				ts_params->mbuf_pool,
-				null_plain_data, test_cfg[i].pkt_sz, 0);
+				null_plain_data, sizeof(null_plain_data),
+				test_cfg[i].pkt_sz);
 			if (ut_params->obuf[j] == NULL)
 				rc = TEST_FAILED;
 		}
@@ -1660,7 +1667,8 @@  test_ipsec_inline_crypto_outb_burst_null_null(int i)
 	/* Generate test mbuf data */
 	for (j = 0; j < num_pkts && rc == 0; j++) {
 		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
-			null_plain_data, test_cfg[i].pkt_sz, 0);
+			null_plain_data, sizeof(null_plain_data),
+			test_cfg[i].pkt_sz);
 		if (ut_params->ibuf[0] == NULL)
 			rc = TEST_FAILED;
 
@@ -1738,15 +1746,16 @@  test_ipsec_inline_proto_outb_burst_null_null(int i)
 	/* Generate test mbuf data */
 	for (j = 0; j < num_pkts && rc == 0; j++) {
 		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
-			null_plain_data, test_cfg[i].pkt_sz, 0);
+			null_plain_data, sizeof(null_plain_data),
+			test_cfg[i].pkt_sz);
 		if (ut_params->ibuf[0] == NULL)
 			rc = TEST_FAILED;
 
 		if (rc == 0) {
 			/* Generate test tunneled mbuf data for comparison */
 			ut_params->obuf[j] = setup_test_string(
-					ts_params->mbuf_pool,
-					null_plain_data, test_cfg[i].pkt_sz, 0);
+				ts_params->mbuf_pool, null_plain_data,
+				sizeof(null_plain_data), test_cfg[i].pkt_sz);
 			if (ut_params->obuf[j] == NULL)
 				rc = TEST_FAILED;
 		}
@@ -1815,7 +1824,8 @@  test_ipsec_lksd_proto_inb_burst_null_null(int i)
 	for (j = 0; j < num_pkts && rc == 0; j++) {
 		/* packet with sequence number 0 is invalid */
 		ut_params->ibuf[j] = setup_test_string(ts_params->mbuf_pool,
-			null_encrypted_data, test_cfg[i].pkt_sz, 0);
+			null_encrypted_data, sizeof(null_encrypted_data),
+			 test_cfg[i].pkt_sz);
 		if (ut_params->ibuf[j] == NULL)
 			rc = TEST_FAILED;
 	}