| Message ID | 20210706144149.3553-1-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] controller: Add stopwatch to measure OF update duration. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
Acked-by: Mark Michelson <mmichels@redhat.com> On 7/6/21 10:41 AM, Dumitru Ceara wrote: > Also, shorten the CONTROLLER_LOOP_STOPWATCH_NAME name as there is a bug > in lib/stopwatch.c which fails to report an error when the stopwatch > name is longer than 32 characters. CONTROLLER_LOOP_STOPWATCH_NAME was > getting very close to that and future commits might mimic the long name > and happen to go over the limit. > > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- > Note: The OVS lib/stopwatch.c implementation should also be fixed to > report an error (or even assert) if the name supplied to > stopwatch_create() is longer than 32 characters. But that's out of the > scope of this patch. > --- > controller/ovn-controller.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 9050380f3..6a9c25f28 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -93,7 +93,8 @@ static unixctl_cb_func debug_delay_nb_cfg_report; > #define DEFAULT_PROBE_INTERVAL_MSEC 5000 > #define OFCTRL_DEFAULT_PROBE_INTERVAL_SEC 0 > > -#define CONTROLLER_LOOP_STOPWATCH_NAME "ovn-controller-flow-generation" > +#define CONTROLLER_LOOP_STOPWATCH_NAME "flow-generation" > +#define OFCTRL_PUT_STOPWATCH_NAME "flow-installation" > > #define OVS_NB_CFG_NAME "ovn-nb-cfg" > > @@ -2845,6 +2846,7 @@ main(int argc, char *argv[]) > update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL, false); > > stopwatch_create(CONTROLLER_LOOP_STOPWATCH_NAME, SW_MS); > + stopwatch_create(OFCTRL_PUT_STOPWATCH_NAME, SW_MS); > > /* Define inc-proc-engine nodes. */ > ENGINE_NODE_CUSTOM_DATA(ct_zones, "ct_zones"); > @@ -3292,6 +3294,8 @@ main(int argc, char *argv[]) > pflow_output_data = engine_get_data(&en_pflow_output); > if (lflow_output_data && pflow_output_data && > ct_zones_data) { > + stopwatch_start(OFCTRL_PUT_STOPWATCH_NAME, > + time_msec()); > ofctrl_put(&lflow_output_data->flow_table, > &pflow_output_data->flow_table, > &ct_zones_data->pending, > @@ -3299,6 +3303,7 @@ main(int argc, char *argv[]) > ofctrl_seqno_get_req_cfg(), > engine_node_changed(&en_lflow_output), > engine_node_changed(&en_pflow_output)); > + stopwatch_stop(OFCTRL_PUT_STOPWATCH_NAME, time_msec()); > } > ofctrl_seqno_run(ofctrl_get_cur_cfg()); > if_status_mgr_run(if_mgr, binding_data, !ovnsb_idl_txn, >
I pushed this to master. On 7/8/21 4:27 PM, Mark Michelson wrote: > Acked-by: Mark Michelson <mmichels@redhat.com> > > On 7/6/21 10:41 AM, Dumitru Ceara wrote: >> Also, shorten the CONTROLLER_LOOP_STOPWATCH_NAME name as there is a bug >> in lib/stopwatch.c which fails to report an error when the stopwatch >> name is longer than 32 characters. CONTROLLER_LOOP_STOPWATCH_NAME was >> getting very close to that and future commits might mimic the long name >> and happen to go over the limit. >> >> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >> --- >> Note: The OVS lib/stopwatch.c implementation should also be fixed to >> report an error (or even assert) if the name supplied to >> stopwatch_create() is longer than 32 characters. But that's out of the >> scope of this patch. >> --- >> controller/ovn-controller.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c >> index 9050380f3..6a9c25f28 100644 >> --- a/controller/ovn-controller.c >> +++ b/controller/ovn-controller.c >> @@ -93,7 +93,8 @@ static unixctl_cb_func debug_delay_nb_cfg_report; >> #define DEFAULT_PROBE_INTERVAL_MSEC 5000 >> #define OFCTRL_DEFAULT_PROBE_INTERVAL_SEC 0 >> -#define CONTROLLER_LOOP_STOPWATCH_NAME "ovn-controller-flow-generation" >> +#define CONTROLLER_LOOP_STOPWATCH_NAME "flow-generation" >> +#define OFCTRL_PUT_STOPWATCH_NAME "flow-installation" >> #define OVS_NB_CFG_NAME "ovn-nb-cfg" >> @@ -2845,6 +2846,7 @@ main(int argc, char *argv[]) >> update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL, false); >> stopwatch_create(CONTROLLER_LOOP_STOPWATCH_NAME, SW_MS); >> + stopwatch_create(OFCTRL_PUT_STOPWATCH_NAME, SW_MS); >> /* Define inc-proc-engine nodes. */ >> ENGINE_NODE_CUSTOM_DATA(ct_zones, "ct_zones"); >> @@ -3292,6 +3294,8 @@ main(int argc, char *argv[]) >> pflow_output_data = >> engine_get_data(&en_pflow_output); >> if (lflow_output_data && pflow_output_data && >> ct_zones_data) { >> + stopwatch_start(OFCTRL_PUT_STOPWATCH_NAME, >> + time_msec()); >> ofctrl_put(&lflow_output_data->flow_table, >> &pflow_output_data->flow_table, >> &ct_zones_data->pending, >> @@ -3299,6 +3303,7 @@ main(int argc, char *argv[]) >> ofctrl_seqno_get_req_cfg(), >> >> engine_node_changed(&en_lflow_output), >> >> engine_node_changed(&en_pflow_output)); >> + stopwatch_stop(OFCTRL_PUT_STOPWATCH_NAME, >> time_msec()); >> } >> ofctrl_seqno_run(ofctrl_get_cur_cfg()); >> if_status_mgr_run(if_mgr, binding_data, >> !ovnsb_idl_txn, >> >
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 9050380f3..6a9c25f28 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -93,7 +93,8 @@ static unixctl_cb_func debug_delay_nb_cfg_report; #define DEFAULT_PROBE_INTERVAL_MSEC 5000 #define OFCTRL_DEFAULT_PROBE_INTERVAL_SEC 0 -#define CONTROLLER_LOOP_STOPWATCH_NAME "ovn-controller-flow-generation" +#define CONTROLLER_LOOP_STOPWATCH_NAME "flow-generation" +#define OFCTRL_PUT_STOPWATCH_NAME "flow-installation" #define OVS_NB_CFG_NAME "ovn-nb-cfg" @@ -2845,6 +2846,7 @@ main(int argc, char *argv[]) update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL, false); stopwatch_create(CONTROLLER_LOOP_STOPWATCH_NAME, SW_MS); + stopwatch_create(OFCTRL_PUT_STOPWATCH_NAME, SW_MS); /* Define inc-proc-engine nodes. */ ENGINE_NODE_CUSTOM_DATA(ct_zones, "ct_zones"); @@ -3292,6 +3294,8 @@ main(int argc, char *argv[]) pflow_output_data = engine_get_data(&en_pflow_output); if (lflow_output_data && pflow_output_data && ct_zones_data) { + stopwatch_start(OFCTRL_PUT_STOPWATCH_NAME, + time_msec()); ofctrl_put(&lflow_output_data->flow_table, &pflow_output_data->flow_table, &ct_zones_data->pending, @@ -3299,6 +3303,7 @@ main(int argc, char *argv[]) ofctrl_seqno_get_req_cfg(), engine_node_changed(&en_lflow_output), engine_node_changed(&en_pflow_output)); + stopwatch_stop(OFCTRL_PUT_STOPWATCH_NAME, time_msec()); } ofctrl_seqno_run(ofctrl_get_cur_cfg()); if_status_mgr_run(if_mgr, binding_data, !ovnsb_idl_txn,
Also, shorten the CONTROLLER_LOOP_STOPWATCH_NAME name as there is a bug in lib/stopwatch.c which fails to report an error when the stopwatch name is longer than 32 characters. CONTROLLER_LOOP_STOPWATCH_NAME was getting very close to that and future commits might mimic the long name and happen to go over the limit. Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- Note: The OVS lib/stopwatch.c implementation should also be fixed to report an error (or even assert) if the name supplied to stopwatch_create() is longer than 32 characters. But that's out of the scope of this patch. --- controller/ovn-controller.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)