> On Oct 17, 2016, at 2:10 AM, Fischetti, Antonio <antonio.fische...@intel.com>
> wrote:
>
> Thanks Jarno, one question below.
>
> Antonio
>
>> -----Original Message-----
>> From: dev [mailto:dev-boun...@openvswitch.org
>> <mailto:dev-boun...@openvswitch.org>] On Behalf Of Jarno
>> Rajahalme
>> Sent: Friday, October 14, 2016 5:03 PM
>> To: Bodireddy, Bhanuprakash <bhanuprakash.bodire...@intel.com
>> <mailto:bhanuprakash.bodire...@intel.com>>
>> Cc: dev@openvswitch.org <mailto:dev@openvswitch.org>
>> Subject: Re: [ovs-dev] [PATCH v3 03/12] flow: Skip invoking expensive
>> count_1bits() with zero input.
>>
>>
>>> On Oct 14, 2016, at 7:37 AM, Bhanuprakash Bodireddy
>> <bhanuprakash.bodire...@intel.com> wrote:
>>>
>>> This patch checks if trash is non-zero and only then resets the flowmap
>>> bit and increment the pointer by set bits as found in trash.
>>>
>>> Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodire...@intel.com>
>>> Co-authored-by: Antonio Fischetti <antonio.fische...@intel.com>
>>> Signed-off-by: Antonio Fischetti <antonio.fische...@intel.com>
>>> Acked-by: Jarno Rajahalme <ja...@ovn.org>
>>> ---
>>> lib/flow.h | 15 ++++++++++-----
>>> 1 file changed, 10 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/lib/flow.h b/lib/flow.h
>>> index 5a14941..74e75d6 100644
>>> --- a/lib/flow.h
>>> +++ b/lib/flow.h
>>> @@ -614,11 +614,16 @@ mf_get_next_in_map(struct mf_for_each_in_map_aux
>> *aux,
>>> * to ‘rm1bit’. */
>>> map_t trash = *fmap & (rm1bit - 1);
>>>
>>> - *fmap -= trash;
>>> - /* count_1bits() is fast for systems where speed matters (e.g.,
>>> - * DPDK), so we don't try avoid using it.
>>> - * Advance 'aux->values' to point to the value for 'rm1bit'. */
>>> - aux->values += count_1bits(trash);
>>> + /* Avoid resetting 'fmap' and calling count_1bits() when trash
>> is
>>> + * zero. */
>>> + if (trash) {
>>> + *fmap -= trash;
>>> + /* count_1bits() is fast for systems where speed matters
>> (e.g.,
>>> + * DPDK), so we don't try avoid using it.
>>
>> The comment above is still wrong as we now test ‘trash’ for non-zero
>> above.
>>
>> Jarno
> [Antonio F] Just to avoid misunderstanding, do you mean we can now entirely
> remove
> the existing comment
> /* count_1bits() is fast for systems where speed....
> right?
> Because with the latest changes now we do try to avoid using count_1bits().
> Is that ok?
>
>
Yes.
Jarno
>>
>>> + * Advance 'aux->values' to point to the value for 'rm1bit'
>> only.
>>> + */
>>> + aux->values += count_1bits(trash);
>>> + }
>>>
>>> *value = *aux->values;
>>> } else {
>>> --
>>> 2.4.11
>>>
>>> _______________________________________________
>>> dev mailing list
>>> dev@openvswitch.org
>>> http://openvswitch.org/mailman/listinfo/dev
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org <mailto:dev@openvswitch.org>
>> http://openvswitch.org/mailman/listinfo/dev
>> <http://openvswitch.org/mailman/listinfo/dev>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
