[API-NEXT,PATCHv3,4/4] validation: packet: test packet reference count

Message ID 1442389983-17007-5-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov Sept. 16, 2015, 7:53 a.m.
Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 test/validation/packet/packet.c | 36 ++++++++++++++++++++++++++++++++++++
 test/validation/packet/packet.h |  1 +
 2 files changed, 37 insertions(+)

Comments

Nicolas Morey-Chaisemartin Oct. 5, 2015, 8:58 a.m. | #1
On 09/16/2015 09:53 AM, Maxim Uvarov wrote:
> +	pkt_ref = odp_packet_create_ref(pkt);
> +	/* Handles should be different */
> +	CU_ASSERT(pkt != pkt_ref);
> +	/* Debug print also should have refcount bits */
> +	CU_ASSERT(odp_packet_to_u64(pkt) !=
> +		  odp_packet_to_u64(pkt_ref));
> +
Isn't this implementation specific ?
We replaced a lot of handles with pointers for performance reasons. In that case, both pkt refs would be the same.

Nicolas
Savolainen, Petri (Nokia - FI/Espoo) Oct. 5, 2015, 10:25 a.m. | #2
> -----Original Message-----

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

> Nicolas Morey-Chaisemartin

> Sent: Monday, October 05, 2015 11:59 AM

> To: Maxim Uvarov; lng-odp@lists.linaro.org

> Subject: Re: [lng-odp] [API-NEXT PATCHv3 4/4] validation: packet: test

> packet reference count

> 

> 

> 

> On 09/16/2015 09:53 AM, Maxim Uvarov wrote:

> > +	pkt_ref = odp_packet_create_ref(pkt);

> > +	/* Handles should be different */

> > +	CU_ASSERT(pkt != pkt_ref);

> > +	/* Debug print also should have refcount bits */

> > +	CU_ASSERT(odp_packet_to_u64(pkt) !=

> > +		  odp_packet_to_u64(pkt_ref));

> > +

> Isn't this implementation specific ?

> We replaced a lot of handles with pointers for performance reasons. In that

> case, both pkt refs would be the same.

> 

> Nicolas


Yes, it is. Handle may or may not be the same. Application must not expect anything about the handle value. Xxx_u64() is only for printing/logging debug information.

-Petri
Maxim Uvarov Oct. 5, 2015, 11:26 a.m. | #3
On 10/05/15 13:25, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>> -----Original Message-----
>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT
>> Nicolas Morey-Chaisemartin
>> Sent: Monday, October 05, 2015 11:59 AM
>> To: Maxim Uvarov; lng-odp@lists.linaro.org
>> Subject: Re: [lng-odp] [API-NEXT PATCHv3 4/4] validation: packet: test
>> packet reference count
>>
>>
>>
>> On 09/16/2015 09:53 AM, Maxim Uvarov wrote:
>>> +	pkt_ref = odp_packet_create_ref(pkt);
>>> +	/* Handles should be different */
>>> +	CU_ASSERT(pkt != pkt_ref);
>>> +	/* Debug print also should have refcount bits */
>>> +	CU_ASSERT(odp_packet_to_u64(pkt) !=
>>> +		  odp_packet_to_u64(pkt_ref));
>>> +
>> Isn't this implementation specific ?
>> We replaced a lot of handles with pointers for performance reasons. In that
>> case, both pkt refs would be the same.
>>
>> Nicolas
> Yes, it is. Handle may or may not be the same. Application must not expect anything about the handle value. Xxx_u64() is only for printing/logging debug information.
>
> -Petri
>
Is that patches good if handles checks will be removed? If you - I will 
send updated version.

Maxim.
Nicolas Morey-Chaisemartin Oct. 5, 2015, 11:50 a.m. | #4
With both these asserts remove, it's good for me.

Nicolas

On 10/05/2015 01:26 PM, Maxim Uvarov wrote:
> On 10/05/15 13:25, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>>> -----Original Message-----
>>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT
>>> Nicolas Morey-Chaisemartin
>>> Sent: Monday, October 05, 2015 11:59 AM
>>> To: Maxim Uvarov; lng-odp@lists.linaro.org
>>> Subject: Re: [lng-odp] [API-NEXT PATCHv3 4/4] validation: packet: test
>>> packet reference count
>>>
>>>
>>>
>>> On 09/16/2015 09:53 AM, Maxim Uvarov wrote:
>>>> +    pkt_ref = odp_packet_create_ref(pkt);
>>>> +    /* Handles should be different */
>>>> +    CU_ASSERT(pkt != pkt_ref);
>>>> +    /* Debug print also should have refcount bits */
>>>> +    CU_ASSERT(odp_packet_to_u64(pkt) !=
>>>> +          odp_packet_to_u64(pkt_ref));
>>>> +
>>> Isn't this implementation specific ?
>>> We replaced a lot of handles with pointers for performance reasons. In that
>>> case, both pkt refs would be the same.
>>>
>>> Nicolas
>> Yes, it is. Handle may or may not be the same. Application must not expect anything about the handle value. Xxx_u64() is only for printing/logging debug information.
>>
>> -Petri
>>
> Is that patches good if handles checks will be removed? If you - I will send updated version.
>
> Maxim.

Patch

diff --git a/test/validation/packet/packet.c b/test/validation/packet/packet.c
index 0c749c3..94d03e0 100644
--- a/test/validation/packet/packet.c
+++ b/test/validation/packet/packet.c
@@ -778,6 +778,41 @@  void packet_test_offset(void)
 	CU_ASSERT_PTR_NOT_NULL(ptr);
 }
 
+void packet_test_ref(void)
+{
+	odp_packet_t pkt = test_packet;
+	odp_packet_t pkt_ref;
+	uint32_t pkt_len = odp_packet_len(pkt);
+	uint8_t *data_buf1;
+	uint8_t *data_buf2;
+
+	data_buf1 = malloc(pkt_len);
+	CU_ASSERT_PTR_NOT_NULL_FATAL(data_buf1);
+	data_buf2 = malloc(pkt_len);
+	CU_ASSERT_PTR_NOT_NULL_FATAL(data_buf2);
+
+	pkt_ref = odp_packet_create_ref(pkt);
+	/* Handles should be different */
+	CU_ASSERT(pkt != pkt_ref);
+	/* Debug print also should have refcount bits */
+	CU_ASSERT(odp_packet_to_u64(pkt) !=
+		  odp_packet_to_u64(pkt_ref));
+
+	/* Check that packet data is the same for pkt and pkt_ref */
+	CU_ASSERT(!odp_packet_copydata_out(pkt, 0, pkt_len, data_buf1));
+	CU_ASSERT(!odp_packet_copydata_out(pkt_ref, 0, pkt_len, data_buf2));
+	CU_ASSERT(!memcmp(data_buf1, data_buf2, pkt_len));
+
+	odp_packet_free(pkt_ref);
+
+	/* Check original packet data after free reference. */
+	CU_ASSERT(!odp_packet_copydata_out(pkt, 0, pkt_len, data_buf2));
+	CU_ASSERT(!memcmp(data_buf1, data_buf2, pkt_len));
+
+	free(data_buf1);
+	free(data_buf2);
+}
+
 CU_TestInfo packet_suite[] = {
 	_CU_TEST_INFO(packet_test_alloc_free),
 	_CU_TEST_INFO(packet_test_alloc_segmented),
@@ -797,6 +832,7 @@  CU_TestInfo packet_suite[] = {
 	_CU_TEST_INFO(packet_test_copy),
 	_CU_TEST_INFO(packet_test_copydata),
 	_CU_TEST_INFO(packet_test_offset),
+	_CU_TEST_INFO(packet_test_ref),
 	CU_TEST_INFO_NULL,
 };
 
diff --git a/test/validation/packet/packet.h b/test/validation/packet/packet.h
index f8a16a8..d1feff8 100644
--- a/test/validation/packet/packet.h
+++ b/test/validation/packet/packet.h
@@ -28,6 +28,7 @@  void packet_test_add_rem_data(void);
 void packet_test_copy(void);
 void packet_test_copydata(void);
 void packet_test_offset(void);
+void packet_test_ref(void);
 
 /* test arrays: */
 extern CU_TestInfo packet_suite[];