[ovs-dev] [PATCH] ofproto-dpif: Restore optimization for no-actions case (without leak).

Ethan Jackson ethan at nicira.com
Wed Jan 11 01:01:08 UTC 2012


The memory management in this function is a bit confusing so I had to
think about it a bit, but this looks correct to me.

Ethan

On Tue, Jan 10, 2012 at 15:39, Ben Pfaff <blp at nicira.com> wrote:
> Commit 968131c1809 (ofproto-dpif: Omit "execute" operation entirely when
> there are no actions.) introduced an optimization for the case where a
> flow translated to ODP actions had no actions at all (i.e. the packet is
> to be dropped).  It also introduced a memory leak (the packet was not
> freed).
>
> Commit 999fba59afd (ofproto-dpif: Implement PACKET_IN in userspace.)
> inadvertently removed the optimization and as a side effect fixed the
> memory leak.
>
> This commit restores the optimization but not the memory leak.
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  ofproto/ofproto-dpif.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index f5280ff..3401b1e 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -2537,7 +2537,6 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct flow_miss *miss,
>         struct flow_miss_op *op;
>         struct dpif_execute *execute;
>
> -        list_remove(&packet->list_node);
>         ofproto->n_matches++;
>
>         if (facet->rule->up.cr.priority == FAIL_OPEN_PRIORITY) {
> @@ -2561,6 +2560,12 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct flow_miss *miss,
>         dpif_flow_stats_extract(&facet->flow, packet, &stats);
>         subfacet_update_stats(ofproto, subfacet, &stats);
>
> +        if (!subfacet->actions_len) {
> +            /* No actions to execute, so skip talking to the dpif. */
> +            continue;
> +        }
> +
> +        list_remove(&packet->list_node);
>         if (flow->vlan_tci != subfacet->initial_tci) {
>             /* This packet was received on a VLAN splinter port.  We added
>              * a VLAN to the packet to make the packet resemble the flow,
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list