[ovs-dev] [PATCH v4 03/12] classifier: Support table versioning
Jarno Rajahalme
jrajahalme at nicira.com
Wed Jun 10 23:24:04 UTC 2015
> On Jun 10, 2015, at 3:51 PM, Ben Pfaff <blp at nicira.com> wrote:
>
> On Tue, Jun 09, 2015 at 05:24:10PM -0700, Jarno Rajahalme wrote:
>> This patch allows classifier rules to become visible and invisible in
>> specific versions. A 'version' is defined as a positive monotonically
>> increasing integer, which never wraps around.
>>
>> The new 'visibility' attribute replaces the prior 'to_be_removed' and
>> 'visible' attributes.
>>
>> When versioning is not used, the 'version' parameter should be passed
>> as 'CLS_MIN_VERSION' when creating rules, and 'CLS_MAX_VERSION' when
>> looking up flows.
>>
>> This feature enables the support for atomic OpenFlow bundles without
>> significant performance penalty on 64-bit systems. There is a
>> performance decrease in 32-bit systems due to 64-bit atomics used.
>>
>> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
>
> The two comments
>
> /* clang does not want to read from a const atomic. */
>
> stand out to me a little, because they imply that this is a special
> property of Clang. I don't think that's true; rather, it's a
> consequence of the C standard definition, which defines atomic "load"
> operations as taking plain types, not const-qualified ones. I think
> that goes back, historically, to the definition of a "side effect" in C:
>
> Accessing a volatile object, modifying an object, modifying a file,
> or calling a function that does any of those operations are all side
> effects, which are changes in the state of the execution
> environment.
>
> That is, *any* access to a volatile object (not just a write) is a side
> effect (and thus cannot be optimized out by the compiler). I get the
> impression that a lot of behavior for atomic types in C11 is related to
> volatile types.
>
> Anyway, personally I'd either drop the comments or just s/clang/C11/.
>
Changed to:
/* C11 does not want to access an atomic via a const object pointer. */
> Acked-by: Ben Pfaff <blp at nicira.com <mailto:blp at nicira.com>>
Thanks for the review, pushed to master.
Jarno
More information about the dev
mailing list