On 5 November 2014 13:31, Ben Pfaff <b...@nicira.com> wrote: > On Fri, Oct 31, 2014 at 04:55:39PM -0700, Joe Stringer wrote: > > Currently, when a revalidator thread first dumps a flow, it creates a > > 'udpif_key' object and caches a copy of a kernel flow key. This allows > > us to perform lookups in the classifier to attribute stats and validate > > the correctness of the datapath flow. > > > > This patch sets up this cache from the handler threads, during flow > > setup. While this patch alone causes a decrease in revalidation > > performance, it allows future patches increase performance by reducing > > the cost of flow dumping. > > > > Revalidators will continue to create ukeys if a flow is dumped that has > > no corresponding ukey. This may happen in corner cases such as when > > ovs-vswitchd is restarted (and flows remain in the datapath) or a user > > installs a flow using ovs-dpctl. > > > > Signed-off-by: Joe Stringer <joestrin...@nicira.com> > > Acked-by: Ben Pfaff <b...@nicira.com> > > I don't yet understand the ukey_persists member. It appears to only > ever be set to true by upcall_cb(). I think the means that only the > dpif-netdev path will ever create and keep ukeys, and that the > kernel-based datapath path will always delete ukeys soon after it > creates them. Is that correct? > > Oh, no, I think I see what's going on. With the kernel-datapath, we > set the ukey member to NULL instead so that the caller can't free it. > Is there a way we could make both paths do this the same way? >
Ah, good catch. I added this member to make it more explicit that the codepath was trying to keep the ukey around. Previously both of the netdev and dpif paths did the "set to NULL" trick that the dpif path uses. I didn't make the connection that this change would apply to the dpif path as well, so I can update that. Acked-by: Ben Pfaff <b...@nicira.com> Thanks for the review. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
