[ovs-dev] knowing when a kernel flow is really deleted

Ben Pfaff blp at nicira.com
Fri Dec 16 17:22:44 UTC 2011


On Thu, Dec 15, 2011 at 05:21:23PM -0800, Jesse Gross wrote:
> On Thu, Dec 15, 2011 at 4:56 PM, Ben Pfaff <blp at nicira.com> wrote:
> > On Thu, Dec 15, 2011 at 04:49:57PM -0800, Jesse Gross wrote:
> >> >> > Could we just use the existing spinlock in sw_flow plus the 'dead'
> >> >> > variable to ensure that no packets go through a deleted flow after
> >> >> > it's deleted    On the delete side, take the spinlock and set 'dead'.
> >> >> > On the fast path, take the spinlock and check 'dead' before executing
> >> >> > the actions, release the spinlock after executing the actions.   We
> >> >> > already have to take the spinlock for every packet anyway to update
> >> >> > the stats, so it's not an additional cost.   I doubt that there's any
> >> >> > parallel work for a given flow anyway (that would imply packet
> >> >> > reordering).   We would have to avoid recursive locking somehow.
> >> >>
> >> >> Hmm, I think that would probably work.   It's slightly convoluted but
> >> >> certainly much less so than the alternatives.   How would you get
> >> >> recursive locking 
> >> >
> >> > Flow loops e.g. through patch ports was what came to mind.
> >>
> >> We don't execute any actions while the spin lock is held and trips
> >> through a patch port will appear as different flows, so I don't think
> >> that there is a possibility of recursive locking.   I think you
> >> basically do:
> >>
> >> For flow processing:
> >>
> >>   * Extract and lookup flow
> >>   * Take spin lock
> >>   * Check to see if flow is dead
> >>   * Else update stats
> >>   * Release spin lock
> >>   * If dead, act like you didn't find the flow
> >>   * Else execute actions
> >>
> >> For flow deletion:
> >>
> >>   * Take spin lock
> >>   * Read stats
> >>   * Mark flow dead
> >>   * Release spin lock
> >
> > My intention was to make "Release spin lock" the last step in both
> > cases, hence the possibility of recursion.   Otherwise, this change
> > ensures that stats are correct but side effects such as queueing a
> > copy of a packet to userspace can still happen after userspace sees
> > the reply to its deletion.
> 
> I was designing for the accurate stats case, I suppose you would need
> to hold onto the lock for the duration of flow execution in order to
> handle packets going to userspace.  I'm not sure how I feel about
> holding that spinlock for that whole time, including all the way
> through the rest of the network stack, although I suppose it's not too
> different from the TX queue lock on devices.  We could handle
> recursion the same way they do, by checking to see if the current CPU
> owns the lock it is trying to acquire.

Accurate stats might be a worthwhile improvement on their own.



More information about the dev mailing list