[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