On 22 Sep 2022, at 13:15, Adrian Moreno wrote:
> On 9/21/22 11:17, Eelco Chaudron wrote: >> >> >> On 20 Sep 2022, at 20:52, Aaron Conole wrote: >> >>> Eelco Chaudron <[email protected]> writes: >>> >>>> 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. >>>> >>>> Signed-off-by: Eelco Chaudron <[email protected]> >>>> --- >>> >>> Some of this work looks to be covered by the patch at >>> https://lists.linuxfoundation.org/pipermail/ovs-dev/2022-July/396545.html >>> >>> Maybe we should fold some of them together, since there seems to be some >>> overlap. >> >> I looked at this before, and both scripts cover a different use case, and >> the USDT probes could not be re-used. The only thing that could be re-used >> is the ovs_struct.h file :) >> >> My suggestion would be to keep both patches separated, as combining them >> would probably result in a series of two individual patches. >> >>>> v2: Added note that script only works a with single datapath configured. >>>> >>>> I'm planning a blog post to explain the Open vSwitch revalidator >>>> implementation, and this tool will help you determine what is >>>> happening in your system. Also, here is a link to an example >>>> of a real-time and overall plot. >>>> >>>> https://photos.app.goo.gl/rdx63zuFure7QE3t6 >>>> >>>> Documentation/topics/usdt-probes.rst | 84 +++ >>>> ofproto/ofproto-dpif-upcall.c | 11 >>>> utilities/automake.mk | 4 >>>> utilities/usdt-scripts/ovs_structs.h | 123 +++++ >>>> utilities/usdt-scripts/reval_monitor.py | 758 >>>> +++++++++++++++++++++++++++++++ >>>> 5 files changed, 979 insertions(+), 1 deletion(-) >>>> create mode 100644 utilities/usdt-scripts/ovs_structs.h >>>> create mode 100755 utilities/usdt-scripts/reval_monitor.py >>>> >>>> diff --git a/Documentation/topics/usdt-probes.rst >>>> b/Documentation/topics/usdt-probes.rst >>>> index 7ce19aaed..bc250e723 100644 >>>> --- a/Documentation/topics/usdt-probes.rst >>>> +++ b/Documentation/topics/usdt-probes.rst >>>> @@ -214,6 +214,10 @@ Available probes in ``ovs_vswitchd``: >>>> - dpif_recv:recv_upcall >>>> - main:poll_block >>>> - main:run_start >>>> +- revalidate_ukey\_\_:entry >>>> +- revalidate_ukey\_\_:exit >>>> +- udpif_revalidator:start_dump >>>> +- udpif_revalidator:sweep_done >>>> >>>> >>>> dpif_netlink_operate\_\_:op_flow_del >>>> @@ -327,6 +331,7 @@ probe main:run_start >>>> ~~~~~~~~~~~~~~~~~~~~ >>>> >>>> **Description**: >>>> + >>>> The ovs-vswitchd's main process contains a loop that runs every time >>>> some work >>>> needs to be done. This probe gets triggered every time the loop starts >>>> from the >>>> beginning. See also the ``main:poll_block`` probe below. >>>> @@ -344,6 +349,7 @@ probe main:poll_block >>>> ~~~~~~~~~~~~~~~~~~~~~ >>>> >>>> **Description**: >>>> + >>>> The ovs-vswitchd's main process contains a loop that runs every time >>>> some work >>>> needs to be done. This probe gets triggered every time the loop is done, >>>> and >>>> it's about to wait for being re-started by a poll_block() call returning. >>>> @@ -358,6 +364,84 @@ See also the ``main:run_start`` probe above. >>>> - ``utilities/usdt-scripts/bridge_loop.bt`` >>>> >>>> >>>> +revalidate_ukey\_\_:entry >>>> +~~~~~~~~~~~~~~~~~~~~~~~~~ >>>> + >>>> +**Description**: >>>> + >>>> +This probe gets triggered on entry of the revalidate_ukey__() function. >>>> + >>>> +**Arguments**: >>>> + >>>> +- *arg0*: ``(struct udpif *) udpif`` >>>> +- *arg1*: ``(struct udpif_key *) ukey`` >>>> +- *arg2*: ``(uint16_t) tcp_flags`` >>>> +- *arg3*: ``(struct ofpbuf *) odp_actions`` >>>> +- *arg4*: ``(struct recirc_refs *) recircs`` >>>> +- *arg5*: ``(struct xlate_cache *) xcache`` >>>> + >>>> +**Script references**: >>>> + >>>> +- ``utilities/usdt-scripts/reval_monitor.py`` >>>> + >>>> + >>>> +revalidate_ukey\_\_:exit >>>> +~~~~~~~~~~~~~~~~~~~~~~~~ >>>> + >>>> +**Description**: >>>> + >>>> +This probe gets triggered right before the revalidate_ukey__() function >>>> exits. >>>> + >>>> +**Arguments**: >>>> + >>>> +- *arg0*: ``(struct udpif *) udpif`` >>>> +- *arg1*: ``(struct udpif_key *) ukey`` >>>> +- *arg2*: ``(enum reval_result) result`` >>>> + >>>> +**Script references**: >>>> + >>>> +*None* >>> >>> Is it a good idea to add something that isn't referenced? >> >> I think it is. To be honest, we should add way more USDT probes in OVS that >> can be used by a developer to debug specific issues. >> >>> For example, >>> your scripts are meant to help understand what happens during >>> revalidation. This symbol isn't referenced in your script, and it makes >>> me think that it isn't particularly useful. >> >> The purpose of these scripts is for the end-user to get some insight into >> the system. But the more important reason for adding the script is that a >> developer has a framework he can quickly modify to debug specific issues. >> >> This probe, for example, can be used to measure the cumulative time spent >> doing revalidation. I had this added to the script, but it caused some >> confusion, which for end-users we should avoid. >> > > For these entry and exit probes, have you considered using uprobe and > uretprobe? They'll be definitely available if compiled with "-g". And even if > these particular ones are not visible in stripped binaries maybe there's some > compilation attribute we can add to make it visible. It’s inlined, so it’s stripped, including some of the variables. This is why we should use the USDT macro, which does all the compiler magic, generally at to cost of a NOP. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
