On Mon, Jan 23, 2023 at 11:28:45AM +0100, Eelco Chaudron wrote:
>
>
> On 19 Jan 2023, at 11:38, Simon Horman wrote:
>
> > On Tue, Jan 17, 2023 at 10:19:23AM +0100, Eelco Chaudron wrote:
> >> This patch adds a Python script that can be used to analyze the
> >> revalidator runs by providing statistics (including some real time
> >> graphs).
> >>
> >> The USDT events can also be captured to a file and used for
> >> later offline analysis.
> >>
> >> The following blog explains the Open vSwitch revalidator
> >> implementation and how this tool can help you understand what is
> >> happening in your system.
> >>
> >> https://developers.redhat.com/articles/2022/10/19/open-vswitch-revalidator-process-explained
> >>
> >> Signed-off-by: Eelco Chaudron <echau...@redhat.com>
> >
> > Thanks Eelco,
> >
> > this looks very nice.
> >
> > A few minor nits from my side, but that not withstanding.
> >
> > Acked-by: Simon Horman <simon.hor...@corigine.com>
> >
>
> Thanks for looking into this Simon!
>
> I will send out a v8 with some of your changes, and one addition to
> auto-generate the ENUMS so the are synced between eBPF and python.
>
> See inline comments below…
Thanks, much appreciated.
> >> +def _process_event(event):
> >> + global graph
> >> +
> >> + if export_file is not None:
> >> + export_file.write("event = {}\n".format(event_to_dict(event)))
> >> +
> >> + if event.id == 0 and not state['running']:
> >> + start = state["last_start"]
> >> + done = state["last_done"]
> >> + if done and start:
> >> + actual_wait = (event.ts - done.ts) / 1000000
> >> + csv = "{}, {}, {}, {}, {}, {}, {}, {}, {}, {:.2f}".format(
> >> + start.ts, done.ts, done.n_flows, graph.ukey_count,
> >> + done.avg_n_flows, done.max_n_flows, done.flow_limit,
> >> + done.dump_duration, done.poll_wait, actual_wait)
> >> +
> >> + if graph.base_time == 0:
> >> + graph = graph._replace(base_time=done.ts)
> >> +
> >> + graph.time.append((done.ts - graph.base_time) / 1000000000)
> >> + graph.n_flows.append(done.n_flows)
> >> + graph.n_reval_flows.append(graph.ukey_count)
> >> + graph.avg_n_flows.append(done.avg_n_flows)
> >> + graph.max_n_flows.append(done.max_n_flows)
> >> + graph.flow_limit.append(done.flow_limit)
> >> + graph.dump_duration.append(done.dump_duration)
> >> + graph.poll_wait.append(done.poll_wait)
> >> + graph.actual_wait.append(actual_wait)
> >> +
> >> + if not options.no_gui and not options.no_realtime_plots:
> >> + updated_graph = dynamic_plot_update(
> >> + graph, refresh=options.update_interval)
> >> + if updated_graph is None:
> >> + raise KeyboardInterrupt
> >> + graph = updated_graph
> >> +
> >> + if options.compress_output:
> >> + last_csv = state["last_csv"]
> >> + if not last_csv or \
> >> + csv.split(",")[2:-1] != last_csv.split(",")[2:-1] or \
> >> + abs((event.ts - done.ts) / 1000000 - done.poll_wait) >
> >> 100:
> >> + print(csv)
> >> + else:
> >> + state["last_not_printed_csv"] = csv
> >> +
> >> + state["last_csv"] = csv
> >> + else:
> >> + print(csv)
> >
> > nit: maybe it is nicer to store csv as a tuple and provide
> > a helper to print it? (Maybe not :)
>
> I’ve decided to keep it as is for now, as else we need to define the tuple
> instead of the string so there will not be much difference.
Sure, this was the suggestion that I was most unsure about.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
