validation: pktio: do lookup before create

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

Commit Message

Maxim Uvarov June 3, 2015, 4:06 p.m.
Use the right order in test: first lookup for pktio,
if it's not exist - create it.

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

Comments

Stuart Haslam June 3, 2015, 4:53 p.m. | #1
On Wed, Jun 03, 2015 at 07:06:01PM +0300, Maxim Uvarov wrote:
> Use the right order in test: first lookup for pktio,
> if it's not exist - create it.
> 
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>  test/validation/odp_pktio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c
> index e1025d6..227ae83 100644
> --- a/test/validation/odp_pktio.c
> +++ b/test/validation/odp_pktio.c
> @@ -225,9 +225,9 @@ static odp_pktio_t create_pktio(const char *iface, int num)
>  {
>  	odp_pktio_t pktio;
>  
> -	pktio = odp_pktio_open(iface, pool[num]);
> +	pktio = odp_pktio_lookup(iface);
>  	if (pktio == ODP_PKTIO_INVALID)
> -		pktio = odp_pktio_lookup(iface);
> +		pktio = odp_pktio_open(iface, pool[num]);
>  	CU_ASSERT(pktio != ODP_PKTIO_INVALID);
>  	CU_ASSERT(odp_pktio_to_u64(pktio) !=
>  		  odp_pktio_to_u64(ODP_PKTIO_INVALID));
> -- 
> 1.9.1
>

Is how it is actually causing a problem? It was intentionally that way
around to avoid a race condition, with the lookup->open sequence two
threads may both fail on the lookup and then both call open and only one
thread ends up with a valid handle. If they call open first, one will
succeed and the other fails then calls lookup which should succeed so
both threads get a valid handle.

Granted this particular code is currently single threaded, but I prefer
the open->lookup sequence generally.
Maxim Uvarov June 4, 2015, 8:33 a.m. | #2
On 06/03/15 19:53, Stuart Haslam wrote:
> On Wed, Jun 03, 2015 at 07:06:01PM +0300, Maxim Uvarov wrote:
>> Use the right order in test: first lookup for pktio,
>> if it's not exist - create it.
>>
>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>> ---
>>   test/validation/odp_pktio.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c
>> index e1025d6..227ae83 100644
>> --- a/test/validation/odp_pktio.c
>> +++ b/test/validation/odp_pktio.c
>> @@ -225,9 +225,9 @@ static odp_pktio_t create_pktio(const char *iface, int num)
>>   {
>>   	odp_pktio_t pktio;
>>   
>> -	pktio = odp_pktio_open(iface, pool[num]);
>> +	pktio = odp_pktio_lookup(iface);
>>   	if (pktio == ODP_PKTIO_INVALID)
>> -		pktio = odp_pktio_lookup(iface);
>> +		pktio = odp_pktio_open(iface, pool[num]);
>>   	CU_ASSERT(pktio != ODP_PKTIO_INVALID);
>>   	CU_ASSERT(odp_pktio_to_u64(pktio) !=
>>   		  odp_pktio_to_u64(ODP_PKTIO_INVALID));
>> -- 
>> 1.9.1
>>
> Is how it is actually causing a problem? It was intentionally that way
> around to avoid a race condition, with the lookup->open sequence two
> threads may both fail on the lookup and then both call open and only one
> thread ends up with a valid handle. If they call open first, one will
> succeed and the other fails then calls lookup which should succeed so
> both threads get a valid handle.
>
> Granted this particular code is currently single threaded, but I prefer
> the open->lookup sequence generally.
>
No, there was no any problem. I just thought that it will be logical to 
do lookup
first then open in single thread app. But I'm also ok to drop this patch 
in case
if we will add more threads in future.

Maxim.

Patch

diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c
index e1025d6..227ae83 100644
--- a/test/validation/odp_pktio.c
+++ b/test/validation/odp_pktio.c
@@ -225,9 +225,9 @@  static odp_pktio_t create_pktio(const char *iface, int num)
 {
 	odp_pktio_t pktio;
 
-	pktio = odp_pktio_open(iface, pool[num]);
+	pktio = odp_pktio_lookup(iface);
 	if (pktio == ODP_PKTIO_INVALID)
-		pktio = odp_pktio_lookup(iface);
+		pktio = odp_pktio_open(iface, pool[num]);
 	CU_ASSERT(pktio != ODP_PKTIO_INVALID);
 	CU_ASSERT(odp_pktio_to_u64(pktio) !=
 		  odp_pktio_to_u64(ODP_PKTIO_INVALID));