On 23 Jan 2023, at 11:32, Simon Horman wrote:

> 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 <[email protected]>
>>>
>>> Thanks Eelco,
>>>
>>> this looks very nice.
>>>
>>> A few minor nits from my side, but that not withstanding.
>>>
>>> Acked-by: Simon Horman <[email protected]>
>>>
>>
>> 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.

V8 is out…

//Eelco

>>>> +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
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to