[ovs-dev] [PATCH v2 2/8] ofproto: Enable in-place modification for recirc actions.

[Ben Pfaff]

On Tue, Nov 24, 2015 at 02:25:47PM -0800, Jarno Rajahalme wrote:
> 
> > On Nov 24, 2015, at 1:53 PM, Ben Pfaff <blp at ovn.org> wrote:
> > 
> > On Tue, Nov 24, 2015 at 01:10:35PM -0800, Jarno Rajahalme wrote:
> >> 
> >>> On Nov 24, 2015, at 10:27 AM, Ben Pfaff <blp at ovn.org> wrote:
> >>> 
> >>> On Fri, Nov 06, 2015 at 04:10:49PM -0800, Jarno Rajahalme wrote:
> >>>> When modifying an existing datapath flow with recirculation actions,
> >>>> the references to old (if any) recirculation actions need to be freed,
> >>>> and references to new recirculation actions need to be stored.
> >>>> 
> >>> Here, it wasn't obvious to me why the logic changed from only allocating
> >>> a recirc_id if we have a packet, to always allocating one (don't we
> >>> still need to reuse the recirc id from a previous translation?):
> >> 
> >> The separation of the packet (upcall) and no packet (revalidation) was
> >> suitable before we added the support for modifying datapath flows
> >> in-place, when only actions change. Before, when doing revalidation
> >> the produced actions were only used for comparison, but now they can
> >> also be used as a replacement for the old datapath actions. This is
> >> why we now need to allocate and hold a reference to a recirculation
> >> context also when revalidating. The reference will be freed if the
> >> actions are freed without installing them to an existing flow. Also,
> >> the recirc_alloc_id_ctx() will reuse existing recirculation contexts
> >> (and adding a reference) if possible. I’ll update the comment to
> >> mention this.
> > 
> > If we always allocate a new recirc id, does that mean that the
> > revalidated flow will always differ from the original one?
> 
> recirc_alloc_id_ctx() will reuse existing recirculation contexts (and adding a reference) if possible, so it will return the same recirculation Id if the post-recirculation processing will be the same. This already makes it possible for two different upcall paths to get the same recirculation ID and to share the same post-recirculation flow.

Thanks for walking me through it.

Acked-by: Ben Pfaff <blp at ovn.org>