Right---there's two separate issues; One is that flaps are only
counted when logging is on, and the other is that faults are only
logged when the connection flaps (rather than on any difference in
error). Currently, old_cfm_fault is a bool while cfm->fault is not;
this makes it harder to follow.
I propose keeping the patch as is, then following up with another for
the second issue:-
--- a/lib/cfm.c
+++ b/lib/cfm.c
@@ -395,7 +395,7 @@ cfm_run(struct cfm *cfm) OVS_EXCLUDED(mutex)
if (timer_expired(&cfm->fault_timer)) {
long long int interval = cfm_fault_interval(cfm);
struct remote_mp *rmp, *rmp_next;
- bool old_cfm_fault = cfm->fault;
+ enum cfm_fault_reason old_cfm_fault = cfm->fault;
bool demand_override;
bool rmp_set_opup = false;
bool rmp_set_opdown = false;
@@ -481,7 +481,9 @@ cfm_run(struct cfm *cfm) OVS_EXCLUDED(mutex)
}
if (old_cfm_fault != cfm->fault) {
- cfm->flap_count++;
+ if (old_cfm_fault == 0 || cfm->fault == 0) {
+ cfm->flap_count++;
+ }
if (!VLOG_DROP_INFO(&rl)) {
struct ds ds = DS_EMPTY_INITIALIZER;
On 6 November 2013 09:41, Alex Wang <[email protected]> wrote:
> Thanks very much for spotting this.
>
> Just one thing,
>
>> Signed-off-by: Joe Stringer <[email protected]>
>> ---
>> ---
>> lib/cfm.c | 27 +++++++++++++--------------
>> 1 file changed, 13 insertions(+), 14 deletions(-)
>>
>> diff --git a/lib/cfm.c b/lib/cfm.c
>> index d256a5f..01c9a8c 100644
>> --- a/lib/cfm.c
>> +++ b/lib/cfm.c
>> @@ -480,20 +480,19 @@ cfm_run(struct cfm *cfm) OVS_EXCLUDED(mutex)
>> cfm->fault |= CFM_FAULT_RECV;
>> }
>>
>> - if (old_cfm_fault != cfm->fault && !VLOG_DROP_INFO(&rl)) {
>> - struct ds ds = DS_EMPTY_INITIALIZER;
>> -
>> - ds_put_cstr(&ds, "from [");
>> - ds_put_cfm_fault(&ds, old_cfm_fault);
>> - ds_put_cstr(&ds, "] to [");
>> - ds_put_cfm_fault(&ds, cfm->fault);
>> - ds_put_char(&ds, ']');
>> - VLOG_INFO("%s: CFM faults changed %s.", cfm->name,
>> ds_cstr(&ds));
>> - ds_destroy(&ds);
>> -
>> - /* If there is a flap, increments the counter. */
>> - if (old_cfm_fault == false || cfm->fault == false) {
>> - cfm->flap_count++;
>> + if (old_cfm_fault != cfm->fault) {
>> + cfm->flap_count++;
>> +
>> + if (!VLOG_DROP_INFO(&rl)) {
>> + struct ds ds = DS_EMPTY_INITIALIZER;
>> +
>> + ds_put_cstr(&ds, "from [");
>> + ds_put_cfm_fault(&ds, old_cfm_fault);
>> + ds_put_cstr(&ds, "] to [");
>> + ds_put_cfm_fault(&ds, cfm->fault);
>> + ds_put_char(&ds, ']');
>> + VLOG_INFO("%s: CFM faults changed %s.", cfm->name,
>> ds_cstr(&ds));
>> + ds_destroy(&ds);
>> }
>> }
>
>
>
> We still want to log the "fault <-> fault" change (though it is not counted
> as a flap). So, could you move the "if (!VLOG_DROP_INFO(&rl)) " outside?
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
