> On Aug 29, 2016, at 2:40 PM, Ben Pfaff <b...@ovn.org> wrote: > > On Mon, Aug 22, 2016 at 04:31:31PM -0700, Jarno Rajahalme wrote: >> As a rule may not be re-inserted to ofproto data structures, it is >> cleaner to have three states for the rule, rather than just two. This >> will be useful for managing learned flows in later patches. >> >> Signed-off-by: Jarno Rajahalme <ja...@ovn.org> > > If you want the OVS_PACKED_ENUM optimization to be useful for enum > rule_state, then the member needs to be near other small members, like > its previous location just after the uint8_t table_id. Putting it > between a struct ovs_refcount and an ovs_be64 isn't going to provide the > space benefits. >
Placing it in a different place was (mis)guided by the addition of GUARDED_BY. Thanks for pointing this out. > Before this patch, ->removed was written without taking the rule's lock. > After this patch, ->state is written only with the rule's lock. Is this > necessary? Does it fix a bug? > No, there is no bug, but may help preventing future bugs. I also made this cleaner by changing to GUARDED_BY(ofproto_mutex), as the state variable is only ever needed when handling a flow mod. This also removes the need for any additional locking. > Thanks, > > Ben. Thanks for the review, Jarno _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
