diff mbox series

[ovs-dev] controller: introduce coverage_counters for ovn-controller incremental processing

Message ID a26e5f960bfa1876d017fa85df74a65c90d254ea.1613152096.git.lorenzo.bianconi@redhat.com
State Changes Requested
Headers show
Series [ovs-dev] controller: introduce coverage_counters for ovn-controller incremental processing | expand

Commit Message

Lorenzo Bianconi Feb. 12, 2021, 5:49 p.m. UTC
In order to help understanding system behaviour for debugging purpose,
introduce coverage counters for ovn-controller I-P engine.

https://bugzilla.redhat.com/show_bug.cgi?id=1890902
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 controller/ovn-controller.c | 77 +++++++++++++++++++++++++++++++++++--
 lib/inc-proc-eng.h          | 13 +++++++
 2 files changed, 87 insertions(+), 3 deletions(-)

Comments

Mark Gray Feb. 16, 2021, 3:01 p.m. UTC | #1
Thanks Lorenzo, looks really useful.

In the subject line, should there be an underscore between coverage and
counters?

On 12/02/2021 17:49, Lorenzo Bianconi wrote:
> In order to help understanding system behaviour for debugging purpose,
> introduce coverage counters for ovn-controller I-P engine.

You may need to clarify the behaviour somewhere in the code and/or the
commit message. When reviewing, I initially assumed that the coverage
counters are measuring how often the functions are getting called (which
is what I was expecting). However, you are actually measuring a behavior
of the incremental engine. It was not immediately obvious to me. Maybe
update the commit message and add a comment in the #defines in
'inc-proc-eng.h'?
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1890902
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---
>  controller/ovn-controller.c | 77 +++++++++++++++++++++++++++++++++++--
>  lib/inc-proc-eng.h          | 13 +++++++
>  2 files changed, 87 insertions(+), 3 deletions(-)
> 
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 61b809593..0650fe353 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -17,6 +17,8 @@
>  
>  #include "ovn-controller.h"
> 
> +#include "coverage.h"

Move this down to the other #includes?
> +
>  #include <errno.h>
>  #include <getopt.h>
>  #include <signal.h>
> @@ -85,6 +87,30 @@ static unixctl_cb_func lflow_cache_flush_cmd;
>  static unixctl_cb_func lflow_cache_show_stats_cmd;
>  static unixctl_cb_func debug_delay_nb_cfg_report;
>  
> +ENGINE_DEF_COVERAGE(flow_output);
> +ENGINE_CHANGE_COVERAGE(flow_output_runtime_data);
> +ENGINE_CHANGE_COVERAGE(flow_output_addr_sets);
> +ENGINE_CHANGE_COVERAGE(flow_output_port_groups);
> +ENGINE_CHANGE_COVERAGE(flow_output_physical_flow_changes);
> +ENGINE_CHANGE_COVERAGE(flow_output_sb_multicast_group);
> +ENGINE_CHANGE_COVERAGE(flow_output_sb_port_binding);
> +ENGINE_CHANGE_COVERAGE(flow_output_sb_mac_binding);
> +ENGINE_CHANGE_COVERAGE(flow_output_sb_logical_flow);
> +ENGINE_CHANGE_COVERAGE(flow_output_sb_load_balancer);
> +
> +ENGINE_DEF_COVERAGE(runtime_data);
> +ENGINE_CHANGE_COVERAGE(runtime_data_sb_datapath_binding);
> +ENGINE_CHANGE_COVERAGE(runtime_data_sb_port_binding);
> +
> +ENGINE_DEF_COVERAGE(addr_sets);
> +ENGINE_CHANGE_COVERAGE(addr_sets_sb_address_set);
> +
> +ENGINE_DEF_COVERAGE(port_groups);
> +ENGINE_DEF_COVERAGE(ct_zones);
> +ENGINE_DEF_COVERAGE(mff_ovn_geneve);
> +ENGINE_DEF_COVERAGE(ofctrl_is_connected);
> +ENGINE_DEF_COVERAGE(physical_flow_changes);
> +
>  #define DEFAULT_BRIDGE_NAME "br-int"
>  #define DEFAULT_PROBE_INTERVAL_MSEC 5000
>  #define OFCTRL_DEFAULT_PROBE_INTERVAL_SEC 0
> @@ -944,6 +970,10 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>      SB_NODE(dns, "dns") \
>      SB_NODE(load_balancer, "load_balancer")
>  
> +#define SB_NODE(NAME, NAME_STR) ENGINE_DEF_COVERAGE(sb_##NAME);
> +    SB_NODES
> +#undef SB_NODE
> +
>  enum sb_engine_node {
>  #define SB_NODE(NAME, NAME_STR) SB_##NAME,
>      SB_NODES
> @@ -961,6 +991,10 @@ enum sb_engine_node {
>      OVS_NODE(interface, "interface") \
>      OVS_NODE(qos, "qos")
>  
> +#define OVS_NODE(NAME, NAME_STR) ENGINE_DEF_COVERAGE(ovs_##NAME);
> +    OVS_NODES
> +#undef OVS_NODE
> +
>  enum ovs_engine_node {
>  #define OVS_NODE(NAME, NAME_STR) OVS_##NAME,
>      OVS_NODES
> @@ -1000,6 +1034,7 @@ en_ofctrl_is_connected_run(struct engine_node *node, void *data)
>              ofctrl_seqno_flush();
>              binding_seqno_flush();
>          }
> +        ENGINE_COVERAGE_INC(ofctrl_is_connected);
>          engine_set_node_state(node, EN_UPDATED);
>          return;
>      }
> @@ -1056,6 +1091,7 @@ en_addr_sets_run(struct engine_node *node, void *data)
>      addr_sets_init(as_table, &as->addr_sets);
>  
>      as->change_tracked = false;
> +    ENGINE_COVERAGE_INC(addr_sets);
>      engine_set_node_state(node, EN_UPDATED);
>  }
>  
> @@ -1083,6 +1119,7 @@ addr_sets_sb_address_set_handler(struct engine_node *node, void *data)
>      }
>  
>      as->change_tracked = true;
> +    ENGINE_CHANGE_COVERAGE_INC(addr_sets_sb_address_set);
>      return true;
>  }
>  
> @@ -1136,6 +1173,7 @@ en_port_groups_run(struct engine_node *node, void *data)
>      port_groups_init(pg_table, &pg->port_groups);
>  
>      pg->change_tracked = false;
> +    ENGINE_COVERAGE_INC(port_groups);
>      engine_set_node_state(node, EN_UPDATED);
>  }
>  
> @@ -1471,6 +1509,7 @@ en_runtime_data_run(struct engine_node *node, void *data)
>  
>      binding_run(&b_ctx_in, &b_ctx_out);
>  
> +    ENGINE_COVERAGE_INC(runtime_data);
>      engine_set_node_state(node, EN_UPDATED);
>  }
>  
> @@ -1519,6 +1558,7 @@ runtime_data_sb_port_binding_handler(struct engine_node *node, void *data)
>              !hmap_is_empty(b_ctx_out.tracked_dp_bindings)) {
>          engine_set_node_state(node, EN_UPDATED);
>      }
> +    ENGINE_CHANGE_COVERAGE_INC(runtime_data_sb_port_binding);
>  
>      return true;
>  }
> @@ -1541,6 +1581,7 @@ runtime_data_sb_datapath_binding_handler(struct engine_node *node OVS_UNUSED,
>              }
>          }
>      }> +    ENGINE_CHANGE_COVERAGE_INC(runtime_data_sb_datapath_binding);>
>      return true;
>  }
> @@ -1593,7 +1634,7 @@ en_ct_zones_run(struct engine_node *node, void *data)
>                      &ct_zones_data->current, ct_zones_data->bitmap,
>                      &ct_zones_data->pending, &rt_data->ct_updated_datapaths);
>  
> -
> +    ENGINE_COVERAGE_INC(ct_zones);
>      engine_set_node_state(node, EN_UPDATED);
>  }
>  
> @@ -1629,6 +1670,7 @@ en_mff_ovn_geneve_run(struct engine_node *node, void *data)
>      if (ed_mff_ovn_geneve->mff_ovn_geneve != mff_ovn_geneve) {
>          ed_mff_ovn_geneve->mff_ovn_geneve = mff_ovn_geneve;
>          engine_set_node_state(node, EN_UPDATED);
> +        ENGINE_COVERAGE_INC(mff_ovn_geneve);
>          return;
>      }
>      engine_set_node_state(node, EN_UNCHANGED);
> @@ -1703,6 +1745,7 @@ en_physical_flow_changes_run(struct engine_node *node, void *data)
>  {
>      struct ed_type_pfc_data *pfc_tdata = data;
>      pfc_tdata->recompute_physical_flows = true;
> +    ENGINE_COVERAGE_INC(physical_flow_changes);
>      engine_set_node_state(node, EN_UPDATED);
>  }
>  
> @@ -2023,7 +2066,7 @@ en_flow_output_run(struct engine_node *node, void *data)
>      init_physical_ctx(node, rt_data, &p_ctx);
>  
>      physical_run(&p_ctx, &fo->flow_table);
> -
> +    ENGINE_COVERAGE_INC(flow_output);
>      engine_set_node_state(node, EN_UPDATED);

Maybe you could update the 'engine_set_node_state' macro to
automatically increment using the __func__ constant and some fancy
templating. Then this would get automatically added anytime added a new
node?
>  }
>  
> @@ -2049,6 +2092,9 @@ flow_output_sb_logical_flow_handler(struct engine_node *node, void *data)
>      bool handled = lflow_handle_changed_flows(&l_ctx_in, &l_ctx_out);
>  
>      engine_set_node_state(node, EN_UPDATED);
> +    if (handled) {
> +        ENGINE_CHANGE_COVERAGE_INC(flow_output_sb_logical_flow);
> +    }
>      return handled;
>  }
>  
> @@ -2075,6 +2121,7 @@ flow_output_sb_mac_binding_handler(struct engine_node *node, void *data)
>              mac_binding_table, local_datapaths, flow_table);
>  
>      engine_set_node_state(node, EN_UPDATED);
> +    ENGINE_CHANGE_COVERAGE_INC(flow_output_sb_mac_binding);
>      return true;
>  }
>  
> @@ -2098,6 +2145,7 @@ flow_output_sb_port_binding_handler(struct engine_node *node,
>      physical_handle_port_binding_changes(&p_ctx, flow_table);
>  
>      engine_set_node_state(node, EN_UPDATED);
> +    ENGINE_CHANGE_COVERAGE_INC(flow_output_sb_port_binding);
>      return true;
>  }
>  
> @@ -2115,6 +2163,7 @@ flow_output_sb_multicast_group_handler(struct engine_node *node, void *data)
>  
>      physical_handle_mc_group_changes(&p_ctx, flow_table);
>  
> +    ENGINE_CHANGE_COVERAGE_INC(flow_output_sb_multicast_group);
>      engine_set_node_state(node, EN_UPDATED);
>      return true;
>  
> @@ -2124,6 +2173,7 @@ static bool
>  _flow_output_resource_ref_handler(struct engine_node *node, void *data,
>                                    enum ref_type ref_type)
>  {
> +    bool update_cc = false;
>      struct ed_type_runtime_data *rt_data =
>          engine_get_input_data("runtime_data", node);
>  
> @@ -2197,6 +2247,7 @@ _flow_output_resource_ref_handler(struct engine_node *node, void *data,
>          }
>          if (changed) {
>              engine_set_node_state(node, EN_UPDATED);
> +            update_cc = true;
>          }
>      }
>      SSET_FOR_EACH (ref_name, updated) {
> @@ -2206,6 +2257,7 @@ _flow_output_resource_ref_handler(struct engine_node *node, void *data,
>          }
>          if (changed) {
>              engine_set_node_state(node, EN_UPDATED);
> +            update_cc = true;
>          }
>      }
>      SSET_FOR_EACH (ref_name, new) {
> @@ -2215,6 +2267,21 @@ _flow_output_resource_ref_handler(struct engine_node *node, void *data,
>          }
>          if (changed) {
>              engine_set_node_state(node, EN_UPDATED);
> +            update_cc = true;
> +        }
> +    }
> +
> +    if (update_cc) {
> +        switch (ref_type) {
> +        case REF_TYPE_ADDRSET:
> +            ENGINE_CHANGE_COVERAGE_INC(flow_output_addr_sets);
> +            break;
> +        case REF_TYPE_PORTGROUP:
> +            ENGINE_CHANGE_COVERAGE_INC(flow_output_port_groups);
> +            break;
> +        case REF_TYPE_PORTBINDING:
> +        default:
> +            break;
>          }
>      }
>  
> @@ -2262,7 +2329,7 @@ flow_output_physical_flow_changes_handler(struct engine_node *node, void *data)
>          physical_run(&p_ctx, &fo->flow_table);
>          return true;
>      }
> -
> +    ENGINE_CHANGE_COVERAGE_INC(flow_output_physical_flow_changes);
>      return true;
>  }
>  
> @@ -2314,6 +2381,7 @@ flow_output_runtime_data_handler(struct engine_node *node,
>          }
>      }
>  
> +    ENGINE_CHANGE_COVERAGE_INC(flow_output_runtime_data);

Its not in this diff but there is another engine_set_node_state() in
this function. Perhaps you can update the engine_set_node_state macro so
it always updates coverage?

>      engine_set_node_state(node, EN_UPDATED);
>      return true;
>  }
> @@ -2332,6 +2400,9 @@ flow_output_sb_load_balancer_handler(struct engine_node *node, void *data)
>      bool handled = lflow_handle_changed_lbs(&l_ctx_in, &l_ctx_out);
>  
>      engine_set_node_state(node, EN_UPDATED);
> +    if (handled) {
> +        ENGINE_CHANGE_COVERAGE_INC(flow_output_sb_load_balancer);
> +    }
>      return handled;
>  }
>  
> diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h
> index 857234677..93e85cbb4 100644
> --- a/lib/inc-proc-eng.h
> +++ b/lib/inc-proc-eng.h
> @@ -312,6 +312,7 @@ en_##DB_NAME##_##TBL_NAME##_run(struct engine_node *node, \
>          EN_OVSDB_GET(node); \
>      if (DB_NAME##rec_##TBL_NAME##_table_track_get_first(table)) { \
>          engine_set_node_state(node, EN_UPDATED); \
> +        ENGINE_COVERAGE_INC(DB_NAME##_##TBL_NAME); \
>          return; \
>      } \
>      engine_set_node_state(node, EN_UNCHANGED); \
> @@ -352,4 +353,16 @@ static void en_##DB_NAME##_##TBL_NAME##_cleanup(void *data OVS_UNUSED) \
>  #define ENGINE_NODE_OVS(TBL_NAME, TBL_NAME_STR) \
>      ENGINE_NODE_OVSDB(ovs, "OVS", TBL_NAME, TBL_NAME_STR);
>  
> +#define ENGINE_DEF_COVERAGE(NAME)   \

The naming convention looks inconsistent here? Should it be
'ENGINE_RUN_COVERAGE_DEF' or maybe even 'ENGINE_RUN_COVERAGE_DEFINE'
(better)? wdyt?

> +    COVERAGE_DEFINE(en_##NAME##_run);
> +
> +#define ENGINE_COVERAGE_INC(NAME)   \
> +    COVERAGE_INC(en_##NAME##_run);
> +
> +#define ENGINE_CHANGE_COVERAGE(NAME) \
> +    COVERAGE_DEFINE(en_##NAME##_handler);

Here could be 'ENGINE_HANDLER_COVERAGE_DEFINE'. It is a bit confusing to
me as I am unsure what 'CHANGE' means.
> +
> +#define ENGINE_CHANGE_COVERAGE_INC(NAME) \
> +    COVERAGE_INC(en_##NAME##_handler);
> +
>  #endif /* lib/inc-proc-eng.h */
>
diff mbox series

Patch

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 61b809593..0650fe353 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -17,6 +17,8 @@ 
 
 #include "ovn-controller.h"
 
+#include "coverage.h"
+
 #include <errno.h>
 #include <getopt.h>
 #include <signal.h>
@@ -85,6 +87,30 @@  static unixctl_cb_func lflow_cache_flush_cmd;
 static unixctl_cb_func lflow_cache_show_stats_cmd;
 static unixctl_cb_func debug_delay_nb_cfg_report;
 
+ENGINE_DEF_COVERAGE(flow_output);
+ENGINE_CHANGE_COVERAGE(flow_output_runtime_data);
+ENGINE_CHANGE_COVERAGE(flow_output_addr_sets);
+ENGINE_CHANGE_COVERAGE(flow_output_port_groups);
+ENGINE_CHANGE_COVERAGE(flow_output_physical_flow_changes);
+ENGINE_CHANGE_COVERAGE(flow_output_sb_multicast_group);
+ENGINE_CHANGE_COVERAGE(flow_output_sb_port_binding);
+ENGINE_CHANGE_COVERAGE(flow_output_sb_mac_binding);
+ENGINE_CHANGE_COVERAGE(flow_output_sb_logical_flow);
+ENGINE_CHANGE_COVERAGE(flow_output_sb_load_balancer);
+
+ENGINE_DEF_COVERAGE(runtime_data);
+ENGINE_CHANGE_COVERAGE(runtime_data_sb_datapath_binding);
+ENGINE_CHANGE_COVERAGE(runtime_data_sb_port_binding);
+
+ENGINE_DEF_COVERAGE(addr_sets);
+ENGINE_CHANGE_COVERAGE(addr_sets_sb_address_set);
+
+ENGINE_DEF_COVERAGE(port_groups);
+ENGINE_DEF_COVERAGE(ct_zones);
+ENGINE_DEF_COVERAGE(mff_ovn_geneve);
+ENGINE_DEF_COVERAGE(ofctrl_is_connected);
+ENGINE_DEF_COVERAGE(physical_flow_changes);
+
 #define DEFAULT_BRIDGE_NAME "br-int"
 #define DEFAULT_PROBE_INTERVAL_MSEC 5000
 #define OFCTRL_DEFAULT_PROBE_INTERVAL_SEC 0
@@ -944,6 +970,10 @@  ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
     SB_NODE(dns, "dns") \
     SB_NODE(load_balancer, "load_balancer")
 
+#define SB_NODE(NAME, NAME_STR) ENGINE_DEF_COVERAGE(sb_##NAME);
+    SB_NODES
+#undef SB_NODE
+
 enum sb_engine_node {
 #define SB_NODE(NAME, NAME_STR) SB_##NAME,
     SB_NODES
@@ -961,6 +991,10 @@  enum sb_engine_node {
     OVS_NODE(interface, "interface") \
     OVS_NODE(qos, "qos")
 
+#define OVS_NODE(NAME, NAME_STR) ENGINE_DEF_COVERAGE(ovs_##NAME);
+    OVS_NODES
+#undef OVS_NODE
+
 enum ovs_engine_node {
 #define OVS_NODE(NAME, NAME_STR) OVS_##NAME,
     OVS_NODES
@@ -1000,6 +1034,7 @@  en_ofctrl_is_connected_run(struct engine_node *node, void *data)
             ofctrl_seqno_flush();
             binding_seqno_flush();
         }
+        ENGINE_COVERAGE_INC(ofctrl_is_connected);
         engine_set_node_state(node, EN_UPDATED);
         return;
     }
@@ -1056,6 +1091,7 @@  en_addr_sets_run(struct engine_node *node, void *data)
     addr_sets_init(as_table, &as->addr_sets);
 
     as->change_tracked = false;
+    ENGINE_COVERAGE_INC(addr_sets);
     engine_set_node_state(node, EN_UPDATED);
 }
 
@@ -1083,6 +1119,7 @@  addr_sets_sb_address_set_handler(struct engine_node *node, void *data)
     }
 
     as->change_tracked = true;
+    ENGINE_CHANGE_COVERAGE_INC(addr_sets_sb_address_set);
     return true;
 }
 
@@ -1136,6 +1173,7 @@  en_port_groups_run(struct engine_node *node, void *data)
     port_groups_init(pg_table, &pg->port_groups);
 
     pg->change_tracked = false;
+    ENGINE_COVERAGE_INC(port_groups);
     engine_set_node_state(node, EN_UPDATED);
 }
 
@@ -1471,6 +1509,7 @@  en_runtime_data_run(struct engine_node *node, void *data)
 
     binding_run(&b_ctx_in, &b_ctx_out);
 
+    ENGINE_COVERAGE_INC(runtime_data);
     engine_set_node_state(node, EN_UPDATED);
 }
 
@@ -1519,6 +1558,7 @@  runtime_data_sb_port_binding_handler(struct engine_node *node, void *data)
             !hmap_is_empty(b_ctx_out.tracked_dp_bindings)) {
         engine_set_node_state(node, EN_UPDATED);
     }
+    ENGINE_CHANGE_COVERAGE_INC(runtime_data_sb_port_binding);
 
     return true;
 }
@@ -1541,6 +1581,7 @@  runtime_data_sb_datapath_binding_handler(struct engine_node *node OVS_UNUSED,
             }
         }
     }
+    ENGINE_CHANGE_COVERAGE_INC(runtime_data_sb_datapath_binding);
 
     return true;
 }
@@ -1593,7 +1634,7 @@  en_ct_zones_run(struct engine_node *node, void *data)
                     &ct_zones_data->current, ct_zones_data->bitmap,
                     &ct_zones_data->pending, &rt_data->ct_updated_datapaths);
 
-
+    ENGINE_COVERAGE_INC(ct_zones);
     engine_set_node_state(node, EN_UPDATED);
 }
 
@@ -1629,6 +1670,7 @@  en_mff_ovn_geneve_run(struct engine_node *node, void *data)
     if (ed_mff_ovn_geneve->mff_ovn_geneve != mff_ovn_geneve) {
         ed_mff_ovn_geneve->mff_ovn_geneve = mff_ovn_geneve;
         engine_set_node_state(node, EN_UPDATED);
+        ENGINE_COVERAGE_INC(mff_ovn_geneve);
         return;
     }
     engine_set_node_state(node, EN_UNCHANGED);
@@ -1703,6 +1745,7 @@  en_physical_flow_changes_run(struct engine_node *node, void *data)
 {
     struct ed_type_pfc_data *pfc_tdata = data;
     pfc_tdata->recompute_physical_flows = true;
+    ENGINE_COVERAGE_INC(physical_flow_changes);
     engine_set_node_state(node, EN_UPDATED);
 }
 
@@ -2023,7 +2066,7 @@  en_flow_output_run(struct engine_node *node, void *data)
     init_physical_ctx(node, rt_data, &p_ctx);
 
     physical_run(&p_ctx, &fo->flow_table);
-
+    ENGINE_COVERAGE_INC(flow_output);
     engine_set_node_state(node, EN_UPDATED);
 }
 
@@ -2049,6 +2092,9 @@  flow_output_sb_logical_flow_handler(struct engine_node *node, void *data)
     bool handled = lflow_handle_changed_flows(&l_ctx_in, &l_ctx_out);
 
     engine_set_node_state(node, EN_UPDATED);
+    if (handled) {
+        ENGINE_CHANGE_COVERAGE_INC(flow_output_sb_logical_flow);
+    }
     return handled;
 }
 
@@ -2075,6 +2121,7 @@  flow_output_sb_mac_binding_handler(struct engine_node *node, void *data)
             mac_binding_table, local_datapaths, flow_table);
 
     engine_set_node_state(node, EN_UPDATED);
+    ENGINE_CHANGE_COVERAGE_INC(flow_output_sb_mac_binding);
     return true;
 }
 
@@ -2098,6 +2145,7 @@  flow_output_sb_port_binding_handler(struct engine_node *node,
     physical_handle_port_binding_changes(&p_ctx, flow_table);
 
     engine_set_node_state(node, EN_UPDATED);
+    ENGINE_CHANGE_COVERAGE_INC(flow_output_sb_port_binding);
     return true;
 }
 
@@ -2115,6 +2163,7 @@  flow_output_sb_multicast_group_handler(struct engine_node *node, void *data)
 
     physical_handle_mc_group_changes(&p_ctx, flow_table);
 
+    ENGINE_CHANGE_COVERAGE_INC(flow_output_sb_multicast_group);
     engine_set_node_state(node, EN_UPDATED);
     return true;
 
@@ -2124,6 +2173,7 @@  static bool
 _flow_output_resource_ref_handler(struct engine_node *node, void *data,
                                   enum ref_type ref_type)
 {
+    bool update_cc = false;
     struct ed_type_runtime_data *rt_data =
         engine_get_input_data("runtime_data", node);
 
@@ -2197,6 +2247,7 @@  _flow_output_resource_ref_handler(struct engine_node *node, void *data,
         }
         if (changed) {
             engine_set_node_state(node, EN_UPDATED);
+            update_cc = true;
         }
     }
     SSET_FOR_EACH (ref_name, updated) {
@@ -2206,6 +2257,7 @@  _flow_output_resource_ref_handler(struct engine_node *node, void *data,
         }
         if (changed) {
             engine_set_node_state(node, EN_UPDATED);
+            update_cc = true;
         }
     }
     SSET_FOR_EACH (ref_name, new) {
@@ -2215,6 +2267,21 @@  _flow_output_resource_ref_handler(struct engine_node *node, void *data,
         }
         if (changed) {
             engine_set_node_state(node, EN_UPDATED);
+            update_cc = true;
+        }
+    }
+
+    if (update_cc) {
+        switch (ref_type) {
+        case REF_TYPE_ADDRSET:
+            ENGINE_CHANGE_COVERAGE_INC(flow_output_addr_sets);
+            break;
+        case REF_TYPE_PORTGROUP:
+            ENGINE_CHANGE_COVERAGE_INC(flow_output_port_groups);
+            break;
+        case REF_TYPE_PORTBINDING:
+        default:
+            break;
         }
     }
 
@@ -2262,7 +2329,7 @@  flow_output_physical_flow_changes_handler(struct engine_node *node, void *data)
         physical_run(&p_ctx, &fo->flow_table);
         return true;
     }
-
+    ENGINE_CHANGE_COVERAGE_INC(flow_output_physical_flow_changes);
     return true;
 }
 
@@ -2314,6 +2381,7 @@  flow_output_runtime_data_handler(struct engine_node *node,
         }
     }
 
+    ENGINE_CHANGE_COVERAGE_INC(flow_output_runtime_data);
     engine_set_node_state(node, EN_UPDATED);
     return true;
 }
@@ -2332,6 +2400,9 @@  flow_output_sb_load_balancer_handler(struct engine_node *node, void *data)
     bool handled = lflow_handle_changed_lbs(&l_ctx_in, &l_ctx_out);
 
     engine_set_node_state(node, EN_UPDATED);
+    if (handled) {
+        ENGINE_CHANGE_COVERAGE_INC(flow_output_sb_load_balancer);
+    }
     return handled;
 }
 
diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h
index 857234677..93e85cbb4 100644
--- a/lib/inc-proc-eng.h
+++ b/lib/inc-proc-eng.h
@@ -312,6 +312,7 @@  en_##DB_NAME##_##TBL_NAME##_run(struct engine_node *node, \
         EN_OVSDB_GET(node); \
     if (DB_NAME##rec_##TBL_NAME##_table_track_get_first(table)) { \
         engine_set_node_state(node, EN_UPDATED); \
+        ENGINE_COVERAGE_INC(DB_NAME##_##TBL_NAME); \
         return; \
     } \
     engine_set_node_state(node, EN_UNCHANGED); \
@@ -352,4 +353,16 @@  static void en_##DB_NAME##_##TBL_NAME##_cleanup(void *data OVS_UNUSED) \
 #define ENGINE_NODE_OVS(TBL_NAME, TBL_NAME_STR) \
     ENGINE_NODE_OVSDB(ovs, "OVS", TBL_NAME, TBL_NAME_STR);
 
+#define ENGINE_DEF_COVERAGE(NAME)   \
+    COVERAGE_DEFINE(en_##NAME##_run);
+
+#define ENGINE_COVERAGE_INC(NAME)   \
+    COVERAGE_INC(en_##NAME##_run);
+
+#define ENGINE_CHANGE_COVERAGE(NAME) \
+    COVERAGE_DEFINE(en_##NAME##_handler);
+
+#define ENGINE_CHANGE_COVERAGE_INC(NAME) \
+    COVERAGE_INC(en_##NAME##_handler);
+
 #endif /* lib/inc-proc-eng.h */