> On Nov 24, 2015, at 1:53 PM, Ben Pfaff <[email protected]> wrote: > > On Tue, Nov 24, 2015 at 01:10:35PM -0800, Jarno Rajahalme wrote: >> >>> On Nov 24, 2015, at 10:27 AM, Ben Pfaff <[email protected]> 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. Jarno _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
