[ovs-dev] [PATCH V2] bfd: Change the update of forwarding flag.

[Joe Stringer]

This one gets curlier the more I look at it. I think that the
behaviour of this patch is fine against master, but it does not tackle
the original, subtle interaction with my global connectivity_seq
patches. I'll try to address the issue in context of those changes
below.

Currently, there are several places that keep bfd->forwarding up to date:-
* bfd_process_packet(), on bfd rx.
* bfd_run(), (ie, bfd tx), when the bfd state is up, but the
detect_timer has expired. (But not when the bfd state is down and
forwarding_if_rx is enabled)
* port_run(), when we check the current port status to help determine
whether we can enable bonds
* instant_stats_run(), when we poll the port for the bfd status and
update the database.

The global seq patchset disables the last two cases unless there have
been bfd status changes. Consider the following state of the world:-
* forwarding_if_rx is enabled
* BFD is down
* We have received packets, so bfd->forwarding remains true
* We stop receiving packets

I believe that in this case, the sensible place to update
bfd->forwarding is in bfd_run(), however this is not currently
happening. I suggest ensuring that bfd_forwarding__() happens in every
bfd_run(), not only when the detect_timer expires.

On 10 December 2013 10:09, Joe Stringer <joestringer at nicira.com> wrote:
> There is a minor test failure with this latest version and my patches,
> but I am happy that this due to problems with the testsuite and not
> this patch. I'll send an update for my relevant patch.
>
> On 9 December 2013 17:36, Alex Wang <alexw at nicira.com> wrote:
>> The only thing is I didn't follow is:
>>
>> """
>> Do we really still need the bfd_forwarding_if_rx_update() function?
>> It's so short and only called in this one place, could we just move
>> it's code there?
>> """
>>
>> Since there are two other places where bfd_forwarding_if_rx_update() are
>> called, and I think they are necessary.
>>
>> Thanks,
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
>>