[ovs-dev] [PATCH] classifier: Simplify versioning.

[Gurucharan Shetty]

On Thu, Jun 11, 2015 at 6:20 PM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>
>> On Jun 11, 2015, at 5:58 PM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>>
>>
>>> On Jun 11, 2015, at 5:51 PM, Gurucharan Shetty <shettyg at nicira.com> wrote:
>>>
>>> On Thu, Jun 11, 2015 at 5:07 PM, Jarno Rajahalme <jrajahalme at nicira.com> wrote:
>>>>
>>>>
>>>>> On Jun 11, 2015, at 16:53, Ben Pfaff <blp at nicira.com> wrote:
>>>>>
>>>>>> On Thu, Jun 11, 2015 at 04:45:31PM -0700, Jarno Rajahalme wrote:
>>>>>> Also, avoid using type larger than int in an enum, as it is not
>>>>>> portable C.
>>>>>
>>>>> Did we establish that that was the problem?
>>>>
>>>> Haven't heard from Guru yet, took your word for it. Anyway, with this it will be easy to change the version to uint32_t for testing purposes, if the change from enum to a macro was not the fix.
>>>
>>> Looks like this patch does help in reducing the number of crashes. But
>>> there are a lot more tests that still crash with the series. The
>>> bundle related tests fail too. I will try to get more backtraces.

Long story, short: This patch (without the additional incremental
provided) fixes all the crashes seen before. I do have the following 3
unit test failures that I still see, which I will look into to provide
more context.


775: ofproto - failing bundle commit (OpenFlow 1.4)  FAILED (ofproto.at:3678)
774: ofproto - bundle with multiple flow mods (OpenFlow 1.4) FAILED (ofproto.at:
3513)
368: ovs-ofctl replace-flows with --bundle           FAILED (ovs-ofctl.at:2837)



>>
>>
>> Try this on top and tell if it makes any difference:
>>
>> --- a/lib/classifier.h
>> +++ b/lib/classifier.h
>> @@ -326,12 +326,12 @@ struct cls_trie {
>>     rcu_trie_ptr root;            /* NULL if none. */
>> };
>>
>> -typedef uint64_t cls_version_t;
>> +typedef uint32_t cls_version_t;
>>
>> #define CLS_NO_VERSION 0
>> #define CLS_MIN_VERSION 1                  /* Default version number to use. */
>> -#define CLS_MAX_VERSION (UINT64_MAX - 1)
>> -#define CLS_NOT_REMOVED_VERSION UINT64_MAX
>> +#define CLS_MAX_VERSION (UINT32_MAX - 1)
>> +#define CLS_NOT_REMOVED_VERSION UINT32_MAX
>>
>> enum {
>>     CLS_MAX_INDICES = 3,   /* Maximum number of lookup indices per subtable. */
>>
>
> For what it’s worth, on Linux 32-bit build (gcc -m32) master, this patch, and this patch with the above incremental all successfully pass the test suite.
>
>   Jarno
>
>>>>
>>>> Jarno
>>>> _______________________________________________
>>>> dev mailing list
>>>> dev at openvswitch.org
>>>> http://openvswitch.org/mailman/listinfo/dev
>>
>