diff mbox

validation: pktio: initialize pkt_seq

Message ID 1463068235-16953-1-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov May 12, 2016, 3:50 p.m. UTC 
recv_packets_tmo() has if for not initialized
pkt_seq array, while init is done in create_packets(),
so we need just reorder that lines.
https://bugs.linaro.org/show_bug.cgi?id=2222

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 test/validation/pktio/pktio.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Bill Fischofer May 12, 2016, 6:44 p.m. UTC | #1 
On Thu, May 12, 2016 at 10:50 AM, Maxim Uvarov <maxim.uvarov@linaro.org>
wrote:

> recv_packets_tmo() has if for not initialized

> pkt_seq array, while init is done in create_packets(),

> so we need just reorder that lines.

> https://bugs.linaro.org/show_bug.cgi?id=2222

>

> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>

>


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



> ---

>  test/validation/pktio/pktio.c | 10 +++++-----

>  1 file changed, 5 insertions(+), 5 deletions(-)

>

> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c

> index a51e0b2..90adc02 100644

> --- a/test/validation/pktio/pktio.c

> +++ b/test/validation/pktio/pktio.c

> @@ -919,16 +919,16 @@ static void test_recv_tmo(recv_tmo_mode_e mode)

>         ret = odp_pktout_queue(pktio_tx, &pktout_queue, 1);

>         CU_ASSERT_FATAL(ret > 0);

>

> +       ret = create_packets(pkt_tbl, pkt_seq, test_pkt_count, pktio_tx,

> +                            pktio_rx);

> +       CU_ASSERT_FATAL(ret == test_pkt_count);

> +

>         /* No packets sent yet, so should wait */

>         ns = 100 * ODP_TIME_MSEC_IN_NS;

> -       ret = recv_packets_tmo(pktio_rx, pkt_tbl, pkt_seq, 1, mode,

> +       ret = recv_packets_tmo(pktio_rx, &pkt_tbl[0], &pkt_seq[0], 1, mode,

>                                odp_pktin_wait_time(ns), ns);

>         CU_ASSERT(ret == 0);

>

> -       ret = create_packets(pkt_tbl, pkt_seq, test_pkt_count, pktio_tx,

> -                            pktio_rx);

> -       CU_ASSERT_FATAL(ret == test_pkt_count);

> -

>         ret = odp_pktout_send(pktout_queue, pkt_tbl, test_pkt_count);

>         CU_ASSERT_FATAL(ret == test_pkt_count);

>

> --

> 2.7.1.250.gff4ea60

>

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> https://lists.linaro.org/mailman/listinfo/lng-odp

>
Elo, Matias (Nokia - FI/Espoo) May 13, 2016, 8:58 a.m. UTC | #2 
> -----Original Message-----

> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT Maxim

> Uvarov

> Sent: Thursday, May 12, 2016 6:51 PM

> To: lng-odp@lists.linaro.org

> Subject: [lng-odp] [PATCH] validation: pktio: initialize pkt_seq

> 

> recv_packets_tmo() has if for not initialized

> pkt_seq array, while init is done in create_packets(),

> so we need just reorder that lines.

> https://bugs.linaro.org/show_bug.cgi?id=2222

> 

> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>

> ---

>  test/validation/pktio/pktio.c | 10 +++++-----

>  1 file changed, 5 insertions(+), 5 deletions(-)

> 

> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c

> index a51e0b2..90adc02 100644

> --- a/test/validation/pktio/pktio.c

> +++ b/test/validation/pktio/pktio.c

> @@ -919,16 +919,16 @@ static void test_recv_tmo(recv_tmo_mode_e mode)

>  	ret = odp_pktout_queue(pktio_tx, &pktout_queue, 1);

>  	CU_ASSERT_FATAL(ret > 0);

> 

> +	ret = create_packets(pkt_tbl, pkt_seq, test_pkt_count, pktio_tx,

> +			     pktio_rx);

> +	CU_ASSERT_FATAL(ret == test_pkt_count);

> +


I think it would clearer to simply initialize pkt_seq to 0 and leave create_packets() where it is now and the packets are actually used.

The pktio_pkt_seq() helper used inside recv_packets_tmo() checks also received packets' magic numbers, so the sequence number can be 0.

-Matias

>  	/* No packets sent yet, so should wait */

>  	ns = 100 * ODP_TIME_MSEC_IN_NS;

> -	ret = recv_packets_tmo(pktio_rx, pkt_tbl, pkt_seq, 1, mode,

> +	ret = recv_packets_tmo(pktio_rx, &pkt_tbl[0], &pkt_seq[0], 1, mode,

>  			       odp_pktin_wait_time(ns), ns);

>  	CU_ASSERT(ret == 0);

> 

> -	ret = create_packets(pkt_tbl, pkt_seq, test_pkt_count, pktio_tx,

> -			     pktio_rx);

> -	CU_ASSERT_FATAL(ret == test_pkt_count);

> -

>  	ret = odp_pktout_send(pktout_queue, pkt_tbl, test_pkt_count);

>  	CU_ASSERT_FATAL(ret == test_pkt_count);

> 

> --

> 2.7.1.250.gff4ea60

> 

> _______________________________________________

> lng-odp mailing list

> lng-odp@lists.linaro.org

> https://lists.linaro.org/mailman/listinfo/lng-odp
Maxim Uvarov May 13, 2016, 12:30 p.m. UTC | #3 
On 05/13/16 11:58, Elo, Matias (Nokia - FI/Espoo) wrote:
>> -----Original Message-----
>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT Maxim
>> Uvarov
>> Sent: Thursday, May 12, 2016 6:51 PM
>> To: lng-odp@lists.linaro.org
>> Subject: [lng-odp] [PATCH] validation: pktio: initialize pkt_seq
>>
>> recv_packets_tmo() has if for not initialized
>> pkt_seq array, while init is done in create_packets(),
>> so we need just reorder that lines.
>> https://bugs.linaro.org/show_bug.cgi?id=2222
>>
>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>> ---
>>   test/validation/pktio/pktio.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
>> index a51e0b2..90adc02 100644
>> --- a/test/validation/pktio/pktio.c
>> +++ b/test/validation/pktio/pktio.c
>> @@ -919,16 +919,16 @@ static void test_recv_tmo(recv_tmo_mode_e mode)
>>   	ret = odp_pktout_queue(pktio_tx, &pktout_queue, 1);
>>   	CU_ASSERT_FATAL(ret > 0);
>>
>> +	ret = create_packets(pkt_tbl, pkt_seq, test_pkt_count, pktio_tx,
>> +			     pktio_rx);
>> +	CU_ASSERT_FATAL(ret == test_pkt_count);
>> +
> I think it would clearer to simply initialize pkt_seq to 0 and leave create_packets() where it is now and the packets are actually used.
>
> The pktio_pkt_seq() helper used inside recv_packets_tmo() checks also received packets' magic numbers, so the sequence number can be 0.
>
> -Matias
yes, you are right.  I did  memset(0), initially but that thought it 
will be good to reorder lines. But did not catch this reference.
Will send v2.

Maxim.
>>   	/* No packets sent yet, so should wait */
>>   	ns = 100 * ODP_TIME_MSEC_IN_NS;
>> -	ret = recv_packets_tmo(pktio_rx, pkt_tbl, pkt_seq, 1, mode,
>> +	ret = recv_packets_tmo(pktio_rx, &pkt_tbl[0], &pkt_seq[0], 1, mode,
>>   			       odp_pktin_wait_time(ns), ns);
>>   	CU_ASSERT(ret == 0);
>>
>> -	ret = create_packets(pkt_tbl, pkt_seq, test_pkt_count, pktio_tx,
>> -			     pktio_rx);
>> -	CU_ASSERT_FATAL(ret == test_pkt_count);
>> -
>>   	ret = odp_pktout_send(pktout_queue, pkt_tbl, test_pkt_count);
>>   	CU_ASSERT_FATAL(ret == test_pkt_count);
>>
>> --
>> 2.7.1.250.gff4ea60
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
diff mbox

Patch

diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
index a51e0b2..90adc02 100644
--- a/test/validation/pktio/pktio.c
+++ b/test/validation/pktio/pktio.c
@@ -919,16 +919,16 @@  static void test_recv_tmo(recv_tmo_mode_e mode)
 	ret = odp_pktout_queue(pktio_tx, &pktout_queue, 1);
 	CU_ASSERT_FATAL(ret > 0);
 
+	ret = create_packets(pkt_tbl, pkt_seq, test_pkt_count, pktio_tx,
+			     pktio_rx);
+	CU_ASSERT_FATAL(ret == test_pkt_count);
+
 	/* No packets sent yet, so should wait */
 	ns = 100 * ODP_TIME_MSEC_IN_NS;
-	ret = recv_packets_tmo(pktio_rx, pkt_tbl, pkt_seq, 1, mode,
+	ret = recv_packets_tmo(pktio_rx, &pkt_tbl[0], &pkt_seq[0], 1, mode,
 			       odp_pktin_wait_time(ns), ns);
 	CU_ASSERT(ret == 0);
 
-	ret = create_packets(pkt_tbl, pkt_seq, test_pkt_count, pktio_tx,
-			     pktio_rx);
-	CU_ASSERT_FATAL(ret == test_pkt_count);
-
 	ret = odp_pktout_send(pktout_queue, pkt_tbl, test_pkt_count);
 	CU_ASSERT_FATAL(ret == test_pkt_count);