Thanks.
I'm going to sit on this for a while to give the fixed code a chance
to get tested.
On Tue, Jan 10, 2012 at 05:01:08PM -0800, Ethan Jackson wrote:
> 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 <b...@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 <b...@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@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
