diff mbox series

[v5,12/15] distributor: fix scalar matching

Message ID 20201008052323.11547-13-l.wojciechow@partner.samsung.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series fix distributor synchronization issues |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Lukasz Wojciechowski Oct. 8, 2020, 5:23 a.m. UTC 
  Fix improper indexes while comparing tags.
In the find_match_scalar() function:
* j iterates over flow tags of following packets;
* w iterates over backlog or in flight tags positions.

Fixes: 775003ad2f96 ("distributor: add new burst-capable library")
Cc: david.hunt@intel.com
Cc: stable@dpdk.org

Signed-off-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
---
 lib/librte_distributor/rte_distributor.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Hunt, David Oct. 9, 2020, 12:31 p.m. UTC | #1 
On 8/10/2020 6:23 AM, Lukasz Wojciechowski wrote:
> Fix improper indexes while comparing tags.
> In the find_match_scalar() function:
> * j iterates over flow tags of following packets;
> * w iterates over backlog or in flight tags positions.
>
> Fixes: 775003ad2f96 ("distributor: add new burst-capable library")
> Cc: david.hunt@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> ---
>   lib/librte_distributor/rte_distributor.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_distributor/rte_distributor.c b/lib/librte_distributor/rte_distributor.c
> index 9fd7dcab7..4bd23a990 100644
> --- a/lib/librte_distributor/rte_distributor.c
> +++ b/lib/librte_distributor/rte_distributor.c
> @@ -261,13 +261,13 @@ find_match_scalar(struct rte_distributor *d,
>   
>   		for (j = 0; j < RTE_DIST_BURST_SIZE ; j++)
>   			for (w = 0; w < RTE_DIST_BURST_SIZE; w++)
> -				if (d->in_flight_tags[i][j] == data_ptr[w]) {
> +				if (d->in_flight_tags[i][w] == data_ptr[j]) {
>   					output_ptr[j] = i+1;
>   					break;
>   				}
>   		for (j = 0; j < RTE_DIST_BURST_SIZE; j++)
>   			for (w = 0; w < RTE_DIST_BURST_SIZE; w++)
> -				if (bl->tags[j] == data_ptr[w]) {
> +				if (bl->tags[w] == data_ptr[j]) {
>   					output_ptr[j] = i+1;
>   					break;
>   				}

Hi Lukasz,

Could you give a bit more information on the problem that this is fixing?

Were you finding that flows were not being assigned to workers correctly 
in the scalar code?

Thanks,
Dave.
Hunt, David Oct. 9, 2020, 12:35 p.m. UTC | #2 
Hi Lukasz,

On 9/10/2020 1:31 PM, David Hunt wrote:
>
> On 8/10/2020 6:23 AM, Lukasz Wojciechowski wrote:
>> Fix improper indexes while comparing tags.
>> In the find_match_scalar() function:
>> * j iterates over flow tags of following packets;
>> * w iterates over backlog or in flight tags positions.
>>
>> Fixes: 775003ad2f96 ("distributor: add new burst-capable library")
>> Cc: david.hunt@intel.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
>> ---
>>   lib/librte_distributor/rte_distributor.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/librte_distributor/rte_distributor.c 
>> b/lib/librte_distributor/rte_distributor.c
>> index 9fd7dcab7..4bd23a990 100644
>> --- a/lib/librte_distributor/rte_distributor.c
>> +++ b/lib/librte_distributor/rte_distributor.c
>> @@ -261,13 +261,13 @@ find_match_scalar(struct rte_distributor *d,
>>             for (j = 0; j < RTE_DIST_BURST_SIZE ; j++)
>>               for (w = 0; w < RTE_DIST_BURST_SIZE; w++)
>> -                if (d->in_flight_tags[i][j] == data_ptr[w]) {
>> +                if (d->in_flight_tags[i][w] == data_ptr[j]) {
>>                       output_ptr[j] = i+1;
>>                       break;
>>                   }
>>           for (j = 0; j < RTE_DIST_BURST_SIZE; j++)
>>               for (w = 0; w < RTE_DIST_BURST_SIZE; w++)
>> -                if (bl->tags[j] == data_ptr[w]) {
>> +                if (bl->tags[w] == data_ptr[j]) {
>>                       output_ptr[j] = i+1;
>>                       break;
>>                   }
>
> Hi Lukasz,
>
> Could you give a bit more information on the problem that this is fixing?
>
> Were you finding that flows were not being assigned to workers 
> correctly in the scalar code?
>
>

You answer this question in the next patch in the series, as you are 
adding a test to check the flows go to the correct workers, etc. You can 
igonore this question, and:

Acked-by: David Hunt <david.hunt@intel.com>
Lukasz Wojciechowski Oct. 9, 2020, 9:02 p.m. UTC | #3 
W dniu 09.10.2020 o 14:35, David Hunt pisze:
> Hi Lukasz,
>
> On 9/10/2020 1:31 PM, David Hunt wrote:
>>
>> On 8/10/2020 6:23 AM, Lukasz Wojciechowski wrote:
>>> Fix improper indexes while comparing tags.
>>> In the find_match_scalar() function:
>>> * j iterates over flow tags of following packets;
>>> * w iterates over backlog or in flight tags positions.
>>>
>>> Fixes: 775003ad2f96 ("distributor: add new burst-capable library")
>>> Cc: david.hunt@intel.com
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
>>> ---
>>>   lib/librte_distributor/rte_distributor.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/librte_distributor/rte_distributor.c 
>>> b/lib/librte_distributor/rte_distributor.c
>>> index 9fd7dcab7..4bd23a990 100644
>>> --- a/lib/librte_distributor/rte_distributor.c
>>> +++ b/lib/librte_distributor/rte_distributor.c
>>> @@ -261,13 +261,13 @@ find_match_scalar(struct rte_distributor *d,
>>>             for (j = 0; j < RTE_DIST_BURST_SIZE ; j++)
>>>               for (w = 0; w < RTE_DIST_BURST_SIZE; w++)
>>> -                if (d->in_flight_tags[i][j] == data_ptr[w]) {
>>> +                if (d->in_flight_tags[i][w] == data_ptr[j]) {
>>>                       output_ptr[j] = i+1;
>>>                       break;
>>>                   }
>>>           for (j = 0; j < RTE_DIST_BURST_SIZE; j++)
>>>               for (w = 0; w < RTE_DIST_BURST_SIZE; w++)
>>> -                if (bl->tags[j] == data_ptr[w]) {
>>> +                if (bl->tags[w] == data_ptr[j]) {
>>>                       output_ptr[j] = i+1;
>>>                       break;
>>>                   }
>>
>> Hi Lukasz,
>>
>> Could you give a bit more information on the problem that this is 
>> fixing?
>>
>> Were you finding that flows were not being assigned to workers 
>> correctly in the scalar code?
>>
>>
>
> You answer this question in the next patch in the series, as you are 
> adding a test to check the flows go to the correct workers, etc. You 
> can igonore this question, and:
>
> Acked-by: David Hunt <david.hunt@intel.com>
>
Thanks for the ack.

And you already probably know the answer about flows, but let me show an 
example:

worker 0 tags:   3 5 7 0 0 0 0 0
incoming flow:   1 2 3 4 5 6 7 8
expected result: 0 0 1 0 1 0 1 0
unfixed result:  1 1 1 0 0 0 0 0

The tags were iterated with "j" variable same that indexed the result table


Best regards

Lukasz

>
>
diff mbox series

Patch

diff --git a/lib/librte_distributor/rte_distributor.c b/lib/librte_distributor/rte_distributor.c
index 9fd7dcab7..4bd23a990 100644
--- a/lib/librte_distributor/rte_distributor.c
+++ b/lib/librte_distributor/rte_distributor.c
@@ -261,13 +261,13 @@  find_match_scalar(struct rte_distributor *d,
 
 		for (j = 0; j < RTE_DIST_BURST_SIZE ; j++)
 			for (w = 0; w < RTE_DIST_BURST_SIZE; w++)
-				if (d->in_flight_tags[i][j] == data_ptr[w]) {
+				if (d->in_flight_tags[i][w] == data_ptr[j]) {
 					output_ptr[j] = i+1;
 					break;
 				}
 		for (j = 0; j < RTE_DIST_BURST_SIZE; j++)
 			for (w = 0; w < RTE_DIST_BURST_SIZE; w++)
-				if (bl->tags[j] == data_ptr[w]) {
+				if (bl->tags[w] == data_ptr[j]) {
 					output_ptr[j] = i+1;
 					break;
 				}