[ovs-dev] [PATCH 11/15] datapath-windows: Add RefCount.h

[Samuel Ghinet]

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 [nithin at vmware.com]
Sent: Saturday, August 16, 2014 7:50 AM
To: Samuel Ghinet
Cc: dev at openvswitch.org
Subject: Re: [ovs-dev] [PATCH 11/15] datapath-windows: Add RefCount.h

On Aug 6, 2014, at 9:14 AM, Samuel Ghinet <sghinet at 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