[ovs-dev] [PATCHv2] revalidator: Prevent handling the same flow twice.

Joe Stringer joestringer at nicira.com
Thu Apr 24 00:20:46 UTC 2014


Pushed to master and branch-2.1.


On 24 April 2014 11:06, Joe Stringer <joestringer at nicira.com> wrote:

> On 24 April 2014 04:56, Alex Wang <alexw at nicira.com> wrote:
>
>>
>> On Tue, Apr 22, 2014 at 10:24 PM, Alex Wang <alexw at nicira.com> wrote:
>>
>>> diff --git a/ofproto/ofproto-dpif-upcall.c
>>>> b/ofproto/ofproto-dpif-upcall.c
>>>> index 416cfdc..906bf17 100644
>>>> --- a/ofproto/ofproto-dpif-upcall.c
>>>> +++ b/ofproto/ofproto-dpif-upcall.c
>>>> @@ -45,6 +45,8 @@
>>>>
>>>>  VLOG_DEFINE_THIS_MODULE(ofproto_dpif_upcall);
>>>>
>>>> +COVERAGE_DEFINE(upcall_duplicate_flow);
>>>> +
>>>>  /* A thread that reads upcalls from dpif, forwards each upcall's
>>>> packet,
>>>>   * and possibly sets up a kernel flow as a cache. */
>>>>  struct handler {
>>>> @@ -161,6 +163,7 @@ struct udpif_key {
>>>>      long long int created;         /* Estimation of creation time. */
>>>>
>>>>      bool mark;                     /* Used by mark and sweep GC
>>>> algorithm. */
>>>> +    bool flow_exists;              /* Ensures flows are only deleted
>>>> once. */
>>>>
>>>>
>>>
>>> Do we still need mark?  I think the function of 'mark' and 'flow_exists'
>>> is overlapped.
>>>
>>
> Strictly speaking, no. But I think there are a couple of benefits:
>
> (1) If they are separate, then we can detect that the flow was already
> handled and chosen to be kept:
>  * Revalidator finishes a round of revalidation. mark is set to false.
> flow_exists is true.
> * Consider when we dump a flow, revalidate it and choose to keep it. We
> set the mark to true. Flow_exists is true.
> * If we dump a duplicate of the flow, then "mark" will be true. This means
> that the flow was handled already. If it was just handled, there is
> unlikely to be any changes. We can skip execution.
>
> (2) If they are separate, then we know when the datapath has skipped a
> flow during flow_dump (if we've seen the flow before at all):
> * If a flow is dumped and we decide to delete it, "mark" and "flow_exists"
> will both be set to false.
> * If a flow is dumped and we decide to keep it, "mark" and "flow_exists"
> will both be set to true.
> * If the datapath doesn't dump a flow that does exist (due to some race
> condition), then "mark" will be false from previous revalidator_sweep() and
> "flow_exists" will be true.
> * Revalidator_sweep(): If a flow has "mark" set false and "flow_exists"
> set true, then we haven't seen the flow. Currently we delete such flows
> from the datapath, so that we can garbage collect the ukey and update the
> stats for that flow.
>
> Thanks for the review Alex.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140424/d484d56f/attachment-0005.html>


More information about the dev mailing list