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> _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
