Pushed to master and branch-2.1.
On 24 April 2014 11:06, Joe Stringer <joestrin...@nicira.com> wrote:
> On 24 April 2014 04:56, Alex Wang <al...@nicira.com> wrote:
>
>>
>> On Tue, Apr 22, 2014 at 10:24 PM, Alex Wang <al...@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.
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
