Re: [ovs-dev] [PATCH v10 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.

2024-03-08 Thread Eelco Chaudron 



On 5 Mar 2024, at 16:44, Aaron Conole wrote:

> From: Kevin Sprague 
>
> During normal operations, it is useful to understand when a particular flow
> gets removed from the system. This can be useful when debugging performance
> issues tied to ofproto flow changes, trying to determine deployed traffic
> patterns, or while debugging dynamic systems where ports come and go.
>
> Prior to this change, there was a lack of visibility around flow expiration.
> The existing debugging infrastructure could tell us when a flow was added to
> the datapath, but not when it was removed or why.
>
> This change introduces a USDT probe at the point where the revalidator
> determines that the flow should be removed.  Additionally, we track the
> reason for the flow eviction and provide that information as well.  With
> this change, we can track the complete flow lifecycle for the netlink
> datapath by hooking the upcall tracepoint in kernel, the flow put USDT, and
> the revalidator USDT, letting us watch as flows are added and removed from
> the kernel datapath.
>
> This change only enables this information via USDT probe, so it won't be
> possible to access this information any other way (see:
> Documentation/topics/usdt-probes.rst).
>
> Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py)
> which serves as a demonstration of how the new USDT probe might be used
> going forward.
>
> Acked-by: Han Zhou 
> Signed-off-by: Kevin Sprague 
> Co-authored-by: Aaron Conole 
> Signed-off-by: Aaron Conole 

Thanks Aaron for following this up. One small nit, but none blocking, maybe can 
fix it on commit.

Acked-by: Eelco Chaudron 

> ---
> v8 -> v9: Reorganized the flow delete reasons enum
>   Updated flow_reval_monitor to use pahole to extract fields
>   Added the purge reason with a proper USDT point
>   Updated documentation
>   Dropped all the outstanding ACKs
>
> v9 -> v10: Reorder the usdt arguments
>Rewrite delete comment
>Added python docstrings to functions
>Use ternary assignment
>Clean up example output comment
>Capitolize 'F' in Flow
>Snip out the comments including help text
>Shrink try / except blocks for the USDT probes
>refactor the print()/sys.exit() into a single call that uses
>sys.stderr
>Re-run black & flake8
>
>
>  Documentation/topics/usdt-probes.rst |  43 +
>  ofproto/ofproto-dpif-upcall.c|  44 +-
>  utilities/automake.mk|   3 +
>  utilities/usdt-scripts/flow_reval_monitor.py | 978 +++
>  4 files changed, 1062 insertions(+), 6 deletions(-)
>  create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py
>
> diff --git a/Documentation/topics/usdt-probes.rst 
> b/Documentation/topics/usdt-probes.rst
> index e527f43bab..b9a6c54b29 100644
> --- a/Documentation/topics/usdt-probes.rst
> +++ b/Documentation/topics/usdt-probes.rst
> @@ -214,8 +214,10 @@ Available probes in ``ovs_vswitchd``:
>  - dpif_recv:recv_upcall
>  - main:poll_block
>  - main:run_start
> +- revalidate:flow_result
>  - revalidate_ukey\_\_:entry
>  - revalidate_ukey\_\_:exit
> +- revalidator_sweep\_\_:flow_result
>  - udpif_revalidator:start_dump
>  - udpif_revalidator:sweep_done
>
> @@ -443,6 +445,47 @@ sweep phase was completed.
>  - ``utilities/usdt-scripts/reval_monitor.py``
>
>
> +probe revalidate:flow_result
> +
> +
> +**Description**:
> +This probe is triggered when the revalidator has executed on a particular
> +flow key to make a determination whether to evict a flow, and the cause
> +for eviction.  The revalidator runs periodically, and this probe will only
> +be triggered when a flow is flagged for revalidation.
> +
> +**Arguments**:
> +
> +- *arg0*: ``(struct udpif *) udpif``
> +- *arg1*: ``(struct udpif_key *) ukey``
> +- *arg2*: ``(enum reval_result) result``
> +- *arg3*: ``(enum flow_del_reason) del_reason``
> +
> +**Script references**:
> +
> +- ``utilities/usdt-scripts/flow_reval_monitor.py``
> +
> +
> +probe revalidator_sweep\_\_:flow_result
> +~~~
> +
> +**Description**:
> +This probe is placed in the path of the revalidator sweep, and is executed
> +under the condition that a flow entry is in an unexpected state, or the
> +flows were asked to be purged due to a user action.
> +
> +**Arguments**:
> +
> +- *arg0*: ``(struct udpif *) udpif``
> +- *arg1*: ``(struct udpif_key *) ukey``
> +- *arg2*: ``(enum reval_result) result``
> +- *arg3*: ``(enum flow_del_reason) del_reason``
> +
> +**Script references**:
> +
> +- ``utilities/usdt-scripts/flow_reval_monitor.py``
> +
> +
>  Adding your own probes
>  --
>
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 9a5c5c29ce..d881956366 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -269,6

[ovs-dev] [PATCH v10 1/2] revalidator: Add a USDT probe during flow deletion with purge reason.

2024-03-05 Thread Aaron Conole 
From: Kevin Sprague 

During normal operations, it is useful to understand when a particular flow
gets removed from the system. This can be useful when debugging performance
issues tied to ofproto flow changes, trying to determine deployed traffic
patterns, or while debugging dynamic systems where ports come and go.

Prior to this change, there was a lack of visibility around flow expiration.
The existing debugging infrastructure could tell us when a flow was added to
the datapath, but not when it was removed or why.

This change introduces a USDT probe at the point where the revalidator
determines that the flow should be removed.  Additionally, we track the
reason for the flow eviction and provide that information as well.  With
this change, we can track the complete flow lifecycle for the netlink
datapath by hooking the upcall tracepoint in kernel, the flow put USDT, and
the revalidator USDT, letting us watch as flows are added and removed from
the kernel datapath.

This change only enables this information via USDT probe, so it won't be
possible to access this information any other way (see:
Documentation/topics/usdt-probes.rst).

Also included is a script (utilities/usdt-scripts/flow_reval_monitor.py)
which serves as a demonstration of how the new USDT probe might be used
going forward.

Acked-by: Han Zhou 
Signed-off-by: Kevin Sprague 
Co-authored-by: Aaron Conole 
Signed-off-by: Aaron Conole 
---
v8 -> v9: Reorganized the flow delete reasons enum
  Updated flow_reval_monitor to use pahole to extract fields
  Added the purge reason with a proper USDT point
  Updated documentation
  Dropped all the outstanding ACKs

v9 -> v10: Reorder the usdt arguments
   Rewrite delete comment
   Added python docstrings to functions
   Use ternary assignment
   Clean up example output comment
   Capitolize 'F' in Flow
   Snip out the comments including help text
   Shrink try / except blocks for the USDT probes
   refactor the print()/sys.exit() into a single call that uses
   sys.stderr
   Re-run black & flake8


 Documentation/topics/usdt-probes.rst |  43 +
 ofproto/ofproto-dpif-upcall.c|  44 +-
 utilities/automake.mk|   3 +
 utilities/usdt-scripts/flow_reval_monitor.py | 978 +++
 4 files changed, 1062 insertions(+), 6 deletions(-)
 create mode 100755 utilities/usdt-scripts/flow_reval_monitor.py

diff --git a/Documentation/topics/usdt-probes.rst 
b/Documentation/topics/usdt-probes.rst
index e527f43bab..b9a6c54b29 100644
--- a/Documentation/topics/usdt-probes.rst
+++ b/Documentation/topics/usdt-probes.rst
@@ -214,8 +214,10 @@ Available probes in ``ovs_vswitchd``:
 - dpif_recv:recv_upcall
 - main:poll_block
 - main:run_start
+- revalidate:flow_result
 - revalidate_ukey\_\_:entry
 - revalidate_ukey\_\_:exit
+- revalidator_sweep\_\_:flow_result
 - udpif_revalidator:start_dump
 - udpif_revalidator:sweep_done
 
@@ -443,6 +445,47 @@ sweep phase was completed.
 - ``utilities/usdt-scripts/reval_monitor.py``
 
 
+probe revalidate:flow_result
+
+
+**Description**:
+This probe is triggered when the revalidator has executed on a particular
+flow key to make a determination whether to evict a flow, and the cause
+for eviction.  The revalidator runs periodically, and this probe will only
+be triggered when a flow is flagged for revalidation.
+
+**Arguments**:
+
+- *arg0*: ``(struct udpif *) udpif``
+- *arg1*: ``(struct udpif_key *) ukey``
+- *arg2*: ``(enum reval_result) result``
+- *arg3*: ``(enum flow_del_reason) del_reason``
+
+**Script references**:
+
+- ``utilities/usdt-scripts/flow_reval_monitor.py``
+
+
+probe revalidator_sweep\_\_:flow_result
+~~~
+
+**Description**:
+This probe is placed in the path of the revalidator sweep, and is executed
+under the condition that a flow entry is in an unexpected state, or the
+flows were asked to be purged due to a user action.
+
+**Arguments**:
+
+- *arg0*: ``(struct udpif *) udpif``
+- *arg1*: ``(struct udpif_key *) ukey``
+- *arg2*: ``(enum reval_result) result``
+- *arg3*: ``(enum flow_del_reason) del_reason``
+
+**Script references**:
+
+- ``utilities/usdt-scripts/flow_reval_monitor.py``
+
+
 Adding your own probes
 --
 
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 9a5c5c29ce..d881956366 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -269,6 +269,20 @@ enum ukey_state {
 };
 #define N_UKEY_STATES (UKEY_DELETED + 1)
 
+enum flow_del_reason {
+FDR_NONE = 0,   /* No deletion reason for the flow. */
+FDR_AVOID_CACHING,  /* Flow deleted to avoid caching. */
+FDR_BAD_ODP_FIT,/* The flow had a bad ODP flow fit. */
+FDR_FLOW_IDLE,  /* The flow went unused and was deleted. */
+FDR_FLOW_LIMIT, /* All flows