[ovs-dev] [PATCH V2 2/2] bridge: Remove the 'Instant' stats.

[Alex Wang]

Hey Joe,


On Tue, Apr 15, 2014 at 10:31 PM, Alex Wang <alexw at nicira.com> wrote:

>
>
>
> On Tue, Apr 15, 2014 at 9:19 PM, Joe Stringer <joe at wand.net.nz> wrote:
>
>> (Apologies for the delay on this discussion)
>>
>> On 11 April 2014 16:54, Alex Wang <alexw at nicira.com> wrote:
>>
>>> On Thu, Apr 10, 2014 at 7:49 PM, Joe Stringer <joe at wand.net.nz> wrote:
>>>
>>>> On 11 April 2014 10:59, Alex Wang <alexw at nicira.com> wrote:
>>>>
>>>>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>>>>> index 04d5454..7f327b9 100644
>>>>> --- a/ofproto/ofproto-dpif.c
>>>>> +++ b/ofproto/ofproto-dpif.c
>>>>> @@ -375,6 +375,8 @@ ofproto_dpif_send_packet_in(struct ofproto_dpif
>>>>> *ofproto,
>>>>>          COVERAGE_INC(packet_in_overflow);
>>>>>          free(CONST_CAST(void *, pin->up.packet));
>>>>>          free(pin);
>>>>> +    } else {
>>>>> +        seq_change(connectivity_seq_get());
>>>>>      }
>>>>>  }
>>>>>
>>>>>
>>>> Why do we modify the connectivity status each time there is a packet_in?
>>>>
>>>
>>>
>>> This relates to your first summary point. ;D
>>> Originally, ofproto_dpif_send_packet_in() depends on ovs-vswitchd() be
>>> waken up every 100ms to
>>> schedule the transmission in connmgr module.  Now, since the 'Instant'
>>> stats logic is removed,
>>> we need to explicitly notify the global seq.  Otherwise, the main thread
>>> will keep sleeping.
>>>
>>> Found it in a unittest failure.
>>>
>>
>> Is there any reason that this is tied to the connectivity_seq, as opposed
>> to a 'struct seq' attached to the connmgr?
>>
>>>
>>
>
> The main reason is that the 'ofproto_dpif_send_packet_in()' only puts it
> in the 'ofproto->pins' list.  And in the run(), those 'pins'
> are given to connmgr via connmgr_send_packet_in().  So, connectivity_seq
> is to force a run().
>
> Another issue is that, we are doing rate-limiting for packet-ins in
> connmgr module.  My initial thought is that it will be complicated
> to involve 'struct seq'.  I'll dig more into it.
>


After checking the failed test, found there was actually a glitch that
caused a race.  So, with my change
the ovs-vswitchd thread is still waken up immeidately for sending
packet-ins, since we change the global
sequence number in 'ofproto_dpif_send_packet_in()'.  Thusly, with this fix:
http://openvswitch.org/pipermail/dev/2014-April/038991.html

there is no longer the packet-in issue.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140416/a9b2c475/attachment-0005.html>