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:
/* Implements OFPFC_ADD and the cases for OFPFC_MODIFY and OFPFC_MODIFY_STRICT
* in which no matching flow already exists in the flow table.
*
* Adds the flow specified by 'fm', to the ofproto's flow table. Returns 0 on
* success, or an OpenFlow error code on failure.
*
* On successful return the caller must complete the operation either by
* calling add_flow_finish(), or add_flow_revert() if the operation needs to
* be reverted.
*
* The caller retains ownership of 'fm->ofpacts'. */
/* 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.
*
* 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.
*/
static enum ofperr
add_flow_init(struct ofproto *ofproto, struct ofproto_flow_mod *ofm,
const struct ofputil_flow_mod *fm)
OVS_EXCLUDED(ofproto_mutex)
Probably, this is an easy fix, but this code is complicated enough that
I'm reluctant to fully review it before the intent is clear from the
comments.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
