[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