> On Aug 15, 2016, at 11:19 AM, Ben Pfaff <b...@ovn.org> wrote: > > On Mon, Aug 15, 2016 at 10:57:32AM -0700, Ben Pfaff wrote: >> On Mon, Aug 15, 2016 at 10:43:45AM -0700, Jarno Rajahalme wrote: >>> >>>> On Aug 14, 2016, at 5:34 PM, Ben Pfaff <b...@ovn.org> wrote: >>>> >>>> On Mon, Aug 08, 2016 at 11:26:30AM -0700, Jarno Rajahalme wrote: >>>>> Instead of storing the (big) struct ofputil_flow_mod, create the new >>>>> rule and/or create the rule criteria for matching at bundle message >>>>> insert time. This change reduces the size of a bundle flow mod from >>>>> 3.5kb to 272 bytes, not counting the created rule, which was anyway >>>>> created during bundle commit. >>>>> >>>>> In successful bundles this shifts work out of the ofproto_mutex >>>>> critical section and should thus reduce the time the mutex is held >>>>> during bundle commit. >>>>> >>>>> Signed-off-by: Jarno Rajahalme <ja...@ovn.org> >>>>> --- >>>>> v4: Fixed comments & memory leak. >>>> >>>> There's something really funny going on above add_flow_init(). It has >>>> two function comments: >>>> >>> >>> I forgot to consolidate the comments and fixed my copy to read: >>> >>> /* add_flow_init(), add_flow_start(), add_flow_revert(), and >>> add_flow_finish() >>> * implement OFPFC_ADD and the cases for OFPFC_MODIFY and OFPFC_MODIFY_STRICT >>> * in which no matching flow already exists in the flow table. >>> * >>> * add_flow_init() creates a new flow according to 'fm' and stores it to >>> 'ofm' >>> * for later reference. If the flow replaces other flow, it will be updated >>> to >>> * match modify semantics later by add_flow_start() (by calling >>> * replace_rule_start()). >>> * >>> * Returns 0 on success, or an OpenFlow error code on failure. >>> * >>> * On successful return the caller must complete the operation by calling >>> * add_flow_start(), and if that succeeds, then either add_flow_finish(), or >>> * add_flow_revert() if the operation needs to be reverted due to a later >>> * failure. >>> */ >>> >>> With this the intent should be clear, or do you want me to issue a new >>> version before you proceed with the review? >> >> I understand now, no revision needed. > > OK, I'm now satisfied with this. I'm certainly not 100% sure that it's > correct, but I'm happy with the approach, the code smells good, it > passes all the unit tests, and your track record gives me high > confidence in your code. > > Acked-by: Ben Pfaff <b...@ovn.org>
Thanks for the review & trust. I pushed this to master. Jarno _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
