Hello Nithin, The part where I am trying to save a lock is in "4. Process the packet using the flow"
Also, I believe that, when ref count = 0, it should mean that the data is "unsafe" (i.e. you need to lock to access / write to it). When ref count = 0, it shouldn't mean that the flow must be deleted. So let me modify the "packet processing context" workflow, to better explain my method: 1. Lock Flowtable (read) - this allows you to read flow table stats, get access to ptrs to flows 2. pFlow = FlowLookupRef() - have the function name mention it refs the object 3. Unlock Flowtable - now some other thread can write stats, add new flows, delete flows. pFlow cannot be deleted ATM because it's in use. 3.1. A possible OVS_REFCOUNT_DESTROY(pFlow, DeleteFunc) by another thread on this same flow marks the pFlow as "delete later" 4. Process the packet using the flow: 4.1. lock pFlow->lock for read 4.2. read state from pFlow 4.3. unlock pFlow->lock This part: 5. Lock Flowtable (write) 6. pFlow = FlowLookup(); or use FlowLookupRef() and deref before calling OVS_REFCOUNT_DESTROY. 7. OVS_REFCOUNT_DESTROY(pFlow, DeleteFunc) - if pFlow is being used by another thread (pFlow's refCount > 0), pFlow is marked for deletion; otherwise, it is destroyed instantly using the DeleteFunc func. 8. Unblock Flowtable. flow deletion should happen only when you get a FLOW/DELETE netlink command. So it's not part of the "packet processing context". [QUOTE] 2. Decrement the refcount on the flow: * if new refcount is 0, cleanup the flow by deleting it from the flowable. * if new refcount is 1, mark the flag on the flow as INACTIVE [/QUOTE] I personally find it to hinder clarity if we give special meaning to refCount =0 and to refCount = 1, so the first reference to the object would start at refCount = 2. Sam ________________________________________ From: Nithin Raju [nit...@vmware.com] Sent: Saturday, August 16, 2014 7:50 AM To: Samuel Ghinet Cc: dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH 11/15] datapath-windows: Add RefCount.h On Aug 6, 2014, at 9:14 AM, Samuel Ghinet <sghi...@cloudbasesolutions.com> wrote: > How it works: > Say we have a list of OVS_FLOWs, where OVS_FLOW uses OVS_REF_COUNT. > > * The OVS_REF_COUNT must be initialized to 0 at object creation. > * Assign a destruction function to the func ptr "Destroy" of OVS_REF_COUNT. > * When you need to retrieve a ptr to an OVS_FLOW, from the list, you must do > the following: > *** lock the OVS_FLOW list for read (obviously) > *** do the lookup: a ptr to an existing OVS_FLOW, pFlow, is found. > *** call "OVS_REFCOUNT_REFERENCE(pFlow);" - by doing so, you deny destruction > of pFlow, while the object > is referenced, ensuring that you keep a valid pointer to a flow. > *** unlock the list > *** return pFlow. > * When you need to modify fields in the pFlow, use the rw lock / spinlock > field of pFlow. > * When you no longer need the ptr, use "OVS_REFCOUNT_DEREFERENCE(pFlow);" > * When you need to do deferred deletion, call "OVS_REFCOUNT_DESTROY(pFlow);": > this will > mark the pFlow for deletion. If the object is not referenced, it will be > destroyed immediately. Otherwise, > it will be destroyed when the refCount = 0. Sam, This is what I have in mind on how the workflow should look like. Nothing fancy, just a standard way of doing ref counting: Context #1: packet processing context ------------------------------------- 1. Lock Flowtable (read) 2. pFlow = FlowLookup() * This one checks if the flag on the flow is ACTIVE * Increment the refcount on the flow 3. Unlock Flowtable 4. Process the packet using the flow 5. Lock Flowtable (write) 6. Decrement the refcount on the flow. * if the new refcount == 0, cleanup the flow by deleting it from the flowable. 7. Unblock Flowtable. Context #2: flow put context ---------------------------- 1. Lock Flowtable (write) 2. Decrement the refcount on the flow: * if new refcount is 0, cleanup the flow by deleting it from the flowable. * if new refcount is 1, mark the flag on the flow as INACTIVE 3. Unblock Flowtable. This is a very simple usage of refcounting. Once we agree on this, we can work out the API specifics etc. Pls. let me know if you have other ideas. thanks, Nithin _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
