On 12/5/23 10:10, Ales Musil wrote: > Add unixctl command called "ofctrl/flow-install-time" > that returns the last time it took OvS to process > and install all flows. The initial time is taken right > before controller queues the updates to rconn. > The end is marked when we receive barrier reply with > corresponding xid. > > Reported-at: https://issues.redhat.com/browse/FDP-134 > Signed-off-by: Ales Musil <amu...@redhat.com> > --- > controller/ofctrl.c | 31 +++++++++++++++++++++++++++++-- > 1 file changed, 29 insertions(+), 2 deletions(-)
Hi Ales, Thanks for the patch! This is supposed to be a rather "stable" command as the CMS wants to rely on this to gather relevant load statistics. We should make sure we document its behavior in ovn-controller.8.xml and mention it in the NEWS file. > > diff --git a/controller/ofctrl.c b/controller/ofctrl.c > index 7aac0128b..4f8b80012 100644 > --- a/controller/ofctrl.c > +++ b/controller/ofctrl.c > @@ -51,6 +51,7 @@ > #include "openvswitch/rconn.h" > #include "socket-util.h" > #include "timeval.h" > +#include "unixctl.h" > #include "util.h" > #include "vswitch-idl.h" > > @@ -323,6 +324,7 @@ struct ofctrl_flow_update { > struct ovs_list list_node; /* In 'flow_updates'. */ > ovs_be32 xid; /* OpenFlow transaction ID for barrier. */ > uint64_t req_cfg; /* Requested sequence number. */ > + long long start_msec; /* Timestamp when the update started. */ > }; > > static struct ofctrl_flow_update * > @@ -402,6 +404,10 @@ static enum mf_field_id mff_ovn_geneve; > * (e.g. after OVS restart). */ > static bool ofctrl_initial_clear; > > +/* The time in ms it took for the last flow installation to be processed > + * by OvS. */ > +static long long last_flow_install_time_ms = 0; > + > static ovs_be32 queue_msg(struct ofpbuf *); > > static struct ofpbuf *encode_flow_mod(struct ofputil_flow_mod *); > @@ -413,6 +419,8 @@ static struct ofpbuf *encode_meter_mod(const struct > ofputil_meter_mod *); > static void ovn_installed_flow_table_clear(void); > static void ovn_installed_flow_table_destroy(void); > > +static void flow_install_time_report(struct unixctl_conn *conn, int argc, > + const char *argv[], void *param); > > static void ofctrl_recv(const struct ofp_header *, enum ofptype); > > @@ -429,6 +437,8 @@ ofctrl_init(struct ovn_extend_table *group_table, > groups = group_table; > meters = meter_table; > shash_init(&meter_bands); > + unixctl_command_register("ofctrl/flow-install-time", "", 0, 0, > + flow_install_time_report, NULL); > } > > /* S_NEW, for a new connection. > @@ -732,6 +742,9 @@ recv_S_UPDATE_FLOWS(const struct ofp_header *oh, enum > ofptype type, > if (fup->req_cfg >= cur_cfg) { > cur_cfg = fup->req_cfg; > } > + > + last_flow_install_time_ms = time_msec() - fup->start_msec; Nit: very unlikely but this can underflow. Nit2: too many empty lines IMO. I'd just move this line immediately before the "if (fup->req_cfg >= cur_cfg) {". > + > mem_stats.oflow_update_usage -= ofctrl_flow_update_size(fup); > ovs_list_remove(&fup->list_node); > free(fup); > @@ -2900,6 +2913,7 @@ ofctrl_put(struct ovn_desired_flow_table *lflow_table, > const struct ofp_header *oh = barrier->data; > ovs_be32 xid_ = oh->xid; > ovs_list_push_back(&msgs, &barrier->list_node); > + long long flow_update_start = time_msec(); > > /* Queue the messages. */ > struct ofpbuf *msg; > @@ -2945,9 +2959,13 @@ ofctrl_put(struct ovn_desired_flow_table *lflow_table, > > /* Add a flow update. */ > fup = xmalloc(sizeof *fup); > + *fup = (struct ofctrl_flow_update) { > + .xid = xid_, > + .req_cfg = req_cfg, > + .start_msec = flow_update_start, > + }; > + > ovs_list_push_back(&flow_updates, &fup->list_node); > - fup->xid = xid_; > - fup->req_cfg = req_cfg; > mem_stats.oflow_update_usage += ofctrl_flow_update_size(fup); > done:; > } else if (!ovs_list_is_empty(&flow_updates)) { > @@ -3090,3 +3108,12 @@ ofctrl_get_memory_usage(struct simap *usage) > ROUND_UP(rconn_packet_counter_n_bytes(tx_counter), 1024) > / 1024); > } > + > +static void > +flow_install_time_report(struct unixctl_conn *conn, int argc OVS_UNUSED, > + const char *argv[] OVS_UNUSED, void *param > OVS_UNUSED) > +{ > + char *msg = xasprintf("%lld ms", last_flow_install_time_ms); > + unixctl_command_reply(conn, msg); > + free(msg); > +} Thanks, Dumitru _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev