[ovs-dev] [PATCH] Granular link health statistics for cfm.

[Ben Pfaff]

On Wed, Apr 04, 2012 at 06:46:20PM -0700, Mehak Mahajan wrote:
> The changes display the cfm_health of an interface.  The cfm_health
> is an exponential weighted moving average of the health of all
> remote_mpids.  The value can vary from 0 to 100, 100 being very healthy
> and 0 being unhealthy.
> 
> Feature #10363
> Requested-by: Ethan Jackson <ethan at nicira.com>
> Signed-off-by: Mehak Mahajan <mmahajan at nicira.com>

Thanks.

I think that the initial value of cfm->health should be -1, because at
the point when the CFM object is created we don't yet have any
information about its health (and won't for some time).

There's an extra blank line here:
+        if (cfm->health_interval == CFM_HEALTH_INTERVAL) {
+
+            /* Calculate the cfm health of the interface.  If the number of

I think you can skip adding the assignment here because we use
xzalloc():
             if (hmap_count(&cfm->remote_mps) < CFM_MAX_RMPS) {
                 rmp = xzalloc(sizeof *rmp);
                 hmap_insert(&cfm->remote_mps, &rmp->node, hash_mpid(ccm_mpid));
+                rmp->num_health_ccm = 0;

I think that the comment on cfm_get_health() should mention the
possibility of a -1 return value.

Perhaps cfm_print_details() should print -1 as "undefined"?

vswitchd/bridge.c should declare cfm_health as "write-only" with:
    ovsdb_idl_omit_alert(idl, &ovsrec_port_col_cfm_health);

vswitch.xml: s/more that/more than/

Thanks,

Ben.