[PATCHv2] validation: packet: do not require a max packet length

Message ID 20170314135512.6485-1-bill.fischofer@linaro.org
State New
Headers show

Commit Message

Bill Fischofer March 14, 2017, 1:55 p.m.
Address bug https://bugs.linaro.org/show_bug.cgi?id=2908 by adding
appropriate pool capability checks to the packet and crypto tests
to account for pkt.max_len, pkt.max_seg_len, or pkt.max_segs_per_pkt
being zero, indicating these limits are bound only by available
memory.

Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

---
 test/common_plat/validation/api/crypto/crypto.c |  6 ++++--
 test/common_plat/validation/api/packet/packet.c | 13 +++++++++++--
 2 files changed, 15 insertions(+), 4 deletions(-)

-- 
2.12.0.rc1

Comments

Balakrishna Garapati March 17, 2017, 2:41 p.m. | #1
Reviewed-by: Balakrishna Garapati <balakrishna.garapati@linaro.org>


/Krishna

On 14 March 2017 at 14:55, Bill Fischofer <bill.fischofer@linaro.org> wrote:

> Address bug https://bugs.linaro.org/show_bug.cgi?id=2908 by adding

> appropriate pool capability checks to the packet and crypto tests

> to account for pkt.max_len, pkt.max_seg_len, or pkt.max_segs_per_pkt

> being zero, indicating these limits are bound only by available

> memory.

>

> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

> ---

>  test/common_plat/validation/api/crypto/crypto.c |  6 ++++--

>  test/common_plat/validation/api/packet/packet.c | 13 +++++++++++--

>  2 files changed, 15 insertions(+), 4 deletions(-)

>

> diff --git a/test/common_plat/validation/api/crypto/crypto.c

> b/test/common_plat/validation/api/crypto/crypto.c

> index e7c2bf32..94beb2f1 100644

> --- a/test/common_plat/validation/api/crypto/crypto.c

> +++ b/test/common_plat/validation/api/crypto/crypto.c

> @@ -48,12 +48,14 @@ int crypto_init(odp_instance_t *inst)

>         params.pkt.num     = PKT_POOL_NUM;

>         params.type        = ODP_POOL_PACKET;

>

> -       if (PKT_POOL_LEN > pool_capa.pkt.max_seg_len) {

> +       if (pool_capa.pkt.max_seg_len &&

> +           PKT_POOL_LEN > pool_capa.pkt.max_seg_len) {

>                 fprintf(stderr, "Warning: small packet segment length\n");

>                 params.pkt.seg_len = pool_capa.pkt.max_seg_len;

>         }

>

> -       if (PKT_POOL_LEN > pool_capa.pkt.max_len) {

> +       if (pool_capa.pkt.max_len &&

> +           PKT_POOL_LEN > pool_capa.pkt.max_len) {

>                 fprintf(stderr, "Pool max packet length too small\n");

>                 return -1;

>         }

> diff --git a/test/common_plat/validation/api/packet/packet.c

> b/test/common_plat/validation/api/packet/packet.c

> index 900c4263..669122a7 100644

> --- a/test/common_plat/validation/api/packet/packet.c

> +++ b/test/common_plat/validation/api/packet/packet.c

> @@ -114,6 +114,8 @@ int packet_suite_init(void)

>                 printf("pool_capability failed\n");

>                 return -1;

>         }

> +       if (capa.pkt.max_segs_per_pkt == 0)

> +               capa.pkt.max_segs_per_pkt = 10;

>

>         /* Pick a typical packet size and decrement it to the single

> segment

>          * limit if needed (min_seg_len maybe equal to max_len

> @@ -366,6 +368,8 @@ void packet_test_alloc_segmented(void)

>         int ret, i, num_alloc;

>

>         CU_ASSERT_FATAL(odp_pool_capability(&capa) == 0);

> +       if (capa.pkt.max_segs_per_pkt == 0)

> +               capa.pkt.max_segs_per_pkt = 10;

>

>         if (capa.pkt.max_len)

>                 max_len = capa.pkt.max_len;

> @@ -1847,6 +1851,9 @@ void packet_test_extend_ref(void)

>  {

>         odp_packet_t max_pkt, ref;

>         uint32_t hr, tr, max_len;

> +       odp_pool_capability_t capa;

> +

> +       CU_ASSERT_FATAL(odp_pool_capability(&capa) == 0);

>

>         max_pkt = odp_packet_copy(segmented_test_packet,

>                                   odp_packet_pool(segmented_test_packet));

> @@ -1860,8 +1867,10 @@ void packet_test_extend_ref(void)

>         odp_packet_push_tail(max_pkt, tr);

>

>         /* Max packet should not be extendable at either end */

> -       CU_ASSERT(odp_packet_extend_tail(&max_pkt, 1, NULL, NULL) < 0);

> -       CU_ASSERT(odp_packet_extend_head(&max_pkt, 1, NULL, NULL) < 0);

> +       if (max_len == capa.pkt.max_len) {

> +               CU_ASSERT(odp_packet_extend_tail(&max_pkt, 1, NULL, NULL)

> < 0);

> +               CU_ASSERT(odp_packet_extend_head(&max_pkt, 1, NULL, NULL)

> < 0);

> +       }

>

>         /* See if we can trunc and extend anyway */

>         CU_ASSERT(odp_packet_trunc_tail(&max_pkt, hr + tr + 1,

> --

> 2.12.0.rc1

>

>
Maxim Uvarov March 17, 2017, 2:45 p.m. | #2
On 03/14/17 16:55, Bill Fischofer wrote:
> Address bug https://bugs.linaro.org/show_bug.cgi?id=2908 by adding

> appropriate pool capability checks to the packet and crypto tests

> to account for pkt.max_len, pkt.max_seg_len, or pkt.max_segs_per_pkt

> being zero, indicating these limits are bound only by available

> memory.

> 

> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

> ---

>  test/common_plat/validation/api/crypto/crypto.c |  6 ++++--

>  test/common_plat/validation/api/packet/packet.c | 13 +++++++++++--

>  2 files changed, 15 insertions(+), 4 deletions(-)

> 

> diff --git a/test/common_plat/validation/api/crypto/crypto.c b/test/common_plat/validation/api/crypto/crypto.c

> index e7c2bf32..94beb2f1 100644

> --- a/test/common_plat/validation/api/crypto/crypto.c

> +++ b/test/common_plat/validation/api/crypto/crypto.c

> @@ -48,12 +48,14 @@ int crypto_init(odp_instance_t *inst)

>  	params.pkt.num     = PKT_POOL_NUM;

>  	params.type        = ODP_POOL_PACKET;

>  

> -	if (PKT_POOL_LEN > pool_capa.pkt.max_seg_len) {

> +	if (pool_capa.pkt.max_seg_len &&

> +	    PKT_POOL_LEN > pool_capa.pkt.max_seg_len) {

>  		fprintf(stderr, "Warning: small packet segment length\n");


what should user do when he sees that message?

Maxim.


>  		params.pkt.seg_len = pool_capa.pkt.max_seg_len;

>  	}

>  

> -	if (PKT_POOL_LEN > pool_capa.pkt.max_len) {

> +	if (pool_capa.pkt.max_len &&

> +	    PKT_POOL_LEN > pool_capa.pkt.max_len) {

>  		fprintf(stderr, "Pool max packet length too small\n");

>  		return -1;

>  	}

> diff --git a/test/common_plat/validation/api/packet/packet.c b/test/common_plat/validation/api/packet/packet.c

> index 900c4263..669122a7 100644

> --- a/test/common_plat/validation/api/packet/packet.c

> +++ b/test/common_plat/validation/api/packet/packet.c

> @@ -114,6 +114,8 @@ int packet_suite_init(void)

>  		printf("pool_capability failed\n");

>  		return -1;

>  	}

> +	if (capa.pkt.max_segs_per_pkt == 0)

> +		capa.pkt.max_segs_per_pkt = 10;

>  

>  	/* Pick a typical packet size and decrement it to the single segment

>  	 * limit if needed (min_seg_len maybe equal to max_len

> @@ -366,6 +368,8 @@ void packet_test_alloc_segmented(void)

>  	int ret, i, num_alloc;

>  

>  	CU_ASSERT_FATAL(odp_pool_capability(&capa) == 0);

> +	if (capa.pkt.max_segs_per_pkt == 0)

> +		capa.pkt.max_segs_per_pkt = 10;

>  

>  	if (capa.pkt.max_len)

>  		max_len = capa.pkt.max_len;

> @@ -1847,6 +1851,9 @@ void packet_test_extend_ref(void)

>  {

>  	odp_packet_t max_pkt, ref;

>  	uint32_t hr, tr, max_len;

> +	odp_pool_capability_t capa;

> +

> +	CU_ASSERT_FATAL(odp_pool_capability(&capa) == 0);

>  

>  	max_pkt = odp_packet_copy(segmented_test_packet,

>  				  odp_packet_pool(segmented_test_packet));

> @@ -1860,8 +1867,10 @@ void packet_test_extend_ref(void)

>  	odp_packet_push_tail(max_pkt, tr);

>  

>  	/* Max packet should not be extendable at either end */

> -	CU_ASSERT(odp_packet_extend_tail(&max_pkt, 1, NULL, NULL) < 0);

> -	CU_ASSERT(odp_packet_extend_head(&max_pkt, 1, NULL, NULL) < 0);

> +	if (max_len == capa.pkt.max_len) {

> +		CU_ASSERT(odp_packet_extend_tail(&max_pkt, 1, NULL, NULL) < 0);

> +		CU_ASSERT(odp_packet_extend_head(&max_pkt, 1, NULL, NULL) < 0);

> +	}

>  

>  	/* See if we can trunc and extend anyway */

>  	CU_ASSERT(odp_packet_trunc_tail(&max_pkt, hr + tr + 1,

>
Bill Fischofer March 17, 2017, 4:59 p.m. | #3
On Fri, Mar 17, 2017 at 9:45 AM, Maxim Uvarov <maxim.uvarov@linaro.org>
wrote:

> On 03/14/17 16:55, Bill Fischofer wrote:

> > Address bug https://bugs.linaro.org/show_bug.cgi?id=2908 by adding

> > appropriate pool capability checks to the packet and crypto tests

> > to account for pkt.max_len, pkt.max_seg_len, or pkt.max_segs_per_pkt

> > being zero, indicating these limits are bound only by available

> > memory.

> >

> > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>

> > ---

> >  test/common_plat/validation/api/crypto/crypto.c |  6 ++++--

> >  test/common_plat/validation/api/packet/packet.c | 13 +++++++++++--

> >  2 files changed, 15 insertions(+), 4 deletions(-)

> >

> > diff --git a/test/common_plat/validation/api/crypto/crypto.c

> b/test/common_plat/validation/api/crypto/crypto.c

> > index e7c2bf32..94beb2f1 100644

> > --- a/test/common_plat/validation/api/crypto/crypto.c

> > +++ b/test/common_plat/validation/api/crypto/crypto.c

> > @@ -48,12 +48,14 @@ int crypto_init(odp_instance_t *inst)

> >       params.pkt.num     = PKT_POOL_NUM;

> >       params.type        = ODP_POOL_PACKET;

> >

> > -     if (PKT_POOL_LEN > pool_capa.pkt.max_seg_len) {

> > +     if (pool_capa.pkt.max_seg_len &&

> > +         PKT_POOL_LEN > pool_capa.pkt.max_seg_len) {

> >               fprintf(stderr, "Warning: small packet segment length\n");

>

> what should user do when he sees that message?

>


It's simply a warning that the test is using a shorter packet length than
it would like because the max_seg_len is smaller than PKT_POOL_LEN. I
suspect when Bug https://bugs.linaro.org/show_bug.cgi?id=2895 is fixed this
can be relaxed. It's there, I believe, because currently crypto does not
support multi-segment packets as input. Nikhil may have additional input
here. The patch is simply to account for the possibility that the
max_seg_len == 0 (as is the case for odp-dpdk) which means that the
implementation doesn't have a hard-coded max_seg_len (and so PKT_POOL_LEN)
can be used here.


>

> Maxim.

>

>

> >               params.pkt.seg_len = pool_capa.pkt.max_seg_len;

> >       }

> >

> > -     if (PKT_POOL_LEN > pool_capa.pkt.max_len) {

> > +     if (pool_capa.pkt.max_len &&

> > +         PKT_POOL_LEN > pool_capa.pkt.max_len) {

> >               fprintf(stderr, "Pool max packet length too small\n");

> >               return -1;

> >       }

> > diff --git a/test/common_plat/validation/api/packet/packet.c

> b/test/common_plat/validation/api/packet/packet.c

> > index 900c4263..669122a7 100644

> > --- a/test/common_plat/validation/api/packet/packet.c

> > +++ b/test/common_plat/validation/api/packet/packet.c

> > @@ -114,6 +114,8 @@ int packet_suite_init(void)

> >               printf("pool_capability failed\n");

> >               return -1;

> >       }

> > +     if (capa.pkt.max_segs_per_pkt == 0)

> > +             capa.pkt.max_segs_per_pkt = 10;

> >

> >       /* Pick a typical packet size and decrement it to the single

> segment

> >        * limit if needed (min_seg_len maybe equal to max_len

> > @@ -366,6 +368,8 @@ void packet_test_alloc_segmented(void)

> >       int ret, i, num_alloc;

> >

> >       CU_ASSERT_FATAL(odp_pool_capability(&capa) == 0);

> > +     if (capa.pkt.max_segs_per_pkt == 0)

> > +             capa.pkt.max_segs_per_pkt = 10;

> >

> >       if (capa.pkt.max_len)

> >               max_len = capa.pkt.max_len;

> > @@ -1847,6 +1851,9 @@ void packet_test_extend_ref(void)

> >  {

> >       odp_packet_t max_pkt, ref;

> >       uint32_t hr, tr, max_len;

> > +     odp_pool_capability_t capa;

> > +

> > +     CU_ASSERT_FATAL(odp_pool_capability(&capa) == 0);

> >

> >       max_pkt = odp_packet_copy(segmented_test_packet,

> >                                 odp_packet_pool(segmented_test_packet));

> > @@ -1860,8 +1867,10 @@ void packet_test_extend_ref(void)

> >       odp_packet_push_tail(max_pkt, tr);

> >

> >       /* Max packet should not be extendable at either end */

> > -     CU_ASSERT(odp_packet_extend_tail(&max_pkt, 1, NULL, NULL) < 0);

> > -     CU_ASSERT(odp_packet_extend_head(&max_pkt, 1, NULL, NULL) < 0);

> > +     if (max_len == capa.pkt.max_len) {

> > +             CU_ASSERT(odp_packet_extend_tail(&max_pkt, 1, NULL, NULL)

> < 0);

> > +             CU_ASSERT(odp_packet_extend_head(&max_pkt, 1, NULL, NULL)

> < 0);

> > +     }

> >

> >       /* See if we can trunc and extend anyway */

> >       CU_ASSERT(odp_packet_trunc_tail(&max_pkt, hr + tr + 1,

> >

>

>

Patch

diff --git a/test/common_plat/validation/api/crypto/crypto.c b/test/common_plat/validation/api/crypto/crypto.c
index e7c2bf32..94beb2f1 100644
--- a/test/common_plat/validation/api/crypto/crypto.c
+++ b/test/common_plat/validation/api/crypto/crypto.c
@@ -48,12 +48,14 @@  int crypto_init(odp_instance_t *inst)
 	params.pkt.num     = PKT_POOL_NUM;
 	params.type        = ODP_POOL_PACKET;
 
-	if (PKT_POOL_LEN > pool_capa.pkt.max_seg_len) {
+	if (pool_capa.pkt.max_seg_len &&
+	    PKT_POOL_LEN > pool_capa.pkt.max_seg_len) {
 		fprintf(stderr, "Warning: small packet segment length\n");
 		params.pkt.seg_len = pool_capa.pkt.max_seg_len;
 	}
 
-	if (PKT_POOL_LEN > pool_capa.pkt.max_len) {
+	if (pool_capa.pkt.max_len &&
+	    PKT_POOL_LEN > pool_capa.pkt.max_len) {
 		fprintf(stderr, "Pool max packet length too small\n");
 		return -1;
 	}
diff --git a/test/common_plat/validation/api/packet/packet.c b/test/common_plat/validation/api/packet/packet.c
index 900c4263..669122a7 100644
--- a/test/common_plat/validation/api/packet/packet.c
+++ b/test/common_plat/validation/api/packet/packet.c
@@ -114,6 +114,8 @@  int packet_suite_init(void)
 		printf("pool_capability failed\n");
 		return -1;
 	}
+	if (capa.pkt.max_segs_per_pkt == 0)
+		capa.pkt.max_segs_per_pkt = 10;
 
 	/* Pick a typical packet size and decrement it to the single segment
 	 * limit if needed (min_seg_len maybe equal to max_len
@@ -366,6 +368,8 @@  void packet_test_alloc_segmented(void)
 	int ret, i, num_alloc;
 
 	CU_ASSERT_FATAL(odp_pool_capability(&capa) == 0);
+	if (capa.pkt.max_segs_per_pkt == 0)
+		capa.pkt.max_segs_per_pkt = 10;
 
 	if (capa.pkt.max_len)
 		max_len = capa.pkt.max_len;
@@ -1847,6 +1851,9 @@  void packet_test_extend_ref(void)
 {
 	odp_packet_t max_pkt, ref;
 	uint32_t hr, tr, max_len;
+	odp_pool_capability_t capa;
+
+	CU_ASSERT_FATAL(odp_pool_capability(&capa) == 0);
 
 	max_pkt = odp_packet_copy(segmented_test_packet,
 				  odp_packet_pool(segmented_test_packet));
@@ -1860,8 +1867,10 @@  void packet_test_extend_ref(void)
 	odp_packet_push_tail(max_pkt, tr);
 
 	/* Max packet should not be extendable at either end */
-	CU_ASSERT(odp_packet_extend_tail(&max_pkt, 1, NULL, NULL) < 0);
-	CU_ASSERT(odp_packet_extend_head(&max_pkt, 1, NULL, NULL) < 0);
+	if (max_len == capa.pkt.max_len) {
+		CU_ASSERT(odp_packet_extend_tail(&max_pkt, 1, NULL, NULL) < 0);
+		CU_ASSERT(odp_packet_extend_head(&max_pkt, 1, NULL, NULL) < 0);
+	}
 
 	/* See if we can trunc and extend anyway */
 	CU_ASSERT(odp_packet_trunc_tail(&max_pkt, hr + tr + 1,